netdev - Re: [PATCH net 1/5] net: wangxun: fix the incorrect display of queue number in statistics

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]

Message-ID: <20240416101631.GA1411807@electric-eye.fr.zoreil.com>
Date: Tue, 16 Apr 2024 12:16:31 +0200
From: Francois Romieu <romieu@...zoreil.com>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, rmk+kernel@...linux.org.uk, andrew@...n.ch,
        netdev@...r.kernel.org, mengyuanlou@...-swift.com,
        duanqiangwen@...-swift.com
Subject: Re: [PATCH net 1/5] net: wangxun: fix the incorrect display of queue
 number in statistics

Jiawen Wu <jiawenwu@...stnetic.com> :
> When using ethtool -S to print hardware statistics, the number of
> Rx/Tx queues printed is greater than the number of queues actually
> used.
> 
> Fixes: 46b92e10d631 ("net: libwx: support hardware statistics")
> Signed-off-by: Jiawen Wu <jiawenwu@...stnetic.com>
> ---
>  .../net/ethernet/wangxun/libwx/wx_ethtool.c   | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> index cc3bec42ed8e..3847c909ba1a 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
> @@ -59,9 +59,17 @@ static const struct wx_stats wx_gstrings_stats[] = {
>  
>  int wx_get_sset_count(struct net_device *netdev, int sset)
>  {
> +	struct wx *wx = netdev_priv(netdev);
> +
>  	switch (sset) {
>  	case ETH_SS_STATS:
> -		return WX_STATS_LEN;
> +		if (wx->num_tx_queues <= WX_NUM_RX_QUEUES) {
> +			return WX_STATS_LEN -
> +			       (WX_NUM_RX_QUEUES - wx->num_tx_queues) *
> +			       (sizeof(struct wx_queue_stats) / sizeof(u64)) * 2;
> +		} else {
> +			return WX_STATS_LEN;
> +		}

The same code appears in wx_get_drvinfo.

1) It may be factored out.
2) The size of stats depends on num_{rx, tx}_queues. Unless there is some
   reason to keep WX_STATS_LEN, you may remove it, avoid the conditional code
   and always perform the required arithmetic.

By the way, I understand that driver allocates num_tx_queues and num_rx_queues
symmetrically as outlined in the comment at the start of wx_ethtool.c.
However, it's a bit unexpected to see this dependency elsewhere.

-- 
Ueimor