1. Patchew
  2. linux
  3. View series

[PATCH v2] wifi: ath11k: Fix invalid ring usage in full monitor mode

Remi Pommarel posted 1 patch 2 months ago 
drivers/net/wireless/ath/ath11k/dp_rx.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
[PATCH v2] wifi: ath11k: Fix invalid ring usage in full monitor mode
Posted by Remi Pommarel 2 months ago 
On full monitor HW the monitor destination rxdma ring does not have the
same descriptor format as in the "classical" mode. The full monitor
destination entries are of hal_sw_monitor_ring type and fetched using
ath11k_dp_full_mon_process_rx while the classical ones are of type
hal_reo_entrance_ring and fetched with ath11k_dp_rx_mon_dest_process.

Although both hal_sw_monitor_ring and hal_reo_entrance_ring are of same
size, the offset to useful info (such as sw_cookie, paddr, etc) are
different. Thus if ath11k_dp_rx_mon_dest_process gets called on full
monitor destination ring, invalid skb buffer id will be fetched from DMA
ring causing issues such as the following rcu_sched stall:

 rcu: INFO: rcu_sched self-detected stall on CPU
 rcu:     0-....: (1 GPs behind) idle=c67/0/0x7 softirq=45768/45769 fqs=1012
  (t=2100 jiffies g=14817 q=8703)
 Task dump for CPU 0:
 task:swapper/0       state:R  running task     stack: 0 pid:    0 ppid:     0 flags:0x0000000a
 Call trace:
  dump_backtrace+0x0/0x160
  show_stack+0x14/0x20
  sched_show_task+0x158/0x184
  dump_cpu_task+0x40/0x4c
  rcu_dump_cpu_stacks+0xec/0x12c
  rcu_sched_clock_irq+0x6c8/0x8a0
  update_process_times+0x88/0xd0
  tick_sched_timer+0x74/0x1e0
  __hrtimer_run_queues+0x150/0x204
  hrtimer_interrupt+0xe4/0x240
  arch_timer_handler_phys+0x30/0x40
  handle_percpu_devid_irq+0x80/0x130
  handle_domain_irq+0x5c/0x90
  gic_handle_irq+0x8c/0xb4
  do_interrupt_handler+0x30/0x54
  el1_interrupt+0x2c/0x4c
  el1h_64_irq_handler+0x14/0x1c
  el1h_64_irq+0x74/0x78
  do_raw_spin_lock+0x60/0x100
  _raw_spin_lock_bh+0x1c/0x2c
  ath11k_dp_rx_mon_mpdu_pop.constprop.0+0x174/0x650
  ath11k_dp_rx_process_mon_status+0x8b4/0xa80
  ath11k_dp_rx_process_mon_rings+0x244/0x510
  ath11k_dp_service_srng+0x190/0x300
  ath11k_pcic_ext_grp_napi_poll+0x30/0xc0
  __napi_poll+0x34/0x174
  net_rx_action+0xf8/0x2a0
  _stext+0x12c/0x2ac
  irq_exit+0x94/0xc0
  handle_domain_irq+0x60/0x90
  gic_handle_irq+0x8c/0xb4
  call_on_irq_stack+0x28/0x44
  do_interrupt_handler+0x4c/0x54
  el1_interrupt+0x2c/0x4c
  el1h_64_irq_handler+0x14/0x1c
  el1h_64_irq+0x74/0x78
  arch_cpu_idle+0x14/0x20
  do_idle+0xf0/0x130
  cpu_startup_entry+0x24/0x50
  rest_init+0xf8/0x104
  arch_call_rest_init+0xc/0x14
  start_kernel+0x56c/0x58c
  __primary_switched+0xa0/0xa8

Thus ath11k_dp_rx_mon_dest_process(), which use classical destination
entry format, should no be called on full monitor capable HW.

Fixes: 67a9d399fcb0 ("ath11k: enable RX PPDU stats in monitor co-exist mode")
Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
v2: set ppdu_status to DP_PPDU_STATUS_DONE as suggested by
    https://lore.kernel.org/ath11k/d376023d-267a-4512-8749-f816fefeb842@quicinc.com/

 drivers/net/wireless/ath/ath11k/dp_rx.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index c087d8a0f5b2..40088e62572e 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -5291,8 +5291,11 @@ int ath11k_dp_rx_process_mon_status(struct ath11k_base *ab, int mac_id,
 		    hal_status == HAL_TLV_STATUS_PPDU_DONE) {
 			rx_mon_stats->status_ppdu_done++;
 			pmon->mon_ppdu_status = DP_PPDU_STATUS_DONE;
-			ath11k_dp_rx_mon_dest_process(ar, mac_id, budget, napi);
-			pmon->mon_ppdu_status = DP_PPDU_STATUS_START;
+			if (!ab->hw_params.full_monitor_mode) {
+				ath11k_dp_rx_mon_dest_process(ar, mac_id,
+							      budget, napi);
+				pmon->mon_ppdu_status = DP_PPDU_STATUS_START;
+			}
 		}
 
 		if (ppdu_info->peer_id == HAL_INVALID_PEERID ||
-- 
2.46.0
Re: [PATCH v2] wifi: ath11k: Fix invalid ring usage in full monitor mode
Posted by Jeff Johnson 1 month, 1 week ago 
On Tue, 24 Sep 2024 21:41:19 +0200, Remi Pommarel wrote:
> On full monitor HW the monitor destination rxdma ring does not have the
> same descriptor format as in the "classical" mode. The full monitor
> destination entries are of hal_sw_monitor_ring type and fetched using
> ath11k_dp_full_mon_process_rx while the classical ones are of type
> hal_reo_entrance_ring and fetched with ath11k_dp_rx_mon_dest_process.
> 
> Although both hal_sw_monitor_ring and hal_reo_entrance_ring are of same
> size, the offset to useful info (such as sw_cookie, paddr, etc) are
> different. Thus if ath11k_dp_rx_mon_dest_process gets called on full
> monitor destination ring, invalid skb buffer id will be fetched from DMA
> ring causing issues such as the following rcu_sched stall:
> 
> [...]

Applied, thanks!

[1/1] wifi: ath11k: Fix invalid ring usage in full monitor mode
      commit: befd716ed429b26eca7abde95da6195c548470de

Best regards,
-- 
Jeff Johnson <quic_jjohnson@quicinc.com>
Re: [PATCH v2] wifi: ath11k: Fix invalid ring usage in full monitor mode
Posted by Praneesh P 1 month, 2 weeks ago 

On 9/25/2024 1:11 AM, Remi Pommarel wrote:
> On full monitor HW the monitor destination rxdma ring does not have the
> same descriptor format as in the "classical" mode. The full monitor
> destination entries are of hal_sw_monitor_ring type and fetched using
> ath11k_dp_full_mon_process_rx while the classical ones are of type
> hal_reo_entrance_ring and fetched with ath11k_dp_rx_mon_dest_process.
> 
> Although both hal_sw_monitor_ring and hal_reo_entrance_ring are of same
> size, the offset to useful info (such as sw_cookie, paddr, etc) are
> different. Thus if ath11k_dp_rx_mon_dest_process gets called on full
> monitor destination ring, invalid skb buffer id will be fetched from DMA
> ring causing issues such as the following rcu_sched stall:
> 
>   rcu: INFO: rcu_sched self-detected stall on CPU
>   rcu:     0-....: (1 GPs behind) idle=c67/0/0x7 softirq=45768/45769 fqs=1012
>    (t=2100 jiffies g=14817 q=8703)
>   Task dump for CPU 0:
>   task:swapper/0       state:R  running task     stack: 0 pid:    0 ppid:     0 flags:0x0000000a
>   Call trace:
>    dump_backtrace+0x0/0x160
>    show_stack+0x14/0x20
>    sched_show_task+0x158/0x184
>    dump_cpu_task+0x40/0x4c
>    rcu_dump_cpu_stacks+0xec/0x12c
>    rcu_sched_clock_irq+0x6c8/0x8a0
>    update_process_times+0x88/0xd0
>    tick_sched_timer+0x74/0x1e0
>    __hrtimer_run_queues+0x150/0x204
>    hrtimer_interrupt+0xe4/0x240
>    arch_timer_handler_phys+0x30/0x40
>    handle_percpu_devid_irq+0x80/0x130
>    handle_domain_irq+0x5c/0x90
>    gic_handle_irq+0x8c/0xb4
>    do_interrupt_handler+0x30/0x54
>    el1_interrupt+0x2c/0x4c
>    el1h_64_irq_handler+0x14/0x1c
>    el1h_64_irq+0x74/0x78
>    do_raw_spin_lock+0x60/0x100
>    _raw_spin_lock_bh+0x1c/0x2c
>    ath11k_dp_rx_mon_mpdu_pop.constprop.0+0x174/0x650
>    ath11k_dp_rx_process_mon_status+0x8b4/0xa80
>    ath11k_dp_rx_process_mon_rings+0x244/0x510
>    ath11k_dp_service_srng+0x190/0x300
>    ath11k_pcic_ext_grp_napi_poll+0x30/0xc0
>    __napi_poll+0x34/0x174
>    net_rx_action+0xf8/0x2a0
>    _stext+0x12c/0x2ac
>    irq_exit+0x94/0xc0
>    handle_domain_irq+0x60/0x90
>    gic_handle_irq+0x8c/0xb4
>    call_on_irq_stack+0x28/0x44
>    do_interrupt_handler+0x4c/0x54
>    el1_interrupt+0x2c/0x4c
>    el1h_64_irq_handler+0x14/0x1c
>    el1h_64_irq+0x74/0x78
>    arch_cpu_idle+0x14/0x20
>    do_idle+0xf0/0x130
>    cpu_startup_entry+0x24/0x50
>    rest_init+0xf8/0x104
>    arch_call_rest_init+0xc/0x14
>    start_kernel+0x56c/0x58c
>    __primary_switched+0xa0/0xa8
> 
> Thus ath11k_dp_rx_mon_dest_process(), which use classical destination
> entry format, should no be called on full monitor capable HW.
>
Thanks, it looks good to me.
Reviewed-by: Praneesh P <quic_ppranees@quicinc.com>
> Fixes: 67a9d399fcb0 ("ath11k: enable RX PPDU stats in monitor co-exist mode")
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
> v2: set ppdu_status to DP_PPDU_STATUS_DONE as suggested by
>      https://lore.kernel.org/ath11k/d376023d-267a-4512-8749-f816fefeb842@quicinc.com/
> 
>   drivers/net/wireless/ath/ath11k/dp_rx.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
> index c087d8a0f5b2..40088e62572e 100644
> --- a/drivers/net/wireless/ath/ath11k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
> @@ -5291,8 +5291,11 @@ int ath11k_dp_rx_process_mon_status(struct ath11k_base *ab, int mac_id,
>   		    hal_status == HAL_TLV_STATUS_PPDU_DONE) {
>   			rx_mon_stats->status_ppdu_done++;
>   			pmon->mon_ppdu_status = DP_PPDU_STATUS_DONE;
> -			ath11k_dp_rx_mon_dest_process(ar, mac_id, budget, napi);
> -			pmon->mon_ppdu_status = DP_PPDU_STATUS_START;
> +			if (!ab->hw_params.full_monitor_mode) {
> +				ath11k_dp_rx_mon_dest_process(ar, mac_id,
> +							      budget, napi);
> +				pmon->mon_ppdu_status = DP_PPDU_STATUS_START;
> +			}
>   		}
>   
>   		if (ppdu_info->peer_id == HAL_INVALID_PEERID ||

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.