netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Daniel Golle <daniel@makrotopia.org>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Maxime Chevallier <maxime.chevallier@bootlin.com>,
	Kory Maincent <kory.maincent@bootlin.com>,
	Edward Cree <ecree.xilinx@gmail.com>,
	Paolo Abeni <pabeni@redhat.com>, Jakub Kicinski <kuba@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	"David S. Miller" <davem@davemloft.net>,
	John Crispin <john@phrozen.org>
Subject: Re: ethtool settings and SFP modules with PHYs
Date: Mon, 16 Sep 2024 18:34:59 +0100	[thread overview]
Message-ID: <ZuhsQxHA+SJFPa5S@shell.armlinux.org.uk> (raw)
In-Reply-To: <ebfeeabd-7f4a-4a80-ba76-561711a9d776@lunn.ch>

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!

  reply	other threads:[~2024-09-16 17:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-16 15:36 ethtool settings and SFP modules with PHYs Daniel Golle
2024-09-16 16:02 ` Maxime Chevallier
2024-09-16 16:12   ` Andrew Lunn
2024-09-16 16:29     ` Maxime Chevallier
2024-09-16 16:03 ` Andrew Lunn
2024-09-16 17:34   ` Russell King (Oracle) [this message]
2024-09-17 15:53     ` Maxime Chevallier
2024-09-17 16:38       ` Russell King (Oracle)
2024-09-17 17:16         ` Daniel Golle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZuhsQxHA+SJFPa5S@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=daniel@makrotopia.org \
    --cc=davem@davemloft.net \
    --cc=ecree.xilinx@gmail.com \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=john@phrozen.org \
    --cc=kory.maincent@bootlin.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).