[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220410164620.2dfzhx6qt4cg6b6o@sx1>
Date: Sun, 10 Apr 2022 09:46:20 -0700
From: Saeed Mahameed <saeedm@...dia.com>
To: Leon Romanovsky <leon@...nel.org>
Cc: Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
"David S . Miller" <davem@...emloft.net>,
Leon Romanovsky <leonro@...dia.com>,
Jason Gunthorpe <jgg@...dia.com>,
linux-netdev <netdev@...r.kernel.org>,
RDMA mailing list <linux-rdma@...r.kernel.org>,
Raed Salem <raeds@...dia.com>
Subject: Re: [PATCH mlx5-next 01/17] net/mlx5: Simplify IPsec flow steering
init/cleanup functions
On 10 Apr 11:28, Leon Romanovsky wrote:
>From: Leon Romanovsky <leonro@...dia.com>
>
>Cleanup IPsec FS initialization and cleanup functions.
Can you be more clear about what are you cleaning up ?
unfolding/joining static functions shouldn't be considered as cleanup.
Also i don't agree these patches should go to mlx5-next, we need to avoid
bloating mlx5-next.
Many of these patches are purely ipsec, yes i understand you are heavily
modifying include/linux/mlx5/accel.h but from what I can tell, it's not
affecting rdma.
Please give me a chance to review the whole series until next week as i am
out of office this week.
>
>Reviewed-by: Raed Salem <raeds@...dia.com>
>Signed-off-by: Leon Romanovsky <leonro@...dia.com>
>---
> .../mellanox/mlx5/core/en_accel/ipsec.c | 4 +-
> .../mellanox/mlx5/core/en_accel/ipsec_fs.c | 73 ++++++-------------
> .../mellanox/mlx5/core/en_accel/ipsec_fs.h | 4 +-
> 3 files changed, 27 insertions(+), 54 deletions(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
>index c280a18ff002..5a10755dd4f1 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
>@@ -424,7 +424,7 @@ int mlx5e_ipsec_init(struct mlx5e_priv *priv)
> }
>
> priv->ipsec = ipsec;
>- mlx5e_accel_ipsec_fs_init(priv);
>+ mlx5e_ipsec_fs_init(ipsec);
> netdev_dbg(priv->netdev, "IPSec attached to netdevice\n");
> return 0;
> }
>@@ -436,7 +436,7 @@ void mlx5e_ipsec_cleanup(struct mlx5e_priv *priv)
> if (!ipsec)
> return;
>
>- mlx5e_accel_ipsec_fs_cleanup(priv);
>+ mlx5e_ipsec_fs_cleanup(ipsec);
> destroy_workqueue(ipsec->wq);
>
> kfree(ipsec);
>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 66b529e36ea1..869b5692e9b9 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
>@@ -632,81 +632,54 @@ void mlx5e_accel_ipsec_fs_del_rule(struct mlx5e_priv *priv,
> tx_del_rule(priv, ipsec_rule);
> }
>
>-static void fs_cleanup_tx(struct mlx5e_priv *priv)
>-{
>- mutex_destroy(&priv->ipsec->tx_fs->mutex);
>- WARN_ON(priv->ipsec->tx_fs->refcnt);
>- kfree(priv->ipsec->tx_fs);
>- priv->ipsec->tx_fs = NULL;
>-}
>-
>-static void fs_cleanup_rx(struct mlx5e_priv *priv)
>+void mlx5e_ipsec_fs_cleanup(struct mlx5e_ipsec *ipsec)
> {
> struct mlx5e_accel_fs_esp_prot *fs_prot;
> struct mlx5e_accel_fs_esp *accel_esp;
> enum accel_fs_esp_type i;
>
>- accel_esp = priv->ipsec->rx_fs;
>+ if (!ipsec->rx_fs)
>+ return;
>+
>+ mutex_destroy(&ipsec->tx_fs->mutex);
>+ WARN_ON(ipsec->tx_fs->refcnt);
>+ kfree(ipsec->tx_fs);
>+
>+ accel_esp = ipsec->rx_fs;
> for (i = 0; i < ACCEL_FS_ESP_NUM_TYPES; i++) {
> fs_prot = &accel_esp->fs_prot[i];
> mutex_destroy(&fs_prot->prot_mutex);
> WARN_ON(fs_prot->refcnt);
> }
>- kfree(priv->ipsec->rx_fs);
>- priv->ipsec->rx_fs = NULL;
>-}
>-
>-static int fs_init_tx(struct mlx5e_priv *priv)
>-{
>- priv->ipsec->tx_fs =
>- kzalloc(sizeof(struct mlx5e_ipsec_tx), GFP_KERNEL);
>- if (!priv->ipsec->tx_fs)
>- return -ENOMEM;
>-
>- mutex_init(&priv->ipsec->tx_fs->mutex);
>- return 0;
>+ kfree(ipsec->rx_fs);
> }
>
>-static int fs_init_rx(struct mlx5e_priv *priv)
>+int mlx5e_ipsec_fs_init(struct mlx5e_ipsec *ipsec)
> {
> struct mlx5e_accel_fs_esp_prot *fs_prot;
> struct mlx5e_accel_fs_esp *accel_esp;
> enum accel_fs_esp_type i;
>+ int err = -ENOMEM;
>
>- priv->ipsec->rx_fs =
>- kzalloc(sizeof(struct mlx5e_accel_fs_esp), GFP_KERNEL);
>- if (!priv->ipsec->rx_fs)
>+ ipsec->tx_fs = kzalloc(sizeof(*ipsec->tx_fs), GFP_KERNEL);
>+ if (!ipsec->tx_fs)
> return -ENOMEM;
>
>- accel_esp = priv->ipsec->rx_fs;
>+ ipsec->rx_fs = kzalloc(sizeof(*ipsec->rx_fs), GFP_KERNEL);
>+ if (!ipsec->rx_fs)
>+ goto err_rx;
>+
>+ mutex_init(&ipsec->tx_fs->mutex);
>+
>+ accel_esp = ipsec->rx_fs;
> for (i = 0; i < ACCEL_FS_ESP_NUM_TYPES; i++) {
> fs_prot = &accel_esp->fs_prot[i];
> mutex_init(&fs_prot->prot_mutex);
> }
>
> return 0;
>-}
>-
>-void mlx5e_accel_ipsec_fs_cleanup(struct mlx5e_priv *priv)
>-{
>- if (!priv->ipsec->rx_fs)
>- return;
>-
>- fs_cleanup_tx(priv);
>- fs_cleanup_rx(priv);
>-}
>-
>-int mlx5e_accel_ipsec_fs_init(struct mlx5e_priv *priv)
>-{
>- int err;
>-
>- err = fs_init_tx(priv);
>- if (err)
>- return err;
>-
>- err = fs_init_rx(priv);
>- if (err)
>- fs_cleanup_tx(priv);
>
>+err_rx:
>+ kfree(ipsec->tx_fs);
> return err;
> }
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.h
>index b70953979709..8e0e829ab58f 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.h
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.h
>@@ -9,8 +9,8 @@
> #include "ipsec_offload.h"
> #include "en/fs.h"
>
>-void mlx5e_accel_ipsec_fs_cleanup(struct mlx5e_priv *priv);
>-int mlx5e_accel_ipsec_fs_init(struct mlx5e_priv *priv);
>+void mlx5e_ipsec_fs_cleanup(struct mlx5e_ipsec *ipsec);
>+int mlx5e_ipsec_fs_init(struct mlx5e_ipsec *ipsec);
> int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_priv *priv,
> struct mlx5_accel_esp_xfrm_attrs *attrs,
> u32 ipsec_obj_id,
>--
>2.35.1
>
Powered by blists - more mailing lists