netdev - Re: [PATCH RFC 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: <abbff26c-c626-42ce-82a9-4dc983372de3@intel.com>
Date: Fri, 23 Feb 2024 13:07:08 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: Rahul Rameshbabu <rrameshbabu@...dia.com>, Saeed Mahameed
	<saeed@...nel.org>, Leon Romanovsky <leon@...nel.org>, "David S. Miller"
	<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Jonathan Corbet
	<corbet@....net>, Richard Cochran <richardcochran@...il.com>, Tariq Toukan
	<tariqt@...dia.com>, Gal Pressman <gal@...dia.com>, Vadim Fedorenko
	<vadim.fedorenko@...ux.dev>, Andrew Lunn <andrew@...n.ch>, Heiner Kallweit
	<hkallweit1@...il.com>, Przemek Kitszel <przemyslaw.kitszel@...el.com>, Ahmed
 Zaki <ahmed.zaki@...el.com>, Alexander Lobakin
	<aleksander.lobakin@...el.com>, Hangbin Liu <liuhangbin@...il.com>, "Paul
 Greenwalt" <paul.greenwalt@...el.com>, Justin Stitt <justinstitt@...gle.com>,
	Randy Dunlap <rdunlap@...radead.org>, Maxime Chevallier
	<maxime.chevallier@...tlin.com>, Kory Maincent <kory.maincent@...tlin.com>,
	Wojciech Drewek <wojciech.drewek@...el.com>, Vladimir Oltean
	<vladimir.oltean@....com>, Jiri Pirko <jiri@...nulli.us>, Alexandre Torgue
	<alexandre.torgue@...s.st.com>, Jose Abreu <joabreu@...opsys.com>, "Dragos
 Tatulea" <dtatulea@...dia.com>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-doc@...r.kernel.org>
Subject: Re: [PATCH RFC net-next v1 1/6] ethtool: add interface to read Tx
 hardware timestamping statistics

On 2/23/2024 11:24 AM, Rahul Rameshbabu wrote:
> +/**
> + * struct ethtool_ts_stats - HW timestamping statistics
> + * @layer: input field denoting whether stats should be queried from the DMA or
> + *        PHY timestamping layer. Defaults to the active layer for packet
> + *        timestamping.
> + * @tx_stats: struct group for TX HW timestamping
> + *	@pkts: Number of packets successfully timestamped by the queried
> + *	      layer.
> + *	@lost: Number of packet timestamps that failed to get applied on a
> + *	      packet by the queried layer.
> + *	@late: Number of packet timestamps that were delivered by the
> + *	      hardware but were lost due to arriving too late.
> + *	@err: Number of timestamping errors that occurred on the queried
> + *	     layer.
> + */
> +struct ethtool_ts_stats {
> +	enum ethtool_ts_stats_layer layer;
> +	struct_group(tx_stats,
> +		u64 pkts;
> +		u64 lost;
> +		u64 late;
> +		u64 err;
> +	);
> +};

The Intel ice drivers has the following Tx timestamp statistics:

tx_hwtstamp_skipped - indicates when we get a Tx timestamp request but
are unable to fulfill it.
tx_hwtstamp_timeouts - indicates we had a Tx timestamp skb waiting for a
timestamp from hardware but it didn't get received within some internal
time limit.
tx_hwtstamp_flushed - indicates that we flushed an outstanding timestamp
before it completed, such as if the link resets or similar.
tx_hwtstamp_discarded - indicates that we obtained a timestamp from
hardware but were unable to complete it due to invalid cached data used
for timestamp extension.

I think these could be translated roughly to one of the lost, late, or
err stats. I am a bit confused as to how drivers could distinguish
between lost and late, but I guess that depends on the specific hardware
design.

In theory we could keep some of these more detailed stats but I don't
think we strictly need to be as detailed as the ice driver is.

The only major addition I think is the skipped stat, which I would
prefer to have. Perhaps that could be tracked in the netdev layer by
checking whether the skb flags to see whether or not the driver actually
set the appropriate flag?

I think i can otherwise translate the flushed status to the lost
category, the timeout to the late category, and everything else to the
error category. I can easily add a counter to track completed timestamps
as well.

TL;DR; I would like to see a "skipped" category since I think that
should be distinguished from general errors.

Thanks!