[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