ethtool settings and SFP modules with PHYs

ethtool settings and SFP modules with PHYs

[Daniel Golle]

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.

My first approach was to try catching the event of the PHY being
attached and then re-applying ethtool settings[1]. As there isn't a
dedicated event for that, I found that IFF_UP && !IFF_LOWER_UP is as
close as it gets.

However, that doesn't go well with some PHY drivers and the result seems
to depend on a race condition.

Simply ignoring the supported link modes and assuming the kernel would
filter them also doesn't work as also the advertised modes get reset
every time the SFP module is removed or inserted.

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?

The user selection would by default select all known link modes, using
ethtool (ioctl or nl) would modify it, while the actually advertised
modes would always be the intersection of user-selected modes and
supported modes.

Alternatively we could of course also introduce a dedicated NETLINK_ROUTE
event which fires exactly one time once a new is PHY attached.

If there is any way to automically apply user-configured ethtool
settings without any of the above, please be so kind and let me know how
that would work also for PHYs on SFP modules.

Thank you!

With Best Regards

Daniel

[1]: https://git.openwrt.org/?p=project/netifd.git;a=commitdiff;h=68c8a4f94cd3cfd654a52cbc8b57c5c9d99640dd

Re: ethtool settings and SFP modules with PHYs

[Maxime Chevallier]

Hello Daniel,

On Mon, 16 Sep 2024 16:36:47 +0100
Daniel Golle <daniel@makrotopia.org> 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.
> 
> My first approach was to try catching the event of the PHY being
> attached and then re-applying ethtool settings[1]. As there isn't a
> dedicated event for that, I found that IFF_UP && !IFF_LOWER_UP is as
> close as it gets.
> 
> However, that doesn't go well with some PHY drivers and the result seems
> to depend on a race condition.
> 
> Simply ignoring the supported link modes and assuming the kernel would
> filter them also doesn't work as also the advertised modes get reset
> every time the SFP module is removed or inserted.
> 
> 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?
> 
> The user selection would by default select all known link modes, using
> ethtool (ioctl or nl) would modify it, while the actually advertised
> modes would always be the intersection of user-selected modes and
> supported modes.

The problem I see is that the modes can change completely depending on
the SFP module that's inserted. If say, you plug a Copper module,
filter advertising to 100BaseT, unplug it, then plug a Fiber module,
you end-up with nothing in the intersection.

Same goes for speed limitation. You can plug a copper SFP, limit the
speed to 100M, switch it with a Fiber SFP that does 1000BaseX, and here
100Mbps isn't possible, and you have an invalid setting.

> 
> Alternatively we could of course also introduce a dedicated NETLINK_ROUTE
> event which fires exactly one time once a new is PHY attached.

That could be done. We have netlink messages in ethtool that report the
PHYs existing on the link (ETHTOOL_A_PHY_GET), that's new and still in
net-next. There's no notification yet, but this is something doable,
adding a netlink notification to indicate that a new PHY was attached.

While this doesn't exist yet, you can take a look at the recent
phy_link_topology work, that allows you to list the PHYs attached to a
netdev, including the ones in SFP modules [1].

A workaround for you can be to wait until the SFP PHY show up in
ethtool --show-phys ethX (it's parent SFP bus name will be populated,
you can filter on that), then re-apply your settings. You'll need a
recent ethtool [2].

A notification would indeed be better, and is something I can prototype
quickly. I was hesitating to add that, but as you show interest in
this, I'm OK to move forward on that :)

[1]: https://lore.kernel.org/netdev/20240821151009.1681151-1-maxime.chevallier@bootlin.com/
[2]: https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?h=next&id=b3ee7c0fa87032dec614dcc716c08a3b77d80fb0

Thanks,

Maxime

Re: ethtool settings and SFP modules with PHYs

[Andrew Lunn]

> A notification would indeed be better, and is something I can prototype
> quickly. I was hesitating to add that, but as you show interest in
> this, I'm OK to move forward on that :)

This might need further brainstorming. What are we actually interested
in?

The EEPROM has been read, we know what sort of SFP it is?

It happens to be a copper SFP, we know what MDIO over I2C protocol to
use, it responds, and the PHY device has been created? Does the SFP
layer actually know this? Are we actually adding a notification for
any PHY, not just an SFP PHY?

	Andrew

Re: ethtool settings and SFP modules with PHYs

[Maxime Chevallier]

On Mon, 16 Sep 2024 18:12:32 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > A notification would indeed be better, and is something I can prototype
> > quickly. I was hesitating to add that, but as you show interest in
> > this, I'm OK to move forward on that :)  
> 
> This might need further brainstorming. What are we actually interested
> in?
> 
> The EEPROM has been read, we know what sort of SFP it is?
> 
> It happens to be a copper SFP, we know what MDIO over I2C protocol to
> use, it responds, and the PHY device has been created? Does the SFP
> layer actually know this? Are we actually adding a notification for
> any PHY, not just an SFP PHY?

What I was refering to is to notify on a phy_link_topo_add_phy() call,
which deals with both "regular" PHYs and SFP PHYs.

I'm still not fully expert on the netlink aspect, but could
ETHTOOL_A_PHY_NTF be emitted upon PHY addition to the topology,
containing exactly the same info as a regular ETHTOOL_A_PHY_GET message
?

At that time, we know what PHY it is, and that it's attached to the
netdev, the eeprom has been read and phydev created.

That covers the PHY aspect, but not the whole SFP aspect. We won't get
that notif for Fibre modules, as there's no PHY.

Triggering notifications upon SFP insertion will require extra
plumbing. As of today, the sfp-bus itself doesn't know which netdev
it's attached to (there's not sfp_bus.netdev pointer), so a notif won't
help much as we don't know which interface the event concerns.

That attachment can happen at link-up time or probe-time, so for example
there might be no easy way to trigger that notification when a module
is inserted while the link is down, that behaviour would depend on the
driver(s) (wether or not it uses phylink, whether or not the SFP is
driver by a PHY or directly by the MAC).

I have prototype code here[1] (beware, one giant hard-to-read patch
without commit log for now, will be split eventually) that deals with
parts of that issue, the end-goal being to report the state of all
front-facing ports (including SFP and their module) to users.

Maxime

[1] : https://github.com/minimaxwell/linux/tree/mc/main

Re: ethtool settings and SFP modules with PHYs

[Andrew Lunn]

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.

> 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?

> 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.

	Andrew

Re: ethtool settings and SFP modules with PHYs

[Russell King (Oracle)]

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!

Re: ethtool settings and SFP modules with PHYs

[Maxime Chevallier]

Hi,

On Mon, 16 Sep 2024 18:34:59 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

[...]

> 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.

As we are brainstorming, may I suggest an alternative approach ?

I'm currently working on getting some way of providing userspace with a
representation of the front-facing ports of a netdev. This would
include SFP module ports, RJ45/8P8C/BaseT ports, BaseT1 ports, you name
it.

This would allow to represent with a bit more clarity use-cases such as
the MCBin's ports controlled by the 88x3310, the combo RJ45 / SFP ports.

The other case would be devices which have several PHYs in parallel.

The way I see it for now, is that we would have internal kernel objects
with a name such as "phy_port" or "phy_mdi" or just "mdi" (I'm bad at
naming), instanciated by PHY drivers (or phylib through generic helpers
in most simple case for single-port PHYs), as well as the SFP
subsystem, but also by NICs that have a front-facing port that's driven
neither through SFPs nor PHYs (through a firmware of some sort).

These phy_port would have a set of callback ops to get their
ethtool_ksettings linkmodes and could be added/removed dynamically, in
the case of SFP module insertion for example.

The notification I would imagine would be "there's a change on the
front-facing port" or "there's a new one, here's what it can do" in the
case of module insertion.

I already have some code for that, and I will talk about this exact
topic tomorrow morning at the networking track of LPC [1]

For the SFP case, the notification would trigger indeed at the
module_start/module_remove step.

All of that is still WIP, but I think it would reply to that exact need
of "notifying users when something happens to the ports", including SFP
module insertion.

I plan to submit an RFC shortly after LPC, I still need to iron some
things out, and I think that RFC will itself trigger a lot of
discussions, but do you see that this kind of work could help resolving
the issue faced by Daniel as well as the need for SFP-event
notifications ?

Thanks,

Maxime

[1] : https://lpc.events/event/18/contributions/1964/

Re: ethtool settings and SFP modules with PHYs

[Russell King (Oracle)]

On Tue, Sep 17, 2024 at 05:53:47PM +0200, Maxime Chevallier wrote:
> For the SFP case, the notification would trigger indeed at the
> module_start/module_remove step.

This (the confusion of module_remove being the opposite of
module_start)...

> 
> All of that is still WIP, but I think it would reply to that exact need
> of "notifying users when something happens to the ports", including SFP
> module insertion.

and talking here about module insertion here, leads me to believe that
you haven't grasped the problem with SFPs, where we don't know what
the module supports at _insertion_ time.

If we're after giving userspace a notification so it can make decisions
about what to do after examining capabilities, then insertion time is
too early.

If we're after giving userspace a notification e.g. that a SFP was
inserted, so please bring up the network interface, then that may be
useful, but userspace needs to understand that SFPs are special and
they can't go configuring the link at this point if it's a SFP.

Honestly, I do not want to expose to userspace this kind of complexity
that's specific to SFPs. It _will_ get it wrong. I also think that it
will tie our hands when working around module problems if we have to
change the way module capabilities are handled - and I don't wish to
be tied by "but that change you made to make module XYZ work breaks
my userspace!" because someone's using these events to do some weirdo
configuration.

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

Re: ethtool settings and SFP modules with PHYs

[Daniel Golle]

On Tue, Sep 17, 2024 at 05:38:39PM +0100, Russell King (Oracle) wrote:
> On Tue, Sep 17, 2024 at 05:53:47PM +0200, Maxime Chevallier wrote:
> > For the SFP case, the notification would trigger indeed at the
> > module_start/module_remove step.
> 
> This (the confusion of module_remove being the opposite of
> module_start)...
> 
> > 
> > All of that is still WIP, but I think it would reply to that exact need
> > of "notifying users when something happens to the ports", including SFP
> > module insertion.
> 
> and talking here about module insertion here, leads me to believe that
> you haven't grasped the problem with SFPs, where we don't know what
> the module supports at _insertion_ time.
> 
> If we're after giving userspace a notification so it can make decisions
> about what to do after examining capabilities, then insertion time is
> too early.
> 

Exactly. It needs to be after the PHY has been probed (which can take
up to 25 seconds after insertion).

> If we're after giving userspace a notification e.g. that a SFP was
> inserted, so please bring up the network interface, then that may be
> useful, but userspace needs to understand that SFPs are special and
> they can't go configuring the link at this point if it's a SFP.
> 
> Honestly, I do not want to expose to userspace this kind of complexity
> that's specific to SFPs. It _will_ get it wrong. I also think that it
> will tie our hands when working around module problems if we have to
> change the way module capabilities are handled - and I don't wish to
> be tied by "but that change you made to make module XYZ work breaks
> my userspace!" because someone's using these events to do some weirdo
> configuration.

The problem I'm trying to address (see initial post) is that the user may
want to configure some properties of the link, such as whether or not to
announce flow-control capabilities to the link partner, or (more rarely)
to limit to think speed to, let's say 100M/Full, even though 1000M/Full
would be possible, e.g. to work around problems with bad cabling.

Doing so, even right now, doesn't require much specific knowledge about
the MAC or SFP/PHY capabilities, all we need to do is to clear some
bits from the advertised field. So in my case userspace doesn't make
any attempt to identify the MAC ro SFP/PHY capabilities or act according
to them.

Currently, in order to know *when* to apply those settings (again) I'm
relying on an unreliable hack upon receiving RTNL events:

if ((flags & IFF_UP) && !(flags & IFF_LOWER_UP))
	system_set_ethtool_settings(dev, &dev->settings);

And that doesn't work well, which is why I started to reach out if there
is a better solution for that.