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