> -----Original Message----- > From: Rahul Rameshbabu <rrameshbabu@xxxxxxxxxx> > Sent: Tuesday, April 2, 2024 10:15 PM > To: Jakub Kicinski <kuba@xxxxxxxxxx> > Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux- > doc@xxxxxxxxxxxxxxx; Zaki, Ahmed <ahmed.zaki@xxxxxxxxx>; Lobakin, Aleksander > <aleksander.lobakin@xxxxxxxxx>; alexandre.torgue@xxxxxxxxxxx; > andrew@xxxxxxx; cjubran@xxxxxxxxxx; corbet@xxxxxxx; davem@xxxxxxxxxxxxx; > dtatulea@xxxxxxxxxx; edumazet@xxxxxxxxxx; gal@xxxxxxxxxx; > hkallweit1@xxxxxxxxx; Keller, Jacob E <jacob.e.keller@xxxxxxxxx>; > jiri@xxxxxxxxxxx; joabreu@xxxxxxxxxxxx; justinstitt@xxxxxxxxxx; > kory.maincent@xxxxxxxxxxx; leon@xxxxxxxxxx; liuhangbin@xxxxxxxxx; > maxime.chevallier@xxxxxxxxxxx; pabeni@xxxxxxxxxx; Greenwalt, Paul > <paul.greenwalt@xxxxxxxxx>; Kitszel, Przemyslaw <przemyslaw.kitszel@xxxxxxxxx>; > rdunlap@xxxxxxxxxxxxx; richardcochran@xxxxxxxxx; saeed@xxxxxxxxxx; > tariqt@xxxxxxxxxx; vadim.fedorenko@xxxxxxxxx; vladimir.oltean@xxxxxxx; > Drewek, Wojciech <wojciech.drewek@xxxxxxxxx> > 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@xxxxxxxxxx> 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