[PATCH net] net/mlx5e: Fix queue stats access to queues without stats

[PATCH net] net/mlx5e: Fix queue stats access to queues without stats

[Breno Leitao]

I am seeing the following mlx5e crash in the Meta fleet:

  pc : mlx5e_get_queue_stats_rx+0x4c/0xc8
  Call trace:
   mlx5e_get_queue_stats_rx+0x4c/0xc8 (P)
   netdev_stat_queue_sum+0x78/0x480
   netdev_nl_qstats_get_dump_one+0xf0/0x268
   netdev_nl_qstats_get_dumpit+0x7c/0x1f8

When dumping device-wide qstats, the core calls the per-queue callbacks
for every queue below real_num_{rx,tx}_queues, regardless of whether the
netdev is up. Some of those queues have no stats behind them:

 - RX stats live in priv->channel_stats[], which only has entries for
   channels that have been opened (stats_nch). real_num_rx_queues can
   be larger, for example after the channel count is raised while the
   netdev is down, or after opening the channels failed part way.

 - TX stats are looked up in priv->txq2sq_stats[], which is NULL for
   SQs that were never activated and for HTB queues while the channels
   are closed.

In both cases the callback dereferences a NULL pointer. The dump needs
no privileges, so any user can trigger it.

Return early for RX queues at or above stats_nch and for TX queues
without SQ stats.

Fixes: 7b66ae536a78 ("net/mlx5e: Add per queue netdev-genl stats")
Cc: stable@vger.kernel.org
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index c3c2c5720c6f32..72bca10c579df9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -5489,7 +5489,7 @@ static void mlx5e_get_queue_stats_rx(struct net_device *dev, int i,
 	struct mlx5e_rq_stats *xskrq_stats;
 	struct mlx5e_rq_stats *rq_stats;
 
-	if (mlx5e_is_uplink_rep(priv) || !mlx5e_stats_nch_read(priv))
+	if (mlx5e_is_uplink_rep(priv) || i >= mlx5e_stats_nch_read(priv))
 		return;
 
 	channel_stats = priv->channel_stats[i];
@@ -5521,6 +5521,9 @@ static void mlx5e_get_queue_stats_tx(struct net_device *dev, int i,
 	 * inactive sqs.
 	 */
 	sq_stats = priv->txq2sq_stats[i];
+	if (!sq_stats)
+		return;
+
 	stats->packets = sq_stats->packets;
 	stats->bytes = sq_stats->bytes;
 

---
base-commit: 241dec6f9ab74d8c3ef246764f3611666b96b9e1
change-id: 20261104-mlx5e-qstats-null-39d642e7eef7

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH net] net/mlx5e: Fix queue stats access to queues without stats

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] When HTB leaves are configured and the netdev is brought down,
  the device-wide qstats lose all HTB TX counters.

--

net/mlx5e: Fix queue stats access to queues without stats

This commit fixes NULL pointer dereferences in the mlx5e per-queue
netdev-genl stats callbacks. The core calls them for every queue below
real_num_{rx,tx}_queues, and some of those queues have no stats behind
them. The RX callback now returns early for queues at or above stats_nch,
and the TX callback returns early when txq2sq_stats[i] is NULL.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index c3c2c5720c6f3..72bca10c579df 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c

[ ... ]

> @@ -5521,6 +5521,9 @@ static void mlx5e_get_queue_stats_tx(struct net_device *dev, int i,
>  	 * inactive sqs.
>  	 */
>  	sq_stats = priv->txq2sq_stats[i];
> +	if (!sq_stats)
> +		return;
> +

[Severity: Medium]
With HTB offload leaves configured, do the HTB TX counters drop out of
the device-wide qstats totals while the netdev is down?

The early return depends on the comment above it, which says
get_base_stats "takes care of inactive sqs". For HTB SQs that does not
seem to hold.

On ifdown, mlx5e_deactivate_channel()->mlx5e_qos_deactivate_queues()
clears the entry for every HTB txq:

drivers/net/ethernet/mellanox/mlx5/core/en/qos.c:mlx5e_qos_deactivate_queues() {
    ...
		c->priv->txq2sq[txq_ix] = NULL;
		c->priv->txq2sq_stats[txq_ix] = NULL;
    ...
}

real_num_tx_queues still includes those txqs, because
mlx5e_update_tx_netdev_queues() sets it to nch * ntc +
mlx5e_htb_cur_leaf_nodes(priv->htb) whether or not the channels are
open. So netdev_nl_stats_by_netdev() still calls
mlx5e_get_queue_stats_tx() for those indices, and with this change it
now returns without reporting anything.

mlx5e_get_base_stats() only walks channel_stats[]->sq[tc] and
ptp_stats.sq[tc]:

	for (i = 0; i < nch; i++) {
		struct mlx5e_channel_stats *channel_stats = priv->channel_stats[i];
		...

It never reads priv->htb_qos_sq_stats[], which is where the persistent
HTB counters live. So tx packets, bytes, hw-gso-packets, hw-gso-bytes,
csum-none, stop and wake for HTB queues disappear from the total on
ifdown and come back on ifup.

That looks like it would make device qstats go backwards on ifdown,
which tools/testing/selftests/drivers/net/stats.py check_down() tests
for:

    ksft_ge(qstat2[k], qstat[k], comment=f"{k} went backwards on device down")

The qstats totals would also stop matching the SW/rtnl totals, since
mlx5e_stats_grp_sw_update_stats_qos() still adds every
htb_qos_sq_stats[i].

Should mlx5e_get_base_stats() add priv->htb_qos_sq_stats[] for HTB
queues whose txq2sq_stats entry is NULL? If not, could the comment
above be corrected to document this gap?

>  	stats->packets = sq_stats->packets;
>  	stats->bytes = sq_stats->bytes;
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261104-mlx5e-qstats-null-v1-1-a6a003245cec%40debian.org