When sending out any kind of traffic, it is essential that the driver
keeps reporting BQL of the number of bytes that have been sent so that
BQL can track the amount of data in the queue and prevents it from
overflowing. If BQL is not reported, the driver may continue sending
packets even when the queue is full, leading to packet loss, congestion
and decreased network performance. Currently this is missing in
emac_xmit_xdp_frame() and this patch fixes it.

Fixes: 62aa3246f462 ("net: ti: icssg-prueth: Add XDP support")
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---
 drivers/net/ethernet/ti/icssg/icssg_common.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index b4be76e13a2f..4f45f2b6b67f 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -187,7 +187,6 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
 			xdp_return_frame(xdpf);
 			break;
 		default:
-			netdev_err(ndev, "tx_complete: invalid swdata type %d\n", swdata->type);
 			prueth_xmit_free(tx_chn, desc_tx);
 			ndev->stats.tx_dropped++;
 			continue;
@@ -567,6 +566,7 @@ u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
 {
 	struct cppi5_host_desc_t *first_desc;
 	struct net_device *ndev = emac->ndev;
+	struct netdev_queue *netif_txq;
 	struct prueth_tx_chn *tx_chn;
 	dma_addr_t desc_dma, buf_dma;
 	struct prueth_swdata *swdata;
@@ -620,12 +620,17 @@ u32 emac_xmit_xdp_frame(struct prueth_emac *emac,
 		swdata->data.xdpf = xdpf;
 	}
 
+	/* Report BQL before sending the packet */
+	netif_txq = netdev_get_tx_queue(ndev, tx_chn->id);
+	netdev_tx_sent_queue(netif_txq, xdpf->len);
+
 	cppi5_hdesc_set_pktlen(first_desc, xdpf->len);
 	desc_dma = k3_cppi_desc_pool_virt2dma(tx_chn->desc_pool, first_desc);
 
 	ret = k3_udma_glue_push_tx_chn(tx_chn->tx_chn, first_desc, desc_dma);
 	if (ret) {
 		netdev_err(ndev, "xdp tx: push failed: %d\n", ret);
+		netdev_tx_completed_queue(netif_txq, 1, xdpf->len);
 		goto drop_free_descs;
 	}
 
@@ -979,6 +984,7 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
 	ret = k3_udma_glue_push_tx_chn(tx_chn->tx_chn, first_desc, desc_dma);
 	if (ret) {
 		netdev_err(ndev, "tx: push failed: %d\n", ret);
+		netdev_tx_completed_queue(netif_txq, 1, pkt_len);
 		goto drop_free_descs;
 	}
 
-- 
2.43.0

On Mon, 28 Apr 2025 17:34:57 +0530 Meghana Malladi wrote:
> When sending out any kind of traffic, it is essential that the driver
> keeps reporting BQL of the number of bytes that have been sent so that
> BQL can track the amount of data in the queue and prevents it from
> overflowing. If BQL is not reported, the driver may continue sending
> packets even when the queue is full, leading to packet loss, congestion
> and decreased network performance. Currently this is missing in
> emac_xmit_xdp_frame() and this patch fixes it.

The ordering of patches in the series is a bit off.
The order should be something like:

  net: ti: icssg-prueth: Set XDP feature flags for ndev
  net: ti: icssg-prueth: Fix kernel panic during concurrent Tx queue ...
  net: ti: icssg-prueth: Fix race condition for traffic from different ...
  net: ti: icssg-prueth: Report BQL before sending XDP packets

This patch is not correct without the extra locking in place.

Hi Jakub,

On 5/1/2025 8:23 PM, Jakub Kicinski wrote:
> On Mon, 28 Apr 2025 17:34:57 +0530 Meghana Malladi wrote:
>> When sending out any kind of traffic, it is essential that the driver
>> keeps reporting BQL of the number of bytes that have been sent so that
>> BQL can track the amount of data in the queue and prevents it from
>> overflowing. If BQL is not reported, the driver may continue sending
>> packets even when the queue is full, leading to packet loss, congestion
>> and decreased network performance. Currently this is missing in
>> emac_xmit_xdp_frame() and this patch fixes it.
> 
> The ordering of patches in the series is a bit off.
> The order should be something like:
> 
>    net: ti: icssg-prueth: Set XDP feature flags for ndev
>    net: ti: icssg-prueth: Fix kernel panic during concurrent Tx queue ...
>    net: ti: icssg-prueth: Fix race condition for traffic from different ...
>    net: ti: icssg-prueth: Report BQL before sending XDP packets
> 
> This patch is not correct without the extra locking in place.

Actually the order of bug fixes which I posted, is the order in which I 
fixed the bugs, while running xdp-trafficgen:
1. ndo_xdp_xmit() wasn't getting called because of missing XDP feature 
flags check.
2. kernel crash: kernel BUG at lib/dynamic_queue_limits.c:99! and was 
fixed by reporting BQL for packet transmission.My bad, I forgot  to add 
this in the commit message.
3. kernel crash: kernel BUG at lib/genalloc.c:508! due to memory 
corruption in DMA descriptors and fixed it with spinlock
4. kernel crash: kernel BUG at lib/dynamic_queue_limits.c:99! due to 
race condition in netif_txq and fixed it with __netif_tx_lock()

But yeah I think the order you suggested makes more sense. Let me try 
that and post v2.

-- 
Thanks,
Meghana Malladi