netdev - RE: [PATCH net-next v1 1/6] ethtool: add interface to read Tx hardware timestamping 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: <CO1PR11MB508917567D84B711C2BD5821D63D2@CO1PR11MB5089.namprd11.prod.outlook.com>
Date: Wed, 3 Apr 2024 18:44:52 +0000
From: "Keller, Jacob E" <jacob.e.keller@...el.com>
To: Rahul Rameshbabu <rrameshbabu@...dia.com>, Jakub Kicinski
	<kuba@...nel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>, "Zaki, Ahmed"
	<ahmed.zaki@...el.com>, "Lobakin, Aleksander" <aleksander.lobakin@...el.com>,
	"alexandre.torgue@...s.st.com" <alexandre.torgue@...s.st.com>,
	"andrew@...n.ch" <andrew@...n.ch>, "cjubran@...dia.com" <cjubran@...dia.com>,
	"corbet@....net" <corbet@....net>, "davem@...emloft.net"
	<davem@...emloft.net>, "dtatulea@...dia.com" <dtatulea@...dia.com>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "gal@...dia.com"
	<gal@...dia.com>, "hkallweit1@...il.com" <hkallweit1@...il.com>,
	"jiri@...nulli.us" <jiri@...nulli.us>, "joabreu@...opsys.com"
	<joabreu@...opsys.com>, "justinstitt@...gle.com" <justinstitt@...gle.com>,
	"kory.maincent@...tlin.com" <kory.maincent@...tlin.com>, "leon@...nel.org"
	<leon@...nel.org>, "liuhangbin@...il.com" <liuhangbin@...il.com>,
	"maxime.chevallier@...tlin.com" <maxime.chevallier@...tlin.com>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "Greenwalt, Paul"
	<paul.greenwalt@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, "rdunlap@...radead.org"
	<rdunlap@...radead.org>, "richardcochran@...il.com"
	<richardcochran@...il.com>, "saeed@...nel.org" <saeed@...nel.org>,
	"tariqt@...dia.com" <tariqt@...dia.com>, "vadim.fedorenko@...ux.dev"
	<vadim.fedorenko@...ux.dev>, "vladimir.oltean@....com"
	<vladimir.oltean@....com>, "Drewek, Wojciech" <wojciech.drewek@...el.com>
Subject: RE: [PATCH net-next v1 1/6] ethtool: add interface to read Tx
 hardware timestamping statistics



> -----Original Message-----
> From: Rahul Rameshbabu <rrameshbabu@...dia.com>
> Sent: Tuesday, April 2, 2024 10:15 PM
> To: Jakub Kicinski <kuba@...nel.org>
> Cc: netdev@...r.kernel.org; linux-kernel@...r.kernel.org; linux-
> doc@...r.kernel.org; Zaki, Ahmed <ahmed.zaki@...el.com>; Lobakin, Aleksander
> <aleksander.lobakin@...el.com>; alexandre.torgue@...s.st.com;
> andrew@...n.ch; cjubran@...dia.com; corbet@....net; davem@...emloft.net;
> dtatulea@...dia.com; edumazet@...gle.com; gal@...dia.com;
> hkallweit1@...il.com; Keller, Jacob E <jacob.e.keller@...el.com>;
> jiri@...nulli.us; joabreu@...opsys.com; justinstitt@...gle.com;
> kory.maincent@...tlin.com; leon@...nel.org; liuhangbin@...il.com;
> maxime.chevallier@...tlin.com; pabeni@...hat.com; Greenwalt, Paul
> <paul.greenwalt@...el.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@...el.com>;
> rdunlap@...radead.org; richardcochran@...il.com; saeed@...nel.org;
> tariqt@...dia.com; vadim.fedorenko@...ux.dev; vladimir.oltean@....com;
> Drewek, Wojciech <wojciech.drewek@...el.com>
> Subject: Re: [PATCH net-next v1 1/6] ethtool: add interface to read Tx hardware
> timestamping statistics
> 
> On Tue, 02 Apr, 2024 19:18:42 -0700 Jakub Kicinski <kuba@...nel.org> wrote:
> > On Tue,  2 Apr 2024 13:52:01 -0700 Rahul Rameshbabu wrote:
> >> +/**
> >> + * struct ethtool_ts_stats - HW timestamping statistics
> >> + * @tx_stats: struct group for TX HW timestamping
> >> + *	@pkts: Number of packets successfully timestamped by the hardware.
> >> + *	@lost: Number of hardware timestamping requests where the
> timestamping
> >> + *		information from the hardware never arrived for submission with
> >> + *		the skb.
> >> + *	@err: Number of arbitrary timestamp generation error events that the
> >> + *		hardware encountered, exclusive of @lost statistics. Cases such
> >> + *		as resource exhaustion, unavailability, firmware errors, and
> >> + *		detected illogical timestamp values not submitted with the skb
> >> + *		are inclusive to this counter.
> >> + */
> >> +struct ethtool_ts_stats {
> >> +	struct_group(tx_stats,
> >
> > Doesn't seem like the group should be documented:
> >
> > include/linux/ethtool.h:503: warning: Excess struct member 'tx_stats'
> description in 'ethtool_ts_stats'
> 
> Was looking into why our internal verification did not catch this. We
> run W=1 with clang, but looks like the warning does not get triggered
> unless explicitly run with scripts/kernel-doc.
> 
>   https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-
> format-kernel-doc-comments
> 
> I have debugged using strace that the way the kernel doc checking works
> when W=1 is set is that the matching source file that is being compiled
> is passed to scripts/kernel-doc, so include files are missed from the
> doc check. I think this is worth adding to the kernel documentation.
> 

It would be great if the W=1 setup could figure out the include files and send those to kernel-doc too, but I'm not sure if this is possible and if so how difficult it would be to implement it. A lot of headers produce warnings because a lot fewer people manually run kernel-doc on the entire source.

> Anyway, I will send out a v2 with this fixed but wait for potentially
> more feedback on v1.
> 
> --
> Thanks,
> 
> Rahul Rameshbabu