patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH 19.11 1/2] net/bnxt: fix link status during device recovery
@ 2020-10-29 10:13 Kalesh A P
  2020-10-29 10:13 ` [dpdk-stable] [PATCH 19.11 2/2] net/bnxt: fix link update Kalesh A P
  2020-10-29 10:37 ` [dpdk-stable] [PATCH 19.11 1/2] net/bnxt: fix link status during device recovery Luca Boccassi
  0 siblings, 2 replies; 3+ messages in thread
From: Kalesh A P @ 2020-10-29 10:13 UTC (permalink / raw)
  To: stable

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

[ upstream commit 6fcd0de92298f61b8f96ba170db9e81923019626 ]

Driver should not send the phy_cfg request to bring link down
during reset recovery. If the driver sends the phy_cfg request
in recovery process, then FW needs to re-establish the link which
in turn increases the recovery time based on PHY type and link partners.

Fixes: df6cd7c1f73a ("net/bnxt: handle reset notify async event from FW")

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index fe240b6..eeecad0 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -943,7 +943,9 @@ static void bnxt_dev_stop_op(struct rte_eth_dev *eth_dev)
 
 	bnxt_cancel_fw_health_check(bp);
 
-	bnxt_dev_set_link_down_op(eth_dev);
+	/* Do not bring link down during reset recovery */
+	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
@@ -3909,7 +3911,7 @@ static void bnxt_write_fw_reset_reg(struct bnxt *bp, uint32_t index)
 
 static void bnxt_dev_cleanup(struct bnxt *bp)
 {
-	bnxt_set_hwrm_link_config(bp, false);
+	bp->eth_dev->data->dev_link.link_status = 0;
 	bp->link_info.link_up = 0;
 	if (bp->eth_dev->data->dev_started)
 		bnxt_dev_stop_op(bp->eth_dev);
-- 
2.10.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [dpdk-stable] [PATCH 19.11 2/2] net/bnxt: fix link update
  2020-10-29 10:13 [dpdk-stable] [PATCH 19.11 1/2] net/bnxt: fix link status during device recovery Kalesh A P
@ 2020-10-29 10:13 ` Kalesh A P
  2020-10-29 10:37 ` [dpdk-stable] [PATCH 19.11 1/2] net/bnxt: fix link status during device recovery Luca Boccassi
  1 sibling, 0 replies; 3+ messages in thread
From: Kalesh A P @ 2020-10-29 10:13 UTC (permalink / raw)
  To: stable

From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

[ 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@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@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 46cf418..6e3367b 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;
@@ -680,6 +680,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 c0e492e..ba9933d 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 eeecad0..5adbf69 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -872,7 +872,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;
@@ -930,6 +930,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 */
@@ -944,15 +945,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);
@@ -1108,14 +1109,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)
@@ -1133,12 +1133,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 ||
@@ -1155,12 +1161,6 @@ int bnxt_link_update(struct rte_eth_dev *eth_dev, int wait_to_complete,
 	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.10.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-stable] [PATCH 19.11 1/2] net/bnxt: fix link status during device recovery
  2020-10-29 10:13 [dpdk-stable] [PATCH 19.11 1/2] net/bnxt: fix link status during device recovery Kalesh A P
  2020-10-29 10:13 ` [dpdk-stable] [PATCH 19.11 2/2] net/bnxt: fix link update Kalesh A P
@ 2020-10-29 10:37 ` Luca Boccassi
  1 sibling, 0 replies; 3+ messages in thread
From: Luca Boccassi @ 2020-10-29 10:37 UTC (permalink / raw)
  To: Kalesh A P, stable

On Thu, 2020-10-29 at 15:43 +0530, Kalesh A P wrote:
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> 
> [ upstream commit 6fcd0de92298f61b8f96ba170db9e81923019626 ]
> 
> Driver should not send the phy_cfg request to bring link down
> during reset recovery. If the driver sends the phy_cfg request
> in recovery process, then FW needs to re-establish the link which
> in turn increases the recovery time based on PHY type and link partners.
> 
> Fixes: df6cd7c1f73a ("net/bnxt: handle reset notify async event from FW")
> 
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>  drivers/net/bnxt/bnxt_ethdev.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Hi,

Both of these patches are 100% identical to the ones I pushed
yesterday? Was there something unclear in the generated email that made
you think the rebase was different that we can fix?

https://github.com/bluca/dpdk-stable/commit/d777cb4ecd402ff3f1e73b056f112f478c688948
https://github.com/bluca/dpdk-stable/commit/059f32044f4682c5aedde42e46b7e628550ea480

-- 
Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-10-29 10:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-29 10:13 [dpdk-stable] [PATCH 19.11 1/2] net/bnxt: fix link status during device recovery Kalesh A P
2020-10-29 10:13 ` [dpdk-stable] [PATCH 19.11 2/2] net/bnxt: fix link update Kalesh A P
2020-10-29 10:37 ` [dpdk-stable] [PATCH 19.11 1/2] net/bnxt: fix link status during device recovery Luca Boccassi

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git