On Thu, Oct 2, 2014 at 11:37 AM, Jiri Benc <jb...@redhat.com> wrote:
> On Mon, 29 Sep 2014 21:39:56 -0700, Pravin B Shelar wrote:
>> This patch mostly is related to tunnel API where RHEL 7
>> kernel API are not in-sync with newer linux kernel API. So
>> extra checks are required to check for parameters of API.
>>
>> Signed-off-by: Pravin B Shelar <pshe...@nicira.com>
>> ---
> [...]
>> --- a/datapath/linux/compat/include/linux/hash.h
>> +++ b/datapath/linux/compat/include/linux/hash.h
>> @@ -2,11 +2,10 @@
>>  #define _LINUX_HASH_WRAPPER_H
>>
>>  #include_next <linux/hash.h>
>> -
>> -#include <linux/version.h>
>> -#if LINUX_VERSION_CODE < KERNEL_VERSION(3,14,0)
>>  #include <asm/hash.h>
>>
>> +#ifndef HAVE_FAST_HASH_OPS
>> +
>>  struct fast_hash_ops {
>>       u32 (*hash)(const void *data, u32 len, u32 seed);
>>       u32 (*hash2)(const u32 *data, u32 len, u32 seed);
>> @@ -40,6 +39,6 @@ extern u32 arch_fast_hash(const void *data, u32 len, u32 
>> seed);
>>   *   Returns 32bit hash.
>>   */
>>  extern u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed);
>> -#endif /* < 3.14 */
>> +#endif /* HASH_FAST_HASH_OPS */
>
> Typo:        ^^^^
>
fixed.

> [...]
>> --- a/datapath/linux/compat/include/net/ip_tunnels.h
>> +++ b/datapath/linux/compat/include/net/ip_tunnels.h
>> @@ -2,7 +2,15 @@
>>  #define __NET_IP_TUNNELS_WRAPPER_H 1
>>
>>  #include <linux/version.h>
>> -#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,12,0)
>> +#if defined(HAVE_GRE_HANDLE_OFFLOADS) && \
>> +     LINUX_VERSION_CODE >= KERNEL_VERSION(3,10,0)
>> +/* RHEL6 and RHEL7 both has backported tunnel API but RHEL6 has
>> + * older version, so avoid using RHEL6 backports.
>> + */
>> +#define GRE_USE_KERNEL_GRE_HANDLE_OFFLOADS
>> +#endif
>
> This looked fragile until I realized that sk_buff->inner_mac_header was
> merged in 3.10 thus the check should be actually correct even in case of
> other distribution kernels. Might be worth adjusting the comment,
> though.
>
This definitely look more complicated than it needs to be. I am
planning simplifying it with follow on patch.

>> +
>> +#ifdef GRE_USE_KERNEL_GRE_HANDLE_OFFLOADS
>>  #include_next <net/ip_tunnels.h>
>
> Seems to be unnecessary to redefine iptunnel_xmit for RHEL6 but doesn't
> seem to hurt, neither.
>
RHEL6 has different parametes, so it is better you use compat version.

>>
>>  #if LINUX_VERSION_CODE < KERNEL_VERSION(3,15,0)
>> @@ -11,7 +19,11 @@ static inline int rpl_iptunnel_xmit(struct sock *sk, 
>> struct rtable *rt,
>>                                   __be32 dst, __u8 proto, __u8 tos,
>>                                   __u8 ttl, __be16 df, bool xnet)
>>  {
>> +#ifdef HAVE_IPTUNNEL_XMIT_NET
>> +     return iptunnel_xmit(NULL, rt, skb, src, dst, proto, tos, ttl, df);
>> +#else
>>       return iptunnel_xmit(rt, skb, src, dst, proto, tos, ttl, df, xnet);
>> +#endif
>>  }
>>  #define iptunnel_xmit rpl_iptunnel_xmit
>>  #endif
> [...]
>
> The nitpicks above are just that, nitpicks.
>
> Acked-by: Jiri Benc <jb...@redhat.com>
>

Thanks, I have pushed it to master and branch-2.3
> --
> Jiri Benc
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to