| Message ID | 20240808051518.3580248-7-dw@davidwei.uk (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 97cbf3d0accce0f29db0be3ac1b50d9d561d3206 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | fix bnxt_en queue reset when queue is active | expand |
On Thu, Aug 8, 2024 at 2:16 PM David Wei <dw@davidwei.uk> wrote: > Hi David, Thank you so much for this work! > The queue API calls bnxt_hwrm_vnic_update() to stop/start the flow of > packets, which can only properly flush the pipeline if FW indicates > support. > > Add a macro BNXT_SUPPORTS_QUEUE_API that checks for the required flags > and only set queue_mgmt_ops if true. > > Signed-off-by: David Wei <dw@davidwei.uk> > --- > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 ++- > drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 +++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > index 7762fa3b646a..85d4fa8c73ae 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > @@ -15717,7 +15717,6 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > dev->stat_ops = &bnxt_stat_ops; > dev->watchdog_timeo = BNXT_TX_TIMEOUT; > dev->ethtool_ops = &bnxt_ethtool_ops; > - dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops; > pci_set_drvdata(pdev, dev); > > rc = bnxt_alloc_hwrm_resources(bp); > @@ -15898,6 +15897,8 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > > if (BNXT_SUPPORTS_NTUPLE_VNIC(bp)) > bp->rss_cap |= BNXT_RSS_CAP_MULTI_RSS_CTX; > + if (BNXT_SUPPORTS_QUEUE_API(bp)) > + dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops; > > rc = register_netdev(dev); > if (rc) > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h > index a2233b2d9329..62e637c5be31 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h > @@ -2451,6 +2451,9 @@ struct bnxt { > #define BNXT_SUPPORTS_MULTI_RSS_CTX(bp) \ > (BNXT_PF(bp) && BNXT_SUPPORTS_NTUPLE_VNIC(bp) && \ > ((bp)->rss_cap & BNXT_RSS_CAP_MULTI_RSS_CTX)) > +#define BNXT_SUPPORTS_QUEUE_API(bp) \ > + (BNXT_PF(bp) && BNXT_SUPPORTS_NTUPLE_VNIC(bp) && \ > + ((bp)->fw_cap & BNXT_FW_CAP_VNIC_RE_FLUSH)) > > u32 hwrm_spec_code; > u16 hwrm_cmd_seq; > -- > 2.43.5 > > What Broadcom NICs support BNXT_SUPPORTS_QUEUE_API? I have been testing the device memory TCP feature with bnxt_en driver and I'm using BCM57508, BCM57608, and BCM57412 NICs. (BCM57508's firmware is too old, but BCM57608's firmware is the latest, BCM57412 too). Currently, I can't test the device memory TCP feature because my NICs don't support BNXT_SUPPORTS_QUEUE_API. The BCM57608 only supports the BNXT_SUPPORTS_NTUPLE_VNIC, but does not support the BNXT_SUPPORTS_QUEUE_API. The BCM57412 doesn't support both of them. I think at least BCM57508 and BCM57608 should support this because it's the same or newer product line as BCM57504 as far as I know. Am I missing something? Thanks a lot! Taehee Yoo
On 2024-08-08 08:13, Taehee Yoo wrote: > On Thu, Aug 8, 2024 at 2:16 PM David Wei <dw@davidwei.uk> wrote: >> > Hi David, > Thank you so much for this work! > >> The queue API calls bnxt_hwrm_vnic_update() to stop/start the flow of >> packets, which can only properly flush the pipeline if FW indicates >> support. >> >> Add a macro BNXT_SUPPORTS_QUEUE_API that checks for the required flags >> and only set queue_mgmt_ops if true. >> >> Signed-off-by: David Wei <dw@davidwei.uk> >> --- >> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 ++- >> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 +++ >> 2 files changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c >> index 7762fa3b646a..85d4fa8c73ae 100644 >> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c >> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c >> @@ -15717,7 +15717,6 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) >> dev->stat_ops = &bnxt_stat_ops; >> dev->watchdog_timeo = BNXT_TX_TIMEOUT; >> dev->ethtool_ops = &bnxt_ethtool_ops; >> - dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops; >> pci_set_drvdata(pdev, dev); >> >> rc = bnxt_alloc_hwrm_resources(bp); >> @@ -15898,6 +15897,8 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) >> >> if (BNXT_SUPPORTS_NTUPLE_VNIC(bp)) >> bp->rss_cap |= BNXT_RSS_CAP_MULTI_RSS_CTX; >> + if (BNXT_SUPPORTS_QUEUE_API(bp)) >> + dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops; >> >> rc = register_netdev(dev); >> if (rc) >> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h >> index a2233b2d9329..62e637c5be31 100644 >> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h >> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h >> @@ -2451,6 +2451,9 @@ struct bnxt { >> #define BNXT_SUPPORTS_MULTI_RSS_CTX(bp) \ >> (BNXT_PF(bp) && BNXT_SUPPORTS_NTUPLE_VNIC(bp) && \ >> ((bp)->rss_cap & BNXT_RSS_CAP_MULTI_RSS_CTX)) >> +#define BNXT_SUPPORTS_QUEUE_API(bp) \ >> + (BNXT_PF(bp) && BNXT_SUPPORTS_NTUPLE_VNIC(bp) && \ >> + ((bp)->fw_cap & BNXT_FW_CAP_VNIC_RE_FLUSH)) >> >> u32 hwrm_spec_code; >> u16 hwrm_cmd_seq; >> -- >> 2.43.5 >> >> > > What Broadcom NICs support BNXT_SUPPORTS_QUEUE_API? > > I have been testing the device memory TCP feature with bnxt_en driver > and I'm using BCM57508, BCM57608, and BCM57412 NICs. > (BCM57508's firmware is too old, but BCM57608's firmware is the > latest, BCM57412 too). > Currently, I can't test the device memory TCP feature because my NICs > don't support BNXT_SUPPORTS_QUEUE_API. > The BCM57608 only supports the BNXT_SUPPORTS_NTUPLE_VNIC, but does not > support the BNXT_SUPPORTS_QUEUE_API. > The BCM57412 doesn't support both of them. > I think at least BCM57508 and BCM57608 should support this because > it's the same or newer product line as BCM57504 as far as I know. > Am I missing something? The hardware is correct (Thor+) but there needs to be a new FW update from Broadcom that returns VNIC_QCAPS_RESP_FLAGS_RE_FLUSH_CAP when queried. CC: Michael Chan <michael.chan@broadcom.com> > > Thanks a lot! > Taehee Yoo
On Fri, Aug 9, 2024 at 12:42 AM David Wei <dw@davidwei.uk> wrote: > > On 2024-08-08 08:13, Taehee Yoo wrote: > > On Thu, Aug 8, 2024 at 2:16 PM David Wei <dw@davidwei.uk> wrote: > >> > > Hi David, > > Thank you so much for this work! > > > >> The queue API calls bnxt_hwrm_vnic_update() to stop/start the flow of > >> packets, which can only properly flush the pipeline if FW indicates > >> support. > >> > >> Add a macro BNXT_SUPPORTS_QUEUE_API that checks for the required flags > >> and only set queue_mgmt_ops if true. > >> > >> Signed-off-by: David Wei <dw@davidwei.uk> > >> --- > >> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 ++- > >> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 +++ > >> 2 files changed, 5 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > >> index 7762fa3b646a..85d4fa8c73ae 100644 > >> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c > >> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c > >> @@ -15717,7 +15717,6 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > >> dev->stat_ops = &bnxt_stat_ops; > >> dev->watchdog_timeo = BNXT_TX_TIMEOUT; > >> dev->ethtool_ops = &bnxt_ethtool_ops; > >> - dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops; > >> pci_set_drvdata(pdev, dev); > >> > >> rc = bnxt_alloc_hwrm_resources(bp); > >> @@ -15898,6 +15897,8 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > >> > >> if (BNXT_SUPPORTS_NTUPLE_VNIC(bp)) > >> bp->rss_cap |= BNXT_RSS_CAP_MULTI_RSS_CTX; > >> + if (BNXT_SUPPORTS_QUEUE_API(bp)) > >> + dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops; > >> > >> rc = register_netdev(dev); > >> if (rc) > >> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h > >> index a2233b2d9329..62e637c5be31 100644 > >> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h > >> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h > >> @@ -2451,6 +2451,9 @@ struct bnxt { > >> #define BNXT_SUPPORTS_MULTI_RSS_CTX(bp) \ > >> (BNXT_PF(bp) && BNXT_SUPPORTS_NTUPLE_VNIC(bp) && \ > >> ((bp)->rss_cap & BNXT_RSS_CAP_MULTI_RSS_CTX)) > >> +#define BNXT_SUPPORTS_QUEUE_API(bp) \ > >> + (BNXT_PF(bp) && BNXT_SUPPORTS_NTUPLE_VNIC(bp) && \ > >> + ((bp)->fw_cap & BNXT_FW_CAP_VNIC_RE_FLUSH)) > >> > >> u32 hwrm_spec_code; > >> u16 hwrm_cmd_seq; > >> -- > >> 2.43.5 > >> > >> > > > > What Broadcom NICs support BNXT_SUPPORTS_QUEUE_API? > > > > I have been testing the device memory TCP feature with bnxt_en driver > > and I'm using BCM57508, BCM57608, and BCM57412 NICs. > > (BCM57508's firmware is too old, but BCM57608's firmware is the > > latest, BCM57412 too). > > Currently, I can't test the device memory TCP feature because my NICs > > don't support BNXT_SUPPORTS_QUEUE_API. > > The BCM57608 only supports the BNXT_SUPPORTS_NTUPLE_VNIC, but does not > > support the BNXT_SUPPORTS_QUEUE_API. > > The BCM57412 doesn't support both of them. > > I think at least BCM57508 and BCM57608 should support this because > > it's the same or newer product line as BCM57504 as far as I know. > > Am I missing something? > > The hardware is correct (Thor+) but there needs to be a new FW update > from Broadcom that returns VNIC_QCAPS_RESP_FLAGS_RE_FLUSH_CAP when > queried. Thanks a lot for the reply :) I will try to check the new firmware. Thanks a lot, Taehee Yoo > > CC: Michael Chan <michael.chan@broadcom.com> > > > > > Thanks a lot! > > Taehee Yoo
On Thu, Aug 8, 2024 at 8:42 AM David Wei <dw@davidwei.uk> wrote: > > On 2024-08-08 08:13, Taehee Yoo wrote: > > What Broadcom NICs support BNXT_SUPPORTS_QUEUE_API? > > > > I have been testing the device memory TCP feature with bnxt_en driver > > and I'm using BCM57508, BCM57608, and BCM57412 NICs. > > (BCM57508's firmware is too old, but BCM57608's firmware is the > > latest, BCM57412 too). > > Currently, I can't test the device memory TCP feature because my NICs > > don't support BNXT_SUPPORTS_QUEUE_API. > > The BCM57608 only supports the BNXT_SUPPORTS_NTUPLE_VNIC, but does not > > support the BNXT_SUPPORTS_QUEUE_API. > > The BCM57412 doesn't support both of them. > > I think at least BCM57508 and BCM57608 should support this because > > it's the same or newer product line as BCM57504 as far as I know. > > Am I missing something? > > The hardware is correct (Thor+) but there needs to be a new FW update > from Broadcom that returns VNIC_QCAPS_RESP_FLAGS_RE_FLUSH_CAP when > queried. > BCM5760X and BCM5750X support it but will require a firmware update. I need to check when the new firmware will go into production. The older firmware will work to some extent but our internal testing shows that the chip can sometimes hang when an RX ring is restarted with heavy traffic running.
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 7762fa3b646a..85d4fa8c73ae 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -15717,7 +15717,6 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) dev->stat_ops = &bnxt_stat_ops; dev->watchdog_timeo = BNXT_TX_TIMEOUT; dev->ethtool_ops = &bnxt_ethtool_ops; - dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops; pci_set_drvdata(pdev, dev); rc = bnxt_alloc_hwrm_resources(bp); @@ -15898,6 +15897,8 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) if (BNXT_SUPPORTS_NTUPLE_VNIC(bp)) bp->rss_cap |= BNXT_RSS_CAP_MULTI_RSS_CTX; + if (BNXT_SUPPORTS_QUEUE_API(bp)) + dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops; rc = register_netdev(dev); if (rc) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h index a2233b2d9329..62e637c5be31 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h @@ -2451,6 +2451,9 @@ struct bnxt { #define BNXT_SUPPORTS_MULTI_RSS_CTX(bp) \ (BNXT_PF(bp) && BNXT_SUPPORTS_NTUPLE_VNIC(bp) && \ ((bp)->rss_cap & BNXT_RSS_CAP_MULTI_RSS_CTX)) +#define BNXT_SUPPORTS_QUEUE_API(bp) \ + (BNXT_PF(bp) && BNXT_SUPPORTS_NTUPLE_VNIC(bp) && \ + ((bp)->fw_cap & BNXT_FW_CAP_VNIC_RE_FLUSH)) u32 hwrm_spec_code; u16 hwrm_cmd_seq;
The queue API calls bnxt_hwrm_vnic_update() to stop/start the flow of packets, which can only properly flush the pipeline if FW indicates support. Add a macro BNXT_SUPPORTS_QUEUE_API that checks for the required flags and only set queue_mgmt_ops if true. Signed-off-by: David Wei <dw@davidwei.uk> --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 ++- drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-)