On Tue, Sep 22, 2020 at 12:45:03PM +0200, Ilya Maximets wrote:
> On 9/17/20 9:59 PM, Greg Rose wrote:
> > The skb now uses l4_hash and it is easier to check for it.  Also
> > fixes a compile error for RHEL 7.7.
> > 
> > Signed-off-by: Greg Rose <gvrose8...@gmail.com>
> > ---
> >  acinclude.m4                                 | 4 ++--
> >  datapath/datapath.c                          | 6 +++---
> >  datapath/linux/compat/include/linux/skbuff.h | 4 ++--
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 84f344da0..d4e249dac 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -874,8 +874,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
> >    OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_clear_hash])
> >    OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [int.skb_zerocopy(],
> >                    [OVS_DEFINE([HAVE_SKB_ZEROCOPY])])
> > -  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [u8.*l4_rxhash],
> > -                  [OVS_DEFINE([HAVE_L4_RXHASH])])
> > +  OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [l4_hash],
> > +                  [OVS_DEFINE([HAVE_L4_HASH])])
> >    OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_ensure_writable])
> >    OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_vlan_pop])
> >    OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [__skb_vlan_pop])
> > diff --git a/datapath/datapath.c b/datapath/datapath.c
> > index 05c1e4274..c3f57f62a 100644
> > --- a/datapath/datapath.c
> > +++ b/datapath/datapath.c
> > @@ -532,10 +532,10 @@ static int queue_userspace_packet(struct datapath 
> > *dp, struct sk_buff *skb,
> >             hash |= OVS_PACKET_HASH_SW_BIT;
> >  #endif
> >  
> > -#ifdef HAVE_L4_RXHASH
> > -   if (skb->l4_rxhash)
> > -#else
> > +#ifdef HAVE_L4_HASH
> >     if (skb->l4_hash)
> > +#else
> > +   if (skb->l4_rxhash)
> >  #endif
> >             hash |= OVS_PACKET_HASH_L4_BIT;
> >  
> > diff --git a/datapath/linux/compat/include/linux/skbuff.h 
> > b/datapath/linux/compat/include/linux/skbuff.h
> > index 6d248b3ed..29253e433 100644
> > --- a/datapath/linux/compat/include/linux/skbuff.h
> > +++ b/datapath/linux/compat/include/linux/skbuff.h
> > @@ -278,7 +278,7 @@ static inline void skb_clear_hash(struct sk_buff *skb)
> >  #ifdef HAVE_RXHASH
> >     skb->rxhash = 0;
> >  #endif
> > -#if defined(HAVE_L4_RXHASH) && !defined(HAVE_RHEL_OVS_HOOK)
> > +#if !defined(HAVE_L4_HASH) && !defined(HAVE_RHEL_OVS_HOOK)
> 
> It looks like commit 21a719d658b4 ("datapath: check for el6 kernels for 
> per_cpu")
> missed updating of this line with HAVE_RHEL6_PER_CPU instead of 
> HAVE_RHEL_OVS_HOOK.
> Maybe that is the root cause of your build issue?
> 
> I'm not sure why, but this check here was introduced as part of the percpu 
> API fix
> in commit 3d174bfd1a6d ("datapath: compat: Fix build on RHEL 6.6").
> 
> Flavio, could you, please, take a look too?

Those were separate things. Going back to RHEL-6, the skb field 
was there, but got commented out, so it used OVS_HOOK to identify
the kernel and so the l4_hash and the "broken" per_cpu.

Then the hooks got backported, but the per_cpu was still in the
same condition (Aug/2015).

Then we dropped support to kernels older than 3.10 and rhel-6
was 2.6.32, so most probably there is room for a clean up.
For instance, the HAVE_RHEL_OVS_HOOK could be removed completely
as part of Feb/2016 '8063e095878 ("datapath: Drop support for
kernel older than 3.10")'.

What happened in RHEL-7.7 is that the field was renamed but
preserving the kABI with a macro:

-       __u8                    l4_rxhash:1;
+       RH_KABI_REPLACE_P(__u8  l4_rxhash:1,
+                         __u8  l4_hash:1)


So, the regex above gets l4_rxhash, but the actual field name is
l4_hash.

-- 
fbl
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to