Message-ID: <ZuhsQxHA+SJFPa5S@shell.armlinux.org.uk>
Date: Mon, 16 Sep 2024 18:34:59 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>
Cc: Daniel Golle <daniel@...rotopia.org>, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, Heiner Kallweit <hkallweit1@...il.com>,
	Maxime Chevallier <maxime.chevallier@...tlin.com>,
	Kory Maincent <kory.maincent@...tlin.com>,
	Edward Cree <ecree.xilinx@...il.com>,
	Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>,
	"David S. Miller" <davem@...emloft.net>,
	John Crispin <john@...ozen.org>
Subject: Re: ethtool settings and SFP modules with PHYs

On Mon, Sep 16, 2024 at 06:03:35PM +0200, Andrew Lunn wrote:
> On Mon, Sep 16, 2024 at 04:36:47PM +0100, Daniel Golle wrote:
> > Hi,
> > 
> > I'm wondering how (or rahter: when?) one is supposed to apply ethtool
> > settings, such as modifying advertisement of speed, duplex, ..., with
> > SFP modules containing a PHY.
> 
> It should actually be more generic than that. You might also want to
> change the settings for Fibre modules. You have a 2.5G capable module
> and MAC, but the link partner can only do 1G. You need to force it
> down to 1G in order to get link.

Exactly. If the SFP gets changed, what it's connected to could be
something radically different.

There's also a _big_ problem here - there are SFPs that auto-detect
what the host is doing when they are inserted, and they adapt to
that. At least some *PON SFPs do this. Changing the settings (by any
method) of the link after the module has successfully established
synchronisation on the host side likely would result in the link
going down.

> > Do you think it would make sense to keep the user selection of
> > advertised modes for each networking device accross removal or insertion
> > of an SFP module?
> 
> No, you have no idea if the same module has been inserted, at least
> with the current code. You could maybe stash the EEPROM contents and
> see if it is the same, but that does not seem reliable to me, what do
> you do when it is different?

Quite. I think if we had a way to notify userspace that something with
the netdev hardware has changed, userspace could e.g. read the SFP
EEPROM itself and decide what settings it wishes to use - thereby
putting the policy decisions about what to do when a SFP is inserted
squarely in userspace's court.

The problem, as we all know, is that SFP EEPROM contents are a law to
themselves, and I wouldn't even think of trying to detect "has a
different module been plugged in from the previous module" by looking
for different EEPROM contents. Yes, the EEPROM has a serial number.
You can bet that there are vendors who program their modules with a
standard content that's the same for each module.

> > Alternatively we could of course also introduce a dedicated NETLINK_ROUTE
> > event which fires exactly one time once a new is PHY attached.
> 
> Something like that. I would probably also do it on remove.
> 
> It does not seem too unreasonable to call netdev_state_change() on
> module insert/remove. But maybe also add an additional property
> indicating if the SFP cage is empty/occupied. The plumbing for that is
> a bit more interesting.

Remember that the SFP code itself doesn't have visibility of the
netdev - that's handled at the higher levels by the PHY driver (if
the network connectivity is via a PHY) or via phylink if it's direct
to the MAC/PCS.

However, things get very complicated. We can't simply just change
configuration when the SFP is inserted.

In order to keep the laser/transmitter in fibre SFPs turned off, we
ensure that TX_DISABLE is asserted when the socket is inactive. I
view this as a safety measure as it avoids the potential for eye
sight damage by reflections.

However, for many copper SFPs, TX_DISABLE seems to be used to hold
the PHY in reset, making it unresponsive via I2C. So, at "module
insert" we don't even know if we have a PHY or not - we can only
take a best guess at whether the module _may_ have a PHY. Remember
that there are modules which do have a PHY, but the PHY is
completely inaccessible.

So, triggering userspace to do something when a module is inserted
is too early - we don't know at that point whether it has a PHY or
what the PHY is, what the capabilities of that PHY are, or anything
like that.

The best place to decide to notify userspace would be at the
module_start() callback - this happens when a module is present,
and the netdev has been brought up. Note that this call will happen
each and every time the netdev is brought up.

module_stop() is module_start()'s opposite method.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Powered by blists - more mailing lists