Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] ice: num_vsis inflated and stale entry left on VSI move failure --
commit a9d16d503219ec18b5a73776a1a7f4b0b851cd1b Author: Jacob Keller <[email protected]> ice: count number of VSIS in agg_vsi_list This patch adds a member num_vsis to the ice_sched_agg_info structure to track the number of VSIs associated with an aggregator node. The counter is incremented when adding a VSI to the list and decremented when removing it, avoiding the need to iterate over the agg_vsi_list. > diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c > b/drivers/net/ethernet/intel/ice/ice_sched.c > index bcfc9db69e17e..08d0f685f6c45 100644 > --- a/drivers/net/ethernet/intel/ice/ice_sched.c > +++ b/drivers/net/ethernet/intel/ice/ice_sched.c [ ... ] > @@ -2915,24 +2920,26 @@ ice_sched_assoc_vsi_to_agg(struct ice_port_info *pi, > u32 agg_id, > /* add VSI ID into the aggregator list */ > agg_vsi_info->vsi_handle = vsi_handle; > list_add(&agg_vsi_info->list_entry, &agg_info->agg_vsi_list); > + agg_info->num_vsis++; > } > /* Move VSI node to new aggregator node for requested TC(s) */ > ice_for_each_traffic_class(tc) { > if (!ice_is_tc_ena(*tc_bitmap, tc)) > continue; > > /* Move VSI to new aggregator */ > status = ice_sched_move_vsi_to_agg(pi, vsi_handle, agg_id, tc); > if (status) > break; [Severity: Medium] If ice_sched_move_vsi_to_agg() fails and returns an error here, the loop will break. Should the function roll back the agg_info->num_vsis increment and remove the newly added agg_vsi_info from agg_info->agg_vsi_list before returning? Without a rollback, a stale VSI entry with an empty tc_bitmap remains in the aggregator list, permanently inflating the num_vsis counter. If subsequent code relies on num_vsis to enforce limits like ICE_MAX_VSIS_IN_AGG_NODE, this could cause the aggregator to prematurely reject new VSIs. > > set_bit(tc, agg_vsi_info->tc_bitmap); > if (old_agg_vsi_info) > clear_bit(tc, old_agg_vsi_info->tc_bitmap); > } > if (old_agg_vsi_info && !old_agg_vsi_info->tc_bitmap[0]) { > list_del(&old_agg_vsi_info->list_entry); > devm_kfree(ice_hw_to_dev(pi->hw), old_agg_vsi_info); > + old_agg_info->num_vsis--; > } > return status; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20261009-jk-cleanup-vsi-aggregator-nodes-v2-0-313964c25...@intel.com?part=2
