Implement byte queue limits to allow queuing disciplines to account for
packets enqueued in the ring buffer but not yet transmitted. There are a
separate set of transmit functions for AT91 that I haven't touched since
I don't have hardware to test on.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/net/ethernet/cadence/macb_main.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 48496209fb16..63c65b4bb348 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1081,6 +1081,9 @@ static void macb_tx_error_task(struct work_struct *work)
 						      tx_error_task);
 	bool			halt_timeout = false;
 	struct macb		*bp = queue->bp;
+	u32			queue_index = queue - bp->queues;
+	u32			packets = 0;
+	u32			bytes = 0;
 	struct macb_tx_skb	*tx_skb;
 	struct macb_dma_desc	*desc;
 	struct sk_buff		*skb;
@@ -1088,8 +1091,7 @@ static void macb_tx_error_task(struct work_struct *work)
 	unsigned long		flags;
 
 	netdev_vdbg(bp->dev, "macb_tx_error_task: q = %u, t = %u, h = %u\n",
-		    (unsigned int)(queue - bp->queues),
-		    queue->tx_tail, queue->tx_head);
+		    queue_index, queue->tx_tail, queue->tx_head);
 
 	/* Prevent the queue NAPI TX poll from running, as it calls
 	 * macb_tx_complete(), which in turn may call netif_wake_subqueue().
@@ -1142,8 +1144,10 @@ static void macb_tx_error_task(struct work_struct *work)
 					    skb->data);
 				bp->dev->stats.tx_packets++;
 				queue->stats.tx_packets++;
+				packets++;
 				bp->dev->stats.tx_bytes += skb->len;
 				queue->stats.tx_bytes += skb->len;
+				bytes += skb->len;
 			}
 		} else {
 			/* "Buffers exhausted mid-frame" errors may only happen
@@ -1160,6 +1164,9 @@ static void macb_tx_error_task(struct work_struct *work)
 		macb_tx_unmap(bp, tx_skb, 0);
 	}
 
+	netdev_tx_completed_queue(netdev_get_tx_queue(bp->dev, queue_index),
+				  packets, bytes);
+
 	/* Set end of TX queue */
 	desc = macb_tx_desc(queue, 0);
 	macb_set_addr(bp, desc, 0);
@@ -1230,6 +1237,7 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 	unsigned int tail;
 	unsigned int head;
 	int packets = 0;
+	u32 bytes = 0;
 
 	spin_lock(&queue->tx_ptr_lock);
 	head = queue->tx_head;
@@ -1271,6 +1279,7 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 				bp->dev->stats.tx_bytes += skb->len;
 				queue->stats.tx_bytes += skb->len;
 				packets++;
+				bytes += skb->len;
 			}
 
 			/* Now we can safely release resources */
@@ -1285,6 +1294,9 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 		}
 	}
 
+	netdev_tx_completed_queue(netdev_get_tx_queue(bp->dev, queue_index),
+				  packets, bytes);
+
 	queue->tx_tail = tail;
 	if (__netif_subqueue_stopped(bp->dev, queue_index) &&
 	    CIRC_CNT(queue->tx_head, queue->tx_tail,
@@ -2386,6 +2398,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Make newly initialized descriptor visible to hardware */
 	wmb();
 	skb_tx_timestamp(skb);
+	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index),
+			     skb->len);
 
 	spin_lock_irq(&bp->lock);
 	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
@@ -3019,6 +3033,7 @@ static int macb_close(struct net_device *dev)
 	netif_tx_stop_all_queues(dev);
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
+		netdev_tx_reset_queue(netdev_get_tx_queue(dev, q));
 		napi_disable(&queue->napi_rx);
 		napi_disable(&queue->napi_tx);
 	}
-- 
2.35.1.1320.gc452695387.dirty

On Fri, 14 Feb 2025 16:16:43 -0500 Sean Anderson wrote:
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 48496209fb16..63c65b4bb348 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1081,6 +1081,9 @@ static void macb_tx_error_task(struct work_struct *work)
>  						      tx_error_task);
>  	bool			halt_timeout = false;
>  	struct macb		*bp = queue->bp;
> +	u32			queue_index = queue - bp->queues;

nit: breaking reverse xmas tree here

> +	u32			packets = 0;
> +	u32			bytes = 0;
>  	struct macb_tx_skb	*tx_skb;
>  	struct macb_dma_desc	*desc;
>  	struct sk_buff		*skb;


> @@ -3019,6 +3033,7 @@ static int macb_close(struct net_device *dev)
>  	netif_tx_stop_all_queues(dev);
>  
>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> +		netdev_tx_reset_queue(netdev_get_tx_queue(dev, q));
>  		napi_disable(&queue->napi_rx);
>  		napi_disable(&queue->napi_tx);

I think you should reset after napi_disable()? 
Lest NAPI runs after the reset and tries to complete on an empty queue..
-- 
pw-bot: cr

On 2/18/25 20:57, Jakub Kicinski wrote:
> On Fri, 14 Feb 2025 16:16:43 -0500 Sean Anderson wrote:
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> index 48496209fb16..63c65b4bb348 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -1081,6 +1081,9 @@ static void macb_tx_error_task(struct work_struct *work)
>>  						      tx_error_task);
>>  	bool			halt_timeout = false;
>>  	struct macb		*bp = queue->bp;
>> +	u32			queue_index = queue - bp->queues;
> 
> nit: breaking reverse xmas tree here

It has to happen here since bp isn't available earlier.

>> +	u32			packets = 0;
>> +	u32			bytes = 0;
>>  	struct macb_tx_skb	*tx_skb;
>>  	struct macb_dma_desc	*desc;
>>  	struct sk_buff		*skb;
> 
> 
>> @@ -3019,6 +3033,7 @@ static int macb_close(struct net_device *dev)
>>  	netif_tx_stop_all_queues(dev);
>>  
>>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
>> +		netdev_tx_reset_queue(netdev_get_tx_queue(dev, q));
>>  		napi_disable(&queue->napi_rx);
>>  		napi_disable(&queue->napi_tx);
> 
> I think you should reset after napi_disable()? 
> Lest NAPI runs after the reset and tries to complete on an empty queue..

OK.

--Sean

On Thu, 20 Feb 2025 10:55:45 -0500 Sean Anderson wrote:
> On 2/18/25 20:57, Jakub Kicinski wrote:
> > On Fri, 14 Feb 2025 16:16:43 -0500 Sean Anderson wrote:  
> >> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> >> index 48496209fb16..63c65b4bb348 100644
> >> --- a/drivers/net/ethernet/cadence/macb_main.c
> >> +++ b/drivers/net/ethernet/cadence/macb_main.c
> >> @@ -1081,6 +1081,9 @@ static void macb_tx_error_task(struct work_struct *work)
> >>  						      tx_error_task);
> >>  	bool			halt_timeout = false;
> >>  	struct macb		*bp = queue->bp;
> >> +	u32			queue_index = queue - bp->queues;  
> > 
> > nit: breaking reverse xmas tree here  
> 
> It has to happen here since bp isn't available earlier.

Move it from init to normal code?

On 2/20/25 11:51, Jakub Kicinski wrote:
> On Thu, 20 Feb 2025 10:55:45 -0500 Sean Anderson wrote:
>> On 2/18/25 20:57, Jakub Kicinski wrote:
>> > On Fri, 14 Feb 2025 16:16:43 -0500 Sean Anderson wrote:  
>> >> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> >> index 48496209fb16..63c65b4bb348 100644
>> >> --- a/drivers/net/ethernet/cadence/macb_main.c
>> >> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> >> @@ -1081,6 +1081,9 @@ static void macb_tx_error_task(struct work_struct *work)
>> >>  						      tx_error_task);
>> >>  	bool			halt_timeout = false;
>> >>  	struct macb		*bp = queue->bp;
>> >> +	u32			queue_index = queue - bp->queues;  
>> > 
>> > nit: breaking reverse xmas tree here  
>> 
>> It has to happen here since bp isn't available earlier.
> 
> Move it from init to normal code?

That's what I ended up doing.

--Sean