diff mbox series

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

Message ID 20221115214456.1456856-2-horatiu.vultur@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: lan966x: Extend xdp support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 12 of 12 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 63 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Horatiu Vultur Nov. 15, 2022, 9:44 p.m. UTC 
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

Alexander Lobakin Nov. 16, 2022, 3:45 p.m. UTC | #1 
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, &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 Vultur Nov. 16, 2022, 6:50 p.m. UTC | #2 
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
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
index 5fbbd479cfb06..3055124b4dd79 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
@@ -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, &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;
 }
 
 int lan966x_fdma_change_mtu(struct lan966x *lan966x)
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c b/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
index e77d9f2aad2b4..8ebde1eb6a09c 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
@@ -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) {