On October 2, 2026 3:07:49 PM GMT+02:00, Petr Oros <[email protected]> wrote: >ice_aq_sw_rules() decides whether a rule removal failed with ENOENT by >reading hw->adminq.sq_last_status after ice_aq_send_cmd() returned. That >field is shared by every admin queue user and is only stable while the >send queue lock is held. Another AQ command completing in between can >overwrite it, so a failed removal is reported as a generic error, and a >successful one can even be turned into -ENOENT, because the check is >not limited to the failure case. A caller that sees -ENOENT keeps its >rule bookkeeping while the rule is gone from the hardware. > >ice_vsi_sync_fltr() has the same problem with ENOSPC. It checks >sq_last_status only after it has freed the list of filters it tried to >add. Every entry is a separate devres allocation, so freeing a list of >thousands of entries takes seconds, and by then the value usually >belongs to a different command, so the MAC filter overflow handling is >never entered. With debug prints of the error and sq_last_status added >to both places: > > ice_aq_sw_rules: opc 0x2a0 status -5 aq 16 > ice 0000:04:00.0 enp4s0f0np0: Failed to add MAC filters err -5 aq 0 > >Use the retval of the descriptor that ice_aq_send_cmd() copies back, >only when the command failed, translate ENOSPC on add to -ENOSPC and >let ice_vsi_sync_fltr() check the return code instead of >sq_last_status. > >i40e fixed the same kind of race in commit 53a9e346e159 ("i40e: Fix race >condition while adding/deleting MAC/VLAN filters"). > >Fixes: ca1fdb885e5f ("ice: return correct error code from ice_aq_sw_rules") >Assisted-by: LLM >Signed-off-by: Petr Oros <[email protected]> >--- > drivers/net/ethernet/intel/ice/ice_main.c | 4 +--- > drivers/net/ethernet/intel/ice/ice_switch.c | 13 ++++++++++--- > 2 files changed, 11 insertions(+), 6 deletions(-) > >diff --git a/drivers/net/ethernet/intel/ice/ice_main.c >b/drivers/net/ethernet/intel/ice/ice_main.c >index 8a21f87eb6ca21..ceb9fec2af21e7 100644 >--- a/drivers/net/ethernet/intel/ice/ice_main.c >+++ b/drivers/net/ethernet/intel/ice/ice_main.c >@@ -396,8 +396,6 @@ static int ice_vsi_sync_fltr(struct ice_vsi *vsi) > struct device *dev = ice_pf_to_dev(vsi->back); > struct net_device *netdev = vsi->netdev; > bool promisc_forced_on = false; >- struct ice_pf *pf = vsi->back; >- struct ice_hw *hw = &pf->hw; > u32 changed_flags = 0; > int err; > >@@ -450,7 +448,7 @@ static int ice_vsi_sync_fltr(struct ice_vsi *vsi) > * should go into promiscuous mode. There should be some > * space reserved for promiscuous filters. > */ >- if (hw->adminq.sq_last_status == LIBIE_AQ_RC_ENOSPC && >+ if (err == -ENOSPC && > !test_and_set_bit(ICE_FLTR_OVERFLOW_PROMISC, > vsi->state)) { > promisc_forced_on = true; >diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c >b/drivers/net/ethernet/intel/ice/ice_switch.c >index 239d4d9633baa6..ae96a2003d5c5e 100644 >--- a/drivers/net/ethernet/intel/ice/ice_switch.c >+++ b/drivers/net/ethernet/intel/ice/ice_switch.c >@@ -1959,9 +1959,16 @@ ice_aq_sw_rules(struct ice_hw *hw, void *rule_list, u16 >rule_list_sz, > desc.flags |= cpu_to_le16(LIBIE_AQ_FLAG_RD); > cmd->num_rules_fltr_entry_index = cpu_to_le16(num_rules); > status = ice_aq_send_cmd(hw, &desc, rule_list, rule_list_sz, cd); >- if (opc != ice_aqc_opc_add_sw_rules && >- hw->adminq.sq_last_status == LIBIE_AQ_RC_ENOENT) >- status = -ENOENT; >+ if (status) { >+ enum libie_aq_err aq_err = le16_to_cpu(desc.retval) & 0xff; >+ >+ if (opc != ice_aqc_opc_add_sw_rules && >+ aq_err == LIBIE_AQ_RC_ENOENT) >+ status = -ENOENT; >+ else if (opc == ice_aqc_opc_add_sw_rules && >+ aq_err == LIBIE_AQ_RC_ENOSPC) >+ status = -ENOSPC; >+ } > > if (!status) { > if (opc == ice_aqc_opc_add_sw_rules)
Reviewed-by: Ivan Vecera <[email protected]>
