netdev - Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose current time stamping layer

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: <20231013170903.p3ycicebnfrsmoks@skbuf>
Date: Fri, 13 Oct 2023 20:09:03 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Köry Maincent <kory.maincent@...tlin.com>,
	Florian Fainelli <florian.fainelli@...adcom.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Jonathan Corbet <corbet@....net>,
	Jay Vosburgh <j.vosburgh@...il.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	Nicolas Ferre <nicolas.ferre@...rochip.com>,
	Claudiu Beznea <claudiu.beznea@...on.dev>,
	Horatiu Vultur <horatiu.vultur@...rochip.com>,
	UNGLinuxDriver@...rochip.com,
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	Richard Cochran <richardcochran@...il.com>,
	Radu Pirea <radu-nicolae.pirea@....nxp.com>,
	Willem de Bruijn <willemdebruijn.kernel@...il.com>,
	Michael Walle <michael@...le.cc>,
	Jacob Keller <jacob.e.keller@...el.com>,
	Maxime Chevallier <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH net-next v5 08/16] net: ethtool: Add a command to expose
 current time stamping layer

On Fri, Oct 13, 2023 at 09:30:56AM -0700, Jakub Kicinski wrote:
> On Fri, 13 Oct 2023 19:14:46 +0300 Vladimir Oltean wrote:
> > > What is "PRECISION"? DMA is a separate block like MAC and PHY.  
> > 
> > If DMA is a separate block like MAC and PHY, can it have its own PHC
> > device, and the ethtool UAPI only lists the timestamping-capable PHCs
> > for one NIC, and is able to select between them? 
> 
> Possibly, I guess. There are some devices which use generic (i.e.
> modeled by Linux as separate struct device) DMA controllers to read 
> out packets from "MAC" FIFOs. In practice I'm not sure if any of those
> DMA controllers has time stamping capabilities.

The answer is not completely satisfactory, I guess. My proposal would
only work if the common denominator for a hardware timestamp provider
could be modeled as a struct ptp_clock, like we do for MAC and phylib
PHYs already, and we call ptp_clock_index() to get the phc_index that
serves as the UAPI token for it.

> > Translation between the UAPI-visible PHC index and MAC, DMA, phylib
> > PHY, other PHY etc can then be done by the kernel as needed.
> 
> Translation by the kernel at which point?

The gist of what I'm proposing is for the core ethtool netlink message
handler to get just the phc_index as an attribute. No other information
as to what it represents. Not that it's netdev, DMA, phylib PHY or whatnot.

The ethtool kernel code would iterate through the stuff registered in
the system for the netdev, calling get_ts_info() or phy_ts_info() on it,
until it finds something which populates struct ethtool_ts_info ::
phc_index with the phc_index retrieved from netlink.

Then, ethtool just talks with the timestamper that matched that phc_index.

Same idea would be applied for the command that lists all timestamping
layers for a netdev. Check get_ts_info(), phy_ts_info(dev->phydev), and
can be extended in the future.

> IMHO it'd indeed be clearer for the user to have an ability to read 
> the PHC for SOF_..._DMA via ETHTOOL_MSG_TS_LIST_GET_REPLY as a separate
> entry, rather than e.g. assume that DMA uses the same PHC as MAC.

I'm not really sure what you're referring to, with SOF_..._DMA.
The DMA, if presented as a PHC as I am proposing, would play the role of
the hardware timestamp provider (think SOF_TIMESTAMPING_TX_HARDWARE |
SOF_TIMESTAMPING_RX_HARDWARE), so there will be no driver-visible
special socket option flags for DMA timestamping.