This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ice: fix stats array overflow via proper realloc
This patch removes ice_vsi_alloc_stat_arrays() and makes
ice_vsi_realloc_stat_arrays() handle the initial (no vsi_stat) case, tracking
the array sizes in the new tx_ring_stats_len / rx_ring_stats_len fields. The
single remaining call site is in ice_vsi_cfg_def(), and the call in
ice_vsi_rebuild() is dropped.
Closes: https://redhat.atlassian.net/browse/RHEL-164321
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c
> b/drivers/net/ethernet/intel/ice/ice_lib.c
> index e48ee5940f17..420e72df7f5e 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
[ ... ]
> @@ -2379,7 +2353,7 @@ static int ice_vsi_cfg_def(struct ice_vsi *vsi)
> return ret;
>
> /* allocate memory for Tx/Rx ring stat pointers */
> - ret = ice_vsi_alloc_stat_arrays(vsi);
> + ret = ice_vsi_realloc_stat_arrays(vsi);
> if (ret)
> goto unroll_vsi_alloc;
>
> @@ -3097,10 +3071,6 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags)
>
> mutex_lock(&vsi->xdp_state_lock);
>
> - ret = ice_vsi_realloc_stat_arrays(vsi);
> - if (ret)
> - goto unlock;
> -
> ice_vsi_decfg(vsi);
> ret = ice_vsi_cfg_def(vsi);
> if (ret)
[Severity: Low]
Does moving the ice_vsi_realloc_stat_arrays() call change what happens when
the stat array allocation fails during a rebuild?
Before this patch, ice_vsi_rebuild() did the realloc before
ice_vsi_decfg(), so a krealloc_array() failure returned -ENOMEM with the VSI
still fully configured:
ret = ice_vsi_realloc_stat_arrays(vsi);
if (ret)
goto unlock;
ice_vsi_decfg(vsi);
Now the only call site is inside ice_vsi_cfg_def(), which runs after
ice_vsi_decfg(), so an -ENOMEM there unwinds via unroll_vsi_alloc and is
propagated out of ice_vsi_rebuild() with the queues, vectors and rings
already released.
For the ethtool -L path that leaves ice_vsi_recfg_qs() in ice_main.c with
only this recovery:
rebuild_err:
dev_err(ice_pf_to_dev(pf), "Error during VSI rebuild: %d.
Unload and reload the driver.\n",
err);
The same outcome was already reachable pre-patch through the other
allocations in ice_vsi_cfg_def() (ice_vsi_alloc_def(),
ice_vsi_alloc_q_vectors(), ice_vsi_alloc_rings(),
ice_vsi_alloc_ring_stats()), so this adds one more allocation to an
already-destructive region rather than a new failure mode.
The commit message describes the move only as:
Also drop one callsite to avoid some duplication.
Could it also mention that the previously non-destructive failure path for
this allocation is gone?
--
pw-bot: cr
