mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tariq Toukan <tariqt@nvidia.com>
To: Breno Leitao <leitao@debian.org>,
	Saeed Mahameed <saeedm@nvidia.com>,
	Tariq Toukan <tariqt@nvidia.com>, Mark Bloch <mbloch@nvidia.com>,
	Leon Romanovsky <leon@kernel.org>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Joe Damato <joe@dama.to>
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-team@meta.com,
	stable@vger.kernel.org
Subject: Re: [PATCH net] net/mlx5e: Fix queue stats access to queues without stats
Date: Sun, 8 Nov 2026 14:32:54 +0200	[thread overview]
Message-ID: <fc44a751-ef19-4552-8968-c1f6a53b9f63@nvidia.com> (raw)
In-Reply-To: <20261104-mlx5e-qstats-null-v1-1-a6a003245cec@debian.org>



On 04/11/2026 19:04, Breno Leitao wrote:
> 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(-)
> 
Thanks for the fix. IMO this is the right minimal change for net.

About the Sashiko finding: right, with HTB configured, the HTB TX stats
drop out of the device totals while the netdev is down, as
mlx5e_get_base_stats() does not count htb_qos_sq_stats[]. But before
this patch the same dump crashed, so it is not a regression, and I'd
fix the accounting separately.

There is more to fix there. While the netdev is down, PTP TX stats are
counted twice, once through the stale txq2sq_stats[] entry and once by
mlx5e_get_base_stats(). And the stats of HTB queues that are no longer
in use are never reported.

For net-next, I suggest reporting per-queue stats only while the 
channels are open, and everything else from mlx5e_get_base_stats(), HTB 
queues included. This follows the netdev_stat_ops documentation ("only 
currently active queues are queried by the per-queue callbacks"). The 
per-queue dump would be empty while the netdev is down, with the device 
totals unchanged across ifdown. The comment above the new NULL check 
would be reworked there as well. We can send it once this patch reaches 
net-next.

Reviewed-by: Tariq Toukan <tariqt@nvidia.com>


      parent reply	other threads:[~2026-11-08 12:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04 17:04 Breno Leitao
2026-11-05 17:05 ` netdev-bot+sashiko
2026-11-08 12:32 ` Tariq Toukan [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fc44a751-ef19-4552-8968-c1f6a53b9f63@nvidia.com \
    --to=tariqt@nvidia.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=joe@dama.to \
    --cc=kernel-team@meta.com \
    --cc=kuba@kernel.org \
    --cc=leitao@debian.org \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=mbloch@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=saeedm@nvidia.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®