From: Cosmin Ratiu There is a mechanism to mutually exclude TC offload and IPsec offload from the same interface (commit [1]) due to ordering issues between SW and HW paths. TC offload makes use of flow_tag to carry the tc mark (even when 0). Upcoming changes to accel protocols will make use of flow_tag to carry the protocol marker. As the flow_tag cannot be partially modified by a steering rule, the last rule setting the flow_tag will overwrite any previous ones. This means that TC offload cannot be active at the same time with any of the currently implemented accel protocols (IPsec, MACsec, PSP). Generalize the TC <-> IPsec mutual exclusion mechanism to be usable by more accel protocols: - move the existing mlx5e_ipsec_{,un}block_tc_offload functions to en_accel.h, rename them to mlx5e_accel_{,un}block_tc_offload. - rename the mdev counter from num_block_ipsec to num_accel. - rename is_tc_ipsec_order_check_needed -> is_tc_accel_check_needed and make it not bail out when IPsec isn't configured. - replace the condition use of the esw write lock as a protection for incrementing the counter with a new mutex instead. The esw might not be available, and it wasn't used correctly on the decrement path anyway, allowing races to happen. Using a dedicated mutex for these counters makes it clear and avoids races. - add underflow warnings for the two counters to catch future miscounting bugs. - move counters and the new lock into a dedicated struct in mlx5_core_dev named 'offload_block'. Now offload_block.num_tc means the number of tc blocks due to accel rules and ofload_block.num_accel means the number of accel blocks due to tc rules. [1] commit c8e350e62fc5 ("net/mlx5e: Make TC and IPsec offloads mutually exclusive on a netdev") Signed-off-by: Cosmin Ratiu Reviewed-by: Dragos Tatulea Reviewed-by: Carolina Jubran Signed-off-by: Tariq Toukan --- .../mellanox/mlx5/core/en_accel/en_accel.h | 22 ++++++++ .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 54 +++---------------- .../net/ethernet/mellanox/mlx5/core/en_tc.c | 46 +++++++++------- .../net/ethernet/mellanox/mlx5/core/main.c | 3 ++ include/linux/mlx5/driver.h | 7 ++- 5 files changed, 64 insertions(+), 68 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h index 3f212e46fc2f..8a2ea7616440 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h @@ -218,6 +218,28 @@ static inline void mlx5e_accel_tx_finish(struct mlx5e_txqsq *sq, #endif } +static inline int mlx5e_accel_block_tc_offload(struct mlx5_core_dev *mdev) +{ + int ret = 0; + + mutex_lock(&mdev->offload_block.lock); + if (mdev->offload_block.num_accel) + ret = -EBUSY; + else + mdev->offload_block.num_tc++; + mutex_unlock(&mdev->offload_block.lock); + + return ret; +} + +static inline void mlx5e_accel_unblock_tc_offload(struct mlx5_core_dev *mdev) +{ + mutex_lock(&mdev->offload_block.lock); + if (!WARN_ON_ONCE(!mdev->offload_block.num_tc)) + mdev->offload_block.num_tc--; + mutex_unlock(&mdev->offload_block.lock); +} + static inline int mlx5e_accel_init_rx(struct mlx5e_priv *priv) { return mlx5e_ktls_init_rx(priv); diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c index 329608c59313..74e0aa5b6133 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c @@ -4,6 +4,7 @@ #include #include "en.h" #include "en/fs.h" +#include "en_accel/en_accel.h" #include "eswitch.h" #include "ipsec.h" #include "fs_core.h" @@ -2574,53 +2575,12 @@ void mlx5e_accel_ipsec_fs_read_stats(struct mlx5e_priv *priv, void *ipsec_stats) } } -#ifdef CONFIG_MLX5_ESWITCH -static int mlx5e_ipsec_block_tc_offload(struct mlx5_core_dev *mdev) -{ - struct mlx5_eswitch *esw = mdev->priv.eswitch; - int err = 0; - - if (esw) { - err = mlx5_esw_lock(esw); - if (err) - return err; - } - - if (mdev->num_block_ipsec) { - err = -EBUSY; - goto unlock; - } - - mdev->num_block_tc++; - -unlock: - if (esw) - mlx5_esw_unlock(esw); - - return err; -} -#else -static int mlx5e_ipsec_block_tc_offload(struct mlx5_core_dev *mdev) -{ - if (mdev->num_block_ipsec) - return -EBUSY; - - mdev->num_block_tc++; - return 0; -} -#endif - -static void mlx5e_ipsec_unblock_tc_offload(struct mlx5_core_dev *mdev) -{ - mdev->num_block_tc--; -} - int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_ipsec_sa_entry *sa_entry) { int err; if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET) { - err = mlx5e_ipsec_block_tc_offload(sa_entry->ipsec->mdev); + err = mlx5e_accel_block_tc_offload(sa_entry->ipsec->mdev); if (err) return err; } @@ -2637,7 +2597,7 @@ int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_ipsec_sa_entry *sa_entry) err_out: if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET) - mlx5e_ipsec_unblock_tc_offload(sa_entry->ipsec->mdev); + mlx5e_accel_unblock_tc_offload(sa_entry->ipsec->mdev); return err; } @@ -2652,7 +2612,7 @@ void mlx5e_accel_ipsec_fs_del_rule(struct mlx5e_ipsec_sa_entry *sa_entry) mlx5_packet_reformat_dealloc(mdev, ipsec_rule->pkt_reformat); if (sa_entry->attrs.type == XFRM_DEV_OFFLOAD_PACKET) - mlx5e_ipsec_unblock_tc_offload(mdev); + mlx5e_accel_unblock_tc_offload(mdev); if (sa_entry->attrs.dir == XFRM_DEV_OFFLOAD_OUT) { tx_ft_put(sa_entry->ipsec, sa_entry->attrs.type); @@ -2686,7 +2646,7 @@ int mlx5e_accel_ipsec_fs_add_pol(struct mlx5e_ipsec_pol_entry *pol_entry) { int err; - err = mlx5e_ipsec_block_tc_offload(pol_entry->ipsec->mdev); + err = mlx5e_accel_block_tc_offload(pol_entry->ipsec->mdev); if (err) return err; @@ -2701,7 +2661,7 @@ int mlx5e_accel_ipsec_fs_add_pol(struct mlx5e_ipsec_pol_entry *pol_entry) return 0; err_out: - mlx5e_ipsec_unblock_tc_offload(pol_entry->ipsec->mdev); + mlx5e_accel_unblock_tc_offload(pol_entry->ipsec->mdev); return err; } @@ -2712,7 +2672,7 @@ void mlx5e_accel_ipsec_fs_del_pol(struct mlx5e_ipsec_pol_entry *pol_entry) mlx5_del_flow_rules(ipsec_rule->rule); - mlx5e_ipsec_unblock_tc_offload(pol_entry->ipsec->mdev); + mlx5e_accel_unblock_tc_offload(pol_entry->ipsec->mdev); if (pol_entry->attrs.dir == XFRM_DEV_OFFLOAD_IN) { rx_ft_put_policy(pol_entry->ipsec, diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c index 1bc7b9019124..70195fcddfc8 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c @@ -4811,14 +4811,14 @@ static bool is_flow_rule_duplicate_allowed(struct net_device *dev, return netif_is_lag_port(dev) && rpriv && rpriv->rep->vport != MLX5_VPORT_UPLINK; } -/* As IPsec and TC order is not aligned between software and hardware-offload, - * either IPsec offload or TC offload, not both, is allowed for a specific interface. +/* TC offload and accel protocols can overwrite each other's flow_tag with + * steering rules and they cannot simultaneously operate on the same interface. + * Additionally, as IPsec and TC order is not aligned between software and + * hardware-offload, only one is allowed for a specific interface. */ -static bool is_tc_ipsec_order_check_needed(struct net_device *filter, struct mlx5e_priv *priv) +static bool is_tc_accel_check_needed(struct net_device *filter, + struct mlx5e_priv *priv) { - if (!IS_ENABLED(CONFIG_MLX5_EN_IPSEC)) - return false; - if (filter != priv->netdev) return false; @@ -4828,27 +4828,35 @@ static bool is_tc_ipsec_order_check_needed(struct net_device *filter, struct mlx return true; } -static int mlx5e_tc_block_ipsec_offload(struct net_device *filter, struct mlx5e_priv *priv) +static int mlx5e_tc_block_accel_offload(struct net_device *filter, + struct mlx5e_priv *priv) { struct mlx5_core_dev *mdev = priv->mdev; + int ret = 0; - if (!is_tc_ipsec_order_check_needed(filter, priv)) + if (!is_tc_accel_check_needed(filter, priv)) return 0; - if (mdev->num_block_tc) - return -EBUSY; - - mdev->num_block_ipsec++; + mutex_lock(&mdev->offload_block.lock); + if (mdev->offload_block.num_tc) + ret = -EBUSY; + else + mdev->offload_block.num_accel++; + mutex_unlock(&mdev->offload_block.lock); - return 0; + return ret; } -static void mlx5e_tc_unblock_ipsec_offload(struct net_device *filter, struct mlx5e_priv *priv) +static void mlx5e_tc_unblock_accel_offload(struct net_device *filter, + struct mlx5e_priv *priv) { - if (!is_tc_ipsec_order_check_needed(filter, priv)) + if (!is_tc_accel_check_needed(filter, priv)) return; - priv->mdev->num_block_ipsec--; + mutex_lock(&priv->mdev->offload_block.lock); + if (!WARN_ON_ONCE(!priv->mdev->offload_block.num_accel)) + priv->mdev->offload_block.num_accel--; + mutex_unlock(&priv->mdev->offload_block.lock); } int mlx5e_configure_flower(struct net_device *dev, struct mlx5e_priv *priv, @@ -4863,7 +4871,7 @@ int mlx5e_configure_flower(struct net_device *dev, struct mlx5e_priv *priv, if (!mlx5_esw_hold(priv->mdev)) return -EBUSY; - err = mlx5e_tc_block_ipsec_offload(dev, priv); + err = mlx5e_tc_block_accel_offload(dev, priv); if (err) goto esw_release; @@ -4912,7 +4920,7 @@ int mlx5e_configure_flower(struct net_device *dev, struct mlx5e_priv *priv, err_free: mlx5e_flow_put(priv, flow); out: - mlx5e_tc_unblock_ipsec_offload(dev, priv); + mlx5e_tc_unblock_accel_offload(dev, priv); mlx5_esw_put(priv->mdev); esw_release: mlx5_esw_release(priv->mdev); @@ -4955,7 +4963,7 @@ int mlx5e_delete_flower(struct net_device *dev, struct mlx5e_priv *priv, trace_mlx5e_delete_flower(f); mlx5e_flow_put(priv, flow); - mlx5e_tc_unblock_ipsec_offload(dev, priv); + mlx5e_tc_unblock_accel_offload(dev, priv); mlx5_esw_put(priv->mdev); return 0; diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c index 643b4aac2033..406c0f7e63d8 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c @@ -1810,6 +1810,7 @@ int mlx5_mdev_init(struct mlx5_core_dev *dev, int profile_idx) lockdep_register_key(&dev->lock_key); mutex_init(&dev->intf_state_mutex); lockdep_set_class(&dev->intf_state_mutex, &dev->lock_key); + mutex_init(&dev->offload_block.lock); mutex_init(&dev->mlx5e_res.uplink_netdev_lock); mutex_init(&dev->wc_state_lock); @@ -1898,6 +1899,7 @@ int mlx5_mdev_init(struct mlx5_core_dev *dev, int profile_idx) mutex_destroy(&priv->alloc_mutex); mutex_destroy(&priv->bfregs.wc_head.lock); mutex_destroy(&priv->bfregs.reg_head.lock); + mutex_destroy(&dev->offload_block.lock); mutex_destroy(&dev->intf_state_mutex); lockdep_unregister_key(&dev->lock_key); return err; @@ -1925,6 +1927,7 @@ void mlx5_mdev_uninit(struct mlx5_core_dev *dev) mutex_destroy(&priv->bfregs.reg_head.lock); mutex_destroy(&dev->wc_state_lock); mutex_destroy(&dev->mlx5e_res.uplink_netdev_lock); + mutex_destroy(&dev->offload_block.lock); mutex_destroy(&dev->intf_state_mutex); lockdep_unregister_key(&dev->lock_key); } diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h index b1871c0821d0..2d9bc752e431 100644 --- a/include/linux/mlx5/driver.h +++ b/include/linux/mlx5/driver.h @@ -788,8 +788,11 @@ struct mlx5_core_dev { u32 vsc_addr; struct mlx5_hv_vhca *hv_vhca; struct mlx5_hwmon *hwmon; - u64 num_block_tc; - u64 num_block_ipsec; + struct { + struct mutex lock; + u64 num_tc; + u64 num_accel; + } offload_block; #ifdef CONFIG_MLX5_MACSEC struct mlx5_macsec_fs *macsec_fs; /* MACsec notifier chain to sync MACsec core and IB database */ -- 2.44.0