On Wed, Mar 21, 2012 at 5:08 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index d64fc32..401e774 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -62,8 +62,8 @@
>  #include "vport-internal_dev.h"
>
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,18) || \
> -    LINUX_VERSION_CODE >= KERNEL_VERSION(3,3,0)
> -#error Kernels before 2.6.18 or after 3.2 are not supported by this version 
> of Open vSwitch.
> +    LINUX_VERSION_CODE > KERNEL_VERSION(3,3,0)
> +#error Kernels before 2.6.18 or after 3.3 are not supported by this version 
> of Open vSwitch.
>  #endif

This needs to be >= 3.4 otherwise when 3.3.1 is released the check
will fail even though we should be compatible with it.

> diff --git a/datapath/linux/compat/include/linux/genetlink.h 
> b/datapath/linux/compat/include/linux/genetlink.h
> index f7b96d9..383f606 100644
> --- a/datapath/linux/compat/include/linux/genetlink.h
> +++ b/datapath/linux/compat/include/linux/genetlink.h
> @@ -4,15 +4,14 @@
>  #include_next <linux/genetlink.h>

Can you add linux/version.h in files where you add version tests (I
think net/dst.h now has this problem as well)?

>  #ifdef CONFIG_PROVE_LOCKING
> -/* No version of the kernel has this function, but our locking scheme depends
> - * on genl_mutex so for clarity we use it where appropriate. */
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,3,0)
>  static inline int lockdep_genl_is_held(void)

It seems somewhat preferable to do the version test on the outside of
CONFIG_PROVE_LOCKING since the latter is effectively part of the
function definition.

> diff --git a/datapath/linux/compat/include/net/dst.h 
> b/datapath/linux/compat/include/net/dst.h
> index f481a9d..6b1385d 100644
> --- a/datapath/linux/compat/include/net/dst.h
> +++ b/datapath/linux/compat/include/net/dst.h
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,3,0)
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,0,0)

I'm not sure why this is two separate tests rather than one using &&.
I also believe that it should be >= 3.1 since this doesn't compile on
my system.

> +static inline struct neighbour *dst_get_neighbour_noref(struct dst_entry 
> *dst)
> +{
> +       return rcu_dereference(dst->_neighbour);

I think this change was just a rename, so I'm not sure that including
the actual function body (as opposed to a #define) adds anything.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to