linux-kernel - Re: [PATCH net-next 3/3] net/mlx4: support per-queue statistics via netlink

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 23 Apr 2024 17:57:18 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Joe Damato <jdamato@...tly.com>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org, tariqt@...dia.com,
 saeedm@...dia.com, mkarsten@...terloo.ca, gal@...dia.com,
 nalramli@...tly.com, "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, "open list:MELLANOX
 MLX4 core VPI driver" <linux-rdma@...r.kernel.org>
Subject: Re: [PATCH net-next 3/3] net/mlx4: support per-queue statistics via
 netlink

On Tue, 23 Apr 2024 12:42:13 -1000 Joe Damato wrote:
> I realized in this case, I'll need to set the fields initialized to 0xff
> above to 0 before doing the increments below.

I don't know mlx4 very well, but glancing at the code - are you sure we
need to loop over the queues is the "base" callbacks?

The base callbacks are for getting "historical" data, i.e. info which
was associated with queues which are no longer present. You seem to
sweep all queues, so I'd have expected "base" to just set the values 
to 0. And the real values to come from the per-queue callbacks.

The init to 0xff looks quite sus.

Also what does this:

>	if (!priv->port_up || mlx4_is_master(priv->mdev->dev))

do? 🤔️ what's a "master" in this context?

> Sorry about that; just realized that now and will fix that in the v2 (along
> with any other feedback I get), probably something:
> 
>   if (priv->rx_ring_num) {
>           rx->packets = 0;
>           rx->bytes = 0;
>           rx->alloc_fail = 0;
>   }
> 
> Here for the RX side and see below for the TX side.

FWIW I added a simple test for making sure queue stats match interface
stats, it's tools/testing/selftests/drivers/net/stats.py

You have to export NETIF=$name to make it run on a real interface.

To copy the tests to a remote machine I do:

make -C tools/testing/selftests/ TARGETS="net drivers/net drivers/net/hw" install INSTALL_PATH=/tmp/ksft-net-drv
rsync -ra --delete /tmp/ksft-net-drv root@...achine}:/root/

HTH