linux-kernel - Re: [PATCH net v3] bonding: fix feature flag setting at init time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 3 Dec 2020 22:14:12 -0500
From:   Jarod Wilson <jarod@...hat.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Ivan Vecera <ivecera@...hat.com>,
        Jay Vosburgh <j.vosburgh@...il.com>,
        Veaceslav Falico <vfalico@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>,
        "David S. Miller" <davem@...emloft.net>,
        Thomas Davis <tadavis@....gov>, Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net v3] bonding: fix feature flag setting at init time

On Thu, Dec 3, 2020 at 11:50 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Wed,  2 Dec 2020 19:43:57 -0500 Jarod Wilson wrote:
> >       bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL | NETIF_F_GSO_UDP_L4;
> > -#ifdef CONFIG_XFRM_OFFLOAD
> > -     bond_dev->hw_features |= BOND_XFRM_FEATURES;
> > -#endif /* CONFIG_XFRM_OFFLOAD */
> >       bond_dev->features |= bond_dev->hw_features;
> >       bond_dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
> >  #ifdef CONFIG_XFRM_OFFLOAD
> > -     /* Disable XFRM features if this isn't an active-backup config */
> > -     if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP)
> > -             bond_dev->features &= ~BOND_XFRM_FEATURES;
> > +     bond_dev->hw_features |= BOND_XFRM_FEATURES;
> > +     /* Only enable XFRM features if this is an active-backup config */
> > +     if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP)
> > +             bond_dev->features |= BOND_XFRM_FEATURES;
> >  #endif /* CONFIG_XFRM_OFFLOAD */
>
> This makes no functional change, or am I reading it wrong?

You are correct, there's ultimately no functional change there, it
primarily just condenses the code down to a single #ifdef block, and
doesn't add and then remove BOND_XFRM_FEATURES from
bond_dev->features, instead omitting it initially and only adding it
when in AB mode. I'd poked at the code in that area while trying to
get to the bottom of this, thought it made it more understandable, so
I left it in, but ultimately, it's not necessary to fix the problem
here.

-- 
Jarod Wilson
jarod@...hat.com