Re: [PATCH net-next v7 15/16] net: ethtool: ts: Let the active time stamping layer be selectable

[Vladimir Oltean]

On Mon, Nov 20, 2023 at 10:51:48AM -0800, Jakub Kicinski wrote:
> On Mon, 20 Nov 2023 19:39:35 +0100 Andrew Lunn wrote:
> > Do you know of real hardware with a MAC and a PHY sharing a PHC?
> 
> mlx5 for sure, but other designs, too. PHY, NIC pipeline and PCIe PTM
> may all need to time stamp from a single time counter.

I'm still waiting for you to fully clarify the "per socket vs global"
aspect, but independently of that, at least I understand why this is a
counter-argument to my proposal. I need to tune it a bit (ASSUMING that
we want DMA timestamps to "look like" hwtimestamps, and not like their
own thing, to user space), because the PHC index would no longer fully
identify a hwtstamp provider, so we need something more.

I imagine both ETHTOOL_MSG_TSINFO_GET and ETHTOOL_MSG_TSINFO_SET to
support a new (nest) nlattr called ETHTOOL_A_TSINFO_HWSTAMP_PROVIDER.

This would contain (u32) ETHTOOL_A_TSINFO_HWSTAMP_PROVIDER_PHC_INDEX
and (u32) ETHTOOL_A_TSINFO_HWSTAMP_PROVIDER_QUALIFIER. It could be
extensible in the future, but this is the baseline and forms the key.

The latter takes values from an:

enum ethtool_hwstamp_provider_qualifier {
	ETHTOOL_HWSTAMP_PROVIDER_QUALIFIER_MAC,
	ETHTOOL_HWSTAMP_PROVIDER_QUALIFIER_PHY,
	ETHTOOL_HWSTAMP_PROVIDER_QUALIFIER_DMA,
};

with Jakub's comments about the various types providing various
qualities of timestamps, given here:
https://lore.kernel.org/netdev/20230511150902.57d9a437@xxxxxxxxxx/

 - PHY - per spec, at the RS layer
 - MAC - "close to the wire" in the MAC, specifically the pipeline
   delay (PHY stamp vs MAC stamp) should be constant for all packets;
   there must be no variable-time buffering and (for Tx) the time
   stamping must be past the stage of the pipeline affected by pause
   frames
 - DMA - worst quality, variable delay timestamp, usually taken when
   packets DMA descriptors (Rx or completion) are being written


It _sounds_ like we've all been talking about the same thing for ages,
but we weren't.


So, a PHC could offer multiple hwtstamp providers, as many as there are
qualifiers to uniquely describe them. Each hwstamp provider is
represented by a single ETHTOOL_A_TSINFO_HWSTAMP_PROVIDER nested nlattr.
In TSINFO_GET requests, there are as many ETHTOOL_A_TSINFO_HWSTAMP_PROVIDER
nests as there are hwtstamp providers for the NIC.

In the "normal" case of one single hwtstamp provider per PHC, it would
be the responsibility of the driver to set its qualifier to the right
thing: phylib to ETHTOOL_HWSTAMP_PROVIDER_QUALIFIER_PHY, and "normal" MAC
drivers to ETHTOOL_HWSTAMP_PROVIDER_QUALIFIER_MAC. Here, the qualifier
isn't more than an extra (partially redundant) mechanism for user space
to know what it's juggling with.

As opposed to Köry's proposal (where "dev->ts_layer == PHY_TIMESTAMPING"
means actual phylib), ETHTOOL_HWSTAMP_PROVIDER_QUALIFIER_PHY describes
the PHY-like timestamp quality of any hwtstamp provider, be it provided
by a phylib PHY or a firmware-based PHY. It doesn't describe "the layer"
itself.

Does this tick all boxes?