Re: [Xen-devel] [PATCH 1 of 4 RFC] xl/remus : Network buffering setup helper functions

[Shriram Rajagopalan <rshriram@xxxxxxxxx>]

On Mon, Jul 29, 2013 at 11:42 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Thu, 2013-07-25 at 00:09 -0700, Shriram Rajagopalan wrote:
>> The functions contained in xl_netbuf.c are built on top of libnl3 API.
>> They setup the necessary infrastructure required to for network output
>> buffering in Remus.  In xend, the Remus python code performed this setup
>> by invoking shell commands.  This code is built purely on top of APIs
>> supplied by libnl3.0.
>>
>> There are two public helper functions:
>> 1) remus_install_netbuf_on_dev to install a network buffer on a vif.
>> Its basically equivalent to:
>> a)find free ifb (say ifb0)
>> b)ip link set ifb0 up
>> c)tc qdisc add dev vif1.0 ingress
>> d)tc filter add dev vif1.0 parent ffff: proto ip pref 10 u32 match u32 0 0 \
>>       action mirred egress redirect dev ifb0
>> e)tc qdisc add dev ifb0 root plug
>> f)get handle to plug qdisc and control it programmatically after suspending
>>   a VM/receiving checkpoint ack from remote
>>
>>
>> 2) remus_uninstall_netbufs to remove all
>> network buffers and ingress qdiscs from the supplied interface list.
>>
>> I have managed to code up 5 of the 6 steps mentioned above, purely in C, by 
>> building
>> on top of libnl3. There is currently no support in libnl3 to implement step 
>> (d) (redirection)
>> and it is pretty complicated to implement. So I am currently resorting to 
>> system() lib call
>> to get the job done.
>>
>> N.B. This implementation is just xl (cmdline utility) specific.
>
> You haven't included a Makefile change so I can't easily tell if this is
> really part of the xl binary, or a separate helper or indeed part of
> libxl.
>

The Makefile changes were clumped together in patch 4, along with changes to
configure.ac.

>> Other toolstacks may choose
>> to reuse this code or do this setup in their own way (scripts/shell 
>> commands, etc.).
>
> It's not clear to me why this code wouldn't be part of libxl to be
> called by xl and other toolstacks as needed. Why would a toolstack want
> to do this differently?
>
> The point of libxl is precisely to relieve toolstacks of the burden of
> doing these sorts of complex domain setup activities themselves by
> putting them in a common library, I can't see why Remus setup would be
> an exception to this.
>
> I think we need to answer this question before reviewing this code,
> since doing it libxl would change the requirements of the code (from the
> API exposed, to logging, to memory managment, to not using system
> directly).
>

 If every one had to do this in the same way, then I agree that this
code should be
moved into libxl.

The setup can be done using a a few lines of shell script or in a 100-200
line C code (like this patch).  Given that there is a choice, I didnt
see any point
in forcing all toolstacks to go the libxl way (resorting to its
internal c code for setting
up the buffers).  But if you think that we might as well make a sane
choice and abstract
this complexity from other toolstacks, I would be happy to push this
into libxl itself.

shriram

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel