* [PATCH net-next] net: cadence: macb: Implement BQL
@ 2025-02-14 21:16 Sean Anderson
2025-02-19 1:57 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Sean Anderson @ 2025-02-14 21:16 UTC (permalink / raw)
To: Nicolas Ferre, Claudiu Beznea, netdev
Cc: David S . Miller, Andrew Lunn, linux-kernel, Jakub Kicinski,
Paolo Abeni, Eric Dumazet, Sean Anderson
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net-next] net: cadence: macb: Implement BQL
2025-02-14 21:16 [PATCH net-next] net: cadence: macb: Implement BQL Sean Anderson
@ 2025-02-19 1:57 ` Jakub Kicinski
2025-02-20 15:55 ` Sean Anderson
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2025-02-19 1:57 UTC (permalink / raw)
To: Sean Anderson
Cc: Nicolas Ferre, Claudiu Beznea, netdev, David S . Miller,
Andrew Lunn, linux-kernel, Paolo Abeni, Eric Dumazet
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net-next] net: cadence: macb: Implement BQL
2025-02-19 1:57 ` Jakub Kicinski
@ 2025-02-20 15:55 ` Sean Anderson
2025-02-20 16:51 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Sean Anderson @ 2025-02-20 15:55 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Nicolas Ferre, Claudiu Beznea, netdev, David S . Miller,
Andrew Lunn, linux-kernel, Paolo Abeni, Eric Dumazet
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net-next] net: cadence: macb: Implement BQL
2025-02-20 15:55 ` Sean Anderson
@ 2025-02-20 16:51 ` Jakub Kicinski
2025-02-20 16:53 ` Sean Anderson
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2025-02-20 16:51 UTC (permalink / raw)
To: Sean Anderson
Cc: Nicolas Ferre, Claudiu Beznea, netdev, David S . Miller,
Andrew Lunn, linux-kernel, Paolo Abeni, Eric Dumazet
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?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: cadence: macb: Implement BQL
2025-02-20 16:51 ` Jakub Kicinski
@ 2025-02-20 16:53 ` Sean Anderson
0 siblings, 0 replies; 5+ messages in thread
From: Sean Anderson @ 2025-02-20 16:53 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Nicolas Ferre, Claudiu Beznea, netdev, David S . Miller,
Andrew Lunn, linux-kernel, Paolo Abeni, Eric Dumazet
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-02-20 16:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-14 21:16 [PATCH net-next] net: cadence: macb: Implement BQL Sean Anderson
2025-02-19 1:57 ` Jakub Kicinski
2025-02-20 15:55 ` Sean Anderson
2025-02-20 16:51 ` Jakub Kicinski
2025-02-20 16:53 ` Sean Anderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).