net/bnxt: improve error recovery information messages

Message ID 20210924051753.30822-1-kalesh-anakkur.purayil@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series net/bnxt: improve error recovery information messages |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional fail Functional Testing issues
ci/iol-x86_64-compile-testing warning Testing issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed

Commit Message

Kalesh A P Sept. 24, 2021, 5:17 a.m. UTC
  From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

The error recovery async event messages are often mistaken
for errors. Improved the wording to clarify the meaning of
these events.
Also, take the first step towards more inclusive language.
The references to master will be changed to primary.
For example: "bnxt_is_master_func" will be renamed to
"bnxt_is_primary_func()".

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        |  6 +++---
 drivers/net/bnxt/bnxt_cpr.c    | 33 ++++++++++++++++++---------------
 drivers/net/bnxt/bnxt_cpr.h    |  2 +-
 drivers/net/bnxt/bnxt_ethdev.c | 16 ++++++++--------
 drivers/net/bnxt/bnxt_hwrm.c   |  4 ++--
 5 files changed, 32 insertions(+), 29 deletions(-)
  

Comments

Ajit Khaparde Sept. 27, 2021, 4:37 a.m. UTC | #1
On Thu, Sep 23, 2021 at 9:57 PM Kalesh A P
<kalesh-anakkur.purayil@broadcom.com> wrote:
>
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>
> The error recovery async event messages are often mistaken
> for errors. Improved the wording to clarify the meaning of
> these events.
> Also, take the first step towards more inclusive language.
> The references to master will be changed to primary.
> For example: "bnxt_is_master_func" will be renamed to
> "bnxt_is_primary_func()".
>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Patch applied to dpdk-next-net-brcm. Thanks

> ---
>  drivers/net/bnxt/bnxt.h        |  6 +++---
>  drivers/net/bnxt/bnxt_cpr.c    | 33 ++++++++++++++++++---------------
>  drivers/net/bnxt/bnxt_cpr.h    |  2 +-
>  drivers/net/bnxt/bnxt_ethdev.c | 16 ++++++++--------
>  drivers/net/bnxt/bnxt_hwrm.c   |  4 ++--
>  5 files changed, 32 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
> index a64b138..5121d05 100644
> --- a/drivers/net/bnxt/bnxt.h
> +++ b/drivers/net/bnxt/bnxt.h
> @@ -501,9 +501,9 @@ struct bnxt_ctx_mem_buf_info {
>  struct bnxt_error_recovery_info {
>         /* All units in milliseconds */
>         uint32_t        driver_polling_freq;
> -       uint32_t        master_func_wait_period;
> +       uint32_t        primary_func_wait_period;
>         uint32_t        normal_func_wait_period;
> -       uint32_t        master_func_wait_period_after_reset;
> +       uint32_t        primary_func_wait_period_after_reset;
>         uint32_t        max_bailout_time_after_reset;
>  #define BNXT_FW_STATUS_REG             0
>  #define BNXT_FW_HEARTBEAT_CNT_REG      1
> @@ -520,7 +520,7 @@ struct bnxt_error_recovery_info {
>         uint8_t         delay_after_reset[BNXT_NUM_RESET_REG];
>  #define BNXT_FLAG_ERROR_RECOVERY_HOST  BIT(0)
>  #define BNXT_FLAG_ERROR_RECOVERY_CO_CPU        BIT(1)
> -#define BNXT_FLAG_MASTER_FUNC          BIT(2)
> +#define BNXT_FLAG_PRIMARY_FUNC         BIT(2)
>  #define BNXT_FLAG_RECOVERY_ENABLED     BIT(3)
>         uint32_t        flags;
>
> diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
> index 12230dc..63ff02a 100644
> --- a/drivers/net/bnxt/bnxt_cpr.c
> +++ b/drivers/net/bnxt/bnxt_cpr.c
> @@ -120,6 +120,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
>         struct bnxt_error_recovery_info *info;
>         uint32_t event_data;
>         uint32_t data1, data2;
> +       uint32_t status;
>
>         data1 = rte_le_to_cpu_32(async_cmp->event_data1);
>         data2 = rte_le_to_cpu_32(async_cmp->event_data2);
> @@ -188,24 +189,26 @@ void bnxt_handle_async_event(struct bnxt *bp,
>                 if (!info)
>                         return;
>
> -               PMD_DRV_LOG(INFO, "Port %u: Error recovery async event received\n",
> -                           port_id);
> -
>                 event_data = data1 & EVENT_DATA1_FLAGS_MASK;
>
> -               if (event_data & EVENT_DATA1_FLAGS_MASTER_FUNC)
> -                       info->flags |= BNXT_FLAG_MASTER_FUNC;
> -               else
> -                       info->flags &= ~BNXT_FLAG_MASTER_FUNC;
> -
> -               if (event_data & EVENT_DATA1_FLAGS_RECOVERY_ENABLED)
> +               if (event_data & EVENT_DATA1_FLAGS_RECOVERY_ENABLED) {
>                         info->flags |= BNXT_FLAG_RECOVERY_ENABLED;
> -               else
> +               } else {
>                         info->flags &= ~BNXT_FLAG_RECOVERY_ENABLED;
> +                       PMD_DRV_LOG(INFO, "Driver recovery watchdog is disabled\n");
> +                       return;
> +               }
> +
> +               if (event_data & EVENT_DATA1_FLAGS_MASTER_FUNC)
> +                       info->flags |= BNXT_FLAG_PRIMARY_FUNC;
> +               else
> +                       info->flags &= ~BNXT_FLAG_PRIMARY_FUNC;
>
> -               PMD_DRV_LOG(INFO, "Port %u: recovery enabled(%d), master function(%d)\n",
> -                           port_id, bnxt_is_recovery_enabled(bp),
> -                           bnxt_is_master_func(bp));
> +               status = bnxt_read_fw_status_reg(bp, BNXT_FW_STATUS_REG);
> +               PMD_DRV_LOG(INFO,
> +                           "Port: %u Driver recovery watchdog, role: %s, FW status: 0x%x (%s)\n",
> +                           port_id, bnxt_is_primary_func(bp) ? "primary" : "backup", status,
> +                           (status == BNXT_FW_STATUS_HEALTHY) ? "healthy" : "unhealthy");
>
>                 if (bp->flags & BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED)
>                         return;
> @@ -361,9 +364,9 @@ int bnxt_event_hwrm_resp_handler(struct bnxt *bp, struct cmpl_base *cmp)
>         return evt;
>  }
>
> -bool bnxt_is_master_func(struct bnxt *bp)
> +bool bnxt_is_primary_func(struct bnxt *bp)
>  {
> -       if (bp->recovery_info->flags & BNXT_FLAG_MASTER_FUNC)
> +       if (bp->recovery_info->flags & BNXT_FLAG_PRIMARY_FUNC)
>                 return true;
>
>         return false;
> diff --git a/drivers/net/bnxt/bnxt_cpr.h b/drivers/net/bnxt/bnxt_cpr.h
> index 4095c8c..73468ed 100644
> --- a/drivers/net/bnxt/bnxt_cpr.h
> +++ b/drivers/net/bnxt/bnxt_cpr.h
> @@ -115,7 +115,7 @@ void bnxt_wait_for_device_shutdown(struct bnxt *bp);
>         HWRM_ASYNC_EVENT_CMPL_ERROR_RECOVERY_EVENT_DATA1_FLAGS_RECOVERY_ENABLED
>
>  bool bnxt_is_recovery_enabled(struct bnxt *bp);
> -bool bnxt_is_master_func(struct bnxt *bp);
> +bool bnxt_is_primary_func(struct bnxt *bp);
>
>  void bnxt_stop_rxtx(struct bnxt *bp);
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index dc7dee1..a7f203c 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -4443,11 +4443,11 @@ static int bnxt_fw_reset_all(struct bnxt *bp)
>         int rc = 0;
>
>         if (info->flags & BNXT_FLAG_ERROR_RECOVERY_HOST) {
> -               /* Reset through master function driver */
> +               /* Reset through primary function driver */
>                 for (i = 0; i < info->reg_array_cnt; i++)
>                         bnxt_write_fw_reset_reg(bp, i);
>                 /* Wait for time specified by FW after triggering reset */
> -               rte_delay_ms(info->master_func_wait_period_after_reset);
> +               rte_delay_ms(info->primary_func_wait_period_after_reset);
>         } else if (info->flags & BNXT_FLAG_ERROR_RECOVERY_CO_CPU) {
>                 /* Reset with the help of Kong processor */
>                 rc = bnxt_hwrm_fw_reset(bp);
> @@ -4464,8 +4464,8 @@ static void bnxt_fw_reset_cb(void *arg)
>         struct bnxt_error_recovery_info *info = bp->recovery_info;
>         int rc = 0;
>
> -       /* Only Master function can do FW reset */
> -       if (bnxt_is_master_func(bp) &&
> +       /* Only Primary function can do FW reset */
> +       if (bnxt_is_primary_func(bp) &&
>             bnxt_is_recovery_enabled(bp)) {
>                 rc = bnxt_fw_reset_all(bp);
>                 if (rc) {
> @@ -4493,8 +4493,8 @@ static void bnxt_fw_reset_cb(void *arg)
>   * advertised by FW in HWRM_ERROR_RECOVERY_QCFG.
>   * When the driver detects heartbeat stop or change in reset_counter,
>   * it has to trigger a reset to recover from the error condition.
> - * A “master PF” is the function who will have the privilege to
> - * initiate the chimp reset. The master PF will be elected by the
> + * A “primary function” is the function who will have the privilege to
> + * initiate the chimp reset. The primary function will be elected by the
>   * firmware and will be notified through async message.
>   */
>  static void bnxt_check_fw_health(void *arg)
> @@ -4532,8 +4532,8 @@ static void bnxt_check_fw_health(void *arg)
>
>         PMD_DRV_LOG(ERR, "Detected FW dead condition\n");
>
> -       if (bnxt_is_master_func(bp))
> -               wait_msec = info->master_func_wait_period;
> +       if (bnxt_is_primary_func(bp))
> +               wait_msec = info->primary_func_wait_period;
>         else
>                 wait_msec = info->normal_func_wait_period;
>
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index d4d8581..503add4 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -5716,11 +5716,11 @@ int bnxt_hwrm_error_recovery_qcfg(struct bnxt *bp)
>         /* FW returned values are in units of 100msec */
>         info->driver_polling_freq =
>                 rte_le_to_cpu_32(resp->driver_polling_freq) * 100;
> -       info->master_func_wait_period =
> +       info->primary_func_wait_period =
>                 rte_le_to_cpu_32(resp->master_func_wait_period) * 100;
>         info->normal_func_wait_period =
>                 rte_le_to_cpu_32(resp->normal_func_wait_period) * 100;
> -       info->master_func_wait_period_after_reset =
> +       info->primary_func_wait_period_after_reset =
>                 rte_le_to_cpu_32(resp->master_func_wait_period_after_reset) * 100;
>         info->max_bailout_time_after_reset =
>                 rte_le_to_cpu_32(resp->max_bailout_time_after_reset) * 100;
> --
> 2.10.1
>
  

Patch

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index a64b138..5121d05 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -501,9 +501,9 @@  struct bnxt_ctx_mem_buf_info {
 struct bnxt_error_recovery_info {
 	/* All units in milliseconds */
 	uint32_t	driver_polling_freq;
-	uint32_t	master_func_wait_period;
+	uint32_t	primary_func_wait_period;
 	uint32_t	normal_func_wait_period;
-	uint32_t	master_func_wait_period_after_reset;
+	uint32_t	primary_func_wait_period_after_reset;
 	uint32_t	max_bailout_time_after_reset;
 #define BNXT_FW_STATUS_REG		0
 #define BNXT_FW_HEARTBEAT_CNT_REG	1
@@ -520,7 +520,7 @@  struct bnxt_error_recovery_info {
 	uint8_t		delay_after_reset[BNXT_NUM_RESET_REG];
 #define BNXT_FLAG_ERROR_RECOVERY_HOST	BIT(0)
 #define BNXT_FLAG_ERROR_RECOVERY_CO_CPU	BIT(1)
-#define BNXT_FLAG_MASTER_FUNC		BIT(2)
+#define BNXT_FLAG_PRIMARY_FUNC		BIT(2)
 #define BNXT_FLAG_RECOVERY_ENABLED	BIT(3)
 	uint32_t	flags;
 
diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index 12230dc..63ff02a 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -120,6 +120,7 @@  void bnxt_handle_async_event(struct bnxt *bp,
 	struct bnxt_error_recovery_info *info;
 	uint32_t event_data;
 	uint32_t data1, data2;
+	uint32_t status;
 
 	data1 = rte_le_to_cpu_32(async_cmp->event_data1);
 	data2 = rte_le_to_cpu_32(async_cmp->event_data2);
@@ -188,24 +189,26 @@  void bnxt_handle_async_event(struct bnxt *bp,
 		if (!info)
 			return;
 
-		PMD_DRV_LOG(INFO, "Port %u: Error recovery async event received\n",
-			    port_id);
-
 		event_data = data1 & EVENT_DATA1_FLAGS_MASK;
 
-		if (event_data & EVENT_DATA1_FLAGS_MASTER_FUNC)
-			info->flags |= BNXT_FLAG_MASTER_FUNC;
-		else
-			info->flags &= ~BNXT_FLAG_MASTER_FUNC;
-
-		if (event_data & EVENT_DATA1_FLAGS_RECOVERY_ENABLED)
+		if (event_data & EVENT_DATA1_FLAGS_RECOVERY_ENABLED) {
 			info->flags |= BNXT_FLAG_RECOVERY_ENABLED;
-		else
+		} else {
 			info->flags &= ~BNXT_FLAG_RECOVERY_ENABLED;
+			PMD_DRV_LOG(INFO, "Driver recovery watchdog is disabled\n");
+			return;
+		}
+
+		if (event_data & EVENT_DATA1_FLAGS_MASTER_FUNC)
+			info->flags |= BNXT_FLAG_PRIMARY_FUNC;
+		else
+			info->flags &= ~BNXT_FLAG_PRIMARY_FUNC;
 
-		PMD_DRV_LOG(INFO, "Port %u: recovery enabled(%d), master function(%d)\n",
-			    port_id, bnxt_is_recovery_enabled(bp),
-			    bnxt_is_master_func(bp));
+		status = bnxt_read_fw_status_reg(bp, BNXT_FW_STATUS_REG);
+		PMD_DRV_LOG(INFO,
+			    "Port: %u Driver recovery watchdog, role: %s, FW status: 0x%x (%s)\n",
+			    port_id, bnxt_is_primary_func(bp) ? "primary" : "backup", status,
+			    (status == BNXT_FW_STATUS_HEALTHY) ? "healthy" : "unhealthy");
 
 		if (bp->flags & BNXT_FLAG_FW_HEALTH_CHECK_SCHEDULED)
 			return;
@@ -361,9 +364,9 @@  int bnxt_event_hwrm_resp_handler(struct bnxt *bp, struct cmpl_base *cmp)
 	return evt;
 }
 
-bool bnxt_is_master_func(struct bnxt *bp)
+bool bnxt_is_primary_func(struct bnxt *bp)
 {
-	if (bp->recovery_info->flags & BNXT_FLAG_MASTER_FUNC)
+	if (bp->recovery_info->flags & BNXT_FLAG_PRIMARY_FUNC)
 		return true;
 
 	return false;
diff --git a/drivers/net/bnxt/bnxt_cpr.h b/drivers/net/bnxt/bnxt_cpr.h
index 4095c8c..73468ed 100644
--- a/drivers/net/bnxt/bnxt_cpr.h
+++ b/drivers/net/bnxt/bnxt_cpr.h
@@ -115,7 +115,7 @@  void bnxt_wait_for_device_shutdown(struct bnxt *bp);
 	HWRM_ASYNC_EVENT_CMPL_ERROR_RECOVERY_EVENT_DATA1_FLAGS_RECOVERY_ENABLED
 
 bool bnxt_is_recovery_enabled(struct bnxt *bp);
-bool bnxt_is_master_func(struct bnxt *bp);
+bool bnxt_is_primary_func(struct bnxt *bp);
 
 void bnxt_stop_rxtx(struct bnxt *bp);
 
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index dc7dee1..a7f203c 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -4443,11 +4443,11 @@  static int bnxt_fw_reset_all(struct bnxt *bp)
 	int rc = 0;
 
 	if (info->flags & BNXT_FLAG_ERROR_RECOVERY_HOST) {
-		/* Reset through master function driver */
+		/* Reset through primary function driver */
 		for (i = 0; i < info->reg_array_cnt; i++)
 			bnxt_write_fw_reset_reg(bp, i);
 		/* Wait for time specified by FW after triggering reset */
-		rte_delay_ms(info->master_func_wait_period_after_reset);
+		rte_delay_ms(info->primary_func_wait_period_after_reset);
 	} else if (info->flags & BNXT_FLAG_ERROR_RECOVERY_CO_CPU) {
 		/* Reset with the help of Kong processor */
 		rc = bnxt_hwrm_fw_reset(bp);
@@ -4464,8 +4464,8 @@  static void bnxt_fw_reset_cb(void *arg)
 	struct bnxt_error_recovery_info *info = bp->recovery_info;
 	int rc = 0;
 
-	/* Only Master function can do FW reset */
-	if (bnxt_is_master_func(bp) &&
+	/* Only Primary function can do FW reset */
+	if (bnxt_is_primary_func(bp) &&
 	    bnxt_is_recovery_enabled(bp)) {
 		rc = bnxt_fw_reset_all(bp);
 		if (rc) {
@@ -4493,8 +4493,8 @@  static void bnxt_fw_reset_cb(void *arg)
  * advertised by FW in HWRM_ERROR_RECOVERY_QCFG.
  * When the driver detects heartbeat stop or change in reset_counter,
  * it has to trigger a reset to recover from the error condition.
- * A “master PF” is the function who will have the privilege to
- * initiate the chimp reset. The master PF will be elected by the
+ * A “primary function” is the function who will have the privilege to
+ * initiate the chimp reset. The primary function will be elected by the
  * firmware and will be notified through async message.
  */
 static void bnxt_check_fw_health(void *arg)
@@ -4532,8 +4532,8 @@  static void bnxt_check_fw_health(void *arg)
 
 	PMD_DRV_LOG(ERR, "Detected FW dead condition\n");
 
-	if (bnxt_is_master_func(bp))
-		wait_msec = info->master_func_wait_period;
+	if (bnxt_is_primary_func(bp))
+		wait_msec = info->primary_func_wait_period;
 	else
 		wait_msec = info->normal_func_wait_period;
 
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index d4d8581..503add4 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -5716,11 +5716,11 @@  int bnxt_hwrm_error_recovery_qcfg(struct bnxt *bp)
 	/* FW returned values are in units of 100msec */
 	info->driver_polling_freq =
 		rte_le_to_cpu_32(resp->driver_polling_freq) * 100;
-	info->master_func_wait_period =
+	info->primary_func_wait_period =
 		rte_le_to_cpu_32(resp->master_func_wait_period) * 100;
 	info->normal_func_wait_period =
 		rte_le_to_cpu_32(resp->normal_func_wait_period) * 100;
-	info->master_func_wait_period_after_reset =
+	info->primary_func_wait_period_after_reset =
 		rte_le_to_cpu_32(resp->master_func_wait_period_after_reset) * 100;
 	info->max_bailout_time_after_reset =
 		rte_le_to_cpu_32(resp->max_bailout_time_after_reset) * 100;