public inbox for linux-mediatek@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH net] net: airoha: fix BQL imbalance in TX path
@ 2026-04-21  6:35 Lorenzo Bianconi
  2026-04-23  8:12 ` Lorenzo Bianconi
  0 siblings, 1 reply; 2+ messages in thread
From: Lorenzo Bianconi @ 2026-04-21  6:35 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Hariprasad Kelam
  Cc: Simon Horman, linux-arm-kernel, linux-mediatek, netdev,
	Lorenzo Bianconi

Fix a possible BQL imbalance in airoha_dev_xmit(), where inflight
packets are accounted only for the AIROHA_NUM_TX_RING netdev TX
queues. The queue index is computed as:

    qid = skb_get_queue_mapping(skb) % ARRAY_SIZE(qdma->q_tx)
    txq = netdev_get_tx_queue(dev, qid);

However, airoha_qdma_tx_napi_poll() accounts completions across all
netdev TX queues (num_tx_queues), leading to inconsistent BQL
accounting.

Also reset all netdev TX queues in the ndo_stop callback.

Fixes: 1d304174106c ("net: airoha: Implement BQL support")
Fixes: c9f947769b77 ("net: airoha: Reset BQL stopping the netdevice")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 19f67c7dd8e1..6c7390f0de5d 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -929,10 +929,9 @@ static int airoha_qdma_tx_napi_poll(struct napi_struct *napi, int budget)
 		q->queued--;
 
 		if (skb) {
-			u16 queue = skb_get_queue_mapping(skb);
 			struct netdev_queue *txq;
 
-			txq = netdev_get_tx_queue(skb->dev, queue);
+			txq = skb_get_tx_queue(skb->dev, skb);
 			netdev_tx_completed_queue(txq, 1, skb->len);
 			dev_kfree_skb_any(skb);
 		}
@@ -1711,7 +1710,7 @@ static int airoha_dev_stop(struct net_device *dev)
 	if (err)
 		return err;
 
-	for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++)
+	for (i = 0; i < dev->num_tx_queues; i++)
 		netdev_tx_reset_subqueue(dev, i);
 
 	airoha_set_gdm_port_fwd_cfg(qdma->eth, REG_GDM_FWD_CFG(port->id),
@@ -2002,7 +2001,7 @@ static netdev_tx_t airoha_dev_xmit(struct sk_buff *skb,
 
 	spin_lock_bh(&q->lock);
 
-	txq = netdev_get_tx_queue(dev, qid);
+	txq = skb_get_tx_queue(dev, skb);
 	nr_frags = 1 + skb_shinfo(skb)->nr_frags;
 
 	if (q->queued + nr_frags >= q->ndesc) {

---
base-commit: a663bac71a2f0b3ac6c373168ca57b2a6e6381aa
change-id: 20260421-airoha-fix-bql-7fff7cebbc9a

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] net: airoha: fix BQL imbalance in TX path
  2026-04-21  6:35 [PATCH net] net: airoha: fix BQL imbalance in TX path Lorenzo Bianconi
@ 2026-04-23  8:12 ` Lorenzo Bianconi
  0 siblings, 0 replies; 2+ messages in thread
From: Lorenzo Bianconi @ 2026-04-23  8:12 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Hariprasad Kelam
  Cc: Simon Horman, linux-arm-kernel, linux-mediatek, netdev

[-- Attachment #1: Type: text/plain, Size: 4017 bytes --]

> Fix a possible BQL imbalance in airoha_dev_xmit(), where inflight
> packets are accounted only for the AIROHA_NUM_TX_RING netdev TX
> queues. The queue index is computed as:
> 
>     qid = skb_get_queue_mapping(skb) % ARRAY_SIZE(qdma->q_tx)
>     txq = netdev_get_tx_queue(dev, qid);
> 
> However, airoha_qdma_tx_napi_poll() accounts completions across all
> netdev TX queues (num_tx_queues), leading to inconsistent BQL
> accounting.
> 
> Also reset all netdev TX queues in the ndo_stop callback.
> 
> Fixes: 1d304174106c ("net: airoha: Implement BQL support")
> Fixes: c9f947769b77 ("net: airoha: Reset BQL stopping the netdevice")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  drivers/net/ethernet/airoha/airoha_eth.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 19f67c7dd8e1..6c7390f0de5d 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -929,10 +929,9 @@ static int airoha_qdma_tx_napi_poll(struct napi_struct *napi, int budget)
>  		q->queued--;
>  
>  		if (skb) {
> -			u16 queue = skb_get_queue_mapping(skb);
>  			struct netdev_queue *txq;
>  
> -			txq = netdev_get_tx_queue(skb->dev, queue);
> +			txq = skb_get_tx_queue(skb->dev, skb);
>  			netdev_tx_completed_queue(txq, 1, skb->len);
>  			dev_kfree_skb_any(skb);
>  		}
> @@ -1711,7 +1710,7 @@ static int airoha_dev_stop(struct net_device *dev)
>  	if (err)
>  		return err;
>  
> -	for (i = 0; i < ARRAY_SIZE(qdma->q_tx); i++)
> +	for (i = 0; i < dev->num_tx_queues; i++)
>  		netdev_tx_reset_subqueue(dev, i);
>  
>  	airoha_set_gdm_port_fwd_cfg(qdma->eth, REG_GDM_FWD_CFG(port->id),
> @@ -2002,7 +2001,7 @@ static netdev_tx_t airoha_dev_xmit(struct sk_buff *skb,
>  
>  	spin_lock_bh(&q->lock);
>  
> -	txq = netdev_get_tx_queue(dev, qid);
> +	txq = skb_get_tx_queue(dev, skb);
>  	nr_frags = 1 + skb_shinfo(skb)->nr_frags;
>  
>  	if (q->queued + nr_frags >= q->ndesc) {
> 
> ---
> base-commit: a663bac71a2f0b3ac6c373168ca57b2a6e6381aa
> change-id: 20260421-airoha-fix-bql-7fff7cebbc9a
> 
> Best regards,
> -- 
> Lorenzo Bianconi <lorenzo@kernel.org>
> 

commenting on Sashiko reported issues:
https://sashiko.dev/#/patchset/20260421-airoha-fix-bql-v1-1-f135afe4275b%40kernel.org

- This isn't a bug in this patch, but does using 0xff as a sentinel value cause a permanent stall?
  I do not think this is a real issue since, according to my understanding, the NIC
  never writes 0xff in irq_q queue.

- This is another pre-existing issue, but does freeing the SKB here cause a DMA use-after-free
  for multi-fragment packets?
  This issue is not related to this patch, and I will fix it in a dedicated
  patch storing the skb pointer in the last descriptor in airoha_dev_xmit()

- Since the QDMA hardware and NAPI instance are shared among multiple ports (qdma->users),
  could active NAPI polling cause a BUG_ON() in dql_completed()?
  This is not an issue related to this patch since here we are just resetting
  all the netdev tx queues instead of just the first ARRAY_SIZE(qdma->q_tx)
  ones.

- This isn't a bug in this patch, but does failing to wait for the DMA engines to become
  idle before unmapping buffers cause memory corruption?
  This issue is not related to this patch and it will be fixed with a dedicated
  patch.

- This is also pre-existing, but can this mapping cause a kernel panic on highmem systems?
  Can we have fragments in high memory? e.g on ARM architecture? Anyway, as
  pointed out by Sashiko, this issue is not related to this patch.

- This isn't introduced here, but does this logic cause a permanent TX stall?
  this issue is already fixed in the following patch:
  https://patchwork.kernel.org/project/netdevbpf/patch/20260421-airoha-xmit-stop-condition-v1-1-e670d6a48467@kernel.org/

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-04-23  8:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21  6:35 [PATCH net] net: airoha: fix BQL imbalance in TX path Lorenzo Bianconi
2026-04-23  8:12 ` Lorenzo Bianconi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox