Re: [PATCH net V2 1/4] {RDMA,net}/mlx5: cache and use TX-enabled aggregate speed for vports

[Or Har-Toov <ohartoov@nvidia.com>]

On 23/10/2026 22:32, sashiko-bot@kernel.org wrote:
> 
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] net/mlx5: lag: missing agg_speed_mbps reset on MPESW and mode-change teardown
> --
> 
> commit ce16ef57f040ab430add649db26c3f383cbab840
> Author: Or Har-Toov <ohartoov@nvidia.com>
> 
> {RDMA,net}/mlx5: cache and use TX-enabled aggregate speed for vports
> 
> This commit caches the calculated operational TX-enabled aggregate speed in
> struct mlx5_lag and exposes mlx5_lag_query_aggregated_speed() to query it
> instead of the bond speed. It also resets the cached speed to SPEED_UNKNOWN
> via mlx5_lag_reset_agg_speed() when resetting vports speed.
> 
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
>> index 4e173b08cb37a..57755ceab8a75 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> 
> [ ... ]
> 
>> @@ -1594,6 +1625,7 @@ void mlx5_lag_reset_vports_speed(struct mlx5_lag *ldev)
>>        int pf_idx;
>>        int ret;
>>
>> +     mlx5_lag_reset_agg_speed(ldev);
>>        mlx5_ldev_for_each(pf_idx, 0, ldev) {
>>                pf = mlx5_lag_pf(ldev, pf_idx);
>>                if (!pf)
> 
> [Severity: Medium]
> Does calling mlx5_lag_reset_agg_speed() only from
> mlx5_lag_reset_vports_speed() miss resetting ldev->agg_speed_mbps (and vport
> speeds) when LAG is torn down via MPESW disable or mode changes?
> 
> Currently, mlx5_lag_reset_vports_speed() is only called from mlx5_do_bond()
> when mlx5_lag_should_disable_lag() returns true:
> 
> drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c:mlx5_do_bond() {
>          ...
>          } else if (mlx5_lag_should_disable_lag(ldev, do_bond)) {
>                  mlx5_lag_reset_vports_speed(ldev);
>                  mlx5_disable_lag(ldev);
>          }
> }
> 
> where mlx5_lag_should_disable_lag() checks:
> 
>          return !do_bond && __mlx5_lag_is_active(ldev) &&
>                 ldev->mode != MLX5_LAG_MODE_MPESW;
> 
> When MPESW LAG is enabled via mlx5_lag_enable_mpesw() ->
> mlx5_lag_shared_fdb_create(), mlx5_lag_set_vports_agg_speed() populates
> ldev->agg_speed_mbps (and mlx5_mpesw_speed_update_work() updates it).
> However, when MPESW is torn down via mlx5_lag_disable_mpesw():
> 
> drivers/net/ethernet/mellanox/mlx5/core/lag/mpesw.c:mlx5_lag_disable_mpesw() {
>          if (ldev->mode != MLX5_LAG_MODE_MPESW)
>                  return;
> 
>          mlx5_mpesw_metadata_cleanup(ldev);
>          mlx5_lag_shared_fdb_destroy(ldev, MLX5_LAG_FILTER_ALL);
>          if (mlx5_lag_has_sd_group(ldev))
>                  mlx5_mpesw_restore_sd_fdb(ldev);
> }
> 
> neither mlx5_lag_reset_vports_speed() nor mlx5_lag_reset_agg_speed() is
> called, and mlx5_lag_should_disable_lag() explicitly excludes
> MLX5_LAG_MODE_MPESW.
> 
> Similarly, during eswitch mode or SR-IOV changes, mlx5_lag_disable_change()
> disables an active LAG directly without calling
> mlx5_lag_reset_vports_speed() or mlx5_lag_reset_agg_speed():
> 
> drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c:mlx5_lag_disable_change() {
>          ...
>          ldev->mode_changes_in_progress++;
>          if (__mlx5_lag_is_active(ldev)) {
>                  if (ldev->mode == MLX5_LAG_MODE_MPESW)
>                          mlx5_lag_disable_mpesw(ldev);
>                  else
>                          mlx5_disable_lag(ldev);
>          }
> }
> 
> Because mlx5_disable_lag() -> mlx5_deactivate_lag() clears ldev->mode to
> MLX5_LAG_MODE_NONE, when mlx5_lag_enable_change() later runs mlx5_do_bond(),
> __mlx5_lag_is_active(ldev) is already false. If do_bond is false (for
> example, if mlx5_lag_check_prereq(ldev) is now false after a mode change or
> if the bond was removed during the transition), mlx5_lag_should_disable_lag()
> evaluates to false and the reset never runs.
> 
> Wouldn't this leave a stale speed in ldev->agg_speed_mbps (and stale vport
> rate limits) after teardown, causing mlx5_lag_query_aggregated_speed() to
> return the old speed instead of -ENODATA, and in the subsequent patch
> ("{RDMA,net}/mlx5: notify IB uplink on LAG aggregate speed change") causing
> mlx5_lag_update_agg_speed() to compare against the stale old_speed and
> suppress IB_EVENT_DEVICE_SPEED_CHANGE if LAG is re-activated at the same
> aggregate speed?
> 
> [ ... ]
> 

A fix for the MPESW was sent in 
https://lore.kernel.org/all/16368a7c-af34-4dfd-9e8c-891d455d2935@nvidia.com/
I will send a V3 for this series and add also the MPESW fix
>>
>>
>>
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261022193053.3815866-1-tariqt@nvidia.com?part=1