Re: [PATCH RFC net-next v8 04/13] net: Change the API of PHY default timestamp to MAC

From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Kory Maincent <kory.maincent@bootlin.com>,
	Florian Fainelli <florian.fainelli@broadcom.com>,
	Broadcom internal kernel review list
	<bcm-kernel-feedback-list@broadcom.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Radu Pirea <radu-nicolae.pirea@oss.nxp.com>,
	Jay Vosburgh <j.vosburgh@gmail.com>,
	Andy Gospodarek <andy@greyhouse.net>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	UNGLinuxDriver@microchip.com, Simon Horman <horms@kernel.org>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org,
	Maxime Chevallier <maxime.chevallier@bootlin.com>
Subject: Re: [PATCH RFC net-next v8 04/13] net: Change the API of PHY default timestamp to MAC
Date: Sat, 17 Feb 2024 13:05:40 -0800	[thread overview]
Message-ID: <87o7cebx9z.fsf@nvidia.com> (raw)
In-Reply-To: <b8926fe5-81ef-40ea-9e87-5e84b368b745@lunn.ch>

On Sat, 17 Feb, 2024 18:07:31 +0100 Andrew Lunn <andrew@lunn.ch> wrote:
> On Fri, Feb 16, 2024 at 10:09:36AM -0800, Rahul Rameshbabu wrote:
>> 
>> On Fri, 16 Feb, 2024 16:52:22 +0100 Kory Maincent <kory.maincent@bootlin.com> wrote:
>> > Change the API to select MAC default time stamping instead of the PHY.
>> > Indeed the PHY is closer to the wire therefore theoretically it has less
>> > delay than the MAC timestamping but the reality is different. Due to lower
>> > time stamping clock frequency, latency in the MDIO bus and no PHC hardware
>> > synchronization between different PHY, the PHY PTP is often less precise
>> > than the MAC. The exception is for PHY designed specially for PTP case but
>> > these devices are not very widespread. For not breaking the compatibility
>> > default_timestamp flag has been introduced in phy_device that is set by
>> > the phy driver to know we are using the old API behavior.
>> >
>> > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
>> > ---
>> 
>> Overall, I agree with the motivation and reasoning behind the patch. It
>> takes dedicated effort to build a good phy timestamping mechanism, so
>> this approach is good. I do have a question though. In this patch if we
>> set the phy as the default timestamp mechanism, does that mean for even
>> non-PTP applications, the phy will be used for timestamping when
>> hardware timestamping is enabled? If so, I think this might need some
>> thought because there are timing applications in general when a
>> timestamp closest to the MAC layer would be best.
>
> Could you give some examples? It seems odd to me, the application
> wants a less accurate timestamp?
>
> Is it more about overheads? A MAC timestamp might be less costly than
> a PHY timestamp?

It's a combination of both though I think primarily about line rate.
This point is somewhat carried over from the previous discussions on
this patch series in the last revision. I assume the device in question
here cannot timestamp at the PHY at a high rate.

  https://lore.kernel.org/netdev/20231120093723.4d88fb2a@kernel.org/

>
> Or is the application not actually doing PTP, it does not care about
> the time of the packet on the wire, but it is more about media access
> control? Maybe the applications you are talking about are misusing the
> PTP API for something its not intended?

So hardware timestamping is not a PTP specific API or application right?
It's purely a socket option that is not tied to PTP (unless I am missing
something here).

  https://docs.kernel.org/networking/timestamping.html#timestamp-generation

So you could use this information for other applications like congestion
control where you do not want to limit the line rate using the PHY
timestamping mechanism.

In mlx5, we only steering PTP traffic to our PHY timestamping mechanism
through a traffic matching logic.

  https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.h?id=a6e0cb150c514efba4aaba4069927de43d80bb59#n71

This is because we do not want to PHY/port timestamp timing related
applications such as congestion control. I think it makes sense for
specialized timestamping applications to instead use the ethtool ioctl
to reconfigure using the PHY timestamps if the device is capable of PHY
timestamping. (So have the change be in userspace application tools like
linuxptp where precise but low <relative> rate timestamp information is
ideal).

--
Thanks,

Rahul Rameshbabu