Re: [PATCH V3 net] net: hns3: fix speed configuration residue after driver reload

[Simon Horman]

On Fri, Jul 24, 2026 at 05:30:36PM +0800, Jijie Shao wrote:
> After setting a 100G optical port to 40G via ethtool and reloading
> the driver, the port remains at 40G instead of reverting to the
> firmware default speed of 100G.
> 
> The commit referenced in Fixes: added two overwrites in
> hclge_init_ae_dev() for non-copper media, so that optical ports
> connected to forced-mode remotes inherit the firmware-preset
> autoneg and speed instead of the hardcoded defaults:
> 
>   req_autoneg = mac.autoneg
>   req_speed   = mac.speed         (when autoneg disabled)
> 
> The autoneg overwrite keeps existing behavior:
> hclge_set_autoneg_speed_dup() already uses mac.autoneg (not
> req_autoneg) since it was introduced, so autoneg inheritance from
> firmware was already in place. This part is kept.

The AI-generated review on netdev-ai [1] flags that this isn't strictly
true as req_autoneg does appear to be used in hclge_set_autoneg_speed_dup().
I don't want to nitpick, but perhaps this is worth clarifying.

[1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260724093036.426631-1-shaojijie%40huawei.com

> The speed overwrite, however, introduces the residue: mac.speed
> reflects whatever was last programmed into the MAC, and after unload
> firmware does not restore the MAC speed to the flash default. So if
> the user changed speed via ethtool in a prior load, mac.speed still
> carries that value on reload and req_speed inherits it.
> 
> Fix by dropping the req_speed overwrite only. req_speed keeps the
> firmware default value set in hclge_configure() (cfg.default_speed),
> so a reload reverts the speed to default, matching the expectation
> that a driver reload resets link configuration.
> 
> Trade-off: on optical ports whose firmware default speed does not
> match a forced-mode remote, reload now drops the link and the user
> must re-apply ethtool configuration. This is acceptable: a driver
> reload is expected to reset link configuration, not to inherit
> runtime state from before unload. The autoneg inheritance is left
> in place as established behavior; changing it is out of scope for
> this patch and would itself be a user-perceivable behavior change.
> 
> Fixes: d9d349c4e8a0 ("net: hns3: differentiate autoneg default values between copper and fiber")
> Signed-off-by: Jijie Shao <shaojijie@xxxxxxxxxx>
> ---
> v3:
>  - Expand commit message to explain the two overwrites added by the
>    Fixes commit: req_autoneg inheritance is kept because it matches
>    existing behavior of hclge_set_autoneg_speed_dup(); req_speed
>    sync is dropped because it introduces the reported residue.
>  - State explicitly that link loss on forced-mode optical ports after
>    reload is acceptable, since a driver reload is expected to reset
>    link configuration rather than inherit runtime state.
>  - Explain why autoneg inheritance is left unchanged: it is
>    established behavior, and changing it is out of scope and would
>    be user-perceivable.

...