public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Jakub Kicinski <kuba@kernel.org>, Andrew Lunn <andrew@lunn.ch>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Eric Dumazet <edumazet@google.com>, Rob Herring <robh@kernel.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Donald Hunter <donald.hunter@gmail.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Jonathan Corbet <corbet@lwn.net>,
	John Fastabend <john.fastabend@gmail.com>,
	Lukasz Majewski <lukma@denx.de>,
	Maxime Chevallier <maxime.chevallier@bootlin.com>,
	Stanislav Fomichev <sdf@fomichev.me>,
	Paolo Abeni <pabeni@redhat.com>, Jiri Pirko <jiri@resnulli.us>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	Divya.Koppera@microchip.com,
	Kory Maincent <kory.maincent@bootlin.com>,
	Vadim Fedorenko <vadim.fedorenko@linux.dev>,
	netdev@vger.kernel.org, Sabrina Dubroca <sd@queasysnail.net>,
	linux-kernel@vger.kernel.org, kernel@pengutronix.de,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [PATCH net-next v8 1/1] Documentation: net: add flow control guide and document ethtool API
Date: Thu, 27 Nov 2025 16:10:21 +0000	[thread overview]
Message-ID: <aSh37U5VOgYqmzqN@shell.armlinux.org.uk> (raw)
In-Reply-To: <aSgX9ue6uUheX4aB@pengutronix.de>

On Thu, Nov 27, 2025 at 10:20:54AM +0100, Oleksij Rempel wrote:
> 2. **Forced Mode** (``autoneg off``)

This should state that this is the "autoneg" on the ethtool -A / --pause
command, not the ethtool -s / --change command.

> Pre-link Configuration (Administrative UP, Physical DOWN) How should drivers
> handle set_pauseparam when the link is physically down?
> 
>  Fully Forced: If speed/duplex are forced, we can validate the pause request
>  immediately.
> 
>  Parallel Detection: If the link comes up later (e.g., as Half Duplex via
>  parallel detection), a previously accepted "forced pause" configuration might
>  become invalid. Should we block forced pause settings until the link is
>  physically up?

Why would the users request become invalid? Why should the user have to
re-set their requested policy if the link flips from FD to HD and back
to FD for whatever reason? The kernel should accept the users requested
policy, and apply it when appropriate (in other words, when in FD mode.)

> State Persistence and Toggling When toggling autoneg (e.g., autoneg on -> off
> -> on), should the kernel or driver cache the previous advertisement?
> 
>   Currently, if a user switches to forced mode and back, the previous
>   advertisement preferences might be lost or reset to defaults depending on the
>   driver.

Turning pause autoneg off should not change the advertisement. It should
be thought of a control that selects whether the results of autoneg are
used vs not used. Note that phylink updates the advertisement even when
pause autoneg is turned off. This follows the stated API documentation
(please ensure your documentation conforms to the already existing API
documentation, and doesn't inadvertently propose something different -
this is exactly why I hate that we're getting multiple definitions of
the same stuff in different places.)

 * If the link is autonegotiated, drivers should use
 * mii_advertise_flowctrl() or similar code to set the advertised
 * pause frame capabilities based on the @rx_pause and @tx_pause flags,
 * even if @autoneg is zero.  They should also allow the advertised
 * pause frame capabilities to be controlled directly through the
 * advertising field of &struct ethtool_cmd.

Note that this requires that the advertisement is updated even if pause
autoneg is zero. Phylink implements this.

>   Similarly, if no administrative configuration has ever been set, what should
>   get_pauseparam report? Should it read the current hardware state (which might
>   be default) or return zero/empty?
> 
> Synchronization with Link Modes Configuring pause via set_pauseparam vs.
> link_ksettings can lead to desynchronization.
> 
>   My testing shows that set_pauseparam often updates the driver's internal
>   pause state but may not trigger the necessary link reset/re-advertisement
>   that link_ksettings does.
> 
>   This results in the reported "Advertised" pause modes in ethtool output being
>   out of sync with the actual Pause API settings.
> 
>   Combining configuration over different interfaces sometimes will avoid
>   link reset, so new configuration is not advertised.

... which I'm sure Andrew will argue is a reason for drivers to use
phylink which implements this properly!

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

  parent reply	other threads:[~2025-11-27 16:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-19 14:03 [PATCH net-next v8 1/1] Documentation: net: add flow control guide and document ethtool API Oleksij Rempel
2025-11-26  2:19 ` Jakub Kicinski
2025-11-26  8:36   ` Oleksij Rempel
2025-11-26 22:42     ` Jakub Kicinski
2025-11-27  9:20       ` Oleksij Rempel
2025-11-27 15:07         ` Andrew Lunn
2025-11-27 15:31           ` Maxime Chevallier
2025-11-27 15:48             ` Andrew Lunn
2025-11-27 16:18               ` Russell King (Oracle)
2025-11-27 16:14           ` Russell King (Oracle)
2025-11-28  1:21             ` Jakub Kicinski
2025-11-27 16:10         ` Russell King (Oracle) [this message]
2025-11-28  1:27     ` Russell King (Oracle)
2025-11-28  1:47       ` Russell King (Oracle)
2025-11-28  8:55         ` Oleksij Rempel
2025-11-28  9:35           ` Russell King (Oracle)
2025-11-28 18:32           ` Jakub Kicinski
2025-11-28 20:16             ` Andrew Lunn
2025-11-28 20:38               ` Russell King (Oracle)
2025-11-28 22:17                 ` Jakub Kicinski
2025-12-01  9:49                   ` Oleksij Rempel

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=aSh37U5VOgYqmzqN@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=Divya.Koppera@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=ast@kernel.org \
    --cc=corbet@lwn.net \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=hawk@kernel.org \
    --cc=hkallweit1@gmail.com \
    --cc=jiri@resnulli.us \
    --cc=john.fastabend@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=kory.maincent@bootlin.com \
    --cc=krzk+dt@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukma@denx.de \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=pabeni@redhat.com \
    --cc=robh@kernel.org \
    --cc=sd@queasysnail.net \
    --cc=sdf@fomichev.me \
    --cc=vadim.fedorenko@linux.dev \
    --cc=vladimir.oltean@nxp.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