Xen devel - Jun 2007 - [PATCH 0/4] [Net] Support accelerated network plugin modules

This is a repost of some earlier patches to the xen-devel mailing list,
with a number of changes thanks to some useful suggestions from others.

I''ve also CC''d netdev@vger.kernel.org at Herbert Xu''s
request as some of
the files being patched may be merged into upstream linux soon, and so
folks there may have
opinions too.

Changes from last time:
 - Improved the link between frontend and accelerated plugin so that
netif_wake_queue is only called when both have available TX slots.
Thanks to Herbert Xu for pointing this out.
 - Removed macro that safely called the accelerated hooks; this code is
now inline in the relevant places. 
 - Some small cleanup modifications, and resync patches to latest xen-
unstable.hg, including source tree layout changes for linux-2.6.18-
xen.hg

The discussion following the previous posting of these patches was
mainly concerned with the approach to locking in the frontend.  Some
felt it was too complex and suggested using RCU instead.  However, I
have been unable to convince myself that RCU would offer suitable
protection against the accelerated plugin module unloading in the middle
of a hook call, particularly where these hook calls might block.  The
existing locking has been well tested and is therefore known to work.
In addition, the complexity is largely to ensure there is low locking
overhead on the data path, and so a change to simplify it would not
likely increase performance.  As a result the locking has not changed
substantially since the last patch.  However if others feel strongly
about this and can convince me that RCU would be adequate, I wouldn''t
object to making this change.

What follows is the text from the original post providing some
background on the patches:

This set of patches provides the hooks and support necessary for
accelerated network plugin modules to attach to Xen''s netback and
netfront.  These modules provide a fast path for network traffic where
there is hardware support available for the netfront driver to send and
receive packets directly to a NIC (such as those available from
Solarflare).

As there are currently no available plugins, I''ve attached a couple of
dummy ones to illustrate how the hooks could be used.  These are
incomplete (and clearly wouldn''t even compile) in that they only
include
code to show the interface between the accelerated module and
netfront/netback.  A lot of the comments hint at what code should go
where.  They don''t show any interface between the accelerated frontend
and accelerated backend, or hardware access, for example, as those would
both be specific to the implementation.  I hope they help illustrate
this, but if you have any questions I''m happy to provide more
information.

A brief overview of the operation of the plugins:  When the accelerated
modules are loaded, a VI is created by the accelerated backend to allow
the accelerated frontend to safely access portions of the NIC.  For RX,
when packets are received by the accelerated backend, it will examine
them and if appropriate insert filters into the NIC to deliver future
packets on that address directly to the accelerated frontend''s VI.  For
TX, netfront gives each accelerated frontend the option of sending each
packet, which it can accept (if it wants to send it directly to the
hardware) or decline (if it thinks this is more appropriate to send via
the normal network path).

We have found that using this approach to accelerating network traffic,
domU to domU connections (across the network) can achieve close to the
performance of dom0 to dom0 connections on a 10Gbps ethernet.  This is
roughly double the bandwidth seen with unmodified Xen.




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 15/6/07 11:46, "Kieran Mansley" <kmansley@solarflare.com>
wrote:
> This is a repost of some earlier patches to the xen-devel mailing list,
> with a number of changes thanks to some useful suggestions from others.
The coding style needs fixing in various ways.

Hard tabs need to be used, no spaces inside brackets, but should include
spaces between if/while/for and bracket, and bracket and brace:
    if (foo) {
Not
    if( foo ){
    if(foo ) {
Or various other possibilities.

No use of the following please:
If (foo) return 1; else return 0;
Is clearer as:
Return foo;

There''s a (good) idiom for error handling in Linux:
If (!error) {
    page of code;
} else {
    oh no, error, print and return;
}
Should be changed to:
If (error) {
    handle error, bail if necessary
}
Page of code, no longer indented by error check;

Apart from the various coding style bits, accelerator_probe_vifs_on_load()
walks the vif_states list with no lock held. Is this safe against the list
changing?

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Friday 15 June 2007 13:26:03 Keir Fraser wrote:
> On 15/6/07 11:46, "Kieran Mansley"
<kmansley@solarflare.com> wrote:
> 
> > This is a repost of some earlier patches to the xen-devel mailing
list,
> > with a number of changes thanks to some useful suggestions from
others.
> 
> The coding style needs fixing in various ways.
> 
> Hard tabs need to be used, no spaces inside brackets, but should include
> spaces between if/while/for and bracket, and bracket and brace:
>     if (foo) {
> Not
>     if( foo ){
>     if(foo ) {
> Or various other possibilities.
> 
> No use of the following please:
> If (foo) return 1; else return 0;
> Is clearer as:
> Return foo;
But it''s not the same.
	return !!foo;
would be the same. And yes, it does matter:

int x(void)
{
	unsigned long long v = 0xFF0000000000ULL;
	/*foo*/
	return v;
}

-- 
Greetings Michael.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 15/6/07 13:11, "Michael Buesch" <mb@bu3sch.de> wrote:
>> No use of the following please:
>> If (foo) return 1; else return 0;
>> Is clearer as:
>> Return foo;
> 
> But it''s not the same.
> return !!foo;
> would be the same. And yes, it does matter:
True in general, but not the cases I''ve seen in this patchset, where
''foo''
is a predicate.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Friday 15 June 2007 14:20:34 Keir Fraser wrote:
> 
> On 15/6/07 13:11, "Michael Buesch" <mb@bu3sch.de> wrote:
> 
> >> No use of the following please:
> >> If (foo) return 1; else return 0;
> >> Is clearer as:
> >> Return foo;
> > 
> > But it''s not the same.
> > return !!foo;
> > would be the same. And yes, it does matter:
> 
> True in general, but not the cases I''ve seen in this patchset,
where ''foo''
> is a predicate.
Ok, if foo is a variable containing an error code, it''s
better to return that error code. I assumed that foo is a
variable containing some value (counter or something).

-- 
Greetings Michael.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 15/6/07 13:21, "Michael Buesch" <mb@bu3sch.de> wrote:
>> True in general, but not the cases I''ve seen in this patchset,
where ''foo''
>> is a predicate.
> 
> Ok, if foo is a variable containing an error code, it''s
> better to return that error code. I assumed that foo is a
> variable containing some value (counter or something).
I mean specifically things like:
 if (be->accelerator == NULL)
    return 1;
 else
    return 0;

 K.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

This is a another iteration of some earlier patches sent to the xen-
devel mailing list, with a number of changes thanks to some useful
suggestions from others.

I''ve also CC''d netdev@vger.kernel.org at Herbert Xu''s
request as some of
the files being patched may be merged into upstream linux soon, and so
folks there may have opinions too.

Major changes from last time:
 - Modify protection in frontend to use an atomic ref count to reduce
the number of spinlocks that are required, as suggested by Keir Fraser
and Zhu Han.  This change required an improvement to the protection of
the hooks when they are being installed for a second (or subsequent
time) to prevent the new copy being inserted before the old ones have
been completely finished with.

 - Move the majority of the acceleration code out of existing
netfront/netback source files and into separate accel.c source file in
each of those directories, as requested by Keir.  Unfortunately separate
header files don''t make a lot of sense due to mutual dependencies.

 - A number of coding style changes, again requested by Keir.  Apologies
for not getting this right first time.

What follows is the full description from the earlier posting, included
here for ease of access should anyone need them:

This set of patches provides the hooks and support necessary for
accelerated network plugin modules to attach to Xen''s netback and
netfront.  These modules provide a fast path for network traffic where
there is hardware support available for the netfront driver to send and
receive packets directly to a NIC (such as those available from
Solarflare).

As there are currently no available plugins, I''ve attached a couple of
dummy ones to illustrate how the hooks could be used.  These are
incomplete (and clearly wouldn''t even compile) in that they only
include
code to show the interface between the accelerated module and
netfront/netback.  A lot of the comments hint at what code should go
where.  They don''t show any interface between the accelerated frontend
and accelerated backend, or hardware access, for example, as those would
both be specific to the implementation.  I hope they help illustrate
this, but if you have any questions I''m happy to provide more
information.

A brief overview of the operation of the plugins:  When the accelerated
modules are loaded, a VI is created by the accelerated backend to allow
the accelerated frontend to safely access portions of the NIC.  For RX,
when packets are received by the accelerated backend, it will examine
them and if appropriate insert filters into the NIC to deliver future
packets on that address directly to the accelerated frontend''s VI.  For
TX, netfront gives each accelerated frontend the option of sending each
packet, which it can accept (if it wants to send it directly to the
hardware) or decline (if it thinks this is more appropriate to send via
the normal network path).

We have found that using this approach to accelerating network traffic,
domU to domU connections (across the network) can achieve close to the
performance of dom0 to dom0 connections on a 10Gbps ethernet.  This is
roughly double the bandwidth seen with unmodified Xen.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

This is a another iteration of some earlier patches sent to the xen-
devel mailing list.  This set of patches provides the hooks and support
necessary for accelerated network plugin modules to attach to Xen''s
netback and netfront.  These modules provide a fast path for network
traffic where there is hardware support available for the netfront
driver to send and receive packets directly to a NIC (such as those
available from Solarflare).

As there were no comments or objections to the last iteration a couple
of weeks ago, I''m hoping these can be applied to xen-unstable soon.

The changes since last time are relatively minor, consisting of some
modifications to the interface between netfront and the frontend
acceleration module to improve support for saving and restoring guest
domains.

Signed-off-by: Kieran Mansley <kmansley@solarflare.com>

Kieran


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel