netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings"
@ 2024-03-07 10:09 Sagar Dhoot (QUIC)
  2024-03-08  4:14 ` Andrew Lunn
  0 siblings, 1 reply; 8+ messages in thread
From: Sagar Dhoot (QUIC) @ 2024-03-07 10:09 UTC (permalink / raw)
  To: mkubecek@suse.cz, netdev@vger.kernel.org
  Cc: Nagarjuna Chaganti (QUIC), Priya Tripathi (QUIC)

Hi Michal and Team,

We are developing an Ethernet driver here in Qualcomm and have a query w.r.t one of the limitations that we have observed with ethtool.

Requirement:
	- We are trying to manage the advertised speed modes based on the input from "ethtool -s" command, and trying to support options like "speed", "autoneg" and "lanes".
	- By default, the "get_link_ksettings" will publish all the supported/advertised speed modes.
	- If the speed is modified using "set_link_ksettings", we will limit the advertised speed mode corresponding to the speed being passed in the ethtool command.
	- And if in the subsequent "set_link_ksettings" command, if the speed is not provided, we want to reset and go back to the default state(i.e. again publish all the supported/advertised speed modes).

Detailed issue sequence and the commands executed:
1. "ethtool eth_interface"
	a. Assuming eth_interface is the interface name.
	b. By default, the "get_link_ksettings" will publish all the supported/advertised speed modes. Let's say we support 10G and 25G. In that case both speed modes will be advertised in the ethtool output.
2. "ethtool -s eth_interface speed 25000 autoneg off"
	a. "set_link_ksettings" will be called and speed value will be passed as 25G.
	b. Advertised speed mode will be restricted to 25G.
	c. Link comes up fine with 25G.
3. "ethtool eth_interface"
	a. "get_link_ksettings" will publish the link as up with 25G in the ethtool output. Advertised speed mode will be set to 25G and 10G will not be included in that list.
4. "ethtool -s eth_interface autoneg off"
	a. "get_link_ksettings" will be called and as per our implementation, as the link as up, we will return the speed as 25G.
	b. "set_link_ksettings" will be called and speed will still be provided as 25G(from step 4a), even though the user has not provided any speed value in the ethtool command.
	c. We will still restrict the advertised speed to 25G. Whereas the expectation was that we reset back to a combination of 10G and 25G again.

So basically, we are trying to understand if there is a way in "set_link_ksettings" to differentiate if the speed value was passed by the user, or not. In short, a way to differentiate between:
	- Speed value explicitly passed by user via ethtool command as seen in step 2a.
		v/s
	- Speed value not set by user but still being passed after queried with "get_link_ksettings" in step 4b.


Can you please help investigate this query and let us know if you need any further details. Thanks in advance!


Thanks,
Sagar


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

* Re: Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings"
  2024-03-07 10:09 Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings" Sagar Dhoot (QUIC)
@ 2024-03-08  4:14 ` Andrew Lunn
  2024-03-08  6:33   ` Sagar Dhoot (QUIC)
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2024-03-08  4:14 UTC (permalink / raw)
  To: Sagar Dhoot (QUIC)
  Cc: mkubecek@suse.cz, netdev@vger.kernel.org,
	Nagarjuna Chaganti (QUIC), Priya Tripathi (QUIC)

On Thu, Mar 07, 2024 at 10:09:18AM +0000, Sagar Dhoot (QUIC) wrote:
> Hi Michal and Team,
> 
> We are developing an Ethernet driver here in Qualcomm and have a query w.r.t one of the limitations that we have observed with ethtool.

Hi Sagar

Please configure your mail client to wrap emails to a little less than
80 characters.

> Detailed issue sequence and the commands executed:
> 1. "ethtool eth_interface"
> 	a. Assuming eth_interface is the interface name.
> 	b. By default, the "get_link_ksettings" will publish all the supported/advertised speed modes. Let's say we support 10G and 25G. In that case both speed modes will be advertised in the ethtool output.
> 2. "ethtool -s eth_interface speed 25000 autoneg off"
> 	a. "set_link_ksettings" will be called and speed value will be passed as 25G.
> 	b. Advertised speed mode will be restricted to 25G.

autoneg is off. So advertised does not matter. You are not advertising
anything. You force the PHY and MAC to a specific speed. You should
not touch your local copy of what the PHY is advertising at this
point. You just disable advertisement in the PHY. The link partner
should then see that autoneg is off, and drop the link. You then need
to configure the partner in the same way, so both ends are forced to
the same mode. The link should then come up.

> 	c. Link comes up fine with 25G.
> 3. "ethtool eth_interface"
> 	a. "get_link_ksettings" will publish the link as up with 25G in the ethtool output. Advertised speed mode will be set to 25G and 10G will not be included in that list.

Nope. I would expect advertised to be still 10G and 25G. All you have
done is disable the PHY from advertisement anything.

When you re-enable autoneg, the PHY should then advertise it can do
25G and 40G to the link peer.

> 4. "ethtool -s eth_interface autoneg off"
> 	a. "get_link_ksettings" will be called and as per our implementation, as the link as up, we will return the speed as 25G.

So you have turned autoneg off, but not specified how the MAC/PHY
should be forced. Defaulting to the last link speed seems sensible.

Maybe you are mixing up advertise on/off with advertise N which allows
you to limit what link modes the PHY will advertise it supports?

       Andrew

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

* RE: Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings"
  2024-03-08  4:14 ` Andrew Lunn
@ 2024-03-08  6:33   ` Sagar Dhoot (QUIC)
  2024-03-08  7:15     ` Michal Kubecek
  2024-03-08 13:56     ` Andrew Lunn
  0 siblings, 2 replies; 8+ messages in thread
From: Sagar Dhoot (QUIC) @ 2024-03-08  6:33 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: mkubecek@suse.cz, netdev@vger.kernel.org,
	Nagarjuna Chaganti (QUIC), Priya Tripathi (QUIC)

Hi Andrew,

Thanks for the quick response. Maybe I have put up a confusing scenario.

Let me rephrase with autoneg on.

1. "ethtool eth_interface"
2. "ethtool -s eth_interface speed 25000 autoneg on"
3. "ethtool -s eth_interface autoneg on"

Once the link is up at step 2, "get_link_ksettings" will return the speed as
25G. And if "set_link_ksettings" is invoked at step 3, it will still pass the 
speed value as 25G retrieved with "get_link_ksettings", even though the 
speed was not explicitly specified in the ethtool command. So, after step2,
if I must go back to the default state i.e., advertise all the supported speed 
modes, is there any way to do so?

Thanks,
Sagar

-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch> 
Sent: Friday, March 8, 2024 9:44 AM
To: Sagar Dhoot (QUIC) <quic_sdhoot@quicinc.com>
Cc: mkubecek@suse.cz; netdev@vger.kernel.org; Nagarjuna Chaganti (QUIC) <quic_nchagant@quicinc.com>; Priya Tripathi (QUIC) <quic_ppriyatr@quicinc.com>
Subject: Re: Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings"

WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.

On Thu, Mar 07, 2024 at 10:09:18AM +0000, Sagar Dhoot (QUIC) wrote:
> Hi Michal and Team,
>
> We are developing an Ethernet driver here in Qualcomm and have a query w.r.t one of the limitations that we have observed with ethtool.

Hi Sagar

Please configure your mail client to wrap emails to a little less than
80 characters.

> Detailed issue sequence and the commands executed:
> 1. "ethtool eth_interface"
>       a. Assuming eth_interface is the interface name.
>       b. By default, the "get_link_ksettings" will publish all the supported/advertised speed modes. Let's say we support 10G and 25G. In that case both speed modes will be advertised in the ethtool output.
> 2. "ethtool -s eth_interface speed 25000 autoneg off"
>       a. "set_link_ksettings" will be called and speed value will be passed as 25G.
>       b. Advertised speed mode will be restricted to 25G.

autoneg is off. So advertised does not matter. You are not advertising anything. You force the PHY and MAC to a specific speed. You should not touch your local copy of what the PHY is advertising at this point. You just disable advertisement in the PHY. The link partner should then see that autoneg is off, and drop the link. You then need to configure the partner in the same way, so both ends are forced to the same mode. The link should then come up.

>       c. Link comes up fine with 25G.
> 3. "ethtool eth_interface"
>       a. "get_link_ksettings" will publish the link as up with 25G in the ethtool output. Advertised speed mode will be set to 25G and 10G will not be included in that list.

Nope. I would expect advertised to be still 10G and 25G. All you have done is disable the PHY from advertisement anything.

When you re-enable autoneg, the PHY should then advertise it can do 25G and 40G to the link peer.

> 4. "ethtool -s eth_interface autoneg off"
>       a. "get_link_ksettings" will be called and as per our implementation, as the link as up, we will return the speed as 25G.

So you have turned autoneg off, but not specified how the MAC/PHY should be forced. Defaulting to the last link speed seems sensible.

Maybe you are mixing up advertise on/off with advertise N which allows you to limit what link modes the PHY will advertise it supports?

       Andrew

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

* Re: Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings"
  2024-03-08  6:33   ` Sagar Dhoot (QUIC)
@ 2024-03-08  7:15     ` Michal Kubecek
  2024-03-08 10:24       ` Sagar Dhoot (QUIC)
  2024-03-08 13:56     ` Andrew Lunn
  1 sibling, 1 reply; 8+ messages in thread
From: Michal Kubecek @ 2024-03-08  7:15 UTC (permalink / raw)
  To: Sagar Dhoot (QUIC)
  Cc: Andrew Lunn, netdev@vger.kernel.org, Nagarjuna Chaganti (QUIC),
	Priya Tripathi (QUIC)

[-- Attachment #1: Type: text/plain, Size: 1804 bytes --]

On Fri, Mar 08, 2024 at 06:33:00AM +0000, Sagar Dhoot (QUIC) wrote:
> Hi Andrew,
> 
> Thanks for the quick response. Maybe I have put up a confusing scenario.
> 
> Let me rephrase with autoneg on.
> 
> 1. "ethtool eth_interface"
> 2. "ethtool -s eth_interface speed 25000 autoneg on"
> 3. "ethtool -s eth_interface autoneg on"
> 
> Once the link is up at step 2, "get_link_ksettings" will return the
> speed as 25G. And if "set_link_ksettings" is invoked at step 3, it
> will still pass the speed value as 25G retrieved with
> "get_link_ksettings", even though the speed was not explicitly
> specified in the ethtool command. So, after step2, if I must go back
> to the default state i.e., advertise all the supported speed 
> modes, is there any way to do so?

IIRC this is backward compatible with how the ioctl interface behaves.
The logic is that if a parameter is omitted, it is supposed to be
preserved; thus the third command simply means "enable the
autonegotiation" and don't do anything else (which is a no-op in this
case).

But I agree that it would be convenient to have a shortcut for "enable
the autonegotiation with all supported modes". On the command line it
could be e.g. something like

  ethtool -s $iface autoneg on advertise all

or

  ethtool -s $iface autoneg on advertise supported

On the implementation level, the problem is that IIRC we have no easy
way to express such request in current netlink API. It could be emulated
by querying the modes first (which returns both advertised and supported
modes) and requesting supported modes to be advertised but that's not
very practical. So probably the best solution would be introducing a new
flag and using the complicated way as a fallback if the kernel does not
support it.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* RE: Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings"
  2024-03-08  7:15     ` Michal Kubecek
@ 2024-03-08 10:24       ` Sagar Dhoot (QUIC)
  0 siblings, 0 replies; 8+ messages in thread
From: Sagar Dhoot (QUIC) @ 2024-03-08 10:24 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: Andrew Lunn, netdev@vger.kernel.org, Nagarjuna Chaganti (QUIC),
	Priya Tripathi (QUIC)

Hi Michal,

Thanks for your response.

Given that we don't have a straightforward way to do such
differentiation in the current framework, we will try to 
manage this internally.

Thanks,
Sagar

-----Original Message-----
From: Michal Kubecek <mkubecek@suse.cz> 
Sent: Friday, March 8, 2024 12:46 PM
To: Sagar Dhoot (QUIC) <quic_sdhoot@quicinc.com>
Cc: Andrew Lunn <andrew@lunn.ch>; netdev@vger.kernel.org; Nagarjuna Chaganti (QUIC) <quic_nchagant@quicinc.com>; Priya Tripathi (QUIC) <quic_ppriyatr@quicinc.com>
Subject: Re: Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings"

On Fri, Mar 08, 2024 at 06:33:00AM +0000, Sagar Dhoot (QUIC) wrote:
> Hi Andrew,
> 
> Thanks for the quick response. Maybe I have put up a confusing scenario.
> 
> Let me rephrase with autoneg on.
> 
> 1. "ethtool eth_interface"
> 2. "ethtool -s eth_interface speed 25000 autoneg on"
> 3. "ethtool -s eth_interface autoneg on"
> 
> Once the link is up at step 2, "get_link_ksettings" will return the 
> speed as 25G. And if "set_link_ksettings" is invoked at step 3, it 
> will still pass the speed value as 25G retrieved with 
> "get_link_ksettings", even though the speed was not explicitly 
> specified in the ethtool command. So, after step2, if I must go back 
> to the default state i.e., advertise all the supported speed modes, is 
> there any way to do so?

IIRC this is backward compatible with how the ioctl interface behaves.
The logic is that if a parameter is omitted, it is supposed to be preserved; thus the third command simply means "enable the autonegotiation" and don't do anything else (which is a no-op in this case).

But I agree that it would be convenient to have a shortcut for "enable the autonegotiation with all supported modes". On the command line it could be e.g. something like

  ethtool -s $iface autoneg on advertise all

or

  ethtool -s $iface autoneg on advertise supported

On the implementation level, the problem is that IIRC we have no easy way to express such request in current netlink API. It could be emulated by querying the modes first (which returns both advertised and supported
modes) and requesting supported modes to be advertised but that's not very practical. So probably the best solution would be introducing a new flag and using the complicated way as a fallback if the kernel does not support it.

Michal

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

* Re: Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings"
  2024-03-08  6:33   ` Sagar Dhoot (QUIC)
  2024-03-08  7:15     ` Michal Kubecek
@ 2024-03-08 13:56     ` Andrew Lunn
  2024-03-08 14:16       ` Michal Kubecek
  1 sibling, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2024-03-08 13:56 UTC (permalink / raw)
  To: Sagar Dhoot (QUIC)
  Cc: mkubecek@suse.cz, netdev@vger.kernel.org,
	Nagarjuna Chaganti (QUIC), Priya Tripathi (QUIC)

On Fri, Mar 08, 2024 at 06:33:00AM +0000, Sagar Dhoot (QUIC) wrote:
> Hi Andrew,
> 
> Thanks for the quick response. Maybe I have put up a confusing scenario.
> 
> Let me rephrase with autoneg on.
> 
> 1. "ethtool eth_interface"
> 2. "ethtool -s eth_interface speed 25000 autoneg on"

phylib will ignore speed if autoneg is on

https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phy.c#L1044

It only copies speed/duplex when autoneg is off.

	if (autoneg == AUTONEG_DISABLE) {
		phydev->speed = speed;
		phydev->duplex = duplex;
	}

When configuring the PHY:

https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phy_device.c#L2234

	if (AUTONEG_ENABLE != phydev->autoneg)
		return genphy_setup_forced(phydev);

So we only force the PHY when autoneg is off, and it is
genphy_setup_forced() which uses this speed/duplex.

If you are trying to influence the link mode while autoneg is
enabled, you should be using advertise N

          advertise N
                  Sets the speed and  duplex  advertised  by  autonegotiation.
                  The  argument is a hexadecimal value using one or a combina‐
                  tion of the following values:
                  0x001                          10baseT Half
                  0x002                          10baseT Full
                  0x100000000000000000000000     10baseT1L Full
                  0x8000000000000000000000000    10baseT1S Full
                  0x10000000000000000000000000   10baseT1S Half
                  0x20000000000000000000000000   10baseT1S_P2MP Half
                  0x004                          100baseT Half
                  0x008                          100baseT Full

I think the hexadecimal part is out of date and you can now also use
symbolic names.

Another thing to think about. What does the speed here actually mean:

> 2. "ethtool -s eth_interface speed 25000 autoneg on"

phylib assumes it is the baseT speed:

        ETHTOOL_LINK_MODE_10baseT_Half_BIT      = 0,
        ETHTOOL_LINK_MODE_10baseT_Full_BIT      = 1,
        ETHTOOL_LINK_MODE_100baseT_Half_BIT     = 2,
        ETHTOOL_LINK_MODE_100baseT_Full_BIT     = 3,
        ETHTOOL_LINK_MODE_1000baseT_Half_BIT    = 4,
        ETHTOOL_LINK_MODE_1000baseT_Full_BIT    = 5,

However, what does 25000 mean? We currently don't have a
25000BaseT. So how do you decide which of the following to select:

        ETHTOOL_LINK_MODE_25000baseCR_Full_BIT  = 31,
        ETHTOOL_LINK_MODE_25000baseKR_Full_BIT  = 32,
        ETHTOOL_LINK_MODE_25000baseSR_Full_BIT  = 33,

This is where advertise N is much better, an actual linkmode, not just
a speed.

	 Andrew


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

* Re: Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings"
  2024-03-08 13:56     ` Andrew Lunn
@ 2024-03-08 14:16       ` Michal Kubecek
  2024-03-08 15:13         ` Andrew Lunn
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Kubecek @ 2024-03-08 14:16 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Sagar Dhoot (QUIC), netdev@vger.kernel.org,
	Nagarjuna Chaganti (QUIC), Priya Tripathi (QUIC)

[-- Attachment #1: Type: text/plain, Size: 910 bytes --]

On Fri, Mar 08, 2024 at 02:56:30PM +0100, Andrew Lunn wrote:
> On Fri, Mar 08, 2024 at 06:33:00AM +0000, Sagar Dhoot (QUIC) wrote:
> > Hi Andrew,
> > 
> > Thanks for the quick response. Maybe I have put up a confusing scenario.
> > 
> > Let me rephrase with autoneg on.
> > 
> > 1. "ethtool eth_interface"
> > 2. "ethtool -s eth_interface speed 25000 autoneg on"
> 
> phylib will ignore speed if autoneg is on

It is not passed there. With "autoneg on", speed, duplex and lanes are
interpreted as conditions for modes to be advertised, i.e. they are
translated to the same request as "advertise ..." with list of supported
modes matching the parameters that were used.

In the ioctl code path, this logic is handled in userspace ethtool (but
only in a limited scope), for netlink API the logic is implemented in
kernel function ethnl_auto_linkmodes() in net/ethtool/linkmodes.c.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings"
  2024-03-08 14:16       ` Michal Kubecek
@ 2024-03-08 15:13         ` Andrew Lunn
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2024-03-08 15:13 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: Sagar Dhoot (QUIC), netdev@vger.kernel.org,
	Nagarjuna Chaganti (QUIC), Priya Tripathi (QUIC)

On Fri, Mar 08, 2024 at 03:16:24PM +0100, Michal Kubecek wrote:
> On Fri, Mar 08, 2024 at 02:56:30PM +0100, Andrew Lunn wrote:
> > On Fri, Mar 08, 2024 at 06:33:00AM +0000, Sagar Dhoot (QUIC) wrote:
> > > Hi Andrew,
> > > 
> > > Thanks for the quick response. Maybe I have put up a confusing scenario.
> > > 
> > > Let me rephrase with autoneg on.
> > > 
> > > 1. "ethtool eth_interface"
> > > 2. "ethtool -s eth_interface speed 25000 autoneg on"
> > 
> > phylib will ignore speed if autoneg is on
> 
> It is not passed there. With "autoneg on", speed, duplex and lanes are
> interpreted as conditions for modes to be advertised, i.e. they are
> translated to the same request as "advertise ..." with list of supported
> modes matching the parameters that were used.

Ah! Interesting. I did not know that.

Thanks
	Andrew

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

end of thread, other threads:[~2024-03-08 15:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-07 10:09 Ethtool query: Reset advertised speed modes if speed value is not passed in "set_link_ksettings" Sagar Dhoot (QUIC)
2024-03-08  4:14 ` Andrew Lunn
2024-03-08  6:33   ` Sagar Dhoot (QUIC)
2024-03-08  7:15     ` Michal Kubecek
2024-03-08 10:24       ` Sagar Dhoot (QUIC)
2024-03-08 13:56     ` Andrew Lunn
2024-03-08 14:16       ` Michal Kubecek
2024-03-08 15:13         ` Andrew Lunn

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