Re: [PATCH net v4 7/7] net: stmmac: selftests: Account for alignment shift on dwmac1000 for Jumbo test

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete fix: the NET_IP_ALIGN compensation is applied only to
  the selftest packet, while the driver keeps programming the full,…

--

net: stmmac: selftests: Account for alignment shift on dwmac1000 for Jumbo test

The Jumbo selftest sends a frame sized from dma_conf.dma_buf_sz, which
started failing on legacy cores after NET_IP_ALIGN was restored in the RX
DMA offset.  The patch shrinks the test frame by NET_IP_ALIGN on non-xmac
cores so the frame no longer spills into a second descriptor.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> index 43b8411c51120..c25dc9f89270c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> @@ -1789,6 +1789,9 @@ static int __stmmac_test_jumbo(struct stmmac_priv *priv, u16 queue)
>  	struct stmmac_packet_attrs attr = { };
>  	int size = priv->dma_conf.dma_buf_sz;
>  
> +	if (!dwmac_is_xmac(priv->plat->core_type))
> +		size -= NET_IP_ALIGN;
> +

[Severity: High]
If the hardware behaviour described in the changelog is accurate, does
this only hide the mismatch in __stmmac_test_jumbo() while leaving the
same shortfall on the receive path for real traffic?

The RX DMA start address is shifted by NET_IP_ALIGN:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_rx_offset() {
	if (stmmac_xdp_is_enabled(priv))
		return XDP_PACKET_HEADROOM + NET_IP_ALIGN;

	return NET_SKB_PAD + NET_IP_ALIGN;
}

but the descriptor buffer length programmed for that shifted address is
still the full dma_buf_sz:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_clear_rx_descriptors() {
		stmmac_init_rx_desc(priv, desc, priv->use_riwt,
				    priv->descriptor_mode,
				    (i == dma_conf->dma_rx_size - 1),
				    dma_conf->dma_buf_sz);
}

drivers/net/ethernet/stmicro/stmmac/enh_desc.c:enh_desc_init_rx_desc() {
	bfsize1 = min(bfsize, BUF_SIZE_8KiB);
	p->des1 |= cpu_to_le32(bfsize1 & ERDES1_BUFFER1_SIZE_MASK);
}

So on non-xmac cores the usable single-descriptor capacity is
dma_buf_sz - NET_IP_ALIGN, which is exactly what this patch encodes in the
test, but nothing reduces the value handed to the hardware.

Can a frame from the wire hit that same boundary?  The buffer size comes
from the MTU, and the rounding uses strict greater-than comparisons:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_setup_dma_desc() {
	len = mtu + ETH_HLEN + 2 * VLAN_HLEN + ETH_FCS_LEN;
	...
	if (bfsize < BUF_SIZE_16KiB)
		bfsize = stmmac_set_bfsize(len);
}

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_set_bfsize() {
	else if (len > DEFAULT_BUFSIZE)
		ret = BUF_SIZE_2KiB;
	else
		ret = DEFAULT_BUFSIZE;
}

With DEFAULT_BUFSIZE == 1536, an MTU of 1510 gives len == 1536 and
dma_buf_sz == 1536; MTU 2022 gives 2048, MTU 4070 gives 4096.  For those
MTUs a maximum-size double-tagged frame including FCS is exactly
dma_buf_sz bytes, so under the changelog's model it needs one more 4-byte
burst than the descriptor advertises and continues into the next
descriptor, where the legacy status handlers drop it:

drivers/net/ethernet/stmicro/stmmac/enh_desc.c:enh_desc_get_rx_status() {
	if (unlikely(!(rdes0 & RDES0_LAST_DESCRIPTOR))) {
		x->rx_length++;
		return discard_frame;
	}
}

drivers/net/ethernet/stmicro/stmmac/norm_desc.c:ndesc_get_rx_status() has
the same check.

Would it be better to subtract the offset misalignment from the buffer
length programmed into the descriptors (or add the NET_IP_ALIGN headroom
to the bfsize computation for non-xmac cores), so that dma_buf_sz keeps
matching what the hardware can actually store in one descriptor?

The patch also carries a Fixes: tag for 23680bf5f8c6, which invites a
stable backport.  Since the change adjusts only the selftest, does the
backport carry the tag without addressing the receive-side exposure?

One more consequence: __stmmac_test_jumbo() is the only in-tree probe of
the advertised single-descriptor maximum.  After this change, does a
future mismatch between the programmed buffer length and the real DMA
capacity stay undetected by the Jumbo test?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917215339.2022523-1-maxime.chevallier%40bootlin.com