diff mbox series

[net-next] net/mlx5: Fix memory leak in IPsec RoCE creation

Message ID 1b414ea3a92aa0d07b6261cf641445f27bc619d8.1676811549.git.leon@kernel.org (mailing list archive)
State Superseded
Headers show
Series [net-next] net/mlx5: Fix memory leak in IPsec RoCE creation | expand

Commit Message

Leon Romanovsky Feb. 19, 2023, 12:59 p.m. UTC 
From: Patrisious Haddad <phaddad@nvidia.com>

During IPsec RoCE TX creation a struct for the flow group creation is
allocated, but never freed. Free that struct once it is no longer in use.

Fixes: 22551e77e550 ("net/mlx5: Configure IPsec steering for egress RoCEv2 traffic")
Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/lib/ipsec_fs_roce.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Jakub Kicinski Feb. 21, 2023, 12:50 a.m. UTC | #1 
On Sun, 19 Feb 2023 14:59:57 +0200 Leon Romanovsky wrote:
> -rule_fail:
> +fail_rule:
>  	mlx5_destroy_flow_group(roce->g);
> -fail:
> +fail_group:
>  	mlx5_destroy_flow_table(ft);
> +fail_table:
> +	kvfree(in);
>  	return err;

If you're touching all of them please name them after what they do.
Much easier to review.
Leon Romanovsky Feb. 21, 2023, 6:39 a.m. UTC | #2 
On Mon, Feb 20, 2023 at 04:50:00PM -0800, Jakub Kicinski wrote:
> On Sun, 19 Feb 2023 14:59:57 +0200 Leon Romanovsky wrote:
> > -rule_fail:
> > +fail_rule:
> >  	mlx5_destroy_flow_group(roce->g);
> > -fail:
> > +fail_group:
> >  	mlx5_destroy_flow_table(ft);
> > +fail_table:
> > +	kvfree(in);
> >  	return err;
> 
> If you're touching all of them please name them after what they do.
> Much easier to review.

I can change it, but all mlx* drivers and randomly chosen place in ice
use label to show what fail and not what will be done. Such notation
gives an ability to refactor code without changing label names if
failed part of code is not removed.

Thanks
Jakub Kicinski Feb. 21, 2023, 4:30 p.m. UTC | #3 
On Tue, 21 Feb 2023 08:39:40 +0200 Leon Romanovsky wrote:
> On Mon, Feb 20, 2023 at 04:50:00PM -0800, Jakub Kicinski wrote:
> > On Sun, 19 Feb 2023 14:59:57 +0200 Leon Romanovsky wrote:  
> > > -rule_fail:
> > > +fail_rule:
> > >  	mlx5_destroy_flow_group(roce->g);
> > > -fail:
> > > +fail_group:
> > >  	mlx5_destroy_flow_table(ft);
> > > +fail_table:
> > > +	kvfree(in);
> > >  	return err;  
> > 
> > If you're touching all of them please name them after what they do.
> > Much easier to review.  
> 
> I can change it, but all mlx* drivers and randomly chosen place in ice
> use label to show what fail and not what will be done. Such notation
> gives an ability to refactor code without changing label names if
> failed part of code is not removed.

Please refactor, and it'd be great if the convention was changed for
all new code in these drivers.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/ipsec_fs_roce.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/ipsec_fs_roce.c
index 2c53589b765d..edc6798b359e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/ipsec_fs_roce.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/ipsec_fs_roce.c
@@ -162,7 +162,7 @@  int mlx5_ipsec_fs_roce_tx_create(struct mlx5_core_dev *mdev,
 	if (IS_ERR(ft)) {
 		err = PTR_ERR(ft);
 		mlx5_core_err(mdev, "Fail to create RoCE IPsec tx ft err=%d\n", err);
-		return err;
+		goto fail_table;
 	}
 
 	roce->ft = ft;
@@ -174,22 +174,25 @@  int mlx5_ipsec_fs_roce_tx_create(struct mlx5_core_dev *mdev,
 	if (IS_ERR(g)) {
 		err = PTR_ERR(g);
 		mlx5_core_err(mdev, "Fail to create RoCE IPsec tx group err=%d\n", err);
-		goto fail;
+		goto fail_group;
 	}
 	roce->g = g;
 
 	err = ipsec_fs_roce_tx_rule_setup(mdev, roce, pol_ft);
 	if (err) {
 		mlx5_core_err(mdev, "Fail to create RoCE IPsec tx rules err=%d\n", err);
-		goto rule_fail;
+		goto fail_rule;
 	}
 
+	kvfree(in);
 	return 0;
 
-rule_fail:
+fail_rule:
 	mlx5_destroy_flow_group(roce->g);
-fail:
+fail_group:
 	mlx5_destroy_flow_table(ft);
+fail_table:
+	kvfree(in);
 	return err;
 }