Thread (4 messages) 4 messages, 2 authors, 2h ago

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

From: Jijie Shao <shaojijie@huawei.com>
Date: 2026-07-29 11:45:27
Also in: lkml

on 2026/7/29 19:03, Simon Horman wrote:
On Fri, Jul 24, 2026 at 05:30:36PM +0800, Jijie Shao wrote:
quoted
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 intent is to restore the pre-c711f6d1cee9 behavior for non-copper
media, where the helper read mac.autoneg directly and fiber ports
inherited firmware autoneg at init. c711f6d1cee9 switched the helper
to req_autoneg, breaking that inheritance; d9d349c4e8a0 (same patchset)
added the explicit req_autoneg = autoneg copy to restore
it, and also added a req_speed = mac.speed copy.

This patch keeps the req_autoneg copy — load-bearing as the sole
init-time path for non-copper media — and drops the req_speed copy,
which is the residue source. The commit message wording about the
helper reading mac.autoneg describes the pre-c711f6d1cee9 state,
not the current code.

Thanks
Jijie.

quoted
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@huawei.com>
---
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.
...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help