Thread (21 messages) 21 messages, 2 authors, 2026-01-13

Re: [PATCH RFC net-next v2 3/8] cadence: macb: Add page pool support handle multi-descriptor frame rx

From: Théo Lebrun <theo.lebrun@bootlin.com>
Date: 2026-01-13 10:43:13

On Mon Jan 12, 2026 at 3:16 PM CET, Paolo Valerio wrote:
On 08 Jan 2026 at 04:43:43 PM, Théo Lebrun [off-list ref] wrote:
quoted
On Sun Dec 21, 2025 at 12:51 AM CET, Paolo Valerio wrote:
quoted
@@ -1382,58 +1382,118 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
+		first_frame = ctrl & MACB_BIT(RX_SOF);
 		len = ctrl & bp->rx_frm_len_mask;
 
-		netdev_vdbg(bp->dev, "gem_rx %u (len %u)\n", entry, len);
+		if (len) {
+			data_len = len;
+			if (!first_frame)
+				data_len -= queue->skb->len;
+		} else {
+			data_len = bp->rx_buffer_size;
+		}
Why deal with the `!len` case? How can it occur? User guide doesn't hint
that. It would mean we would grab uninitialised bytes as we assume len
is the max buffer size.
Good point. After taking a second look, !len may not be the most reliable
way to check this.
From the datasheet, status signals are only valid (with some exceptions)
when MACB_BIT(RX_EOF) is set. As a side effect, len is always zero on my
hw for frames without the EOF bit, but it's probably better to just rely
on MACB_BIT(RX_EOF) instead of reading something that may end up being
unreliable.
100%, I do agree!
quoted
quoted
+		bp->rx_buffer_size = SKB_DATA_ALIGN(size);
+		if (gem_total_rx_buffer_size(bp) > PAGE_SIZE) {
+			overhead = bp->rx_headroom +
+				SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+			bp->rx_buffer_size = rounddown(PAGE_SIZE - overhead,
+						       RX_BUFFER_MULTIPLE);
+		}
I've seen your comment in [0/8]. Do you have any advice on how to test
this clamping? All I can think of is to either configure a massive MTU
or, more easily, cheat with the headroom.
I normally test the set with 4k PAGE_SIZE and, as you said, setting the
mtu to something bigger than that. This is still possible with 8k pages
(given .jumbo_max_len = 10240).
Ah yes there is .jumbo_max_len, but our PAGE_SIZE==16K > .jumbo_max_len
so we cannot land in that codepath.
quoted
Also, should we warn? It means MTU-sized packets will be received in
fragments. It will work but is probably unexpected by users and a
slowdown reason that users might want to know about.
I'm not sure about the warning as I don't see this as a user level detail.
For debugging purpose, I guess we should be fine the last print out (even
better once extended with your suggestion). Of course, feel free to disagree.
I'm fine with no warnings. We'll check our performance anyways. :-)
If it changes we'll notice.

Regards,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help