From: Paul Chaignon <paul.chaignon@gmail.com>
To: sun jian <sun.jian.kdev@gmail.com>
Cc: bot+bpf-ci@kernel.org, bpf@vger.kernel.org,
netdev@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org, ast@kernel.org,
daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev,
eddyz87@gmail.com, memxor@gmail.com, song@kernel.org,
yonghong.song@linux.dev, jolsa@kernel.org, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
horms@kernel.org, shuah@kernel.org, hawk@kernel.org,
john.fastabend@gmail.com, sdf@fomichev.me, toke@redhat.com,
lorenzo@kernel.org, martin.lau@kernel.org, clm@meta.com,
ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf v3 2/2] selftests/bpf: Cover partial copy of non-linear test_run output
Date: Thu, 18 Jun 2026 10:44:46 +0200 [thread overview]
Message-ID: <ajOv_oOd1zInaW1b@mail.gmail.com> (raw)
In-Reply-To: <CABFUUZFeh18OjQ6EhjD17ZwQKb1aiVNkYKv-hAkVGrHhPpjE4Q@mail.gmail.com>
On Wed, Jun 17, 2026 at 10:19:52PM +0800, sun jian wrote:
> On Wed, Jun 17, 2026 at 6:31 PM <bot+bpf-ci@kernel.org> wrote:
> >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c b/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c
> > > index 01f1d1b6715a..9cc898e6a9f7 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c
> > > @@ -4,6 +4,10 @@
> > >
> > > #include "test_pkt_access.skel.h"
> > >
> > > +#define NONLINEAR_PKT_LEN 9000
> > > +#define NONLINEAR_LINEAR_DATA_LEN 64
> > > +#define SHORT_OUT_LEN 100
> > > +
> >
> > [ ... ]
> >
> > > @@ -20,6 +24,69 @@ static void check_run_cnt(int prog_fd, __u64 run_cnt)
> > > "incorrect number of repetitions, want %llu have %llu\n", run_cnt, info.run_cnt);
> > > }
> > >
> > > +static void init_pkt(__u8 *pkt, size_t len)
> > > +{
> > > + size_t i;
> > > +
> > > + for (i = 0; i < len; i++)
> > > + pkt[i] = i & 0xff;
> > > +}
> >
> > A question was raised on v2 about whether pkt_v4 could be reused by
> > reducing the linear area to ETH_HLEN, rather than introducing a custom
> > init_pkt() with a 9000-byte stack packet.
> >
> > Can't we reuse pkt_v4 here by reducing the linear area to ETH_HLEN?
> > The v3 still adds init_pkt() and the NONLINEAR_PKT_LEN packet, so this
> > doesn't seem to have been picked up.
> >
> > > +
> > > +static void test_skb_nonlinear_data_out_partial(struct test_pkt_access *skel)
> > > +{
> > > + LIBBPF_OPTS(bpf_test_run_opts, topts);
> > > + __u8 pkt[NONLINEAR_PKT_LEN];
> > > + __u8 out[SHORT_OUT_LEN];
> > > + struct __sk_buff skb = {};
> > > + int prog_fd, err;
> > > +
> > > + init_pkt(pkt, sizeof(pkt));
> > > +
> > > + skb.data_end = NONLINEAR_LINEAR_DATA_LEN;
> > > +
> > > + topts.data_in = pkt;
> > > + topts.data_size_in = sizeof(pkt);
> > > + topts.data_out = out;
> > > + topts.data_size_out = sizeof(out);
> > > + topts.ctx_in = &skb;
> > > + topts.ctx_size_in = sizeof(skb);
> > > +
> > > + prog_fd = bpf_program__fd(skel->progs.tc_pass_prog);
> >
> > [ ... ]
> >
> > > diff --git a/tools/testing/selftests/bpf/progs/test_pkt_access.c b/tools/testing/selftests/bpf/progs/test_pkt_access.c
> > > index bce7173152c6..cd284401eebd 100644
> > > --- a/tools/testing/selftests/bpf/progs/test_pkt_access.c
> > > +++ b/tools/testing/selftests/bpf/progs/test_pkt_access.c
> > > @@ -150,3 +150,15 @@ int test_pkt_access(struct __sk_buff *skb)
> > >
> > > return TC_ACT_UNSPEC;
> > > }
> > > +
> > > +SEC("tc")
> > > +int tc_pass_prog(struct __sk_buff *skb)
> > > +{
> > > + return TC_ACT_OK;
> > > +}
> > > +
> > > +SEC("xdp.frags")
> > > +int xdp_frags_pass_prog(struct xdp_md *ctx)
> > > +{
> > > + return XDP_PASS;
> > > +}
> >
> > A related suggestion on v2 was that, once pkt_v4 is reused, the existing
> > BPF program could be reused instead of adding new pass-through programs.
> >
> > Could tc_pass_prog and xdp_frags_pass_prog be dropped in favour of the
> > existing program? The v3 still adds both of these, so this point also
> > seems to be open.
> >
> >
> > ---
> > AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> > See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> >
> > CI run summary: https://github.com/kernel-patches/bpf/actions/runs/27680511802
>
> Hi,
>
> Thanks for checking this.
Hi Sun Jian,
It would help if you could reply inline instead of at the end of the
messages, especially when there are multiple comments. See [1] for an
explanation of how that works.
1: https://kernelnewbies.org/FirstKernelPatch#Responding_inline
>
> I tried reusing pkt_v4 and the existing TC program, but they do not fit
> the skb case this test is trying to cover.
>
> For skb test_run, IPv4/IPv6 inputs with a too-short L3 header in the
> linear area are rejected before bpf_test_finish(). With pkt_v4 and a
> linear area of ETH_HLEN, the test fails with -EINVAL before reaching the
> partial copy-out path. If the linear area is increased enough to pass the
> IPv4 check, pkt_v4 is too small to both trigger the old
> copy_size - frag_size path and verify that the copied prefix spans the
> linear data and the first fragment. pkt_v6 has the same issue: after
> making the IPv6 header linear, only 20 bytes remain in frags.
>
> The existing test_pkt_access program has its own packet-access coverage
> goals and is not just a pass-through carrier. With such a short linear
> area or small packet fixture, it can fail before the test hits the
> bpf_test_finish()'s partial copy-out path. A pass-through TC program is
> therefore a better fit, because it keeps the test focused on the
> bpf_test_finish() copy-out semantics.
If we're keeping tc_pass_prog() then can't we use pkt_v4 and get rid of
init_pkt?
>
> For XDP, this object does not have an existing xdp.frags pass-through
> program, so the small XDP frags program is needed to cover the other
> caller of the shared bpf_test_finish() path.
>
> Thanks,
> Sun Jian
next prev parent reply other threads:[~2026-06-18 8:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-17 9:35 [PATCH bpf v3 0/2] Fix partial copy of non-linear test_run output Sun Jian
2026-06-17 9:35 ` [PATCH bpf v3 1/2] bpf: " Sun Jian
2026-06-18 8:46 ` Paul Chaignon
2026-06-17 9:35 ` [PATCH bpf v3 2/2] selftests/bpf: Cover " Sun Jian
2026-06-17 9:45 ` sashiko-bot
2026-06-17 10:31 ` bot+bpf-ci
2026-06-17 14:19 ` sun jian
2026-06-18 8:44 ` Paul Chaignon [this message]
2026-06-18 10:45 ` sun jian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ajOv_oOd1zInaW1b@mail.gmail.com \
--to=paul.chaignon@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bot+bpf-ci@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=clm@meta.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=eddyz87@gmail.com \
--cc=edumazet@google.com \
--cc=hawk@kernel.org \
--cc=horms@kernel.org \
--cc=ihor.solodrai@linux.dev \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=lorenzo@kernel.org \
--cc=martin.lau@kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sdf@fomichev.me \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=sun.jian.kdev@gmail.com \
--cc=toke@redhat.com \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.