netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* ixgbe: How to do this without a module parameter?
@ 2021-10-25 20:23 Robert Schlabbach
  2021-10-25 20:59 ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 8+ messages in thread
From: Robert Schlabbach @ 2021-10-25 20:23 UTC (permalink / raw)
  To: netdev

A while ago, Intel devs sneaked a hack into the ixgbe driver which disables
NBASE-T support by default:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c?id=a296d665eae1e8ec6445683bfb999c884058426a

Only after a user complaint, Intel bothered to reveal their reason for this:

https://www.mail-archive.com/e1000-devel@lists.sourceforge.net/msg12615.html

But this comes at the expense of NBASE-T users, who are left wondering why their
NIC (which Intel sells as supporting NBASE-T) only comes up with GbE links. To
fix this, I submitted this patch:

https://lists.osuosl.org/pipermail/intel-wired-lan/Week-of-Mon-20211018/026326.html

However, Intel devs pointed out to me that private module parameters would no
longer be accepted. Indeed, after some search I found this in the archive:

https://lore.kernel.org/netdev/20170324.144017.1545614773504954414.davem@davemloft.net/

The reason given there is that a module parameter is the "worst user experience
possible". But I think the absolutely worst user experience possible is having
to figure out a complex script that:

- compiles a list of all net devices provided by the ixgbe module
- retrieves the supported link speeds and converts them to a hex mask
- ORs the NBASE-T speeds into this hex mask
- finally runs ethtool to set the hex mask of the speeds to advertise

Even as a developer with 10 years experience with Linux, I would have to spend
quite a while writing such a script, and then figuring out how to have it
executed at the right time during startup. I suppose the vast majority of
Linux admins would be overwhelmed with that.

In contrast, explaining how to set the module parameter to control NBASE-T
support is a two-liner, see my patch above where I added that to the ixgbe.rst
module documentation. I think that's feasible for most Linux admins.

So my question is: Can anyone come up with a solution allowing to control
NBASE-T support in the ixgbe module in a way that's feasible for most Linux
admins, that works without a module parameter?

If not, could an exception be made for this patch to allow an extra parameter
for the ixgbe module?

Or does anyone have an even better idea?

Best regards,
-Robert Schlabbach


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ixgbe: How to do this without a module parameter?
  2021-10-25 20:23 ixgbe: How to do this without a module parameter? Robert Schlabbach
@ 2021-10-25 20:59 ` Toke Høiland-Jørgensen
  2021-10-25 21:11   ` Andrew Lunn
  0 siblings, 1 reply; 8+ messages in thread
From: Toke Høiland-Jørgensen @ 2021-10-25 20:59 UTC (permalink / raw)
  To: Robert Schlabbach, netdev

Robert Schlabbach <Robert.Schlabbach@gmx.net> writes:

> A while ago, Intel devs sneaked a hack into the ixgbe driver which disables
> NBASE-T support by default:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c?id=a296d665eae1e8ec6445683bfb999c884058426a
>
> Only after a user complaint, Intel bothered to reveal their reason for this:
>
> https://www.mail-archive.com/e1000-devel@lists.sourceforge.net/msg12615.html
>
> But this comes at the expense of NBASE-T users, who are left wondering why their
> NIC (which Intel sells as supporting NBASE-T) only comes up with GbE links. To
> fix this, I submitted this patch:
>
> https://lists.osuosl.org/pipermail/intel-wired-lan/Week-of-Mon-20211018/026326.html
>
> However, Intel devs pointed out to me that private module parameters would no
> longer be accepted. Indeed, after some search I found this in the archive:
>
> https://lore.kernel.org/netdev/20170324.144017.1545614773504954414.davem@davemloft.net/
>
> The reason given there is that a module parameter is the "worst user experience
> possible". But I think the absolutely worst user experience possible is having
> to figure out a complex script that:
>
> - compiles a list of all net devices provided by the ixgbe module
> - retrieves the supported link speeds and converts them to a hex mask
> - ORs the NBASE-T speeds into this hex mask
> - finally runs ethtool to set the hex mask of the speeds to advertise
>
> Even as a developer with 10 years experience with Linux, I would have to spend
> quite a while writing such a script, and then figuring out how to have it
> executed at the right time during startup. I suppose the vast majority of
> Linux admins would be overwhelmed with that.
>
> In contrast, explaining how to set the module parameter to control NBASE-T
> support is a two-liner, see my patch above where I added that to the ixgbe.rst
> module documentation. I think that's feasible for most Linux admins.
>
> So my question is: Can anyone come up with a solution allowing to control
> NBASE-T support in the ixgbe module in a way that's feasible for most Linux
> admins, that works without a module parameter?
>
> If not, could an exception be made for this patch to allow an extra parameter
> for the ixgbe module?
>
> Or does anyone have an even better idea?

If it can be set with ethtool already, and the issue is mostly the
user-friendliness of this interface, how about teaching ethtool a
symbolic parameter to do this for you? E.g. something equivalent to:

'ethtool --change eth0 advertise +nbase-t' ?

Personally I wouldn't mind having this (symbolic names) for all the
supported advertised modes; I also think it's a pain to have to go
lookup the bit values whenever I need to change this...

-Toke


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ixgbe: How to do this without a module parameter?
  2021-10-25 20:59 ` Toke Høiland-Jørgensen
@ 2021-10-25 21:11   ` Andrew Lunn
  2021-10-25 23:47     ` Robert Schlabbach
  2021-10-26 23:50     ` Aw: " Robert Schlabbach
  0 siblings, 2 replies; 8+ messages in thread
From: Andrew Lunn @ 2021-10-25 21:11 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: Robert Schlabbach, netdev

> If it can be set with ethtool already, and the issue is mostly the
> user-friendliness of this interface, how about teaching ethtool a
> symbolic parameter to do this for you? E.g. something equivalent to:
> 
> 'ethtool --change eth0 advertise +nbase-t' ?
> 
> Personally I wouldn't mind having this (symbolic names) for all the
> supported advertised modes; I also think it's a pain to have to go
> lookup the bit values whenever I need to change this...

Something like this has been talked about before, but nobody ever
spent the time to do it. ethtool has all the needed strings, so it
should not be too hard to actually do for link modes.

nbase-t is a bit different since it is not an actual link mode, but a
combination of many link modes.

What is also useful is that you can put ethtool settings into
/etc/network/interfaces.

	Andrew

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ixgbe: How to do this without a module parameter?
  2021-10-25 21:11   ` Andrew Lunn
@ 2021-10-25 23:47     ` Robert Schlabbach
  2021-10-26  0:32       ` Andrew Lunn
  2021-10-26 23:50     ` Aw: " Robert Schlabbach
  1 sibling, 1 reply; 8+ messages in thread
From: Robert Schlabbach @ 2021-10-25 23:47 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev

From: "Andrew Lunn" <andrew@lunn.ch>
> What is also useful is that you can put ethtool settings into
> /etc/network/interfaces.

Thank you very much, that was very helpful! I tried it, and indeed, I only had
to add one line to /etc/network/interfaces (the first three lines below were
already there):

# The primary network interface
allow-hotplug eno3
iface eno3 inet dhcp
     pre-up ethtool -s eno3 advertise 0x1800000001028 || true

And I found this to work just as well as the module parameter - the link came
right up at NBASE-T speed after boot.

This was on a Debian 11.1 setup. I suppose this may not work on all distros,
but it works well enough for me.

So I realize using ethtool is a viable solution after all and the module
parameter is not needed. I'd still wish the ixgbe driver would default to full
functionality and require the users with the "bad" switches in their networks
to employ ethtool to cripple its function, but I suppose that'd be tough to
sell to Intel...

Best regards,
-Robert Schlabbach

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ixgbe: How to do this without a module parameter?
  2021-10-25 23:47     ` Robert Schlabbach
@ 2021-10-26  0:32       ` Andrew Lunn
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2021-10-26  0:32 UTC (permalink / raw)
  To: Robert Schlabbach; +Cc: netdev

> So I realize using ethtool is a viable solution after all and the module
> parameter is not needed. I'd still wish the ixgbe driver would default to full
> functionality and require the users with the "bad" switches in their networks
> to employ ethtool to cripple its function, but I suppose that'd be tough to
> sell to Intel...

Maybe, maybe not. Quoting the above message:

> This is the first time I've heard of anyone asking for 2.5G or 5G
> outside of the telecom space, so we went with the option of changing
> the default.

NBASE-T is no longer just telecom space. It is slowly becoming more
and more popular in general deployment.

At some point, there will be more standard conforming switches than
broken switches, and then it would make sense to enable the higher
speeds by default. Especially when everybody else is doing
NBASE-T. You see a lot of ARM SoCs with such ports, etc.

	 Andrew

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Aw: Re: ixgbe: How to do this without a module parameter?
  2021-10-25 21:11   ` Andrew Lunn
  2021-10-25 23:47     ` Robert Schlabbach
@ 2021-10-26 23:50     ` Robert Schlabbach
  2021-10-27  0:15       ` Andrew Lunn
  1 sibling, 1 reply; 8+ messages in thread
From: Robert Schlabbach @ 2021-10-26 23:50 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Toke Høiland-Jørgensen, netdev

"Andrew Lunn" <andrew@lunn.ch> wrote:
> > Personally I wouldn't mind having this (symbolic names) for all the
> > supported advertised modes; I also think it's a pain to have to go
> > lookup the bit values whenever I need to change this...
>
> Something like this has been talked about before, but nobody ever
> spent the time to do it. ethtool has all the needed strings, so it
> should not be too hard to actually do for link modes.

It appears somebody already did, because I found that my Debian 11.1
server (with kernel version 5.14) actually takes this command:

$ ethtool -s eno3 advertise 2500baseT/Full on 5000baseT/Full on

The parsing seems to be implemented in the kernel, because when I copy
the binary over to my Ubuntu machine (with kernel version 4.15), any
attempt to specify a link mode name only yields:

ethtool: bad command line argument(s)

Best regards,
-Robert Schlabbach

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Re: ixgbe: How to do this without a module parameter?
  2021-10-26 23:50     ` Aw: " Robert Schlabbach
@ 2021-10-27  0:15       ` Andrew Lunn
  2021-10-27 19:12         ` Robert Schlabbach
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2021-10-27  0:15 UTC (permalink / raw)
  To: Robert Schlabbach; +Cc: Toke Høiland-Jørgensen, netdev

On Wed, Oct 27, 2021 at 01:50:52AM +0200, Robert Schlabbach wrote:
> "Andrew Lunn" <andrew@lunn.ch> wrote:
> > > Personally I wouldn't mind having this (symbolic names) for all the
> > > supported advertised modes; I also think it's a pain to have to go
> > > lookup the bit values whenever I need to change this...
> >
> > Something like this has been talked about before, but nobody ever
> > spent the time to do it. ethtool has all the needed strings, so it
> > should not be too hard to actually do for link modes.
> 
> It appears somebody already did, because I found that my Debian 11.1
> server (with kernel version 5.14) actually takes this command:
> 
> $ ethtool -s eno3 advertise 2500baseT/Full on 5000baseT/Full on
> 
> The parsing seems to be implemented in the kernel, because when I copy
> the binary over to my Ubuntu machine (with kernel version 4.15), any
> attempt to specify a link mode name only yields:

Actually, it might not be. A kernel that old might not have netlink
ethtool, just the old ioctl interface. It could be the command line
parsing is dependent on which API is used to the kernel.

ethtool --debug 255 eno3

will show you the netlink messages going back and forth.

	Andrew

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ixgbe: How to do this without a module parameter?
  2021-10-27  0:15       ` Andrew Lunn
@ 2021-10-27 19:12         ` Robert Schlabbach
  0 siblings, 0 replies; 8+ messages in thread
From: Robert Schlabbach @ 2021-10-27 19:12 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Toke Høiland-Jørgensen, netdev

"Andrew Lunn" <andrew@lunn.ch> wrote:
> On Wed, Oct 27, 2021 at 01:50:52AM +0200, Robert Schlabbach wrote:
> > The parsing seems to be implemented in the kernel
>
> Actually, it might not be. A kernel that old might not have netlink
> ethtool, just the old ioctl interface. It could be the command line
> parsing is dependent on which API is used to the kernel.
>
> ethtool --debug 255 eno3

I should have mentioned that I also searched the ethtool source code
for link mode name parsing and came up empty. While ethtool contains
link mode names, those are "private" to the dump_link_mode_caps()
function and unavailable for any other access/use.

The ethtool debug option you gave shows that ethtool indeed sends the
link mode name strings down to the kernel for parsing:

sending genetlink packet (100 bytes):
    msg length 100 ethool ETHTOOL_MSG_LINKMODES_SET
    ETHTOOL_MSG_LINKMODES_SET
        ETHTOOL_A_LINKMODES_HEADER
            ETHTOOL_A_HEADER_DEV_NAME = "eno3"
        ETHTOOL_A_LINKMODES_OURS
            ETHTOOL_A_BITSET_BITS
                ETHTOOL_A_BITSET_BITS_BIT
                    ETHTOOL_A_BITSET_BIT_NAME = "2500baseT/Full"
                    ETHTOOL_A_BITSET_BIT_VALUE = true
                ETHTOOL_A_BITSET_BITS_BIT
                    ETHTOOL_A_BITSET_BIT_NAME = "5000baseT/Full"
                    ETHTOOL_A_BITSET_BIT_VALUE = true

And trying nonsense names for link modes confirms that ethtool does not
parse them at all, but rather passes them unchecked:

$ ethtool --debug 255 -s eno3 advertise NonSenseModeName off EvenMoreNonSense on

sending genetlink packet (104 bytes):
    msg length 104 ethool ETHTOOL_MSG_LINKMODES_SET
    ETHTOOL_MSG_LINKMODES_SET
        ETHTOOL_A_LINKMODES_HEADER
            ETHTOOL_A_HEADER_DEV_NAME = "eno3"
        ETHTOOL_A_LINKMODES_OURS
            ETHTOOL_A_BITSET_BITS
                ETHTOOL_A_BITSET_BITS_BIT
                    ETHTOOL_A_BITSET_BIT_NAME = "NonSenseModeName"
                ETHTOOL_A_BITSET_BITS_BIT
                    ETHTOOL_A_BITSET_BIT_NAME = "EvenMoreNonSense"
                    ETHTOOL_A_BITSET_BIT_VALUE = true

Running these commands on an old 4.15 kernel fails very early, supposedly when
ethtool checks whether the kernel supports above messages:

sending genetlink packet (32 bytes):
    msg length 32 genl-ctrl
    CTRL_CMD_GETFAMILY
        CTRL_ATTR_FAMILY_NAME = "ethtool"

offending message:
    ETHTOOL_MSG_LINKINFO_SET
        ETHTOOL_A_LINKINFO_PORT = 101
ethtool: bad command line argument(s)
For more information run ethtool -h

So a more user-friendly option to specify link modes to advertise is already
implemented, but only in more recent kernel versions.

Best regards,
-Robert Schlabbach

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-10-27 19:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-25 20:23 ixgbe: How to do this without a module parameter? Robert Schlabbach
2021-10-25 20:59 ` Toke Høiland-Jørgensen
2021-10-25 21:11   ` Andrew Lunn
2021-10-25 23:47     ` Robert Schlabbach
2021-10-26  0:32       ` Andrew Lunn
2021-10-26 23:50     ` Aw: " Robert Schlabbach
2021-10-27  0:15       ` Andrew Lunn
2021-10-27 19:12         ` Robert Schlabbach

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