* [dpdk-dev] [PATCH] net/bnxt: improve error recovery information messages
@ 2021-09-24 5:17 Kalesh A P
2021-09-27 4:37 ` Ajit Khaparde
0 siblings, 1 reply; 2+ messages in thread
From: Kalesh A P @ 2021-09-24 5:17 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, ajit.khaparde
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 7550 bytes --]
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(-)
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [dpdk-dev] [PATCH] net/bnxt: improve error recovery information messages
2021-09-24 5:17 [dpdk-dev] [PATCH] net/bnxt: improve error recovery information messages Kalesh A P
@ 2021-09-27 4:37 ` Ajit Khaparde
0 siblings, 0 replies; 2+ messages in thread
From: Ajit Khaparde @ 2021-09-27 4:37 UTC (permalink / raw)
To: Kalesh A P; +Cc: dpdk-dev, Ferruh Yigit
[-- Attachment #1: Type: text/plain, Size: 9397 bytes --]
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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-09-27 4:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24 5:17 [dpdk-dev] [PATCH] net/bnxt: improve error recovery information messages Kalesh A P
2021-09-27 4:37 ` Ajit Khaparde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).