[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 09:51:11 UTC 2009


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.

I'm not sure why skb_copy doesn't include those fields in kernels where
they are present. 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.

Ian.

> 
> Bug #2299
> ---
>  datapath/actions.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/datapath/actions.c b/datapath/actions.c
> index a037e43..24043b2 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -28,6 +28,13 @@ make_writable(struct sk_buff *skb, gfp_t gfp)
>  	if (skb_shared(skb) || skb_cloned(skb)) {
>  		struct sk_buff *nskb = skb_copy(skb, gfp);
>  		if (nskb) {
> +#ifdef CONFIG_XEN
> +			/* These fields are copied in skb_clone but not in
> +			 * skb_copy or related functions.  We need to manually
> +			 * copy them over here. */
> +			nskb->proto_data_valid = skb->proto_data_valid;
> +			nskb->proto_csum_blank = skb->proto_csum_blank;
> +#endif
>  			kfree_skb(skb);
>  			return nskb;
>  		}






More information about the dev mailing list