RE: [PATCH v4 3/5] bpf: add a test for BPF ELF load

[Marat Khalili <marat.khalili@huawei.com>]

Thank you for doing this! See comments inline.

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday 4 November 2025 16:07
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Konstantin Ananyev <konstantin.ananyev@huawei.com>
> Subject: [PATCH v4 3/5] bpf: add a test for BPF ELF load
> 
> Create an ELF file to load using clang.
> Repackage the object into an array using xdd.
> Write a test to see load and run the BPF.
> 
> Draft version made with Claude AI, but it didn't work.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  app/test/bpf/load.c      |  62 +++++++++++++++++
>  app/test/bpf/meson.build |  52 ++++++++++++++
>  app/test/meson.build     |   2 +
>  app/test/test_bpf.c      | 147 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 263 insertions(+)
>  create mode 100644 app/test/bpf/load.c
>  create mode 100644 app/test/bpf/meson.build
> 
> diff --git a/app/test/bpf/load.c b/app/test/bpf/load.c
> new file mode 100644
> index 0000000000..9678c110d9
> --- /dev/null
> +++ b/app/test/bpf/load.c
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * BPF program for testing rte_bpf_elf_load
> + */
> +
> +typedef unsigned char uint8_t;
> +typedef unsigned short uint16_t;
> +typedef unsigned int uint32_t;
> +typedef unsigned long uint64_t;
> +
> +/* Match the structures from test_bpf.c */
> +struct dummy_offset {
> +	uint64_t u64;
> +	uint32_t u32;
> +	uint16_t u16;
> +	uint8_t  u8;
> +} __attribute__((packed));
> +
> +struct dummy_vect8 {
> +	struct dummy_offset in[8];
> +	struct dummy_offset out[8];
> +};

Nit: I am sympathetic to re-using types already defined in test_bpf.c, but we
don't really need 8 elements, and first two members in dummy_offset are no
longer atomic which may or may not create problems someday. Perhaps it would be
easier and less confusing to just define another type for this test.

> +
> +/* External function declaration - provided by test via xsym */
> +extern void dummy_func1(const void *p, uint32_t *v32, uint64_t *v64);

Any reason not to make first argument `struct dummy_vect8`? Not that it
mattered for the test, just creates an impression that something is not working 
or not trivial here.

> +
> +/*
> + * Test BPF function that will be loaded from ELF
> + * This function:
> + * 1. Reads values from input structure
> + * 2. Performs some computations
> + * 3. Writes results to output structure
> + * 4. Returns sum of values
> + */
> +__attribute__((section("func"), used))

Do we need `used` here and in other section attributes? I do not thing `clang
-c` will eliminate non-static/inline functions from the object file, or maybe I
don't understand the purpose (perhaps needs a comment).

> +uint64_t
> +test_func(struct dummy_vect8 *arg)
> +{
> +	uint64_t sum = 0;
> +	uint32_t v32;
> +	uint64_t v64;
> +
> +	/* Load input values */
> +	v32 = arg->in[0].u32;
> +	v64 = arg->in[0].u64;
> +
> +	/* Call external function */
> +	dummy_func1(arg, &v32, &v64);
> +
> +	/* Store results */
> +	arg->out[0].u32 = v32;
> +	arg->out[0].u64 = v64;
> +
> +	/* Calculate sum */
> +	sum = arg->in[0].u64;
> +	sum += arg->in[0].u32;
> +	sum += arg->in[0].u16;
> +	sum += arg->in[0].u8;
> +	sum += v32;
> +	sum += v64;
> +
> +	return sum;
> +}

// snip meson files, lgtm

> --- a/app/test/test_bpf.c
> +++ b/app/test/test_bpf.c
> @@ -6,6 +6,7 @@
>  #include <string.h>
>  #include <stdint.h>
>  #include <inttypes.h>
> +#include <unistd.h>
> 
>  #include <rte_memory.h>
>  #include <rte_debug.h>
> @@ -14,6 +15,8 @@
>  #include <rte_random.h>
>  #include <rte_byteorder.h>
>  #include <rte_errno.h>
> +#include <rte_bpf.h>
> +
>  #include "test.h"
> 
>  #if !defined(RTE_LIB_BPF)
> @@ -3278,6 +3281,150 @@ test_bpf(void)
> 
>  REGISTER_FAST_TEST(bpf_autotest, true, true, test_bpf);
> 
> +#ifdef TEST_BPF_ELF_LOAD
> +
> +/*
> + * Helper function to write BPF object data to temporary file.
> + * Returns temp file path on success, NULL on failure.
> + * Caller must free the returned path and unlink the file.
> + */
> +static char *
> +create_temp_bpf_file(const uint8_t *data, size_t size, const char *suffix)

Nit: last argument is not technically a suffix.

> +{
> +	char *tmpfile = NULL;
> +	int fd;
> +	ssize_t written;
> +
> +	if (asprintf(&tmpfile, "/tmp/dpdk_bpf_%s_XXXXXX", suffix) < 0) {

Not sure what the actual standard is, but I found in some previous discussions
a suggestion to use $TMPDIR and only fall back to /tmp if it is not defined.

Also, maybe give it .o suffix and use mkstemps below? 

> +		printf("%s@%d: asprintf failed: %s\n",
> +		       __func__, __LINE__, strerror(errno));
> +		return NULL;
> +	}
> +
> +	/* Create and open temp file */
> +	fd = mkstemp(tmpfile);
> +	if (fd < 0) {
> +		printf("%s@%d: mkstemp(%s) failed: %s\n",
> +		       __func__, __LINE__, tmpfile, strerror(errno));
> +		free(tmpfile);
> +		return NULL;
> +	}
> +
> +	/* Write BPF object data */
> +	written = write(fd, data, size);
> +	close(fd);
> +
> +	if (written != (ssize_t)size) {
> +		printf("%s@%d: write failed: %s\n",
> +		       __func__, __LINE__, strerror(errno));
> +		unlink(tmpfile);
> +		free(tmpfile);
> +		return NULL;
> +	}
> +
> +	return tmpfile;
> +}
> +
> +#include "test_bpf_load.h"
> +
> +static int
> +test_bpf_elf_load(void)
> +{
> +	uint8_t tbuf[sizeof(struct dummy_vect8)];

Why do we have to use unit8_t array here instead of just struct dummy_vect8?

If we do have to, how is alignment ensured?

> +	const struct rte_bpf_xsym xsym[] = {
> +		{
> +			.name = RTE_STR(dummy_func1),
> +			.type = RTE_BPF_XTYPE_FUNC,
> +			.func = {
> +				.val = (void *)dummy_func1,
> +				.nb_args = 3,
> +				.args = {
> +					[0] = {
> +						.type = RTE_BPF_ARG_PTR,
> +						.size = sizeof(struct dummy_offset),
> +					},
> +					[1] = {
> +						.type = RTE_BPF_ARG_PTR,
> +						.size = sizeof(uint32_t),
> +					},
> +					[2] = {
> +						.type = RTE_BPF_ARG_PTR,
> +						.size = sizeof(uint64_t),
> +					},
> +				},
> +			},
> +		},
> +	};
> +	int ret;
> +
> +	/* Create temp file from embedded BPF object */
> +	char *tmpfile = create_temp_bpf_file(app_test_bpf_load_o,
> +					     app_test_bpf_load_o_len,
> +					     "load");
> +	if (tmpfile == NULL)
> +		return -1;
> +
> +	/* Try to load BPF program from temp file */
> +	const struct rte_bpf_prm prm = {
> +		.xsym = xsym,
> +		.nb_xsym = RTE_DIM(xsym),
> +		.prog_arg = {
> +			.type = RTE_BPF_ARG_PTR,
> +			.size = sizeof(tbuf),
> +		},
> +	};
> +	struct rte_bpf *bpf = rte_bpf_elf_load(&prm, tmpfile, "func");
> +	TEST_ASSERT(bpf != NULL, "failed to load BPF from %s %d:%s",
> +		    tmpfile, rte_errno, strerror(rte_errno));
> +
> +	/* Prepare test data */
> +	struct dummy_vect8 *dv = (struct dummy_vect8 *)tbuf;
> +	memset(dv, 0, sizeof(*dv));
> +	dv->in[0].u64 = (int32_t)TEST_FILL_1;
> +	dv->in[0].u32 = dv->in[0].u64;
> +	dv->in[0].u16 = dv->in[0].u64;
> +	dv->in[0].u8 = dv->in[0].u64;
> +
> +	/* Execute loaded BPF program */
> +	uint64_t sum = rte_bpf_exec(bpf, tbuf);
> +	TEST_ASSERT(sum != 0, "BPF execution returned: %" PRIu64, sum);

Could we be more specific with the value we expect?

> +
> +	/* Test JIT if available */
> +	struct rte_bpf_jit jit;
> +	ret = rte_bpf_get_jit(bpf, &jit);
> +	TEST_ASSERT(ret == 0, "rte_bpf_get_jit failed: %d", ret);
> +
> +	if (jit.func != NULL) {
> +		memset(dv, 0, sizeof(*dv));
> +		dv->in[0].u64 = (int32_t)TEST_FILL_1;
> +		dv->in[0].u32 = dv->in[0].u64;
> +		dv->in[0].u16 = dv->in[0].u64;
> +		dv->in[0].u8 = dv->in[0].u64;
> +
> +		uint64_t jsum  = jit.func(tbuf);
> +		TEST_ASSERT_EQUAL(sum, jsum, "BPF JIT execution difference");
> +	}
> +
> +	rte_bpf_destroy(bpf);
> +	unlink(tmpfile);
> +	free(tmpfile);

If any of the above asserts fail, leak sanitizer will also be unhappy. Perhaps
we could unlink tmpfile just after the rte_bpf_elf_load call, and unload BPF
before verifying return values from calls.

> +
> +	printf("%s: ELF load test passed\n", __func__);
> +	return TEST_SUCCESS;
> +}
> +#else
> +
> +static int
> +test_bpf_elf_load(void)
> +{
> +	printf("BPF compile not supported, skipping test\n");
> +	return TEST_SKIPPED;
> +}
> +
> +#endif /* !TEST_BPF_ELF_LOAD */
> +
> +REGISTER_FAST_TEST(bpf_elf_load_autotest, true, true, test_bpf_elf_load);
> +
>  #ifndef RTE_HAS_LIBPCAP
> 
>  static int
> --
> 2.51.0