Re: [PATCH net-next v2] eth: mlx5: link NAPI instances to queues and IRQs

[Rahul Rameshbabu <rrameshbabu@nvidia.com>]

On Tue, 06 Feb, 2024 02:49:56 +0000 Joe Damato <jdamato@fastly.com> wrote:
> Make mlx5 compatible with the newly added netlink queue GET APIs.
>
> v1 -> v2:
>   - Move netlink NULL code to mlx5e_deactivate_channel
>   - Move netif_napi_set_irq to mlx5e_open_channel and avoid storing the
>     irq, after netif_napi_add which itself sets the IRQ to -1.
>   - Fix white space where IRQ is stored in mlx5e_open_channel
>
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index c8e8f512803e..3e74c7de6050 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -2560,6 +2560,7 @@ static int mlx5e_open_channel(struct mlx5e_priv *priv, int ix,
>  	c->lag_port = mlx5e_enumerate_lag_port(priv->mdev, ix);
>  
>  	netif_napi_add(netdev, &c->napi, mlx5e_napi_poll);
> +	netif_napi_set_irq(&c->napi, irq);
>  
>  	err = mlx5e_open_queues(c, params, cparam);
>  	if (unlikely(err))
> @@ -2602,6 +2603,9 @@ static void mlx5e_activate_channel(struct mlx5e_channel *c)
>  		mlx5e_activate_xsk(c);
>  	else
>  		mlx5e_activate_rq(&c->rq);
> +
> +	netif_queue_set_napi(c->netdev, c->ix, NETDEV_QUEUE_TYPE_TX, &c->napi);
> +	netif_queue_set_napi(c->netdev, c->ix, NETDEV_QUEUE_TYPE_RX, &c->napi);
>  }
>  
>  static void mlx5e_deactivate_channel(struct mlx5e_channel *c)
> @@ -2619,6 +2623,9 @@ static void mlx5e_deactivate_channel(struct mlx5e_channel *c)
>  		mlx5e_deactivate_txqsq(&c->sq[tc]);
>  	mlx5e_qos_deactivate_queues(c);
>  
> +	netif_queue_set_napi(c->netdev, c->ix, NETDEV_QUEUE_TYPE_TX, NULL);
> +	netif_queue_set_napi(c->netdev, c->ix, NETDEV_QUEUE_TYPE_RX, NULL);
> +

I think it would be better to clean the associations before actually
deactivating the queues. Your teardown becomes LIFO/flipped order of
what is done in mlx5_activate_channel.

>  	napi_disable(&c->napi);
>  }

In general, the netdev community maintains a rule for not reposting new
versions of patches in 24hr periods to avoid these types of situations.

Lets add the feedback of updating the commit message feedback in the v1
thread into v3.

       https://lore.kernel.org/netdev/20240206025153.GA11388@fastly.com/T/#mcbf987c817c0d06c29364410ba8ab10b144c753d

Lets send out that v3 a day from now if that's alright. This way we can
pick up feedback from others if needed, but I think we are converging
here.

--
Thanks,

Rahul Rameshbabu