Re: [PATCH bpf-next v6 1/9] selftests/bpf: Add backlog for network_helper_opts

[Martin KaFai Lau]

On 6/28/24 6:20 PM, Geliang Tang wrote:
From: Geliang Tang <tanggeli...@kylinos.cn>
Some callers expect __start_server() helper to pass their own "backlog"
value to listen() instead of the default of 1. So this patch adds struct
member "backlog" for network_helper_opts to allow callers to set "backlog"
value via start_server_str() helper.

Signed-off-by: Geliang Tang <tanggeli...@kylinos.cn>
---
  tools/testing/selftests/bpf/network_helpers.c | 2 +-
  tools/testing/selftests/bpf/network_helpers.h | 1 +
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/network_helpers.c 
b/tools/testing/selftests/bpf/network_helpers.c
index 44c2c8fa542a..16cbb3fdcabf 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -106,7 +106,7 @@ static int __start_server(int type, const struct sockaddr 
*addr, socklen_t addrl
        }
  
  	if (type == SOCK_STREAM) {
-               if (listen(fd, 1) < 0) {
+               if (listen(fd, opts->backlog ? : 1) < 0) {


iirc, listen(fd, 0 /* backlog */) can be used to enforce syncookie. Meaning 
backlog 0 is a legit value.

Using 0 as a default and changing it to 1 here is fine. It makes the test 
program easier to write for the common case. Enforcing syncookie mode by using 
backlog 0 is a niche use case but it should at least have a way for the caller 
to do that.

May be using -ve backlog to do that, like

        if (listen(fd, opts->backlog ? max(opts->backlog, 0): 1) < 0)


                        log_err("Failed to listed on socket");
                        goto error_close;
                }
diff --git a/tools/testing/selftests/bpf/network_helpers.h 
b/tools/testing/selftests/bpf/network_helpers.h
index 9ea36524b9db..8339c4e4b075 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -25,6 +25,7 @@ struct network_helper_opts {
        int timeout_ms;
        bool must_fail;
        int proto;
+       int backlog;

and add a comment here to explain the meaning of the -ve backlog.

        int (*post_socket_cb)(int fd, void *opts);
        void *cb_opts;
  };