Various cleanups for mlx5
Subsystems (5)
cover Various cleanups for mlx5
From: Leon Romanovsky <[email protected]>
Changelog:
v1:
Patch #1:
* Removed extra blank line
* Removed ipsec initialization for uplink
v0: https://lore.kernel.org/all/[email protected]
-----------------------------------------------------------------------
Hi,
This is a batch of various cleanups which I collected during development of mlx5 IPsec full offload support.
Thanks.
Leon Romanovsky (6):
net/mlx5e: Support devlink reload of IPsec core net/mlx5: Return ready to use ASO WQE net/mlx5e: Don't access directly DMA device pointer net/mlx5e: Delete always true DMA check net/mlx5: Remove redundant check
net/mlx5e: Use read lock for eswitch get callbacks
.../ethernet/mellanox/mlx5/core/en/tc/meter.c | 1 - .../mellanox/mlx5/core/en_accel/ipsec.c | 17 ++++++++--------- .../mellanox/mlx5/core/en_accel/ipsec.h | 5 ++--- .../mellanox/mlx5/core/en_accel/macsec.c | 11 +++++------ .../net/ethernet/mellanox/mlx5/core/en_main.c | 7 ++----- .../net/ethernet/mellanox/mlx5/core/en_rep.c | 9 +++------ .../mellanox/mlx5/core/eswitch_offloads.c | 12 ++++++------ .../net/ethernet/mellanox/mlx5/core/lib/aso.c | 10 +++++----- .../net/ethernet/mellanox/mlx5/core/lib/aso.h | 2 +- 9 files changed, 32 insertions(+), 42 deletions(-)
Signature · 1 line
2.37.3
PATCH 1/6 net/mlx5e: Support devlink reload of IPsec core
From: Leon Romanovsky <[email protected]>
Change IPsec initialization flow to allow future creation of hardware resources that should be released and allocated during devlink reload operation. As part of that change, update function signature to be void as no callers are actually interested in it.
--- .../mellanox/mlx5/core/en_accel/ipsec.c | 17 ++++++++--------- .../mellanox/mlx5/core/en_accel/ipsec.h | 5 ++--- .../net/ethernet/mellanox/mlx5/core/en_main.c | 7 ++----- .../net/ethernet/mellanox/mlx5/core/en_rep.c | 9 +++------ 4 files changed, 15 insertions(+), 23 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c +8 -9
@@ -348,29 +348,27 @@ static void mlx5e_xfrm_free_state(struct xfrm_state *x)kfree (sa_entry); } -int mlx5e_ipsec_init (struct mlx5e_priv *priv) +void mlx5e_ipsec_init (struct mlx5e_priv *priv) {struct mlx5e_ipsec *ipsec; -int ret; +int ret = -ENOMEM ;if (!mlx5_ipsec_device_caps (priv->mdev)) {netdev_dbg (priv->netdev,"Not an IPSec offload device\n" ); -return 0 ; +return ; } ipsec =kzalloc (sizeof (*ipsec),GFP_KERNEL );if (!ipsec) -return -ENOMEM ; +return ;hash_init (ipsec->sadb_rx);spin_lock_init (&ipsec->sadb_rx_lock); ipsec->mdev = priv->mdev; ipsec->wq =alloc_ordered_workqueue ("mlx5e_ipsec: %s" ,0 , priv->netdev->name); -if (!ipsec->wq) { - ret = -ENOMEM ; +if (!ipsec->wq) goto err_wq; - } ret =mlx5e_accel_ipsec_fs_init (ipsec);if (ret)
@@ -378,13 +376,14 @@ int mlx5e_ipsec_init(struct mlx5e_priv *priv) priv->ipsec = ipsec;netdev_dbg (priv->netdev,"IPSec attached to netdevice\n" ); -return 0 ; +return ; err_fs_init:destroy_workqueue (ipsec->wq); err_wq:kfree (ipsec); -return (ret != -EOPNOTSUPP ) ? ret :0 ; +mlx5_core_err (priv->mdev,"IPSec initialization failed, %d\n" , ret); +return ; }void mlx5e_ipsec_cleanup (struct mlx5e_priv *priv)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h +2 -3
@@ -146,7 +146,7 @@ struct mlx5e_ipsec_sa_entry {struct mlx5e_ipsec_modify_state_work modify_work; }; -int mlx5e_ipsec_init (struct mlx5e_priv *priv); +void mlx5e_ipsec_init (struct mlx5e_priv *priv);void mlx5e_ipsec_cleanup (struct mlx5e_priv *priv);void mlx5e_ipsec_build_netdev (struct mlx5e_priv *priv);
@@ -174,9 +174,8 @@ mlx5e_ipsec_sa2dev(struct mlx5e_ipsec_sa_entry *sa_entry) return sa_entry->ipsec->mdev; }#else -static inline int mlx5e_ipsec_init (struct mlx5e_priv *priv) +static inline void mlx5e_ipsec_init (struct mlx5e_priv *priv) { -return 0 ; }static inline void mlx5e_ipsec_cleanup (struct mlx5e_priv *priv)
drivers/net/ethernet/mellanox/mlx5/core/en_main.c +2 -5
@@ -5225,10 +5225,6 @@ static int mlx5e_nic_init(struct mlx5_core_dev *mdev, } priv->fs = fs; - err =mlx5e_ipsec_init (priv); -if (err) -mlx5_core_err (mdev,"IPSec initialization failed, %d\n" , err); - err =mlx5e_ktls_init (priv);if (err)mlx5_core_err (mdev,"TLS initialization failed, %d\n" , err);
@@ -5241,7 +5237,6 @@ static void mlx5e_nic_cleanup(struct mlx5e_priv *priv) {mlx5e_health_destroy_reporters (priv);mlx5e_ktls_cleanup (priv); -mlx5e_ipsec_cleanup (priv);mlx5e_fs_cleanup (priv->fs); }
@@ -5370,6 +5365,7 @@ static void mlx5e_nic_enable(struct mlx5e_priv *priv)int err;mlx5e_fs_init_l2_addr (priv->fs, netdev); +mlx5e_ipsec_init (priv); err =mlx5e_macsec_init (priv);if (err)
@@ -5433,6 +5429,7 @@ static void mlx5e_nic_disable(struct mlx5e_priv *priv)mlx5_lag_remove_netdev (mdev, priv->netdev);mlx5_vxlan_reset_to_default (mdev->vxlan);mlx5e_macsec_cleanup (priv); +mlx5e_ipsec_cleanup (priv); }int mlx5e_update_nic_rx (struct mlx5e_priv *priv)
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c +3 -6
@@ -761,7 +761,6 @@ static int mlx5e_init_ul_rep(struct mlx5_core_dev *mdev, struct net_device *netdev) {struct mlx5e_priv *priv =netdev_priv (netdev); -int err; priv->fs =mlx5e_fs_init (priv->profile, mdev, !test_bit (MLX5E_STATE_DESTROYING , &priv->state));
@@ -770,10 +769,6 @@ static int mlx5e_init_ul_rep(struct mlx5_core_dev *mdev,return -ENOMEM ; } - err =mlx5e_ipsec_init (priv); -if (err) -mlx5_core_err (mdev,"Uplink rep IPsec initialization failed, %d\n" , err); -mlx5e_vxlan_set_netdev_info (priv);mlx5e_build_rep_params (netdev);mlx5e_timestamp_init (priv);
@@ -783,7 +778,6 @@ static int mlx5e_init_ul_rep(struct mlx5_core_dev *mdev,static void mlx5e_cleanup_rep (struct mlx5e_priv *priv) {mlx5e_fs_cleanup (priv->fs); -mlx5e_ipsec_cleanup (priv); }static int mlx5e_create_rep_ttc_table (struct mlx5e_priv *priv)
@@ -1074,6 +1068,8 @@ static void mlx5e_rep_enable(struct mlx5e_priv *priv) {struct mlx5e_rep_priv *rpriv = priv->ppriv; +mlx5e_ipsec_init (priv); +mlx5e_set_netdev_mtu_boundaries (priv);mlx5e_rep_neigh_init (rpriv); }
@@ -1083,6 +1079,7 @@ static void mlx5e_rep_disable(struct mlx5e_priv *priv)struct mlx5e_rep_priv *rpriv = priv->ppriv;mlx5e_rep_neigh_cleanup (rpriv); +mlx5e_ipsec_cleanup (priv); }static int mlx5e_update_rep_rx (struct mlx5e_priv *priv)
Signature · 1 line
2.37.3
Re: [PATCH net-next v1 1/6] net/mlx5e: Support devlink reload of IPsec core
Quoted 9 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Change IPsec initialization flow to allow future creation of hardware resources that should be released and allocated during devlink reload operation. As part of that change, update function signature to be void as no callers are actually interested in it.
Signed-off-by Leon Romanovsky <[email protected]>---
...
Quoted 13 lines
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c +0
-1@@ -761,7 +761,6 @@ static int mlx5e_init_ul_rep(struct mlx5_core_dev *mdev, struct net_device *netdev) {struct mlx5e_priv *priv =netdev_priv (netdev); -int err; priv->fs =mlx5e_fs_init (priv->profile, mdev, !test_bit (MLX5E_STATE_DESTROYING , &priv->state));
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c static int mlx5e_init_ul_rep(struct mlx5_core_dev *mdev, · line 776
Quoted 7 lines
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c +0
-4@@ -770,10 +769,6 @@ static int mlx5e_init_ul_rep(struct mlx5_core_dev *mdev,return -ENOMEM ; } - err =mlx5e_ipsec_init (priv); -if (err) -mlx5_core_err (mdev,"Uplink rep IPsec initialization failed, %d\n" , err); -
Original code had ipsec enabled for uplink rep and nic profile only, but not other vport reps. your below code will enable ipsec and alloc resources for other vport reps, which will be a huge waste of memory and hw resources on switchdev system with lots of vports.
Please make sure not to enable ipsec on ALL reps.
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c static void mlx5e_rep_enable(struct mlx5e_priv *priv)
Quoted 12 lines
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c +0
-1mlx5e_vxlan_set_netdev_info (priv);mlx5e_build_rep_params (netdev);mlx5e_timestamp_init (priv);@@ -783,7 +778,6 @@ static int mlx5e_init_ul_rep(struct mlx5_core_dev *mdev,static void mlx5e_cleanup_rep (struct mlx5e_priv *priv) {mlx5e_fs_cleanup (priv->fs); -mlx5e_ipsec_cleanup (priv); }static int mlx5e_create_rep_ttc_table (struct mlx5e_priv *priv)@@ -1074,6 +1068,8 @@ static void mlx5e_rep_enable(struct mlx5e_priv *priv)
this function isn't invoked from uplink rep, so you broke functionality.
Quoted 19 lines
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c +3
-0{struct mlx5e_rep_priv *rpriv = priv->ppriv; +mlx5e_ipsec_init (priv); +mlx5e_set_netdev_mtu_boundaries (priv);mlx5e_rep_neigh_init (rpriv); }@@ -1083,6 +1079,7 @@ static void mlx5e_rep_disable(struct mlx5e_priv *priv)struct mlx5e_rep_priv *rpriv = priv->ppriv;mlx5e_rep_neigh_cleanup (rpriv); +mlx5e_ipsec_cleanup (priv); }static int mlx5e_update_rep_rx (struct mlx5e_priv *priv)Signature · 1 line
2.37.3
PATCH 2/6 net/mlx5: Return ready to use ASO WQE
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
There is no need in hiding returned ASO WQE type by providing void*, use the real type instead. Do it together with zeroing that memory, so ASO WQE will be ready to use immediately.
--- drivers/net/ethernet/mellanox/mlx5/core/en/tc/meter.c | 1 - drivers/net/ethernet/mellanox/mlx5/core/lib/aso.c | 7 +++++-- drivers/net/ethernet/mellanox/mlx5/core/lib/aso.h | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en/tc/meter.c +0 -1
@@ -162,7 +162,6 @@ mlx5e_tc_meter_modify(struct mlx5_core_dev *mdev, MLX5_ACCESS_ASO_OPC_MOD_FLOW_METER); aso_ctrl = &aso_wqe->aso_ctrl; -memset (aso_ctrl,0 ,sizeof (*aso_ctrl)); aso_ctrl->data_mask_mode =MLX5_ASO_DATA_MASK_MODE_BYTEWISE_64BYTE <<6 ; aso_ctrl->condition_1_0_operand =MLX5_ASO_ALWAYS_TRUE |MLX5_ASO_ALWAYS_TRUE <<4 ;
drivers/net/ethernet/mellanox/mlx5/core/lib/aso.c +5 -2
@@ -349,12 +349,15 @@ void mlx5_aso_build_wqe(struct mlx5_aso *aso, u8 ds_cnt, cseg->general_id =cpu_to_be32 (obj_id); } -void *mlx5_aso_get_wqe (struct mlx5_aso *aso) +struct mlx5_aso_wqe *mlx5_aso_get_wqe (struct mlx5_aso *aso) { +struct mlx5_aso_wqe *wqe;u16 pi; pi =mlx5_wq_cyc_ctr2ix (&aso->wq, aso->pc); -return mlx5_wq_cyc_get_wqe (&aso->wq, pi); + wqe =mlx5_wq_cyc_get_wqe (&aso->wq, pi); +memset (wqe,0 ,sizeof (*wqe)); +return wqe; } void mlx5_aso_post_wqe(struct mlx5_aso *aso, bool with_data,
drivers/net/ethernet/mellanox/mlx5/core/lib/aso.h +1 -1
@@ -77,7 +77,7 @@ enum {struct mlx5_aso ; -void *mlx5_aso_get_wqe (struct mlx5_aso *aso); +struct mlx5_aso_wqe *mlx5_aso_get_wqe (struct mlx5_aso *aso);void mlx5_aso_build_wqe (struct mlx5_aso *aso,u8 ds_cnt,struct mlx5_aso_wqe *aso_wqe,u32 obj_id,u32 opc_mode);
Signature · 1 line
2.37.3
PATCH 3/6 net/mlx5e: Don't access directly DMA device pointer
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
Use specialized helper to fetch DMA device pointer.
--- drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c +1 -1
@@ -186,7 +186,7 @@ static int mlx5e_macsec_aso_reg_mr(struct mlx5_core_dev *mdev, struct mlx5e_macsreturn err; } - dma_device = &mdev->pdev->dev; + dma_device =mlx5_core_dma_dev (mdev); dma_addr =dma_map_single (dma_device, umr->ctx,sizeof (umr->ctx),DMA_BIDIRECTIONAL ); err =dma_mapping_error (dma_device, dma_addr);if (err) {
Signature · 1 line
2.37.3
PATCH 4/6 net/mlx5e: Delete always true DMA check
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
DMA address always exists for MACsec ASO object.
--- .../net/ethernet/mellanox/mlx5/core/en_accel/macsec.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c +4 -5
@@ -1304,11 +1304,10 @@ static void macsec_aso_build_wqe_ctrl_seg(struct mlx5e_macsec_aso *macsec_aso, struct mlx5_aso_ctrl_param *param) {memset (aso_ctrl,0 ,sizeof (*aso_ctrl)); -if (macsec_aso->umr->dma_addr) { - aso_ctrl->va_l =cpu_to_be32 (macsec_aso->umr->dma_addr |ASO_CTRL_READ_EN ); - aso_ctrl->va_h =cpu_to_be32 ((u64 )macsec_aso->umr->dma_addr >>32 ); - aso_ctrl->l_key =cpu_to_be32 (macsec_aso->umr->mkey); - } + aso_ctrl->va_l = +cpu_to_be32 (macsec_aso->umr->dma_addr |ASO_CTRL_READ_EN ); + aso_ctrl->va_h =cpu_to_be32 ((u64 )macsec_aso->umr->dma_addr >>32 ); + aso_ctrl->l_key =cpu_to_be32 (macsec_aso->umr->mkey);if (!param)return ;
Signature · 1 line
2.37.3
PATCH 5/6 net/mlx5: Remove redundant check
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
If ASO failed in creation, it won't be called to destroy either. The kernel coding pattern is to make sure that callers are calling to destroy only for valid objects.
--- drivers/net/ethernet/mellanox/mlx5/core/lib/aso.c | 3 --- 1 file changed, 3 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/lib/aso.c +0 -3
@@ -327,9 +327,6 @@ struct mlx5_aso *mlx5_aso_create(struct mlx5_core_dev *mdev, u32 pdn)void mlx5_aso_destroy (struct mlx5_aso *aso) { -if (IS_ERR_OR_NULL (aso)) -return ; -mlx5_aso_destroy_sq (aso);mlx5_aso_destroy_cq (&aso->cq);kfree (aso);
Signature · 1 line
2.37.3
PATCH 6/6 net/mlx5e: Use read lock for eswitch get callbacks
In reply to Leon RomanovskyFrom: Leon Romanovsky <[email protected]>
In commit 367dfa121205 ("net/mlx5: Remove devl_unlock from mlx5_eswtich_mode_callback_enter") all functions were converted to use write lock without relation to their actual purpose.
Change the devlink eswitch getters to use read and not write locks.
--- .../ethernet/mellanox/mlx5/core/eswitch_offloads.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c +6 -6
@@ -3553,9 +3553,9 @@ int mlx5_devlink_eswitch_mode_get(struct devlink *devlink, u16 *mode)if (IS_ERR (esw))return PTR_ERR (esw); -down_write (&esw->mode_lock); +down_read (&esw->mode_lock); err =esw_mode_to_devlink (esw->mode, mode); -up_write (&esw->mode_lock); +up_read (&esw->mode_lock);return err; }
@@ -3653,9 +3653,9 @@ int mlx5_devlink_eswitch_inline_mode_get(struct devlink *devlink, u8 *mode)if (IS_ERR (esw))return PTR_ERR (esw); -down_write (&esw->mode_lock); +down_read (&esw->mode_lock); err =esw_inline_mode_to_devlink (esw->offloads.inline_mode, mode); -up_write (&esw->mode_lock); +up_read (&esw->mode_lock);return err; }
@@ -3727,9 +3727,9 @@ int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink,if (IS_ERR (esw))return PTR_ERR (esw); -down_write (&esw->mode_lock); +down_read (&esw->mode_lock); *encap = esw->offloads.encap; -up_write (&esw->mode_lock); +up_read (&esw->mode_lock);return 0 ; }
Signature · 1 line
2.37.3
Quoted 7 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Changelog:
v1:
Patch #1:
* Removed extra blank line
* Removed ipsec initialization for uplink
This will break functionality, ipsec should only be enabled on nic and uplink but not on other vport representors.
Leon let's finish review internally, this series has to go through my trees anyways.
Quoted 11 lines · Saeed Mahameed
Quoted 7 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Changelog:
v1:
Patch #1:
* Removed extra blank line
* Removed ipsec initialization for uplinkThis will break functionality, ipsec should only be enabled on nic and uplink but not on other vport representors.
I didn't hear any complain from verification. The devlink patch is in my tree for months already.
Quoted 3 lines
Leon let's finish review internally, this series has to go through my trees anyways.
What about other 5 patches? Let's progress with them, please.
Thanks
Quoted 2 lines
Quoted 10 lines · Leon Romanovsky
Quoted 9 lines · Saeed Mahameed
Quoted 7 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Changelog:
v1:
Patch #1:
* Removed extra blank line
* Removed ipsec initialization for uplink
Quoted 6 lines
Quoted 2 lines
This will break functionality, ipsec should only be enabled on nic and uplink but not on other vport representors.
I didn't hear any complain from verification. The devlink patch is in my tree for months already.
the regression is clear in the code. Ask Raed he added the functionality for uplink.
Quoted 6 lines
Quoted 3 lines
Leon let's finish review internally, this series has to go through my trees anyways.
What about other 5 patches? Let's progress with them, please.
Sure, I will take other patches into my tree, but please let's do internal reviews next time, no need to clutter ML with mlx5 specific stuff.
Quoted 14 lines · Saeed Mahameed
Quoted 13 lines · Leon Romanovsky
Quoted 11 lines · Saeed Mahameed
Quoted 7 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Changelog:
v1:
Patch #1:
* Removed extra blank line
* Removed ipsec initialization for uplinkThis will break functionality, ipsec should only be enabled on nic and uplink but not on other vport representors.
Quoted 6 lines
Quoted 3 lines
I didn't hear any complain from verification. The devlink patch is in my tree for months already.
the regression is clear in the code. Ask Raed he added the functionality for uplink.
Just to wrap this discussion. Last half a year or so, all IPsec bugs found internally are assigned on me.
Thanks