[PATCH] Ethernet Bridging: Enable Hardware Checksumming

[PATCH] Ethernet Bridging: Enable Hardware Checksumming

[Jon Mason]

Currently, locally generated ethernet traffic does not take advantage of
hardware checksum offload when acting as a child device under a bridge
device.  This is because the upper layers do not see the available
features of the child devices only the features of the bridge device
(which is empty).  

There is an easy solution for this (see patch below), include hardware
checksum and scatter gather as features of the bridge device.  In the 
case that the physical ethernet device does not support scatter 
gather or hardware checksum, dev_queue_xmit() will check the
dev->features and do the necessary linearization and calculate the
checksum.

Signed-off-by: Jon Mason <jdmason@us.ibm.com>

--- net/bridge/br_device.c.orig	2005-05-13 11:23:02.552751024 -0500
+++ net/bridge/br_device.c	2005-05-13 11:25:39.155943720 -0500
@@ -101,4 +101,5 @@ void br_dev_setup(struct net_device *dev
 	dev->tx_queue_len = 0;
 	dev->set_mac_address = NULL;
 	dev->priv_flags = IFF_EBRIDGE;
+	dev->features = NETIF_F_HW_CSUM | NETIF_F_SG;
 }

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[Nivedita Singhvi]

Jon Mason wrote:

> Currently, locally generated ethernet traffic does not take advantage of
> hardware checksum offload when acting as a child device under a bridge
> device.  This is because the upper layers do not see the available
> features of the child devices only the features of the bridge device
> (which is empty).  
> 
> There is an easy solution for this (see patch below), include hardware
> checksum and scatter gather as features of the bridge device.  In the 
> case that the physical ethernet device does not support scatter 
> gather or hardware checksum, dev_queue_xmit() will check the
> dev->features and do the necessary linearization and calculate the
> checksum.
> 
> Signed-off-by: Jon Mason <jdmason@us.ibm.com>
> 
> --- net/bridge/br_device.c.orig	2005-05-13 11:23:02.552751024 -0500
> +++ net/bridge/br_device.c	2005-05-13 11:25:39.155943720 -0500
> @@ -101,4 +101,5 @@ void br_dev_setup(struct net_device *dev
>  	dev->tx_queue_len = 0;
>  	dev->set_mac_address = NULL;
>  	dev->priv_flags = IFF_EBRIDGE;
> +	dev->features = NETIF_F_HW_CSUM | NETIF_F_SG;
>  }

Stephen,

I think this is a good thing to have, gives us performance
gain (equivalent to using/not using checksum offload) and
has very little impact on the bridging layer, and is
independent of the virtualization stuff..

thanks,
Nivedita

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[Herbert Xu]

Jon Mason <jdmason@us.ibm.com> wrote:
> 
> checksum and scatter gather as features of the bridge device.  In the 
> case that the physical ethernet device does not support scatter 
> gather or hardware checksum, dev_queue_xmit() will check the
> dev->features and do the necessary linearization and calculate the
> checksum.

Unfortunately skb_linearize is a lot more expensive than not
generating the non-linear skb's in the first place.

So this is going to hurt people using the bridge devices over
physical devices that don't support SG.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[David S. Miller]

From: Nivedita Singhvi <niv@us.ibm.com>
Date: Wed, 18 May 2005 17:03:51 -0700

> I think this is a good thing to have, gives us performance
> gain (equivalent to using/not using checksum offload) and
> has very little impact on the bridging layer, and is
> independent of the virtualization stuff..

I agree but keep in mind that when bridging over non-checksumming
devices it will be more expensive.  Especially for TCP.

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[Nivedita Singhvi]

Herbert Xu wrote:

> Jon Mason <jdmason@us.ibm.com> wrote:
> 
>>checksum and scatter gather as features of the bridge device.  In the 
>>case that the physical ethernet device does not support scatter 
>>gather or hardware checksum, dev_queue_xmit() will check the
>>dev->features and do the necessary linearization and calculate the
>>checksum.
> 
> 
> Unfortunately skb_linearize is a lot more expensive than not
> generating the non-linear skb's in the first place.
> 
> So this is going to hurt people using the bridge devices over
> physical devices that don't support SG.

Fair point - though I would argue that devices that
support SG are much more common now - so optimizing
for the more common case might be preferable - since
people using bridging with devices that support
checksum offload are taking quite a hit when they
do bridging - perhaps this could be a conditional?

thanks,
Nivedita

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[Nivedita Singhvi]

David S. Miller wrote:

>>I think this is a good thing to have, gives us performance
>>gain (equivalent to using/not using checksum offload) and
>>has very little impact on the bridging layer, and is
>>independent of the virtualization stuff..
> 
> 
> I agree but keep in mind that when bridging over non-checksumming
> devices it will be more expensive.  Especially for TCP.

Yep - as I just said to Herbert - perhaps worthwhile then
conditional upon support of checksum offload & SG?

thanks,
Nivedita

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[David S. Miller]

From: Nivedita Singhvi <niv@us.ibm.com>
Date: Thu, 19 May 2005 08:10:13 -0700

> Fair point - though I would argue that devices that
> support SG are much more common now - so optimizing
> for the more common case might be preferable - since
> people using bridging with devices that support
> checksum offload are taking quite a hit when they
> do bridging - perhaps this could be a conditional?

I disagree.  Many low costs devices are non-SG still,
there is no reason to penalize them explicitly.

I think instead we should look at ways to propagate
the hardware device features to the bridge.  Even if
a bridge is composed of multiple devices, we just
advertise the subset of features actually supported.

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[Stephen Hemminger]

Here is a somewhat different patch from 2.6.12-rc4-bridge
http://developer.osdl.org/shemminger/patches/2.6.12-rc4-bridge/

The bridge doesn't need locking, or checksumming and can allow highdma
buffers; all of which are handled by net/core/dev.c if needed.


===================================================================
--- netem-2.6.12-rc4.orig/net/bridge/br_device.c
+++ netem-2.6.12-rc4/net/bridge/br_device.c
@@ -101,4 +99,6 @@ void br_dev_setup(struct net_device *dev
 	dev->tx_queue_len = 0;
 	dev->set_mac_address = NULL;
 	dev->priv_flags = IFF_EBRIDGE;
+	dev->features |= NETIF_F_SG | NETIF_F_NO_CSUM 
+		| NETIF_F_HIGHDMA | NETIF_F_LLTX;
 }

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[Stephen Hemminger]

On Thu, 19 May 2005 08:13:40 -0700
Nivedita Singhvi <niv@us.ibm.com> wrote:

> David S. Miller wrote:
> 
> >>I think this is a good thing to have, gives us performance
> >>gain (equivalent to using/not using checksum offload) and
> >>has very little impact on the bridging layer, and is
> >>independent of the virtualization stuff..
> > 
> > 
> > I agree but keep in mind that when bridging over non-checksumming
> > devices it will be more expensive.  Especially for TCP.
> 
> Yep - as I just said to Herbert - perhaps worthwhile then
> conditional upon support of checksum offload & SG?

What about mixed device bridges? 

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[Nivedita Singhvi]

David S. Miller wrote:

> I think instead we should look at ways to propagate
> the hardware device features to the bridge.  Even if
> a bridge is composed of multiple devices, we just
> advertise the subset of features actually supported.

Just for some background:

In the typical Xen virtual environment, the primary
interface is typically bridged with virtual interfaces
that talk to other partitions.

Since currently the bridge doesn't advertise them,
this results in the loss of hw checksum offload,
segmentation offload, and sendfile exploitation
(sendfile() falls down to the usual tcp_sendmsg()
case) etc on the primary partition traffic talking
to remote hosts.

This can be rather painful, and performance comparisons
with other OSs which don't do this are even more of
an apples to oranges comparison since this is
somewhat silently happening.

Hence the need for enhancing the bridging environment
somewhat :).

thanks,
Nivedita

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[David S. Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Thu, 19 May 2005 13:33:33 -0700

> The bridge doesn't need locking, or checksumming and can allow highdma
> buffers; all of which are handled by net/core/dev.c if needed.

As discuesed elsewhere, this handling by net/core/dev.c makes
TCP sending much more expensive when it is actually used.

Furthermore, I just found another hole in the idea to propagate
sub-device features into the bridge device.

If one device has NETIF_F_HW_CSUM and the others have NETIF_F_IP_CSUM,
both bits will be set in the bridge device and things will entirely
break.  The two checksumming on output schemes are different, and all
of the stack assumes that only one of these two bits are set.

I have such a setup in two of my sparc64 systems (sunhme does
NETIF_F_HW_CSUM, and tg3 does NETIF_F_IP_CSUM).  Also, my PowerMAC G5
has this problem too, the onboard sungem chip does NETIF_F_HW_CSUM and
the tg3 I have in a PCI slot does NETIF_F_P_CSUM.  So given that
half the machines I have powered on right here could trigger the
problem, it's far from theoretical :-)

There are multiple spots that want to do this kind of stuff
now (bridging, vlan, bonding) which indicates that some sort
of common infrastructure should be written to implement this
kind of stuff.

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[Jon Mason]

On Thursday 19 May 2005 04:48 pm, David S. Miller wrote:
> From: Stephen Hemminger <shemminger@osdl.org>
> Date: Thu, 19 May 2005 13:33:33 -0700
>
> > The bridge doesn't need locking, or checksumming and can allow highdma
> > buffers; all of which are handled by net/core/dev.c if needed.
>
> As discuesed elsewhere, this handling by net/core/dev.c makes
> TCP sending much more expensive when it is actually used.
>
> Furthermore, I just found another hole in the idea to propagate
> sub-device features into the bridge device.
>
> If one device has NETIF_F_HW_CSUM and the others have NETIF_F_IP_CSUM,
> both bits will be set in the bridge device and things will entirely
> break.  The two checksumming on output schemes are different, and all
> of the stack assumes that only one of these two bits are set.
>
> I have such a setup in two of my sparc64 systems (sunhme does
> NETIF_F_HW_CSUM, and tg3 does NETIF_F_IP_CSUM).  Also, my PowerMAC G5
> has this problem too, the onboard sungem chip does NETIF_F_HW_CSUM and
> the tg3 I have in a PCI slot does NETIF_F_IP_CSUM.  So given that
> half the machines I have powered on right here could trigger the
> problem, it's far from theoretical :-)
>
> There are multiple spots that want to do this kind of stuff
> now (bridging, vlan, bonding) which indicates that some sort
> of common infrastructure should be written to implement this
> kind of stuff.

Since NETIF_F_HW_CSUM encompasses NETIF_F_IP_CSUM, perhaps it would be better 
to use the latter until such time as a new infrastructure can be implimented.

Thanks,
Jon

Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming

[David S. Miller]

From: Jon Mason <jdmason@us.ibm.com>
Date: Thu, 19 May 2005 18:23:31 -0500

> Since NETIF_F_HW_CSUM encompasses NETIF_F_IP_CSUM, perhaps it would
> be better to use the latter until such time as a new infrastructure
> can be implimented.

Probably, that is the best idea, it is again another
definition of "subset".