[net-next,v2,0/3] fix bnxt_en queue reset when queue is active

Message

David Wei July 31, 2024, 1:15 p.m. UTC

The current bnxt_en queue API implementation is buggy when resetting a
queue that has active traffic. The problem is that there is no FW
involved to stop the flow of packets and relying on napi_disable() isn't
enough.

To fix this, call bnxt_hwrm_vnic_update() with MRU set to 0 for both the
default and the ntuple vnic to stop the flow of packets. This works for
any Rx queue and not only those that have ntuple rules since every Rx
queue is either in the default or the ntuple vnic.

The first patch is from Michael Chan and adds the prerequisite vnic
functions and definitions.

Tested on BCM957504 while iperf3 is active:

1. Reset a queue that has an ntuple rule steering flow into it
2. Reset all queues in order, one at a time

In both cases the flow is not interrupted.

Sending this to net-next as there is no in-tree kernel consumer of queue
API just yet, and there is a patch that changes when the queue_mgmt_ops
is registered.

---
v2:
 - split setting vnic->mru into a separate patch (Wojciech)
 - clarify why napi_enable()/disable() is removed

David Wei (3):
  bnxt_en: set vnic->mru in bnxt_hwrm_vnic_cfg()
  bnxt_en: stop packet flow during bnxt_queue_stop/start
  bnxt_en: only set dev->queue_mgmt_ops if BNXT_SUPPORTS_NTUPLE_VNIC

Michael Chan (1):
  bnxt_en: Add support to call FW to update a VNIC

 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 50 ++++++++++++++++---
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  3 ++
 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 37 ++++++++++++++
 3 files changed, 83 insertions(+), 7 deletions(-)

Comments

Wojciech Drewek July 31, 2024, 1:33 p.m. UTC | #1

On 31.07.2024 15:15, David Wei wrote:
> The current bnxt_en queue API implementation is buggy when resetting a
> queue that has active traffic. The problem is that there is no FW
> involved to stop the flow of packets and relying on napi_disable() isn't
> enough.
> 
> To fix this, call bnxt_hwrm_vnic_update() with MRU set to 0 for both the
> default and the ntuple vnic to stop the flow of packets. This works for
> any Rx queue and not only those that have ntuple rules since every Rx
> queue is either in the default or the ntuple vnic.
> 
> The first patch is from Michael Chan and adds the prerequisite vnic
> functions and definitions.
> 
> Tested on BCM957504 while iperf3 is active:
> 
> 1. Reset a queue that has an ntuple rule steering flow into it
> 2. Reset all queues in order, one at a time
> 
> In both cases the flow is not interrupted.
> 
> Sending this to net-next as there is no in-tree kernel consumer of queue
> API just yet, and there is a patch that changes when the queue_mgmt_ops
> is registered.
> 
> ---
> v2:
>  - split setting vnic->mru into a separate patch (Wojciech)
>  - clarify why napi_enable()/disable() is removed

Thanks!
For the whole series:
Reviewed-by: Wojciech Drewek <[e-mail]>

Remember to include my tag if next version will be needed :)

> 
> David Wei (3):
>   bnxt_en: set vnic->mru in bnxt_hwrm_vnic_cfg()
>   bnxt_en: stop packet flow during bnxt_queue_stop/start
>   bnxt_en: only set dev->queue_mgmt_ops if BNXT_SUPPORTS_NTUPLE_VNIC
> 
> Michael Chan (1):
>   bnxt_en: Add support to call FW to update a VNIC
> 
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 50 ++++++++++++++++---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  3 ++
>  drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 37 ++++++++++++++
>  3 files changed, 83 insertions(+), 7 deletions(-)
>