diff mbox series

[2/3] net/sfc: revise FW RSRC free error logs in transfer rules

Message ID 20210420211006.19170-2-ivan.malov@oktetlabs.ru (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [1/3] net/sfc: fix outer rule and encap. header rollback on errors |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ivan Malov April 20, 2021, 9:10 p.m. UTC 
  The current code simply forwards FW resource free failure
to the application leaving the operation incomplete. This
stalls the application and makes debugging very difficult.

Make the driver proceed with handling FW resource free in
the case of errors. Add explicit error logging statements.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
 drivers/net/sfc/sfc_mae.c | 109 ++++++++++++++++++++++----------------
 1 file changed, 63 insertions(+), 46 deletions(-)
diff mbox series

Patch

diff --git a/drivers/net/sfc/sfc_mae.c b/drivers/net/sfc/sfc_mae.c
index 0270c91e2..445368868 100644
--- a/drivers/net/sfc/sfc_mae.c
+++ b/drivers/net/sfc/sfc_mae.c
@@ -199,8 +199,11 @@  sfc_mae_outer_rule_del(struct sfc_adapter *sa,
 	if (rule->refcnt != 0)
 		return;
 
-	SFC_ASSERT(rule->fw_rsrc.rule_id.id == EFX_MAE_RSRC_ID_INVALID);
-	SFC_ASSERT(rule->fw_rsrc.refcnt == 0);
+	if (rule->fw_rsrc.rule_id.id != EFX_MAE_RSRC_ID_INVALID ||
+	    rule->fw_rsrc.refcnt != 0) {
+		sfc_err(sa, "deleting outer_rule=%p abandons its FW resource: OR_ID=0x%08x, refcnt=%u",
+			rule, rule->fw_rsrc.rule_id.id, rule->fw_rsrc.refcnt);
+	}
 
 	efx_mae_match_spec_fini(sa->nic, rule->match_spec);
 
@@ -245,7 +248,7 @@  sfc_mae_outer_rule_enable(struct sfc_adapter *sa,
 	return 0;
 }
 
-static int
+static void
 sfc_mae_outer_rule_disable(struct sfc_adapter *sa,
 			   struct sfc_mae_outer_rule *rule)
 {
@@ -253,20 +256,24 @@  sfc_mae_outer_rule_disable(struct sfc_adapter *sa,
 	int rc;
 
 	SFC_ASSERT(sfc_adapter_is_locked(sa));
-	SFC_ASSERT(fw_rsrc->rule_id.id != EFX_MAE_RSRC_ID_INVALID);
-	SFC_ASSERT(fw_rsrc->refcnt != 0);
+
+	if (fw_rsrc->rule_id.id == EFX_MAE_RSRC_ID_INVALID ||
+	    fw_rsrc->refcnt == 0) {
+		sfc_err(sa, "failed to disable outer_rule=%p: already disabled; OR_ID=0x%08x, refcnt=%u",
+			rule, fw_rsrc->rule_id.id, fw_rsrc->refcnt);
+		return;
+	}
 
 	if (fw_rsrc->refcnt == 1) {
 		rc = efx_mae_outer_rule_remove(sa->nic, &fw_rsrc->rule_id);
-		if (rc != 0)
-			return rc;
-
+		if (rc != 0) {
+			sfc_err(sa, "failed to disable outer_rule=%p with OR_ID=0x%08x: %s",
+				rule, fw_rsrc->rule_id.id, strerror(rc));
+		}
 		fw_rsrc->rule_id.id = EFX_MAE_RSRC_ID_INVALID;
 	}
 
 	--(fw_rsrc->refcnt);
-
-	return 0;
 }
 
 static struct sfc_mae_encap_header *
@@ -344,8 +351,12 @@  sfc_mae_encap_header_del(struct sfc_adapter *sa,
 	if (encap_header->refcnt != 0)
 		return;
 
-	SFC_ASSERT(encap_header->fw_rsrc.eh_id.id == EFX_MAE_RSRC_ID_INVALID);
-	SFC_ASSERT(encap_header->fw_rsrc.refcnt == 0);
+	if (encap_header->fw_rsrc.eh_id.id != EFX_MAE_RSRC_ID_INVALID ||
+	    encap_header->fw_rsrc.refcnt != 0) {
+		sfc_err(sa, "deleting encap_header=%p abandons its FW resource: EH_ID=0x%08x, refcnt=%u",
+			encap_header, encap_header->fw_rsrc.eh_id.id,
+			encap_header->fw_rsrc.refcnt);
+	}
 
 	TAILQ_REMOVE(&mae->encap_headers, encap_header, entries);
 	rte_free(encap_header->buf);
@@ -396,7 +407,7 @@  sfc_mae_encap_header_enable(struct sfc_adapter *sa,
 	return 0;
 }
 
-static int
+static void
 sfc_mae_encap_header_disable(struct sfc_adapter *sa,
 			     struct sfc_mae_encap_header *encap_header)
 {
@@ -404,26 +415,29 @@  sfc_mae_encap_header_disable(struct sfc_adapter *sa,
 	int rc;
 
 	if (encap_header == NULL)
-		return 0;
+		return;
 
 	SFC_ASSERT(sfc_adapter_is_locked(sa));
 
 	fw_rsrc = &encap_header->fw_rsrc;
 
-	SFC_ASSERT(fw_rsrc->eh_id.id != EFX_MAE_RSRC_ID_INVALID);
-	SFC_ASSERT(fw_rsrc->refcnt != 0);
+	if (fw_rsrc->eh_id.id == EFX_MAE_RSRC_ID_INVALID ||
+	    fw_rsrc->refcnt == 0) {
+		sfc_err(sa, "failed to disable encap_header=%p: already disabled; EH_ID=0x%08x, refcnt=%u",
+			encap_header, fw_rsrc->eh_id.id, fw_rsrc->refcnt);
+		return;
+	}
 
 	if (fw_rsrc->refcnt == 1) {
 		rc = efx_mae_encap_header_free(sa->nic, &fw_rsrc->eh_id);
-		if (rc != 0)
-			return rc;
-
+		if (rc != 0) {
+			sfc_err(sa, "failed to disable encap_header=%p with EH_ID=0x%08x: %s",
+				encap_header, fw_rsrc->eh_id.id, strerror(rc));
+		}
 		fw_rsrc->eh_id.id = EFX_MAE_RSRC_ID_INVALID;
 	}
 
 	--(fw_rsrc->refcnt);
-
-	return 0;
 }
 
 static struct sfc_mae_action_set *
@@ -489,8 +503,12 @@  sfc_mae_action_set_del(struct sfc_adapter *sa,
 	if (action_set->refcnt != 0)
 		return;
 
-	SFC_ASSERT(action_set->fw_rsrc.aset_id.id == EFX_MAE_RSRC_ID_INVALID);
-	SFC_ASSERT(action_set->fw_rsrc.refcnt == 0);
+	if (action_set->fw_rsrc.aset_id.id != EFX_MAE_RSRC_ID_INVALID ||
+	    action_set->fw_rsrc.refcnt != 0) {
+		sfc_err(sa, "deleting action_set=%p abandons its FW resource: AS_ID=0x%08x, refcnt=%u",
+			action_set, action_set->fw_rsrc.aset_id.id,
+			action_set->fw_rsrc.refcnt);
+	}
 
 	efx_mae_action_set_spec_fini(sa->nic, action_set->spec);
 	sfc_mae_encap_header_del(sa, action_set->encap_header);
@@ -520,7 +538,7 @@  sfc_mae_action_set_enable(struct sfc_adapter *sa,
 		rc = efx_mae_action_set_alloc(sa->nic, action_set->spec,
 					      &fw_rsrc->aset_id);
 		if (rc != 0) {
-			(void)sfc_mae_encap_header_disable(sa, encap_header);
+			sfc_mae_encap_header_disable(sa, encap_header);
 
 			return rc;
 		}
@@ -531,7 +549,7 @@  sfc_mae_action_set_enable(struct sfc_adapter *sa,
 	return 0;
 }
 
-static int
+static void
 sfc_mae_action_set_disable(struct sfc_adapter *sa,
 			   struct sfc_mae_action_set *action_set)
 {
@@ -539,24 +557,26 @@  sfc_mae_action_set_disable(struct sfc_adapter *sa,
 	int rc;
 
 	SFC_ASSERT(sfc_adapter_is_locked(sa));
-	SFC_ASSERT(fw_rsrc->aset_id.id != EFX_MAE_RSRC_ID_INVALID);
-	SFC_ASSERT(fw_rsrc->refcnt != 0);
+
+	if (fw_rsrc->aset_id.id == EFX_MAE_RSRC_ID_INVALID ||
+	    fw_rsrc->refcnt == 0) {
+		sfc_err(sa, "failed to disable action_set=%p: already disabled; AS_ID=0x%08x, refcnt=%u",
+			action_set, fw_rsrc->aset_id.id, fw_rsrc->refcnt);
+		return;
+	}
 
 	if (fw_rsrc->refcnt == 1) {
 		rc = efx_mae_action_set_free(sa->nic, &fw_rsrc->aset_id);
-		if (rc != 0)
-			return rc;
-
+		if (rc != 0) {
+			sfc_err(sa, "failed to disable action_set=%p with AS_ID=0x%08x: %s",
+				action_set, fw_rsrc->aset_id.id, strerror(rc));
+		}
 		fw_rsrc->aset_id.id = EFX_MAE_RSRC_ID_INVALID;
 
-		rc = sfc_mae_encap_header_disable(sa, action_set->encap_header);
-		if (rc != 0)
-			return rc;
+		sfc_mae_encap_header_disable(sa, action_set->encap_header);
 	}
 
 	--(fw_rsrc->refcnt);
-
-	return 0;
 }
 
 void
@@ -2850,11 +2870,11 @@  sfc_mae_flow_insert(struct sfc_adapter *sa,
 	return 0;
 
 fail_action_rule_insert:
-	(void)sfc_mae_action_set_disable(sa, action_set);
+	sfc_mae_action_set_disable(sa, action_set);
 
 fail_action_set_enable:
 	if (outer_rule != NULL)
-		(void)sfc_mae_outer_rule_disable(sa, outer_rule);
+		sfc_mae_outer_rule_disable(sa, outer_rule);
 
 fail_outer_rule_enable:
 	return rc;
@@ -2874,19 +2894,16 @@  sfc_mae_flow_remove(struct sfc_adapter *sa,
 	SFC_ASSERT(action_set != NULL);
 
 	rc = efx_mae_action_rule_remove(sa->nic, &spec_mae->rule_id);
-	if (rc != 0)
-		return rc;
-
-	spec_mae->rule_id.id = EFX_MAE_RSRC_ID_INVALID;
-
-	rc = sfc_mae_action_set_disable(sa, action_set);
 	if (rc != 0) {
-		sfc_err(sa, "failed to disable the action set (rc = %d)", rc);
-		/* Despite the error, proceed with outer rule removal. */
+		sfc_err(sa, "failed to disable flow=%p with AR_ID=0x%08x: %s",
+			flow, spec_mae->rule_id.id, strerror(rc));
 	}
+	spec_mae->rule_id.id = EFX_MAE_RSRC_ID_INVALID;
+
+	sfc_mae_action_set_disable(sa, action_set);
 
 	if (outer_rule != NULL)
-		return sfc_mae_outer_rule_disable(sa, outer_rule);
+		sfc_mae_outer_rule_disable(sa, outer_rule);
 
 	return 0;
 }