Thread (37 messages) 37 messages, 8 authors, 2021-08-20

Re: [PATCH net-next] stmmac: align RX buffers

From: Matteo Croce <hidden>
Date: 2021-08-20 18:15:02
Also in: linux-riscv, lkml

On Fri, Aug 20, 2021 at 8:09 PM Marc Zyngier [off-list ref] wrote:
On Fri, 20 Aug 2021 18:56:33 +0100,
Matteo Croce [off-list ref] wrote:
quoted
On Fri, Aug 20, 2021 at 7:51 PM Marc Zyngier [off-list ref] wrote:
quoted
On Fri, 20 Aug 2021 18:35:45 +0100,
Matteo Croce [off-list ref] wrote:
quoted
quoted
quoted
I think it's wrong. The original offset was 0, and to align it to the
boundary we need to add just NET_IP_ALIGN, which is two.
NET_SKB_PAD is a much bigger value, (I think 64), which is used to
reserve space to prepend an header, e.g. with tunnels.
How about the other adjustments that Eric mentioned regarding the size
of the buffer? Aren't they required?
I guess that if stmmac_rx_buf1_len() needed such adjustment, it would
be already broken when XDP is in use.
When you use XDP, stmmac_rx_offset() adds a pretty big headroom of 256
byte, which would easily trigger an overflow if not accounted.
Did you try attaching a simple XDP program on a stock 5.13 kernel?
Yes, as mentioned in [1], to which you replied...

        M.

[1] https://lore.kernel.org/r/87wnohqty1.wl-maz@kernel.org (local)
Great.
So I doubt that the adjustment is needed.
Does it work with all the frame size?
I have no idea. Honestly, you are the one who should be able to answer
these questions, given that you should have worked out how the buffer
allocations work in this particular driver.

This whole "let's try another random set of values until something
sticks" is not how things ought to be done, and doesn't fill me with
the utmost confidence that 5.14 (which apparently may well be cut in
*two days*) is going to have a solid stmmac driver.

I re-re-request that this patch gets reverted until you figure out
what is wrong with the initial patch.

Thanks,
I would have done it, but I'll not have the hardware until next week at least,
otherwise I'd have tried all these tests myself.

I'm sure that NET_SKB_PAD doesn't need to be there, if just removing
it fixes the problem, consider applying it and put a Fixes tag.

-- 
per aspera ad upstream
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help