mvneta: introduce XDP multi-buffer support

From: Sameeh Jubran <[email protected]>

Introduce the two following bpf helpers in order to provide some metadata about a xdp multi-buff fame to bpf layer:

- bpf_xdp_get_frags_count()
get the number of fragments for a given xdp multi-buffer.

* bpf_xdp_get_frags_total_size()
get the total size of fragments for a given xdp multi-buffer.

Signed-off-by Sameeh Jubran <[email protected]>

Co-developed-by Lorenzo Bianconi <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 include/uapi/linux/bpf.h       | 14 ++++++++++++
 net/core/filter.c              | 42 ++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h | 14 ++++++++++++
 3 files changed, 70 insertions(+)

include/uapi/linux/bpf.h +0 -0

index 4f556cfcbfbe..0715995eb18c 100644
@@ -3668,6 +3668,18 @@ union bpf_attr {
  * 	Return
  * 		The helper returns **TC_ACT_REDIRECT** on success or
  * 		**TC_ACT_SHOT** on error.

include/uapi/linux/bpf.h +2 -0

+ *	Return
+ *		The total size of fragments for a given xdp multi-buffer.
  */

include/uapi/linux/bpf.h +10 -0

+const struct bpf_func_proto bpf_xdp_get_frags_count_proto = {
+	.func		= bpf_xdp_get_frags_count,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
+BPF_CALL_1(bpf_xdp_get_frags_total_size, struct  xdp_buff*, xdp)
+{
+	struct skb_shared_info *sinfo;

include/uapi/linux/bpf.h +3 -0

+
+	return size;
+}

Quoted 5 lines · Lorenzo Bianconi

From: Sameeh Jubran <[email protected]>

Introduce the two following bpf helpers in order to provide some metadata about a xdp multi-buff fame to bpf layer:

Quoted 2 lines

- bpf_xdp_get_frags_count()
get the number of fragments for a given xdp multi-buffer.

Same comment as in the cover letter can you provide a use case for how/where I would use xdp_get_frags_count()? Is it just for debug? If its just debug do we really want a uapi helper for it.

Quoted 4 lines

* bpf_xdp_get_frags_total_size()
get the total size of fragments for a given xdp multi-buffer.

This is awkward IMO. If total size is needed it should return total size in all cases not just in the mb case otherwise programs will have two paths the mb path and the non-mb path. And if you have mixed workload the branch predictor will miss? Plus its extra instructions to load.

And if its useful for something beyond just debug and its going to be read every packet or something I think we should put it in the metadata so that its not hidden behind a helper which likely will show up as overhead on a 40+gbps nic. The use case I have in mind is counting bytes maybe sliced by IP or protocol. Here you will always read it and I don't want code with a if/else stuck in the middle when if we do it right we have a single read.

Quoted 19 lines

Signed-off-by Sameeh Jubran <[email protected]>

Co-developed-by Lorenzo Bianconi <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 include/uapi/linux/bpf.h       | 14 ++++++++++++
 net/core/filter.c              | 42 ++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h | 14 ++++++++++++
 3 files changed, 70 insertions(+)

include/uapi/linux/bpf.h +0 -0

index 4f556cfcbfbe..0715995eb18c 100644
@@ -3668,6 +3668,18 @@ union bpf_attr {
  * 	Return
  * 		The helper returns **TC_ACT_REDIRECT** on success or
  * 		**TC_ACT_SHOT** on error.

include/uapi/linux/bpf.h union bpf_attr { · line 3680

Quoted 10 lines

include/uapi/linux/bpf.h +10 -0

+ *
+ * int bpf_xdp_get_frags_count(struct xdp_buff *xdp_md)
+ *	Description
+ *		Get the number of fragments for a given xdp multi-buffer.
+ *	Return
+ *		The number of fragments
+ *
+ * int bpf_xdp_get_frags_total_size(struct xdp_buff *xdp_md)
+ *	Description
+ *		Get the total size of fragments for a given xdp multi-buffer.

Why just fragments? Will I have to also add the initial frag0 to it or not. I think the description is a bit ambiguous.

Quoted 3 lines

include/uapi/linux/bpf.h +2 -0

+ *	Return
+ *		The total size of fragments for a given xdp multi-buffer.
  */

[...]

Quoted 18 lines

include/uapi/linux/bpf.h +18 -0

+const struct bpf_func_proto bpf_xdp_get_frags_count_proto = {
+	.func		= bpf_xdp_get_frags_count,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
+BPF_CALL_1(bpf_xdp_get_frags_total_size, struct  xdp_buff*, xdp)
+{
+	struct skb_shared_info *sinfo;
+	int nfrags, i, size = 0;
+
+	if (likely(!xdp->mb))
+		return 0;
+
+	sinfo = xdp_get_shared_info_from_buff(xdp);
+	nfrags = min_t(u8, sinfo->nr_frags, MAX_SKB_FRAGS);
+

include/uapi/linux/bpf.h

Quoted 2 lines

include/uapi/linux/bpf.h +2 -0

+	for (i = 0; i < nfrags; i++)
+		size += skb_frag_size(&sinfo->frags[i]);

Wont the hardware just know this? I think walking the frag list just to get the total seems wrong. The hardware should have a total_len field somewhere we can just read no? If mvneta doesn't know the total length that seems like a driver limitation and we shouldn't encode it in the helper.

Quoted 4 lines

include/uapi/linux/bpf.h +3 -0

+
+	return size;
+}

Introduce the capability to allocate a xdp multi-buff in bpf_prog_test_run_xdp routine. This is a preliminary patch to introduce the selftests for new xdp multi-buff ebpf helpers

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 net/bpf/test_run.c | 51 
 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 43 insertions(+), 8 deletions(-)

net/bpf/test_run.c +2 -2

index bd291f5f539c..ec7286cd051b 100644
@@ -617,44 +617,79 @@ int bpf_prog_test_run_xdp(struct bpf_prog 
 *prog, const union bpf_attr *kattr,
 {
 	u32 tailroom = SKB_DATA_ALIGN(sizeof(struct 
 skb_shared_info));
 	u32 headroom = XDP_PACKET_HEADROOM;
-	u32 size = kattr->test.data_size_in;
 	u32 repeat = kattr->test.repeat;
 	struct netdev_rx_queue *rxqueue;
+	struct skb_shared_info *sinfo;
 	struct xdp_buff xdp = {};
+	u32 max_data_sz, size;
 	u32 retval, duration;
-	u32 max_data_sz;

net/bpf/test_run.c +34 -3

+	size = min_t(u32, kattr->test.data_size_in, max_data_sz);
+	data_len = size;
 
-	data = bpf_test_init(kattr, kattr->test.data_size_in,
-			     max_data_sz, headroom, tailroom);
+	data = bpf_test_init(kattr, size, max_data_sz, headroom, 
 tailroom);
 	if (IS_ERR(data))
 		return PTR_ERR(data);
 
 	xdp.data_hard_start = data;
 	xdp.data = data + headroom;
 	xdp.data_meta = xdp.data;
-	xdp.data_end = xdp.data + size;
+	xdp.data_end = xdp.data + data_len;
 	xdp.frame_sz = headroom + max_data_sz + tailroom;
 
+	sinfo = xdp_get_shared_info_from_buff(&xdp);
+	if (unlikely(kattr->test.data_size_in > size)) {
+		void __user *data_in = 
 u64_to_user_ptr(kattr->test.data_in);
+
+		while (size < kattr->test.data_size_in) {
+			skb_frag_t *frag = 
 &sinfo->frags[sinfo->nr_frags];
+			struct page *page;
+			int data_len;
+
+			page = alloc_page(GFP_KERNEL);
+			if (!page) {
+				ret = -ENOMEM;
+				goto out;
+			}
+
+			__skb_frag_set_page(frag, page);
+			data_len = min_t(int, 
 kattr->test.data_size_in - size,
+					 PAGE_SIZE);
+			skb_frag_size_set(frag, data_len);
+			if (copy_from_user(page_address(page), 
 data_in + size,
+					   data_len)) {
+				ret = -EFAULT;
+				goto out;
+			}
+			sinfo->nr_frags++;
+			size += data_len;
+		}
+		xdp.mb = 1;
+	}
+
 	rxqueue = 
 __netif_get_rx_queue(current->nsproxy->net_ns->loopback_dev, 
 0);
 	xdp.rxq = &rxqueue->xdp_rxq;

net/bpf/test_run.c +4 -0

+
 	ret = bpf_test_finish(kattr, uattr, xdp.data, size, 
 retval, duration);
 out:
 	bpf_prog_change_xdp(prog, NULL);
+	for (i = 0; i < sinfo->nr_frags; i++)
+		__free_page(skb_frag_page(&sinfo->frags[i]));
 	kfree(data);
+
 	return ret;
 }

Quoted 35 lines · Lorenzo Bianconi

Introduce the capability to allocate a xdp multi-buff in bpf_prog_test_run_xdp routine. This is a preliminary patch to introduce
the selftests for new xdp multi-buff ebpf helpers

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 net/bpf/test_run.c | 51  ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 43 insertions(+), 8 deletions(-)

net/bpf/test_run.c +3 -3

index bd291f5f539c..ec7286cd051b 100644
@@ -617,44 +617,79 @@ int bpf_prog_test_run_xdp(struct bpf_prog 
 *prog, const union bpf_attr *kattr,
 {
 	u32 tailroom = SKB_DATA_ALIGN(sizeof(struct 
 skb_shared_info));
 	u32 headroom = XDP_PACKET_HEADROOM;
-	u32 size = kattr->test.data_size_in;
 	u32 repeat = kattr->test.repeat;
 	struct netdev_rx_queue *rxqueue;
+	struct skb_shared_info *sinfo;
 	struct xdp_buff xdp = {};
+	u32 max_data_sz, size;
 	u32 retval, duration;
-	u32 max_data_sz;
+	int i, ret, data_len;
 	void *data;
-	int ret;
 
 	if (kattr->test.ctx_in || kattr->test.ctx_out)
 		return -EINVAL;
 

net/bpf/test_run.c int bpf_prog_test_run_xdp(struct bpf_prog · line 635

Quoted 3 lines

net/bpf/test_run.c +0 -1

-	/* XDP have extra tailroom as (most) drivers use full page 
 */
 	max_data_sz = 4096 - headroom - tailroom;  

For the sake of consistency, can this 4096 be changed to PAGE_SIZE ?

Same as in

     data_len = min_t(int, kattr->test.data_size_in - size, 
     PAGE_SIZE);

expression below

This series introduce XDP multi-buffer support. The mvneta driver is the first to support these new "non-linear" xdp_{buff,frame}. Reviewers please focus on how these new types of xdp_{buff,frame} packets traverse the different layers and the layout design. It is on purpose that BPF-helpers are kept simple, as we don't want to expose the internal layout to allow later changes.

For now, to keep the design simple and to maintain performance, the XDP BPF-prog (still) only have access to the first-buffer. It is left for later (another patchset) to add payload access across multiple buffers. This patchset should still allow for these future extensions. The goal is to lift the XDP MTU restriction that comes with XDP, but maintain same performance as before.

The main idea for the new multi-buffer layout is to reuse the same layout used for non-linear SKB. This rely on the "skb_shared_info" struct at the end of the first buffer to link together subsequent buffers. Keeping the layout compatible with SKBs is also done to ease and speedup creating an SKB from an xdp_{buff,frame}. Converting xdp_frame to SKB and deliver it to the network stack is shown in cpumap code (patch 13/13).

A multi-buffer bit (mb) has been introduced in xdp_{buff,frame} structure to notify the bpf/network layer if this is a xdp multi-buffer frame (mb = 1) or not (mb = 0).
The mb bit will be set by a xdp multi-buffer capable driver only for non-linear frames maintaining the capability to receive linear frames without any extra cost since the skb_shared_info structure at the end of the first buffer will be initialized only if mb is set.

In order to provide to userspace some metdata about the non-linear xdp_{buff,frame}, we introduced 2 bpf helpers:
- bpf_xdp_get_frags_count:
get the number of fragments for a given xdp multi-buffer.
- bpf_xdp_get_frags_total_size:
get the total size of fragments for a given xdp multi-buffer.

Typical use cases for this series are:
- Jumbo-frames
- Packet header split (please see Google���s use-case @ NetDevConf 0x14, [0])
- TSO

More info about the main idea behind this approach can be found here [1][2].

We carried out some throughput tests in a standard linear frame scenario in order to verify we did not introduced any performance regression adding xdp multi-buff support to mvneta:

offered load is ~ 1000Kpps, packet size is 64B, mvneta descriptor size is one PAGE

commit: 879456bedbe5 ("net: mvneta: avoid possible cache misses in mvneta_rx_swbm")

- xdp-pass:      ~162Kpps
- xdp-drop:      ~701Kpps
- xdp-tx:        ~185Kpps
- xdp-redirect:  ~202Kpps

mvneta xdp multi-buff:

- xdp-pass:      ~163Kpps
- xdp-drop:      ~739Kpps
- xdp-tx:        ~182Kpps
- xdp-redirect:  ~202Kpps

Changes since v3:
- rebase ontop of bpf-next
- add patch 10/13 to copy back paged data from a xdp multi-buff frame to userspace buffer for xdp multi-buff selftests

Changes since v2:
- add throughput measurements
- drop bpf_xdp_adjust_mb_header bpf helper
- introduce selftest for xdp multibuffer
- addressed comments on bpf_xdp_get_frags_count
- introduce xdp multi-buff support to cpumaps

Changes since v1:
- Fix use-after-free in xdp_return_{buff/frame}
- Introduce bpf helpers
- Introduce xdp_mb sample program
- access skb_shared_info->nr_frags only on the last fragment

Changes since RFC:
- squash multi-buffer bit initialization in a single patch
- add mvneta non-linear XDP buff support for tx side

[0] https://netdevconf.info/0x14/session.html?talk-the-path-to-tcp-4k-mtu-and-rx-zerocopy
[1] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
[2] https://netdevconf.info/0x14/session.html?tutorial-add-XDP-support-to-a-NIC-driver (XDPmulti-buffers section)

Lorenzo Bianconi (11):
xdp: introduce mb in xdp_buff/xdp_frame
xdp: initialize xdp_buff mb bit to 0 in all XDP drivers
net: mvneta: update mb bit before passing the xdp buffer to eBPF layer
xdp: add multi-buff support to xdp_return_{buff/frame}
net: mvneta: add multi buffer support to XDP_TX
bpf: move user_size out of bpf_test_init
bpf: introduce multibuff support to bpf_prog_test_run_xdp()
bpf: test_run: add skb_shared_info pointer in bpf_test_finish
signature
bpf: add xdp multi-buffer selftest
net: mvneta: enable jumbo frames for XDP
bpf: cpumap: introduce xdp multi-buff support

Sameeh Jubran (2):
bpf: introduce bpf_xdp_get_frags_{count, total_size} helpers samples/bpf: add bpf program that uses xdp mb helpers

 drivers/net/ethernet/amazon/ena/ena_netdev.c  |   1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c |   1 +
 .../net/ethernet/cavium/thunder/nicvf_main.c  |   1 +
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |   1 +
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   1 +
 drivers/net/ethernet/intel/ice/ice_txrx.c     |   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   1 +
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c |   1 +
 drivers/net/ethernet/marvell/mvneta.c         | 131 +++++++------
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   |   1 +
 drivers/net/ethernet/mellanox/mlx4/en_rx.c    |   1 +
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |   1 +
 .../ethernet/netronome/nfp/nfp_net_common.c   |   1 +
 drivers/net/ethernet/qlogic/qede/qede_fp.c    |   1 +
 drivers/net/ethernet/sfc/rx.c                 |   1 +
 drivers/net/ethernet/socionext/netsec.c       |   1 +
 drivers/net/ethernet/ti/cpsw.c                |   1 +
 drivers/net/ethernet/ti/cpsw_new.c            |   1 +
 drivers/net/hyperv/netvsc_bpf.c               |   1 +
 drivers/net/tun.c                             |   2 +
 drivers/net/veth.c                            |   1 +
 drivers/net/virtio_net.c                      |   2 +
 drivers/net/xen-netfront.c                    |   1 +
 include/net/xdp.h                             |  31 ++-
 include/uapi/linux/bpf.h                      |  14 ++
 kernel/bpf/cpumap.c                           |  45 +----
 net/bpf/test_run.c                            | 118 ++++++++++--
 net/core/dev.c                                |   1 +
 net/core/filter.c                             |  42 ++++
 net/core/xdp.c                                | 104 ++++++++++
 samples/bpf/Makefile                          |   3 +
 samples/bpf/xdp_mb_kern.c                     |  68 +++++++
 samples/bpf/xdp_mb_user.c                     | 182 ++++++++++++++++++
 tools/include/uapi/linux/bpf.h                |  14 ++
 .../testing/selftests/bpf/prog_tests/xdp_mb.c |  79 ++++++++
 .../selftests/bpf/progs/test_xdp_multi_buff.c |  24 +++
 36 files changed, 757 insertions(+), 123 deletions(-)
 create mode 100644 samples/bpf/xdp_mb_kern.c
 create mode 100644 samples/bpf/xdp_mb_user.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_mb.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_multi_buff.c

Signature · 1 line

2.26.2

Quoted 14 lines · Lorenzo Bianconi

This series introduce XDP multi-buffer support. The mvneta driver is the first to support these new "non-linear" xdp_{buff,frame}. Reviewers please focus on how these new types of xdp_{buff,frame} packets traverse the different layers and the layout design. It is on purpose that BPF-helpers are kept simple, as we don't want to expose the internal layout to allow later changes.

For now, to keep the design simple and to maintain performance, the XDP BPF-prog (still) only have access to the first-buffer. It is left for later (another patchset) to add payload access across multiple buffers. This patchset should still allow for these future extensions. The goal is to lift the XDP MTU restriction that comes with XDP, but maintain same performance as before.

Quoted 7 lines

The main idea for the new multi-buffer layout is to reuse the same layout used for non-linear SKB. This rely on the "skb_shared_info" struct at the end of the first buffer to link together subsequent buffers. Keeping the layout compatible with SKBs is also done to ease and speedup creating an SKB from an xdp_{buff,frame}. Converting xdp_frame to SKB and deliver it to the network stack is shown in cpumap code (patch 13/13).

Using the end of the buffer for the skb_shared_info struct is going to become driver API so unwinding it if it proves to be a performance issue is going to be ugly. So same question as before, for the use case where we receive packet and do XDP_TX with it how do we avoid cache miss overhead? This is not just a hypothetical use case, the Facebook load balancer is doing this as well as Cilium and allowing this with multi-buffer packets >1500B would be useful.

Can we write the skb_shared_info lazily? It should only be needed once we know the packet is going up the stack to some place that needs the info. Which we could learn from the return code of the XDP program.

Quoted 9 lines

A multi-buffer bit (mb) has been introduced in xdp_{buff,frame} structure to notify the bpf/network layer if this is a xdp multi-buffer frame (mb = 1) or not (mb = 0).
The mb bit will be set by a xdp multi-buffer capable driver only for non-linear frames maintaining the capability to receive linear frames without any extra cost since the skb_shared_info structure at the end of the first buffer will be initialized only if mb is set.

Thanks above is clearer.

Quoted 7 lines

In order to provide to userspace some metdata about the non-linear xdp_{buff,frame}, we introduced 2 bpf helpers:
- bpf_xdp_get_frags_count:
get the number of fragments for a given xdp multi-buffer.
- bpf_xdp_get_frags_total_size:
get the total size of fragments for a given xdp multi-buffer.

Whats the use case for these? Do you have an example where knowing the frags count is going to be something a BPF program will use? Having total size seems interesting but perhaps we should push that into the metadata so its pulled into the cache if users are going to be reading it on every packet or something.

Quoted 116 lines

Typical use cases for this series are:
- Jumbo-frames
- Packet header split (please see Google���s use-case @ NetDevConf 0x14, [0])
- TSO

More info about the main idea behind this approach can be found here [1][2].

We carried out some throughput tests in a standard linear frame scenario in order to verify we did not introduced any performance regression adding xdp multi-buff support to mvneta:

offered load is ~ 1000Kpps, packet size is 64B, mvneta descriptor size is one PAGE

commit: 879456bedbe5 ("net: mvneta: avoid possible cache misses in mvneta_rx_swbm")

- xdp-pass:      ~162Kpps
- xdp-drop:      ~701Kpps
- xdp-tx:        ~185Kpps
- xdp-redirect:  ~202Kpps

mvneta xdp multi-buff:

- xdp-pass:      ~163Kpps
- xdp-drop:      ~739Kpps
- xdp-tx:        ~182Kpps
- xdp-redirect:  ~202Kpps

Changes since v3:
- rebase ontop of bpf-next
- add patch 10/13 to copy back paged data from a xdp multi-buff frame to userspace buffer for xdp multi-buff selftests

Changes since v2:
- add throughput measurements
- drop bpf_xdp_adjust_mb_header bpf helper
- introduce selftest for xdp multibuffer
- addressed comments on bpf_xdp_get_frags_count
- introduce xdp multi-buff support to cpumaps

Changes since v1:
- Fix use-after-free in xdp_return_{buff/frame}
- Introduce bpf helpers
- Introduce xdp_mb sample program
- access skb_shared_info->nr_frags only on the last fragment

Changes since RFC:
- squash multi-buffer bit initialization in a single patch
- add mvneta non-linear XDP buff support for tx side

[0] https://netdevconf.info/0x14/session.html?talk-the-path-to-tcp-4k-mtu-and-rx-zerocopy
[1] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
[2] https://netdevconf.info/0x14/session.html?tutorial-add-XDP-support-to-a-NIC-driver (XDPmulti-buffers section)

Lorenzo Bianconi (11):
xdp: introduce mb in xdp_buff/xdp_frame
xdp: initialize xdp_buff mb bit to 0 in all XDP drivers
net: mvneta: update mb bit before passing the xdp buffer to eBPF layer
xdp: add multi-buff support to xdp_return_{buff/frame}
net: mvneta: add multi buffer support to XDP_TX
bpf: move user_size out of bpf_test_init
bpf: introduce multibuff support to bpf_prog_test_run_xdp()
bpf: test_run: add skb_shared_info pointer in bpf_test_finish
signature
bpf: add xdp multi-buffer selftest
net: mvneta: enable jumbo frames for XDP
bpf: cpumap: introduce xdp multi-buff support

Sameeh Jubran (2):
bpf: introduce bpf_xdp_get_frags_{count, total_size} helpers samples/bpf: add bpf program that uses xdp mb helpers

 drivers/net/ethernet/amazon/ena/ena_netdev.c  |   1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c |   1 +
 .../net/ethernet/cavium/thunder/nicvf_main.c  |   1 +
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |   1 +
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |   1 +
 drivers/net/ethernet/intel/ice/ice_txrx.c     |   1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   1 +
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c |   1 +
 drivers/net/ethernet/marvell/mvneta.c         | 131 +++++++------
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   |   1 +
 drivers/net/ethernet/mellanox/mlx4/en_rx.c    |   1 +
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |   1 +
 .../ethernet/netronome/nfp/nfp_net_common.c   |   1 +
 drivers/net/ethernet/qlogic/qede/qede_fp.c    |   1 +
 drivers/net/ethernet/sfc/rx.c                 |   1 +
 drivers/net/ethernet/socionext/netsec.c       |   1 +
 drivers/net/ethernet/ti/cpsw.c                |   1 +
 drivers/net/ethernet/ti/cpsw_new.c            |   1 +
 drivers/net/hyperv/netvsc_bpf.c               |   1 +
 drivers/net/tun.c                             |   2 +
 drivers/net/veth.c                            |   1 +
 drivers/net/virtio_net.c                      |   2 +
 drivers/net/xen-netfront.c                    |   1 +
 include/net/xdp.h                             |  31 ++-
 include/uapi/linux/bpf.h                      |  14 ++
 kernel/bpf/cpumap.c                           |  45 +----
 net/bpf/test_run.c                            | 118 ++++++++++--
 net/core/dev.c                                |   1 +
 net/core/filter.c                             |  42 ++++
 net/core/xdp.c                                | 104 ++++++++++
 samples/bpf/Makefile                          |   3 +
 samples/bpf/xdp_mb_kern.c                     |  68 +++++++
 samples/bpf/xdp_mb_user.c                     | 182 ++++++++++++++++++
 tools/include/uapi/linux/bpf.h                |  14 ++
 .../testing/selftests/bpf/prog_tests/xdp_mb.c |  79 ++++++++
 .../selftests/bpf/progs/test_xdp_multi_buff.c |  24 +++
 36 files changed, 757 insertions(+), 123 deletions(-)
 create mode 100644 samples/bpf/xdp_mb_kern.c
 create mode 100644 samples/bpf/xdp_mb_user.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_mb.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_multi_buff.c

Signature · 1 line

2.26.2

Quoted 31 lines

Quoted 14 lines · Lorenzo Bianconi

This series introduce XDP multi-buffer support. The mvneta driver is the first to support these new "non-linear" xdp_{buff,frame}. Reviewers please focus on how these new types of xdp_{buff,frame} packets traverse the different layers and the layout design. It is on purpose that BPF-helpers are kept simple, as we don't want to expose the internal layout to allow later changes.

For now, to keep the design simple and to maintain performance, the XDP BPF-prog (still) only have access to the first-buffer. It is left for later (another patchset) to add payload access across multiple buffers. This patchset should still allow for these future extensions. The goal is to lift the XDP MTU restriction that comes with XDP, but maintain same performance as before.

Quoted 7 lines

The main idea for the new multi-buffer layout is to reuse the same layout used for non-linear SKB. This rely on the "skb_shared_info" struct at the end of the first buffer to link together subsequent buffers. Keeping the layout compatible with SKBs is also done to ease and speedup creating an SKB from an xdp_{buff,frame}. Converting xdp_frame to SKB and deliver it to the network stack is shown in cpumap code (patch 13/13).

Using the end of the buffer for the skb_shared_info struct is going to become driver API so unwinding it if it proves to be a performance issue is going to be ugly. So same question as before, for the use case where we receive packet and do XDP_TX with it how do we avoid cache miss overhead? This is not just a hypothetical use case, the Facebook load balancer is doing this as well as Cilium and allowing this with multi-buffer packets >1500B would be useful.

Quoted 3 lines

Can we write the skb_shared_info lazily? It should only be needed once we know the packet is going up the stack to some place that needs the info. Which we could learn from the return code of the XDP program.

Hi John,

I agree, I think for XDP_TX use-case it is not strictly necessary to fill the skb_hared_info. The driver can just keep this info on the stack and use it inserting the packet back to the DMA ring.
For mvneta I implemented it in this way to keep the code aligned with ndo_xdp_xmit path since it is a low-end device. I guess we are not introducing any API constraint for XDP_TX. A high-end device can implement multi-buff for XDP_TX in a different way in order to avoid the cache miss.

We need to fill the skb_shared info only when we want to pass the frame to the network stack (build_skb() can directly reuse skb_shared_info->frags[]) or for XDP_REDIRECT use-case.

Quoted 21 lines

Quoted 10 lines

A multi-buffer bit (mb) has been introduced in xdp_{buff,frame} structure to notify the bpf/network layer if this is a xdp multi-buffer frame (mb = 1) or not (mb = 0).
The mb bit will be set by a xdp multi-buffer capable driver only for non-linear frames maintaining the capability to receive linear frames without any extra cost since the skb_shared_info structure at the end of the first buffer will be initialized only if mb is set.

Thanks above is clearer.

Quoted 7 lines

In order to provide to userspace some metdata about the non-linear xdp_{buff,frame}, we introduced 2 bpf helpers:
- bpf_xdp_get_frags_count:
get the number of fragments for a given xdp multi-buffer.
- bpf_xdp_get_frags_total_size:
get the total size of fragments for a given xdp multi-buffer.

Quoted 5 lines

Whats the use case for these? Do you have an example where knowing the frags count is going to be something a BPF program will use? Having total size seems interesting but perhaps we should push that into the metadata so its pulled into the cache if users are going to be reading it on every packet or something.

At the moment we do not have any use-case for these helpers (not considering the sample in the series :)). We introduced them to provide some basic metadata about the non-linear xdp_frame.
IIRC we decided to introduce some helpers instead of adding this info in xdp_frame in order to save space on it (for xdp it is essential xdp_frame to fit in a single cache-line).

Regards,
Lorenzo

Quoted 31 lines · Lorenzo Bianconi

Quoted 31 lines

Quoted 14 lines · Lorenzo Bianconi

This series introduce XDP multi-buffer support. The mvneta driver is the first to support these new "non-linear" xdp_{buff,frame}. Reviewers please focus on how these new types of xdp_{buff,frame} packets traverse the different layers and the layout design. It is on purpose that BPF-helpers are kept simple, as we don't want to expose the internal layout to allow later changes.

For now, to keep the design simple and to maintain performance, the XDP BPF-prog (still) only have access to the first-buffer. It is left for later (another patchset) to add payload access across multiple buffers. This patchset should still allow for these future extensions. The goal is to lift the XDP MTU restriction that comes with XDP, but maintain same performance as before.

Quoted 7 lines

The main idea for the new multi-buffer layout is to reuse the same layout used for non-linear SKB. This rely on the "skb_shared_info" struct at the end of the first buffer to link together subsequent buffers. Keeping the layout compatible with SKBs is also done to ease and speedup creating an SKB from an xdp_{buff,frame}. Converting xdp_frame to SKB and deliver it to the network stack is shown in cpumap code (patch 13/13).

Using the end of the buffer for the skb_shared_info struct is going to become driver API so unwinding it if it proves to be a performance issue is going to be ugly. So same question as before, for the use case where we receive packet and do XDP_TX with it how do we avoid cache miss overhead? This is not just a hypothetical use case, the Facebook load balancer is doing this as well as Cilium and allowing this with multi-buffer packets >1500B would be useful.

Quoted 5 lines

Quoted 3 lines

Can we write the skb_shared_info lazily? It should only be needed once we know the packet is going up the stack to some place that needs the info. Which we could learn from the return code of the XDP program.

Hi John,

Hi, I'll try to join the two threads this one and the one on helpers here so we don't get too fragmented.

Quoted 8 lines

I agree, I think for XDP_TX use-case it is not strictly necessary to fill the skb_hared_info. The driver can just keep this info on the stack and use it inserting the packet back to the DMA ring.
For mvneta I implemented it in this way to keep the code aligned with ndo_xdp_xmit path since it is a low-end device. I guess we are not introducing any API constraint for XDP_TX. A high-end device can implement multi-buff for XDP_TX in a different way in order to avoid the cache miss.

Agree it would be an implementation detail for XDP_TX except the two helpers added in this series currently require it to be there.

Quoted 4 lines

We need to fill the skb_shared info only when we want to pass the frame to the network stack (build_skb() can directly reuse skb_shared_info->frags[]) or for XDP_REDIRECT use-case.

It might be good to think about the XDP_REDIRECT case as well then. If the frags list fit in the metadata/xdp_frame would we expect better performance?

Looking at skb_shared_info{} that is a rather large structure with many

fields that look unnecessary for XDP_REDIRECT case and only needed when passing to the stack.

Fundamentally, a frag just needs

struct bio_vec {

     struct page *bv_page;     // 8B
     unsigned int bv_len;      // 4B
     unsigned int bv_offset;   // 4B

} // 16B

With header split + data we only need a single frag so we could use just 16B. And worse case jumbo frame + header split seems 3 entries would be enough giving 48B (header plus 3 4k pages).

Could we just stick this in the metadata and make it read only? Then programs that care can read it and get all the info they need without helpers.

I would expect performance to be better in the XDP_TX and XDP_REDIRECT cases. And copying an extra worse case 48B in passing to the stack I guess is not measurable given all the work needed in that path.

Quoted 50 lines · On Fri, 02 Oct 2020 11:06:12 -0700 John Fastabend <[email protected]>…

Quoted 31 lines · Lorenzo Bianconi

Quoted 31 lines

Quoted 14 lines · Lorenzo Bianconi

This series introduce XDP multi-buffer support. The mvneta driver is the first to support these new "non-linear" xdp_{buff,frame}. Reviewers please focus on how these new types of xdp_{buff,frame} packets traverse the different layers and the layout design. It is on purpose that BPF-helpers are kept simple, as we don't want to expose the internal layout to allow later changes.

For now, to keep the design simple and to maintain performance, the XDP BPF-prog (still) only have access to the first-buffer. It is left for later (another patchset) to add payload access across multiple buffers. This patchset should still allow for these future extensions. The goal is to lift the XDP MTU restriction that comes with XDP, but maintain same performance as before.

Quoted 7 lines

The main idea for the new multi-buffer layout is to reuse the same layout used for non-linear SKB. This rely on the "skb_shared_info" struct at the end of the first buffer to link together subsequent buffers. Keeping the layout compatible with SKBs is also done to ease and speedup creating an SKB from an xdp_{buff,frame}. Converting xdp_frame to SKB and deliver it to the network stack is shown in cpumap code (patch 13/13).

Using the end of the buffer for the skb_shared_info struct is going to become driver API so unwinding it if it proves to be a performance issue is going to be ugly. So same question as before, for the use case where we receive packet and do XDP_TX with it how do we avoid cache miss overhead? This is not just a hypothetical use case, the Facebook load balancer is doing this as well as Cilium and allowing this with multi-buffer packets >1500B would be useful.

Quoted 5 lines

Quoted 3 lines

Can we write the skb_shared_info lazily? It should only be needed once we know the packet is going up the stack to some place that needs the info. Which we could learn from the return code of the XDP program.

Hi John,

Hi, I'll try to join the two threads this one and the one on helpers here so we don't get too fragmented.

Quoted 8 lines

I agree, I think for XDP_TX use-case it is not strictly necessary to fill the skb_hared_info. The driver can just keep this info on the stack and use it inserting the packet back to the DMA ring.
For mvneta I implemented it in this way to keep the code aligned with ndo_xdp_xmit path since it is a low-end device. I guess we are not introducing any API constraint for XDP_TX. A high-end device can implement multi-buff for XDP_TX in a different way in order to avoid the cache miss.

Quoted 2 lines

Agree it would be an implementation detail for XDP_TX except the two helpers added in this series currently require it to be there.

That is a good point. If you look at the details, the helpers use xdp_buff->mb bit to guard against accessing the "shared_info" cacheline. Thus, for the normal single frame case XDP_TX should not see a slowdown. Do we really need to optimize XDP_TX multi-frame case(?)

Quoted 10 lines

Quoted 6 lines

We need to fill the skb_shared info only when we want to pass the frame to the network stack (build_skb() can directly reuse skb_shared_info->frags[]) or for XDP_REDIRECT use-case.

It might be good to think about the XDP_REDIRECT case as well then. If the frags list fit in the metadata/xdp_frame would we expect better performance?

I don't like to use space in xdp_frame for this. (1) We (Ahern and I) are planning to use the space in xdp_frame for RX-csum + RX-hash +vlan, which will be more common (e.g. all packets will have HW RX+csum). (2) I consider XDP multi-buffer the exception case, that will not be used in most cases, so why reserve space for that in this cache-line.

IMHO we CANNOT allow any slowdown for existing XDP use-cases, but IMHO XDP multi-buffer use-cases are allowed to run "slower".

Quoted 3 lines

Looking at skb_shared_info{} that is a rather large structure with many

A cache-line detail about skb_shared_info: The first frags[0] member is in the first cache-line. Meaning that it is still fast to have xdp frames with 1 extra buffer.

Quoted 3 lines

fields that look unnecessary for XDP_REDIRECT case and only needed when passing to the stack.

Yes, I think we can use first cache-line of skb_shared_info more optimally (via defining a xdp_shared_info struct). But I still want us to use this specific cache-line. Let me explain why below. (Avoiding cache-line misses is all about the details, so I hope you can follow).

Hopefully most driver developers understand/knows this. In the RX-loop the current RX-descriptor have a status that indicate there are more frame, usually expressed as non-EOP (End-Of-Packet). Thus, a driver can start a prefetchw of this shared_info cache-line, prior to processing the RX-desc that describe the multi-buffer.
(Remember this shared_info is constructed prior to calling XDP and any
XDP_TX action, thus the XDP prog should not see a cache-line miss when using the BPF-helper to read shared_info area).

Quoted 10 lines

Fundamentally, a frag just needs

struct bio_vec {

     struct page *bv_page;     // 8B
     unsigned int bv_len;      // 4B
     unsigned int bv_offset;   // 4B

} // 16B

Quoted 3 lines

With header split + data we only need a single frag so we could use just 16B. And worse case jumbo frame + header split seems 3 entries would be enough giving 48B (header plus 3 4k pages).

For jumbo-frame 9000 MTU 2 entries might be enough, as we also have room in the first buffer (((9000-(4096-256-320))/4096 = 1.33789).

The problem is that we need to support TSO (TCP Segmentation Offload) use-case, which can have more frames. Thus, 3 entries will not be enough.

Quoted 4 lines

Could we just stick this in the metadata and make it read only? Then programs that care can read it and get all the info they need without helpers.

I don't see how that is possible. (1) the metadata area is only 32 bytes, (2) when freeing an xdp_frame the kernel need to know the layout as these points will be free'ed.

Quoted 5 lines

I would expect performance to be better in the XDP_TX and XDP_REDIRECT cases. And copying an extra worse case 48B in passing to the stack I guess is not measurable given all the work needed in that path.

I do agree, that when passing to netstack we can do a transformation from xdp_shared_info to skb_shared_info with a fairly small cost. (The TSO case would require more copying).

Notice that allocating an SKB, will always clear the first 32 bytes of skb_shared_info. If the XDP driver-code path have done the prefetch as described above, then we should see a speedup for netstack delivery.

In general I see no reason to populate these fields before the XDP program runs. Someone needs to convince me why having frags info before program runs is useful. In general headers should be preserved and first frag already included in the data pointers. If users start parsing further they might need it, but this series doesn't provide a way to do that so IMO without those helpers its a bit difficult to debate.

Specifically for XDP_TX case we can just flip the descriptors from RX ring to TX ring and keep moving along. This is going to be ideal on 40/100Gbps nics.

I'm not arguing that its likely possible to put some prefetch logic in there and keep the pipe full, but I would need to see that on a 100gbps nic to be convinced the details here are going to work. Or at minimum a 40gbps nic.

Quoted 2 lines

Not against it, but these things are a bit tricky. Couple things I still want to see/understand

- Lets see a 40gbps use a prefetch and verify it works in practice
- Explain why we can't just do this after XDP program runs

- How will we read data in the frag list if we need to parse headers inside the frags[].

The above would be best to answer now rather than later IMO.

Thanks,
John

[...]

Quoted 7 lines

In general I see no reason to populate these fields before the XDP program runs. Someone needs to convince me why having frags info before program runs is useful. In general headers should be preserved and first frag already included in the data pointers. If users start parsing further they might need it, but this series doesn't provide a way to do that so IMO without those helpers its a bit difficult to debate.

We need to populate the skb_shared_info before running the xdp program in order to allow the ebpf sanbox to access this data. If we restrict the access to the first buffer only I guess we can avoid to do that but I think there is a value allowing the xdp program to access this data.

A possible optimization can be access the shared_info only once before running the ebpf program constructing the shared_info using a struct allocated on the stack.

Moreover we can define a "xdp_shared_info" struct to alias the skb_shared_info one in order to have most on frags elements in the first "shared_info" cache line.

Quoted 12 lines

Specifically for XDP_TX case we can just flip the descriptors from RX ring to TX ring and keep moving along. This is going to be ideal on 40/100Gbps nics.

I'm not arguing that its likely possible to put some prefetch logic in there and keep the pipe full, but I would need to see that on a 100gbps nic to be convinced the details here are going to work. Or at minimum a 40gbps nic.

Quoted 2 lines

[...]

Quoted 5 lines

Not against it, but these things are a bit tricky. Couple things I still want to see/understand

- Lets see a 40gbps use a prefetch and verify it works in practice
- Explain why we can't just do this after XDP program runs

how can we allow the ebpf program to access paged data if we do not do that?

Quoted 8 lines

- How will we read data in the frag list if we need to parse headers inside the frags[].

The above would be best to answer now rather than later IMO.

Thanks,
John

Regards,
Lorenzo

Quoted 10 lines · Lorenzo Bianconi

[...]

Quoted 7 lines

In general I see no reason to populate these fields before the XDP program runs. Someone needs to convince me why having frags info before program runs is useful. In general headers should be preserved and first frag already included in the data pointers. If users start parsing further they might need it, but this series doesn't provide a way to do that so IMO without those helpers its a bit difficult to debate.

Quoted 4 lines

We need to populate the skb_shared_info before running the xdp program in order to allow the ebpf sanbox to access this data. If we restrict the access to the first buffer only I guess we can avoid to do that but I think there is a value allowing the xdp program to access this data.

I agree. We could also only populate the fields if the program accesses the fields.

Quoted 3 lines

A possible optimization can be access the shared_info only once before running the ebpf program constructing the shared_info using a struct allocated on the stack.

Seems interesting, might be a good idea.

Quoted 7 lines

Moreover we can define a "xdp_shared_info" struct to alias the skb_shared_info one in order to have most on frags elements in the first "shared_info" cache line.

Quoted 4 lines

Specifically for XDP_TX case we can just flip the descriptors from RX ring to TX ring and keep moving along. This is going to be ideal on 40/100Gbps nics.

Quoted 4 lines

Quoted 4 lines

I'm not arguing that its likely possible to put some prefetch logic in there and keep the pipe full, but I would need to see that on a 100gbps nic to be convinced the details here are going to work. Or at minimum a 40gbps nic.

Quoted 5 lines

[...]

Quoted 3 lines

Not against it, but these things are a bit tricky. Couple things I still want to see/understand

Quoted 4 lines

Quoted 2 lines

- Lets see a 40gbps use a prefetch and verify it works in practice
- Explain why we can't just do this after XDP program runs

how can we allow the ebpf program to access paged data if we do not do that?

I don't see an easy way, but also this series doesn't have the data access support.

Its hard to tell until we get at least a 40gbps nic if my concern about performance is real or not. Prefetching smartly could resolve some of the issue I guess.

If the Intel folks are working on it I think waiting would be great. Otherwise at minimum drop the helpers and be prepared to revert things if needed.

Quoted 11 lines · On Mon, 05 Oct 2020 21:29:36 -0700 John Fastabend <[email protected]>…

Quoted 10 lines · Lorenzo Bianconi

[...]

Quoted 7 lines

In general I see no reason to populate these fields before the XDP program runs. Someone needs to convince me why having frags info before program runs is useful. In general headers should be preserved and first frag already included in the data pointers. If users start parsing further they might need it, but this series doesn't provide a way to do that so IMO without those helpers its a bit difficult to debate.

Quoted 7 lines

Quoted 4 lines

We need to populate the skb_shared_info before running the xdp program in order to allow the ebpf sanbox to access this data. If we restrict the access to the first buffer only I guess we can avoid to do that but I think there is a value allowing the xdp program to access this data.

I agree. We could also only populate the fields if the program accesses the fields.

Notice, a driver will not initialize/use the shared_info area unless there are more segments. And (we have already established) the xdp->mb bit is guarding BPF-prog from accessing shared_info area.

Quoted 4 lines

Quoted 3 lines

A possible optimization can be access the shared_info only once before running the ebpf program constructing the shared_info using a struct allocated on the stack.

Quoted 1 line

Seems interesting, might be a good idea.

It *might* be a good idea ("alloc" shared_info on stack), but we should benchmark this. The prefetch trick might be fast enough. But also keep in mind the performance target, as with large size frames the packet-per-sec we need to handle dramatically drop.

I do think it makes sense to drop the helpers for now, and focus on how this new multi-buffer frame type is handled in the existing code, and do some benchmarking on higher speed NIC, before the BPF-helper start to lockdown/restrict what we can change/revert as they define UAPI.

E.g. existing code that need to handle this is existing helper bpf_xdp_adjust_tail, which is something I have broad up before and even described in[1]. Lets make sure existing code works with proposed design, before introducing new helpers (and this makes it easier to revert).

[1] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org#xdp-tail-adjust

Signature · 4 lines

Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Quoted 32 lines

Quoted 11 lines · On Mon, 05 Oct 2020 21:29:36 -0700 John Fastabend <[email protected]>…

Quoted 10 lines · Lorenzo Bianconi

[...]

Quoted 7 lines

In general I see no reason to populate these fields before the XDP program runs. Someone needs to convince me why having frags info before program runs is useful. In general headers should be preserved and first frag already included in the data pointers. If users start parsing further they might need it, but this series doesn't provide a way to do that so IMO without those helpers its a bit difficult to debate.

Quoted 7 lines

Quoted 4 lines

We need to populate the skb_shared_info before running the xdp program in order to allow the ebpf sanbox to access this data. If we restrict the access to the first buffer only I guess we can avoid to do that but I think there is a value allowing the xdp program to access this data.

I agree. We could also only populate the fields if the program accesses the fields.

Notice, a driver will not initialize/use the shared_info area unless there are more segments. And (we have already established) the xdp->mb bit is guarding BPF-prog from accessing shared_info area.

Quoted 5 lines

Quoted 3 lines

A possible optimization can be access the shared_info only once before running the ebpf program constructing the shared_info using a struct allocated on the stack.

Seems interesting, might be a good idea.

Quoted 4 lines

It *might* be a good idea ("alloc" shared_info on stack), but we should benchmark this. The prefetch trick might be fast enough. But also keep in mind the performance target, as with large size frames the packet-per-sec we need to handle dramatically drop.

right. I guess we need to define a workload we want to run for the xdp multi-buff use-case (e.g. if MTU is 9K we will have ~3 frames for each packets and # of pps will be much slower)

Quoted 3 lines

[...]

Quoted 5 lines

I do think it makes sense to drop the helpers for now, and focus on how this new multi-buffer frame type is handled in the existing code, and do some benchmarking on higher speed NIC, before the BPF-helper start to lockdown/restrict what we can change/revert as they define UAPI.

ack, I will drop them in v5.

Regards,
Lorenzo

Quoted 14 lines

E.g. existing code that need to handle this is existing helper bpf_xdp_adjust_tail, which is something I have broad up before and even described in[1]. Lets make sure existing code works with proposed design, before introducing new helpers (and this makes it easier to revert).

[1] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org#xdp-tail-adjust

Signature · 4 lines

Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Quoted 14 lines · Lorenzo Bianconi

This series introduce XDP multi-buffer support. The mvneta driver is the first to support these new "non-linear" xdp_{buff,frame}. Reviewers please focus on how these new types of xdp_{buff,frame} packets traverse the different layers and the layout design. It is on purpose that BPF-helpers are kept simple, as we don't want to expose the internal layout to allow later changes.

For now, to keep the design simple and to maintain performance, the XDP BPF-prog (still) only have access to the first-buffer. It is left for later (another patchset) to add payload access across multiple buffers. This patchset should still allow for these future extensions. The goal is to lift the XDP MTU restriction that comes with XDP, but maintain same performance as before.

Quoted 7 lines

The main idea for the new multi-buffer layout is to reuse the same layout used for non-linear SKB. This rely on the "skb_shared_info" struct at the end of the first buffer to link together subsequent buffers. Keeping the layout compatible with SKBs is also done to ease and speedup creating an SKB from an xdp_{buff,frame}. Converting xdp_frame to SKB and deliver it to the network stack is shown in cpumap code (patch 13/13).

Using the end of the buffer for the skb_shared_info struct is going to become driver API so unwinding it if it proves to be a performance issue is going to be ugly. So same question as before, for the use case where we receive packet and do XDP_TX with it how do we avoid cache miss overhead? This is not just a hypothetical use case, the Facebook load balancer is doing this as well as Cilium and allowing this with multi-buffer packets >1500B would be useful.

Quoted 24 lines

Typical use cases for this series are:
- Jumbo-frames
- Packet header split (please see Google���s use-case @ NetDevConf 0x14, [0])
- TSO

More info about the main idea behind this approach can be found here [1][2].

We carried out some throughput tests in a standard linear frame scenario in order to verify we did not introduced any performance regression adding xdp multi-buff support to mvneta:

offered load is ~ 1000Kpps, packet size is 64B, mvneta descriptor size is one PAGE

commit: 879456bedbe5 ("net: mvneta: avoid possible cache misses in mvneta_rx_swbm")

- xdp-pass:      ~162Kpps
- xdp-drop:      ~701Kpps
- xdp-tx:        ~185Kpps
- xdp-redirect:  ~202Kpps

mvneta xdp multi-buff:

- xdp-pass:      ~163Kpps
- xdp-drop:      ~739Kpps
- xdp-tx:        ~182Kpps
- xdp-redirect:  ~202Kpps

Quoted 2 lines

[..] Typically for such features to land is to require at least 2 drivers implementing it.

-----Original Message-----
From: Daniel Borkmann <[email protected]>
Sent: Friday, October 2, 2020 10:53 PM
To: John Fastabend <[email protected]>; Lorenzo Bianconi <[email protected]>; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Agroskin, Shay <[email protected]>; Jubran, Samih <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]
Subject: RE: [EXTERNAL] [PATCH v4 bpf-next 00/13] mvneta: introduce XDP multi-buffer support

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.

Quoted 30 lines · John Fastabend

Quoted 14 lines · Lorenzo Bianconi

This series introduce XDP multi-buffer support. The mvneta driver is the first to support these new "non-linear" xdp_{buff,frame}. Reviewers please focus on how these new types of xdp_{buff,frame} packets traverse the different layers and the layout design. It is on purpose that BPF-helpers are kept simple, as we don't want to expose the internal layout to allow later changes.

For now, to keep the design simple and to maintain performance, the XDP BPF-prog (still) only have access to the first-buffer. It is left for later (another patchset) to add payload access across multiple buffers. This patchset should still allow for these future extensions. The goal is to lift the XDP MTU restriction that comes with XDP, but maintain same performance as before.

Quoted 7 lines

The main idea for the new multi-buffer layout is to reuse the same layout used for non-linear SKB. This rely on the "skb_shared_info" struct at the end of the first buffer to link together subsequent buffers. Keeping the layout compatible with SKBs is also done to ease and speedup creating an SKB from an xdp_{buff,frame}. Converting xdp_frame to SKB and deliver it to the network stack is shown in cpumap code (patch 13/13).

Using the end of the buffer for the skb_shared_info struct is going to become driver API so unwinding it if it proves to be a performance issue is going to be ugly. So same question as before, for the use case where we receive packet and do XDP_TX with it how do we avoid cache miss overhead? This is not just a hypothetical use case, the Facebook load balancer is doing this as well as Cilium and allowing this with multi-buffer packets >1500B would be useful.

[...]

Fully agree. My other question would be if someone else right now is in the process of implementing this scheme for a 40G+ NIC? My concern is the numbers below are rather on the lower end of the spectrum, so I would like to see a comparison of XDP as-is today vs XDP multi-buff on a higher end NIC so that we have a picture how well the current designed scheme works there and into which performance issue we'll run e.g.
under typical XDP L4 load balancer scenario with XDP_TX. I think this would be crucial before the driver API becomes 'sort of' set in stone where others start to adapting it and changing design becomes painful. Do ena folks have an implementation ready as well? And what about virtio_net, for example, anyone committing there too? Typically for such features to land is to require at least 2 drivers implementing it.

Quoted 7 lines

Quoted 7 lines

Typical use cases for this series are:
- Jumbo-frames
- Packet header split (please see Google s use-case @ NetDevConf
0x14, [0])
- TSO

More info about the main idea behind this approach can be found here

[1][2].

Quoted 9 lines

Quoted 9 lines

We carried out some throughput tests in a standard linear frame scenario in order to verify we did not introduced any performance regression adding xdp multi-buff support to mvneta:

offered load is ~ 1000Kpps, packet size is 64B, mvneta descriptor size is one PAGE

commit: 879456bedbe5 ("net: mvneta: avoid possible cache misses in

mvneta_rx_swbm")

Quoted 10 lines

Quoted 10 lines

- xdp-pass:      ~162Kpps
- xdp-drop:      ~701Kpps
- xdp-tx:        ~185Kpps
- xdp-redirect:  ~202Kpps

mvneta xdp multi-buff:

- xdp-pass:      ~163Kpps
- xdp-drop:      ~739Kpps
- xdp-tx:        ~182Kpps
- xdp-redirect:  ~202Kpps

[...]