[PATCH net] Revert "net/mlx5: E-Switch, preserve max tx speed on vport state modification"

[PATCH net] Revert "net/mlx5: E-Switch, preserve max tx speed on vport state modification"

[Tariq Toukan]

From: Or Har-Toov <ohartoov@nvidia.com>

This reverts commit ad0ae7aefa7a ("net/mlx5: E-Switch, preserve max tx
speed on vport state modification").

mlx5_modify_vport_admin_state() and mlx5_esw_adj_vport_modify() query
the vport's current max_tx_speed before modifying its state, and
write that value back, to avoid resetting it to 0 as a side effect
of an unrelated admin-state change.

That's unnecessary: max_tx_speed is optional in MODIFY_VPORT_STATE -
FW skips writing it whenever it's 0, treating that as "not provided"
rather than "reset to zero". Leaving it unset already preserves FW's
current value, with no query needed.

Fixes: ad0ae7aefa7a ("net/mlx5: E-Switch, preserve max tx speed on vport state modification")
Signed-off-by: Or Har-Toov <ohartoov@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/esw/adj_vport.c        | 20 ----------------
 .../net/ethernet/mellanox/mlx5/core/vport.c   | 24 -------------------
 2 files changed, 44 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/adj_vport.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/adj_vport.c
index 7950b82d8b8a..3624c680a861 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/adj_vport.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/adj_vport.c
@@ -11,26 +11,6 @@ int mlx5_esw_adj_vport_modify(struct mlx5_core_dev *dev, u16 vport,
 
 	lockdep_assert_held(&dev->priv.eswitch->state_lock);
 
-	if (MLX5_CAP_ESW(dev, esw_vport_state_max_tx_speed)) {
-		u8 op_mod = MLX5_VPORT_STATE_OP_MOD_ESW_VPORT;
-		struct mlx5_vport *esw_vport;
-		u32 speed = 0;
-		int err;
-
-		err = mlx5_query_vport_max_tx_speed(dev, op_mod, vport,
-						    true, &speed, NULL);
-		if (err) {
-			esw_vport = mlx5_eswitch_get_vport(dev->priv.eswitch,
-							   vport);
-			speed = IS_ERR(esw_vport) ? 0 :
-				esw_vport->agg_max_tx_speed;
-			mlx5_core_dbg(dev,
-				      "Failed to query vport %d max tx speed, err=%d, using cached %u\n",
-				      vport, err, speed);
-		}
-		MLX5_SET(modify_vport_state_in, in, max_tx_speed, speed);
-	}
-
 	MLX5_SET(modify_vport_state_in, in, opcode,
 		 MLX5_CMD_OP_MODIFY_VPORT_STATE);
 	MLX5_SET(modify_vport_state_in, in, op_mod,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/vport.c b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
index 5774ec117428..78f3d15b599d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/vport.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
@@ -93,30 +93,6 @@ int mlx5_modify_vport_admin_state(struct mlx5_core_dev *mdev, u8 opmod,
 	lockdep_assert_held(&mdev->priv.eswitch->state_lock);
 #endif
 
-	if (MLX5_CAP_ESW(mdev, esw_vport_state_max_tx_speed) &&
-	    opmod == MLX5_VPORT_STATE_OP_MOD_ESW_VPORT &&
-	    vport != MLX5_VPORT_UPLINK) {
-		u32 speed = 0;
-		int err;
-
-		err = mlx5_query_vport_max_tx_speed(mdev, opmod, vport,
-						    other_vport, &speed, NULL);
-		if (err) {
-#ifdef CONFIG_MLX5_ESWITCH
-			struct mlx5_vport *esw_vport;
-
-			esw_vport = mlx5_eswitch_get_vport(mdev->priv.eswitch,
-							   vport);
-			speed = IS_ERR(esw_vport) ? 0 :
-				esw_vport->agg_max_tx_speed;
-#endif
-			mlx5_core_dbg(mdev,
-				      "Failed to query vport %d max tx speed, err=%d, using cached %u\n",
-				      vport, err, speed);
-		}
-		MLX5_SET(modify_vport_state_in, in, max_tx_speed, speed);
-	}
-
 	MLX5_SET(modify_vport_state_in, in, opcode,
 		 MLX5_CMD_OP_MODIFY_VPORT_STATE);
 	MLX5_SET(modify_vport_state_in, in, op_mod, opmod);

base-commit: 6dc989ea46b96ce170840174b4a38c4a387fb005
-- 
2.44.0

Re: [PATCH net] Revert "net/mlx5: E-Switch, preserve max tx speed on vport state modification"

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The revert's correctness rests on a firmware claim that cannot
  be checked from the kernel tree, and that claim contradicts the commit…

--

Revert "net/mlx5: E-Switch, preserve max tx speed on vport state modification"

This reverts the query-and-write-back of max_tx_speed in
mlx5_modify_vport_admin_state() and mlx5_esw_adj_vport_modify(). The
commit message says FW treats a zero max_tx_speed in MODIFY_VPORT_STATE
as "not provided", so no query is needed to keep the current value.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/vport.c b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
> index 5774ec1174287..78f3d15b599d0 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/vport.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
> @@ -93,30 +93,6 @@ int mlx5_modify_vport_admin_state(struct mlx5_core_dev *mdev, u8 opmod,
>  	lockdep_assert_held(&mdev->priv.eswitch->state_lock);
>  #endif
>  
> -	if (MLX5_CAP_ESW(mdev, esw_vport_state_max_tx_speed) &&
> -	    opmod == MLX5_VPORT_STATE_OP_MOD_ESW_VPORT &&
> -	    vport != MLX5_VPORT_UPLINK) {
> -		u32 speed = 0;
> -		int err;
> -
> -		err = mlx5_query_vport_max_tx_speed(mdev, opmod, vport,
> -						    other_vport, &speed, NULL);

[ ... ]

> -		MLX5_SET(modify_vport_state_in, in, max_tx_speed, speed);
> -	}
> -
>  	MLX5_SET(modify_vport_state_in, in, opcode,
>  		 MLX5_CMD_OP_MODIFY_VPORT_STATE);
>  	MLX5_SET(modify_vport_state_in, in, op_mod, opmod);

[Severity: Medium]

The commit being reverted, ad0ae7aefa7a, describes the opposite firmware
behavior:

  "When modifying vport state, the firmware interprets a zero in the max tx
  speed field as an intentional reset, which can overwrite previously set
  values."

This revert says:

  "FW skips writing it whenever it's 0, treating that as "not provided"
  rather than "reset to zero"."

Which of these is correct, and does it hold for every FW version?

Could the commit message explain why the earlier description was wrong?
For example, it could cite the PRM or a FW version where this behavior
was confirmed.

The command layout has valid bits for the connect fields but none for
max_tx_speed:

include/linux/mlx5/mlx5_ifc.h:mlx5_ifc_modify_vport_state_in_bits {
    ...
	u8         max_tx_speed[0x10];
	u8         ingress_connect[0x1];
	u8         egress_connect[0x1];
	u8         ingress_connect_valid[0x1];
	u8         egress_connect_valid[0x1];
    ...
}

So after this patch, mlx5_modify_vport_admin_state() and
mlx5_esw_adj_vport_modify() always send max_tx_speed=0. Whether that
keeps the current speed is decided entirely by FW.

Suppose some FW that advertises esw_vport_state_max_tx_speed does reset
on zero. Would routine admin state changes then clear the LAG-aggregated
speed programmed by mlx5_lag_modify_device_vports_speed()? For example:

ip link set <rep> down
  mlx5e_rep_close()
    mlx5_modify_vport_admin_state(priv->mdev,
                                  MLX5_VPORT_STATE_OP_MOD_ESW_VPORT,
                                  rep->vport, 1,
                                  MLX5_VPORT_ADMIN_STATE_DOWN);

The same applies to mlx5e_rep_open(), mlx5e_rep_change_carrier() and
mlx5_eswitch_set_vport_state(). It also applies to adjacent vport connect
and disconnect, which mlx5_esw_fdb_active() and mlx5_esw_fdb_inactive()
do through mlx5_esw_adj_vport_modify().

In that case vport->agg_max_tx_speed would still hold the aggregated
value. Nothing would reapply it until the next LAG event or
mlx5_esw_vport_enable(). That is the hardware/software mismatch that
ad0ae7aefa7a set out to fix.

Also, this patch carries a Fixes: tag, but the commit message only calls
the reverted code "unnecessary" and names no defect. The reverted code
added one query and wrote back the value FW had just reported.

Is there a user-visible problem being fixed here? If not, would this fit
better as a cleanup without the Fixes: tag?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261004083531.216988-1-tariqt%40nvidia.com

Re: [PATCH net] Revert "net/mlx5: E-Switch, preserve max tx speed on vport state modification"

[patchwork-bot+netdevbpf]

Hello:

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

On Sun, 4 Oct 2026 11:35:31 +0300 you wrote:
> From: Or Har-Toov <ohartoov@nvidia.com>
> 
> This reverts commit ad0ae7aefa7a ("net/mlx5: E-Switch, preserve max tx
> speed on vport state modification").
> 
> mlx5_modify_vport_admin_state() and mlx5_esw_adj_vport_modify() query
> the vport's current max_tx_speed before modifying its state, and
> write that value back, to avoid resetting it to 0 as a side effect
> of an unrelated admin-state change.
> 
> [...]

Here is the summary with links:
  - [net] Revert "net/mlx5: E-Switch, preserve max tx speed on vport state modification"
    https://git.kernel.org/netdev/net/c/86b785b18f8f

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