[ovs-dev] [PATCH 1/2] datapath: Check for proto_data_valid member instead of kernel version.

[Ian Campbell]

On Wed, 2009-11-18 at 23:27 +0000, Ben Pfaff wrote:
> Commit 5ef800a69 "datapath: Copy Xen's checksumming fields when doing
> skb_copy" should copy proto_data_valid between sk_buffs when that field
> is present.  However the check for CONFIG_XEN plus kernel version 2.6.18
> isn't sufficient, because SLES 11 kernels are version 2.6.27 but do have
> this field.

Looks good to me. I think you might also want:

Subject: Use HAVE_PROTO_DATA_VALID when defining vswitch_skb_checksum_setup

The purpose of the non-empty variant of vswitch_skb_checksum_setup is to
synchronise the proto_data_valid and proto_csum_blank fields into the
standard skb csum/ip_summed fields, therefore it is more correct to key
off of HAVE_PROTO_DATA_VALID.

Signed-off-by: Ian Campbell <ian.campbell at citrix.com>
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 6e97c34..ff08cdc 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -574,7 +574,7 @@ static int dp_frame_hook(struct net_bridge_port *p, struct sk_buff **pskb)
 #error
 #endif
 
-#if defined(CONFIG_XEN) && LINUX_VERSION_CODE == KERNEL_VERSION(2,6,18)
+#if defined(CONFIG_XEN) && defined(HAVE_PROTO_DATA_VALID)
 /* This code is copied verbatim from net/dev/core.c in Xen's
  * linux-2.6.18-92.1.10.el5.xs5.0.0.394.644.  We can't call those functions
  * directly because they aren't exported. */

> 
> This commit adds a configure-time check for the presence of the member
> instead of attempting to guess based on the kernel version.
> 
> Thanks to Ian Campbell for reporting this problem.
> 
> CC: <Ian.Campbell at citrix.com>
> ---
>  acinclude.m4       |    7 +++++++
>  datapath/actions.c |    2 +-
>  2 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index 6115669..e938e6f 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -144,6 +144,13 @@ AC_DEFUN([OVS_CHECK_LINUX26_COMPAT], [
>                    [OVS_DEFINE([HAVE_CSUM_UNFOLD])])
>    OVS_GREP_IFELSE([$KSRC26/include/linux/skbuff.h], [skb_cow],
>                    [OVS_DEFINE([HAVE_SKB_COW])])
> +  # Check for the proto_data_valid member in struct sk_buff.  The [^@]
> +  # is necessary because some versions of this header remove the
> +  # member but retain the kerneldoc comment that describes it (which
> +  # starts with @).  The brackets must be doubled because of m4
> +  # quoting rules.
> +  OVS_GREP_IFELSE([$KSRC26/include/linux/skbuff.h], [[[^@]]proto_data_valid],
> +                  [OVS_DEFINE([HAVE_PROTO_DATA_VALID])])
>    OVS_CHECK_LOG2_H
>    OVS_CHECK_VETH
>    if cmp -s datapath/linux-2.6/kcompat.h.new \
> diff --git a/datapath/actions.c b/datapath/actions.c
> index 4f23c83..cadab05 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -37,7 +37,7 @@ make_writable(struct sk_buff *skb, unsigned min_headroom, gfp_t gfp)
>  			nskb->ip_summed = skb->ip_summed;
>  			nskb->csum = skb->csum;
>  #endif
> -#if defined(CONFIG_XEN) && LINUX_VERSION_CODE == KERNEL_VERSION(2,6,18)
> +#if defined(CONFIG_XEN) && defined(HAVE_PROTO_DATA_VALID)
>  			/* These fields are copied in skb_clone but not in
>  			 * skb_copy or related functions.  We need to manually
>  			 * copy them over here. */