From: Ajit Khaparde <ajit.khaparde@broadcom.com>
To: Kalesh A P <kalesh-anakkur.purayil@broadcom.com>
Cc: dpdk-dev <dev@dpdk.org>, Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [dpdk-dev] [PATCH] net/bnxt: improve error recovery information messages
Date: Sun, 26 Sep 2021 21:37:33 -0700 [thread overview]
Message-ID: <CACZ4nhudr-UDkx1q37SROYR4VJWKO8UBX=2oo3q82Vo_wSRohA@mail.gmail.com> (raw)
In-Reply-To: <20210924051753.30822-1-kalesh-anakkur.purayil@broadcom.com>
[-- 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
>
prev parent reply other threads:[~2021-09-27 4:37 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-24 5:17 Kalesh A P
2021-09-27 4:37 ` Ajit Khaparde [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CACZ4nhudr-UDkx1q37SROYR4VJWKO8UBX=2oo3q82Vo_wSRohA@mail.gmail.com' \
--to=ajit.khaparde@broadcom.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=kalesh-anakkur.purayil@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).