netdev - Re: [PATCH net-next 3/5] ethtool: add interface to read representor Rx 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: <20240405215335.7a5601ca@kernel.org>
Date: Fri, 5 Apr 2024 21:53:35 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Tariq Toukan <tariqt@...dia.com>, Carolina Jubran <cjubran@...dia.com>,
 Simon Horman <horms@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>, Paolo Abeni
 <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
 <netdev@...r.kernel.org>, Saeed Mahameed <saeedm@...dia.com>, Gal Pressman
 <gal@...dia.com>, Leon Romanovsky <leonro@...dia.com>, Rahul Rameshbabu
 <rrameshbabu@...dia.com>
Subject: Re: [PATCH net-next 3/5] ethtool: add interface to read representor
 Rx statistics

On Thu, 4 Apr 2024 20:33:55 +0300 Tariq Toukan wrote:
> Implement common representor port statistics in
> a rep_port_stats struct_group, introducing a new
> 'out of buffer' stats for when packets are dropped
> due to a lack of receive buffers in RX queue.
> 
> The port statistics represent the statistics of the
> function with which the representor is associated.
> 
> Print the representor port stats when the
> --groups rep-port or --all-groups are used.

I re-read what Tariq said on v1 and I clearly missed the point,
sorry about that. Looking that his patch makes it pretty obvious.

With the netdev netlink family in place I was hoping we would
only put in ethtool stats for functionality already configured
via ethtool or clearly related to Ethernet.

But before we go to netdev - can you think of more such error stats 
that we may need to add? Since it's the equivalent of rtnl rx_missed 
I think we should consider putting it in netdev_offload_xstats. Maybe
following the same definition as packet/byte counters? Report sum by
default and CPU ones under IFLA_OFFLOAD_XSTATS_CPU_HIT? Or new enum
entry?

Simon, WDYT?

> +/**
> + * struct ethtool_rep_port_stats - representor port statistics
> + * @rep_port_stats: struct group for representor port

In more trivial remarks - kernel-doc script apparently doesn't want
the group to be documented (any more?)

> + *	@out_of_buf: Number of packets were dropped due to buffer exhaustion.
> + */
> +struct ethtool_rep_port_stats {
> +	struct_group(rep_port_stats,
> +		u64 out_of_buf;
> +	);
> +};
> +