Some more patches for wcn36xx
cover Some more patches for wcn36xx
Here are some more patches for the wcn36xx driver that emerged from reviews and my attempts to fix the connection timeout issues. Some of them merely bring the driver in sync with what the "prima" downstream driver is doing, and they seemingly make the driver more stable. Others just apply some best practice patterns or do some cleanup.
The first patch in the series is one that I've sent earlier as RFC, and Loic expressed his support in a follow-up. I still believe it does the right thing.
However, the connection timeout problems are still there unfortunately, but they seemingly appear to happen less often now.
To recap, when the driver gets stuck, not a single packet is being sent out anymore. I have a 2nd machine tuned to the same channel to capture all packets from the MAC of the device under test, and it shows a flat line once the connection timeouts happen. Some more context is given in this bug report:
https://bugs.96boards.org/show_bug.cgi?id=538
One interesting find is that setting the debug_mask module parameter to 0x4 or 0x400 (or both), the issue is much less likely to happen. All this does is adding printk()s before each message is sent out, and that affects the timings of course. I tried adding mdelay() there instead, and they too make the effect less likely, but they don't make it go away.
Hence I believe that some sort of firmware internal buffer is overrun if too many SMD requests fly in in a short amount of time. The firmware does, however, still ack all packets just fine on the SMD channels, and also the DXE communication flows are all healthy. No errors are reported anywhere, but nothing is being put on the ether anymore.
I'm running out of ideas at this point. I guess without access to the firmware sources, it's virtually impossible to grok what's going on and how to work around whatever is causing it.
If anyone has an idea on how to debug this frustrating issue any further, please let me know. A reproducer is described in the bug reported linked to above.
Thanks,
Daniel
Daniel Mack (10):
wcn36xx: fix buffer commit logic on TX path
wcn36xx: set DMA mask explicitly
wcn36xx: don't disable RX IRQ from handler
wcn36xx: clear all masks in RX interrupt
wcn36xx: only handle packets when ED or DONE bit is set
wcn36xx: consider CTRL_EOP bit when looking for valid descriptors
wcn36xx: set PREASSOC and IDLE stated when BSS info changes
wcn36xx: drain pending indicator messages on shutdown
wcn36xx: simplify wcn36xx_smd_open()
wcn36xx: improve debug and error messages for SMD
drivers/net/wireless/ath/wcn36xx/dxe.c | 176 ++++++++++++++++++++------------ drivers/net/wireless/ath/wcn36xx/main.c | 10 ++ drivers/net/wireless/ath/wcn36xx/smd.c | 32 +++--- 3 files changed, 137 insertions(+), 81 deletions(-)
Signature · 1 line
2.14.3
PATCH 1/10 wcn36xx: fix buffer commit logic on TX path
When wcn36xx_dxe_tx_frame() is entered while the device is still processing the queue asyncronously, we are racing against the firmware code with updates to the buffer descriptors. Presumably, the firmware scans the ring buffer that holds the descriptors and scans for a valid control descriptor, and then assumes that the next descriptor contains the payload. If, however, the control descriptor is marked valid, but the payload descriptor isn't, the packet is not sent out.
Another issue with the current code is that is lacks memory barriers before descriptors are marked valid. This is important because the CPU may reorder writes to memory, even if it is allocated as coherent DMA area, and hence the device may see incompletely written data.
To fix this, the code in wcn36xx_dxe_tx_frame() was restructured a bit so that the payload descriptor is made valid before the control descriptor. Memory barriers are added to ensure coherency of shared memory areas.
--- drivers/net/wireless/ath/wcn36xx/dxe.c | 75 +++++++++++++++++----------------- 1 file changed, 38 insertions(+), 37 deletions(-)
drivers/net/wireless/ath/wcn36xx/dxe.c +38 -37
@@ -642,8 +642,8 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn, struct sk_buff *skb, bool is_low) { -struct wcn36xx_dxe_ctl *ctl =NULL ; -struct wcn36xx_dxe_desc *desc =NULL ; +struct wcn36xx_dxe_desc *desc_bd, *desc_skb; +struct wcn36xx_dxe_ctl *ctl_bd, *ctl_skb;struct wcn36xx_dxe_ch *ch =NULL ;unsigned long flags;int ret;
@@ -651,74 +651,75 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn, ch = is_low ? &wcn->dxe_tx_l_ch : &wcn->dxe_tx_h_ch;spin_lock_irqsave (&ch->lock, flags); - ctl = ch->head_blk_ctl; + ctl_bd = ch->head_blk_ctl; + ctl_skb = ctl_bd->next;/* * If skb is not null that means that we reached the tail of the ring * hence ring is full. Stop queues to let mac80211 back off until ring * has an empty slot again. */ -if (NULL != ctl->next->skb) { +if (NULL != ctl_skb->skb) {ieee80211_stop_queues (wcn->hw); wcn->queues_stopped = true;spin_unlock_irqrestore (&ch->lock, flags);return -EBUSY ; } - ctl->skb =NULL ; - desc = ctl->desc; +if (unlikely (ctl_skb->bd_cpu_addr)) { +wcn36xx_err ("bd_cpu_addr cannot be NULL for skb DXE\n" ); + ret = -EINVAL ; + goto unlock; + } + + desc_bd = ctl_bd->desc; + desc_skb = ctl_skb->desc; + + ctl_bd->skb =NULL ;/* write buffer descriptor */ -memcpy (ctl->bd_cpu_addr, bd,sizeof (*bd)); +memcpy (ctl_bd->bd_cpu_addr, bd,sizeof (*bd));/* Set source address of the BD we send */ - desc->src_addr_l = ctl->bd_phy_addr; - - desc->dst_addr_l = ch->dxe_wq; - desc->fr_len =sizeof (struct wcn36xx_tx_bd ); - desc->ctrl = ch->ctrl_bd; + desc_bd->src_addr_l = ctl_bd->bd_phy_addr; + desc_bd->dst_addr_l = ch->dxe_wq; + desc_bd->fr_len =sizeof (struct wcn36xx_tx_bd );wcn36xx_dbg (WCN36XX_DBG_DXE ,"DXE TX\n" );wcn36xx_dbg_dump (WCN36XX_DBG_DXE_DUMP ,"DESC1 >>> " , - (char *)desc,sizeof (*desc)); + (char *)desc_bd,sizeof (*desc_bd));wcn36xx_dbg_dump (WCN36XX_DBG_DXE_DUMP , -"BD >>> " , (char *)ctl->bd_cpu_addr, +"BD >>> " , (char *)ctl_bd->bd_cpu_addr,sizeof (struct wcn36xx_tx_bd )); -/* Set source address of the SKB we send */ - ctl = ctl->next; - desc = ctl->desc; -if (ctl->bd_cpu_addr) { -wcn36xx_err ("bd_cpu_addr cannot be NULL for skb DXE\n" ); - ret = -EINVAL ; - goto unlock; - } - - desc->src_addr_l =dma_map_single (wcn->dev, - skb->data, - skb->len, -DMA_TO_DEVICE ); -if (dma_mapping_error (wcn->dev, desc->src_addr_l)) { + desc_skb->src_addr_l =dma_map_single (wcn->dev, + skb->data, + skb->len, +DMA_TO_DEVICE ); +if (dma_mapping_error (wcn->dev, desc_skb->src_addr_l)) {dev_err (wcn->dev,"unable to DMA map src_addr_l\n" ); ret = -ENOMEM ; goto unlock; } - ctl->skb = skb; - desc->dst_addr_l = ch->dxe_wq; - desc->fr_len = ctl->skb->len; - -/* set dxe descriptor to VALID */ - desc->ctrl = ch->ctrl_skb; + ctl_skb->skb = skb; + desc_skb->dst_addr_l = ch->dxe_wq; + desc_skb->fr_len = ctl_skb->skb->len;wcn36xx_dbg_dump (WCN36XX_DBG_DXE_DUMP ,"DESC2 >>> " , - (char *)desc,sizeof (*desc)); + (char *)desc_skb,sizeof (*desc_skb));wcn36xx_dbg_dump (WCN36XX_DBG_DXE_DUMP ,"SKB >>> " , - (char *)ctl->skb->data, ctl->skb->len); + (char *)ctl_skb->skb->data, ctl_skb->skb->len);/* Move the head of the ring to the next empty descriptor */ - ch->head_blk_ctl = ctl->next; + ch->head_blk_ctl = ctl_skb->next; + +/* Commit all previous writes and set descriptors to VALID */ +wmb (); + desc_skb->ctrl = ch->ctrl_skb; +wmb (); + desc_bd->ctrl = ch->ctrl_bd;/* * When connected and trying to send data frame chip can be in sleep
Signature · 1 line
2.14.3
Re: [PATCH 01/10] wcn36xx: fix buffer commit logic on TX path
Quoted 152 lines · Daniel Mack
When
wcn36xx_dxe_tx_frame()is entered while the device is still processing the queue asyncronously, we are racing against the firmware code with updates to the buffer descriptors. Presumably, the firmware scans the ring buffer that holds the descriptors and scans for a valid control descriptor, and then assumes that the next descriptor contains the payload. If, however, the control descriptor is marked valid, but the payload descriptor isn't, the packet is not sent out.Another issue with the current code is that is lacks memory barriers before descriptors are marked valid. This is important because the CPU may reorder writes to memory, even if it is allocated as coherent DMA area, and hence the device may see incompletely written data.
To fix this, the code in
wcn36xx_dxe_tx_frame()was restructured a bit so that the payload descriptor is made valid before the control descriptor. Memory barriers are added to ensure coherency of shared memory areas.Signed-off-by Daniel Mack <[email protected]>--- drivers/net/wireless/ath/wcn36xx/dxe.c | 75 +++++++++++++++++----------------- 1 file changed, 38 insertions(+), 37 deletions(-)drivers/net/wireless/ath/wcn36xx/dxe.c +38
-38@@ -642,8 +642,8 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn, struct sk_buff *skb, bool is_low) { -struct wcn36xx_dxe_ctl *ctl =NULL ; -struct wcn36xx_dxe_desc *desc =NULL ; +struct wcn36xx_dxe_desc *desc_bd, *desc_skb; +struct wcn36xx_dxe_ctl *ctl_bd, *ctl_skb;struct wcn36xx_dxe_ch *ch =NULL ;unsigned long flags;int ret;@@ -651,74 +651,75 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn, ch = is_low ? &wcn->dxe_tx_l_ch : &wcn->dxe_tx_h_ch;spin_lock_irqsave (&ch->lock, flags); - ctl = ch->head_blk_ctl; + ctl_bd = ch->head_blk_ctl; + ctl_skb = ctl_bd->next;/* * If skb is not null that means that we reached the tail of the ring * hence ring is full. Stop queues to let mac80211 back off until ring * has an empty slot again. */ -if (NULL != ctl->next->skb) { +if (NULL != ctl_skb->skb) {ieee80211_stop_queues (wcn->hw); wcn->queues_stopped = true;spin_unlock_irqrestore (&ch->lock, flags);return -EBUSY ; } - ctl->skb =NULL ; - desc = ctl->desc; +if (unlikely (ctl_skb->bd_cpu_addr)) { +wcn36xx_err ("bd_cpu_addr cannot be NULL for skb DXE\n" ); + ret = -EINVAL ; + goto unlock; + } + + desc_bd = ctl_bd->desc; + desc_skb = ctl_skb->desc; + + ctl_bd->skb =NULL ;/* write buffer descriptor */ -memcpy (ctl->bd_cpu_addr, bd,sizeof (*bd)); +memcpy (ctl_bd->bd_cpu_addr, bd,sizeof (*bd));/* Set source address of the BD we send */ - desc->src_addr_l = ctl->bd_phy_addr; - - desc->dst_addr_l = ch->dxe_wq; - desc->fr_len =sizeof (struct wcn36xx_tx_bd ); - desc->ctrl = ch->ctrl_bd; + desc_bd->src_addr_l = ctl_bd->bd_phy_addr; + desc_bd->dst_addr_l = ch->dxe_wq; + desc_bd->fr_len =sizeof (struct wcn36xx_tx_bd );wcn36xx_dbg (WCN36XX_DBG_DXE ,"DXE TX\n" );wcn36xx_dbg_dump (WCN36XX_DBG_DXE_DUMP ,"DESC1 >>> " , - (char *)desc,sizeof (*desc)); + (char *)desc_bd,sizeof (*desc_bd));wcn36xx_dbg_dump (WCN36XX_DBG_DXE_DUMP , -"BD >>> " , (char *)ctl->bd_cpu_addr, +"BD >>> " , (char *)ctl_bd->bd_cpu_addr,sizeof (struct wcn36xx_tx_bd )); -/* Set source address of the SKB we send */ - ctl = ctl->next; - desc = ctl->desc; -if (ctl->bd_cpu_addr) { -wcn36xx_err ("bd_cpu_addr cannot be NULL for skb DXE\n" ); - ret = -EINVAL ; - goto unlock; - } - - desc->src_addr_l =dma_map_single (wcn->dev, - skb->data, - skb->len, -DMA_TO_DEVICE ); -if (dma_mapping_error (wcn->dev, desc->src_addr_l)) { + desc_skb->src_addr_l =dma_map_single (wcn->dev, + skb->data, + skb->len, +DMA_TO_DEVICE ); +if (dma_mapping_error (wcn->dev, desc_skb->src_addr_l)) {dev_err (wcn->dev,"unable to DMA map src_addr_l\n" ); ret = -ENOMEM ; goto unlock; } - ctl->skb = skb; - desc->dst_addr_l = ch->dxe_wq; - desc->fr_len = ctl->skb->len; - -/* set dxe descriptor to VALID */ - desc->ctrl = ch->ctrl_skb; + ctl_skb->skb = skb; + desc_skb->dst_addr_l = ch->dxe_wq; + desc_skb->fr_len = ctl_skb->skb->len;wcn36xx_dbg_dump (WCN36XX_DBG_DXE_DUMP ,"DESC2 >>> " , - (char *)desc,sizeof (*desc)); + (char *)desc_skb,sizeof (*desc_skb));wcn36xx_dbg_dump (WCN36XX_DBG_DXE_DUMP ,"SKB >>> " , - (char *)ctl->skb->data, ctl->skb->len); + (char *)ctl_skb->skb->data, ctl_skb->skb->len);/* Move the head of the ring to the next empty descriptor */ - ch->head_blk_ctl = ctl->next; + ch->head_blk_ctl = ctl_skb->next; + +/* Commit all previous writes and set descriptors to VALID */ +wmb (); + desc_skb->ctrl = ch->ctrl_skb; +wmb (); + desc_bd->ctrl = ch->ctrl_bd;/* * When connected and trying to send data frame chip can be in sleep -- 2.14.3
Quoted 4 lines
_______________________________________________
wcn36xx mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/wcn36xx
Re: [01/10] wcn36xx: fix buffer commit logic on TX path
In reply to Daniel MackQuoted 13 lines · Daniel Mack
When
wcn36xx_dxe_tx_frame()is entered while the device is still processing the queue asyncronously, we are racing against the firmware code with updates to the buffer descriptors. Presumably, the firmware scans the ring buffer that holds the descriptors and scans for a valid control descriptor, and then assumes that the next descriptor contains the payload. If, however, the control descriptor is marked valid, but the payload descriptor isn't, the packet is not sent out.Another issue with the current code is that is lacks memory barriers before descriptors are marked valid. This is important because the CPU may reorder writes to memory, even if it is allocated as coherent DMA area, and hence the device may see incompletely written data.
Quoted 6 lines
To fix this, the code in
wcn36xx_dxe_tx_frame()was restructured a bit so that the payload descriptor is made valid before the control descriptor. Memory barriers are added to ensure coherency of shared memory areas.Signed-off-by Daniel Mack <[email protected]>Signed-off-by Kalle Valo <[email protected]>
10 patches applied to ath-next branch of ath.git, thanks.
9a81cc23dfef wcn36xx: fix buffer commit logic on TX path 57e06e0e2a86 wcn36xx: set DMA mask explicitlyba437e72378c wcn36xx: don't disable RX IRQ from handler edd23ab403cf wcn36xx: clear all masks in RX interrupt ce1d4be82b10 wcn36xx: only handle packets when ED or DONE bit is set 18c7ed138824 wcn36xx: consider CTRL_EOP bit when looking for valid descriptors 2a46c829a926 wcn36xx: set PREASSOC and IDLE stated when BSS info changes 773f9a28bcda wcn36xx: drain pending indicator messages on shutdown a50c6c8412f7 wcn36xx: simplify wcn36xx_smd_open()ffbc9197b472 wcn36xx: improve debug and error messages for SMD
Signature · 3 lines
https://patchwork.kernel.org/patch/10404031/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
PATCH 2/10 wcn36xx: set DMA mask explicitly
In reply to Daniel MackThe device takes 32-bit addresses only, so inform the DMA API about it. This is the default on msm8016, so that doesn't change anything, but it's best practice to be explicit.
--- drivers/net/wireless/ath/wcn36xx/main.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/net/wireless/ath/wcn36xx/main.c +6 -0
@@ -1309,6 +1309,12 @@ static int wcn36xx_probe(struct platform_device *pdev)mutex_init (&wcn->hal_mutex);mutex_init (&wcn->scan_lock); + ret =dma_set_mask_and_coherent (wcn->dev,DMA_BIT_MASK (32 )); +if (ret <0 ) { +wcn36xx_err ("failed to set DMA mask: %d\n" , ret); + goto out_wq; + } +INIT_WORK (&wcn->scan_work, wcn36xx_hw_scan_worker); wcn->smd_channel =qcom_wcnss_open_channel (wcnss,"WLAN_CTRL" , wcn36xx_smd_rsp_process, hw);
Signature · 1 line
2.14.3
Re: [PATCH 02/10] wcn36xx: set DMA mask explicitly
Quoted 30 lines · Daniel Mack
The device takes 32-bit addresses only, so inform the DMA API about it. This is the default on msm8016, so that doesn't change anything, but it's best practice to be explicit.
Signed-off-by Daniel Mack <[email protected]>--- drivers/net/wireless/ath/wcn36xx/main.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/net/wireless/ath/wcn36xx/main.c +6
-1@@ -1309,6 +1309,12 @@ static int wcn36xx_probe(struct platform_device *pdev)mutex_init (&wcn->hal_mutex);mutex_init (&wcn->scan_lock); + ret =dma_set_mask_and_coherent (wcn->dev,DMA_BIT_MASK (32 )); +if (ret <0 ) { +wcn36xx_err ("failed to set DMA mask: %d\n" , ret); + goto out_wq; + } +INIT_WORK (&wcn->scan_work, wcn36xx_hw_scan_worker); wcn->smd_channel =qcom_wcnss_open_channel (wcnss,"WLAN_CTRL" , wcn36xx_smd_rsp_process, hw); --2.14.3
Quoted 4 lines
_______________________________________________
wcn36xx mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/wcn36xx
PATCH 3/10 wcn36xx: don't disable RX IRQ from handler
In reply to Daniel MackThere's no need to disable the IRQ from inside its handler. Instead just grab the spinlock of the channel that is being processed.
--- drivers/net/wireless/ath/wcn36xx/dxe.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
drivers/net/wireless/ath/wcn36xx/dxe.c +11 -4
@@ -476,9 +476,8 @@ static irqreturn_t wcn36xx_irq_rx_ready(int irq, void *dev) {struct wcn36xx *wcn = (struct wcn36xx *)dev; -disable_irq_nosync (wcn->rx_irq);wcn36xx_dxe_rx_frame (wcn); -enable_irq (wcn->rx_irq); +return IRQ_HANDLED ; }
@@ -514,8 +513,8 @@ static int wcn36xx_dxe_request_irqs(struct wcn36xx *wcn)static int wcn36xx_rx_handle_packets (struct wcn36xx *wcn,struct wcn36xx_dxe_ch *ch) { -struct wcn36xx_dxe_ctl *ctl = ch->head_blk_ctl; -struct wcn36xx_dxe_desc *dxe = ctl->desc; +struct wcn36xx_dxe_desc *dxe; +struct wcn36xx_dxe_ctl *ctl;dma_addr_t dma_addr;struct sk_buff *skb;int ret =0 , int_mask;
@@ -529,6 +528,11 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn, int_mask =WCN36XX_DXE_INT_CH3_MASK ; } +spin_lock (&ch->lock); + + ctl = ch->head_blk_ctl; + dxe = ctl->desc; +while (!(READ_ONCE (dxe->ctrl) & WCN36xx_DXE_CTRL_VLD)) { skb = ctl->skb; dma_addr = dxe->dst_addr_l;
@@ -549,6 +553,9 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn,wcn36xx_dxe_write_register (wcn,WCN36XX_DXE_ENCH_ADDR , int_mask); ch->head_blk_ctl = ctl; + +spin_unlock (&ch->lock); +return 0 ; }
Signature · 1 line
2.14.3
PATCH 4/10 wcn36xx: clear all masks in RX interrupt
In reply to Daniel MackLike on the TX side, check for the interrupt reason when the RX interrupt is latched and clear the ERR, DONE and ED masks.
This seems to help with connection timeouts and network stream starvatations. And FWIW, the downstream driver does the same thing.
Note that in analogy to the TX side, WCN36XX_DXE_0_INT_CLR should be set to WCN36XX_INT_MASK_CHAN_RX_{L,H} rather than WCN36XX_DXE_INT_CH{1,3}_MASK. It did the right thing however, as the defines happen to have identical values.
Also, instead of determining register addresses and values inside wcn36xx_rx_handle_packets(), pass them as arguments.
--- drivers/net/wireless/ath/wcn36xx/dxe.c | 62 ++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 22 deletions(-)
drivers/net/wireless/ath/wcn36xx/dxe.c +40 -22
@@ -511,23 +511,40 @@ static int wcn36xx_dxe_request_irqs(struct wcn36xx *wcn) }static int wcn36xx_rx_handle_packets (struct wcn36xx *wcn, -struct wcn36xx_dxe_ch *ch) +struct wcn36xx_dxe_ch *ch, +u32 ctrl, +u32 en_mask, +u32 int_mask, +u32 status_reg) {struct wcn36xx_dxe_desc *dxe;struct wcn36xx_dxe_ctl *ctl;dma_addr_t dma_addr;struct sk_buff *skb; -int ret =0 , int_mask; -u32 value; +u32 int_reason; +int ret; -if (ch->ch_type ==WCN36XX_DXE_CH_RX_L ) { - value =WCN36XX_DXE_CTRL_RX_L ; - int_mask =WCN36XX_DXE_INT_CH1_MASK ; - }else { - value =WCN36XX_DXE_CTRL_RX_H ; - int_mask =WCN36XX_DXE_INT_CH3_MASK ; +wcn36xx_dxe_read_register (wcn, status_reg, &int_reason); +wcn36xx_dxe_write_register (wcn,WCN36XX_DXE_0_INT_CLR , int_mask); + +if (int_reason &WCN36XX_CH_STAT_INT_ERR_MASK ) { +wcn36xx_dxe_write_register (wcn, +WCN36XX_DXE_0_INT_ERR_CLR , + int_mask); + +wcn36xx_err ("DXE IRQ reported error on RX channel\n" ); } +if (int_reason &WCN36XX_CH_STAT_INT_DONE_MASK ) +wcn36xx_dxe_write_register (wcn, +WCN36XX_DXE_0_INT_DONE_CLR , + int_mask); + +if (int_reason &WCN36XX_CH_STAT_INT_ED_MASK ) +wcn36xx_dxe_write_register (wcn, +WCN36XX_DXE_0_INT_ED_CLR , + int_mask); +spin_lock (&ch->lock); ctl = ch->head_blk_ctl;
@@ -546,11 +563,11 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn,wcn36xx_rx_skb (wcn, skb); }/* else keep old skb not submitted and use it for rx DMA */ - dxe->ctrl = value; + dxe->ctrl = ctrl; ctl = ctl->next; dxe = ctl->desc; } -wcn36xx_dxe_write_register (wcn,WCN36XX_DXE_ENCH_ADDR , int_mask); +wcn36xx_dxe_write_register (wcn,WCN36XX_DXE_ENCH_ADDR , en_mask); ch->head_blk_ctl = ctl;
@@ -566,19 +583,20 @@ void wcn36xx_dxe_rx_frame(struct wcn36xx *wcn)wcn36xx_dxe_read_register (wcn,WCN36XX_DXE_INT_SRC_RAW_REG , &int_src);/* RX_LOW_PRI */ -if (int_src &WCN36XX_DXE_INT_CH1_MASK ) { -wcn36xx_dxe_write_register (wcn,WCN36XX_DXE_0_INT_CLR , -WCN36XX_DXE_INT_CH1_MASK ); -wcn36xx_rx_handle_packets (wcn, &(wcn->dxe_rx_l_ch)); - } +if (int_src &WCN36XX_DXE_INT_CH1_MASK ) +wcn36xx_rx_handle_packets (wcn, &wcn->dxe_rx_l_ch, +WCN36XX_DXE_CTRL_RX_L , +WCN36XX_DXE_INT_CH1_MASK , +WCN36XX_INT_MASK_CHAN_RX_L , +WCN36XX_DXE_CH_STATUS_REG_ADDR_RX_L );/* RX_HIGH_PRI */ -if (int_src &WCN36XX_DXE_INT_CH3_MASK ) { -/* Clean up all the INT within this channel */ -wcn36xx_dxe_write_register (wcn,WCN36XX_DXE_0_INT_CLR , -WCN36XX_DXE_INT_CH3_MASK ); -wcn36xx_rx_handle_packets (wcn, &(wcn->dxe_rx_h_ch)); - } +if (int_src &WCN36XX_DXE_INT_CH3_MASK ) +wcn36xx_rx_handle_packets (wcn, &wcn->dxe_rx_h_ch, +WCN36XX_DXE_CTRL_RX_H , +WCN36XX_DXE_INT_CH3_MASK , +WCN36XX_INT_MASK_CHAN_RX_H , +WCN36XX_DXE_CH_STATUS_REG_ADDR_RX_H );if (!int_src)wcn36xx_warn ("No DXE interrupt pending\n" );
Signature · 1 line
2.14.3
PATCH 5/10 wcn36xx: only handle packets when ED or DONE bit is set
In reply to Daniel MackOn RX and TX interrupts, check for the WCN36XX_CH_STAT_INT_ED_MASK or WCN36XX_CH_STAT_INT_DONE_MASK in the interrupt reason register, and only handle packets when it is set. This way, reap_tx_dxes() is only invoked when needed.
This brings the dequeing logic in line with what the prima downstream driver is doing.
While at it, also log the interrupt reason.
--- drivers/net/wireless/ath/wcn36xx/dxe.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
drivers/net/wireless/ath/wcn36xx/dxe.c +16 -4
@@ -430,8 +430,12 @@ static irqreturn_t wcn36xx_irq_tx_complete(int irq, void *dev) WCN36XX_INT_MASK_CHAN_TX_H); } -wcn36xx_dbg (WCN36XX_DBG_DXE ,"dxe tx ready high\n" ); -reap_tx_dxes (wcn, &wcn->dxe_tx_h_ch); +wcn36xx_dbg (WCN36XX_DBG_DXE ,"dxe tx ready high, reason %08x\n" , + int_reason); + +if (int_reason & (WCN36XX_CH_STAT_INT_DONE_MASK | +WCN36XX_CH_STAT_INT_ED_MASK )) +reap_tx_dxes (wcn, &wcn->dxe_tx_h_ch); }if (int_src &WCN36XX_INT_MASK_CHAN_TX_L ) {
@@ -465,8 +469,12 @@ static irqreturn_t wcn36xx_irq_tx_complete(int irq, void *dev) WCN36XX_INT_MASK_CHAN_TX_L); } -wcn36xx_dbg (WCN36XX_DBG_DXE ,"dxe tx ready low\n" ); -reap_tx_dxes (wcn, &wcn->dxe_tx_l_ch); +wcn36xx_dbg (WCN36XX_DBG_DXE ,"dxe tx ready low, reason %08x\n" , + int_reason); + +if (int_reason & (WCN36XX_CH_STAT_INT_DONE_MASK | +WCN36XX_CH_STAT_INT_ED_MASK )) +reap_tx_dxes (wcn, &wcn->dxe_tx_l_ch); }return IRQ_HANDLED ;
@@ -545,6 +553,10 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn,WCN36XX_DXE_0_INT_ED_CLR , int_mask); +if (!(int_reason & (WCN36XX_CH_STAT_INT_DONE_MASK | +WCN36XX_CH_STAT_INT_ED_MASK ))) +return 0 ; +spin_lock (&ch->lock); ctl = ch->head_blk_ctl;
Signature · 1 line
2.14.3
PATCH 6/10 wcn36xx: consider CTRL_EOP bit when looking for valid descriptors
In reply to Daniel MackIn reap_tx_dxes(), when we iterate over the linked descriptors, only consider such valid that have WCN36xx_DXE_CTRL_EOP set.
This is what the prima downstream driver is doing as well.
--- drivers/net/wireless/ath/wcn36xx/dxe.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/wireless/ath/wcn36xx/dxe.c +3 -1
@@ -370,7 +370,9 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch) do { if (READ_ONCE(ctl->desc->ctrl) & WCN36xx_DXE_CTRL_VLD) break; - if (ctl->skb) { + + if (ctl->skb && + READ_ONCE(ctl->desc->ctrl) & WCN36xx_DXE_CTRL_EOP) { dma_unmap_single(wcn->dev, ctl->desc->src_addr_l, ctl->skb->len, DMA_TO_DEVICE); info = IEEE80211_SKB_CB(ctl->skb);
Signature · 1 line
2.14.3
PATCH 7/10 wcn36xx: set PREASSOC and IDLE stated when BSS info changes
In reply to Daniel MackWhen a BSSID is joined, set the link status to 'preassoc', and set it to 'idle' when the BSS is deleted.
This is what the downstream driver is doing, and it seems to improve the reliability during connect/disconnect stress tests.
--- drivers/net/wireless/ath/wcn36xx/main.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/net/wireless/ath/wcn36xx/main.c +4 -0
@@ -798,6 +798,8 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw,if (!is_zero_ether_addr (bss_conf->bssid)) { vif_priv->is_joining = true; vif_priv->bss_index =WCN36XX_HAL_BSS_INVALID_IDX ; +wcn36xx_smd_set_link_st (wcn, bss_conf->bssid, vif->addr, +WCN36XX_HAL_LINK_PREASSOC_STATE );wcn36xx_smd_join (wcn, bss_conf->bssid, vif->addr,WCN36XX_HW_CHANNEL (wcn)); wcn36xx_smd_config_bss(wcn, vif, NULL,
@@ -805,6 +807,8 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw, }else { vif_priv->is_joining = false;wcn36xx_smd_delete_bss (wcn, vif); +wcn36xx_smd_set_link_st (wcn, bss_conf->bssid, vif->addr, +WCN36XX_HAL_LINK_IDLE_STATE ); vif_priv->encrypt_type =WCN36XX_HAL_ED_NONE ; } }
Signature · 1 line
2.14.3
PATCH 8/10 wcn36xx: drain pending indicator messages on shutdown
In reply to Daniel MackWhen the interface is shut down, wcn36xx_smd_close() potentially races against the queue worker. Make sure to cancel the work, and then free all the remnants in hal_ind_queue manually.
This is again just a theoretical issue, not something that was triggered in the wild.
--- drivers/net/wireless/ath/wcn36xx/smd.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/net/wireless/ath/wcn36xx/smd.c +6 -0
@@ -2513,5 +2513,11 @@ int wcn36xx_smd_open(struct wcn36xx *wcn)void wcn36xx_smd_close (struct wcn36xx *wcn) { +struct wcn36xx_hal_ind_msg *msg, *tmp; + +cancel_work_sync (&wcn->hal_ind_work);destroy_workqueue (wcn->hal_ind_wq); + +list_for_each_entry_safe (msg, tmp, &wcn->hal_ind_queue, list) +kfree (msg); }
Signature · 1 line
2.14.3
PATCH 9/10 wcn36xx: simplify wcn36xx_smd_open()
In reply to Daniel MackDrop the extra warning about failed allocations, both the core and the only caller of this function will warn loud enough in such cases.
--- drivers/net/wireless/ath/wcn36xx/smd.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
drivers/net/wireless/ath/wcn36xx/smd.c +3 -9
@@ -2494,21 +2494,15 @@ static void wcn36xx_ind_smd_work(struct work_struct *work) }int wcn36xx_smd_open (struct wcn36xx *wcn) { -int ret =0 ; wcn->hal_ind_wq =create_freezable_workqueue ("wcn36xx_smd_ind" ); -if (!wcn->hal_ind_wq) { -wcn36xx_err ("failed to allocate wq\n" ); - ret = -ENOMEM ; - goto out; - } +if (!wcn->hal_ind_wq) +return -ENOMEM ; +INIT_WORK (&wcn->hal_ind_work, wcn36xx_ind_smd_work);INIT_LIST_HEAD (&wcn->hal_ind_queue);spin_lock_init (&wcn->hal_ind_lock);return 0 ; - -out: -return ret; }void wcn36xx_smd_close (struct wcn36xx *wcn)
Signature · 1 line
2.14.3
PATCH 10/10 wcn36xx: improve debug and error messages for SMD
In reply to Daniel MackAdd a missing newline in wcn36xx_smd_send_and_wait() and also log the command request and response type that was processed.
--- drivers/net/wireless/ath/wcn36xx/smd.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
drivers/net/wireless/ath/wcn36xx/smd.c +10 -4
@@ -252,23 +252,29 @@ static int wcn36xx_smd_send_and_wait(struct wcn36xx *wcn, size_t len) {int ret =0 ;unsigned long start; +struct wcn36xx_hal_msg_header *hdr = + (struct wcn36xx_hal_msg_header *)wcn->hal_buf; +u16 req_type = hdr->msg_type; +wcn36xx_dbg_dump (WCN36XX_DBG_SMD_DUMP ,"HAL >>> " , wcn->hal_buf, len);init_completion (&wcn->hal_rsp_compl); start = jiffies; ret =rpmsg_send (wcn->smd_channel, wcn->hal_buf, len);if (ret) { -wcn36xx_err ("HAL TX failed\n" ); +wcn36xx_err ("HAL TX failed for req %d\n" , req_type); goto out; }if (wait_for_completion_timeout (&wcn->hal_rsp_compl,msecs_to_jiffies (HAL_MSG_TIMEOUT )) <=0 ) { -wcn36xx_err ("Timeout! No SMD response in %dms\n" , -HAL_MSG_TIMEOUT ); +wcn36xx_err ("Timeout! No SMD response to req %d in %dms\n" , + req_type,HAL_MSG_TIMEOUT ); ret = -ETIME ; goto out; } -wcn36xx_dbg (WCN36XX_DBG_SMD ,"SMD command completed in %dms" , +wcn36xx_dbg (WCN36XX_DBG_SMD , +"SMD command (req %d, rsp %d) completed in %dms\n" , + req_type, hdr->msg_type,jiffies_to_msecs (jiffies - start)); out:return ret;
Signature · 1 line
2.14.3
Quoted 5 lines · Daniel Mack
Hence I believe that some sort of firmware internal buffer is overrun if too many SMD requests fly in in a short amount of time. The firmware does, however, still ack all packets just fine on the SMD channels, and also the DXE communication flows are all healthy. No errors are reported anywhere, but nothing is being put on the ether anymore.
And FTR, there is a commit in the prima repository that caught my attention a while back:
https://source.codeaurora.org/external/wlan/prima/commit/?id=93cd8f3c
What this does (through an remarkable number of indirection layers) is sending the DUMP_COMMAND_REQ command with args = (274, 0, 0, 0, 0) when management frames get stuck, which smells pretty much like the issue I'm seeing. Doing the same with the mainline driver and the debugfs interface it exposes doesn't have any effect though.
But even if it did work, I wouldn't see a way to detect the situation in which this is needed reliably.
Daniel
Quoted 12 lines · Daniel Mack
Quoted 5 lines · Daniel Mack
Hence I believe that some sort of firmware internal buffer is overrun if too many SMD requests fly in in a short amount of time. The firmware does, however, still ack all packets just fine on the SMD channels, and also the DXE communication flows are all healthy. No errors are reported anywhere, but nothing is being put on the ether anymore.
And FTR, there is a commit in the prima repository that caught my attention a while back:
https://source.codeaurora.org/external/wlan/prima/commit/?id=93cd8f3c
Quoted 8 lines
What this does (through an remarkable number of indirection layers) is sending the
DUMP_COMMAND_REQcommand with args = (274, 0, 0, 0, 0) when management frames get stuck, which smells pretty much like the issue I'm seeing. Doing the same with the mainline driver and the debugfs interface it exposes doesn't have any effect though.But even if it did work, I wouldn't see a way to detect the situation in which this is needed reliably.
The firmware version might make a difference so I recommend always mentioning the firmware version as well. For example, what if your firmware does not support that command or parameter?
Also I would recommend to file a bug to bugzilla.kernel.org so that all the information is one place and it can be easily updated. Now it's pretty difficult to get the big picture from various emails on the list.
Signature · 1 line
Kalle Valo
Hi Kalle,
Quoted 20 lines · Kalle Valo
Quoted 18 lines · Daniel Mack
Quoted 5 lines · Daniel Mack
Hence I believe that some sort of firmware internal buffer is overrun if too many SMD requests fly in in a short amount of time. The firmware does, however, still ack all packets just fine on the SMD channels, and also the DXE communication flows are all healthy. No errors are reported anywhere, but nothing is being put on the ether anymore.
And FTR, there is a commit in the prima repository that caught my attention a while back:
https://source.codeaurora.org/external/wlan/prima/commit/?id=93cd8f3c
What this does (through an remarkable number of indirection layers) is sending the
DUMP_COMMAND_REQcommand with args = (274, 0, 0, 0, 0) when management frames get stuck, which smells pretty much like the issue I'm seeing. Doing the same with the mainline driver and the debugfs interface it exposes doesn't have any effect though.
Quoted 6 lines
Quoted 2 lines
But even if it did work, I wouldn't see a way to detect the situation in which this is needed reliably.
The firmware version might make a difference so I recommend always mentioning the firmware version as well. For example, what if your firmware does not support that command or parameter?
Sure, that could be the case. FTR - the firmware I'm using is the one that came out of the Qualcomm r1034.2.1 BSP. It is recognized by the driver as 'WCN v2.0 RadioPhy vRhea_GF_1.12 with 19.2MHz XO'.
Quoted 3 lines
Also I would recommend to file a bug to bugzilla.kernel.org so that all the information is one place and it can be easily updated. Now it's pretty difficult to get the big picture from various emails on the list.
Yes, I agree it's a bit convoluted. However, there's already the bug report on 96board.org that Bjorn opened some time back, and I considered that sufficient. IMO, it has all the information needed, plus a link to a tool to reproduce the issue.
https://bugs.96boards.org/show_bug.cgi?id=538
The patches in this series can be considered as general cleanups that are not necessarily related to this particular issue.
Thanks,
Daniel
Quoted 24 lines · Daniel Mack
Quoted 23 lines · Kalle Valo
Quoted 20 lines · Daniel Mack
Quoted 5 lines · Daniel Mack
Hence I believe that some sort of firmware internal buffer is overrun if too many SMD requests fly in in a short amount of time. The firmware does, however, still ack all packets just fine on the SMD channels, and also the DXE communication flows are all healthy. No errors are reported anywhere, but nothing is being put on the ether anymore.
And FTR, there is a commit in the prima repository that caught my attention a while back:
https://source.codeaurora.org/external/wlan/prima/commit/?id=93cd8f3c
What this does (through an remarkable number of indirection layers) is sending the
DUMP_COMMAND_REQcommand with args = (274, 0, 0, 0, 0) when management frames get stuck, which smells pretty much like the issue I'm seeing. Doing the same with the mainline driver and the debugfs interface it exposes doesn't have any effect though.But even if it did work, I wouldn't see a way to detect the situation in which this is needed reliably.
Quoted 7 lines
Quoted 3 lines
The firmware version might make a difference so I recommend always mentioning the firmware version as well. For example, what if your firmware does not support that command or parameter?
Sure, that could be the case. FTR - the firmware I'm using is the one that came out of the Qualcomm r1034.2.1 BSP. It is recognized by the driver as 'WCN v2.0 RadioPhy vRhea_GF_1.12 with 19.2MHz XO'.
Ok, thanks. Please add that to the bug report.
Quoted 10 lines
Quoted 3 lines
Also I would recommend to file a bug to bugzilla.kernel.org so that all the information is one place and it can be easily updated. Now it's pretty difficult to get the big picture from various emails on the list.
Yes, I agree it's a bit convoluted. However, there's already the bug report on 96board.org that Bjorn opened some time back, and I considered that sufficient. IMO, it has all the information needed, plus a link to a tool to reproduce the issue.
Yeah, bugs.96boards.org is fine. As long as there's one place which collects all the information about the bug.
But IMHO the bug report is not telling much, all I get is that TX frames get stuck but not even that is confirmed. After reading it I have at least these questions:
* Is it really confirmed that the issue is that TX frames are stuck? For example, using a wireless sniffer would confirm that.
* Are only management frames stuck or does it also involve data frames?
* Based on the bug report the TX stuck issue seems to happen during authentication, but what happens before that? Does wcn36xx get disconnected from AP or what?
* Any wcn36xx logs about the issue (with or without debug logs)? Also matching wpasupplicant logs would help.
* Does this only happen with encryption or also in open mode?
* How long does it take with qconnman-stress to reproduce the issue?
* Does the radio environment make any difference on reproducibility? For example, clear enviroment vs lots of traffic/interference?
Signature · 1 line
Kalle Valo
Hi Kalle,
Quoted 17 lines · Kalle Valo
Quoted 6 lines · Daniel Mack
Quoted 3 lines · Kalle Valo
Also I would recommend to file a bug to bugzilla.kernel.org so that all the information is one place and it can be easily updated. Now it's pretty difficult to get the big picture from various emails on the list.
Quoted 6 lines
Yes, I agree it's a bit convoluted. However, there's already the bug report on 96board.org that Bjorn opened some time back, and I considered that sufficient. IMO, it has all the information needed, plus a link to a tool to reproduce the issue.
https://bugs.96boards.org/show_bug.cgi?id=538Yeah, bugs.96boards.org is fine. As long as there's one place which collects all the information about the bug.
Quoted 6 lines
But IMHO the bug report is not telling much, all I get is that TX frames get stuck but not even that is confirmed. After reading it I have at least these questions:
* Is it really confirmed that the issue is that TX frames are stuck? For example, using a wireless sniffer would confirm that.
Yes, that's confirmed. I have a 2nd machine tuned to the same channel than the network I use for testing, and once the timeouts happen, I cannot see any frame anymore from the MAC of the wcn36xx. No probe requests for scans, no authentication attempts, nothing.
As my test constantly connects and disconnects, the last thing I see in wireshark is a deauthentication frame.
Quoted 1 line
* Are only management frames stuck or does it also involve data frames?
It seems that once a network is successfully joined, the network stability is fine. I haven't seen any starvation of streams lately, at least not with the the patches in this series which I'm running since a while. That is, until a disconnect/reconnect attempt is made, and at this point, only management frames are involved.
Quoted 3 lines
* Based on the bug report the TX stuck issue seems to happen during authentication, but what happens before that? Does wcn36xx get disconnected from AP or what?
As I said, my test setup includes repeated disconnections to make the bug appear. It sometimes happens at the first attempt after a fresh boot, however, so the stress test only makes debugging a bit easier by increasing the likeliness.
Quoted 2 lines
* Any wcn36xx logs about the issue (with or without debug logs)? Also matching wpasupplicant logs would help.
The problem with this is that it's not exactly clear what kind of effect we're looking at. With all the debug flags of the driver enabled, it produces so much log output that wpa_supplicant gives up due to timeouts. The other weird issue is that with WCN36XX_DBG_MAC and/or WCN36XX_DBG_SMD enabled, the effect is _much_ harder to trigger.
Quoted 1 line
* Does this only happen with encryption or also in open mode?
That's a good question. I'll go check with an open network.
Quoted 1 line
* How long does it take with qconnman-stress to reproduce the issue?
Usually less than 10 minutes.
Quoted 2 lines
* Does the radio environment make any difference on reproducibility? For example, clear enviroment vs lots of traffic/interference?
It seems it does, yes. Tests at night seem to take a bit more time to make the effect happen. But then again, it could also be unrelated. I can't be certain at this point.
I'll submit some more information to the bug report. What would help me is if other people tried to reproduce the effect using the stress tool and confirm my findings. Chances are I've been staring at this for too long :)
Thanks again,
Daniel
wcn36xx: bug #538: stuck tx management frames
(I'll change the subject to better reflect what we are discussing.)
Quoted 25 lines · Daniel Mack
Quoted 17 lines · Kalle Valo
Quoted 6 lines · Daniel Mack
Quoted 3 lines · Kalle Valo
Also I would recommend to file a bug to bugzilla.kernel.org so that all the information is one place and it can be easily updated. Now it's pretty difficult to get the big picture from various emails on the list.
Quoted 6 lines
Yes, I agree it's a bit convoluted. However, there's already the bug report on 96board.org that Bjorn opened some time back, and I considered that sufficient. IMO, it has all the information needed, plus a link to a tool to reproduce the issue.
https://bugs.96boards.org/show_bug.cgi?id=538Yeah, bugs.96boards.org is fine. As long as there's one place which collects all the information about the bug.
Quoted 6 lines
But IMHO the bug report is not telling much, all I get is that TX frames get stuck but not even that is confirmed. After reading it I have at least these questions:
* Is it really confirmed that the issue is that TX frames are stuck? For example, using a wireless sniffer would confirm that.
Quoted 7 lines
Yes, that's confirmed. I have a 2nd machine tuned to the same channel than the network I use for testing, and once the timeouts happen, I cannot see any frame anymore from the MAC of the wcn36xx. No probe requests for scans, no authentication attempts, nothing.
As my test constantly connects and disconnects, the last thing I see in wireshark is a deauthentication frame.
Thanks, this is good to know.
Quoted 7 lines
Quoted 1 line
* Are only management frames stuck or does it also involve data frames?
It seems that once a network is successfully joined, the network stability is fine. I haven't seen any starvation of streams lately, at least not with the the patches in this series which I'm running since a while. That is, until a disconnect/reconnect attempt is made, and at this point, only management frames are involved.
Ah, maybe originally you were seeing different issues with similar symptoms? But now you have fixed the other bugsand now the stuck transmitted management frame issue is left? Just guessing...
Quoted 12 lines
Quoted 3 lines
* Based on the bug report the TX stuck issue seems to happen during authentication, but what happens before that? Does wcn36xx get disconnected from AP or what?
As I said, my test setup includes repeated disconnections to make the bug appear. It sometimes happens at the first attempt after a fresh boot, however, so the stress test only makes debugging a bit easier by increasing the likeliness.
Quoted 2 lines
* Any wcn36xx logs about the issue (with or without debug logs)? Also matching wpasupplicant logs would help.
Quoted 6 lines
The problem with this is that it's not exactly clear what kind of effect we're looking at. With all the debug flags of the driver enabled, it produces so much log output that
wpa_supplicantgives up due to timeouts. The other weird issue is that withWCN36XX_DBG_MACand/orWCN36XX_DBG_SMDenabled, the effect is _much_ harder to trigger.
It would be great to have wcn36xx logging via tracing, just like ath10k and iwlwifi does. This way logging shouldn't slow down the system too much and with wpasupplicant's -T switch you can even get wpasupplicant's debug messages to the same log with proper timestamps! And almost forgot, you can also include mac80211 tracing logs as well:
https://wireless.wiki.kernel.org/en/developers/documentation/mac80211/tracing
https://wireless.wiki.kernel.org/en/users/drivers/ath10k/debug#tracing
See ath10k_dbg() and trace_ath10k_log_dbg() for ideas how to implement this, and you can also take a look at iwlwifi. Should be pretty easy. Patches more than welcome :)
Quoted 3 lines
Quoted 1 line
* Does this only happen with encryption or also in open mode?
That's a good question. I'll go check with an open network.
Thanks.
Quoted 3 lines
Quoted 1 line
* How long does it take with qconnman-stress to reproduce the issue?
Usually less than 10 minutes.
That's really good, makes it so much easier to verify potential fixes.
Quoted 6 lines
Quoted 2 lines
* Does the radio environment make any difference on reproducibility? For example, clear enviroment vs lots of traffic/interference?
It seems it does, yes. Tests at night seem to take a bit more time to make the effect happen. But then again, it could also be unrelated. I can't be certain at this point.
Can you describe what kind of radio environment you have, is it a busy office complex? How many APs around etc?
Signature · 1 line
Kalle Valo
Re: wcn36xx: bug #538: stuck tx management frames
Quoted 10 lines · Kalle Valo
Quoted 8 lines · Daniel Mack
Quoted 1 line · Kalle Valo
Daniel Mack <[email protected]> writes:
It seems that once a network is successfully joined, the network stability is fine. I haven't seen any starvation of streams lately, at least not with the the patches in this series which I'm running since a while. That is, until a disconnect/reconnect attempt is made, and at this point, only management frames are involved.
Quoted 3 lines
Ah, maybe originally you were seeing different issues with similar symptoms? But now you have fixed the other bugsand now the stuck transmitted management frame issue is left? Just guessing...
Yeah, I wish I had a clearer picture on all this myself :(
My patches definitely address some of the issues I have seen before, also the fixes for potential race conditions are likely to have a positive effect. But as you guessed yourself, I'm afraid that there's a multitude of possible sources for bugs, so it's hard to tell.
Quoted 6 lines
It would be great to have wcn36xx logging via tracing, just like ath10k and iwlwifi does. This way logging shouldn't slow down the system too much and with wpasupplicant's -T switch you can even get wpasupplicant's debug messages to the same log with proper timestamps! And almost forgot, you can also include mac80211 tracing logs as well:
Quoted 7 lines
https://wireless.wiki.kernel.org/en/developers/documentation/mac80211/tracing
https://wireless.wiki.kernel.org/en/users/drivers/ath10k/debug#tracing
See
ath10k_dbg()andtrace_ath10k_log_dbg()for ideas how to implement this, and you can also take a look at iwlwifi. Should be pretty easy. Patches more than welcome :)
Okay, I'll see if I can find some time to look into this.
The reason why I didn't focus the logging possibilities is that I looked at the SMD messages that are flying around which result from ieee80211 API calls into the driver, and I can't seem to find anything that's wrong, especially not before the timeouts occur. Hence, I don't actually expect any oddness on the ieee80211 layer.
But I agree that in general, better logging is certainly helpful.
Quoted 6 lines
Quoted 3 lines
It seems it does, yes. Tests at night seem to take a bit more time to make the effect happen. But then again, it could also be unrelated. I can't be certain at this point.
Can you describe what kind of radio environment you have, is it a busy office complex? How many APs around etc?
I've tried different environments. In the office with 15-20 laptops/mobiles in the room I see about 10-15 APs. At home, where I did long-term nightly test, there's maybe a higher number of APs, but fewer devices on the channel of the AP that I used for tests.
I haven't used any more sophisticated environments like a sealed reverberation chamber yet though.
Thanks,
Daniel
Re: wcn36xx: bug #538: stuck tx management frames
Quoted 15 lines · Daniel Mack
Quoted 10 lines · Kalle Valo
Quoted 8 lines · Daniel Mack
Quoted 1 line · Kalle Valo
Daniel Mack <[email protected]> writes:
It seems that once a network is successfully joined, the network stability is fine. I haven't seen any starvation of streams lately, at least not with the the patches in this series which I'm running since a while. That is, until a disconnect/reconnect attempt is made, and at this point, only management frames are involved.
Quoted 3 lines
Ah, maybe originally you were seeing different issues with similar symptoms? But now you have fixed the other bugsand now the stuck transmitted management frame issue is left? Just guessing...
Quoted 6 lines
Yeah, I wish I had a clearer picture on all this myself :(
My patches definitely address some of the issues I have seen before, also the fixes for potential race conditions are likely to have a positive effect. But as you guessed yourself, I'm afraid that there's a multitude of possible sources for bugs, so it's hard to tell.
Sure, but things seem to going be forward in steady state. Thanks for your hard work!
Quoted 16 lines
Quoted 6 lines
It would be great to have wcn36xx logging via tracing, just like ath10k and iwlwifi does. This way logging shouldn't slow down the system too much and with wpasupplicant's -T switch you can even get wpasupplicant's debug messages to the same log with proper timestamps! And almost forgot, you can also include mac80211 tracing logs as well:
Quoted 7 lines
https://wireless.wiki.kernel.org/en/developers/documentation/mac80211/tracing
https://wireless.wiki.kernel.org/en/users/drivers/ath10k/debug#tracing
See
ath10k_dbg()andtrace_ath10k_log_dbg()for ideas how to implement this, and you can also take a look at iwlwifi. Should be pretty easy. Patches more than welcome :)Okay, I'll see if I can find some time to look into this.
Quoted 7 lines
The reason why I didn't focus the logging possibilities is that I looked at the SMD messages that are flying around which result from ieee80211 API calls into the driver, and I can't seem to find anything that's wrong, especially not before the timeouts occur. Hence, I don't actually expect any oddness on the ieee80211 layer.
But I agree that in general, better logging is certainly helpful.
Yeah, I'm not expecting tracing logs to solve this case either but maybe we could find some hints. And it just makes it so much easier to see what's really happening from tracing logs instead trying to guess from the bug description. "a tracing log is worth a thousand words" ;)
But if you don't have time to implement tracing support to wcn36xx hopefully someone else has, all one needs is a DragonBoard 410c. A simple project for a student or someone who wants to contribute something to the kernel.
Quoted 7 lines
Quoted 6 lines
Quoted 3 lines
It seems it does, yes. Tests at night seem to take a bit more time to make the effect happen. But then again, it could also be unrelated. I can't be certain at this point.
Can you describe what kind of radio environment you have, is it a busy office complex? How many APs around etc?
Quoted 7 lines
I've tried different environments. In the office with 15-20 laptops/mobiles in the room I see about 10-15 APs. At home, where I did long-term nightly test, there's maybe a higher number of APs, but fewer devices on the channel of the AP that I used for tests.
I haven't used any more sophisticated environments like a sealed reverberation chamber yet though.
Ok, but please let me know if you see any differences caused by the environment.
Signature · 1 line
Kalle Valo
Re: wcn36xx: bug #538: stuck tx management frames
Quoted 56 lines · Kalle Valo
Quoted 15 lines · Daniel Mack
Quoted 10 lines · Kalle Valo
Quoted 8 lines · Daniel Mack
Quoted 1 line · Kalle Valo
Daniel Mack <[email protected]> writes:
It seems that once a network is successfully joined, the network stability is fine. I haven't seen any starvation of streams lately, at least not with the the patches in this series which I'm running since a while. That is, until a disconnect/reconnect attempt is made, and at this point, only management frames are involved.
Quoted 3 lines
Ah, maybe originally you were seeing different issues with similar symptoms? But now you have fixed the other bugsand now the stuck transmitted management frame issue is left? Just guessing...
Quoted 6 lines
Yeah, I wish I had a clearer picture on all this myself :(
My patches definitely address some of the issues I have seen before, also the fixes for potential race conditions are likely to have a positive effect. But as you guessed yourself, I'm afraid that there's a multitude of possible sources for bugs, so it's hard to tell.
Sure, but things seem to going be forward in steady state. Thanks for your hard work!
Quoted 16 lines
Quoted 6 lines
It would be great to have wcn36xx logging via tracing, just like ath10k and iwlwifi does. This way logging shouldn't slow down the system too much and with wpasupplicant's -T switch you can even get wpasupplicant's debug messages to the same log with proper timestamps! And almost forgot, you can also include mac80211 tracing logs as well:
Quoted 7 lines
https://wireless.wiki.kernel.org/en/developers/documentation/mac80211/tracing
https://wireless.wiki.kernel.org/en/users/drivers/ath10k/debug#tracing
See
ath10k_dbg()andtrace_ath10k_log_dbg()for ideas how to implement this, and you can also take a look at iwlwifi. Should be pretty easy. Patches more than welcome :)Okay, I'll see if I can find some time to look into this.
Quoted 7 lines
The reason why I didn't focus the logging possibilities is that I looked at the SMD messages that are flying around which result from ieee80211 API calls into the driver, and I can't seem to find anything that's wrong, especially not before the timeouts occur. Hence, I don't actually expect any oddness on the ieee80211 layer.
But I agree that in general, better logging is certainly helpful.
Yeah, I'm not expecting tracing logs to solve this case either but maybe we could find some hints. And it just makes it so much easier to see what's really happening from tracing logs instead trying to guess from the bug description. "a tracing log is worth a thousand words" ;)
Quoted 4 lines
But if you don't have time to implement tracing support to wcn36xx hopefully someone else has, all one needs is a DragonBoard 410c. A simple project for a student or someone who wants to contribute something to the kernel.
I have time to do it. If nobody else started yet... Thanks.
Quoted 26 lines
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
It seems it does, yes. Tests at night seem to take a bit more time to make the effect happen. But then again, it could also be unrelated. I can't be certain at this point.
Can you describe what kind of radio environment you have, is it a busy office complex? How many APs around etc?
Quoted 7 lines
I've tried different environments. In the office with 15-20 laptops/mobiles in the room I see about 10-15 APs. At home, where I did long-term nightly test, there's maybe a higher number of APs, but fewer devices on the channel of the AP that I used for tests.
I haven't used any more sophisticated environments like a sealed reverberation chamber yet though.
Ok, but please let me know if you see any differences caused by the environment.
--
Kalle Valo_______________________________________________
wcn36xx mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/wcn36xx