The 03/10/2023 18:44, Vladimir Oltean wrote: > > On Fri, Mar 10, 2023 at 02:15:29PM +0100, Horatiu Vultur wrote: > > I was thinking about another scenario (I am sorry if this was already > > discussed). > > Currently when setting up to do the timestamp, the MAC will check if the > > PHY has timestamping support if that is the case the PHY will do the > > timestamping. So in case the switch was supposed to be a TC then we had > > to make sure that the HW was setting up some rules not to forward PTP > > frames by HW but to copy these frames to CPU. > > With this new implementation, this would not be possible anymore as the > > MAC will not be notified when doing the timestamping in the PHY. > > Does it mean that now the switch should allocate these rules at start > > time? > > I would say no (to the allocation of trapping rules at startup time). > It was argued before by people present in this thread that it should be > possible (and default behavior) for switches to forward PTP frames as if > they were PTP-unaware: > https://patchwork.ozlabs.org/project/netdev/patch/20190813025214.18601-5-yangbo.lu@xxxxxxx/ Thanks for the explanation! > > But it raises a really good point about how much care a switch driver > needs to take, such that with PTP timestamping, it must trap but not > timestamp the PTP frames. > > There is a huge amount of variability here today. > > The ocelot driver would be broken with PHY timestamping, since it would > flood the PTP messages (it installs the traps only if it is responsible > for taking the timestamps too). > > The lan966x driver is very fine-tuned to call lan966x_ptp_setup_traps() > regardless of what phy_has_hwtstamp() says. > > The sparx5 driver doesn't even seem to install traps at all (unclear if > they are predefined in hardware or not). They are not predefined in HW, I have on my TODO list to add those traps I just need to get the time to do this. > > I guess that we want something like lan966x to keep working, since it > sounds like it's making the sanest decision about what to do. > > But, as you point out, with Köry's/Richard's proposal, the MAC driver > will be bypassed when the selected timestamping layer is the PHY, and > that's a problem currently. > > May I suggest the following? There was another RFC which proposed the > introduction of a netdev notifier when timestamping is turned on/off: > https://lore.kernel.org/netdev/20220317225035.3475538-1-vladimir.oltean@xxxxxxx/ > > It didn't go beyond RFC status, because I started doing what Jakub > suggested (converting the raw ioctls handlers to NDOs) but quickly got > absolutely swamped into the whole mess. > > If we have a notifier, then we can make switch drivers do things > differently. They can activate timestamping per se in the timestamping > NDO (which is only called when the MAC is the active timestamping layer), > and they can activate PTP traps in the netdev notifier (which is called > any time a timestamping status change takes place - the notifier info > should contain details about which net_device and timestamping layer > this is, for example). > > It's just a proposal of how to create an alternative notification path > that doesn't disturb the goals of this patch set. -- /Horatiu
