Re: [PATCH bpf-next v6 3/5] selftests/bpf: Add ksock kfunc test

On Wed, Aug 12, 2026 at 05:25:42PM +0000, [email protected] wrote:
> > Add a selftest that exercises the ksock kfuncs end-to-end. One syscall
> > bpf setup program creates a ksock context and connects the socket.
> > Another LSM sleepable bpf program looks up the context and send test
> > data. The userspace harness creates a network namespace and a new socket
> > on loopback, run the setup and send syscall bpf progs then check that
> > the userspace socket received the data from bpf.
> 
> The description says "run the setup and send syscall bpf progs" but the
> test adds only one SEC("syscall") program, ksock_setup:
> 
> tools/testing/selftests/bpf/progs/ksock_lsm.c:
>     SEC("syscall")
>     int ksock_setup(void *ctx)
>     { ... }
> 
> which the harness runs once:
> 
>     err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.ksock_setup), &opts);
> 
> The send doesn't happen in a syscall program at all. It happens in the
> sleepable LSM program ksock_socket_bind (SEC("lsm.s/socket_bind")), which
> the harness triggers indirectly:
> 
>     skel->links.ksock_socket_bind = bpf_program__attach_lsm(skel->progs.ksock_socket_bind);
>     ...
>     skel->bss->target_pid = getpid();
>     err = bind(tfd, (struct sockaddr *)&trigger_addr, sizeof(trigger_addr));
> 
> This also contradicts the preceding sentence which correctly states the
> sender is "Another LSM sleepable bpf program". Should this say "run the
> setup syscall bpf prog and trigger the LSM send hook" or similar?
> 
> Also in the same sentences: "looks up the context and send test data"
> (sends) and "The userspace harness ... run the setup" (runs).

Yeah this is from a earlier version of this, will fix it.

> 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/ksock.c b/tools/testing/selftests/bpf/prog_tests/ksock.c
> > new file mode 100644
> > index 0000000000000..30e89d34d388f
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/ksock.c
> > @@ -0,0 +1,127 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2026 Isovalent */
> > +
> > +#include <arpa/inet.h>
> > +
> > +#include "test_progs.h"
> > +#include "network_helpers.h"
> > +#include "ksock_lsm.skel.h"
> > +
> > +#define NS_TEST "ksock_lsm_ns"
> > +#define RECV_PORT 7777
> > +#define RECV_TIMEOUT_SEC 5
> > +
> > +struct ksock_test_env {
> > +	bool netns_created;
> > +	struct nstoken *nstoken;
> > +	int rfd;
> > +};
> > +
> > +static bool ksock_test_env_setup(struct ksock_test_env *env)
> > +{
> 
> [ ... ]
> 
> > +	memset(env, 0, sizeof(*env));
> > +	env->rfd = -1;
> > +
> > +	SYS(fail, "ip netns add %s", NS_TEST);
> > +	env->netns_created = true;
> > +	SYS(fail, "ip -net %s link set lo up", NS_TEST);
> 
> This isn't a bug, but the test hand-rolls network namespace setup that
> network_helpers.h (already included at the top) provides verbatim. Could
> ksock_test_env_setup() use make_netns() here?
> 
> make_netns() in tools/testing/selftests/bpf/network_helpers.c runs exactly
> those two commands:
> 
>     r = asprintf(&cmd, "ip netns add %s", name);
>     ...
>     r = asprintf(&cmd, "ip -n %s link set lo up", name);
> 
> and it also unwinds the namespace if bringing lo up fails, which the
> open-coded version does not do. The current path leaves the netns to be
> cleaned up by the caller's fail: label instead.
> 
> The teardown at the end of test_ksock_lsm():
> 
>     SYS_NOFAIL("ip netns del %s >/dev/null 2>&1", NS_TEST);
> 
> is also what remove_netns() does, and SYS_NOFAIL() already appends
> " >/dev/null 2>&1" (test_progs.h), so the explicit redirection is applied
> twice and the command ends up as
> "ip netns del ksock_lsm_ns >/dev/null 2>&1 >/dev/null 2>&1".
> 
> Using make_netns()/remove_netns() would remove the duplicated redirection,
> drop the env->netns_created bookkeeping field, and inherit the shared
> helper's teardown-on-partial-failure behaviour.

Good point.

> 
> > +
> > +	env->nstoken = open_netns(NS_TEST);
> 
> [ ... ]
> 
> 
> ---
> 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/31618864886