[net,01/12] net/mlx5: Fix memory leak on flow table creation error flow

Commit Message

Saeed Mahameed Jan. 26, 2021, 11:43 p.m. UTC

From: Roi Dayan <[e-mail]>

When we create the ft object we also init rhltable in ft->fgs_hash.
So in error flow before kfree of ft we need to destroy that rhltable.

Fixes: 693c6883bbc4 ("net/mlx5: Add hash table for flow groups in flow table")
Signed-off-by: Roi Dayan <[e-mail]>
Reviewed-by: Maor Dickman <[e-mail]>
Signed-off-by: Saeed Mahameed <[e-mail]>
---
 drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 1 +
 1 file changed, 1 insertion(+)

Comments

[e-mail] Jan. 28, 2021, 3:30 a.m. UTC | #1

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Tue, 26 Jan 2021 15:43:34 -0800 you wrote:
> From: Roi Dayan <[e-mail]>
> 
> When we create the ft object we also init rhltable in ft->fgs_hash.
> So in error flow before kfree of ft we need to destroy that rhltable.
> 
> Fixes: 693c6883bbc4 ("net/mlx5: Add hash table for flow groups in flow table")
> Signed-off-by: Roi Dayan <[e-mail]>
> Reviewed-by: Maor Dickman <[e-mail]>
> Signed-off-by: Saeed Mahameed <[e-mail]>
> 
> [...]

Here is the summary with links:
  - [net,01/12] net/mlx5: Fix memory leak on flow table creation error flow
    https://git.kernel.org/netdev/net/c/487c6ef81eb9
  - [net,02/12] net/mlx5e: E-switch, Fix rate calculation for overflow
    https://git.kernel.org/netdev/net/c/1fe3e3166b35
  - [net,03/12] net/mlx5e: free page before return
    https://git.kernel.org/netdev/net/c/258ed19f075f
  - [net,04/12] net/mlx5e: Reduce tc unsupported key print level
    https://git.kernel.org/netdev/net/c/48470a90a42a
  - [net,05/12] net/mlx5e: Fix IPSEC stats
    https://git.kernel.org/netdev/net/c/45c9a30835d8
  - [net,06/12] net/mlx5: Maintain separate page trees for ECPF and PF functions
    https://git.kernel.org/netdev/net/c/0aa128475d33
  - [net,07/12] net/mlx5e: Disable hw-tc-offload when MLX5_CLS_ACT config is disabled
    https://git.kernel.org/netdev/net/c/156878d0e697
  - [net,08/12] net/mlx5e: Fix CT rule + encap slow path offload and deletion
    https://git.kernel.org/netdev/net/c/89e394675818
  - [net,09/12] net/mlx5e: Correctly handle changing the number of queues when the interface is down
    https://git.kernel.org/netdev/net/c/57ac4a31c483
  - [net,10/12] net/mlx5e: Revert parameters on errors when changing trust state without reset
    https://git.kernel.org/netdev/net/c/912c9b5fcca1
  - [net,11/12] net/mlx5e: Revert parameters on errors when changing MTU and LRO state without reset
    https://git.kernel.org/netdev/net/c/8355060f5ec3
  - [net,12/12] net/mlx5: CT: Fix incorrect removal of tuple_nat_node from nat rhashtable
    https://git.kernel.org/netdev/net/c/e2194a1744e8

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Saeed Mahameed Jan. 28, 2021, 8:33 a.m. UTC | #2

On Thu, 2021-01-28 at 03:30 +0000, [e-mail]
wrote:
> Hello:
> 
> This series was applied to netdev/net.git (refs/heads/master):
> 
> 
...

>     https://git.kernel.org/netdev/net/c/89e394675818
>   - [net,09/12] net/mlx5e: Correctly handle changing the number of
> queues when the interface is down



Hi Jakub, 

I just noticed that this patch will conflict with HTB offlaod feature
in net-next, I couldn't foresee it before in my queues since HTB wasn't
submitted through my trees.

anyway to solve the conflict just use this hunk:

 +      /* Don't allow changing the number of channels if HTB offload
is active,
 +       * because the numeration of the QoS SQs will change, while
per-queue
 +       * qdiscs are attached.
 +       */
 +      if (priv->htb.maj_id) {
 +              err = -EINVAL;
 +              netdev_err(priv->netdev, "%s: HTB offload is active,
cannot change the number of channels\n",
 +                         __func__);
 +              goto out;
 +      }
 +
-       new_channels.params = priv->channels.params;
-       new_channels.params.num_channels = count;
+       new_channels.params = *cur_params;



Thanks,
Saeed.

Jakub Kicinski Jan. 29, 2021, 1:37 a.m. UTC | #3

On Thu, 28 Jan 2021 00:33:20 -0800 Saeed Mahameed wrote:
> On Thu, 2021-01-28 at 03:30 +0000, [e-mail]
> wrote:
> > Hello:
> > 
> > This series was applied to netdev/net.git (refs/heads/master):
> > 
> >   
> ...
> 
> >     https://git.kernel.org/netdev/net/c/89e394675818
> >   - [net,09/12] net/mlx5e: Correctly handle changing the number of
> > queues when the interface is down  
> 
> 
> 
> Hi Jakub, 
> 
> I just noticed that this patch will conflict with HTB offlaod feature
> in net-next, I couldn't foresee it before in my queues since HTB wasn't
> submitted through my trees.
> 
> anyway to solve the conflict just use this hunk:
> 
>  +      /* Don't allow changing the number of channels if HTB offload
> is active,
>  +       * because the numeration of the QoS SQs will change, while
> per-queue
>  +       * qdiscs are attached.
>  +       */
>  +      if (priv->htb.maj_id) {
>  +              err = -EINVAL;
>  +              netdev_err(priv->netdev, "%s: HTB offload is active,
> cannot change the number of channels\n",
>  +                         __func__);
>  +              goto out;
>  +      }
>  +
> -       new_channels.params = priv->channels.params;
> -       new_channels.params.num_channels = count;
> +       new_channels.params = *cur_params;
> 

Done, thanks!

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
index b899539a0786..0fcee702b808 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
@@ -1141,6 +1141,7 @@  static struct mlx5_flow_table *__mlx5_create_flow_table(struct mlx5_flow_namespa
 destroy_ft:
 	root->cmds->destroy_flow_table(root, ft);
 free_ft:
+	rhltable_destroy(&ft->fgs_hash);
 	kfree(ft);
 unlock_root:
 	mutex_unlock(&root->chain_lock);