From patchwork Tue Apr 20 21:10:05 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ivan Malov X-Patchwork-Id: 91884 X-Patchwork-Delegate: ferruh.yigit@amd.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4000FA0524; Tue, 20 Apr 2021 23:10:10 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1314541735; Tue, 20 Apr 2021 23:10:10 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 5A20941439 for ; Tue, 20 Apr 2021 23:10:09 +0200 (CEST) Received: from localhost.localdomain (unknown [188.242.7.54]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id EFD027F5F1; Wed, 21 Apr 2021 00:10:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru EFD027F5F1 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1618953009; bh=aQ37LFzmwXLWHWZQIBAyB2GsHVbQDXHUlo2eDBZOVbo=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=W3HjvfoJxrQUvDVdVZkNkXVNfVsnss2X+TDbNohkglZhB6j9lHaSBX6BDu+BF9Xwu 7Lw7mT95hvzCJ2kUsNGd2Dw7jHgDvfmmYzJ98KhS6HbO7SjucwE2i7K7NFWueVzxpi Y0IqlTzy4VdPm42+zJnJGn3fuIU80oF7Oo6kkEwA= From: Ivan Malov To: dev@dpdk.org Cc: Andrew Rybchenko , Andy Moreton Date: Wed, 21 Apr 2021 00:10:05 +0300 Message-Id: <20210420211006.19170-2-ivan.malov@oktetlabs.ru> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210420211006.19170-1-ivan.malov@oktetlabs.ru> References: <20210420211006.19170-1-ivan.malov@oktetlabs.ru> MIME-Version: 1.0 Subject: [dpdk-dev] [PATCH 2/3] net/sfc: revise FW RSRC free error logs in transfer rules X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 Reviewed-by: Andrew Rybchenko Reviewed-by: Andy Moreton --- drivers/net/sfc/sfc_mae.c | 109 ++++++++++++++++++++++---------------- 1 file changed, 63 insertions(+), 46 deletions(-) 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; }