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

On Wed, Aug 12, 2026 at 05:25:42PM +0000, bot+bpf-ci@kernel.org wrote:

quoted

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.

quoted

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)
+{

[ ... ]

quoted

+	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.

quoted

+
+	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