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

[Jarod Wilson]

On Thu, Dec 3, 2020 at 11:50 AM Jakub Kicinski <kuba@xxxxxxxxxx> 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@xxxxxxxxxx