From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id AC198A04B5 for ; Thu, 29 Oct 2020 10:24:14 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9D407C82A; Thu, 29 Oct 2020 10:24:13 +0100 (CET) Received: from mail-io1-f49.google.com (mail-io1-f49.google.com [209.85.166.49]) by dpdk.org (Postfix) with ESMTP id CFC79C82A for ; Thu, 29 Oct 2020 10:24:11 +0100 (CET) Received: by mail-io1-f49.google.com with SMTP id z5so2626057iob.1 for ; Thu, 29 Oct 2020 02:24:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ugMW2l6c7KetVRW75RKWJfPjnauA9oG/v7/NHuonDJU=; b=AQ7xWE1617YY+YweZHqxzZFgiQZJN6Ds+zSKFAakgj6GItnYqzUYWZPA7jJNHneRgn z9dIHEpUivb350yJF0MrMmiYm/18UVqj6q0xCaQFo8sb/Vgs/WfQzTaf8/sZIAkFCbd+ 7ZgWRkHmTAQTzjrQObQqW7rdF1HHeKaqwp86w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ugMW2l6c7KetVRW75RKWJfPjnauA9oG/v7/NHuonDJU=; b=QEiiq4/p/Uhqg/F77a8YW2b08rfbOhz3HMv7mAgd9iRBKtrmIiTfgiH3r++ewYXCuh NssDJfBk8h9kV53ExBVMzxW0VCkJwiUThB+tw3YgPUne4VcyFejkeAu6jQrHmDO4JYkm lJ20vF8frIGII+3xfsYtfiSjLOq27J+ZjkRuq+hs2crGZFeB82k/Rsfvls7Asf8nOU3R VprHFrZJs0qNvKd8M2KpPgHEcd5Ep+r3AuW6poSvGPLnwsoelSSzNVPn0gb0r9ZWM/lT aEcgd77y9tc/A3DIgijeixBoJcWzHC6Jolq5x8uNuw9tP00ddoMaXj6dqXNEeHrgNBnE fB6w== X-Gm-Message-State: AOAM533/yQHSNmGE50rGqqYby8gufhJpzuGtsUiOzaIiKStfNlm6AOga bZAevPMlVlPDhhjYJ2+UBVGtncJZiMQB9h7jiLfYkw== X-Google-Smtp-Source: ABdhPJyD//y0K61yQjRbTDH0o46yv0JjWoY1TV0IRWEfxZguR+DTyDitC0zsCEDhXgU8qCQb3/94mimEtf/TWHUhZSc= X-Received: by 2002:a5d:9602:: with SMTP id w2mr2687379iol.120.1603963451090; Thu, 29 Oct 2020 02:24:11 -0700 (PDT) MIME-Version: 1.0 References: <20201028104606.3504127-1-luca.boccassi@gmail.com> <20201028104606.3504127-127-luca.boccassi@gmail.com> In-Reply-To: <20201028104606.3504127-127-luca.boccassi@gmail.com> From: Kalesh Anakkur Purayil Date: Thu, 29 Oct 2020 14:54:00 +0530 Message-ID: To: Luca Boccassi Cc: Somnath Kotur , Ajit Khaparde , dpdk stable Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-stable] patch 'net/bnxt: fix link update' has been queued to stable release 19.11.6 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On Wed, Oct 28, 2020 at 4:22 PM 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 > 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 > Reviewed-by: Somnath Kotur > Reviewed-by: Ajit Khaparde > --- > 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 > 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@dpdk.org > > Signed-off-by: Kalesh AP > Reviewed-by: Somnath Kotur > Reviewed-by: Ajit Khaparde > --- > - 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