[ovs-dev] [PATCH] datapath: Support for Linux kernel 3.3

[Pravin Shelar]

On Thu, Mar 22, 2012 at 11:11 AM, Jesse Gross <jesse at nicira.com> wrote:
> On Wed, Mar 21, 2012 at 5:08 PM, Pravin B Shelar <pshelar at 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.
>
ok.

>> 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)?
>
ok

>>  #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.
>
ok

>> 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.
>
ok, I think it was changed late in 3.0, I was looking at latest 3.0
revision (.25) which shld work with this check.
I will fix this.

>> +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.
ok.