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

On Thu, Aug 13, 2026 at 12:13:34PM +0000, [email protected] wrote:
> > 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..05d7b7424aee3
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/ksock.c
> > @@ -0,0 +1,124 @@
> > +// 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
> 
> [ ... ]
> 
> > +static bool ksock_test_env_setup(struct ksock_test_env *env)
> > +{
> > +	struct sockaddr_in addr = {
> > +		.sin_family = AF_INET,
> > +		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
> > +		.sin_port = htons(RECV_PORT),
> > +	};
> > +	struct timeval tv = { .tv_sec = RECV_TIMEOUT_SEC };
> > +	int err;
> > +
> > +	memset(env, 0, sizeof(*env));
> > +	env->rfd = -1;
> > +
> > +	if (!ASSERT_OK(make_netns(NS_TEST), "make_netns"))
> > +		goto fail;
> > +
> > +	env->nstoken = open_netns(NS_TEST);
> > +	if (!ASSERT_OK_PTR(env->nstoken, "open_netns"))
> > +		goto fail;
> > +
> > +	env->rfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
> > +	if (!ASSERT_OK_FD(env->rfd, "receiver socket"))
> > +		goto fail;
> > +
> > +	err = bind(env->rfd, (struct sockaddr *)&addr, sizeof(addr));
> > +	if (!ASSERT_OK(err, "bind receiver"))
> > +		goto fail;
> > +
> > +	err = setsockopt(env->rfd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
> > +	if (!ASSERT_OK(err, "set rcvtimeo"))
> > +		goto fail;
> 
> A subsystem pattern flags this socket setup sequence as potentially
> worth simplifying. The network_helpers.h header already provides
> start_server(family, type, addr, port, timeout_ms) which performs
> socket() + bind() + settimeo() for SOCK_DGRAM.
> 
> Other SOCK_DGRAM tests in prog_tests/ use start_server() for this:
> cg_storage_multi.c, connect_force_port.c, cgroup_getset_retval.c,
> sock_destroy.c, and tc_redirect.c. Could this be written as:
> 
>     env->rfd = start_server(AF_INET, SOCK_DGRAM, "127.0.0.1",
>                             RECV_PORT, RECV_TIMEOUT_SEC * 1000);
> 
> to drop the addr and tv locals and use the shared error logging?
> 
> That said, there is also precedent for the hand-rolled form in the same
> directory (cgroup_storage.c, udp_limit.c, xdp_metadata.c, and others),
> so this may just be a style choice.

Yeah unsure it's worth simplifying.

> 
> [ ... ]
> 
> 
> ---
> 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/31694719067