[PATCH bpf-next 0/6] selftests/bpf: integrate test_tcp_check_syncookie.sh into test_progs

List overview All Threads
Download

newer

older

[PATCH v5 0/5] Add PSCI v1.3...

[PATCH net-next v9 00/23]...

Alexis Lothoré (eBPF Foundation)

16 Oct 2024 16 Oct '24

6:35 p.m.

Hello, this series aims to bring test_tcp_check_syncookie.sh scope into test_progs to make sure that the corresponding tests are also run automatically in CI. This script tests for bpf_tcp_{gen,check}_syncookie and bpf_skc_lookup_tcp, in different contexts (ipv4, v6 or dual, and with tc and xdp programs). Some other tests like btf_skc_cls_ingress have some overlapping tests with test_tcp_check_syncookie.sh, so this series moves the missing bits from test_tcp_check_syncookie.sh into btf_skc_cls_ingress, which is already integrated into test_progs. - the first three commits bring some minor improvements to btf_skc_cls_ingress without changing its testing scope - fourth and fifth commits bring test_tcp_check_syncookie.sh features into btf_skc_cls_ingress - last commit removes test_tcp_check_syncookie.sh

The only topic for which I am not sure for this integration is the necessity or not to run the tests with different program types: test_tcp_check_syncookie.sh runs tests with both tc and xdp programs, but btf_skc_cls_ingress currently tests those helpers only with a tc program. Would it make sense to also make sure that btf_skc_cls_ingress is tested with all the programs types supported by those helpers ?

The series has been tested both in CI and in a local x86_64 qemu environment: # ./test_progs -a btf_skc_cls_ingress #38/1 btf_skc_cls_ingress/conn_ipv4:OK #38/2 btf_skc_cls_ingress/conn_ipv6:OK #38/3 btf_skc_cls_ingress/conn_dual:OK #38/4 btf_skc_cls_ingress/syncookie_ipv4:OK #38/5 btf_skc_cls_ingress/syncookie_ipv6:OK #38/6 btf_skc_cls_ingress/syncookie_dual:OK #38 btf_skc_cls_ingress:OK Summary: 1/6 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- Alexis Lothoré (eBPF Foundation) (6): selftests/bpf: factorize conn and syncookies tests in a single runner selftests/bpf: add missing ns cleanups in btf_skc_cls_ingress selftests/bpf: get rid of global vars in btf_skc_cls_ingress selftests/bpf: add ipv4 and dual ipv4/ipv6 support in btf_skc_cls_ingress selftests/bpf: test MSS value returned with bpf_tcp_gen_syncookie selftests/bpf: remove test_tcp_check_syncookie

tools/testing/selftests/bpf/.gitignore | 1 - tools/testing/selftests/bpf/Makefile | 9 +- .../selftests/bpf/prog_tests/btf_skc_cls_ingress.c | 265 +++++++++++++-------- .../selftests/bpf/progs/test_btf_skc_cls_ingress.c | 83 +++++-- .../bpf/progs/test_tcp_check_syncookie_kern.c | 167 ------------- .../selftests/bpf/test_tcp_check_syncookie.sh | 85 ------- .../selftests/bpf/test_tcp_check_syncookie_user.c | 213 ----------------- 7 files changed, 222 insertions(+), 601 deletions(-) --- base-commit: 030207b7fce8bad6827615cfc2c6592916e2c336 change-id: 20241015-syncookie-ea7686264586

Best regards,

-- Alexis Lothoré, Bootlin Embedded Linux and Kernel engineering https://bootlin.com

Show replies by date

Alexis Lothoré (eBPF Foundation)

16 Oct 16 Oct

6:35 p.m.

New subject: [PATCH bpf-next 1/6] selftests/bpf: factorize conn and syncookies tests in a single runner

btf_skc_cls_ingress currently describe two tests, both running a simple tcp server and then initializing a connection to it. The sole difference between the tests is about the tcp_syncookie configuration, and some checks around this feature being enabled/disabled.

Share the common code between those two tests by moving the code into a single runner, parameterized by a "gen_cookies" argument. Split the performed checks accordingly.

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- .../selftests/bpf/prog_tests/btf_skc_cls_ingress.c | 106 +++++++-------------- 1 file changed, 37 insertions(+), 69 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c index ef4d6a3ae4231b250f43f8f93eeca6499abeaadc..5d8d7736edc095b647ca3fbc12cac0440b60140e 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c @@ -71,13 +71,14 @@ static void print_err_line(void) printf("bpf prog error at line %u\n", skel->bss->linum); }

-static void test_conn(void) +static void run_test(bool gen_cookies) { + const char *tcp_syncookies = gen_cookies ? "2" : "1"; int listen_fd = -1, cli_fd = -1, srv_fd = -1, err; socklen_t addrlen = sizeof(srv_sa6); int srv_port;

- if (write_sysctl("/proc/sys/net/ipv4/tcp_syncookies", "1")) + if (write_sysctl("/proc/sys/net/ipv4/tcp_syncookies", tcp_syncookies)) return;

listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0); @@ -99,19 +100,36 @@ static void test_conn(void) if (CHECK_FAIL(srv_fd == -1)) goto done;

- if (CHECK(skel->bss->listen_tp_sport != srv_port || - skel->bss->req_sk_sport != srv_port, - "Unexpected sk src port", - "listen_tp_sport:%u req_sk_sport:%u expected:%u\n", - skel->bss->listen_tp_sport, skel->bss->req_sk_sport, - srv_port)) + if (CHECK(skel->bss->listen_tp_sport != srv_port, + "Unexpected listen tp src port", + "listen_tp_sport:%u expected:%u\n", + skel->bss->listen_tp_sport, srv_port)) goto done;

- if (CHECK(skel->bss->gen_cookie || skel->bss->recv_cookie, - "Unexpected syncookie states", - "gen_cookie:%u recv_cookie:%u\n", - skel->bss->gen_cookie, skel->bss->recv_cookie)) - goto done; + if (!gen_cookies) { + if (CHECK(skel->bss->req_sk_sport != srv_port, + "Unexpected req_sk src port", + "req_sk_sport:%u expected:%u\n", + skel->bss->req_sk_sport, srv_port)) + goto done; + if (CHECK(skel->bss->gen_cookie || skel->bss->recv_cookie, + "Unexpected syncookie states", + "gen_cookie:%u recv_cookie:%u\n", + skel->bss->gen_cookie, skel->bss->recv_cookie)) + goto done; + } else { + if (CHECK(skel->bss->req_sk_sport, + "Unexpected req_sk src port", + "req_sk_sport:%u expected:0\n", + skel->bss->req_sk_sport)) + goto done; + if (CHECK(!skel->bss->gen_cookie || + skel->bss->gen_cookie != skel->bss->recv_cookie, + "Unexpected syncookie states", + "gen_cookie:%u recv_cookie:%u\n", + skel->bss->gen_cookie, skel->bss->recv_cookie)) + goto done; + }

CHECK(skel->bss->linum, "bpf prog detected error", "at line %u\n", skel->bss->linum); @@ -125,64 +143,14 @@ static void test_conn(void) close(srv_fd); }

-static void test_syncookie(void) +static void test_conn(void) { - int listen_fd = -1, cli_fd = -1, srv_fd = -1, err; - socklen_t addrlen = sizeof(srv_sa6); - int srv_port; - - /* Enforce syncookie mode */ - if (write_sysctl("/proc/sys/net/ipv4/tcp_syncookies", "2")) - return; - - listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0); - if (CHECK_FAIL(listen_fd == -1)) - return; - - err = getsockname(listen_fd, (struct sockaddr *)&srv_sa6, &addrlen); - if (CHECK(err, "getsockname(listen_fd)", "err:%d errno:%d\n", err, - errno)) - goto done; - memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6)); - srv_port = ntohs(srv_sa6.sin6_port); - - cli_fd = connect_to_fd(listen_fd, 0); - if (CHECK_FAIL(cli_fd == -1)) - goto done; - - srv_fd = accept(listen_fd, NULL, NULL); - if (CHECK_FAIL(srv_fd == -1)) - goto done; - - if (CHECK(skel->bss->listen_tp_sport != srv_port, - "Unexpected tp src port", - "listen_tp_sport:%u expected:%u\n", - skel->bss->listen_tp_sport, srv_port)) - goto done; - - if (CHECK(skel->bss->req_sk_sport, - "Unexpected req_sk src port", - "req_sk_sport:%u expected:0\n", - skel->bss->req_sk_sport)) - goto done; - - if (CHECK(!skel->bss->gen_cookie || - skel->bss->gen_cookie != skel->bss->recv_cookie, - "Unexpected syncookie states", - "gen_cookie:%u recv_cookie:%u\n", - skel->bss->gen_cookie, skel->bss->recv_cookie)) - goto done; - - CHECK(skel->bss->linum, "bpf prog detected error", "at line %u\n", - skel->bss->linum); + run_test(false); +}

-done: - if (listen_fd != -1) - close(listen_fd); - if (cli_fd != -1) - close(cli_fd); - if (srv_fd != -1) - close(srv_fd); +static void test_syncookie(void) +{ + run_test(true); }

struct test {

-- 2.46.2

Alexis Lothoré (eBPF Foundation)

6:35 p.m.

New subject: [PATCH bpf-next 2/6] selftests/bpf: add missing ns cleanups in btf_skc_cls_ingress

btf_skc_cls_ingress.c currently runs two subtests, and create a dedicated network namespace for each, but never cleans up the created namespace once the test has ended.

Add missing namespace cleanup after each namespace to avoid accumulating namespaces for each new subtest. While at it, switch namespace management to netns_{new,free}

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- .../selftests/bpf/prog_tests/btf_skc_cls_ingress.c | 31 ++++++++++++++-------- 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c index 5d8d7736edc095b647ca3fbc12cac0440b60140e..8d1fa8806cdda088d264b44104f7c80726b025e2 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c @@ -17,32 +17,34 @@ #include "test_progs.h" #include "test_btf_skc_cls_ingress.skel.h"

+#define TEST_NS "skc_cls_ingress" + static struct test_btf_skc_cls_ingress *skel; static struct sockaddr_in6 srv_sa6; static __u32 duration;

-static int prepare_netns(void) +static struct netns_obj *prepare_netns(void) { LIBBPF_OPTS(bpf_tc_hook, qdisc_lo, .attach_point = BPF_TC_INGRESS); LIBBPF_OPTS(bpf_tc_opts, tc_attach, .prog_fd = bpf_program__fd(skel->progs.cls_ingress)); + struct netns_obj *ns = NULL;

- if (CHECK(unshare(CLONE_NEWNET), "create netns", - "unshare(CLONE_NEWNET): %s (%d)", - strerror(errno), errno)) - return -1; + ns = netns_new(TEST_NS, true); + if (!ASSERT_OK_PTR(ns, "create and join netns")) + return ns;

if (CHECK(system("ip link set dev lo up"), "ip link set dev lo up", "failed\n")) - return -1; + goto free_ns;

qdisc_lo.ifindex = if_nametoindex("lo"); if (!ASSERT_OK(bpf_tc_hook_create(&qdisc_lo), "qdisc add dev lo clsact")) - return -1; + goto free_ns;

if (!ASSERT_OK(bpf_tc_attach(&qdisc_lo, &tc_attach), "filter add dev lo ingress")) - return -1; + goto free_ns;

/* Ensure 20 bytes options (i.e. in total 40 bytes tcp header) for the * bpf_tcp_gen_syncookie() helper. @@ -50,9 +52,13 @@ static int prepare_netns(void) if (write_sysctl("/proc/sys/net/ipv4/tcp_window_scaling", "1") || write_sysctl("/proc/sys/net/ipv4/tcp_timestamps", "1") || write_sysctl("/proc/sys/net/ipv4/tcp_sack", "1")) - return -1; + goto free_ns; + + return ns;

- return 0; +free_ns: + netns_free(ns); + return NULL; }

static void reset_test(void) @@ -169,6 +175,7 @@ void test_btf_skc_cls_ingress(void) int i;

skel = test_btf_skc_cls_ingress__open_and_load(); + struct netns_obj *ns; if (CHECK(!skel, "test_btf_skc_cls_ingress__open_and_load", "failed\n")) return;

@@ -176,13 +183,15 @@ void test_btf_skc_cls_ingress(void) if (!test__start_subtest(tests[i].desc)) continue;

- if (prepare_netns()) + ns = prepare_netns(); + if (!ns) break;

tests[i].run();

print_err_line(); reset_test(); + netns_free(ns); }

test_btf_skc_cls_ingress__destroy(skel);

-- 2.46.2

Martin KaFai Lau

18 Oct 18 Oct

11:57 p.m.

New subject: [PATCH bpf-next 2/6] selftests/bpf: add missing ns cleanups in btf_skc_cls_ingress

On 10/16/24 11:35 AM, Alexis Lothoré (eBPF Foundation) wrote:

...

btf_skc_cls_ingress.c currently runs two subtests, and create a dedicated network namespace for each, but never cleans up the created namespace once the test has ended.

Add missing namespace cleanup after each namespace to avoid accumulating namespaces for each new subtest. While at it, switch namespace management to netns_{new,free}

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com

.../selftests/bpf/prog_tests/btf_skc_cls_ingress.c | 31 ++++++++++++++-------- 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c index 5d8d7736edc095b647ca3fbc12cac0440b60140e..8d1fa8806cdda088d264b44104f7c80726b025e2 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c @@ -17,32 +17,34 @@ #include "test_progs.h" #include "test_btf_skc_cls_ingress.skel.h" +#define TEST_NS "skc_cls_ingress"

static struct test_btf_skc_cls_ingress *skel; static struct sockaddr_in6 srv_sa6; static __u32 duration;

-static int prepare_netns(void) +static struct netns_obj *prepare_netns(void) { LIBBPF_OPTS(bpf_tc_hook, qdisc_lo, .attach_point = BPF_TC_INGRESS); LIBBPF_OPTS(bpf_tc_opts, tc_attach, .prog_fd = bpf_program__fd(skel->progs.cls_ingress));

struct netns_obj *ns = NULL;
if (CHECK(unshare(CLONE_NEWNET), "create netns",
  "unshare(CLONE_NEWNET): %s (%d)",
  strerror(errno), errno))
return -1;
ns = netns_new(TEST_NS, true);

if (!ASSERT_OK_PTR(ns, "create and join netns"))
return ns;
if (CHECK(system("ip link set dev lo up"), "ip link set dev lo up", "failed\n"))

nit. netns_new() takes care of "lo up" also, so the above can be removed.

test_progs.c has restore_netns() after each test, so the netns was not cleaned up. The second unshare should have freed the earlier netns also.

Using netns_new() removed the boiler plate codes. It is nice to see this change here regardless.

...

return -1;
goto free_ns;
qdisc_lo.ifindex = if_nametoindex("lo"); if (!ASSERT_OK(bpf_tc_hook_create(&qdisc_lo), "qdisc add dev lo clsact"))
return -1;
goto free_ns;
if (!ASSERT_OK(bpf_tc_attach(&qdisc_lo, &tc_attach), "filter add dev lo ingress"))
return -1;
goto free_ns;
/* Ensure 20 bytes options (i.e. in total 40 bytes tcp header) for the * bpf_tcp_gen_syncookie() helper. @@ -50,9 +52,13 @@ static int prepare_netns(void) if (write_sysctl("/proc/sys/net/ipv4/tcp_window_scaling", "1") || write_sysctl("/proc/sys/net/ipv4/tcp_timestamps", "1") || write_sysctl("/proc/sys/net/ipv4/tcp_sack", "1"))
return -1;
goto free_ns;
return ns;
return 0;

+free_ns:

netns_free(ns);

return NULL; }

Alexis Lothoré

19 Oct 19 Oct

12:13 p.m.

New subject: [PATCH bpf-next 2/6] selftests/bpf: add missing ns cleanups in btf_skc_cls_ingress

Hi Martin, thanks for the review !

On 10/19/24 01:57, Martin KaFai Lau wrote:

...

On 10/16/24 11:35 AM, Alexis Lothoré (eBPF Foundation) wrote:

...
btf_skc_cls_ingress.c currently runs two subtests, and create a dedicated network namespace for each, but never cleans up the created namespace once the test has ended.

Add missing namespace cleanup after each namespace to avoid accumulating namespaces for each new subtest. While at it, switch namespace management to netns_{new,free}

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com

[...]

...

...
- if (CHECK(unshare(CLONE_NEWNET), "create netns", - "unshare(CLONE_NEWNET): %s (%d)", - strerror(errno), errno)) - return -1; + ns = netns_new(TEST_NS, true); + if (!ASSERT_OK_PTR(ns, "create and join netns")) + return ns; if (CHECK(system("ip link set dev lo up"), "ip link set dev lo up", "failed\n"))

nit. netns_new() takes care of "lo up" also, so the above can be removed.

Ah, indeed, I missed it in make_netns. Thanks, I'll remove this part from the test then.

...

test_progs.c has restore_netns() after each test, so the netns was not cleaned up. The second unshare should have freed the earlier netns also.

Using netns_new() removed the boiler plate codes. It is nice to see this change here regardless.

-- Alexis Lothoré, Bootlin Embedded Linux and Kernel engineering https://bootlin.com

Alexis Lothoré (eBPF Foundation)

16 Oct 16 Oct

6:35 p.m.

New subject: [PATCH bpf-next 3/6] selftests/bpf: get rid of global vars in btf_skc_cls_ingress

There are a few global variables in btf_skc_cls_ingress.c, which are not really used by different tests. Get rid of those global variables, by performing the following updates: - make srv_sa6 local to the main runner function - make skel local to the main function, and propagate it through function arguments - get rid of duration by replacing CHECK macros with the ASSERT_XXX macros. While updating those assert macros: - do not return early on asserts performing some actual tests, let the other tests run as well (keep the early return for parts handling test setup) - instead of converting the CHECK on skel->bss->linum, just remove it, since there is already a call to print_err_line after the test to print the failing line in the bpf program

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- .../selftests/bpf/prog_tests/btf_skc_cls_ingress.c | 91 +++++++++------------- 1 file changed, 35 insertions(+), 56 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c index 8d1fa8806cdda088d264b44104f7c80726b025e2..a20d104f9909e5ba20ddc4c107b910956f042fc1 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c @@ -19,11 +19,7 @@

#define TEST_NS "skc_cls_ingress"

-static struct test_btf_skc_cls_ingress *skel; -static struct sockaddr_in6 srv_sa6; -static __u32 duration; - -static struct netns_obj *prepare_netns(void) +static struct netns_obj *prepare_netns(struct test_btf_skc_cls_ingress *skel) { LIBBPF_OPTS(bpf_tc_hook, qdisc_lo, .attach_point = BPF_TC_INGRESS); LIBBPF_OPTS(bpf_tc_opts, tc_attach, @@ -34,9 +30,7 @@ static struct netns_obj *prepare_netns(void) if (!ASSERT_OK_PTR(ns, "create and join netns")) return ns;

- if (CHECK(system("ip link set dev lo up"), - "ip link set dev lo up", "failed\n")) - goto free_ns; + SYS(free_ns, "ip link set dev lo up");

qdisc_lo.ifindex = if_nametoindex("lo"); if (!ASSERT_OK(bpf_tc_hook_create(&qdisc_lo), "qdisc add dev lo clsact")) @@ -61,7 +55,7 @@ static struct netns_obj *prepare_netns(void) return NULL; }

-static void reset_test(void) +static void reset_test(struct test_btf_skc_cls_ingress *skel) { memset(&skel->bss->srv_sa6, 0, sizeof(skel->bss->srv_sa6)); skel->bss->listen_tp_sport = 0; @@ -71,16 +65,17 @@ static void reset_test(void) skel->bss->linum = 0; }

-static void print_err_line(void) +static void print_err_line(struct test_btf_skc_cls_ingress *skel) { if (skel->bss->linum) printf("bpf prog error at line %u\n", skel->bss->linum); }

-static void run_test(bool gen_cookies) +static void run_test(struct test_btf_skc_cls_ingress *skel, bool gen_cookies) { const char *tcp_syncookies = gen_cookies ? "2" : "1"; int listen_fd = -1, cli_fd = -1, srv_fd = -1, err; + struct sockaddr_in6 srv_sa6; socklen_t addrlen = sizeof(srv_sa6); int srv_port;

@@ -88,58 +83,41 @@ static void run_test(bool gen_cookies) return;

listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0); - if (CHECK_FAIL(listen_fd == -1)) + if (!ASSERT_OK_FD(listen_fd, "start server")) return;

err = getsockname(listen_fd, (struct sockaddr *)&srv_sa6, &addrlen); - if (CHECK(err, "getsockname(listen_fd)", "err:%d errno:%d\n", err, - errno)) + if (!ASSERT_OK(err, "getsockname(listen_fd)")) goto done; memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6)); srv_port = ntohs(srv_sa6.sin6_port);

cli_fd = connect_to_fd(listen_fd, 0); - if (CHECK_FAIL(cli_fd == -1)) + if (!ASSERT_OK_FD(cli_fd, "connect client")) goto done;

srv_fd = accept(listen_fd, NULL, NULL); - if (CHECK_FAIL(srv_fd == -1)) + if (!ASSERT_OK_FD(srv_fd, "accept connection")) goto done;

- if (CHECK(skel->bss->listen_tp_sport != srv_port, - "Unexpected listen tp src port", - "listen_tp_sport:%u expected:%u\n", - skel->bss->listen_tp_sport, srv_port)) - goto done; + ASSERT_EQ(skel->bss->listen_tp_sport, srv_port, "listen tp src port");

if (!gen_cookies) { - if (CHECK(skel->bss->req_sk_sport != srv_port, - "Unexpected req_sk src port", - "req_sk_sport:%u expected:%u\n", - skel->bss->req_sk_sport, srv_port)) - goto done; - if (CHECK(skel->bss->gen_cookie || skel->bss->recv_cookie, - "Unexpected syncookie states", - "gen_cookie:%u recv_cookie:%u\n", - skel->bss->gen_cookie, skel->bss->recv_cookie)) - goto done; + ASSERT_EQ(skel->bss->req_sk_sport, srv_port, + "request socket source port with syncookies disabled"); + ASSERT_EQ(skel->bss->gen_cookie, 0, + "generated syncookie with syncookies disabled"); + ASSERT_EQ(skel->bss->recv_cookie, 0, + "received syncookie with syncookies disabled"); } else { - if (CHECK(skel->bss->req_sk_sport, - "Unexpected req_sk src port", - "req_sk_sport:%u expected:0\n", - skel->bss->req_sk_sport)) - goto done; - if (CHECK(!skel->bss->gen_cookie || - skel->bss->gen_cookie != skel->bss->recv_cookie, - "Unexpected syncookie states", - "gen_cookie:%u recv_cookie:%u\n", - skel->bss->gen_cookie, skel->bss->recv_cookie)) - goto done; + ASSERT_EQ(skel->bss->req_sk_sport, 0, + "request socket source port with syncookies enabled"); + ASSERT_NEQ(skel->bss->gen_cookie, 0, + "syncookie properly generated"); + ASSERT_EQ(skel->bss->gen_cookie, skel->bss->recv_cookie, + "matching syncookies on client and server"); }

- CHECK(skel->bss->linum, "bpf prog detected error", "at line %u\n", - skel->bss->linum); - done: if (listen_fd != -1) close(listen_fd); @@ -149,19 +127,19 @@ static void run_test(bool gen_cookies) close(srv_fd); }

-static void test_conn(void) +static void test_conn(struct test_btf_skc_cls_ingress *skel) { - run_test(false); + run_test(skel, false); }

-static void test_syncookie(void) +static void test_syncookie(struct test_btf_skc_cls_ingress *skel) { - run_test(true); + run_test(skel, true); }

struct test { const char *desc; - void (*run)(void); + void (*run)(struct test_btf_skc_cls_ingress *skel); };

#define DEF_TEST(name) { #name, test_##name } @@ -172,25 +150,26 @@ static struct test tests[] = {

void test_btf_skc_cls_ingress(void) { + struct test_btf_skc_cls_ingress *skel; + struct netns_obj *ns; int i;

skel = test_btf_skc_cls_ingress__open_and_load(); - struct netns_obj *ns; - if (CHECK(!skel, "test_btf_skc_cls_ingress__open_and_load", "failed\n")) + if (!ASSERT_OK_PTR(skel, "test_btf_skc_cls_ingress__open_and_load")) return;

for (i = 0; i < ARRAY_SIZE(tests); i++) { if (!test__start_subtest(tests[i].desc)) continue;

- ns = prepare_netns(); + ns = prepare_netns(skel); if (!ns) break;

- tests[i].run(); + tests[i].run(skel);

- print_err_line(); - reset_test(); + print_err_line(skel); + reset_test(skel); netns_free(ns); }

-- 2.46.2

Alexis Lothoré (eBPF Foundation)

6:35 p.m.

New subject: [PATCH bpf-next 4/6] selftests/bpf: add ipv4 and dual ipv4/ipv6 support in btf_skc_cls_ingress

btf_skc_cls_ingress test currently checks that syncookie and bpf_sk_assign/release helpers behave correctly in multiple scenarios, but only with ipv4 socket.

Increase those helpers coverage by adding testing support for IPv6-only sockets and IPv4/IPv6 sockets. The rework is mostly based on features brought earlier in test_tcp_check_syncookie.sh to cover some fixes performed on those helpers, but test_tcp_check_syncookie.sh is not integrated in test_progs. The most notable changes linked to this are: - some rework in the corresponding eBPF program to support both types of traffic - the switch from start_server to start_server_str to allow to check some socket options - the introduction of new subtests for ipv4 and ipv4/ipv6

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- The rework has been tested in a local Qemu environment and in CI: # ./test_progs -a btf_skc_cls_ingress #38/1 btf_skc_cls_ingress/conn_ipv4:OK #38/2 btf_skc_cls_ingress/conn_ipv6:OK #38/3 btf_skc_cls_ingress/conn_dual:OK #38/4 btf_skc_cls_ingress/syncookie_ipv4:OK #38/5 btf_skc_cls_ingress/syncookie_ipv6:OK #38/6 btf_skc_cls_ingress/syncookie_dual:OK #38 btf_skc_cls_ingress:OK Summary: 1/6 PASSED, 0 SKIPPED, 0 FAILED --- .../selftests/bpf/prog_tests/btf_skc_cls_ingress.c | 116 ++++++++++++++++++--- .../selftests/bpf/progs/test_btf_skc_cls_ingress.c | 81 +++++++++----- 2 files changed, 161 insertions(+), 36 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c index a20d104f9909e5ba20ddc4c107b910956f042fc1..e0f8fe818f4230a1d5bf0118133c5a9fb50345e1 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c @@ -19,6 +19,15 @@

#define TEST_NS "skc_cls_ingress"

+#define BIT(n) (1 << (n)) +#define TEST_MODE_IPV4 BIT(0) +#define TEST_MODE_IPV6 BIT(1) +#define TEST_MODE_DUAL (TEST_MODE_IPV4 | TEST_MODE_IPV6) + +#define SERVER_ADDR_IPV4 "127.0.0.1" +#define SERVER_ADDR_IPV6 "::1" +#define SERVER_ADDR_DUAL "::0" + static struct netns_obj *prepare_netns(struct test_btf_skc_cls_ingress *skel) { LIBBPF_OPTS(bpf_tc_hook, qdisc_lo, .attach_point = BPF_TC_INGRESS); @@ -57,6 +66,7 @@ static struct netns_obj *prepare_netns(struct test_btf_skc_cls_ingress *skel)

static void reset_test(struct test_btf_skc_cls_ingress *skel) { + memset(&skel->bss->srv_sa4, 0, sizeof(skel->bss->srv_sa4)); memset(&skel->bss->srv_sa6, 0, sizeof(skel->bss->srv_sa6)); skel->bss->listen_tp_sport = 0; skel->bss->req_sk_sport = 0; @@ -71,26 +81,84 @@ static void print_err_line(struct test_btf_skc_cls_ingress *skel) printf("bpf prog error at line %u\n", skel->bss->linum); }

-static void run_test(struct test_btf_skc_cls_ingress *skel, bool gen_cookies) +static int v6only_true(int fd, void *opts) +{ + int mode = true; + + return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &mode, sizeof(mode)); +} + +static int v6only_false(int fd, void *opts) +{ + int mode = false; + + return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &mode, sizeof(mode)); +} + +static void run_test(struct test_btf_skc_cls_ingress *skel, bool gen_cookies, + int ip_mode) { const char *tcp_syncookies = gen_cookies ? "2" : "1"; int listen_fd = -1, cli_fd = -1, srv_fd = -1, err; + struct network_helper_opts opts = { 0 }; + struct sockaddr_storage *addr; struct sockaddr_in6 srv_sa6; - socklen_t addrlen = sizeof(srv_sa6); + struct sockaddr_in srv_sa4; + socklen_t addr_len; + int sock_family; + char *srv_addr; int srv_port;

+ switch (ip_mode) { + case TEST_MODE_IPV4: + sock_family = AF_INET; + srv_addr = SERVER_ADDR_IPV4; + addr = (struct sockaddr_storage *)&srv_sa4; + addr_len = sizeof(srv_sa4); + break; + case TEST_MODE_IPV6: + opts.post_socket_cb = v6only_true; + sock_family = AF_INET6; + srv_addr = SERVER_ADDR_IPV6; + addr = (struct sockaddr_storage *)&srv_sa6; + addr_len = sizeof(srv_sa6); + break; + case TEST_MODE_DUAL: + opts.post_socket_cb = v6only_false; + sock_family = AF_INET6; + srv_addr = SERVER_ADDR_DUAL; + addr = (struct sockaddr_storage *)&srv_sa6; + addr_len = sizeof(srv_sa6); + break; + default: + break; + } + if (write_sysctl("/proc/sys/net/ipv4/tcp_syncookies", tcp_syncookies)) return;

- listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0); + listen_fd = start_server_str(sock_family, SOCK_STREAM, srv_addr, 0, + &opts); if (!ASSERT_OK_FD(listen_fd, "start server")) return;

- err = getsockname(listen_fd, (struct sockaddr *)&srv_sa6, &addrlen); + err = getsockname(listen_fd, (struct sockaddr *)addr, &addr_len); if (!ASSERT_OK(err, "getsockname(listen_fd)")) goto done; - memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6)); - srv_port = ntohs(srv_sa6.sin6_port); + + switch (ip_mode) { + case TEST_MODE_IPV4: + memcpy(&skel->bss->srv_sa4, &srv_sa4, sizeof(srv_sa4)); + srv_port = ntohs(srv_sa4.sin_port); + break; + case TEST_MODE_IPV6: + case TEST_MODE_DUAL: + memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6)); + srv_port = ntohs(srv_sa6.sin6_port); + break; + default: + break; + }

cli_fd = connect_to_fd(listen_fd, 0); if (!ASSERT_OK_FD(cli_fd, "connect client")) @@ -127,14 +195,34 @@ static void run_test(struct test_btf_skc_cls_ingress *skel, bool gen_cookies) close(srv_fd); }

-static void test_conn(struct test_btf_skc_cls_ingress *skel) +static void test_conn_ipv4(struct test_btf_skc_cls_ingress *skel) +{ + run_test(skel, false, TEST_MODE_IPV4); +} + +static void test_conn_ipv6(struct test_btf_skc_cls_ingress *skel) +{ + run_test(skel, false, TEST_MODE_IPV6); +} + +static void test_conn_dual(struct test_btf_skc_cls_ingress *skel) +{ + run_test(skel, false, TEST_MODE_DUAL); +} + +static void test_syncookie_ipv4(struct test_btf_skc_cls_ingress *skel) +{ + run_test(skel, true, TEST_MODE_IPV4); +} + +static void test_syncookie_ipv6(struct test_btf_skc_cls_ingress *skel) { - run_test(skel, false); + run_test(skel, true, TEST_MODE_IPV6); }

-static void test_syncookie(struct test_btf_skc_cls_ingress *skel) +static void test_syncookie_dual(struct test_btf_skc_cls_ingress *skel) { - run_test(skel, true); + run_test(skel, true, TEST_MODE_DUAL); }

struct test { @@ -144,8 +232,12 @@ struct test {

#define DEF_TEST(name) { #name, test_##name } static struct test tests[] = { - DEF_TEST(conn), - DEF_TEST(syncookie), + DEF_TEST(conn_ipv4), + DEF_TEST(conn_ipv6), + DEF_TEST(conn_dual), + DEF_TEST(syncookie_ipv4), + DEF_TEST(syncookie_ipv6), + DEF_TEST(syncookie_dual), };

void test_btf_skc_cls_ingress(void) diff --git a/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c index f0759efff6ef15d2663927400dd064c53b020f78..cd528f8792ff2eb14683cbc13e8b0f3fd38329e9 100644 --- a/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c @@ -10,6 +10,7 @@ #endif

struct sockaddr_in6 srv_sa6 = {}; +struct sockaddr_in srv_sa4 = {}; __u16 listen_tp_sport = 0; __u16 req_sk_sport = 0; __u32 recv_cookie = 0; @@ -18,8 +19,8 @@ __u32 linum = 0;

#define LOG() ({ if (!linum) linum = __LINE__; })

-static void test_syncookie_helper(struct ipv6hdr *ip6h, struct tcphdr *th, - struct tcp_sock *tp, +static void test_syncookie_helper(void *iphdr, int iphdr_size, + struct tcphdr *th, struct tcp_sock *tp, struct __sk_buff *skb) { if (th->syn) { @@ -38,7 +39,7 @@ static void test_syncookie_helper(struct ipv6hdr *ip6h, struct tcphdr *th, return; }

- mss_cookie = bpf_tcp_gen_syncookie(tp, ip6h, sizeof(*ip6h), + mss_cookie = bpf_tcp_gen_syncookie(tp, iphdr, iphdr_size, th, 40); if (mss_cookie < 0) { if (mss_cookie != -ENOENT) @@ -48,7 +49,7 @@ static void test_syncookie_helper(struct ipv6hdr *ip6h, struct tcphdr *th, } } else if (gen_cookie) { /* It was in cookie mode */ - int ret = bpf_tcp_check_syncookie(tp, ip6h, sizeof(*ip6h), + int ret = bpf_tcp_check_syncookie(tp, iphdr, iphdr_size, th, sizeof(*th));

if (ret < 0) { @@ -60,26 +61,63 @@ static void test_syncookie_helper(struct ipv6hdr *ip6h, struct tcphdr *th, } }

-static int handle_ip6_tcp(struct ipv6hdr *ip6h, struct __sk_buff *skb) +static int handle_ip_tcp(struct ethhdr *eth, struct __sk_buff *skb) { - struct bpf_sock_tuple *tuple; + struct bpf_sock_tuple *tuple = NULL; + unsigned int tuple_len = 0; struct bpf_sock *bpf_skc; - unsigned int tuple_len; + struct ipv6hdr *ip6h; + void *iphdr = NULL; + int iphdr_size = 0; + struct iphdr *ip4h; struct tcphdr *th; void *data_end;

data_end = (void *)(long)(skb->data_end);

- th = (struct tcphdr *)(ip6h + 1); - if (th + 1 > data_end) - return TC_ACT_OK; + switch (eth->h_proto) { + case bpf_htons(ETH_P_IP): + ip4h = (struct iphdr *)(eth + 1); + if (ip4h + 1 > data_end) + return TC_ACT_OK; + if (ip4h->protocol != IPPROTO_TCP) + return TC_ACT_OK; + th = (struct tcphdr *)(ip4h + 1); + if (th + 1 > data_end) + return TC_ACT_OK; + /* Is it the testing traffic? */ + if (th->dest != srv_sa4.sin_port) + return TC_ACT_OK; + tuple_len = sizeof(tuple->ipv4); + tuple = (struct bpf_sock_tuple *)&ip4h->saddr; + iphdr = ip4h; + iphdr_size = sizeof(*ip4h); + break; + case bpf_htons(ETH_P_IPV6): + ip6h = (struct ipv6hdr *)(eth + 1); + if (ip6h + 1 > data_end) + return TC_ACT_OK; + if (ip6h->nexthdr != IPPROTO_TCP) + return TC_ACT_OK; + th = (struct tcphdr *)(ip6h + 1); + if (th + 1 > data_end) + return TC_ACT_OK; + /* Is it the testing traffic? */ + if (th->dest != srv_sa6.sin6_port) + return TC_ACT_OK; + tuple_len = sizeof(tuple->ipv6); + tuple = (struct bpf_sock_tuple *)&ip6h->saddr; + iphdr = ip6h; + iphdr_size = sizeof(*ip6h); + break; + default: + return TC_ACT_OK; + }

- /* Is it the testing traffic? */ - if (th->dest != srv_sa6.sin6_port) + if (!tuple) { + LOG(); return TC_ACT_OK; - - tuple_len = sizeof(tuple->ipv6); - tuple = (struct bpf_sock_tuple *)&ip6h->saddr; + } if ((void *)tuple + tuple_len > data_end) { LOG(); return TC_ACT_OK; @@ -126,7 +164,7 @@ static int handle_ip6_tcp(struct ipv6hdr *ip6h, struct __sk_buff *skb)

listen_tp_sport = tp->inet_conn.icsk_inet.sk.__sk_common.skc_num;

- test_syncookie_helper(ip6h, th, tp, skb); + test_syncookie_helper(iphdr, iphdr_size, th, tp, skb); bpf_sk_release(tp); return TC_ACT_OK; } @@ -142,7 +180,6 @@ static int handle_ip6_tcp(struct ipv6hdr *ip6h, struct __sk_buff *skb) SEC("tc") int cls_ingress(struct __sk_buff *skb) { - struct ipv6hdr *ip6h; struct ethhdr *eth; void *data_end;

@@ -152,15 +189,11 @@ int cls_ingress(struct __sk_buff *skb) if (eth + 1 > data_end) return TC_ACT_OK;

- if (eth->h_proto != bpf_htons(ETH_P_IPV6)) - return TC_ACT_OK; - - ip6h = (struct ipv6hdr *)(eth + 1); - if (ip6h + 1 > data_end) + if (eth->h_proto != bpf_htons(ETH_P_IP) && + eth->h_proto != bpf_htons(ETH_P_IPV6)) return TC_ACT_OK;

- if (ip6h->nexthdr == IPPROTO_TCP) - return handle_ip6_tcp(ip6h, skb); + return handle_ip_tcp(eth, skb);

return TC_ACT_OK; }

-- 2.46.2

Martin KaFai Lau

19 Oct 19 Oct

12:30 a.m.

New subject: [PATCH bpf-next 4/6] selftests/bpf: add ipv4 and dual ipv4/ipv6 support in btf_skc_cls_ingress

On 10/16/24 11:35 AM, Alexis Lothoré (eBPF Foundation) wrote:

...

btf_skc_cls_ingress test currently checks that syncookie and bpf_sk_assign/release helpers behave correctly in multiple scenarios, but only with ipv4 socket.

Increase those helpers coverage by adding testing support for IPv6-only sockets and IPv4/IPv6 sockets. The rework is mostly based on features brought earlier in test_tcp_check_syncookie.sh to cover some fixes performed on those helpers, but test_tcp_check_syncookie.sh is not integrated in test_progs. The most notable changes linked to this are:

some rework in the corresponding eBPF program to support both types of traffic

the switch from start_server to start_server_str to allow to check some socket options

the introduction of new subtests for ipv4 and ipv4/ipv6

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com

The rework has been tested in a local Qemu environment and in CI: # ./test_progs -a btf_skc_cls_ingress #38/1 btf_skc_cls_ingress/conn_ipv4:OK #38/2 btf_skc_cls_ingress/conn_ipv6:OK #38/3 btf_skc_cls_ingress/conn_dual:OK #38/4 btf_skc_cls_ingress/syncookie_ipv4:OK #38/5 btf_skc_cls_ingress/syncookie_ipv6:OK #38/6 btf_skc_cls_ingress/syncookie_dual:OK #38 btf_skc_cls_ingress:OK Summary: 1/6 PASSED, 0 SKIPPED, 0 FAILED

.../selftests/bpf/prog_tests/btf_skc_cls_ingress.c | 116 ++++++++++++++++++--- .../selftests/bpf/progs/test_btf_skc_cls_ingress.c | 81 +++++++++----- 2 files changed, 161 insertions(+), 36 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c index a20d104f9909e5ba20ddc4c107b910956f042fc1..e0f8fe818f4230a1d5bf0118133c5a9fb50345e1 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c @@ -19,6 +19,15 @@ #define TEST_NS "skc_cls_ingress" +#define BIT(n) (1 << (n)) +#define TEST_MODE_IPV4 BIT(0) +#define TEST_MODE_IPV6 BIT(1) +#define TEST_MODE_DUAL (TEST_MODE_IPV4 | TEST_MODE_IPV6)

+#define SERVER_ADDR_IPV4 "127.0.0.1" +#define SERVER_ADDR_IPV6 "::1" +#define SERVER_ADDR_DUAL "::0"

static struct netns_obj *prepare_netns(struct test_btf_skc_cls_ingress *skel) { LIBBPF_OPTS(bpf_tc_hook, qdisc_lo, .attach_point = BPF_TC_INGRESS);

@@ -57,6 +66,7 @@ static struct netns_obj *prepare_netns(struct test_btf_skc_cls_ingress *skel) static void reset_test(struct test_btf_skc_cls_ingress *skel) {

memset(&skel->bss->srv_sa4, 0, sizeof(skel->bss->srv_sa4)); memset(&skel->bss->srv_sa6, 0, sizeof(skel->bss->srv_sa6)); skel->bss->listen_tp_sport = 0; skel->bss->req_sk_sport = 0;

@@ -71,26 +81,84 @@ static void print_err_line(struct test_btf_skc_cls_ingress *skel) printf("bpf prog error at line %u\n", skel->bss->linum); } -static void run_test(struct test_btf_skc_cls_ingress *skel, bool gen_cookies) +static int v6only_true(int fd, void *opts) +{

int mode = true;

return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &mode, sizeof(mode));

+}

+static int v6only_false(int fd, void *opts) +{

int mode = false;

return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &mode, sizeof(mode));

+}

+static void run_test(struct test_btf_skc_cls_ingress *skel, bool gen_cookies,
     int ip_mode)
{ const char *tcp_syncookies = gen_cookies ? "2" : "1"; int listen_fd = -1, cli_fd = -1, srv_fd = -1, err;
struct network_helper_opts opts = { 0 };

struct sockaddr_storage *addr; struct sockaddr_in6 srv_sa6;
socklen_t addrlen = sizeof(srv_sa6);

struct sockaddr_in srv_sa4;

socklen_t addr_len;

int sock_family;

char *srv_addr; int srv_port;
switch (ip_mode) {

case TEST_MODE_IPV4:
sock_family = AF_INET;
srv_addr = SERVER_ADDR_IPV4;
addr = (struct sockaddr_storage *)&srv_sa4;
addr_len = sizeof(srv_sa4);
break;
case TEST_MODE_IPV6:
opts.post_socket_cb = v6only_true;
sock_family = AF_INET6;
srv_addr = SERVER_ADDR_IPV6;
addr = (struct sockaddr_storage *)&srv_sa6;
addr_len = sizeof(srv_sa6);
break;
case TEST_MODE_DUAL:
opts.post_socket_cb = v6only_false;
sock_family = AF_INET6;
srv_addr = SERVER_ADDR_DUAL;
addr = (struct sockaddr_storage *)&srv_sa6;
addr_len = sizeof(srv_sa6);
break;
default:
	break;

nit. indentation is off.

better directly "return;", in case future something complains vars are not init.

...

}

if (write_sysctl("/proc/sys/net/ipv4/tcp_syncookies", tcp_syncookies)) return;

listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0);
listen_fd = start_server_str(sock_family, SOCK_STREAM, srv_addr, 0,
		     &opts);
if (!ASSERT_OK_FD(listen_fd, "start server")) return;
err = getsockname(listen_fd, (struct sockaddr *)&srv_sa6, &addrlen);

err = getsockname(listen_fd, (struct sockaddr *)addr, &addr_len); if (!ASSERT_OK(err, "getsockname(listen_fd)")) goto done;

memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6));

srv_port = ntohs(srv_sa6.sin6_port);
switch (ip_mode) {

case TEST_MODE_IPV4:
memcpy(&skel->bss->srv_sa4, &srv_sa4, sizeof(srv_sa4));
srv_port = ntohs(srv_sa4.sin_port);
break;
case TEST_MODE_IPV6:

case TEST_MODE_DUAL:
memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6));
srv_port = ntohs(srv_sa6.sin6_port);
break;
default:
	break;

indentation off. also "goto done;"

...

}

cli_fd = connect_to_fd(listen_fd, 0); if (!ASSERT_OK_FD(cli_fd, "connect client")) @@ -127,14 +195,34 @@ static void run_test(struct test_btf_skc_cls_ingress *skel, bool gen_cookies) close(srv_fd); } -static void test_conn(struct test_btf_skc_cls_ingress *skel) +static void test_conn_ipv4(struct test_btf_skc_cls_ingress *skel) +{

run_test(skel, false, TEST_MODE_IPV4);

+}

+static void test_conn_ipv6(struct test_btf_skc_cls_ingress *skel) +{

run_test(skel, false, TEST_MODE_IPV6);

+}

+static void test_conn_dual(struct test_btf_skc_cls_ingress *skel) +{

run_test(skel, false, TEST_MODE_DUAL);

+}

+static void test_syncookie_ipv4(struct test_btf_skc_cls_ingress *skel) +{

run_test(skel, true, TEST_MODE_IPV4);

+}

+static void test_syncookie_ipv6(struct test_btf_skc_cls_ingress *skel) {

run_test(skel, false);

run_test(skel, true, TEST_MODE_IPV6); }

-static void test_syncookie(struct test_btf_skc_cls_ingress *skel) +static void test_syncookie_dual(struct test_btf_skc_cls_ingress *skel) {

run_test(skel, true);

run_test(skel, true, TEST_MODE_DUAL); }

struct test { @@ -144,8 +232,12 @@ struct test { #define DEF_TEST(name) { #name, test_##name } static struct test tests[] = {

DEF_TEST(conn),

DEF_TEST(syncookie),

DEF_TEST(conn_ipv4),

DEF_TEST(conn_ipv6),

DEF_TEST(conn_dual),

DEF_TEST(syncookie_ipv4),

DEF_TEST(syncookie_ipv6),

DEF_TEST(syncookie_dual), };

void test_btf_skc_cls_ingress(void) diff --git a/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c index f0759efff6ef15d2663927400dd064c53b020f78..cd528f8792ff2eb14683cbc13e8b0f3fd38329e9 100644 --- a/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c @@ -10,6 +10,7 @@ #endif struct sockaddr_in6 srv_sa6 = {}; +struct sockaddr_in srv_sa4 = {}; __u16 listen_tp_sport = 0; __u16 req_sk_sport = 0; __u32 recv_cookie = 0; @@ -18,8 +19,8 @@ __u32 linum = 0; #define LOG() ({ if (!linum) linum = __LINE__; }) -static void test_syncookie_helper(struct ipv6hdr *ip6h, struct tcphdr *th,
		  struct tcp_sock *tp,
+static void test_syncookie_helper(void *iphdr, int iphdr_size,
		  struct tcphdr *th, struct tcp_sock *tp,
	  struct __sk_buff *skb)
{ if (th->syn) {
@@ -38,7 +39,7 @@ static void test_syncookie_helper(struct ipv6hdr *ip6h, struct tcphdr *th, return; }
mss_cookie = bpf_tcp_gen_syncookie(tp, ip6h, sizeof(*ip6h),
mss_cookie = bpf_tcp_gen_syncookie(tp, iphdr, iphdr_size,
			   th, 40);
if (mss_cookie < 0) { if (mss_cookie != -ENOENT)
@@ -48,7 +49,7 @@ static void test_syncookie_helper(struct ipv6hdr *ip6h, struct tcphdr *th, } } else if (gen_cookie) { /* It was in cookie mode */
int ret = bpf_tcp_check_syncookie(tp, ip6h, sizeof(*ip6h),
int ret = bpf_tcp_check_syncookie(tp, iphdr, iphdr_size,
			  th, sizeof(*th));
if (ret < 0) { @@ -60,26 +61,63 @@ static void test_syncookie_helper(struct ipv6hdr *ip6h, struct tcphdr *th, } } -static int handle_ip6_tcp(struct ipv6hdr *ip6h, struct __sk_buff *skb) +static int handle_ip_tcp(struct ethhdr *eth, struct __sk_buff *skb) {

struct bpf_sock_tuple *tuple;

struct bpf_sock_tuple *tuple = NULL;

unsigned int tuple_len = 0; struct bpf_sock *bpf_skc;

unsigned int tuple_len;

struct ipv6hdr *ip6h;

void *iphdr = NULL;

int iphdr_size = 0;

struct iphdr *ip4h;

nit. All new "= 0;" and "= NULL;" init should not be needed.

...

struct tcphdr *th; void *data_end; data_end = (void *)(long)(skb->data_end);

th = (struct tcphdr *)(ip6h + 1);
if (th + 1 > data_end)
```
return TC_ACT_OK;
```

switch (eth->h_proto) {
case bpf_htons(ETH_P_IP):
```
ip4h = (struct iphdr *)(eth + 1);
```
```
if (ip4h + 1 > data_end)
```
```
	return TC_ACT_OK;
```
```
if (ip4h->protocol != IPPROTO_TCP)
```
```
	return TC_ACT_OK;
```
```
th = (struct tcphdr *)(ip4h + 1);
```
```
if (th + 1 > data_end)
```
```
	return TC_ACT_OK;
```
```
/* Is it the testing traffic? */
```
```
if (th->dest != srv_sa4.sin_port)
```
```
	return TC_ACT_OK;
```
```
tuple_len = sizeof(tuple->ipv4);
```

tuple = (struct bpf_sock_tuple *)&ip4h->saddr;

```
iphdr = ip4h;
```
```
iphdr_size = sizeof(*ip4h);
```
```
break;
```
case bpf_htons(ETH_P_IPV6):
```
ip6h = (struct ipv6hdr *)(eth + 1);
```
```
if (ip6h + 1 > data_end)
```
```
	return TC_ACT_OK;
```
```
if (ip6h->nexthdr != IPPROTO_TCP)
```
```
	return TC_ACT_OK;
```
```
th = (struct tcphdr *)(ip6h + 1);
```
```
if (th + 1 > data_end)
```
```
	return TC_ACT_OK;
```
```
/* Is it the testing traffic? */
```
```
if (th->dest != srv_sa6.sin6_port)
```
```
	return TC_ACT_OK;
```
```
tuple_len = sizeof(tuple->ipv6);
```

tuple = (struct bpf_sock_tuple *)&ip6h->saddr;

```
iphdr = ip6h;
```
```
iphdr_size = sizeof(*ip6h);
```
```
break;
```
default:
```
	return TC_ACT_OK;
```

indentation is off also.

...

}

/* Is it the testing traffic? */

if (th->dest != srv_sa6.sin6_port)

if (!tuple) {

!tuple should not be possible. can be removed.

...

LOG();
return TC_ACT_OK;
tuple_len = sizeof(tuple->ipv6);

tuple = (struct bpf_sock_tuple *)&ip6h->saddr;

} if ((void *)tuple + tuple_len > data_end) { LOG(); return TC_ACT_OK;

@@ -126,7 +164,7 @@ static int handle_ip6_tcp(struct ipv6hdr *ip6h, struct __sk_buff *skb) listen_tp_sport = tp->inet_conn.icsk_inet.sk.__sk_common.skc_num;
test_syncookie_helper(ip6h, th, tp, skb);
test_syncookie_helper(iphdr, iphdr_size, th, tp, skb);
bpf_sk_release(tp); return TC_ACT_OK; }
@@ -142,7 +180,6 @@ static int handle_ip6_tcp(struct ipv6hdr *ip6h, struct __sk_buff *skb) SEC("tc") int cls_ingress(struct __sk_buff *skb) {

struct ipv6hdr *ip6h; struct ethhdr *eth; void *data_end;

@@ -152,15 +189,11 @@ int cls_ingress(struct __sk_buff *skb) if (eth + 1 > data_end) return TC_ACT_OK;
if (eth->h_proto != bpf_htons(ETH_P_IPV6))
return TC_ACT_OK;
ip6h = (struct ipv6hdr *)(eth + 1);

if (ip6h + 1 > data_end)
if (eth->h_proto != bpf_htons(ETH_P_IP) &&
   eth->h_proto != bpf_htons(ETH_P_IPV6))
return TC_ACT_OK;
if (ip6h->nexthdr == IPPROTO_TCP)
return handle_ip6_tcp(ip6h, skb);
return handle_ip_tcp(eth, skb);

return TC_ACT_OK;

The last double return should have been removed also.

...

}

Alexis Lothoré

12:43 p.m.

New subject: [PATCH bpf-next 4/6] selftests/bpf: add ipv4 and dual ipv4/ipv6 support in btf_skc_cls_ingress

On 10/19/24 02:30, Martin KaFai Lau wrote:

...

On 10/16/24 11:35 AM, Alexis Lothoré (eBPF Foundation) wrote:

[...]

...

...
+ switch (ip_mode) { + case TEST_MODE_IPV4: + sock_family = AF_INET; + srv_addr = SERVER_ADDR_IPV4; + addr = (struct sockaddr_storage *)&srv_sa4; + addr_len = sizeof(srv_sa4); + break; + case TEST_MODE_IPV6: + opts.post_socket_cb = v6only_true; + sock_family = AF_INET6; + srv_addr = SERVER_ADDR_IPV6; + addr = (struct sockaddr_storage *)&srv_sa6; + addr_len = sizeof(srv_sa6); + break; + case TEST_MODE_DUAL: + opts.post_socket_cb = v6only_false; + sock_family = AF_INET6; + srv_addr = SERVER_ADDR_DUAL; + addr = (struct sockaddr_storage *)&srv_sa6; + addr_len = sizeof(srv_sa6); + break; + default: + break;

nit. indentation is off.

True, and for some reason checkpatch does not raise any warning here. This will be fixed in v2 for all switch cases

...

better directly "return;", in case future something complains vars are not init.

ACK. I'll also add a PRINT_FAIL call in those default statements.

...

...
+    }

if (write_sysctl("/proc/sys/net/ipv4/tcp_syncookies", tcp_syncookies))           return; -    listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0); +    listen_fd = start_server_str(sock_family, SOCK_STREAM, srv_addr, 0, +                     &opts);       if (!ASSERT_OK_FD(listen_fd, "start server"))           return; -    err = getsockname(listen_fd, (struct sockaddr *)&srv_sa6, &addrlen); +    err = getsockname(listen_fd, (struct sockaddr *)addr, &addr_len);       if (!ASSERT_OK(err, "getsockname(listen_fd)"))           goto done; -    memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6)); -    srv_port = ntohs(srv_sa6.sin6_port);

+    switch (ip_mode) { +    case TEST_MODE_IPV4: +        memcpy(&skel->bss->srv_sa4, &srv_sa4, sizeof(srv_sa4)); +        srv_port = ntohs(srv_sa4.sin_port); +        break; +    case TEST_MODE_IPV6: +    case TEST_MODE_DUAL: +        memcpy(&skel->bss->srv_sa6, &srv_sa6, sizeof(srv_sa6)); +        srv_port = ntohs(srv_sa6.sin6_port); +        break; +    default: +            break;

indentation off. also "goto done;"

I can add the goto done as a safety net (eg in case someone modifies the test later and miss some details), but with the return added on the switch above (also done on ip_mode), the goto should never be executed.

[...]

...

...
-static int handle_ip6_tcp(struct ipv6hdr *ip6h, struct __sk_buff *skb) +static int handle_ip_tcp(struct ethhdr *eth, struct __sk_buff *skb) { - struct bpf_sock_tuple *tuple; + struct bpf_sock_tuple *tuple = NULL; + unsigned int tuple_len = 0; struct bpf_sock *bpf_skc; - unsigned int tuple_len; + struct ipv6hdr *ip6h; + void *iphdr = NULL; + int iphdr_size = 0; + struct iphdr *ip4h;

nit. All new "= 0;" and "= NULL;" init should not be needed.

I added those "by default" because my last series raised some build errors with clang while building fine with gcc (but in the end, there was indeed code paths possibly using those variables uninitialized). ACK, I'll remove those initializers.

[...]

...

...
+ } - /* Is it the testing traffic? */ - if (th->dest != srv_sa6.sin6_port) + if (!tuple) {

!tuple should not be possible. can be removed.

I initially added that check because of the verifier complained about tuple being scalar. But I have checked and removed it again and it is now... accepted ? I guess I was still missing some other parts when I got this error, very likely the `return TC_ACT_OK` in the default case. I'll then remove this check in v2.

[...]

...

...
- if (ip6h->nexthdr == IPPROTO_TCP) - return handle_ip6_tcp(ip6h, skb); + return handle_ip_tcp(eth, skb); return TC_ACT_OK;

The last double return should have been removed also.

That's indeed a miss, I'll remove it.

Thanks,

Alexis

-- Alexis Lothoré, Bootlin Embedded Linux and Kernel engineering https://bootlin.com

Alexis Lothoré (eBPF Foundation)

16 Oct 16 Oct

6:35 p.m.

New subject: [PATCH bpf-next 5/6] selftests/bpf: test MSS value returned with bpf_tcp_gen_syncookie

One remaining difference between test_tcp_check_syncookie.sh and btf_skc_cls_ingress is a small test on the mss value embedded in the cookie generated with the eBPF helper.

Bring the corresponding test in btf_skc_cls_ingress.

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c | 7 +++++++ tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c | 2 ++ 2 files changed, 9 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c index e0f8fe818f4230a1d5bf0118133c5a9fb50345e1..64601bafdd23e7fb13ebd496ac914660685f1efc 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_skc_cls_ingress.c @@ -27,6 +27,8 @@ #define SERVER_ADDR_IPV4 "127.0.0.1" #define SERVER_ADDR_IPV6 "::1" #define SERVER_ADDR_DUAL "::0" +/* RFC791, 576 for minimal IPv4 datagram, minus 40 bytes of TCP header */ +#define MIN_IPV4_MSS 536

static struct netns_obj *prepare_netns(struct test_btf_skc_cls_ingress *skel) { @@ -73,6 +75,7 @@ static void reset_test(struct test_btf_skc_cls_ingress *skel) skel->bss->recv_cookie = 0; skel->bss->gen_cookie = 0; skel->bss->linum = 0; + skel->bss->mss = 0; }

static void print_err_line(struct test_btf_skc_cls_ingress *skel) @@ -184,6 +187,10 @@ static void run_test(struct test_btf_skc_cls_ingress *skel, bool gen_cookies, "syncookie properly generated"); ASSERT_EQ(skel->bss->gen_cookie, skel->bss->recv_cookie, "matching syncookies on client and server"); + ASSERT_GT(skel->bss->mss, MIN_IPV4_MSS, + "MSS in cookie min value"); + ASSERT_LT(skel->bss->mss, USHRT_MAX, + "MSS in cookie max value"); }

done: diff --git a/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c b/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c index cd528f8792ff2eb14683cbc13e8b0f3fd38329e9..3810ce98d3d54276f9a0130a887f59c79a21288e 100644 --- a/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c +++ b/tools/testing/selftests/bpf/progs/test_btf_skc_cls_ingress.c @@ -15,6 +15,7 @@ __u16 listen_tp_sport = 0; __u16 req_sk_sport = 0; __u32 recv_cookie = 0; __u32 gen_cookie = 0; +__u32 mss = 0; __u32 linum = 0;

#define LOG() ({ if (!linum) linum = __LINE__; }) @@ -46,6 +47,7 @@ static void test_syncookie_helper(void *iphdr, int iphdr_size, LOG(); } else { gen_cookie = (__u32)mss_cookie; + mss = mss_cookie >> 32; } } else if (gen_cookie) { /* It was in cookie mode */

-- 2.46.2

Alexis Lothoré (eBPF Foundation)

6:35 p.m.

New subject: [PATCH bpf-next 6/6] selftests/bpf: remove test_tcp_check_syncookie

Now that btf_skc_cls_ingress has the same coverage as test_tcp_check_syncookie, remove the second one and keep the first one as it is integrated in test_progs

Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com --- tools/testing/selftests/bpf/.gitignore | 1 - tools/testing/selftests/bpf/Makefile | 9 +- .../bpf/progs/test_tcp_check_syncookie_kern.c | 167 ---------------- .../selftests/bpf/test_tcp_check_syncookie.sh | 85 -------- .../selftests/bpf/test_tcp_check_syncookie_user.c | 213 --------------------- 5 files changed, 3 insertions(+), 472 deletions(-)

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore index e6533b3400de5ef9b0c0e02aa0e1afedcab9b349..7e88551f2d38bdfa87fd56ac83874c88c4335927 100644 --- a/tools/testing/selftests/bpf/.gitignore +++ b/tools/testing/selftests/bpf/.gitignore @@ -24,7 +24,6 @@ test_flow_dissector flow_dissector_load test_tcpnotify_user test_libbpf -test_tcp_check_syncookie_user test_sysctl xdping test_cpp diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 28a76baa854d3ef45bc6d511ad9188e737f0ebe8..7815b815fcef4a8e42f0211253c1e351a9af2abf 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -137,7 +137,6 @@ TEST_PROGS := test_kmod.sh \ test_xdp_vlan_mode_generic.sh \ test_xdp_vlan_mode_native.sh \ test_lwt_ip_encap.sh \ - test_tcp_check_syncookie.sh \ test_tc_tunnel.sh \ test_tc_edt.sh \ test_xdping.sh \ @@ -154,10 +153,9 @@ TEST_PROGS_EXTENDED := with_addr.sh \

# Compile but not part of 'make run_tests' TEST_GEN_PROGS_EXTENDED = \ - flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \ - test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \ - xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \ - xdp_features bpf_test_no_cfi.ko + flow_dissector_load test_flow_dissector test_lirc_mode2_user xdping \ + test_cpp runqslower bench bpf_testmod.ko xskxceiver xdp_redirect_multi \ + xdp_synproxy veristat xdp_hw_metadata xdp_features bpf_test_no_cfi.ko

TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi

@@ -347,7 +345,6 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS) $(OUTPUT)/test_maps: $(TESTING_HELPERS) $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS) $(UNPRIV_HELPERS) $(OUTPUT)/xsk.o: $(BPFOBJ) -$(OUTPUT)/test_tcp_check_syncookie_user: $(NETWORK_HELPERS)

BPFTOOL ?= $(DEFAULT_BPFTOOL) $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \ diff --git a/tools/testing/selftests/bpf/progs/test_tcp_check_syncookie_kern.c b/tools/testing/selftests/bpf/progs/test_tcp_check_syncookie_kern.c deleted file mode 100644 index 6edebce563b57e96b04a20b6d14dbcdca62152d5..0000000000000000000000000000000000000000 --- a/tools/testing/selftests/bpf/progs/test_tcp_check_syncookie_kern.c +++ /dev/null @@ -1,167 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -// Copyright (c) 2018 Facebook -// Copyright (c) 2019 Cloudflare - -#include <string.h> - -#include <linux/bpf.h> -#include <linux/pkt_cls.h> -#include <linux/if_ether.h> -#include <linux/in.h> -#include <linux/ip.h> -#include <linux/ipv6.h> -#include <sys/socket.h> -#include <linux/tcp.h> - -#include <bpf/bpf_helpers.h> -#include <bpf/bpf_endian.h> - -struct { - __uint(type, BPF_MAP_TYPE_ARRAY); - __type(key, __u32); - __type(value, __u32); - __uint(max_entries, 3); -} results SEC(".maps"); - -static __always_inline __s64 gen_syncookie(void *data_end, struct bpf_sock *sk, - void *iph, __u32 ip_size, - struct tcphdr *tcph) -{ - __u32 thlen = tcph->doff * 4; - - if (tcph->syn && !tcph->ack) { - // packet should only have an MSS option - if (thlen != 24) - return 0; - - if ((void *)tcph + thlen > data_end) - return 0; - - return bpf_tcp_gen_syncookie(sk, iph, ip_size, tcph, thlen); - } - return 0; -} - -static __always_inline void check_syncookie(void *ctx, void *data, - void *data_end) -{ - struct bpf_sock_tuple tup; - struct bpf_sock *sk; - struct ethhdr *ethh; - struct iphdr *ipv4h; - struct ipv6hdr *ipv6h; - struct tcphdr *tcph; - int ret; - __u32 key_mss = 2; - __u32 key_gen = 1; - __u32 key = 0; - __s64 seq_mss; - - ethh = data; - if (ethh + 1 > data_end) - return; - - switch (bpf_ntohs(ethh->h_proto)) { - case ETH_P_IP: - ipv4h = data + sizeof(struct ethhdr); - if (ipv4h + 1 > data_end) - return; - - if (ipv4h->ihl != 5) - return; - - tcph = data + sizeof(struct ethhdr) + sizeof(struct iphdr); - if (tcph + 1 > data_end) - return; - - tup.ipv4.saddr = ipv4h->saddr; - tup.ipv4.daddr = ipv4h->daddr; - tup.ipv4.sport = tcph->source; - tup.ipv4.dport = tcph->dest; - - sk = bpf_skc_lookup_tcp(ctx, &tup, sizeof(tup.ipv4), - BPF_F_CURRENT_NETNS, 0); - if (!sk) - return; - - if (sk->state != BPF_TCP_LISTEN) - goto release; - - seq_mss = gen_syncookie(data_end, sk, ipv4h, sizeof(*ipv4h), - tcph); - - ret = bpf_tcp_check_syncookie(sk, ipv4h, sizeof(*ipv4h), - tcph, sizeof(*tcph)); - break; - - case ETH_P_IPV6: - ipv6h = data + sizeof(struct ethhdr); - if (ipv6h + 1 > data_end) - return; - - if (ipv6h->nexthdr != IPPROTO_TCP) - return; - - tcph = data + sizeof(struct ethhdr) + sizeof(struct ipv6hdr); - if (tcph + 1 > data_end) - return; - - memcpy(tup.ipv6.saddr, &ipv6h->saddr, sizeof(tup.ipv6.saddr)); - memcpy(tup.ipv6.daddr, &ipv6h->daddr, sizeof(tup.ipv6.daddr)); - tup.ipv6.sport = tcph->source; - tup.ipv6.dport = tcph->dest; - - sk = bpf_skc_lookup_tcp(ctx, &tup, sizeof(tup.ipv6), - BPF_F_CURRENT_NETNS, 0); - if (!sk) - return; - - if (sk->state != BPF_TCP_LISTEN) - goto release; - - seq_mss = gen_syncookie(data_end, sk, ipv6h, sizeof(*ipv6h), - tcph); - - ret = bpf_tcp_check_syncookie(sk, ipv6h, sizeof(*ipv6h), - tcph, sizeof(*tcph)); - break; - - default: - return; - } - - if (seq_mss > 0) { - __u32 cookie = (__u32)seq_mss; - __u32 mss = seq_mss >> 32; - - bpf_map_update_elem(&results, &key_gen, &cookie, 0); - bpf_map_update_elem(&results, &key_mss, &mss, 0); - } - - if (ret == 0) { - __u32 cookie = bpf_ntohl(tcph->ack_seq) - 1; - - bpf_map_update_elem(&results, &key, &cookie, 0); - } - -release: - bpf_sk_release(sk); -} - -SEC("tc") -int check_syncookie_clsact(struct __sk_buff *skb) -{ - check_syncookie(skb, (void *)(long)skb->data, - (void *)(long)skb->data_end); - return TC_ACT_OK; -} - -SEC("xdp") -int check_syncookie_xdp(struct xdp_md *ctx) -{ - check_syncookie(ctx, (void *)(long)ctx->data, - (void *)(long)ctx->data_end); - return XDP_PASS; -} - -char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/test_tcp_check_syncookie.sh b/tools/testing/selftests/bpf/test_tcp_check_syncookie.sh deleted file mode 100755 index b42c24282c2543a61b559e70c4961011ddb8c463..0000000000000000000000000000000000000000 --- a/tools/testing/selftests/bpf/test_tcp_check_syncookie.sh +++ /dev/null @@ -1,85 +0,0 @@ -#!/bin/sh -# SPDX-License-Identifier: GPL-2.0 -# Copyright (c) 2018 Facebook -# Copyright (c) 2019 Cloudflare - -set -eu -readonly NS1="ns1-$(mktemp -u XXXXXX)" - -wait_for_ip() -{ - local _i - printf "Wait for IP %s to become available " "$1" - for _i in $(seq ${MAX_PING_TRIES}); do - printf "." - if ns1_exec ping -c 1 -W 1 "$1" >/dev/null 2>&1; then - echo " OK" - return - fi - sleep 1 - done - echo 1>&2 "ERROR: Timeout waiting for test IP to become available." - exit 1 -} - -get_prog_id() -{ - awk '/ id / {sub(/.* id /, "", $0); print($1)}' -} - -ns1_exec() -{ - ip netns exec ${NS1} "$@" -} - -setup() -{ - ip netns add ${NS1} - ns1_exec ip link set lo up - - ns1_exec sysctl -w net.ipv4.tcp_syncookies=2 - ns1_exec sysctl -w net.ipv4.tcp_window_scaling=0 - ns1_exec sysctl -w net.ipv4.tcp_timestamps=0 - ns1_exec sysctl -w net.ipv4.tcp_sack=0 - - wait_for_ip 127.0.0.1 - wait_for_ip ::1 -} - -cleanup() -{ - ip netns del ns1 2>/dev/null || : -} - -main() -{ - trap cleanup EXIT 2 3 6 15 - setup - - printf "Testing clsact..." - ns1_exec tc qdisc add dev "${TEST_IF}" clsact - ns1_exec tc filter add dev "${TEST_IF}" ingress \ - bpf obj "${BPF_PROG_OBJ}" sec "${CLSACT_SECTION}" da - - BPF_PROG_ID=$(ns1_exec tc filter show dev "${TEST_IF}" ingress | \ - get_prog_id) - ns1_exec "${PROG}" "${BPF_PROG_ID}" - ns1_exec tc qdisc del dev "${TEST_IF}" clsact - - printf "Testing XDP..." - ns1_exec ip link set "${TEST_IF}" xdp \ - object "${BPF_PROG_OBJ}" section "${XDP_SECTION}" - BPF_PROG_ID=$(ns1_exec ip link show "${TEST_IF}" | get_prog_id) - ns1_exec "${PROG}" "${BPF_PROG_ID}" -} - -DIR=$(dirname $0) -TEST_IF=lo -MAX_PING_TRIES=5 -BPF_PROG_OBJ="${DIR}/test_tcp_check_syncookie_kern.bpf.o" -CLSACT_SECTION="tc" -XDP_SECTION="xdp" -BPF_PROG_ID=0 -PROG="${DIR}/test_tcp_check_syncookie_user" - -main diff --git a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c b/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c deleted file mode 100644 index 3844f9b8232a25278e8190bc9b878ed73060fd4d..0000000000000000000000000000000000000000 --- a/tools/testing/selftests/bpf/test_tcp_check_syncookie_user.c +++ /dev/null @@ -1,213 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -// Copyright (c) 2018 Facebook -// Copyright (c) 2019 Cloudflare - -#include <limits.h> -#include <string.h> -#include <stdlib.h> -#include <unistd.h> - -#include <arpa/inet.h> -#include <netinet/in.h> -#include <sys/types.h> -#include <sys/socket.h> - -#include <bpf/bpf.h> -#include <bpf/libbpf.h> - -#include "cgroup_helpers.h" -#include "network_helpers.h" - -static int get_map_fd_by_prog_id(int prog_id, bool *xdp) -{ - struct bpf_prog_info info = {}; - __u32 info_len = sizeof(info); - __u32 map_ids[1]; - int prog_fd = -1; - int map_fd = -1; - - prog_fd = bpf_prog_get_fd_by_id(prog_id); - if (prog_fd < 0) { - log_err("Failed to get fd by prog id %d", prog_id); - goto err; - } - - info.nr_map_ids = 1; - info.map_ids = (__u64)(unsigned long)map_ids; - - if (bpf_prog_get_info_by_fd(prog_fd, &info, &info_len)) { - log_err("Failed to get info by prog fd %d", prog_fd); - goto err; - } - - if (!info.nr_map_ids) { - log_err("No maps found for prog fd %d", prog_fd); - goto err; - } - - *xdp = info.type == BPF_PROG_TYPE_XDP; - - map_fd = bpf_map_get_fd_by_id(map_ids[0]); - if (map_fd < 0) - log_err("Failed to get fd by map id %d", map_ids[0]); -err: - if (prog_fd >= 0) - close(prog_fd); - return map_fd; -} - -static int run_test(int server_fd, int results_fd, bool xdp) -{ - int client = -1, srv_client = -1; - int ret = 0; - __u32 key = 0; - __u32 key_gen = 1; - __u32 key_mss = 2; - __u32 value = 0; - __u32 value_gen = 0; - __u32 value_mss = 0; - - if (bpf_map_update_elem(results_fd, &key, &value, 0) < 0) { - log_err("Can't clear results"); - goto err; - } - - if (bpf_map_update_elem(results_fd, &key_gen, &value_gen, 0) < 0) { - log_err("Can't clear results"); - goto err; - } - - if (bpf_map_update_elem(results_fd, &key_mss, &value_mss, 0) < 0) { - log_err("Can't clear results"); - goto err; - } - - client = connect_to_fd(server_fd, 0); - if (client == -1) - goto err; - - srv_client = accept(server_fd, NULL, 0); - if (srv_client == -1) { - log_err("Can't accept connection"); - goto err; - } - - if (bpf_map_lookup_elem(results_fd, &key, &value) < 0) { - log_err("Can't lookup result"); - goto err; - } - - if (value == 0) { - log_err("Didn't match syncookie: %u", value); - goto err; - } - - if (bpf_map_lookup_elem(results_fd, &key_gen, &value_gen) < 0) { - log_err("Can't lookup result"); - goto err; - } - - if (xdp && value_gen == 0) { - // SYN packets do not get passed through generic XDP, skip the - // rest of the test. - printf("Skipping XDP cookie check\n"); - goto out; - } - - if (bpf_map_lookup_elem(results_fd, &key_mss, &value_mss) < 0) { - log_err("Can't lookup result"); - goto err; - } - - if (value != value_gen) { - log_err("BPF generated cookie does not match kernel one"); - goto err; - } - - if (value_mss < 536 || value_mss > USHRT_MAX) { - log_err("Unexpected MSS retrieved"); - goto err; - } - - goto out; - -err: - ret = 1; -out: - close(client); - close(srv_client); - return ret; -} - -static int v6only_true(int fd, void *opts) -{ - int mode = true; - - return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &mode, sizeof(mode)); -} - -static int v6only_false(int fd, void *opts) -{ - int mode = false; - - return setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &mode, sizeof(mode)); -} - -int main(int argc, char **argv) -{ - struct network_helper_opts opts = { 0 }; - int server = -1; - int server_v6 = -1; - int server_dual = -1; - int results = -1; - int err = 0; - bool xdp; - - if (argc < 2) { - fprintf(stderr, "Usage: %s prog_id\n", argv[0]); - exit(1); - } - - /* Use libbpf 1.0 API mode */ - libbpf_set_strict_mode(LIBBPF_STRICT_ALL); - - results = get_map_fd_by_prog_id(atoi(argv[1]), &xdp); - if (results < 0) { - log_err("Can't get map"); - goto err; - } - - server = start_server_str(AF_INET, SOCK_STREAM, "127.0.0.1", 0, NULL); - if (server == -1) - goto err; - - opts.post_socket_cb = v6only_true; - server_v6 = start_server_str(AF_INET6, SOCK_STREAM, "::1", 0, &opts); - if (server_v6 == -1) - goto err; - - opts.post_socket_cb = v6only_false; - server_dual = start_server_str(AF_INET6, SOCK_STREAM, "::0", 0, &opts); - if (server_dual == -1) - goto err; - - if (run_test(server, results, xdp)) - goto err; - - if (run_test(server_v6, results, xdp)) - goto err; - - if (run_test(server_dual, results, xdp)) - goto err; - - printf("ok\n"); - goto out; -err: - err = 1; -out: - close(server); - close(server_v6); - close(server_dual); - close(results); - return err; -}

-- 2.46.2

Martin KaFai Lau

19 Oct 19 Oct

12:34 a.m.

New subject: [PATCH bpf-next 6/6] selftests/bpf: remove test_tcp_check_syncookie

On 10/16/24 11:35 AM, Alexis Lothoré (eBPF Foundation) wrote:

...

.../bpf/progs/test_tcp_check_syncookie_kern.c | 167 ---------------- .../selftests/bpf/test_tcp_check_syncookie.sh | 85 -------- .../selftests/bpf/test_tcp_check_syncookie_user.c | 213 ---------------------

Nice.

Left some comments in the earlier patches. Overall looks good. Thanks.

pw-bot: cr

days inactive

days old

linux-kselftest-mirror@lists.linaro.org

11 comments

participants

tags (0)

participants (3)

Alexis Lothoré
Alexis Lothoré (eBPF Foundation)
Martin KaFai Lau