[ovs-dev] [PATCH 1/2] datapath: Copy Xen's checksumming fields when doing skb_copy.

Ian Campbell Ian.Campbell at citrix.com
Wed Nov 18 22:31:11 UTC 2009


On Wed, 2009-11-18 at 21:37 +0000, Jesse Gross wrote: 
> 
> Ian Campbell wrote:
> > On Wed, 2009-11-18 at 04:16 +0000, Jesse Gross wrote:
> >   
> >> Two fields that control checksumming were added to sk_buff in
> >> Xen: proto_data_valid and proto_csum_blank.  These fields are copied
> >> when doing a skb_clone but not in other functions such as skb_copy,
> >> which can lead to checksum errors in TCP and UDP when offloading is
> >> enabled in the guest.  To fix this we manually copy these fields,
> >> though ideally this should be fixed upstream in Xen.
> >>     
> >
> > I think this only applies to older 2.6.18-xen kernel versions. For a
> > PVOPS kernel and some newer old-style Xen ports (such as the upcoming
> > 2.6.27 based XenServer kernel) these fields no longer exist. They do
> > exist in the SLES11 2.6.27 kernel which XenServer will use as a base
> > though.
> >   
> 
> That sounds right, though I'm not really familiar with all the 
> variations of Xen.  I think I initially looked at an older 2.6.27 based 
> kernel and saw the fields were still there.  They still exist in the 
> skbuff.h comments in XenServer - it might be nice to take that out.

Thanks, I noticed the comments while I was double checking I'd really
gotten rid of the fields and removed them.

> > I'm not sure why skb_copy doesn't include those fields in kernels where
> > they are present. 
> 
> I'm pretty sure that it is just a mistake.  I took a look at the git 
> history for the mainline kernel and it seems that there were several 
> other fields which were also accidentally added to only some of the 
> functions and it was pretty random as to what you would actually get - 
> for example ip_summed was in skb_copy but not in skb_copy_expand.  These 
> have since been cleaned up and comments added that say "don't do this".
> 
> > Assuming it is an omission, rather than deliberate, it
> > seems odd that we have gotten away with it for such a long time. At
> > first glance it looks like skb_copy isn't really all the widely used
> > unless you have specific firewall, bonding, buggy-hardware setups or are
> > using obscure network protocols so maybe we've just gotten away with it.
> >   
> 
> I think it's pretty rare to need to do an skb_copy and where it does 
> happen it usually doesn't affect checksumming (because the copy happens 
> after these bits have been looked at).  In this case I ran to the 
> problem because it was causing issues with RSPAN, which needs to make a 
> copy of the sk_buff since it is getting send out on 2 different VLANs.

Makes sense.

Ian.





More information about the dev mailing list