[net-next,v2,1/5] net: lan966x: Add XDP_PACKET_HEADROOM
Commit Message
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>
---
.../ethernet/microchip/lan966x/lan966x_fdma.c | 16 +++++++++++-----
.../net/ethernet/microchip/lan966x/lan966x_xdp.c | 3 ++-
2 files changed, 13 insertions(+), 6 deletions(-)
Comments
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?
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
> skb_put(skb, FDMA_DCB_STATUS_BLOCKL(db->status));
>
> lan966x_ifh_get_timestamp(skb->data, ×tamp);
> @@ -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
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, ×tamp);
> > @@ -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
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0+
+#include <linux/bpf.h>
+
#include "lan966x_main.h"
static int lan966x_fdma_channel_active(struct lan966x *lan966x)
@@ -16,7 +18,7 @@ static struct page *lan966x_fdma_rx_alloc_page(struct lan966x_rx *rx,
if (unlikely(!page))
return NULL;
- db->dataptr = page_pool_get_dma_addr(page);
+ db->dataptr = page_pool_get_dma_addr(page) + XDP_PACKET_HEADROOM;
return page;
}
@@ -72,7 +74,7 @@ static int lan966x_fdma_rx_alloc_page_pool(struct lan966x_rx *rx)
.nid = NUMA_NO_NODE,
.dev = lan966x->dev,
.dma_dir = DMA_FROM_DEVICE,
- .offset = 0,
+ .offset = XDP_PACKET_HEADROOM,
.max_len = rx->max_mtu -
SKB_DATA_ALIGN(sizeof(struct skb_shared_info)),
};
@@ -432,11 +434,13 @@ static int lan966x_fdma_rx_check_frame(struct lan966x_rx *rx, u64 *src_port)
if (unlikely(!page))
return FDMA_ERROR;
- dma_sync_single_for_cpu(lan966x->dev, (dma_addr_t)db->dataptr,
+ dma_sync_single_for_cpu(lan966x->dev,
+ (dma_addr_t)db->dataptr + XDP_PACKET_HEADROOM,
FDMA_DCB_STATUS_BLOCKL(db->status),
DMA_FROM_DEVICE);
- lan966x_ifh_get_src_port(page_address(page), src_port);
+ lan966x_ifh_get_src_port(page_address(page) + XDP_PACKET_HEADROOM,
+ src_port);
if (WARN_ON(*src_port >= lan966x->num_phys_ports))
return FDMA_ERROR;
@@ -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);
skb_put(skb, FDMA_DCB_STATUS_BLOCKL(db->status));
lan966x_ifh_get_timestamp(skb->data, ×tamp);
@@ -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;
}
int lan966x_fdma_change_mtu(struct lan966x *lan966x)
@@ -44,7 +44,8 @@ int lan966x_xdp_run(struct lan966x_port *port, struct page *page, u32 data_len)
xdp_init_buff(&xdp, PAGE_SIZE << lan966x->rx.page_order,
&port->xdp_rxq);
- xdp_prepare_buff(&xdp, page_address(page), IFH_LEN_BYTES,
+ xdp_prepare_buff(&xdp, page_address(page),
+ IFH_LEN_BYTES + XDP_PACKET_HEADROOM,
data_len - IFH_LEN_BYTES, false);
act = bpf_prog_run_xdp(xdp_prog, &xdp);
switch (act) {