[mlx5-next,01/17] net/mlx5: Simplify IPsec flow steering init/cleanup functions

Commit Message

Leon Romanovsky April 10, 2022, 8:28 a.m. UTC

From: Leon Romanovsky <[e-mail]>

Cleanup IPsec FS initialization and cleanup functions.

Reviewed-by: Raed Salem <[e-mail]>
Signed-off-by: Leon Romanovsky <[e-mail]>
---
 .../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(-)

Comments

Saeed Mahameed April 10, 2022, 4:46 p.m. UTC | #1

On 10 Apr 11:28, Leon Romanovsky wrote:
>From: Leon Romanovsky <[e-mail]>
>
>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 <[e-mail]>
>Signed-off-by: Leon Romanovsky <[e-mail]>
>---
> .../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
>

Leon Romanovsky April 10, 2022, 5:21 p.m. UTC | #2

On Sun, Apr 10, 2022 at 09:46:20AM -0700, Saeed Mahameed wrote:
> On 10 Apr 11:28, Leon Romanovsky wrote:
> > From: Leon Romanovsky <[e-mail]>
> > 
> > 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.

And how would you describe extensive usage of one time called functions
that have no use as standalone ones?

This patch makes sure that all flow steering initialized and cleaned at
one place and allows me to present coherent picture of what is needed
for IPsec FS.

You should focus on the end result of this series rather on single patch.
 15 files changed, 320 insertions(+), 839 deletions(-)

> 
> 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.

I'm deleting include/linux/mlx5/accel.h, it is not needed.
But I don't care about the target, it can be net-next and not
mlx5-next.

> 
> Please give me a chance to review the whole series until next week as i am
> out of office this week.

I see this problematic as next week will be combination of my Passover
vacation and paternity leave at the same time.

Thanks

> 
> > 
> > Reviewed-by: Raed Salem <[e-mail]>
> > Signed-off-by: Leon Romanovsky <[e-mail]>
> > ---
> > .../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
> >

Saeed Mahameed April 10, 2022, 9:58 p.m. UTC | #3

On 10 Apr 20:21, Leon Romanovsky wrote:
>On Sun, Apr 10, 2022 at 09:46:20AM -0700, Saeed Mahameed wrote:
>> On 10 Apr 11:28, Leon Romanovsky wrote:
>> > From: Leon Romanovsky <[e-mail]>
>> >
>> > 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.
>
>And how would you describe extensive usage of one time called functions
>that have no use as standalone ones?
>

Functional programming.

>This patch makes sure that all flow steering initialized and cleaned at
>one place and allows me to present coherent picture of what is needed
>for IPsec FS.
>

This is already the case before this patch.

>You should focus on the end result of this series rather on single patch.
> 15 files changed, 320 insertions(+), 839 deletions(-)

Overall the series is fine, this patch in particular is unnecessary cancelation of
others previous decisions, which i personally like and might as well have
suggested myself, so let's avoid such clutter.

Leon Romanovsky April 11, 2022, 6:37 a.m. UTC | #4

On Sun, Apr 10, 2022 at 02:58:13PM -0700, Saeed Mahameed wrote:
> On 10 Apr 20:21, Leon Romanovsky wrote:
> > On Sun, Apr 10, 2022 at 09:46:20AM -0700, Saeed Mahameed wrote:
> > > On 10 Apr 11:28, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <[e-mail]>
> > > >
> > > > 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.
> > 
> > And how would you describe extensive usage of one time called functions
> > that have no use as standalone ones?
> > 
> 
> Functional programming.

The separation between various modules and their functions is function
programming, but wrapper in .c file over basic kernel primitive (kzalloc)
is obfuscation.

> 
> > This patch makes sure that all flow steering initialized and cleaned at
> > one place and allows me to present coherent picture of what is needed
> > for IPsec FS.
> > 
> 
> This is already the case before this patch.

With two main differences: 
First, it is is less code to achieve the same and second, it is easy
to read (I read this code a lot lately).

 3 files changed, 27 insertions(+), 54 deletions(-)

> 
> > You should focus on the end result of this series rather on single patch.
> > 15 files changed, 320 insertions(+), 839 deletions(-)
> 
> Overall the series is fine, this patch in particular is unnecessary cancelation of
> others previous decisions, which i personally like and might as well have
> suggested myself, so let's avoid such clutter.

Sorry, but I disagree that removal of useless indirection that hurts
readability is clutter. It is refactoring.

Please focus on end goal of this series.

Thanks

> 
>

Patch

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,