Re: [PATCH net-next 05/15] net/mlx5: LAG, decouple iteration end-of-walk from MLX5_MAX_PORTS

[Shay Drory <shayd@nvidia.com>]

On 10/11/2026 13:00, Paolo Abeni wrote:
> Hi,
>
> there are a couple of unanswered comments here...
>
> On 11/3/26 18:09, Tariq Toukan wrote:
> > @@ -2095,7 +2096,7 @@ static int mlx5_handle_changeupper_event(struct mlx5_lag *ldev,
> >   				break;
> >   			}
> >   		}
> > -		if (i < MLX5_MAX_PORTS) {
> > +		if (i < ldev->ports) {
>
> [medium]
> Should this compare against ldev->max_funcs, like the other converted call
> sites?
> The iterator's end-of-walk value is now ldev->max_funcs. That holds both in
> mlx5_get_next_lag_func() and in the mlx5_lag_for_each() condition.
> Indices are never compacted and max_funcs never shrinks. So a PORT-marked
> member can sit at an index in [ldev->ports, max_funcs) while the ldev still
> has ldev->ports members. One way to get there: more PFs than num_lag_ports
> join the ldev, and a lower-index one is removed later.
> In that case the inner loop finds the matching pf and breaks with
> i >= ldev->ports. idx has already been incremented, but this block is
> skipped, so the bond_status |= (1 << idx) and has_inactive updates are
> lost. The later check would then fail:
> 	is_in_lag = num_slaves == ldev->ports &&
> 		bond_status == GENMASK(ldev->ports - 1, 0);
> Wouldn't that leave this bond silently not offloaded?
> The old i < MLX5_MAX_PORTS check covered the whole index range that the old
> xa_alloc() limit allowed. This code is unchanged at the end of the series.
>
> This looks valid to me.

This is answered in the cover letter: mlx5 creates an ldev per sw_guid,
and PFs have a different sw_guid than VFs/SFs, so only PFs share this
ldev. The only case with more PFs than num_lag_ports in one ldev is SD,
and for SD the netdev notifier, and with it
mlx5_handle_changeupper_event(), is not registered. So a PORT-marked
member here always sits at an index below ldev->ports.

> >   			if (netif_is_bond_master(upper)) {
> >   				slave = bond_slave_get_rcu(ndev_tmp);
> >   				if (slave)
> > @@ -2360,13 +2361,16 @@ int mlx5_ldev_add_mdev(struct mlx5_lag *ldev,
> >   	if (!pf)
> >   		return -ENOMEM;
> >   
> > -	err = xa_alloc(&ldev->pfs, &idx, pf, XA_LIMIT(0, MLX5_MAX_PORTS - 1),
> > +	err = xa_alloc(&ldev->pfs, &idx, pf, XA_LIMIT(0, U8_MAX - 1),
> >   		       GFP_KERNEL);
>
> [high]
> Can this let member indices reach 8 or more while several arrays are still
> sized MLX5_MAX_PORTS?
>
> this is solved at the end of the series, so not really a problem.
>
> Note that there are more unanswered comments on patch 15/15.
>
> I think a v2 is need. For the future I suggest using smaller series:
> 15 non trivial patches will have increasingly hard time with AI
> reviews.

Ack, will prepare a v2.

> /P