Skip to content

Commit 007ab53

Browse files
jarodwilsonkuba-moo
authored andcommitted
bonding: fix feature flag setting at init time
Don't try to adjust XFRM support flags if the bond device isn't yet registered. Bad things can currently happen when netdev_change_features() is called without having wanted_features fully filled in yet. This code runs both on post-module-load mode changes, as well as at module init time, and when run at module init time, it is before register_netdevice() has been called and filled in wanted_features. The empty wanted_features led to features also getting emptied out, which was definitely not the intended behavior, so prevent that from happening. Originally, I'd hoped to stop adjusting wanted_features at all in the bonding driver, as it's documented as being something only the network core should touch, but we actually do need to do this to properly update both the features and wanted_features fields when changing the bond type, or we get to a situation where ethtool sees: esp-hw-offload: off [requested on] I do think we should be using netdev_update_features instead of netdev_change_features here though, so we only send notifiers when the features actually changed. Fixes: a3b658c ("bonding: allow xfrm offload setup post-module-load") Reported-by: Ivan Vecera <ivecera@redhat.com> Suggested-by: Ivan Vecera <ivecera@redhat.com> Cc: Jay Vosburgh <j.vosburgh@gmail.com> Cc: Veaceslav Falico <vfalico@gmail.com> Cc: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Jarod Wilson <jarod@redhat.com> Link: https://lore.kernel.org/r/20201205172229.576587-1-jarod@redhat.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 819f56b commit 007ab53

File tree

2 files changed

+15
-9
lines changed

2 files changed

+15
-9
lines changed

drivers/net/bonding/bond_options.c

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,19 @@ const struct bond_option *bond_opt_get(unsigned int option)
745745
return &bond_opts[option];
746746
}
747747

748+
static void bond_set_xfrm_features(struct net_device *bond_dev, u64 mode)
749+
{
750+
if (!IS_ENABLED(CONFIG_XFRM_OFFLOAD))
751+
return;
752+
753+
if (mode == BOND_MODE_ACTIVEBACKUP)
754+
bond_dev->wanted_features |= BOND_XFRM_FEATURES;
755+
else
756+
bond_dev->wanted_features &= ~BOND_XFRM_FEATURES;
757+
758+
netdev_update_features(bond_dev);
759+
}
760+
748761
static int bond_option_mode_set(struct bonding *bond,
749762
const struct bond_opt_value *newval)
750763
{
@@ -767,13 +780,8 @@ static int bond_option_mode_set(struct bonding *bond,
767780
if (newval->value == BOND_MODE_ALB)
768781
bond->params.tlb_dynamic_lb = 1;
769782

770-
#ifdef CONFIG_XFRM_OFFLOAD
771-
if (newval->value == BOND_MODE_ACTIVEBACKUP)
772-
bond->dev->wanted_features |= BOND_XFRM_FEATURES;
773-
else
774-
bond->dev->wanted_features &= ~BOND_XFRM_FEATURES;
775-
netdev_change_features(bond->dev);
776-
#endif /* CONFIG_XFRM_OFFLOAD */
783+
if (bond->dev->reg_state == NETREG_REGISTERED)
784+
bond_set_xfrm_features(bond->dev, newval->value);
777785

778786
/* don't cache arp_validate between modes */
779787
bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;

include/net/bonding.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,8 @@
8686
#define bond_for_each_slave_rcu(bond, pos, iter) \
8787
netdev_for_each_lower_private_rcu((bond)->dev, pos, iter)
8888

89-
#ifdef CONFIG_XFRM_OFFLOAD
9089
#define BOND_XFRM_FEATURES (NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM | \
9190
NETIF_F_GSO_ESP)
92-
#endif /* CONFIG_XFRM_OFFLOAD */
9391

9492
#ifdef CONFIG_NET_POLL_CONTROLLER
9593
extern atomic_t netpoll_block_tx;

0 commit comments

Comments
 (0)