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

[Nicolas Ferre <nicolas.ferre@microchip.com>]

On 23/10/2025 at 18:22, 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).
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

For the record:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks Théo! Regards,
   Nicolas

> ---
>   drivers/net/ethernet/cadence/macb.h      |  3 +++
>   drivers/net/ethernet/cadence/macb_main.c | 23 ++++++++++++++++++++---
>   2 files changed, 23 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..be3d0c2313a1 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,11 @@ 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 +4336,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
>