Re: [PATCH net v2] net: stmmac: restore NET_IP_ALIGN in the RX DMA offset
David Laight <[email protected]>
| Newsgroups | org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <20260824152312.3f6dd2b7@pumpkin> |
On Mon, 24 Aug 2026 14:50:14 +0200 (CEST) Pascal Kneuper <[email protected]> wrote: > Since the RX path was converted to zero-copy, the page pool page is handed > to the stack directly as the skb head, and the offset the DMA engine writes > at is what determines the alignment of the packet headers. > > Before the conversion the payload was copied into an skb obtained from > napi_alloc_skb(), which reserves NET_SKB_PAD + NET_IP_ALIGN. The > conversion moved the headroom into stmmac_rx_offset() but did not carry > over NET_IP_ALIGN, so on architectures where NET_IP_ALIGN is 2 the IP > header now lands misaligned: > > 64 (NET_SKB_PAD) + 14 (ethernet) + 20 (IP) = 98 > > Same for the XDP branch: > > 256 (XDP_PACKET_HEADROOM) + 14 (ethernet) + 20 (IP) = 290 > > On ARM32 this is fatal, because ldm and ldrd trap on unaligned addresses > even when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set. I suspect an alternative is mark the structure(s) as __packed when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set. The compiler will then only use 'normal' memory instructions which won't fault. OTOH aligning the buffers is likely to be better. David > > Any received echo request panics the machine, e.g: > > Unhandled fault: alignment exception (0x001) at 0x81873062 > Internal error: : 1 [#1] SMP ARM > Hardware name: Altera SOCFPGA Arria10 > PC is at icmp_echo+0x38/0xa8 > LR is at icmp_rcv+0x22c/0x370 > Call trace: > icmp_echo from icmp_rcv+0x22c/0x370 > icmp_rcv from ip_protocol_deliver_rcu+0x2c/0x224 > ip_protocol_deliver_rcu from ip_local_deliver+0xc8/0x1a0 > ip_local_deliver from ip_sublist_rcv_finish+0x3c/0x50 > ip_sublist_rcv_finish from ip_list_rcv_finish+0x110/0x118 > ip_list_rcv_finish from ip_list_rcv+0xc8/0xdc > ip_list_rcv from __netif_receive_skb_list_core+0x170/0x1c0 > ... > napi_complete_done from stmmac_napi_poll_rx+0xcb0/0x1030 > Code: e24dd068 e59020a0 e28dc010 e0822001 (e8920003) > Kernel panic - not syncing: Fatal exception in interrupt > > The faulting instruction is the ldm of *icmp_hdr(skb) in icmp_echo(). > > Fix by adding NET_IP_ALIGN back to the RX offset, which restores the > alignment the stack used to get. > > Note that commit a955318fe67e ("stmmac: align RX buffers") made a similar > change in 2021 and was reverted by commit 12d125b4574b ("stmmac: Revert > "stmmac: align RX buffers"") because it caused packet corruption. That > patch raised the offset from 0 without adjusting the buffer size > accounting, so the DMA engine could arguably write past the end of the RX > buffers, though this was never root caused. > Commit df542f669307 ("net: stmmac: Switch to zero-copy in non-XDP RX > path") since derives the page pool allocation from stmmac_rx_offset(), so > the extra bytes are accounted for. > > Fixes: df542f669307 ("net: stmmac: Switch to zero-copy in non-XDP RX path") > Cc: Daniel Baldin <[email protected]> > Assisted-by: GitHub-Copilot-CLI:claude-opus-5 > Signed-off-by: Pascal Kneuper <[email protected]> > --- > v2: > - also add NET_IP_ALIGN to the XDP branch, reproduced the same panic > with an XDP_PASS program attached (Jakub Kicinski) > - retitle accordingly, v1 was non-XDP only > > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index a71f0df263785..4d4b155d0d931 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -1527,9 +1527,9 @@ static void stmmac_display_rings(struct stmmac_priv *priv, > static unsigned int stmmac_rx_offset(struct stmmac_priv *priv) > { > if (stmmac_xdp_is_enabled(priv)) > - return XDP_PACKET_HEADROOM; > + return XDP_PACKET_HEADROOM + NET_IP_ALIGN; > > - return NET_SKB_PAD; > + return NET_SKB_PAD + NET_IP_ALIGN; > } > > static int stmmac_set_bfsize(int mtu)