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/ 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). 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.
