Re: [PATCH iwl-net 07/10] ice: take the switch rule AQ error from the response descriptor

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 <redacted>
---
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)