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

[Matteo Croce]

On Mon, 16 Aug 2021 08:12:08 -0700
Jakub Kicinski <kuba@xxxxxxxxxx> wrote:

> On Thu, 12 Aug 2021 12:05:38 +0100 Marc Zyngier wrote:
> > > A possible fix, which takes in account also the XDP headroom for
> > > stmmac_rx_buf1_len() only could be (only compile tested, I don't
> > > have the hardware now):  
> > 
> > However, this doesn't fix my issue. I still get all sort of
> > corruption. Probably stmmac_rx_buf2_len() also need adjusting (it
> > has a similar logic as its buf1 counterpart...)
> > 
> > Unless you can fix it very quickly, and given that we're towards the
> > end of the cycle, I'd be more comfortable if we reverted this patch.
> 
> Any luck investigating this one? The rc6 announcement sounds like
> there may not be that many more rc releases for 5.14.

Hi Jackub.

Unfortunately I have only a device with stmmac, and it works fine with
the patch. It seems that not all hardware suffers from this issue.

Also, using NET_IP_ALIGN on RX is a common pattern, I think that any
ethernet device is doing the same to align the IPv4 header.

Anyway, I asked for two tests on the affected device:
1. Change NET_IP_ALIGN with 8, to see if the DMA has problems in
   receiving to a non word aligned address
2. load a nop XDP program (I provided one), to see if the problem is
   already there when XDP is used

I doubt that changing also stmmac_rx_buf2_len would help,
but it's worth a try, here is a patch:

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 7b8404a21544..73d1f0ec66ff 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -93,7 +93,7 @@ static int tc = TC_DEFAULT;
 module_param(tc, int, 0644);
 MODULE_PARM_DESC(tc, "DMA threshold control value");
 
-#define	DEFAULT_BUFSIZE	1536
+#define	DEFAULT_BUFSIZE	1536 + XDP_PACKET_HEADROOM + NET_IP_ALIGN
 static int buf_sz = DEFAULT_BUFSIZE;
 module_param(buf_sz, int, 0644);
 MODULE_PARM_DESC(buf_sz, "DMA buffer size");
@@ -4508,12 +4508,12 @@ static unsigned int stmmac_rx_buf1_len(struct stmmac_priv *priv,
 
 	/* First descriptor, not last descriptor and not split header */
 	if (status & rx_not_ls)
-		return priv->dma_buf_sz;
+		return priv->dma_buf_sz - stmmac_rx_offset(priv);
 
 	plen = stmmac_get_rx_frame_len(priv, p, coe);
 
 	/* First descriptor and last descriptor and not split header */
-	return min_t(unsigned int, priv->dma_buf_sz, plen);
+	return min_t(unsigned int, priv->dma_buf_sz - stmmac_rx_offset(priv), plen);
 }
 
 static unsigned int stmmac_rx_buf2_len(struct stmmac_priv *priv,
@@ -4529,12 +4529,12 @@ static unsigned int stmmac_rx_buf2_len(struct stmmac_priv *priv,
 
 	/* Not last descriptor */
 	if (status & rx_not_ls)
-		return priv->dma_buf_sz;
+		return priv->dma_buf_sz - stmmac_rx_offset(priv);
 
 	plen = stmmac_get_rx_frame_len(priv, p, coe);
 
 	/* Last descriptor */
-	return plen - len;
+	return plen - len - stmmac_rx_offset(priv);
 }
 
 static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,


Regards,
-- 
per aspera ad upstream