Re: [PATCH bpf-next v1 1/4] selftests/bpf: Migrate *_POST_BIND test cases to prog_tests

[Martin KaFai Lau <martin.lau@linux.dev>]

On 10/17/24 3:49 PM, Jordan Rife wrote:
> Move all BPF_CGROUP_INET6_POST_BIND and BPF_CGROUP_INET4_POST_BIND test
> cases to a new prog_test, prog_tests/sock_post_bind.c, except for
> LOAD_REJECT test cases.
> 
> Signed-off-by: Jordan Rife <jrife@google.com>
> ---
>   .../selftests/bpf/prog_tests/sock_post_bind.c | 417 ++++++++++++++++++
>   tools/testing/selftests/bpf/test_sock.c       | 245 ----------
>   2 files changed, 417 insertions(+), 245 deletions(-)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/sock_post_bind.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/sock_post_bind.c b/tools/testing/selftests/bpf/prog_tests/sock_post_bind.c
> new file mode 100644
> index 000000000000..c46537e3b9d4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/sock_post_bind.c
> @@ -0,0 +1,417 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +#include <test_progs.h>
> +#include "cgroup_helpers.h"
> +
> +static char bpf_log_buf[4096];
> +static bool verbose;

How is verbose used and is it still needed?

[ ... ]

> +	if (bind(sockfd, (const struct sockaddr *)&addr, len) == -1) {
> +		/* sys_bind() may fail for different reasons, errno has to be
> +		 * checked to confirm that BPF program rejected it.
> +		 */
> +		if (errno != EPERM)
> +			goto err;
> +		if (port_retry)
> +			goto retry;
> +		res = BIND_REJECT;
> +		goto out;
> +	}

[ ... ]

> +void test_sock_post_bind(void)
> +{
> +	int cgroup_fd, i;
> +
> +	cgroup_fd = test__join_cgroup("/post_bind");
> +	if (!ASSERT_GE(cgroup_fd, 0, "join_cgroup"))

nit. ASSERT_OK_FD().

Since the test binds to a specific ip/port, please run it in its own netns. It 
is easy to do with netns_new and netns_free, a recent example:

https://lore.kernel.org/bpf/20241020-syncookie-v2-1-2db240225fed@bootlin.com/

The same netns can be reused for different subtests of this "sock_post_bind" test.

Others look good. Thanks for moving the test to test_progs.

pw-bot: cr