| Message ID | 20230124181220.2871611-3-kuifeng@meta.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Enable bpf_setsockopt() on ktls enabled sockets. | expand |
On 1/24/23 10:12 AM, Kui-Feng Lee wrote: > +static void test_ktls(int family) > +{ > + struct tls12_crypto_info_aes_gcm_128 aes128; > + struct setget_sockopt__bss *bss = skel->bss; > + int cfd = -1, sfd = -1, fd = -1, ret; > + > + memset(bss, 0, sizeof(*bss)); > + > + sfd = start_server(family, SOCK_STREAM, > + family == AF_INET6 ? addr6_str : addr4_str, 0, 0); > + if (!ASSERT_GE(sfd, 0, "start_server")) > + return; > + fd = connect_to_fd(sfd, 0); > + if (!ASSERT_GE(fd, 0, "connect_to_fd")) > + goto err_out; > + > + cfd = accept(sfd, NULL, 0); > + if (!ASSERT_GE(cfd, 0, "accept")) > + goto err_out; > + > + close(sfd); > + sfd = -1; > + > + /* Setup KTLS */ > + ret = setsockopt(fd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls")); > + if (!ASSERT_OK(ret, "setsockopt")) > + goto err_out; > + ret = setsockopt(cfd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls")); > + if (!ASSERT_OK(ret, "setsockopt")) > + goto err_out; > + > + memset(&aes128, 0, sizeof(aes128)); > + aes128.info.version = TLS_1_2_VERSION; > + aes128.info.cipher_type = TLS_CIPHER_AES_GCM_128; > + > + ret = setsockopt(fd, SOL_TLS, TLS_TX, &aes128, sizeof(aes128)); > + if (!ASSERT_OK(ret, "setsockopt")) > + goto err_out; > + > + ret = setsockopt(cfd, SOL_TLS, TLS_RX, &aes128, sizeof(aes128)); > + if (!ASSERT_OK(ret, "setsockopt")) > + goto err_out; > + > + /* KTLS is enabled */ > + > + close(fd); > + /* At this point, the cfd socket is at the CLOSE_WAIT state > + * and still run TLS protocol. The test for > + * BPF_TCP_CLOSE_WAIT should be run at this point. > + */ Just came to my mind. I think it is better to ensure the cfd got the FIN first to avoid potential (unlikely) flaky test: ret = read(cfd, ...); ASSERT_EQ(ret, 0, ...); > + close(cfd); > + > + ASSERT_EQ(bss->nr_listen, 1, "nr_listen"); > + ASSERT_EQ(bss->nr_connect, 1, "nr_connect"); > + ASSERT_EQ(bss->nr_active, 1, "nr_active"); > + ASSERT_EQ(bss->nr_passive, 1, "nr_passive"); > + ASSERT_EQ(bss->nr_socket_post_create, 2, "nr_socket_post_create"); > + ASSERT_EQ(bss->nr_binddev, 2, "nr_bind"); > + ASSERT_EQ(bss->nr_fin_wait1, 1, "nr_fin_wait1"); > + return; > + > +err_out: > + close(fd); > + close(cfd); > + close(sfd); > +} > + > void test_setget_sockopt(void) > { > cg_fd = test__join_cgroup(CG_NAME); > @@ -118,6 +186,8 @@ void test_setget_sockopt(void) > test_tcp(AF_INET); > test_udp(AF_INET6); > test_udp(AF_INET); > + test_ktls(AF_INET6); > + test_ktls(AF_INET); > > done: > setget_sockopt__destroy(skel); > diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt.c b/tools/testing/selftests/bpf/progs/setget_sockopt.c > index 9523333b8905..027d95755f9f 100644 > --- a/tools/testing/selftests/bpf/progs/setget_sockopt.c > +++ b/tools/testing/selftests/bpf/progs/setget_sockopt.c > @@ -6,6 +6,8 @@ > #include <bpf/bpf_core_read.h> > #include <bpf/bpf_helpers.h> > #include <bpf/bpf_tracing.h> > +#define BPF_PROG_TEST_TCP_HDR_OPTIONS > +#include "test_tcp_hdr_options.h" Instead of having dependency on another test's header, > > #ifndef ARRAY_SIZE > #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > @@ -22,6 +24,7 @@ int nr_active; > int nr_connect; > int nr_binddev; > int nr_socket_post_create; > +int nr_fin_wait1; > > struct sockopt_test { > int opt; > @@ -386,6 +389,11 @@ int skops_sockopt(struct bpf_sock_ops *skops) > nr_passive += !(bpf_test_sockopt(skops, sk) || > test_tcp_maxseg(skops, sk) || > test_tcp_saved_syn(skops, sk)); > + set_hdr_cb_flags(skops, BPF_SOCK_OPS_STATE_CB_FLAG); how about directly doing this: bpf_sock_ops_cb_flags_set(skops, skops->bpf_sock_ops_cb_flags | BPF_SOCK_OPS_STATE_CB_FLAG); > + break; > + case BPF_SOCK_OPS_STATE_CB: > + if (skops->args[1] == BPF_TCP_CLOSE_WAIT) > + nr_fin_wait1 += !bpf_test_sockopt(skops, sk); > break; > } >
On 1/25/23 10:09, Martin KaFai Lau wrote: > On 1/24/23 10:12 AM, Kui-Feng Lee wrote: index > 9523333b8905..027d95755f9f 100644 .... skip .... >> --- a/tools/testing/selftests/bpf/progs/setget_sockopt.c >> +++ b/tools/testing/selftests/bpf/progs/setget_sockopt.c >> @@ -6,6 +6,8 @@ >> #include <bpf/bpf_core_read.h> >> #include <bpf/bpf_helpers.h> >> #include <bpf/bpf_tracing.h> >> +#define BPF_PROG_TEST_TCP_HDR_OPTIONS >> +#include "test_tcp_hdr_options.h" > > Instead of having dependency on another test's header, > >> #ifndef ARRAY_SIZE >> #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) >> @@ -22,6 +24,7 @@ int nr_active; >> int nr_connect; >> int nr_binddev; >> int nr_socket_post_create; >> +int nr_fin_wait1; >> struct sockopt_test { >> int opt; >> @@ -386,6 +389,11 @@ int skops_sockopt(struct bpf_sock_ops *skops) >> nr_passive += !(bpf_test_sockopt(skops, sk) || >> test_tcp_maxseg(skops, sk) || >> test_tcp_saved_syn(skops, sk)); >> + set_hdr_cb_flags(skops, BPF_SOCK_OPS_STATE_CB_FLAG); > > how about directly doing this: > bpf_sock_ops_cb_flags_set(skops, > skops->bpf_sock_ops_cb_flags | > BPF_SOCK_OPS_STATE_CB_FLAG); Sure! It makes sense. > >> + break; >> + case BPF_SOCK_OPS_STATE_CB: >> + if (skops->args[1] == BPF_TCP_CLOSE_WAIT) >> + nr_fin_wait1 += !bpf_test_sockopt(skops, sk); >> break; >> } >
diff --git a/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c b/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c index 018611e6b248..7bf3736b7286 100644 --- a/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c +++ b/tools/testing/selftests/bpf/prog_tests/setget_sockopt.c @@ -4,6 +4,7 @@ #define _GNU_SOURCE #include <sched.h> #include <linux/socket.h> +#include <linux/tls.h> #include <net/if.h> #include "test_progs.h" @@ -83,6 +84,73 @@ static void test_udp(int family) ASSERT_EQ(bss->nr_binddev, 1, "nr_bind"); } +static void test_ktls(int family) +{ + struct tls12_crypto_info_aes_gcm_128 aes128; + struct setget_sockopt__bss *bss = skel->bss; + int cfd = -1, sfd = -1, fd = -1, ret; + + memset(bss, 0, sizeof(*bss)); + + sfd = start_server(family, SOCK_STREAM, + family == AF_INET6 ? addr6_str : addr4_str, 0, 0); + if (!ASSERT_GE(sfd, 0, "start_server")) + return; + fd = connect_to_fd(sfd, 0); + if (!ASSERT_GE(fd, 0, "connect_to_fd")) + goto err_out; + + cfd = accept(sfd, NULL, 0); + if (!ASSERT_GE(cfd, 0, "accept")) + goto err_out; + + close(sfd); + sfd = -1; + + /* Setup KTLS */ + ret = setsockopt(fd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls")); + if (!ASSERT_OK(ret, "setsockopt")) + goto err_out; + ret = setsockopt(cfd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls")); + if (!ASSERT_OK(ret, "setsockopt")) + goto err_out; + + memset(&aes128, 0, sizeof(aes128)); + aes128.info.version = TLS_1_2_VERSION; + aes128.info.cipher_type = TLS_CIPHER_AES_GCM_128; + + ret = setsockopt(fd, SOL_TLS, TLS_TX, &aes128, sizeof(aes128)); + if (!ASSERT_OK(ret, "setsockopt")) + goto err_out; + + ret = setsockopt(cfd, SOL_TLS, TLS_RX, &aes128, sizeof(aes128)); + if (!ASSERT_OK(ret, "setsockopt")) + goto err_out; + + /* KTLS is enabled */ + + close(fd); + /* At this point, the cfd socket is at the CLOSE_WAIT state + * and still run TLS protocol. The test for + * BPF_TCP_CLOSE_WAIT should be run at this point. + */ + close(cfd); + + ASSERT_EQ(bss->nr_listen, 1, "nr_listen"); + ASSERT_EQ(bss->nr_connect, 1, "nr_connect"); + ASSERT_EQ(bss->nr_active, 1, "nr_active"); + ASSERT_EQ(bss->nr_passive, 1, "nr_passive"); + ASSERT_EQ(bss->nr_socket_post_create, 2, "nr_socket_post_create"); + ASSERT_EQ(bss->nr_binddev, 2, "nr_bind"); + ASSERT_EQ(bss->nr_fin_wait1, 1, "nr_fin_wait1"); + return; + +err_out: + close(fd); + close(cfd); + close(sfd); +} + void test_setget_sockopt(void) { cg_fd = test__join_cgroup(CG_NAME); @@ -118,6 +186,8 @@ void test_setget_sockopt(void) test_tcp(AF_INET); test_udp(AF_INET6); test_udp(AF_INET); + test_ktls(AF_INET6); + test_ktls(AF_INET); done: setget_sockopt__destroy(skel); diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt.c b/tools/testing/selftests/bpf/progs/setget_sockopt.c index 9523333b8905..027d95755f9f 100644 --- a/tools/testing/selftests/bpf/progs/setget_sockopt.c +++ b/tools/testing/selftests/bpf/progs/setget_sockopt.c @@ -6,6 +6,8 @@ #include <bpf/bpf_core_read.h> #include <bpf/bpf_helpers.h> #include <bpf/bpf_tracing.h> +#define BPF_PROG_TEST_TCP_HDR_OPTIONS +#include "test_tcp_hdr_options.h" #ifndef ARRAY_SIZE #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) @@ -22,6 +24,7 @@ int nr_active; int nr_connect; int nr_binddev; int nr_socket_post_create; +int nr_fin_wait1; struct sockopt_test { int opt; @@ -386,6 +389,11 @@ int skops_sockopt(struct bpf_sock_ops *skops) nr_passive += !(bpf_test_sockopt(skops, sk) || test_tcp_maxseg(skops, sk) || test_tcp_saved_syn(skops, sk)); + set_hdr_cb_flags(skops, BPF_SOCK_OPS_STATE_CB_FLAG); + break; + case BPF_SOCK_OPS_STATE_CB: + if (skops->args[1] == BPF_TCP_CLOSE_WAIT) + nr_fin_wait1 += !bpf_test_sockopt(skops, sk); break; }
Ensures that whenever bpf_setsockopt() is called with the SOL_TCP option on a ktls enabled socket, the call will be accepted by the system. The provided test makes sure of this by performing an examination when the server side socket is in the CLOSE_WAIT state. At this stage, ktls is still enabled on the server socket and can be used to test if bpf_setsockopt() works correctly with linux. Signed-off-by: Kui-Feng Lee <kuifeng@meta.com> --- .../selftests/bpf/prog_tests/setget_sockopt.c | 70 +++++++++++++++++++ .../selftests/bpf/progs/setget_sockopt.c | 8 +++ 2 files changed, 78 insertions(+)