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

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

[ ... ]


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