diff mbox series

[v7,bpf-next,09/14] bpd: add multi-buffer support to xdp copy helpers

Message ID d7e913be6855a2aeaaff16de39960b38afd06da7.1616179034.git.lorenzo@kernel.org (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series mvneta: introduce XDP multi-buffer support | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 8 maintainers not CCed: linux-kselftest@vger.kernel.org yhs@fb.com hawk@kernel.org kpsingh@kernel.org andrii@kernel.org kafai@fb.com songliubraving@fb.com shuah@kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 45 this patch: 45
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 45 this patch: 45
netdev/header_inline success Link

Commit Message

lorenzo@kernel.org March 19, 2021, 9:47 p.m. UTC 
From: Eelco Chaudron <echaudro@redhat.com>

This patch adds support for multi-buffer for the following helpers:
  - bpf_xdp_output()
  - bpf_perf_event_output()

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 net/core/filter.c                             |  60 ++++++++-
 .../selftests/bpf/prog_tests/xdp_bpf2bpf.c    | 127 ++++++++++++------
 .../selftests/bpf/progs/test_xdp_bpf2bpf.c    |   3 +-
 3 files changed, 146 insertions(+), 44 deletions(-)

Comments

kernel test robot March 20, 2021, 12:51 a.m. UTC | #1 
Hi Lorenzo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Lorenzo-Bianconi/mvneta-introduce-XDP-multi-buffer-support/20210320-055103
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: arm-randconfig-r023-20210318 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 436c6c9c20cc522c92a923440a5fc509c342a7db)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/9603affe766576f892f3a8e02d58dbe83092c74a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Lorenzo-Bianconi/mvneta-introduce-XDP-multi-buffer-support/20210320-055103
        git checkout 9603affe766576f892f3a8e02d58dbe83092c74a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/core/filter.c:4579:16: warning: variable 'copy_len' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized]
                           for (i = 0; i < xdp_sinfo->nr_frags; i++) {
                                       ^~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:4593:30: note: uninitialized use occurs here
                   memcpy(dst_buff, src_buff, copy_len);
                                              ^~~~~~~~
   net/core/filter.c:4579:16: note: remove the condition if it is always true
                           for (i = 0; i < xdp_sinfo->nr_frags; i++) {
                                       ^~~~~~~~~~~~~~~~~~~~~~~
   net/core/filter.c:4570:25: note: initialize the variable 'copy_len' to silence this warning
                   unsigned long copy_len;
                                         ^
                                          = 0
   1 warning generated.


vim +4579 net/core/filter.c

  4551	
  4552	static unsigned long bpf_xdp_copy(void *dst_buff, const void *ctx,
  4553					  unsigned long off, unsigned long len)
  4554	{
  4555		struct xdp_buff *xdp = (struct xdp_buff *)ctx;
  4556		struct xdp_shared_info *xdp_sinfo;
  4557		unsigned long base_len;
  4558		const void *src_buff;
  4559	
  4560		if (likely(!xdp->mb)) {
  4561			src_buff = xdp->data;
  4562			memcpy(dst_buff, src_buff + off, len);
  4563	
  4564			return 0;
  4565		}
  4566	
  4567		base_len = xdp->data_end - xdp->data;
  4568		xdp_sinfo = xdp_get_shared_info_from_buff(xdp);
  4569		do {
  4570			unsigned long copy_len;
  4571	
  4572			if (off < base_len) {
  4573				src_buff = xdp->data + off;
  4574				copy_len = min(len, base_len - off);
  4575			} else {
  4576				unsigned long frag_off_total = base_len;
  4577				int i;
  4578	
> 4579				for (i = 0; i < xdp_sinfo->nr_frags; i++) {
  4580					skb_frag_t *frag = &xdp_sinfo->frags[i];
  4581					unsigned long frag_len = xdp_get_frag_size(frag);
  4582					unsigned long frag_off = off - frag_off_total;
  4583	
  4584					if (frag_off < frag_len) {
  4585						src_buff = xdp_get_frag_address(frag) +
  4586							   frag_off;
  4587						copy_len = min(len, frag_len - frag_off);
  4588						break;
  4589					}
  4590					frag_off_total += frag_len;
  4591				}
  4592			}
  4593			memcpy(dst_buff, src_buff, copy_len);
  4594			off += copy_len;
  4595			len -= copy_len;
  4596			dst_buff += copy_len;
  4597		} while (len);
  4598	
  4599		return 0;
  4600	}
  4601	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot March 20, 2021, 3:47 a.m. UTC | #2 
Hi Lorenzo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Lorenzo-Bianconi/mvneta-introduce-XDP-multi-buffer-support/20210320-055103
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
compiler: nios2-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


cppcheck warnings: (new ones prefixed by >>)
        ^
   net/core/filter.c:10085:6: note: Variable 'ret' is reassigned a value before the old one has been used.
    ret = -EACCES;
        ^
   net/core/filter.c:10090:6: note: Variable 'ret' is reassigned a value before the old one has been used.
    ret = fprog->len;
        ^
>> net/core/filter.c:4593:30: warning: Uninitialized variable: copy_len [uninitvar]
     memcpy(dst_buff, src_buff, copy_len);
                                ^

vim +4593 net/core/filter.c

  4551	
  4552	static unsigned long bpf_xdp_copy(void *dst_buff, const void *ctx,
  4553					  unsigned long off, unsigned long len)
  4554	{
  4555		struct xdp_buff *xdp = (struct xdp_buff *)ctx;
  4556		struct xdp_shared_info *xdp_sinfo;
  4557		unsigned long base_len;
  4558		const void *src_buff;
  4559	
  4560		if (likely(!xdp->mb)) {
  4561			src_buff = xdp->data;
  4562			memcpy(dst_buff, src_buff + off, len);
  4563	
  4564			return 0;
  4565		}
  4566	
  4567		base_len = xdp->data_end - xdp->data;
  4568		xdp_sinfo = xdp_get_shared_info_from_buff(xdp);
  4569		do {
  4570			unsigned long copy_len;
  4571	
  4572			if (off < base_len) {
  4573				src_buff = xdp->data + off;
  4574				copy_len = min(len, base_len - off);
  4575			} else {
  4576				unsigned long frag_off_total = base_len;
  4577				int i;
  4578	
  4579				for (i = 0; i < xdp_sinfo->nr_frags; i++) {
  4580					skb_frag_t *frag = &xdp_sinfo->frags[i];
  4581					unsigned long frag_len = xdp_get_frag_size(frag);
  4582					unsigned long frag_off = off - frag_off_total;
  4583	
  4584					if (frag_off < frag_len) {
  4585						src_buff = xdp_get_frag_address(frag) +
  4586							   frag_off;
  4587						copy_len = min(len, frag_len - frag_off);
  4588						break;
  4589					}
  4590					frag_off_total += frag_len;
  4591				}
  4592			}
> 4593			memcpy(dst_buff, src_buff, copy_len);
  4594			off += copy_len;
  4595			len -= copy_len;
  4596			dst_buff += copy_len;
  4597		} while (len);
  4598	
  4599		return 0;
  4600	}
  4601	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/net/core/filter.c b/net/core/filter.c
index 18b2c9bacba1..a607ea8321bd 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4549,10 +4549,53 @@  static const struct bpf_func_proto bpf_sk_ancestor_cgroup_id_proto = {
 };
 #endif
 
-static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
+static unsigned long bpf_xdp_copy(void *dst_buff, const void *ctx,
 				  unsigned long off, unsigned long len)
 {
-	memcpy(dst_buff, src_buff + off, len);
+	struct xdp_buff *xdp = (struct xdp_buff *)ctx;
+	struct xdp_shared_info *xdp_sinfo;
+	unsigned long base_len;
+	const void *src_buff;
+
+	if (likely(!xdp->mb)) {
+		src_buff = xdp->data;
+		memcpy(dst_buff, src_buff + off, len);
+
+		return 0;
+	}
+
+	base_len = xdp->data_end - xdp->data;
+	xdp_sinfo = xdp_get_shared_info_from_buff(xdp);
+	do {
+		unsigned long copy_len;
+
+		if (off < base_len) {
+			src_buff = xdp->data + off;
+			copy_len = min(len, base_len - off);
+		} else {
+			unsigned long frag_off_total = base_len;
+			int i;
+
+			for (i = 0; i < xdp_sinfo->nr_frags; i++) {
+				skb_frag_t *frag = &xdp_sinfo->frags[i];
+				unsigned long frag_len = xdp_get_frag_size(frag);
+				unsigned long frag_off = off - frag_off_total;
+
+				if (frag_off < frag_len) {
+					src_buff = xdp_get_frag_address(frag) +
+						   frag_off;
+					copy_len = min(len, frag_len - frag_off);
+					break;
+				}
+				frag_off_total += frag_len;
+			}
+		}
+		memcpy(dst_buff, src_buff, copy_len);
+		off += copy_len;
+		len -= copy_len;
+		dst_buff += copy_len;
+	} while (len);
+
 	return 0;
 }
 
@@ -4564,10 +4607,19 @@  BPF_CALL_5(bpf_xdp_event_output, struct xdp_buff *, xdp, struct bpf_map *, map,
 	if (unlikely(flags & ~(BPF_F_CTXLEN_MASK | BPF_F_INDEX_MASK)))
 		return -EINVAL;
 	if (unlikely(!xdp ||
-		     xdp_size > (unsigned long)(xdp->data_end - xdp->data)))
+		     (likely(!xdp->mb) &&
+		      xdp_size > (unsigned long)(xdp->data_end - xdp->data))))
 		return -EFAULT;
+	if (unlikely(xdp->mb)) {
+		struct xdp_shared_info *xdp_sinfo;
+
+		xdp_sinfo = xdp_get_shared_info_from_buff(xdp);
+		if (unlikely(xdp_size > ((int)(xdp->data_end - xdp->data) +
+					 xdp_sinfo->data_length)))
+			return -EFAULT;
+	}
 
-	return bpf_event_output(map, flags, meta, meta_size, xdp->data,
+	return bpf_event_output(map, flags, meta, meta_size, xdp,
 				xdp_size, bpf_xdp_copy);
 }
 
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
index 2c6c570b21f8..355e64526f3f 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_bpf2bpf.c
@@ -10,11 +10,20 @@  struct meta {
 	int pkt_len;
 };
 
+struct test_ctx_s {
+	bool passed;
+	int pkt_size;
+};
+
+struct test_ctx_s test_ctx;
+
 static void on_sample(void *ctx, int cpu, void *data, __u32 size)
 {
-	int duration = 0;
 	struct meta *meta = (struct meta *)data;
 	struct ipv4_packet *trace_pkt_v4 = data + sizeof(*meta);
+	unsigned char *raw_pkt = data + sizeof(*meta);
+	struct test_ctx_s *tst_ctx = ctx;
+	int duration = 0;
 
 	if (CHECK(size < sizeof(pkt_v4) + sizeof(*meta),
 		  "check_size", "size %u < %zu\n",
@@ -25,25 +34,90 @@  static void on_sample(void *ctx, int cpu, void *data, __u32 size)
 		  "meta->ifindex = %d\n", meta->ifindex))
 		return;
 
-	if (CHECK(meta->pkt_len != sizeof(pkt_v4), "check_meta_pkt_len",
-		  "meta->pkt_len = %zd\n", sizeof(pkt_v4)))
+	if (CHECK(meta->pkt_len != tst_ctx->pkt_size, "check_meta_pkt_len",
+		  "meta->pkt_len = %d\n", tst_ctx->pkt_size))
 		return;
 
 	if (CHECK(memcmp(trace_pkt_v4, &pkt_v4, sizeof(pkt_v4)),
 		  "check_packet_content", "content not the same\n"))
 		return;
 
-	*(bool *)ctx = true;
+	if (meta->pkt_len > sizeof(pkt_v4)) {
+		for (int i = 0; i < (meta->pkt_len - sizeof(pkt_v4)); i++) {
+			if (raw_pkt[i + sizeof(pkt_v4)] != (unsigned char)i) {
+				CHECK(true, "check_packet_content",
+				      "byte %zu does not match %u != %u\n",
+				      i + sizeof(pkt_v4),
+				      raw_pkt[i + sizeof(pkt_v4)],
+				      (unsigned char)i);
+				break;
+			}
+		}
+	}
+
+	tst_ctx->passed = true;
 }
 
-void test_xdp_bpf2bpf(void)
+static int run_xdp_bpf2bpf_pkt_size(int pkt_fd, struct perf_buffer *pb,
+				    struct test_xdp_bpf2bpf *ftrace_skel,
+				    int pkt_size)
 {
 	__u32 duration = 0, retval, size;
-	char buf[128];
+	unsigned char buf_in[9000];
+	unsigned char buf[9000];
+	int err;
+
+	if (pkt_size > sizeof(buf_in) || pkt_size < sizeof(pkt_v4))
+		return -EINVAL;
+
+	test_ctx.passed = false;
+	test_ctx.pkt_size = pkt_size;
+
+	memcpy(buf_in, &pkt_v4, sizeof(pkt_v4));
+	if (pkt_size > sizeof(pkt_v4)) {
+		for (int i = 0; i < (pkt_size - sizeof(pkt_v4)); i++)
+			buf_in[i + sizeof(pkt_v4)] = i;
+	}
+
+	/* Run test program */
+	err = bpf_prog_test_run(pkt_fd, 1, buf_in, pkt_size,
+				buf, &size, &retval, &duration);
+
+	if (CHECK(err || retval != XDP_PASS || size != pkt_size,
+		  "ipv4", "err %d errno %d retval %d size %d\n",
+		  err, errno, retval, size))
+		return -1;
+
+	/* Make sure bpf_xdp_output() was triggered and it sent the expected
+	 * data to the perf ring buffer.
+	 */
+	err = perf_buffer__poll(pb, 100);
+	if (CHECK(err <= 0, "perf_buffer__poll", "err %d\n", err))
+		return -1;
+
+	if (CHECK_FAIL(!test_ctx.passed))
+		return -1;
+
+	/* Verify test results */
+	if (CHECK(ftrace_skel->bss->test_result_fentry != if_nametoindex("lo"),
+		  "result", "fentry failed err %llu\n",
+		  ftrace_skel->bss->test_result_fentry))
+		return -1;
+
+	if (CHECK(ftrace_skel->bss->test_result_fexit != XDP_PASS, "result",
+		  "fexit failed err %llu\n",
+		  ftrace_skel->bss->test_result_fexit))
+		return -1;
+
+	return 0;
+}
+
+void test_xdp_bpf2bpf(void)
+{
 	int err, pkt_fd, map_fd;
-	bool passed = false;
-	struct iphdr *iph = (void *)buf + sizeof(struct ethhdr);
-	struct iptnl_info value4 = {.family = AF_INET};
+	__u32 duration = 0;
+	int pkt_sizes[] = {sizeof(pkt_v4), 1024, 4100, 8200};
+	struct iptnl_info value4 = {.family = AF_INET6};
 	struct test_xdp *pkt_skel = NULL;
 	struct test_xdp_bpf2bpf *ftrace_skel = NULL;
 	struct vip key4 = {.protocol = 6, .family = AF_INET};
@@ -87,40 +161,15 @@  void test_xdp_bpf2bpf(void)
 
 	/* Set up perf buffer */
 	pb_opts.sample_cb = on_sample;
-	pb_opts.ctx = &passed;
+	pb_opts.ctx = &test_ctx;
 	pb = perf_buffer__new(bpf_map__fd(ftrace_skel->maps.perf_buf_map),
-			      1, &pb_opts);
+			      8, &pb_opts);
 	if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb)))
 		goto out;
 
-	/* Run test program */
-	err = bpf_prog_test_run(pkt_fd, 1, &pkt_v4, sizeof(pkt_v4),
-				buf, &size, &retval, &duration);
-
-	if (CHECK(err || retval != XDP_TX || size != 74 ||
-		  iph->protocol != IPPROTO_IPIP, "ipv4",
-		  "err %d errno %d retval %d size %d\n",
-		  err, errno, retval, size))
-		goto out;
-
-	/* Make sure bpf_xdp_output() was triggered and it sent the expected
-	 * data to the perf ring buffer.
-	 */
-	err = perf_buffer__poll(pb, 100);
-	if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err))
-		goto out;
-
-	CHECK_FAIL(!passed);
-
-	/* Verify test results */
-	if (CHECK(ftrace_skel->bss->test_result_fentry != if_nametoindex("lo"),
-		  "result", "fentry failed err %llu\n",
-		  ftrace_skel->bss->test_result_fentry))
-		goto out;
-
-	CHECK(ftrace_skel->bss->test_result_fexit != XDP_TX, "result",
-	      "fexit failed err %llu\n", ftrace_skel->bss->test_result_fexit);
-
+	for (int i = 0; i < ARRAY_SIZE(pkt_sizes); i++)
+		run_xdp_bpf2bpf_pkt_size(pkt_fd, pb, ftrace_skel,
+					 pkt_sizes[i]);
 out:
 	if (pb)
 		perf_buffer__free(pb);
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c b/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
index a038e827f850..d5a5f603d252 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_bpf2bpf.c
@@ -27,6 +27,7 @@  struct xdp_buff {
 	void *data_hard_start;
 	unsigned long handle;
 	struct xdp_rxq_info *rxq;
+	__u32 frame_length;
 } __attribute__((preserve_access_index));
 
 struct meta {
@@ -49,7 +50,7 @@  int BPF_PROG(trace_on_entry, struct xdp_buff *xdp)
 	void *data = (void *)(long)xdp->data;
 
 	meta.ifindex = xdp->rxq->dev->ifindex;
-	meta.pkt_len = data_end - data;
+	meta.pkt_len = xdp->frame_length;
 	bpf_xdp_output(xdp, &perf_buf_map,
 		       ((__u64) meta.pkt_len << 32) |
 		       BPF_F_CURRENT_CPU,