netdev - Re: [PATCH net-next RFC v4 4/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]

Date: Thu, 11 May 2023 18:56:40 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Köry Maincent <kory.maincent@...tlin.com>,
	netdev@...r.kernel.org, glipus@...il.com,
	maxime.chevallier@...tlin.com, vadim.fedorenko@...ux.dev,
	richardcochran@...il.com, gerhard@...leder-embedded.com,
	thomas.petazzoni@...tlin.com, krzysztof.kozlowski+dt@...aro.org,
	robh+dt@...nel.org, linux@...linux.org.uk
Subject: Re: [PATCH net-next RFC v4 4/5] net: Let the active time stamping
 layer be selectable.

On Thu, May 11, 2023 at 08:36:20AM -0700, Jakub Kicinski wrote:
> On Thu, 11 May 2023 16:48:07 +0300 Vladimir Oltean wrote:
> > > Ok, right you move it on to dsa stub. What do you think of our case, should we
> > > continue with netdev notifier?   
> > 
> > I don't know.
> > 
> > AFAIU, the plan forward with this patch set is that, if the active
> > timestamping layer is the PHY, phy_mii_ioctl() gets called and the MAC
> > driver does not get notified in any way of that. That is an issue
> > because if it's a switch, it will want to trap PTP even if it doesn't
> > timestamp it, and with this proposal it doesn't get a chance to do that.
> > 
> > What is your need for this? Do you have this scenario? If not, just drop
> > this part from the patch.
> > 
> > Jakub, you said "nope" to netdev notifiers, what would you suggest here
> > instead? ndo_change_ptp_traps()?
> 
> More importantly "monolithic" drivers have DMA/MAC/PHY all under 
> the NDO so assuming that SOF_PHY_TIMESTAMPING implies a phylib PHY
> is not going to work.
> 
> We need a more complex calling convention for the NDO.

It's the first time I become aware of the issue of PHY timestamping in
monolithic drivers that don't use phylib, and it's actually a very good
point. I guess that input gives a clearer set of constraints for Köry to
design an API where the selected timestamping layer is maybe passed to
ndo_hwtstamp_set() and MAC drivers are obliged to look at it.

OTOH, a complaint about the current ndo_eth_ioctl() -> phy_mii_ioctl()
code path was that phylib PHY drivers need to have the explicit blessing
from the MAC driver in order to enable timestamping. This patch set
attempts to circumvent that, and you're basically saying that it shouldn't.