Skip to content

Commit

Permalink
bond: Disable TLS features indication
Browse files Browse the repository at this point in the history
Bond agnostically interacts with TLS device-offload requests via the
.ndo_sk_get_lower_dev operation. Return value is true iff bond
guarantees fixed mapping between the TLS connection and a lower netdev.

Due to this nature, the bond TLS device offload features are not
explicitly controllable in the bond layer. As of today, these are
read-only values based on the evaluation of bond_sk_check().  However,
this indication might be incorrect and misleading, when the feature bits
are "fixed" by some dependency features.  For example,
NETIF_F_HW_TLS_TX/RX are forcefully cleared in case the corresponding
checksum offload is disabled. But in fact the bond ability to still
offload TLS connections to the lower device is not hurt.

This means that these bits can not be trusted, and hence better become
unused.

This patch revives some old discussion [1] and proposes a much simpler
solution: Clear the bond's TLS features bits. Everyone should stop
reading them.

[1] https://lore.kernel.org/netdev/20210526095747.22446-1-tariqt@nvidia.com/

Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Gal Pressman <gal@nvidia.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20221025105300.4718-1-tariqt@nvidia.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
  • Loading branch information
Tariq Toukan authored and Paolo Abeni committed Oct 27, 2022
1 parent 330543d commit 28581b9
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 34 deletions.
13 changes: 1 addition & 12 deletions drivers/net/bonding/bond_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ netdev_tx_t bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
return dev_queue_xmit(skb);
}

bool bond_sk_check(struct bonding *bond)
static bool bond_sk_check(struct bonding *bond)
{
switch (BOND_MODE(bond)) {
case BOND_MODE_8023AD:
Expand Down Expand Up @@ -1398,13 +1398,6 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
netdev_features_t mask;
struct slave *slave;

#if IS_ENABLED(CONFIG_TLS_DEVICE)
if (bond_sk_check(bond))
features |= BOND_TLS_FEATURES;
else
features &= ~BOND_TLS_FEATURES;
#endif

mask = features;

features &= ~NETIF_F_ONE_FOR_ALL;
Expand Down Expand Up @@ -5806,10 +5799,6 @@ void bond_setup(struct net_device *bond_dev)
if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP)
bond_dev->features |= BOND_XFRM_FEATURES;
#endif /* CONFIG_XFRM_OFFLOAD */
#if IS_ENABLED(CONFIG_TLS_DEVICE)
if (bond_sk_check(bond))
bond_dev->features |= BOND_TLS_FEATURES;
#endif
}

/* Destroy a bonding device.
Expand Down
18 changes: 0 additions & 18 deletions drivers/net/bonding/bond_options.c
Original file line number Diff line number Diff line change
Expand Up @@ -842,19 +842,6 @@ static bool bond_set_xfrm_features(struct bonding *bond)
return true;
}

static bool bond_set_tls_features(struct bonding *bond)
{
if (!IS_ENABLED(CONFIG_TLS_DEVICE))
return false;

if (bond_sk_check(bond))
bond->dev->wanted_features |= BOND_TLS_FEATURES;
else
bond->dev->wanted_features &= ~BOND_TLS_FEATURES;

return true;
}

static int bond_option_mode_set(struct bonding *bond,
const struct bond_opt_value *newval)
{
Expand Down Expand Up @@ -885,7 +872,6 @@ static int bond_option_mode_set(struct bonding *bond,
bool update = false;

update |= bond_set_xfrm_features(bond);
update |= bond_set_tls_features(bond);

if (update)
netdev_update_features(bond->dev);
Expand Down Expand Up @@ -1418,10 +1404,6 @@ static int bond_option_xmit_hash_policy_set(struct bonding *bond,
newval->string, newval->value);
bond->params.xmit_policy = newval->value;

if (bond->dev->reg_state == NETREG_REGISTERED)
if (bond_set_tls_features(bond))
netdev_update_features(bond->dev);

return 0;
}

Expand Down
4 changes: 0 additions & 4 deletions include/net/bonding.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@
#define BOND_XFRM_FEATURES (NETIF_F_HW_ESP | NETIF_F_HW_ESP_TX_CSUM | \
NETIF_F_GSO_ESP)

#define BOND_TLS_FEATURES (NETIF_F_HW_TLS_TX | NETIF_F_HW_TLS_RX)

#ifdef CONFIG_NET_POLL_CONTROLLER
extern atomic_t netpoll_block_tx;

Expand Down Expand Up @@ -280,8 +278,6 @@ struct bond_vlan_tag {
unsigned short vlan_id;
};

bool bond_sk_check(struct bonding *bond);

/**
* Returns NULL if the net_device does not belong to any of the bond's slaves
*
Expand Down

0 comments on commit 28581b9

Please sign in to comment.