[PATCH v2 net] bonding: only set speed/duplex to unknown, if getting speed failed

[PATCH v2 net] bonding: only set speed/duplex to unknown, if getting speed failed

[Thomas Bogendoerfer]

bond_update_speed_duplex() first set speed/duplex to unknown and
then asks slave driver for current speed/duplex. Since getting
speed/duplex might take longer there is a race, where this false state
is visible by /proc/net/bonding. With commit 691b2bf14946 ("bonding:
 update port speed when getting bond speed") this race gets more visible,
if user space is calling ethtool on a regular base.

Fix this by only setting speed/duplex to unknown, if link speed is
really unknown/unusable.

Fixes: 98f41f694f46 ("bonding:update speed/duplex for NETDEV_CHANGE")
Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
v2: corrected fixes tag
v1: https://lore.kernel.org/all/20260130111904.144024-1-tbogendoerfer@suse.de/

 drivers/net/bonding/bond_main.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e7caf400a59c..4cdf89b21ca0 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -791,26 +791,29 @@ static int bond_update_speed_duplex(struct slave *slave)
 	struct ethtool_link_ksettings ecmd;
 	int res;
 
-	slave->speed = SPEED_UNKNOWN;
-	slave->duplex = DUPLEX_UNKNOWN;
-
 	res = __ethtool_get_link_ksettings(slave_dev, &ecmd);
 	if (res < 0)
-		return 1;
+		goto speed_duplex_unknown;
 	if (ecmd.base.speed == 0 || ecmd.base.speed == ((__u32)-1))
-		return 1;
+		goto speed_duplex_unknown;
 	switch (ecmd.base.duplex) {
 	case DUPLEX_FULL:
 	case DUPLEX_HALF:
 		break;
 	default:
-		return 1;
+		goto speed_duplex_unknown;
 	}
 
 	slave->speed = ecmd.base.speed;
 	slave->duplex = ecmd.base.duplex;
 
 	return 0;
+
+speed_duplex_unknown:
+	slave->speed = SPEED_UNKNOWN;
+	slave->duplex = DUPLEX_UNKNOWN;
+
+	return 1;
 }
 
 const char *bond_slave_link_status(s8 link)
-- 
2.43.0

Re: [PATCH v2 net] bonding: only set speed/duplex to unknown, if getting speed failed

[Jay Vosburgh]

Thomas Bogendoerfer <tbogendoerfer@suse.de> wrote:

>bond_update_speed_duplex() first set speed/duplex to unknown and
>then asks slave driver for current speed/duplex. Since getting
>speed/duplex might take longer there is a race, where this false state
>is visible by /proc/net/bonding. With commit 691b2bf14946 ("bonding:
> update port speed when getting bond speed") this race gets more visible,
>if user space is calling ethtool on a regular base.
>
>Fix this by only setting speed/duplex to unknown, if link speed is
>really unknown/unusable.
>
>Fixes: 98f41f694f46 ("bonding:update speed/duplex for NETDEV_CHANGE")
>Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>

Acked-by: Jay Vosburgh <jv@jvosburgh.net>

	I think this Fixes tag is fine, it's far enough back in time to
be effectively all stable releases.  The actual change in the patch is
obviously correct, as well.

	-J

>---
>v2: corrected fixes tag
>v1: https://lore.kernel.org/all/20260130111904.144024-1-tbogendoerfer@suse.de/
>
> drivers/net/bonding/bond_main.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index e7caf400a59c..4cdf89b21ca0 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -791,26 +791,29 @@ static int bond_update_speed_duplex(struct slave *slave)
> 	struct ethtool_link_ksettings ecmd;
> 	int res;
> 
>-	slave->speed = SPEED_UNKNOWN;
>-	slave->duplex = DUPLEX_UNKNOWN;
>-
> 	res = __ethtool_get_link_ksettings(slave_dev, &ecmd);
> 	if (res < 0)
>-		return 1;
>+		goto speed_duplex_unknown;
> 	if (ecmd.base.speed == 0 || ecmd.base.speed == ((__u32)-1))
>-		return 1;
>+		goto speed_duplex_unknown;
> 	switch (ecmd.base.duplex) {
> 	case DUPLEX_FULL:
> 	case DUPLEX_HALF:
> 		break;
> 	default:
>-		return 1;
>+		goto speed_duplex_unknown;
> 	}
> 
> 	slave->speed = ecmd.base.speed;
> 	slave->duplex = ecmd.base.duplex;
> 
> 	return 0;
>+
>+speed_duplex_unknown:
>+	slave->speed = SPEED_UNKNOWN;
>+	slave->duplex = DUPLEX_UNKNOWN;
>+
>+	return 1;
> }
> 
> const char *bond_slave_link_status(s8 link)
>-- 
>2.43.0
>

---
	-Jay Vosburgh, jv@jvosburgh.net

Re: [PATCH v2 net] bonding: only set speed/duplex to unknown, if getting speed failed

[Nikolay Aleksandrov]

On 03/02/2026 16:11, Thomas Bogendoerfer wrote:
> bond_update_speed_duplex() first set speed/duplex to unknown and
> then asks slave driver for current speed/duplex. Since getting
> speed/duplex might take longer there is a race, where this false state
> is visible by /proc/net/bonding. With commit 691b2bf14946 ("bonding:
>   update port speed when getting bond speed") this race gets more visible,
> if user space is calling ethtool on a regular base.
> 
> Fix this by only setting speed/duplex to unknown, if link speed is
> really unknown/unusable.
> 
> Fixes: 98f41f694f46 ("bonding:update speed/duplex for NETDEV_CHANGE")
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
> v2: corrected fixes tag
> v1: https://lore.kernel.org/all/20260130111904.144024-1-tbogendoerfer@suse.de/
> 
>   drivers/net/bonding/bond_main.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
> 

Please CC all people who were involved in the previous patch version review.
Looks good to me,
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH v2 net] bonding: only set speed/duplex to unknown, if getting speed failed

[Hangbin Liu]

On Tue, Feb 03, 2026 at 03:11:52PM +0100, Thomas Bogendoerfer wrote:
> bond_update_speed_duplex() first set speed/duplex to unknown and
> then asks slave driver for current speed/duplex. Since getting
> speed/duplex might take longer there is a race, where this false state
> is visible by /proc/net/bonding. With commit 691b2bf14946 ("bonding:
>  update port speed when getting bond speed") this race gets more visible,
> if user space is calling ethtool on a regular base.
> 
> Fix this by only setting speed/duplex to unknown, if link speed is
> really unknown/unusable.
> 
> Fixes: 98f41f694f46 ("bonding:update speed/duplex for NETDEV_CHANGE")
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>

Reviewed-by: Hangbin Liu <liuhangbin@gmail.com>

Re: [PATCH v2 net] bonding: only set speed/duplex to unknown, if getting speed failed

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  3 Feb 2026 15:11:52 +0100 you wrote:
> bond_update_speed_duplex() first set speed/duplex to unknown and
> then asks slave driver for current speed/duplex. Since getting
> speed/duplex might take longer there is a race, where this false state
> is visible by /proc/net/bonding. With commit 691b2bf14946 ("bonding:
>  update port speed when getting bond speed") this race gets more visible,
> if user space is calling ethtool on a regular base.
> 
> [...]

Here is the summary with links:
  - [v2,net] bonding: only set speed/duplex to unknown, if getting speed failed
    https://git.kernel.org/netdev/net/c/48dec8d88af9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html