[PATCH net-next v2 2/5] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment

Théo Lebrun posted 5 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH net-next v2 2/5] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment
Posted by Théo Lebrun 3 months, 2 weeks ago
If HW is RSC capable, it cannot add dummy bytes at the start of IP
packets. Alignment (ie number of dummy bytes) is configured using the
RBOF field inside the NCFGR register.

On the software side, the skb_reserve(skb, NET_IP_ALIGN) call must only
be done if those dummy bytes are added by the hardware; notice the
skb_reserve() is done AFTER writing the address to the device.

We cannot do the skb_reserve() call BEFORE writing the address because
the address field ignores the low 2/3 bits. Conclusion: in some cases,
we risk not being able to respect the NET_IP_ALIGN value (which is
picked based on unaligned CPU access performance).

Fixes: 4df95131ea80 ("net/macb: change RX path for GEM")
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb.h      |  3 +++
 drivers/net/ethernet/cadence/macb_main.c | 21 ++++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 5b7d4cdb204d..93e8dd092313 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -537,6 +537,8 @@
 /* Bitfields in DCFG6. */
 #define GEM_PBUF_LSO_OFFSET			27
 #define GEM_PBUF_LSO_SIZE			1
+#define GEM_PBUF_RSC_OFFSET			26
+#define GEM_PBUF_RSC_SIZE			1
 #define GEM_PBUF_CUTTHRU_OFFSET			25
 #define GEM_PBUF_CUTTHRU_SIZE			1
 #define GEM_DAW64_OFFSET			23
@@ -775,6 +777,7 @@
 #define MACB_CAPS_MACB_IS_GEM			BIT(20)
 #define MACB_CAPS_DMA_64B			BIT(21)
 #define MACB_CAPS_DMA_PTP			BIT(22)
+#define MACB_CAPS_RSC				BIT(23)
 
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE			0x01
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 39673f5c3337..0817a99bb369 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1300,8 +1300,19 @@ static void gem_rx_refill(struct macb_queue *queue)
 			dma_wmb();
 			macb_set_addr(bp, desc, paddr);
 
-			/* properly align Ethernet header */
-			skb_reserve(skb, NET_IP_ALIGN);
+			/* Properly align Ethernet header.
+			 *
+			 * Hardware can add dummy bytes if asked using the RBOF
+			 * field inside the NCFGR register. That feature isn't
+			 * available if hardware is RSC capable.
+			 *
+			 * We cannot fallback to doing the 2-byte shift before
+			 * DMA mapping because the address field does not allow
+			 * setting the low 2/3 bits.
+			 * It is 3 bits if HW_DMA_CAP_PTP, else 2 bits.
+			 */
+			if (!(bp->caps & MACB_CAPS_RSC))
+				skb_reserve(skb, NET_IP_ALIGN);
 		} else {
 			desc->ctrl = 0;
 			dma_wmb();
@@ -2773,7 +2784,9 @@ static void macb_init_hw(struct macb *bp)
 	macb_set_hwaddr(bp);
 
 	config = macb_mdc_clk_div(bp);
-	config |= MACB_BF(RBOF, NET_IP_ALIGN);	/* Make eth data aligned */
+	/* Make eth data aligned. If RSC capable, that offset is ignored by HW. */
+	if (!(bp->caps & MACB_CAPS_RSC))
+		config |= MACB_BF(RBOF, NET_IP_ALIGN);
 	config |= MACB_BIT(DRFCS);		/* Discard Rx FCS */
 	if (bp->caps & MACB_CAPS_JUMBO)
 		config |= MACB_BIT(JFRAME);	/* Enable jumbo frames */
@@ -4321,6 +4334,8 @@ static void macb_configure_caps(struct macb *bp,
 		dcfg = gem_readl(bp, DCFG2);
 		if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF))) == 0)
 			bp->caps |= MACB_CAPS_FIFO_MODE;
+		if (GEM_BFEXT(PBUF_RSC, gem_readl(bp, DCFG6)))
+			bp->caps |= MACB_CAPS_RSC;
 		if (gem_has_ptp(bp)) {
 			if (!GEM_BFEXT(TSU, gem_readl(bp, DCFG5)))
 				dev_err(&bp->pdev->dev,

-- 
2.51.1

Re: [PATCH net-next v2 2/5] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment
Posted by Andrew Lunn 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 09:38:11AM +0200, Théo Lebrun wrote:
> If HW is RSC capable, it cannot add dummy bytes at the start of IP
> packets. Alignment (ie number of dummy bytes) is configured using the
> RBOF field inside the NCFGR register.
> 
> On the software side, the skb_reserve(skb, NET_IP_ALIGN) call must only
> be done if those dummy bytes are added by the hardware; notice the
> skb_reserve() is done AFTER writing the address to the device.
> 
> We cannot do the skb_reserve() call BEFORE writing the address because
> the address field ignores the low 2/3 bits. Conclusion: in some cases,
> we risk not being able to respect the NET_IP_ALIGN value (which is
> picked based on unaligned CPU access performance).
> 
> Fixes: 4df95131ea80 ("net/macb: change RX path for GEM")

Is this a real fix? You should not mix new development with
fixes. Either post this patch to net, or drop the Fixes: tag for
net-next.

    Andrew

---
pw-bot: cr
Re: [PATCH net-next v2 2/5] net: macb: match skb_reserve(skb, NET_IP_ALIGN) with HW alignment
Posted by Théo Lebrun 3 months, 2 weeks ago
On Wed Oct 22, 2025 at 9:27 PM CEST, Andrew Lunn wrote:
> On Wed, Oct 22, 2025 at 09:38:11AM +0200, Théo Lebrun wrote:
>> If HW is RSC capable, it cannot add dummy bytes at the start of IP
>> packets. Alignment (ie number of dummy bytes) is configured using the
>> RBOF field inside the NCFGR register.
>> 
>> On the software side, the skb_reserve(skb, NET_IP_ALIGN) call must only
>> be done if those dummy bytes are added by the hardware; notice the
>> skb_reserve() is done AFTER writing the address to the device.
>> 
>> We cannot do the skb_reserve() call BEFORE writing the address because
>> the address field ignores the low 2/3 bits. Conclusion: in some cases,
>> we risk not being able to respect the NET_IP_ALIGN value (which is
>> picked based on unaligned CPU access performance).
>> 
>> Fixes: 4df95131ea80 ("net/macb: change RX path for GEM")
>
> Is this a real fix? You should not mix new development with
> fixes. Either post this patch to net, or drop the Fixes: tag for
> net-next.

No, it isn't fixing any platform that currently uses the driver.
Dropped the "Fixes:" trailer for next revision.

Thanks Andrew,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com