Re: [PATCH V2 net] net: hns3: fix speed configuration residue after driver reload
From: Jijie Shao
Date: Fri Jul 24 2026 - 03:12:47 EST
on 2026/7/21 19:05, Paolo Abeni wrote:
From: AI Reviewer <ai@xxxxxxxxxxx>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: hns3: fix speed configuration residue after driver reload
This change drops the req_speed overwrite in hclge_init_ae_dev() so
req_speed keeps the firmware default set during hclge_configure()
after a driver reload, and only req_autoneg continues to be
synchronized from mac.autoneg on non-copper media.
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c[High]
index fc8587c80813..164c3ecf195c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -9498,12 +9498,8 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
if (ret)
goto err_ptp_uninit;
- if (hdev->hw.mac.media_type != HNAE3_MEDIA_TYPE_COPPER) {
+ if (hdev->hw.mac.media_type != HNAE3_MEDIA_TYPE_COPPER)
hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg;
- if (hdev->hw.mac.autoneg == AUTONEG_DISABLE &&
- hdev->hw.mac.speed != SPEED_UNKNOWN)
- hdev->hw.mac.req_speed = hdev->hw.mac.speed;
- }
Could removing this block reintroduce the scenario that d9d349c4e8a0
("net: hns3: differentiate autoneg default values between copper and
fiber") was written to handle?
The Fixes-tagged commit added this req_speed overwrite specifically for
optical ports where firmware presets autoneg=DISABLE at a specific
forced speed (a case its commit message describes as common in data
centers where the remote switch runs in forced mode).
There appear to be two distinct notions of "firmware default" involved:
- cfg.default_speed read from flash static parameters via
hclge_get_cfg() and stored into req_speed by hclge_configure()
- the live firmware/SFP state read via hclge_get_sfp_info() into
mac.autoneg and mac.speed
On a fiber port where the flash default_speed is 100G but firmware has
preset the port to forced 25G to match a fixed-speed remote, after this
change:
hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg; /* = DISABLE */
/* req_speed still holds flash default 100G */
hclge_set_autoneg_speed_dup() would then see !req_autoneg and call:
hclge_cfg_mac_speed_dup_hw(hdev, hdev->hw.mac.req_speed=100G, ...);
forcing 100G against a 25G-only peer.
Is that scenario still handled, or is it expected not to occur in
practice?
We want driver reload to reset MAC speed to the firmware default.
If that doesn't match a forced-mode remote on an optical port,
link drops and the user reconfigures with ethtool. That's fine.
The req_speed = mac.speed line was added in the V2 of the June
series to avoid that link drop, but it caused the actual reported
bug: speed stays at the last user setting across reload. This
patch drops that line and goes back to the V1 behavior, which
only synced req_autoneg.
V1:
https://lore.kernel.org/all/20260617112721.75186-5-shaojijie@xxxxxxxxxx/
V2:
https://lore.kernel.org/all/20260624141319.271439-5-shaojijie@xxxxxxxxxx/
If we need to cover the forced-mode optical case without
reintroducing the residue, that's a separate problem — a one-line
req_speed = mac.speed can't tell user-configured speed apart
from firmware-preset speed.
[Medium]
The commit message reasons about mac.speed carrying over user ethtool
changes across a driver reload:
"In hclge_init_ae_dev(), hclge_update_port_info() reads mac.speed
from hardware, which reflects the last user configuration (e.g.
ethtool changes), not the firmware default."
Does the same reasoning apply to mac.autoneg?
hclge_get_sfp_info() sets mac->autoneg from the live firmware state, so
a prior "ethtool -s ethX autoneg off speed 40000" on a 100G optical
port would leave firmware reporting autoneg=DISABLE after reload.
After this change, req_speed correctly reverts to the flash default
100G, but this line still runs:
hdev->hw.mac.req_autoneg = hdev->hw.mac.autoneg;
so req_autoneg stays at AUTONEG_DISABLE rather than reverting to the
AUTONEG_ENABLE default that hclge_configure() would set for fiber.
Why is speed residue fixed while autoneg residue is preserved?
The asymmetry is on purpose. req_autoneg has been inherited from mac.autoneg since long before this work; that behavior has always been there and is left unchanged to avoid surprising users. req_speed = mac.speed was added in June V2 and is what caused the reported residue, so dropping it is the fix. I'll send a v3 with the commit message noting that link loss on forced-mode optical ports after reload is expected. Best, Jijie