linux-kernel - Re: [PATCH v3 3/5] net: Let the active time stamping layer be selectable.

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: <20230313081700.ie7cwjvrlky4e27b@soft-dev3-1>
Date:   Mon, 13 Mar 2023 09:17:00 +0100
From:   Horatiu Vultur <horatiu.vultur@...rochip.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
CC:     Michael Walle <michael@...le.cc>,
        Köry Maincent <kory.maincent@...tlin.com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <linux-omap@...r.kernel.org>,
        Maxime Chevallier <maxime.chevallier@...tlin.com>,
        Richard Cochran <richardcochran@...il.com>,
        <thomas.petazzoni@...tlin.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        "Jakub Kicinski" <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Jonathan Corbet <corbet@....net>,
        "Jay Vosburgh" <j.vosburgh@...il.com>,
        Veaceslav Falico <vfalico@...il.com>,
        "Andy Gospodarek" <andy@...yhouse.net>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        <UNGLinuxDriver@...rochip.com>,
        Minghao Chi <chi.minghao@....com.cn>,
        "Jie Wang" <wangjie125@...wei.com>,
        Oleksij Rempel <linux@...pel-privat.de>,
        "Sean Anderson" <sean.anderson@...o.com>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Alexander Lobakin <alexandr.lobakin@...el.com>,
        Marco Bonelli <marco@...eim.net>
Subject: Re: [PATCH v3 3/5] net: Let the active time stamping layer be
 selectable.

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@nxp.com/

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@nxp.com/
> 
> 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