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

[Jijie Shao <shaojijie@huawei.com>]

on 2026/7/29 19:03, Simon Horman wrote:
> 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 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.


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