Thank you for doing this! See comments inline.
> -----Original Message-----
> From: Stephen Hemminger <[email protected]>
> Sent: Tuesday 4 November 2025 16:07
> To: [email protected]
> Cc: Stephen Hemminger <[email protected]>; Konstantin Ananyev
> <[email protected]>
> 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 <[email protected]>
> ---
> 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
