[dpdk-stable] patch 'net/bnxt: fix link update' has been queued to stable release 19.11.6
Kalesh Anakkur Purayil
kalesh-anakkur.purayil at broadcom.com
Thu Oct 29 10:24:00 CET 2020
On Wed, Oct 28, 2020 at 4:22 PM <luca.boccassi at gmail.com> wrote:
> Hi,
>
> FYI, your patch has been queued to stable release 19.11.6
>
> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
> It will be pushed if I get no objections before 10/30/20. So please
> shout if anyone has objections.
>
> Also note that after the patch there's a diff of the upstream commit vs the
> patch applied to the branch. This will indicate if there was any rebasing
> needed to apply to the stable branch. If there were code changes for
> rebasing
> (ie: not only metadata diffs), please double check that the rebase was
> correctly done.
>
> Thanks.
>
> Luca Boccassi
>
> ---
> From 6037750360153c816ead7b67b3a522cfff6763c1 Mon Sep 17 00:00:00 2001
> From: Kalesh AP <kalesh-anakkur.purayil at broadcom.com>
> Date: Tue, 6 Oct 2020 21:31:56 +0530
> Subject: [PATCH] net/bnxt: fix link update
>
> [ upstream commit af57c49ca101d8f4c4138082b49dc86d4857f8de ]
>
> 1. When port is stopped, we can forcibly set the link status for the
> device to down.
> 2. VFs and MH PFs do not have the privilege to bring the link down.
> As a result driver prints "Link Up" when port is stopped.
> 3. When driver receives link status/speed/config async event from fw,
> driver invokes bnxt_link_update() with exp_link_status as ETH_LINK_UP
> This is not logically correct as the async event could be for Link up
> or link down or for speed change.
>
> Fixes: 074cacb9907a ("net/bnxt: fix link during port toggle")
>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil at broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur at broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde at broadcom.com>
> ---
> drivers/net/bnxt/bnxt.h | 6 +++--
> drivers/net/bnxt/bnxt_cpr.c | 2 +-
> drivers/net/bnxt/bnxt_ethdev.c | 40 +++++++++++++++++-----------------
> 3 files changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
> index cdb3e2d3ff..78249be6c7 100644
> --- a/drivers/net/bnxt/bnxt.h
> +++ b/drivers/net/bnxt/bnxt.h
> @@ -233,8 +233,8 @@ struct bnxt_pf_info {
> };
>
> /* Max wait time for link up is 10s and link down is 500ms */
> -#define BNXT_LINK_UP_WAIT_CNT 200
> -#define BNXT_LINK_DOWN_WAIT_CNT 10
> +#define BNXT_MAX_LINK_WAIT_CNT 200
> +#define BNXT_MIN_LINK_WAIT_CNT 10
> #define BNXT_LINK_WAIT_INTERVAL 50
> struct bnxt_link_info {
> uint32_t phy_flags;
> @@ -681,6 +681,8 @@ void bnxt_schedule_fw_health_check(struct bnxt *bp);
>
> bool is_bnxt_supported(struct rte_eth_dev *dev);
> bool bnxt_stratus_device(struct bnxt *bp);
> +int bnxt_link_update_op(struct rte_eth_dev *eth_dev,
> + int wait_to_complete);
> extern const struct rte_flow_ops bnxt_flow_ops;
> #define bnxt_acquire_flow_lock(bp) \
> pthread_mutex_lock(&(bp)->flow_lock)
> diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
> index c0e492e6c2..ba9933dc9b 100644
> --- a/drivers/net/bnxt/bnxt_cpr.c
> +++ b/drivers/net/bnxt/bnxt_cpr.c
> @@ -63,7 +63,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
> case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CHANGE:
> case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE:
> /* FALLTHROUGH */
> - bnxt_link_update(bp->eth_dev, 0, ETH_LINK_UP);
> + bnxt_link_update_op(bp->eth_dev, 0);
> break;
> case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_PF_DRVR_UNLOAD:
> PMD_DRV_LOG(INFO, "Async event: PF driver unloaded\n");
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c
> index c6e9ad8e6a..a94b73623c 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -871,7 +871,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev
> *eth_dev)
> eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
> eth_dev->data->dev_started = 1;
>
> - bnxt_link_update(eth_dev, 1, ETH_LINK_UP);
> + bnxt_link_update_op(eth_dev, 1);
>
> if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
> vlan_mask |= ETH_VLAN_FILTER_MASK;
> @@ -928,6 +928,7 @@ static void bnxt_dev_stop_op(struct rte_eth_dev
> *eth_dev)
> struct bnxt *bp = eth_dev->data->dev_private;
> struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
> struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
> + struct rte_eth_link link;
>
> eth_dev->data->dev_started = 0;
> /* Prevent crashes when queues are still in use */
> @@ -942,15 +943,15 @@ static void bnxt_dev_stop_op(struct rte_eth_dev
> *eth_dev)
> bnxt_cancel_fw_health_check(bp);
>
> /* Do not bring link down during reset recovery */
> - if (!is_bnxt_in_error(bp))
> + if (!is_bnxt_in_error(bp)) {
> bnxt_dev_set_link_down_op(eth_dev);
> -
> - /* Wait for link to be reset and the async notification to process.
> - * During reset recovery, there is no need to wait and
> - * VF/NPAR functions do not have privilege to change PHY config.
> - */
> - if (!is_bnxt_in_error(bp) && BNXT_SINGLE_PF(bp))
> - bnxt_link_update(eth_dev, 1, ETH_LINK_DOWN);
> + /* Wait for link to be reset */
> + if (BNXT_SINGLE_PF(bp))
> + rte_delay_ms(500);
> + /* clear the recorded link status */
> + memset(&link, 0, sizeof(link));
> + rte_eth_linkstatus_set(eth_dev, &link);
> + }
>
> /* Clean queue intr-vector mapping */
> rte_intr_efd_disable(intr_handle);
> @@ -1106,14 +1107,13 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev
> *eth_dev,
> return rc;
> }
>
> -int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete,
> - bool exp_link_status)
> +int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
> {
> int rc = 0;
> struct bnxt *bp = eth_dev->data->dev_private;
> struct rte_eth_link new;
> - int cnt = exp_link_status ? BNXT_LINK_UP_WAIT_CNT :
> - BNXT_LINK_DOWN_WAIT_CNT;
> + int cnt = wait_to_complete ? BNXT_MAX_LINK_WAIT_CNT :
> + BNXT_MIN_LINK_WAIT_CNT;
>
> rc = is_bnxt_in_error(bp);
> if (rc)
> @@ -1131,12 +1131,18 @@ int bnxt_link_update(struct rte_eth_dev *eth_dev,
> int wait_to_complete,
> goto out;
> }
>
> - if (!wait_to_complete || new.link_status ==
> exp_link_status)
> + if (!wait_to_complete || new.link_status)
> break;
>
> rte_delay_ms(BNXT_LINK_WAIT_INTERVAL);
> } while (cnt--);
>
> + /* Only single function PF can bring phy down.
> + * When port is stopped, report link down for VF/MH/NPAR functions.
> + */
> + if (!BNXT_SINGLE_PF(bp) && !eth_dev->data->dev_started)
> + memset(&new, 0, sizeof(new));
> +
> out:
> /* Timed out or success */
> if (new.link_status != eth_dev->data->dev_link.link_status ||
> @@ -1153,12 +1159,6 @@ out:
> return rc;
> }
>
> -static int bnxt_link_update_op(struct rte_eth_dev *eth_dev,
> - int wait_to_complete)
> -{
> - return bnxt_link_update(eth_dev, wait_to_complete, ETH_LINK_UP);
> -}
> -
> static int bnxt_promiscuous_enable_op(struct rte_eth_dev *eth_dev)
> {
> struct bnxt *bp = eth_dev->data->dev_private;
> --
> 2.20.1
>
> ---
> Diff of the applied patch vs upstream commit (please double-check if
> non-empty:
> ---
> --- - 2020-10-28 10:35:15.689269158 +0000
> +++ 0127-net-bnxt-fix-link-update.patch 2020-10-28 10:35:11.692832791 +0000
> @@ -1,8 +1,10 @@
> -From af57c49ca101d8f4c4138082b49dc86d4857f8de Mon Sep 17 00:00:00 2001
> +From 6037750360153c816ead7b67b3a522cfff6763c1 Mon Sep 17 00:00:00 2001
> From: Kalesh AP <kalesh-anakkur.purayil at broadcom.com>
> Date: Tue, 6 Oct 2020 21:31:56 +0530
> Subject: [PATCH] net/bnxt: fix link update
>
> +[ upstream commit af57c49ca101d8f4c4138082b49dc86d4857f8de ]
> +
> 1. When port is stopped, we can forcibly set the link status for the
> device to down.
> 2. VFs and MH PFs do not have the privilege to bring the link down.
> @@ -13,22 +15,21 @@
> or link down or for speed change.
>
> Fixes: 074cacb9907a ("net/bnxt: fix link during port toggle")
> -Cc: stable at dpdk.org
>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil at broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur at broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde at broadcom.com>
> ---
> - drivers/net/bnxt/bnxt.h | 4 ++--
> + drivers/net/bnxt/bnxt.h | 6 +++--
> drivers/net/bnxt/bnxt_cpr.c | 2 +-
> drivers/net/bnxt/bnxt_ethdev.c | 40 +++++++++++++++++-----------------
> - 3 files changed, 23 insertions(+), 23 deletions(-)
> + 3 files changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
> -index ae38428e69..eca74486e7 100644
> +index cdb3e2d3ff..78249be6c7 100644
> --- a/drivers/net/bnxt/bnxt.h
> +++ b/drivers/net/bnxt/bnxt.h
> -@@ -268,8 +268,8 @@ struct bnxt_pf_info {
> +@@ -233,8 +233,8 @@ struct bnxt_pf_info {
> };
>
> /* Max wait time for link up is 10s and link down is 500ms */
> @@ -39,11 +40,20 @@
> #define BNXT_LINK_WAIT_INTERVAL 50
> struct bnxt_link_info {
> uint32_t phy_flags;
> +@@ -681,6 +681,8 @@ void bnxt_schedule_fw_health_check(struct bnxt *bp);
> +
> + bool is_bnxt_supported(struct rte_eth_dev *dev);
> + bool bnxt_stratus_device(struct bnxt *bp);
> ++int bnxt_link_update_op(struct rte_eth_dev *eth_dev,
> ++ int wait_to_complete);
> + extern const struct rte_flow_ops bnxt_flow_ops;
> + #define bnxt_acquire_flow_lock(bp) \
> + pthread_mutex_lock(&(bp)->flow_lock)
> diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
> -index 8311e265ae..a3a7e6ab70 100644
> +index c0e492e6c2..ba9933dc9b 100644
> --- a/drivers/net/bnxt/bnxt_cpr.c
> +++ b/drivers/net/bnxt/bnxt_cpr.c
> -@@ -111,7 +111,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
> +@@ -63,7 +63,7 @@ void bnxt_handle_async_event(struct bnxt *bp,
> case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CHANGE:
> case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE:
> /* FALLTHROUGH */
> @@ -53,10 +63,10 @@
> case HWRM_ASYNC_EVENT_CMPL_EVENT_ID_PF_DRVR_UNLOAD:
> PMD_DRV_LOG(INFO, "Async event: PF driver unloaded\n");
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c
> -index 624cb20311..1bb0aa838a 100644
> +index c6e9ad8e6a..a94b73623c 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> -@@ -1279,7 +1279,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev
> *eth_dev)
> +@@ -871,7 +871,7 @@ static int bnxt_dev_start_op(struct rte_eth_dev
> *eth_dev)
> eth_dev->data->scattered_rx = bnxt_scattered_rx(eth_dev);
> eth_dev->data->dev_started = 1;
>
> @@ -65,15 +75,15 @@
>
> if (rx_offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
> vlan_mask |= ETH_VLAN_FILTER_MASK;
> -@@ -1347,6 +1347,7 @@ static void bnxt_dev_stop_op(struct rte_eth_dev
> *eth_dev)
> +@@ -928,6 +928,7 @@ static void bnxt_dev_stop_op(struct rte_eth_dev
> *eth_dev)
> struct bnxt *bp = eth_dev->data->dev_private;
> struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
> struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
> + struct rte_eth_link link;
>
> eth_dev->data->dev_started = 0;
> - eth_dev->data->scattered_rx = 0;
> -@@ -1369,15 +1370,15 @@ static void bnxt_dev_stop_op(struct rte_eth_dev
> *eth_dev)
> + /* Prevent crashes when queues are still in use */
> +@@ -942,15 +943,15 @@ static void bnxt_dev_stop_op(struct rte_eth_dev
> *eth_dev)
> bnxt_cancel_fw_health_check(bp);
>
> /* Do not bring link down during reset recovery */
> @@ -97,7 +107,7 @@
>
> /* Clean queue intr-vector mapping */
> rte_intr_efd_disable(intr_handle);
> -@@ -1557,14 +1558,13 @@ static int bnxt_mac_addr_add_op(struct
> rte_eth_dev *eth_dev,
> +@@ -1106,14 +1107,13 @@ static int bnxt_mac_addr_add_op(struct
> rte_eth_dev *eth_dev,
> return rc;
> }
>
> @@ -115,7 +125,7 @@
>
> rc = is_bnxt_in_error(bp);
> if (rc)
> -@@ -1582,12 +1582,18 @@ int bnxt_link_update(struct rte_eth_dev *eth_dev,
> int wait_to_complete,
> +@@ -1131,12 +1131,18 @@ int bnxt_link_update(struct rte_eth_dev *eth_dev,
> int wait_to_complete,
> goto out;
> }
>
> @@ -135,12 +145,12 @@
> out:
> /* Timed out or success */
> if (new.link_status != eth_dev->data->dev_link.link_status ||
> -@@ -1604,12 +1610,6 @@ out:
> +@@ -1153,12 +1159,6 @@ out:
> return rc;
> }
>
> --int bnxt_link_update_op(struct rte_eth_dev *eth_dev,
> -- int wait_to_complete)
> +-static int bnxt_link_update_op(struct rte_eth_dev *eth_dev,
> +- int wait_to_complete)
> -{
> - return bnxt_link_update(eth_dev, wait_to_complete, ETH_LINK_UP);
> -}
>
Hi Luca,
I see that the rebase is not correct, I will fix it and send the
patch to 19.11 LTS.
--
Regards,
Kalesh A P
More information about the stable
mailing list