netdev - Re: Moving forward with stacked PHYs

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] [day] [month] [year] [list]

Message-ID: <320e6ab6-3299-4b6c-af66-191dc9c5052d@lunn.ch>
Date: Mon, 25 Nov 2024 17:34:23 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc: Oleksij Rempel <o.rempel@...gutronix.de>, netdev@...r.kernel.org,
	pabeni@...hat.com, kuba@...nel.org, edumazet@...gle.com,
	davem@...emloft.net, Russell King <linux@...linux.org.uk>,
	Romain Gantois <romain.gantois@...tlin.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	Vladimir Oltean <olteanv@...il.com>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Kory Maincent <kory.maincent@...tlin.com>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: Moving forward with stacked PHYs

> The way I see this is based on the phy_link_topology. What is done
> currently is that the SFP PHY is added to the topology when :
> 
>  - The .connect_phy() SFP upstream op is called, but ONLY if the
> upstream PHY is attached to its netdev (otherwise, the upstream PHY
> isn't in the topology)
> 
>  - Alternatively if the SFP phy is already attached to its upstream,
> both the upstream PHY and the SFP PHY will be added to the tpopology
> when the upstream PHY gets attached to its netdev.
> 
> The notification would be sent at that time. We can't really send it
> before the PHYs are part of the topology because at that point we don't
> know to which netdev it belongs.

I agree we cannot notify until we know where it goes in the chain. But
i was wondering if this is too early? Is it guaranteed that
phy_init_hw() is complete? We don't want userspace trying to
reconfigure the PHY of it then to be overwritten.

It would be good if the commit message adding the notification
explains what has already happened when the notification is sent, what
state the PHY is in.

> The way I see that, based on the appereance of PHYs, userspace may want
> to re-ajust configuration, especially if :
>  - The PHY is attached in .ndo_open() and
>  - The PHY provides some kind of offloading capability (Timestamping,
> maybe more such as macsec)
> 
> In that case, it's possible that userspace is interested in knowing
> that a new PHY is here to re-adjust the offloads to the PHY.

So do we need to be at the point that we know its EEE capabilities? It
has registers its MACSec capabilities and timestamper? Again, this
should be part of the commit message. What can a user of the
notification expect to have happened, and what has not yet happened,
or is ongoing and might race with.

	Andrew