| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMB2axNx_NDkC+nZdpOB5kPmq0Sf1=d2g4NjkPnxEURfuV2eKA@mail.gmail.com>
Date: Thu, 18 Sep 2025 12:43:13 -0700
From: Amery Hung <ameryhung@...il.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, alexei.starovoitov@...il.com,
andrii@...nel.org, daniel@...earbox.net, paul.chaignon@...il.com,
kuba@...nel.org, stfomichev@...il.com, martin.lau@...nel.org,
mohsin.bashr@...il.com, noren@...dia.com, dtatulea@...dia.com,
saeedm@...dia.com, tariqt@...dia.com, mbloch@...dia.com, kernel-team@...a.com
Subject: Re: [PATCH bpf-next v4 5/6] selftests/bpf: Test bpf_xdp_pull_data
On Thu, Sep 18, 2025 at 4:34 AM Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> On Wed, Sep 17, 2025 at 03:55:12PM -0700, Amery Hung wrote:
> > Test bpf_xdp_pull_data() with xdp packets with different layouts. The
> > xdp bpf program first checks if the layout is as expected. Then, it
> > calls bpf_xdp_pull_data(). Finally, it checks the 0xbb marker at offset
> > 1024 using directly packet access.
> >
> > Signed-off-by: Amery Hung <ameryhung@...il.com>
> > ---
> > .../selftests/bpf/prog_tests/xdp_pull_data.c | 176 ++++++++++++++++++
> > .../selftests/bpf/progs/test_xdp_pull_data.c | 48 +++++
> > 2 files changed, 224 insertions(+)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_pull_data.c
> > create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_pull_data.c
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_pull_data.c b/tools/testing/selftests/bpf/prog_tests/xdp_pull_data.c
> > new file mode 100644
> > index 000000000000..c16801b73fed
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/xdp_pull_data.c
> > @@ -0,0 +1,176 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <test_progs.h>
> > +#include <network_helpers.h>
> > +#include "test_xdp_pull_data.skel.h"
> > +
> > +#define PULL_MAX (1 << 31)
> > +#define PULL_PLUS_ONE (1 << 30)
> > +
> > +#define XDP_PACKET_HEADROOM 256
> > +
> > +/* Find sizes of struct skb_shared_info and struct xdp_frame so that
> > + * we can calculate the maximum pull lengths for test cases
>
> do you really need this hack? Wouldn't it be possible to find these sizes
> via BTF?
It is possible. I will use kernel BTF to find the sizes.
>
> > + */
> > +static int find_xdp_sizes(struct test_xdp_pull_data *skel, int frame_sz)
> > +{
> > + LIBBPF_OPTS(bpf_test_run_opts, topts);
> > + struct xdp_md ctx = {};
> > + int prog_fd, err;
> > + __u8 *buf;
> > +
> > + buf = calloc(frame_sz, sizeof(__u8));
> > + if (!ASSERT_OK_PTR(buf, "calloc buf"))
> > + return -ENOMEM;
> > +
> > + topts.data_in = buf;
> > + topts.data_out = buf;
> > + topts.data_size_in = frame_sz;
> > + topts.data_size_out = frame_sz;
> > + /* Pass a data_end larger than the linear space available to make sure
> > + * bpf_prog_test_run_xdp() will fill the linear data area so that
> > + * xdp_find_data_hard_end can infer the size of struct skb_shared_info
>
> what is xdp_find_data_hard_end ?
It is supposed to be the XDP program, xdp_find_sizes. Will remove it
as we use BTF to find sizes.
>
> > + */
> > + ctx.data_end = frame_sz;
> > + topts.ctx_in = &ctx;
> > + topts.ctx_out = &ctx;
> > + topts.ctx_size_in = sizeof(ctx);
> > + topts.ctx_size_out = sizeof(ctx);
> > +
> > + prog_fd = bpf_program__fd(skel->progs.xdp_find_sizes);
> > + err = bpf_prog_test_run_opts(prog_fd, &topts);
> > + ASSERT_OK(err, "bpf_prog_test_run_opts");
> > +
> > + free(buf);
> > +
> > + return err;
> > +}
> > +
> > +/* xdp_pull_data_prog will directly read a marker 0xbb stored at buf[1024]
> > + * so caller expecting XDP_PASS should always pass pull_len no less than 1024
> > + */
> > +static void run_test(struct test_xdp_pull_data *skel, int retval,
> > + int frame_sz, int buff_len, int meta_len, int data_len,
> > + int pull_len)
> > +{
> > + LIBBPF_OPTS(bpf_test_run_opts, topts);
> > + struct xdp_md ctx = {};
> > + int prog_fd, err;
> > + __u8 *buf;
> > +
> > + buf = calloc(buff_len, sizeof(__u8));
> > + if (!ASSERT_OK_PTR(buf, "calloc buf"))
> > + return;
> > +
> > + buf[meta_len + 1023] = 0xaa;
> > + buf[meta_len + 1024] = 0xbb;
> > + buf[meta_len + 1025] = 0xcc;
> > +
> > + topts.data_in = buf;
> > + topts.data_out = buf;
> > + topts.data_size_in = buff_len;
> > + topts.data_size_out = buff_len;
> > + ctx.data = meta_len;
> > + ctx.data_end = meta_len + data_len;
> > + topts.ctx_in = &ctx;
> > + topts.ctx_out = &ctx;
> > + topts.ctx_size_in = sizeof(ctx);
> > + topts.ctx_size_out = sizeof(ctx);
> > +
> > + skel->bss->data_len = data_len;
> > + if (pull_len & PULL_MAX) {
> > + int headroom = XDP_PACKET_HEADROOM - meta_len - skel->bss->xdpf_sz;
> > + int tailroom = frame_sz - XDP_PACKET_HEADROOM -
> > + data_len - skel->bss->sinfo_sz;
> > +
> > + pull_len = pull_len & PULL_PLUS_ONE ? 1 : 0;
>
> nit: pull_len = !!(pull_len & PULL_PLUS_ONE);
>
> > + pull_len += headroom + tailroom + data_len;
> > + }
> > + skel->bss->pull_len = pull_len;
> > +
> > + prog_fd = bpf_program__fd(skel->progs.xdp_pull_data_prog);
> > + err = bpf_prog_test_run_opts(prog_fd, &topts);
> > + ASSERT_OK(err, "bpf_prog_test_run_opts");
> > + ASSERT_EQ(topts.retval, retval, "xdp_pull_data_prog retval");
> > +
> > + if (retval == XDP_DROP)
> > + goto out;
> > +
> > + ASSERT_EQ(ctx.data_end, meta_len + pull_len, "linear data size");
> > + ASSERT_EQ(topts.data_size_out, buff_len, "linear + non-linear data size");
> > + /* Make sure data around xdp->data_end was not messed up by
> > + * bpf_xdp_pull_data()
> > + */
> > + ASSERT_EQ(buf[meta_len + 1023], 0xaa, "data[1023]");
> > + ASSERT_EQ(buf[meta_len + 1024], 0xbb, "data[1024]");
> > + ASSERT_EQ(buf[meta_len + 1025], 0xcc, "data[1025]");
> > +out:
> > + free(buf);
> > +}
> > +
> > +static void test_xdp_pull_data_basic(void)
> > +{
> > + u32 pg_sz, max_meta_len, max_data_len;
> > + struct test_xdp_pull_data *skel;
> > +
> > + skel = test_xdp_pull_data__open_and_load();
> > + if (!ASSERT_OK_PTR(skel, "test_xdp_pull_data__open_and_load"))
> > + return;
> > +
> > + pg_sz = sysconf(_SC_PAGE_SIZE);
> > +
> > + if (find_xdp_sizes(skel, pg_sz))
> > + goto out;
> > +
> > + max_meta_len = XDP_PACKET_HEADROOM - skel->bss->xdpf_sz;
> > + max_data_len = pg_sz - XDP_PACKET_HEADROOM - skel->bss->sinfo_sz;
> > +
> > + /* linear xdp pkt, pull 0 byte */
> > + run_test(skel, XDP_PASS, pg_sz, 2048, 0, 2048, 2048);
>
> you're passing pg_sz to avoid repeated syscalls I assume? Is it worth to pass
> prog_fd as well?
The performance is not too much of a concern here as it is selftest. I
would not add prog_fd to avoid confusion.
>
> > +
> > + /* multi-buf pkt, pull results in linear xdp pkt */
> > + run_test(skel, XDP_PASS, pg_sz, 2048, 0, 1024, 2048);
> > +
> > + /* multi-buf pkt, pull 1 byte to linear data area */
> > + run_test(skel, XDP_PASS, pg_sz, 9000, 0, 1024, 1025);
> > +
> > + /* multi-buf pkt, pull 0 byte to linear data area */
> > + run_test(skel, XDP_PASS, pg_sz, 9000, 0, 1025, 1025);
> > +
> > + /* multi-buf pkt, empty linear data area, pull requires memmove */
> > + run_test(skel, XDP_PASS, pg_sz, 9000, 0, 0, PULL_MAX);
> > +
> > + /* multi-buf pkt, no headroom */
> > + run_test(skel, XDP_PASS, pg_sz, 9000, max_meta_len, 1024, PULL_MAX);
> > +
> > + /* multi-buf pkt, no tailroom, pull requires memmove */
> > + run_test(skel, XDP_PASS, pg_sz, 9000, 0, max_data_len, PULL_MAX);
> > +
>
> nit: double empty line
Will add:
/* Test cases with invalid pull length */
>
> > +
> > + /* linear xdp pkt, pull more than total data len */
> > + run_test(skel, XDP_DROP, pg_sz, 2048, 0, 2048, 2049);
> > +
> > + /* multi-buf pkt with no space left in linear data area */
> > + run_test(skel, XDP_DROP, pg_sz, 9000, max_meta_len, max_data_len,
> > + PULL_MAX | PULL_PLUS_ONE);
> > +
> > + /* multi-buf pkt, empty linear data area */
> > + run_test(skel, XDP_DROP, pg_sz, 9000, 0, 0, PULL_MAX | PULL_PLUS_ONE);
> > +
> > + /* multi-buf pkt, no headroom */
> > + run_test(skel, XDP_DROP, pg_sz, 9000, max_meta_len, 1024,
> > + PULL_MAX | PULL_PLUS_ONE);
> > +
> > + /* multi-buf pkt, no tailroom */
> > + run_test(skel, XDP_DROP, pg_sz, 9000, 0, max_data_len,
> > + PULL_MAX | PULL_PLUS_ONE);
> > +
> > +out:
> > + test_xdp_pull_data__destroy(skel);
> > +}
> > +
> > +void test_xdp_pull_data(void)
> > +{
> > + if (test__start_subtest("xdp_pull_data"))
> > + test_xdp_pull_data_basic();
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/test_xdp_pull_data.c b/tools/testing/selftests/bpf/progs/test_xdp_pull_data.c
> > new file mode 100644
> > index 000000000000..dd901bb109b6
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/test_xdp_pull_data.c
> > @@ -0,0 +1,48 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include "vmlinux.h"
> > +#include <bpf/bpf_helpers.h>
> > +
> > +int xdpf_sz;
> > +int sinfo_sz;
> > +int data_len;
> > +int pull_len;
> > +
> > +#define XDP_PACKET_HEADROOM 256
> > +
> > +SEC("xdp.frags")
> > +int xdp_find_sizes(struct xdp_md *ctx)
> > +{
> > + xdpf_sz = sizeof(struct xdp_frame);
> > + sinfo_sz = __PAGE_SIZE - XDP_PACKET_HEADROOM -
> > + (ctx->data_end - ctx->data);
> > +
> > + return XDP_PASS;
> > +}
Will remove this XDP program
Thank you for your review!
[...]
> > --
> > 2.47.3
> >
Powered by blists - more mailing lists