Date: Wed, 29 Nov 2023 23:00:34 +0100
From: Köry Maincent <kory.maincent@...tlin.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>, Florian
 Fainelli <florian.fainelli@...adcom.com>, Broadcom internal kernel review
 list <bcm-kernel-feedback-list@...adcom.com>, Heiner Kallweit
 <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo
 Abeni <pabeni@...hat.com>, Richard Cochran <richardcochran@...il.com>, Radu
 Pirea <radu-nicolae.pirea@....nxp.com>, Jay Vosburgh
 <j.vosburgh@...il.com>, Andy Gospodarek <andy@...yhouse.net>, Nicolas Ferre
 <nicolas.ferre@...rochip.com>, Claudiu Beznea <claudiu.beznea@...on.dev>,
 Willem de Bruijn <willemdebruijn.kernel@...il.com>, Jonathan Corbet
 <corbet@....net>, Horatiu Vultur <horatiu.vultur@...rochip.com>,
 UNGLinuxDriver@...rochip.com, Simon Horman <horms@...nel.org>, Thomas
 Petazzoni <thomas.petazzoni@...tlin.com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, Maxime Chevallier
 <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH net-next v7 15/16] net: ethtool: ts: Let the active time
 stamping layer be selectable

On Wed, 29 Nov 2023 22:37:00 +0200
Vladimir Oltean <vladimir.oltean@....com> wrote:

> On Wed, Nov 29, 2023 at 09:09:59PM +0100, Köry Maincent wrote:
> > On Mon, 20 Nov 2023 13:45:51 -0800
> > Jakub Kicinski <kuba@...nel.org> wrote:

> > > Sounds reasonable. Having more attributes than just PHC index works.
> > > Given the lack of distinction between MAC and PHY for integrated NICs
> > > I'd lean towards ditching the "layers" completely and exposing 
> > > an "approximate" vs "precise" boolean. Approximate being the DMA point
> > > for NICs, but more generically a point that is separated from the wire
> > > by buffering or other variable length delay. Precise == IEEE 1588
> > > quality.  
> > 
> > Hello Jakub, just wondering.
> > I can add this hwtstamp provider qualifier in the next series version but it
> > won't be used as it is set and used at the driver level and no driver is
> > using it for now. It would not be accepted if I use something that is not
> > used, right? Do you still think I should add this in v8?  
> 
> Not sure why you say "not used", though. Are you not planning to expose
> the qualifier as an attribute to the listing of hwtstamp providers
> offered to user space by ETHTOOL_MSG_TSINFO_GET?

Yes I will, I was just saying that all the PHC would be set as precise for now.
Approximate timestamp quality won't be used because IIUC there are no NIC driver
supporting it yet.

> Personally, I worry that if the qualifier gets added later (not now) to
> the UAPI, we will end up having user space software (written now) that
> iterates through the provider listing thinking that there may only ever
> be one provider offered by one PHC, and will stop at the first such
> provider found, whichever that may be.
> 
> With the added qualifier, there's a higher chance that user space
> searches will be for a {phc, qualifier} pair (even if there will only be
> 1 possible qualifier type), and the future addition of a new hwtstamp
> provider will keep existing software working in the same way as before,
> i.e. user space won't select the DMA provider by mistake, by ignoring
> the qualifier attribute altogether.
> 
> Generally I'm against adding things upfront that can only be in a certain
> way, but in this case I believe that it is necessary in order for the
> future extensions that were discussed to be possible. The qualifier is
> part of the user space search key and thus pretty important.
> 
> My 2 cents, Jakub can absolutely disagree.

Alright, this seems relevant.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Powered by blists - more mailing lists