* [PATCH net 0/2] Correct BD length masks and BQL accounting for multi-BD TX packets @ 2026-03-24 14:53 Suraj Gupta 2026-03-24 14:53 ` [PATCH net 1/2] net: xilinx: axienet: Correct BD length masks to match AXIDMA IP spec Suraj Gupta 2026-03-24 14:53 ` [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets Suraj Gupta 0 siblings, 2 replies; 9+ messages in thread From: Suraj Gupta @ 2026-03-24 14:53 UTC (permalink / raw) To: andrew+netdev, davem, edumazet, kuba, pabeni, michal.simek, sean.anderson, radhey.shyam.pandey, horms Cc: netdev, linux-arm-kernel, linux-kernel, harini.katakam This patch series fixes two issues in the Xilinx AXI Ethernet driver: 1. Corrects the BD length masks to match the AXIDMA IP spec. 2. Fixes BQL accounting for multi-BD TX packets. Suraj Gupta (2): net: xilinx: axienet: Correct BD length masks to match AXIDMA IP spec net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets drivers/net/ethernet/xilinx/xilinx_axienet.h | 7 +++++-- .../net/ethernet/xilinx/xilinx_axienet_main.c | 20 +++++++++---------- 2 files changed, 15 insertions(+), 12 deletions(-) -- 2.49.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net 1/2] net: xilinx: axienet: Correct BD length masks to match AXIDMA IP spec 2026-03-24 14:53 [PATCH net 0/2] Correct BD length masks and BQL accounting for multi-BD TX packets Suraj Gupta @ 2026-03-24 14:53 ` Suraj Gupta 2026-03-24 16:09 ` Sean Anderson 2026-03-24 14:53 ` [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets Suraj Gupta 1 sibling, 1 reply; 9+ messages in thread From: Suraj Gupta @ 2026-03-24 14:53 UTC (permalink / raw) To: andrew+netdev, davem, edumazet, kuba, pabeni, michal.simek, sean.anderson, radhey.shyam.pandey, horms Cc: netdev, linux-arm-kernel, linux-kernel, harini.katakam The XAXIDMA_BD_CTRL_LENGTH_MASK and XAXIDMA_BD_STS_ACTUAL_LEN_MASK macros were defined as 0x007FFFFF (23 bits), but the AXI DMA IP product guide (PG021) specifies the buffer length field as bits 25:0 (26 bits). Update both masks to 0x03FFFFFF to match the IP documentation. In practice this had no functional impact, since Ethernet frames are far smaller than 2^23 bytes and the extra bits were always zero, but the masks should still reflect the hardware specification. Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver") Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com> --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 5ff742103beb..602389843342 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -105,7 +105,7 @@ #define XAXIDMA_BD_HAS_DRE_MASK 0xF00 /* Whether has DRE mask */ #define XAXIDMA_BD_WORDLEN_MASK 0xFF /* Whether has DRE mask */ -#define XAXIDMA_BD_CTRL_LENGTH_MASK 0x007FFFFF /* Requested len */ +#define XAXIDMA_BD_CTRL_LENGTH_MASK 0x03FFFFFF /* Requested len */ #define XAXIDMA_BD_CTRL_TXSOF_MASK 0x08000000 /* First tx packet */ #define XAXIDMA_BD_CTRL_TXEOF_MASK 0x04000000 /* Last tx packet */ #define XAXIDMA_BD_CTRL_ALL_MASK 0x0C000000 /* All control bits */ @@ -130,7 +130,7 @@ #define XAXIDMA_BD_CTRL_TXEOF_MASK 0x04000000 /* Last tx packet */ #define XAXIDMA_BD_CTRL_ALL_MASK 0x0C000000 /* All control bits */ -#define XAXIDMA_BD_STS_ACTUAL_LEN_MASK 0x007FFFFF /* Actual len */ +#define XAXIDMA_BD_STS_ACTUAL_LEN_MASK 0x03FFFFFF /* Actual len */ #define XAXIDMA_BD_STS_COMPLETE_MASK 0x80000000 /* Completed */ #define XAXIDMA_BD_STS_DEC_ERR_MASK 0x40000000 /* Decode error */ #define XAXIDMA_BD_STS_SLV_ERR_MASK 0x20000000 /* Slave error */ -- 2.49.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/2] net: xilinx: axienet: Correct BD length masks to match AXIDMA IP spec 2026-03-24 14:53 ` [PATCH net 1/2] net: xilinx: axienet: Correct BD length masks to match AXIDMA IP spec Suraj Gupta @ 2026-03-24 16:09 ` Sean Anderson 0 siblings, 0 replies; 9+ messages in thread From: Sean Anderson @ 2026-03-24 16:09 UTC (permalink / raw) To: Suraj Gupta, andrew+netdev, davem, edumazet, kuba, pabeni, michal.simek, radhey.shyam.pandey, horms Cc: netdev, linux-arm-kernel, linux-kernel, harini.katakam On 3/24/26 10:53, Suraj Gupta wrote: > The XAXIDMA_BD_CTRL_LENGTH_MASK and XAXIDMA_BD_STS_ACTUAL_LEN_MASK > macros were defined as 0x007FFFFF (23 bits), but the AXI DMA IP > product guide (PG021) specifies the buffer length field as bits 25:0 > (26 bits). Update both masks to 0x03FFFFFF to match the IP > documentation. > > In practice this had no functional impact, since Ethernet frames are > far smaller than 2^23 bytes and the extra bits were always zero, but > the masks should still reflect the hardware specification. > > Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver") > Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com> > --- > drivers/net/ethernet/xilinx/xilinx_axienet.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h > index 5ff742103beb..602389843342 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h > @@ -105,7 +105,7 @@ > #define XAXIDMA_BD_HAS_DRE_MASK 0xF00 /* Whether has DRE mask */ > #define XAXIDMA_BD_WORDLEN_MASK 0xFF /* Whether has DRE mask */ > > -#define XAXIDMA_BD_CTRL_LENGTH_MASK 0x007FFFFF /* Requested len */ > +#define XAXIDMA_BD_CTRL_LENGTH_MASK 0x03FFFFFF /* Requested len */ > #define XAXIDMA_BD_CTRL_TXSOF_MASK 0x08000000 /* First tx packet */ > #define XAXIDMA_BD_CTRL_TXEOF_MASK 0x04000000 /* Last tx packet */ > #define XAXIDMA_BD_CTRL_ALL_MASK 0x0C000000 /* All control bits */ > @@ -130,7 +130,7 @@ > #define XAXIDMA_BD_CTRL_TXEOF_MASK 0x04000000 /* Last tx packet */ > #define XAXIDMA_BD_CTRL_ALL_MASK 0x0C000000 /* All control bits */ > > -#define XAXIDMA_BD_STS_ACTUAL_LEN_MASK 0x007FFFFF /* Actual len */ > +#define XAXIDMA_BD_STS_ACTUAL_LEN_MASK 0x03FFFFFF /* Actual len */ I think #define XAXIDMA_BD_STS_ACTUAL_LEN_MASK GENMASK(25, 0) is clearer. > #define XAXIDMA_BD_STS_COMPLETE_MASK 0x80000000 /* Completed */ > #define XAXIDMA_BD_STS_DEC_ERR_MASK 0x40000000 /* Decode error */ > #define XAXIDMA_BD_STS_SLV_ERR_MASK 0x20000000 /* Slave error */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets 2026-03-24 14:53 [PATCH net 0/2] Correct BD length masks and BQL accounting for multi-BD TX packets Suraj Gupta 2026-03-24 14:53 ` [PATCH net 1/2] net: xilinx: axienet: Correct BD length masks to match AXIDMA IP spec Suraj Gupta @ 2026-03-24 14:53 ` Suraj Gupta 2026-03-24 16:09 ` Sean Anderson 1 sibling, 1 reply; 9+ messages in thread From: Suraj Gupta @ 2026-03-24 14:53 UTC (permalink / raw) To: andrew+netdev, davem, edumazet, kuba, pabeni, michal.simek, sean.anderson, radhey.shyam.pandey, horms Cc: netdev, linux-arm-kernel, linux-kernel, harini.katakam When a TX packet spans multiple buffer descriptors (scatter-gather), the per-BD byte count is accumulated into a local variable that resets on each NAPI poll. If the BDs for a single packet complete across different polls, the earlier bytes are lost and never credited to BQL. This causes BQL to think bytes are permanently in-flight, eventually stalling the TX queue. Fix this by replacing the local accumulator with a persistent counter (tx_compl_bytes) that survives across polls and is reset only after updating BQL and stats. Fixes: c900e49d58eb ("net: xilinx: axienet: Implement BQL") Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com> --- drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 +++ .../net/ethernet/xilinx/xilinx_axienet_main.c | 20 +++++++++---------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h index 602389843342..a4444c939451 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h @@ -509,6 +509,8 @@ struct skbuf_dma_descriptor { * complete. Only updated at runtime by TX NAPI poll. * @tx_bd_tail: Stores the index of the next Tx buffer descriptor in the ring * to be populated. + * @tx_compl_bytes: Accumulates TX completion length until a full packet is + * reported to the stack. * @tx_packets: TX packet count for statistics * @tx_bytes: TX byte count for statistics * @tx_stat_sync: Synchronization object for TX stats @@ -592,6 +594,7 @@ struct axienet_local { u32 tx_bd_num; u32 tx_bd_ci; u32 tx_bd_tail; + u32 tx_compl_bytes; u64_stats_t tx_packets; u64_stats_t tx_bytes; struct u64_stats_sync tx_stat_sync; diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index b06e4c37ff61..95bf61986cb7 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -692,6 +692,8 @@ static void axienet_dma_stop(struct axienet_local *lp) axienet_lock_mii(lp); __axienet_device_reset(lp); axienet_unlock_mii(lp); + + lp->tx_compl_bytes = 0; } /** @@ -770,8 +772,6 @@ static int axienet_device_reset(struct net_device *ndev) * @first_bd: Index of first descriptor to clean up * @nr_bds: Max number of descriptors to clean up * @force: Whether to clean descriptors even if not complete - * @sizep: Pointer to a u32 filled with the total sum of all bytes - * in all cleaned-up descriptors. Ignored if NULL. * @budget: NAPI budget (use 0 when not called from NAPI poll) * * Would either be called after a successful transmit operation, or after @@ -780,7 +780,7 @@ static int axienet_device_reset(struct net_device *ndev) * Return: The number of packets handled. */ static int axienet_free_tx_chain(struct axienet_local *lp, u32 first_bd, - int nr_bds, bool force, u32 *sizep, int budget) + int nr_bds, bool force, int budget) { struct axidma_bd *cur_p; unsigned int status; @@ -819,8 +819,8 @@ static int axienet_free_tx_chain(struct axienet_local *lp, u32 first_bd, cur_p->cntrl = 0; cur_p->status = 0; - if (sizep) - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; + if (!force) + lp->tx_compl_bytes += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; } if (!force) { @@ -999,18 +999,18 @@ static int axienet_tx_poll(struct napi_struct *napi, int budget) { struct axienet_local *lp = container_of(napi, struct axienet_local, napi_tx); struct net_device *ndev = lp->ndev; - u32 size = 0; int packets; packets = axienet_free_tx_chain(lp, lp->tx_bd_ci, lp->tx_bd_num, false, - &size, budget); + budget); if (packets) { - netdev_completed_queue(ndev, packets, size); + netdev_completed_queue(ndev, packets, lp->tx_compl_bytes); u64_stats_update_begin(&lp->tx_stat_sync); u64_stats_add(&lp->tx_packets, packets); - u64_stats_add(&lp->tx_bytes, size); + u64_stats_add(&lp->tx_bytes, lp->tx_compl_bytes); u64_stats_update_end(&lp->tx_stat_sync); + lp->tx_compl_bytes = 0; /* Matches barrier in axienet_start_xmit */ smp_mb(); @@ -1115,7 +1115,7 @@ axienet_start_xmit(struct sk_buff *skb, struct net_device *ndev) netdev_err(ndev, "TX DMA mapping error\n"); ndev->stats.tx_dropped++; axienet_free_tx_chain(lp, orig_tail_ptr, ii + 1, - true, NULL, 0); + true, 0); dev_kfree_skb_any(skb); return NETDEV_TX_OK; } -- 2.49.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets 2026-03-24 14:53 ` [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets Suraj Gupta @ 2026-03-24 16:09 ` Sean Anderson 2026-03-25 5:30 ` Gupta, Suraj 0 siblings, 1 reply; 9+ messages in thread From: Sean Anderson @ 2026-03-24 16:09 UTC (permalink / raw) To: Suraj Gupta, andrew+netdev, davem, edumazet, kuba, pabeni, michal.simek, radhey.shyam.pandey, horms Cc: netdev, linux-arm-kernel, linux-kernel, harini.katakam On 3/24/26 10:53, Suraj Gupta wrote: > When a TX packet spans multiple buffer descriptors (scatter-gather), > the per-BD byte count is accumulated into a local variable that resets > on each NAPI poll. If the BDs for a single packet complete across > different polls, the earlier bytes are lost and never credited to BQL. > This causes BQL to think bytes are permanently in-flight, eventually > stalling the TX queue. > > Fix this by replacing the local accumulator with a persistent counter > (tx_compl_bytes) that survives across polls and is reset only after > updating BQL and stats. Do we need this? Can't we just do something like diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index 415e9bc252527..1ea8a6592bce1 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -768,6 +768,7 @@ static int axienet_free_tx_chain(struct axienet_local *lp, u32 *sizep, int budge if (cur_p->skb) { struct axienet_cb *cb = (void *)cur_p->skb->cb; + *sizep += skb->len; dma_unmap_sgtable(lp->dev, &cb->sgt, DMA_TO_DEVICE, 0); sg_free_table_chained(&cb->sgt, XAE_INLINE_SG_CNT); napi_consume_skb(cur_p->skb, budget); @@ -783,8 +784,6 @@ static int axienet_free_tx_chain(struct axienet_local *lp, u32 *sizep, int budge wmb(); cur_p->cntrl = 0; cur_p->status = 0; - - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; } smp_store_release(&lp->tx_bd_ci, (ci + i) & (lp->tx_bd_num - 1)); > Fixes: c900e49d58eb ("net: xilinx: axienet: Implement BQL") > Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com> > --- > drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 +++ > .../net/ethernet/xilinx/xilinx_axienet_main.c | 20 +++++++++---------- > 2 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h > index 602389843342..a4444c939451 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h > @@ -509,6 +509,8 @@ struct skbuf_dma_descriptor { > * complete. Only updated at runtime by TX NAPI poll. > * @tx_bd_tail: Stores the index of the next Tx buffer descriptor in the ring > * to be populated. > + * @tx_compl_bytes: Accumulates TX completion length until a full packet is > + * reported to the stack. > * @tx_packets: TX packet count for statistics > * @tx_bytes: TX byte count for statistics > * @tx_stat_sync: Synchronization object for TX stats > @@ -592,6 +594,7 @@ struct axienet_local { > u32 tx_bd_num; > u32 tx_bd_ci; > u32 tx_bd_tail; > + u32 tx_compl_bytes; > u64_stats_t tx_packets; > u64_stats_t tx_bytes; > struct u64_stats_sync tx_stat_sync; > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > index b06e4c37ff61..95bf61986cb7 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > @@ -692,6 +692,8 @@ static void axienet_dma_stop(struct axienet_local *lp) > axienet_lock_mii(lp); > __axienet_device_reset(lp); > axienet_unlock_mii(lp); > + > + lp->tx_compl_bytes = 0; > } > > /** > @@ -770,8 +772,6 @@ static int axienet_device_reset(struct net_device *ndev) > * @first_bd: Index of first descriptor to clean up > * @nr_bds: Max number of descriptors to clean up > * @force: Whether to clean descriptors even if not complete > - * @sizep: Pointer to a u32 filled with the total sum of all bytes > - * in all cleaned-up descriptors. Ignored if NULL. > * @budget: NAPI budget (use 0 when not called from NAPI poll) > * > * Would either be called after a successful transmit operation, or after > @@ -780,7 +780,7 @@ static int axienet_device_reset(struct net_device *ndev) > * Return: The number of packets handled. > */ > static int axienet_free_tx_chain(struct axienet_local *lp, u32 first_bd, > - int nr_bds, bool force, u32 *sizep, int budget) > + int nr_bds, bool force, int budget) > { > struct axidma_bd *cur_p; > unsigned int status; > @@ -819,8 +819,8 @@ static int axienet_free_tx_chain(struct axienet_local *lp, u32 first_bd, > cur_p->cntrl = 0; > cur_p->status = 0; > > - if (sizep) > - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; > + if (!force) > + lp->tx_compl_bytes += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; > } > > if (!force) { > @@ -999,18 +999,18 @@ static int axienet_tx_poll(struct napi_struct *napi, int budget) > { > struct axienet_local *lp = container_of(napi, struct axienet_local, napi_tx); > struct net_device *ndev = lp->ndev; > - u32 size = 0; > int packets; > > packets = axienet_free_tx_chain(lp, lp->tx_bd_ci, lp->tx_bd_num, false, > - &size, budget); > + budget); > > if (packets) { > - netdev_completed_queue(ndev, packets, size); > + netdev_completed_queue(ndev, packets, lp->tx_compl_bytes); > u64_stats_update_begin(&lp->tx_stat_sync); > u64_stats_add(&lp->tx_packets, packets); > - u64_stats_add(&lp->tx_bytes, size); > + u64_stats_add(&lp->tx_bytes, lp->tx_compl_bytes); > u64_stats_update_end(&lp->tx_stat_sync); > + lp->tx_compl_bytes = 0; > > /* Matches barrier in axienet_start_xmit */ > smp_mb(); > @@ -1115,7 +1115,7 @@ axienet_start_xmit(struct sk_buff *skb, struct net_device *ndev) > netdev_err(ndev, "TX DMA mapping error\n"); > ndev->stats.tx_dropped++; > axienet_free_tx_chain(lp, orig_tail_ptr, ii + 1, > - true, NULL, 0); > + true, 0); > dev_kfree_skb_any(skb); > return NETDEV_TX_OK; > } ^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets 2026-03-24 16:09 ` Sean Anderson @ 2026-03-25 5:30 ` Gupta, Suraj 2026-03-26 15:38 ` Sean Anderson 0 siblings, 1 reply; 9+ messages in thread From: Gupta, Suraj @ 2026-03-25 5:30 UTC (permalink / raw) To: Sean Anderson, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, Simek, Michal, Pandey, Radhey Shyam, horms@kernel.org Cc: netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Katakam, Harini [Public] > -----Original Message----- > From: Sean Anderson <sean.anderson@linux.dev> > Sent: Tuesday, March 24, 2026 9:39 PM > To: Gupta, Suraj <Suraj.Gupta2@amd.com>; andrew+netdev@lunn.ch; > davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > pabeni@redhat.com; Simek, Michal <michal.simek@amd.com>; Pandey, > Radhey Shyam <radhey.shyam.pandey@amd.com>; horms@kernel.org > Cc: netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org; Katakam, Harini <harini.katakam@amd.com> > Subject: Re: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD > TX packets > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > On 3/24/26 10:53, Suraj Gupta wrote: > > When a TX packet spans multiple buffer descriptors (scatter-gather), > > the per-BD byte count is accumulated into a local variable that resets > > on each NAPI poll. If the BDs for a single packet complete across > > different polls, the earlier bytes are lost and never credited to BQL. > > This causes BQL to think bytes are permanently in-flight, eventually > > stalling the TX queue. > > > > Fix this by replacing the local accumulator with a persistent counter > > (tx_compl_bytes) that survives across polls and is reset only after > > updating BQL and stats. > > Do we need this? Can't we just do something like > Nope, the 'size' variable passed to axienet_free_tx_chain() is local to axienet_tx_poll() and goes out of scope between different polls. This means it can't track completion bytes across multiple NAPI polls. Regards, Suraj > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > index 415e9bc252527..1ea8a6592bce1 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > @@ -768,6 +768,7 @@ static int axienet_free_tx_chain(struct axienet_local > *lp, u32 *sizep, int budge > if (cur_p->skb) { > struct axienet_cb *cb = (void *)cur_p->skb->cb; > > + *sizep += skb->len; > dma_unmap_sgtable(lp->dev, &cb->sgt, DMA_TO_DEVICE, 0); > sg_free_table_chained(&cb->sgt, XAE_INLINE_SG_CNT); > napi_consume_skb(cur_p->skb, budget); @@ -783,8 +784,6 @@ > static int axienet_free_tx_chain(struct axienet_local *lp, u32 *sizep, int budge > wmb(); > cur_p->cntrl = 0; > cur_p->status = 0; > - > - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; > } > > smp_store_release(&lp->tx_bd_ci, (ci + i) & (lp->tx_bd_num - 1)); > > > Fixes: c900e49d58eb ("net: xilinx: axienet: Implement BQL") > > Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com> > > --- > > drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 +++ > > .../net/ethernet/xilinx/xilinx_axienet_main.c | 20 +++++++++---------- > > 2 files changed, 13 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h > > b/drivers/net/ethernet/xilinx/xilinx_axienet.h > > index 602389843342..a4444c939451 100644 > > --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h > > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h > > @@ -509,6 +509,8 @@ struct skbuf_dma_descriptor { > > * complete. Only updated at runtime by TX NAPI poll. > > * @tx_bd_tail: Stores the index of the next Tx buffer descriptor in the ring > > * to be populated. > > + * @tx_compl_bytes: Accumulates TX completion length until a full packet is > > + * reported to the stack. > > * @tx_packets: TX packet count for statistics > > * @tx_bytes: TX byte count for statistics > > * @tx_stat_sync: Synchronization object for TX stats @@ -592,6 > > +594,7 @@ struct axienet_local { > > u32 tx_bd_num; > > u32 tx_bd_ci; > > u32 tx_bd_tail; > > + u32 tx_compl_bytes; > > u64_stats_t tx_packets; > > u64_stats_t tx_bytes; > > struct u64_stats_sync tx_stat_sync; diff --git > > a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > index b06e4c37ff61..95bf61986cb7 100644 > > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > > @@ -692,6 +692,8 @@ static void axienet_dma_stop(struct axienet_local > *lp) > > axienet_lock_mii(lp); > > __axienet_device_reset(lp); > > axienet_unlock_mii(lp); > > + > > + lp->tx_compl_bytes = 0; > > } > > > > /** > > @@ -770,8 +772,6 @@ static int axienet_device_reset(struct net_device > *ndev) > > * @first_bd: Index of first descriptor to clean up > > * @nr_bds: Max number of descriptors to clean up > > * @force: Whether to clean descriptors even if not complete > > - * @sizep: Pointer to a u32 filled with the total sum of all bytes > > - * in all cleaned-up descriptors. Ignored if NULL. > > * @budget: NAPI budget (use 0 when not called from NAPI poll) > > * > > * Would either be called after a successful transmit operation, or > > after @@ -780,7 +780,7 @@ static int axienet_device_reset(struct net_device > *ndev) > > * Return: The number of packets handled. > > */ > > static int axienet_free_tx_chain(struct axienet_local *lp, u32 first_bd, > > - int nr_bds, bool force, u32 *sizep, int budget) > > + int nr_bds, bool force, int budget) > > { > > struct axidma_bd *cur_p; > > unsigned int status; > > @@ -819,8 +819,8 @@ static int axienet_free_tx_chain(struct axienet_local > *lp, u32 first_bd, > > cur_p->cntrl = 0; > > cur_p->status = 0; > > > > - if (sizep) > > - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; > > + if (!force) > > + lp->tx_compl_bytes += status & > > + XAXIDMA_BD_STS_ACTUAL_LEN_MASK; > > } > > > > if (!force) { > > @@ -999,18 +999,18 @@ static int axienet_tx_poll(struct napi_struct > > *napi, int budget) { > > struct axienet_local *lp = container_of(napi, struct axienet_local, napi_tx); > > struct net_device *ndev = lp->ndev; > > - u32 size = 0; > > int packets; > > > > packets = axienet_free_tx_chain(lp, lp->tx_bd_ci, lp->tx_bd_num, false, > > - &size, budget); > > + budget); > > > > if (packets) { > > - netdev_completed_queue(ndev, packets, size); > > + netdev_completed_queue(ndev, packets, > > + lp->tx_compl_bytes); > > u64_stats_update_begin(&lp->tx_stat_sync); > > u64_stats_add(&lp->tx_packets, packets); > > - u64_stats_add(&lp->tx_bytes, size); > > + u64_stats_add(&lp->tx_bytes, lp->tx_compl_bytes); > > u64_stats_update_end(&lp->tx_stat_sync); > > + lp->tx_compl_bytes = 0; > > > > /* Matches barrier in axienet_start_xmit */ > > smp_mb(); > > @@ -1115,7 +1115,7 @@ axienet_start_xmit(struct sk_buff *skb, struct > net_device *ndev) > > netdev_err(ndev, "TX DMA mapping error\n"); > > ndev->stats.tx_dropped++; > > axienet_free_tx_chain(lp, orig_tail_ptr, ii + 1, > > - true, NULL, 0); > > + true, 0); > > dev_kfree_skb_any(skb); > > return NETDEV_TX_OK; > > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets 2026-03-25 5:30 ` Gupta, Suraj @ 2026-03-26 15:38 ` Sean Anderson 2026-03-26 19:51 ` Gupta, Suraj 0 siblings, 1 reply; 9+ messages in thread From: Sean Anderson @ 2026-03-26 15:38 UTC (permalink / raw) To: Gupta, Suraj, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, Simek, Michal, Pandey, Radhey Shyam, horms@kernel.org Cc: netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Katakam, Harini On 3/25/26 01:30, Gupta, Suraj wrote: > [Public] > >> -----Original Message----- >> From: Sean Anderson <sean.anderson@linux.dev> >> Sent: Tuesday, March 24, 2026 9:39 PM >> To: Gupta, Suraj <Suraj.Gupta2@amd.com>; andrew+netdev@lunn.ch; >> davem@davemloft.net; edumazet@google.com; kuba@kernel.org; >> pabeni@redhat.com; Simek, Michal <michal.simek@amd.com>; Pandey, >> Radhey Shyam <radhey.shyam.pandey@amd.com>; horms@kernel.org >> Cc: netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- >> kernel@vger.kernel.org; Katakam, Harini <harini.katakam@amd.com> >> Subject: Re: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD >> TX packets >> >> Caution: This message originated from an External Source. Use proper caution >> when opening attachments, clicking links, or responding. >> >> >> On 3/24/26 10:53, Suraj Gupta wrote: >> > When a TX packet spans multiple buffer descriptors (scatter-gather), >> > the per-BD byte count is accumulated into a local variable that resets >> > on each NAPI poll. If the BDs for a single packet complete across >> > different polls, the earlier bytes are lost and never credited to BQL. >> > This causes BQL to think bytes are permanently in-flight, eventually >> > stalling the TX queue. >> > >> > Fix this by replacing the local accumulator with a persistent counter >> > (tx_compl_bytes) that survives across polls and is reset only after >> > updating BQL and stats. >> >> Do we need this? Can't we just do something like >> > > Nope, the 'size' variable passed to axienet_free_tx_chain() is local > to axienet_tx_poll() and goes out of scope between different polls. > This means it can't track completion bytes across multiple NAPI polls. Yes, but that's fine since we only update completed bytes when we retire at least one packet: packets = axienet_free_tx_chain(lp, &size, budget); if (packets) { netdev_completed_queue(ndev, packets, size); u64_stats_update_begin(&lp->tx_stat_sync); u64_stats_add(&lp->tx_packets, packets); u64_stats_add(&lp->tx_bytes, size); u64_stats_update_end(&lp->tx_stat_sync); /* Matches barrier in axienet_start_xmit */ smp_mb(); if (((tail - ci) & (lp->rx_bd_num - 1)) >= MAX_SKB_FRAGS + 1) netif_wake_queue(ndev); } and this matches the value we use when enqueuing a packet tail_p = lp->tx_bd_p + sizeof(*lp->tx_bd_v) * (last_p - lp->tx_bd_v); smp_store_release(&lp->tx_bd_tail, new_tail_ptr); netdev_sent_queue(ndev, skb->len); > Regards, > Suraj > >> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> index 415e9bc252527..1ea8a6592bce1 100644 >> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> @@ -768,6 +768,7 @@ static int axienet_free_tx_chain(struct axienet_local >> *lp, u32 *sizep, int budge >> if (cur_p->skb) { >> struct axienet_cb *cb = (void *)cur_p->skb->cb; >> >> + *sizep += skb->len; >> dma_unmap_sgtable(lp->dev, &cb->sgt, DMA_TO_DEVICE, 0); >> sg_free_table_chained(&cb->sgt, XAE_INLINE_SG_CNT); >> napi_consume_skb(cur_p->skb, budget); @@ -783,8 +784,6 @@ >> static int axienet_free_tx_chain(struct axienet_local *lp, u32 *sizep, int budge >> wmb(); >> cur_p->cntrl = 0; >> cur_p->status = 0; >> - >> - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; >> } >> >> smp_store_release(&lp->tx_bd_ci, (ci + i) & (lp->tx_bd_num - 1)); >> >> > Fixes: c900e49d58eb ("net: xilinx: axienet: Implement BQL") >> > Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com> >> > --- >> > drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 +++ >> > .../net/ethernet/xilinx/xilinx_axienet_main.c | 20 +++++++++---------- >> > 2 files changed, 13 insertions(+), 10 deletions(-) >> > >> > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h >> > b/drivers/net/ethernet/xilinx/xilinx_axienet.h >> > index 602389843342..a4444c939451 100644 >> > --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h >> > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h >> > @@ -509,6 +509,8 @@ struct skbuf_dma_descriptor { >> > * complete. Only updated at runtime by TX NAPI poll. >> > * @tx_bd_tail: Stores the index of the next Tx buffer descriptor in the ring >> > * to be populated. >> > + * @tx_compl_bytes: Accumulates TX completion length until a full packet is >> > + * reported to the stack. >> > * @tx_packets: TX packet count for statistics >> > * @tx_bytes: TX byte count for statistics >> > * @tx_stat_sync: Synchronization object for TX stats @@ -592,6 >> > +594,7 @@ struct axienet_local { >> > u32 tx_bd_num; >> > u32 tx_bd_ci; >> > u32 tx_bd_tail; >> > + u32 tx_compl_bytes; >> > u64_stats_t tx_packets; >> > u64_stats_t tx_bytes; >> > struct u64_stats_sync tx_stat_sync; diff --git >> > a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> > b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> > index b06e4c37ff61..95bf61986cb7 100644 >> > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> > @@ -692,6 +692,8 @@ static void axienet_dma_stop(struct axienet_local >> *lp) >> > axienet_lock_mii(lp); >> > __axienet_device_reset(lp); >> > axienet_unlock_mii(lp); >> > + >> > + lp->tx_compl_bytes = 0; >> > } >> > >> > /** >> > @@ -770,8 +772,6 @@ static int axienet_device_reset(struct net_device >> *ndev) >> > * @first_bd: Index of first descriptor to clean up >> > * @nr_bds: Max number of descriptors to clean up >> > * @force: Whether to clean descriptors even if not complete >> > - * @sizep: Pointer to a u32 filled with the total sum of all bytes >> > - * in all cleaned-up descriptors. Ignored if NULL. >> > * @budget: NAPI budget (use 0 when not called from NAPI poll) >> > * >> > * Would either be called after a successful transmit operation, or >> > after @@ -780,7 +780,7 @@ static int axienet_device_reset(struct net_device >> *ndev) >> > * Return: The number of packets handled. >> > */ >> > static int axienet_free_tx_chain(struct axienet_local *lp, u32 first_bd, >> > - int nr_bds, bool force, u32 *sizep, int budget) >> > + int nr_bds, bool force, int budget) >> > { >> > struct axidma_bd *cur_p; >> > unsigned int status; >> > @@ -819,8 +819,8 @@ static int axienet_free_tx_chain(struct axienet_local >> *lp, u32 first_bd, >> > cur_p->cntrl = 0; >> > cur_p->status = 0; >> > >> > - if (sizep) >> > - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; >> > + if (!force) >> > + lp->tx_compl_bytes += status & >> > + XAXIDMA_BD_STS_ACTUAL_LEN_MASK; >> > } >> > >> > if (!force) { >> > @@ -999,18 +999,18 @@ static int axienet_tx_poll(struct napi_struct >> > *napi, int budget) { >> > struct axienet_local *lp = container_of(napi, struct axienet_local, napi_tx); >> > struct net_device *ndev = lp->ndev; >> > - u32 size = 0; >> > int packets; >> > >> > packets = axienet_free_tx_chain(lp, lp->tx_bd_ci, lp->tx_bd_num, false, >> > - &size, budget); >> > + budget); >> > >> > if (packets) { >> > - netdev_completed_queue(ndev, packets, size); >> > + netdev_completed_queue(ndev, packets, >> > + lp->tx_compl_bytes); >> > u64_stats_update_begin(&lp->tx_stat_sync); >> > u64_stats_add(&lp->tx_packets, packets); >> > - u64_stats_add(&lp->tx_bytes, size); >> > + u64_stats_add(&lp->tx_bytes, lp->tx_compl_bytes); >> > u64_stats_update_end(&lp->tx_stat_sync); >> > + lp->tx_compl_bytes = 0; >> > >> > /* Matches barrier in axienet_start_xmit */ >> > smp_mb(); >> > @@ -1115,7 +1115,7 @@ axienet_start_xmit(struct sk_buff *skb, struct >> net_device *ndev) >> > netdev_err(ndev, "TX DMA mapping error\n"); >> > ndev->stats.tx_dropped++; >> > axienet_free_tx_chain(lp, orig_tail_ptr, ii + 1, >> > - true, NULL, 0); >> > + true, 0); >> > dev_kfree_skb_any(skb); >> > return NETDEV_TX_OK; >> > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets 2026-03-26 15:38 ` Sean Anderson @ 2026-03-26 19:51 ` Gupta, Suraj 2026-03-26 19:56 ` Sean Anderson 0 siblings, 1 reply; 9+ messages in thread From: Gupta, Suraj @ 2026-03-26 19:51 UTC (permalink / raw) To: Sean Anderson, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, Simek, Michal, Pandey, Radhey Shyam, horms@kernel.org Cc: netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Katakam, Harini [Public] > -----Original Message----- > From: Sean Anderson <sean.anderson@linux.dev> > Sent: Thursday, March 26, 2026 9:08 PM > To: Gupta, Suraj <Suraj.Gupta2@amd.com>; andrew+netdev@lunn.ch; > davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > pabeni@redhat.com; Simek, Michal <michal.simek@amd.com>; Pandey, > Radhey Shyam <radhey.shyam.pandey@amd.com>; horms@kernel.org > Cc: netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org; Katakam, Harini <harini.katakam@amd.com> > Subject: Re: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD > TX packets > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > On 3/25/26 01:30, Gupta, Suraj wrote: > > [Public] > > > >> -----Original Message----- > >> From: Sean Anderson <sean.anderson@linux.dev> > >> Sent: Tuesday, March 24, 2026 9:39 PM > >> To: Gupta, Suraj <Suraj.Gupta2@amd.com>; andrew+netdev@lunn.ch; > >> davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > >> pabeni@redhat.com; Simek, Michal <michal.simek@amd.com>; Pandey, > >> Radhey Shyam <radhey.shyam.pandey@amd.com>; horms@kernel.org > >> Cc: netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > >> linux- kernel@vger.kernel.org; Katakam, Harini > >> <harini.katakam@amd.com> > >> Subject: Re: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting > >> for multi-BD TX packets > >> > >> Caution: This message originated from an External Source. Use proper > >> caution when opening attachments, clicking links, or responding. > >> > >> > >> On 3/24/26 10:53, Suraj Gupta wrote: > >> > When a TX packet spans multiple buffer descriptors > >> > (scatter-gather), the per-BD byte count is accumulated into a local > >> > variable that resets on each NAPI poll. If the BDs for a single > >> > packet complete across different polls, the earlier bytes are lost and never > credited to BQL. > >> > This causes BQL to think bytes are permanently in-flight, > >> > eventually stalling the TX queue. > >> > > >> > Fix this by replacing the local accumulator with a persistent > >> > counter > >> > (tx_compl_bytes) that survives across polls and is reset only after > >> > updating BQL and stats. > >> > >> Do we need this? Can't we just do something like > >> > > > > Nope, the 'size' variable passed to axienet_free_tx_chain() is local > > to axienet_tx_poll() and goes out of scope between different polls. > > This means it can't track completion bytes across multiple NAPI polls. > > Yes, but that's fine since we only update completed bytes when we retire at > least one packet: > > packets = axienet_free_tx_chain(lp, &size, budget); > if (packets) { > netdev_completed_queue(ndev, packets, size); > u64_stats_update_begin(&lp->tx_stat_sync); > u64_stats_add(&lp->tx_packets, packets); > u64_stats_add(&lp->tx_bytes, size); > u64_stats_update_end(&lp->tx_stat_sync); > > /* Matches barrier in axienet_start_xmit */ > smp_mb(); > if (((tail - ci) & (lp->rx_bd_num - 1)) >= MAX_SKB_FRAGS + 1) > netif_wake_queue(ndev); > } > > and this matches the value we use when enqueuing a packet > > tail_p = lp->tx_bd_p + sizeof(*lp->tx_bd_v) * (last_p - lp->tx_bd_v); > smp_store_release(&lp->tx_bd_tail, new_tail_ptr); > netdev_sent_queue(ndev, skb->len); > Let’s say we have a packet with 4 fragments. When xmit() is called, the last buffer descriptor (the 4th one) is marked with the retiral flag[1]. Now, during completion, suppose the first 3 buffer descriptors are processed in one NAPI poll, and the 4th descriptor is handled in a subsequent poll. In this scenario, the size won’t get updated for the first 3 BDs because there’s no packet retiral for them. Then, during the second poll, the size update will only reflect the completion length of the 4th BD. This means that the bytes corresponding to the first 3 fragments are never credited to BQL, which leads to incorrect accounting and, eventually, to the TX queue stalling as described. Proposed changes to accumulate the completion length across polls until the entire packet is processed directly addresses this gap. [1]: https://elixir.bootlin.com/linux/v7.0-rc5/source/drivers/net/ethernet/xilinx/xilinx_axienet_main.c#L1127 Regards, Suraj > > Regards, > > Suraj > > > >> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> index 415e9bc252527..1ea8a6592bce1 100644 > >> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> @@ -768,6 +768,7 @@ static int axienet_free_tx_chain(struct > >> axienet_local *lp, u32 *sizep, int budge > >> if (cur_p->skb) { > >> struct axienet_cb *cb = (void > >> *)cur_p->skb->cb; > >> > >> + *sizep += skb->len; > >> dma_unmap_sgtable(lp->dev, &cb->sgt, DMA_TO_DEVICE, 0); > >> sg_free_table_chained(&cb->sgt, XAE_INLINE_SG_CNT); > >> napi_consume_skb(cur_p->skb, budget); @@ > >> -783,8 +784,6 @@ static int axienet_free_tx_chain(struct axienet_local *lp, > u32 *sizep, int budge > >> wmb(); > >> cur_p->cntrl = 0; > >> cur_p->status = 0; > >> - > >> - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; > >> } > >> > >> smp_store_release(&lp->tx_bd_ci, (ci + i) & (lp->tx_bd_num - > >> 1)); > >> > >> > Fixes: c900e49d58eb ("net: xilinx: axienet: Implement BQL") > >> > Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com> > >> > --- > >> > drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 +++ > >> > .../net/ethernet/xilinx/xilinx_axienet_main.c | 20 > >> > +++++++++---------- > >> > 2 files changed, 13 insertions(+), 10 deletions(-) > >> > > >> > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h > >> > b/drivers/net/ethernet/xilinx/xilinx_axienet.h > >> > index 602389843342..a4444c939451 100644 > >> > --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h > >> > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h > >> > @@ -509,6 +509,8 @@ struct skbuf_dma_descriptor { > >> > * complete. Only updated at runtime by TX NAPI poll. > >> > * @tx_bd_tail: Stores the index of the next Tx buffer descriptor in the > ring > >> > * to be populated. > >> > + * @tx_compl_bytes: Accumulates TX completion length until a full > packet is > >> > + * reported to the stack. > >> > * @tx_packets: TX packet count for statistics > >> > * @tx_bytes: TX byte count for statistics > >> > * @tx_stat_sync: Synchronization object for TX stats @@ -592,6 > >> > +594,7 @@ struct axienet_local { > >> > u32 tx_bd_num; > >> > u32 tx_bd_ci; > >> > u32 tx_bd_tail; > >> > + u32 tx_compl_bytes; > >> > u64_stats_t tx_packets; > >> > u64_stats_t tx_bytes; > >> > struct u64_stats_sync tx_stat_sync; diff --git > >> > a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> > b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> > index b06e4c37ff61..95bf61986cb7 100644 > >> > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> > @@ -692,6 +692,8 @@ static void axienet_dma_stop(struct > >> > axienet_local > >> *lp) > >> > axienet_lock_mii(lp); > >> > __axienet_device_reset(lp); > >> > axienet_unlock_mii(lp); > >> > + > >> > + lp->tx_compl_bytes = 0; > >> > } > >> > > >> > /** > >> > @@ -770,8 +772,6 @@ static int axienet_device_reset(struct > >> > net_device > >> *ndev) > >> > * @first_bd: Index of first descriptor to clean up > >> > * @nr_bds: Max number of descriptors to clean up > >> > * @force: Whether to clean descriptors even if not complete > >> > - * @sizep: Pointer to a u32 filled with the total sum of all bytes > >> > - * in all cleaned-up descriptors. Ignored if NULL. > >> > * @budget: NAPI budget (use 0 when not called from NAPI poll) > >> > * > >> > * Would either be called after a successful transmit operation, > >> > or after @@ -780,7 +780,7 @@ static int axienet_device_reset(struct > >> > net_device > >> *ndev) > >> > * Return: The number of packets handled. > >> > */ > >> > static int axienet_free_tx_chain(struct axienet_local *lp, u32 first_bd, > >> > - int nr_bds, bool force, u32 *sizep, int budget) > >> > + int nr_bds, bool force, int budget) > >> > { > >> > struct axidma_bd *cur_p; > >> > unsigned int status; > >> > @@ -819,8 +819,8 @@ static int axienet_free_tx_chain(struct > >> > axienet_local > >> *lp, u32 first_bd, > >> > cur_p->cntrl = 0; > >> > cur_p->status = 0; > >> > > >> > - if (sizep) > >> > - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; > >> > + if (!force) > >> > + lp->tx_compl_bytes += status & > >> > + XAXIDMA_BD_STS_ACTUAL_LEN_MASK; > >> > } > >> > > >> > if (!force) { > >> > @@ -999,18 +999,18 @@ static int axienet_tx_poll(struct napi_struct > >> > *napi, int budget) { > >> > struct axienet_local *lp = container_of(napi, struct axienet_local, > napi_tx); > >> > struct net_device *ndev = lp->ndev; > >> > - u32 size = 0; > >> > int packets; > >> > > >> > packets = axienet_free_tx_chain(lp, lp->tx_bd_ci, lp->tx_bd_num, false, > >> > - &size, budget); > >> > + budget); > >> > > >> > if (packets) { > >> > - netdev_completed_queue(ndev, packets, size); > >> > + netdev_completed_queue(ndev, packets, > >> > + lp->tx_compl_bytes); > >> > u64_stats_update_begin(&lp->tx_stat_sync); > >> > u64_stats_add(&lp->tx_packets, packets); > >> > - u64_stats_add(&lp->tx_bytes, size); > >> > + u64_stats_add(&lp->tx_bytes, lp->tx_compl_bytes); > >> > u64_stats_update_end(&lp->tx_stat_sync); > >> > + lp->tx_compl_bytes = 0; > >> > > >> > /* Matches barrier in axienet_start_xmit */ > >> > smp_mb(); > >> > @@ -1115,7 +1115,7 @@ axienet_start_xmit(struct sk_buff *skb, > >> > struct > >> net_device *ndev) > >> > netdev_err(ndev, "TX DMA mapping error\n"); > >> > ndev->stats.tx_dropped++; > >> > axienet_free_tx_chain(lp, orig_tail_ptr, ii + 1, > >> > - true, NULL, 0); > >> > + true, 0); > >> > dev_kfree_skb_any(skb); > >> > return NETDEV_TX_OK; > >> > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets 2026-03-26 19:51 ` Gupta, Suraj @ 2026-03-26 19:56 ` Sean Anderson 0 siblings, 0 replies; 9+ messages in thread From: Sean Anderson @ 2026-03-26 19:56 UTC (permalink / raw) To: Gupta, Suraj, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, Simek, Michal, Pandey, Radhey Shyam, horms@kernel.org Cc: netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Katakam, Harini On 3/26/26 15:51, Gupta, Suraj wrote: > [Public] > >> -----Original Message----- >> From: Sean Anderson <sean.anderson@linux.dev> >> Sent: Thursday, March 26, 2026 9:08 PM >> To: Gupta, Suraj <Suraj.Gupta2@amd.com>; andrew+netdev@lunn.ch; >> davem@davemloft.net; edumazet@google.com; kuba@kernel.org; >> pabeni@redhat.com; Simek, Michal <michal.simek@amd.com>; Pandey, >> Radhey Shyam <radhey.shyam.pandey@amd.com>; horms@kernel.org >> Cc: netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- >> kernel@vger.kernel.org; Katakam, Harini <harini.katakam@amd.com> >> Subject: Re: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD >> TX packets >> >> Caution: This message originated from an External Source. Use proper caution >> when opening attachments, clicking links, or responding. >> >> >> On 3/25/26 01:30, Gupta, Suraj wrote: >> > [Public] >> > >> >> -----Original Message----- >> >> From: Sean Anderson <sean.anderson@linux.dev> >> >> Sent: Tuesday, March 24, 2026 9:39 PM >> >> To: Gupta, Suraj <Suraj.Gupta2@amd.com>; andrew+netdev@lunn.ch; >> >> davem@davemloft.net; edumazet@google.com; kuba@kernel.org; >> >> pabeni@redhat.com; Simek, Michal <michal.simek@amd.com>; Pandey, >> >> Radhey Shyam <radhey.shyam.pandey@amd.com>; horms@kernel.org >> >> Cc: netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >> >> linux- kernel@vger.kernel.org; Katakam, Harini >> >> <harini.katakam@amd.com> >> >> Subject: Re: [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting >> >> for multi-BD TX packets >> >> >> >> Caution: This message originated from an External Source. Use proper >> >> caution when opening attachments, clicking links, or responding. >> >> >> >> >> >> On 3/24/26 10:53, Suraj Gupta wrote: >> >> > When a TX packet spans multiple buffer descriptors >> >> > (scatter-gather), the per-BD byte count is accumulated into a local >> >> > variable that resets on each NAPI poll. If the BDs for a single >> >> > packet complete across different polls, the earlier bytes are lost and never >> credited to BQL. >> >> > This causes BQL to think bytes are permanently in-flight, >> >> > eventually stalling the TX queue. >> >> > >> >> > Fix this by replacing the local accumulator with a persistent >> >> > counter >> >> > (tx_compl_bytes) that survives across polls and is reset only after >> >> > updating BQL and stats. >> >> >> >> Do we need this? Can't we just do something like >> >> >> > >> > Nope, the 'size' variable passed to axienet_free_tx_chain() is local >> > to axienet_tx_poll() and goes out of scope between different polls. >> > This means it can't track completion bytes across multiple NAPI polls. >> >> Yes, but that's fine since we only update completed bytes when we retire at >> least one packet: >> >> packets = axienet_free_tx_chain(lp, &size, budget); >> if (packets) { >> netdev_completed_queue(ndev, packets, size); >> u64_stats_update_begin(&lp->tx_stat_sync); >> u64_stats_add(&lp->tx_packets, packets); >> u64_stats_add(&lp->tx_bytes, size); >> u64_stats_update_end(&lp->tx_stat_sync); >> >> /* Matches barrier in axienet_start_xmit */ >> smp_mb(); >> if (((tail - ci) & (lp->rx_bd_num - 1)) >= MAX_SKB_FRAGS + 1) >> netif_wake_queue(ndev); >> } >> >> and this matches the value we use when enqueuing a packet >> >> tail_p = lp->tx_bd_p + sizeof(*lp->tx_bd_v) * (last_p - lp->tx_bd_v); >> smp_store_release(&lp->tx_bd_tail, new_tail_ptr); >> netdev_sent_queue(ndev, skb->len); >> > > Let’s say we have a packet with 4 fragments. When xmit() is called, the last buffer descriptor (the 4th one) is marked with the retiral flag[1]. Now, during completion, suppose the first 3 buffer descriptors are processed in one NAPI poll, and the 4th descriptor is handled in a subsequent poll. In this scenario, the size won’t get updated for the first 3 BDs because there’s no packet retiral for them. Then, during the second poll, the size update will only reflect the completion length of the 4th BD. That's why I suggested using skb->len and not the size from the descriptor. --Sean > This means that the bytes corresponding to the first 3 fragments are never credited to BQL, which leads to incorrect accounting and, eventually, to the TX queue stalling as described. > Proposed changes to accumulate the completion length across polls until the entire packet is processed directly addresses this gap. > > [1]: https://elixir.bootlin.com/linux/v7.0-rc5/source/drivers/net/ethernet/xilinx/xilinx_axienet_main.c#L1127 > > > Regards, > Suraj > >> > Regards, >> > Suraj >> > >> >> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> >> b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> >> index 415e9bc252527..1ea8a6592bce1 100644 >> >> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> >> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> >> @@ -768,6 +768,7 @@ static int axienet_free_tx_chain(struct >> >> axienet_local *lp, u32 *sizep, int budge >> >> if (cur_p->skb) { >> >> struct axienet_cb *cb = (void >> >> *)cur_p->skb->cb; >> >> >> >> + *sizep += skb->len; >> >> dma_unmap_sgtable(lp->dev, &cb->sgt, DMA_TO_DEVICE, 0); >> >> sg_free_table_chained(&cb->sgt, XAE_INLINE_SG_CNT); >> >> napi_consume_skb(cur_p->skb, budget); @@ >> >> -783,8 +784,6 @@ static int axienet_free_tx_chain(struct axienet_local *lp, >> u32 *sizep, int budge >> >> wmb(); >> >> cur_p->cntrl = 0; >> >> cur_p->status = 0; >> >> - >> >> - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; >> >> } >> >> >> >> smp_store_release(&lp->tx_bd_ci, (ci + i) & (lp->tx_bd_num - >> >> 1)); >> >> >> >> > Fixes: c900e49d58eb ("net: xilinx: axienet: Implement BQL") >> >> > Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com> >> >> > --- >> >> > drivers/net/ethernet/xilinx/xilinx_axienet.h | 3 +++ >> >> > .../net/ethernet/xilinx/xilinx_axienet_main.c | 20 >> >> > +++++++++---------- >> >> > 2 files changed, 13 insertions(+), 10 deletions(-) >> >> > >> >> > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h >> >> > b/drivers/net/ethernet/xilinx/xilinx_axienet.h >> >> > index 602389843342..a4444c939451 100644 >> >> > --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h >> >> > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h >> >> > @@ -509,6 +509,8 @@ struct skbuf_dma_descriptor { >> >> > * complete. Only updated at runtime by TX NAPI poll. >> >> > * @tx_bd_tail: Stores the index of the next Tx buffer descriptor in the >> ring >> >> > * to be populated. >> >> > + * @tx_compl_bytes: Accumulates TX completion length until a full >> packet is >> >> > + * reported to the stack. >> >> > * @tx_packets: TX packet count for statistics >> >> > * @tx_bytes: TX byte count for statistics >> >> > * @tx_stat_sync: Synchronization object for TX stats @@ -592,6 >> >> > +594,7 @@ struct axienet_local { >> >> > u32 tx_bd_num; >> >> > u32 tx_bd_ci; >> >> > u32 tx_bd_tail; >> >> > + u32 tx_compl_bytes; >> >> > u64_stats_t tx_packets; >> >> > u64_stats_t tx_bytes; >> >> > struct u64_stats_sync tx_stat_sync; diff --git >> >> > a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> >> > b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> >> > index b06e4c37ff61..95bf61986cb7 100644 >> >> > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> >> > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> >> > @@ -692,6 +692,8 @@ static void axienet_dma_stop(struct >> >> > axienet_local >> >> *lp) >> >> > axienet_lock_mii(lp); >> >> > __axienet_device_reset(lp); >> >> > axienet_unlock_mii(lp); >> >> > + >> >> > + lp->tx_compl_bytes = 0; >> >> > } >> >> > >> >> > /** >> >> > @@ -770,8 +772,6 @@ static int axienet_device_reset(struct >> >> > net_device >> >> *ndev) >> >> > * @first_bd: Index of first descriptor to clean up >> >> > * @nr_bds: Max number of descriptors to clean up >> >> > * @force: Whether to clean descriptors even if not complete >> >> > - * @sizep: Pointer to a u32 filled with the total sum of all bytes >> >> > - * in all cleaned-up descriptors. Ignored if NULL. >> >> > * @budget: NAPI budget (use 0 when not called from NAPI poll) >> >> > * >> >> > * Would either be called after a successful transmit operation, >> >> > or after @@ -780,7 +780,7 @@ static int axienet_device_reset(struct >> >> > net_device >> >> *ndev) >> >> > * Return: The number of packets handled. >> >> > */ >> >> > static int axienet_free_tx_chain(struct axienet_local *lp, u32 first_bd, >> >> > - int nr_bds, bool force, u32 *sizep, int budget) >> >> > + int nr_bds, bool force, int budget) >> >> > { >> >> > struct axidma_bd *cur_p; >> >> > unsigned int status; >> >> > @@ -819,8 +819,8 @@ static int axienet_free_tx_chain(struct >> >> > axienet_local >> >> *lp, u32 first_bd, >> >> > cur_p->cntrl = 0; >> >> > cur_p->status = 0; >> >> > >> >> > - if (sizep) >> >> > - *sizep += status & XAXIDMA_BD_STS_ACTUAL_LEN_MASK; >> >> > + if (!force) >> >> > + lp->tx_compl_bytes += status & >> >> > + XAXIDMA_BD_STS_ACTUAL_LEN_MASK; >> >> > } >> >> > >> >> > if (!force) { >> >> > @@ -999,18 +999,18 @@ static int axienet_tx_poll(struct napi_struct >> >> > *napi, int budget) { >> >> > struct axienet_local *lp = container_of(napi, struct axienet_local, >> napi_tx); >> >> > struct net_device *ndev = lp->ndev; >> >> > - u32 size = 0; >> >> > int packets; >> >> > >> >> > packets = axienet_free_tx_chain(lp, lp->tx_bd_ci, lp->tx_bd_num, false, >> >> > - &size, budget); >> >> > + budget); >> >> > >> >> > if (packets) { >> >> > - netdev_completed_queue(ndev, packets, size); >> >> > + netdev_completed_queue(ndev, packets, >> >> > + lp->tx_compl_bytes); >> >> > u64_stats_update_begin(&lp->tx_stat_sync); >> >> > u64_stats_add(&lp->tx_packets, packets); >> >> > - u64_stats_add(&lp->tx_bytes, size); >> >> > + u64_stats_add(&lp->tx_bytes, lp->tx_compl_bytes); >> >> > u64_stats_update_end(&lp->tx_stat_sync); >> >> > + lp->tx_compl_bytes = 0; >> >> > >> >> > /* Matches barrier in axienet_start_xmit */ >> >> > smp_mb(); >> >> > @@ -1115,7 +1115,7 @@ axienet_start_xmit(struct sk_buff *skb, >> >> > struct >> >> net_device *ndev) >> >> > netdev_err(ndev, "TX DMA mapping error\n"); >> >> > ndev->stats.tx_dropped++; >> >> > axienet_free_tx_chain(lp, orig_tail_ptr, ii + 1, >> >> > - true, NULL, 0); >> >> > + true, 0); >> >> > dev_kfree_skb_any(skb); >> >> > return NETDEV_TX_OK; >> >> > } ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-03-26 19:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-24 14:53 [PATCH net 0/2] Correct BD length masks and BQL accounting for multi-BD TX packets Suraj Gupta 2026-03-24 14:53 ` [PATCH net 1/2] net: xilinx: axienet: Correct BD length masks to match AXIDMA IP spec Suraj Gupta 2026-03-24 16:09 ` Sean Anderson 2026-03-24 14:53 ` [PATCH net 2/2] net: xilinx: axienet: Fix BQL accounting for multi-BD TX packets Suraj Gupta 2026-03-24 16:09 ` Sean Anderson 2026-03-25 5:30 ` Gupta, Suraj 2026-03-26 15:38 ` Sean Anderson 2026-03-26 19:51 ` Gupta, Suraj 2026-03-26 19:56 ` Sean Anderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox