On Thu, Jun 18, 2026 at 4:44 PM Paul Chaignon <[email protected]> wrote: > > On Wed, Jun 17, 2026 at 10:19:52PM +0800, sun jian wrote: > > On Wed, Jun 17, 2026 at 6:31 PM <[email protected]> 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
Acknowledged, I will reply 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? > pkt_v4 is too small to construct a meaningful nonlinear skb with a stable linear/frag split while still exercising the partial copy-out boundary in bpf_test_finish(). With pkt_v4, we either do not reach a fragmented layout, or lose control over the linear/frag boundary needed to exercise the regression path. This test uses a 9000B packet so it does not depend on small-packet allocation details. Smaller packets might work depending on allocation state, but 9000B reliably gives us a non-linear skb with page frags and a stable linear/frag boundary for the copy-out regression. init_pkt() is needed to ensure deterministic byte content across both linear and fragmented regions so that the memcmp-based validation is stable. Thanks, Sun Jian > > > > 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
