| Message ID | 20220622172407.411411-1-jakub@cloudflare.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] selftests/bpf: Test sockmap update when socket has ULP | expand |
Jakub Sitnicki wrote: > Cover the scenario when we cannot insert a socket into the sockmap, because > it has it is using ULP. Failed insert should not have any effect on the ULP > state. This is a regression test. > > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > --- Thanks, looks good. One small nit. > > +#include <netinet/tcp.h> > #include "test_progs.h" > > #define MAX_TEST_NAME 80 > @@ -92,9 +93,78 @@ static void test_sockmap_ktls_disconnect_after_delete(int family, int map) > close(srv); > } > > +static void test_sockmap_ktls_update_fails_when_sock_has_ulp(int family, int map) > +{ > + struct sockaddr_storage addr = {}; > + socklen_t len = sizeof(addr); > + struct sockaddr_in6 *v6; > + struct sockaddr_in *v4; > + int err, s, zero = 0; > + > + s = socket(family, SOCK_STREAM, 0); > + if (!ASSERT_GE(s, 0, "socket")) > + return; > + > + switch (family) { > + case AF_INET: > + v4 = (struct sockaddr_in *)&addr; > + v4->sin_family = AF_INET; > + break; > + case AF_INET6: > + v6 = (struct sockaddr_in6 *)&addr; > + v6->sin6_family = AF_INET6; >k+ break; > + default: > + PRINT_FAIL("unsupported socket family %d", family); Probably want goto close here right? > + return; > + } > + > + err = bind(s, (struct sockaddr *)&addr, len); > + if (!ASSERT_OK(err, "bind")) > + goto close; > + > + err = getsockname(s, (struct sockaddr *)&addr, &len); > + if (!ASSERT_OK(err, "getsockname")) > + goto close; > + > + err = connect(s, (struct sockaddr *)&addr, len); > + if (!ASSERT_OK(err, "connect")) > + goto close; > + > + /* save sk->sk_prot and set it to tls_prots */ > + err = setsockopt(s, IPPROTO_TCP, TCP_ULP, "tls", strlen("tls")); > + if (!ASSERT_OK(err, "setsockopt(TCP_ULP)")) > + goto close; > + > + /* sockmap update should not affect saved sk_prot */ > + err = bpf_map_update_elem(map, &zero, &s, BPF_ANY); > + if (!ASSERT_ERR(err, "sockmap update elem")) > + goto close; > + > + /* call sk->sk_prot->setsockopt to dispatch to saved sk_prot */ > + err = setsockopt(s, IPPROTO_TCP, TCP_NODELAY, &zero, sizeof(zero)); > + ASSERT_OK(err, "setsockopt(TCP_NODELAY)"); > + > +close: > + close(s);
On Wed, Jun 22, 2022 at 10:42 PM -07, John Fastabend wrote: > Jakub Sitnicki wrote: >> Cover the scenario when we cannot insert a socket into the sockmap, because >> it has it is using ULP. Failed insert should not have any effect on the ULP >> state. This is a regression test. >> >> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> >> --- > > Thanks, looks good. One small nit. > >> >> +#include <netinet/tcp.h> >> #include "test_progs.h" >> >> #define MAX_TEST_NAME 80 >> @@ -92,9 +93,78 @@ static void test_sockmap_ktls_disconnect_after_delete(int family, int map) >> close(srv); >> } >> >> +static void test_sockmap_ktls_update_fails_when_sock_has_ulp(int family, int map) >> +{ >> + struct sockaddr_storage addr = {}; >> + socklen_t len = sizeof(addr); >> + struct sockaddr_in6 *v6; >> + struct sockaddr_in *v4; >> + int err, s, zero = 0; >> + >> + s = socket(family, SOCK_STREAM, 0); >> + if (!ASSERT_GE(s, 0, "socket")) >> + return; >> + >> + switch (family) { >> + case AF_INET: >> + v4 = (struct sockaddr_in *)&addr; >> + v4->sin_family = AF_INET; >> + break; >> + case AF_INET6: >> + v6 = (struct sockaddr_in6 *)&addr; >> + v6->sin6_family = AF_INET6; >>k+ break; >> + default: >> + PRINT_FAIL("unsupported socket family %d", family); > > Probably want goto close here right? Ah, thanks. Sent v2. I hope we can borrow a trick from systemd's book and adapt __attribute__((cleanup(f))) in the future. [...]
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c b/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c index af293ea1542c..86b0741d2464 100644 --- a/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c @@ -4,6 +4,7 @@ * Tests for sockmap/sockhash holding kTLS sockets. */ +#include <netinet/tcp.h> #include "test_progs.h" #define MAX_TEST_NAME 80 @@ -92,9 +93,78 @@ static void test_sockmap_ktls_disconnect_after_delete(int family, int map) close(srv); } +static void test_sockmap_ktls_update_fails_when_sock_has_ulp(int family, int map) +{ + struct sockaddr_storage addr = {}; + socklen_t len = sizeof(addr); + struct sockaddr_in6 *v6; + struct sockaddr_in *v4; + int err, s, zero = 0; + + s = socket(family, SOCK_STREAM, 0); + if (!ASSERT_GE(s, 0, "socket")) + return; + + switch (family) { + case AF_INET: + v4 = (struct sockaddr_in *)&addr; + v4->sin_family = AF_INET; + break; + case AF_INET6: + v6 = (struct sockaddr_in6 *)&addr; + v6->sin6_family = AF_INET6; + break; + default: + PRINT_FAIL("unsupported socket family %d", family); + return; + } + + err = bind(s, (struct sockaddr *)&addr, len); + if (!ASSERT_OK(err, "bind")) + goto close; + + err = getsockname(s, (struct sockaddr *)&addr, &len); + if (!ASSERT_OK(err, "getsockname")) + goto close; + + err = connect(s, (struct sockaddr *)&addr, len); + if (!ASSERT_OK(err, "connect")) + goto close; + + /* save sk->sk_prot and set it to tls_prots */ + err = setsockopt(s, IPPROTO_TCP, TCP_ULP, "tls", strlen("tls")); + if (!ASSERT_OK(err, "setsockopt(TCP_ULP)")) + goto close; + + /* sockmap update should not affect saved sk_prot */ + err = bpf_map_update_elem(map, &zero, &s, BPF_ANY); + if (!ASSERT_ERR(err, "sockmap update elem")) + goto close; + + /* call sk->sk_prot->setsockopt to dispatch to saved sk_prot */ + err = setsockopt(s, IPPROTO_TCP, TCP_NODELAY, &zero, sizeof(zero)); + ASSERT_OK(err, "setsockopt(TCP_NODELAY)"); + +close: + close(s); +} + +static const char *fmt_test_name(const char *subtest_name, int family, + enum bpf_map_type map_type) +{ + const char *map_type_str = BPF_MAP_TYPE_SOCKMAP ? "SOCKMAP" : "SOCKHASH"; + const char *family_str = AF_INET ? "IPv4" : "IPv6"; + static char test_name[MAX_TEST_NAME]; + + snprintf(test_name, MAX_TEST_NAME, + "sockmap_ktls %s %s %s", + subtest_name, family_str, map_type_str); + + return test_name; +} + static void run_tests(int family, enum bpf_map_type map_type) { - char test_name[MAX_TEST_NAME]; int map; map = bpf_map_create(map_type, NULL, sizeof(int), sizeof(int), 1, NULL); @@ -103,14 +173,10 @@ static void run_tests(int family, enum bpf_map_type map_type) return; } - snprintf(test_name, MAX_TEST_NAME, - "sockmap_ktls disconnect_after_delete %s %s", - family == AF_INET ? "IPv4" : "IPv6", - map_type == BPF_MAP_TYPE_SOCKMAP ? "SOCKMAP" : "SOCKHASH"); - if (!test__start_subtest(test_name)) - return; - - test_sockmap_ktls_disconnect_after_delete(family, map); + if (test__start_subtest(fmt_test_name("disconnect_after_delete", family, map_type))) + test_sockmap_ktls_disconnect_after_delete(family, map); + if (test__start_subtest(fmt_test_name("update_fails_when_sock_has_ulp", family, map_type))) + test_sockmap_ktls_update_fails_when_sock_has_ulp(family, map); close(map); }
Cover the scenario when we cannot insert a socket into the sockmap, because it has it is using ULP. Failed insert should not have any effect on the ULP state. This is a regression test. Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> --- CC: john.fastabend@gmail.com CC: jakub@cloudflare.com CC: yoshfuji@linux-ipv6.org CC: dsahern@kernel.org CC: ast@kernel.org CC: daniel@iogearbox.net CC: andrii@kernel.org CC: kafai@fb.com CC: songliubraving@fb.com CC: yhs@fb.com CC: kpsingh@kernel.org CC: borisp@nvidia.com CC: cong.wang@bytedance.com CC: bpf@vger.kernel.org --- .../selftests/bpf/prog_tests/sockmap_ktls.c | 84 +++++++++++++++++-- 1 file changed, 75 insertions(+), 9 deletions(-)