Re: [PATCH net-next v2 1/5] net: lan966x: Add XDP_PACKET_HEADROOM

Horatiu Vultur
2022-11-16T18:48:51Z

The 11/16/2022 16:45, Alexander Lobakin wrote:
> [Some people who received this message don't often get email from alexandr.lobakin@intel.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> From: Horatiu Vultur <horatiu.vultur@microchip.com>
> Date: Tue, 15 Nov 2022 22:44:52 +0100
>
> > Update the page_pool params to allocate XDP_PACKET_HEADROOM space as
> > headroom for all received frames.
> > This is needed for when the XDP_TX and XDP_REDIRECT are implemented.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
>
> [...]
>
> > @@ -466,6 +470,7 @@ static struct sk_buff *lan966x_fdma_rx_get_frame(struct lan966x_rx *rx,
> >
> > skb_mark_for_recycle(skb);
> >
> > + skb_reserve(skb, XDP_PACKET_HEADROOM);
>
> Oh, forgot to ask previously. Just curious, which platforms do
> usually have this NIC? Do those platforms have
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS set?

I am running on ARM and I can see that
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set.

> If no, then adding %NET_SKB_PAD to the headroom can significantly
> improve performance, as currently you have 28 bytes of IFH + 14
> bytes of Eth header, so IP header is not aligned to 4 bytes
> boundary. Kernel and other drivers often expect IP header to be
> aligned. Adding %NET_SKB_PAD to the headroom addresses that.
> ...but be careful, I've just realized that you have IFH in front
> of Eth header, that means that it will also become unaligned after
> that change, so make sure you don't access it with words bigger
> than 2 bytes. Just test all the variants and pick the best :D

Thanks for a detail explanation!

>
> > skb_put(skb, FDMA_DCB_STATUS_BLOCKL(db->status));
> >
> > lan966x_ifh_get_timestamp(skb->data, &timestamp);
> > @@ -786,7 +791,8 @@ static int lan966x_fdma_get_max_frame(struct lan966x *lan966x)
> > return lan966x_fdma_get_max_mtu(lan966x) +
> > IFH_LEN_BYTES +
> > SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) +
> > - VLAN_HLEN * 2;
> > + VLAN_HLEN * 2 +
> > + XDP_PACKET_HEADROOM;
> > }
>
> [...]
>
> > --
> > 2.38.0
>
> Thanks,
> Olek

--
/Horatiu