* [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs
@ 2024-01-24 18:53 John Fastabend
2024-01-24 18:54 ` [PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper John Fastabend
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: John Fastabend @ 2024-01-24 18:53 UTC (permalink / raw)
To: jakub, bpf; +Cc: netdev, john.fastabend, andrii
Its much easier to write and read tests than it was when sockmap was
originally created. At that time we created a test_sockmap prog that
did sockmap tests. But, its showing its age now. For example it reads
user vars out of maps, is hard to run targetted tests, has a different
format from the familiar test_progs and so on.
I recently thought there was an issue with pop helpers so I created
some tests to try and track it down. It turns out it was a bug in the
BPF program we had not the kernel. But, I think it makes sense to
start deprecating test_sockmap and converting these to the nicer
test_progs.
So this is a first round of test_prog tests for sockmap cork and
pop helpers. I'll add push and pull tests shortly. I think its fine,
maybe preferred to review smaller patchsets, to send these
incrementally as I get them created.
Thanks!
v2: fix unint vars in some branches from `make RELEASE=1`
John Fastabend (4):
bpf: sockmap, add test for sk_msg prog pop msg helper
bpf: sockmap, add a sendmsg test so we can check that path
bpf: sockmap, add a cork to force buffering of the scatterlist
bpf: sockmap test cork and pop combined
.../bpf/prog_tests/sockmap_helpers.h | 18 +
.../bpf/prog_tests/sockmap_msg_helpers.c | 353 ++++++++++++++++++
.../bpf/progs/test_sockmap_msg_helpers.c | 67 ++++
3 files changed, 438 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
--
2.33.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper
2024-01-24 18:53 [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs John Fastabend
@ 2024-01-24 18:54 ` John Fastabend
2024-01-26 11:48 ` Jakub Sitnicki
2024-01-24 18:54 ` [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path John Fastabend
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: John Fastabend @ 2024-01-24 18:54 UTC (permalink / raw)
To: jakub, bpf; +Cc: netdev, john.fastabend, andrii
For msg_pop sk_msg helpers we only have older tests in test_sockmap, but
these are showing their age. They don't use any of the newer style BPF
and also require running test_sockmap. Lets use the prog_test framework
and add a test for msg_pop.
This is a much nicer test env using newer style BPF. We can
extend this to support all the other helpers shortly.
The bpf program is a template that lets us run through all the helpers
so we can cover not just pop, but all the other helpers as well.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
.../bpf/prog_tests/sockmap_helpers.h | 10 +
.../bpf/prog_tests/sockmap_msg_helpers.c | 210 ++++++++++++++++++
.../bpf/progs/test_sockmap_msg_helpers.c | 52 +++++
3 files changed, 272 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
index e880f97bc44d..781cbdf01d7b 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
@@ -112,6 +112,16 @@
__ret; \
})
+#define xrecv_nonblock(fd, buf, len, flags) \
+ ({ \
+ ssize_t __ret = recv_timeout((fd), (buf), (len), (flags), \
+ IO_TIMEOUT_SEC); \
+ if (__ret == -1) \
+ FAIL_ERRNO("recv"); \
+ __ret; \
+ })
+
+
#define xsocket(family, sotype, flags) \
({ \
int __ret = socket(family, sotype, flags); \
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
new file mode 100644
index 000000000000..9ffe02f45808
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020 Cloudflare
+#include <error.h>
+#include <netinet/tcp.h>
+#include <sys/epoll.h>
+
+#include "test_progs.h"
+#include "test_sockmap_msg_helpers.skel.h"
+#include "sockmap_helpers.h"
+
+#define TCP_REPAIR 19 /* TCP sock is under repair right now */
+
+#define TCP_REPAIR_ON 1
+#define TCP_REPAIR_OFF_NO_WP -1 /* Turn off without window probes */
+
+struct msg_test_opts {
+ struct test_sockmap_msg_helpers *skel;
+ int server;
+ int client;
+};
+
+#define POP_END -1
+
+static void pop_simple_send(struct msg_test_opts *opts, int start, int len)
+{
+ struct test_sockmap_msg_helpers *skel = opts->skel;
+ char buf[] = "abcdefghijklmnopqrstuvwxyz";
+ char recvbuf[sizeof(buf)];
+ size_t sent, recv, cmp;
+
+ skel->bss->pop = true;
+
+ if (start == -1)
+ start = sizeof(buf) - len - 1;
+
+ skel->bss->pop_start = start;
+ skel->bss->pop_len = len;
+
+ sent = xsend(opts->client, buf, sizeof(buf), 0);
+ if (sent < sizeof(buf))
+ FAIL("xsend failed");
+
+ ASSERT_OK(skel->bss->err, "pop error");
+
+ recv = xrecv_nonblock(opts->server, recvbuf, sizeof(buf), 0);
+ if (recv != sent - skel->bss->pop_len)
+ FAIL("Received incorrect number number of bytes after pop");
+
+ cmp = memcmp(&buf[0], &recvbuf[0], start);
+ ASSERT_OK(cmp, "pop cmp start bytes failed");
+ cmp = memcmp(&buf[start+len], &recvbuf[start], sizeof(buf) - start - len);
+ ASSERT_OK(cmp, "pop cmp end bytes failed");
+}
+
+static void test_sockmap_pop(void)
+{
+ struct msg_test_opts opts;
+ struct test_sockmap_msg_helpers *skel;
+ int s, client, server;
+ int err, map, prog;
+
+ skel = test_sockmap_msg_helpers__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+
+ map = bpf_map__fd(skel->maps.sock_map);
+ prog = bpf_program__fd(skel->progs.msg_helpers);
+ err = bpf_prog_attach(prog, map, BPF_SK_MSG_VERDICT, 0);
+ if (!ASSERT_OK(err, "bpf_prog_attach"))
+ goto out;
+
+ s = socket_loopback(AF_INET, SOCK_STREAM);
+ if (s < 0)
+ goto out;
+
+ err = create_pair(s, AF_INET, SOCK_STREAM, &client, &server);
+ if (err < 0)
+ goto close_loopback;
+
+ err = add_to_sockmap(map, client, server);
+ if (err < 0)
+ goto close_sockets;
+
+ opts.client = client;
+ opts.server = server;
+ opts.skel = skel;
+
+ /* Pop from start */
+ pop_simple_send(&opts, 0, 5);
+ /* Pop from the middle */
+ pop_simple_send(&opts, 10, 5);
+ /* Pop from end */
+ pop_simple_send(&opts, POP_END, 5);
+
+close_sockets:
+ close(client);
+ close(server);
+close_loopback:
+ close(s);
+out:
+ test_sockmap_msg_helpers__destroy(skel);
+}
+
+static void test_sockmap_pop_errors(void)
+{
+ char buf[] = "abcdefghijklmnopqrstuvwxyz";
+ struct test_sockmap_msg_helpers *skel;
+ int i, recv, err, map, prog;
+ char recvbuf[sizeof(buf)];
+ int s, client, server;
+
+ skel = test_sockmap_msg_helpers__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+
+ map = bpf_map__fd(skel->maps.sock_map);
+ prog = bpf_program__fd(skel->progs.msg_helpers);
+ err = bpf_prog_attach(prog, map, BPF_SK_MSG_VERDICT, 0);
+ if (!ASSERT_OK(err, "bpf_prog_attach"))
+ goto out;
+
+ s = socket_loopback(AF_INET, SOCK_STREAM);
+ if (s < 0)
+ goto out;
+
+ err = create_pair(s, AF_INET, SOCK_STREAM, &client, &server);
+ if (err < 0)
+ goto close_loopback;
+
+ err = add_to_sockmap(map, client, server);
+ if (err < 0)
+ goto close_sockets;
+
+ skel->bss->pop = true;
+
+ /* Pop larger than buffer */
+ skel->bss->pop_start = 0;
+ skel->bss->pop_len = sizeof(buf) + 1;
+ xsend(client, buf, sizeof(buf), 0);
+ ASSERT_ERR(skel->bss->err, "popping more bytes than msg did not throw an error");
+ xrecv_nonblock(server, recvbuf, sizeof(recvbuf), 0);
+
+ /* Pop past end of buffer */
+ skel->bss->pop_start = sizeof(buf) - 5;
+ skel->bss->pop_len = 10;
+ xsend(client, buf, sizeof(buf), 0);
+ ASSERT_ERR(skel->bss->err, "popping past end of msg did not throw an error");
+ xrecv_nonblock(server, recvbuf, sizeof(recvbuf), 0);
+
+ /* Pop larger than buffer on complex send */
+ skel->bss->pop_start = 0;
+ skel->bss->pop_len = 0;
+ for (i = 0; i < 14; i++)
+ xsend(client, buf, sizeof(buf), MSG_MORE);
+ skel->bss->pop_start = 0;
+ skel->bss->pop_len = sizeof(buf) * 32;
+ xsend(client, buf, sizeof(buf), MSG_MORE);
+ ASSERT_ERR(skel->bss->err, "popping more bytes than sg msg did not throw an error");
+ i = 0;
+ do {
+ i++;
+ recv = xrecv_nonblock(server, recvbuf, sizeof(recvbuf), 0);
+ } while (recv > 0 && i < 15);
+
+ /* Pop past end of complex send */
+ skel->bss->pop_start = 0;
+ skel->bss->pop_len = 0;
+ for (i = 0; i < 14; i++)
+ xsend(client, buf, sizeof(buf), MSG_MORE);
+ skel->bss->pop_start = sizeof(buf) * 14;
+ skel->bss->pop_len = sizeof(buf) + 1;
+ xsend(client, buf, sizeof(buf), MSG_MORE);
+ ASSERT_ERR(skel->bss->err, "popping past end of sg msg did not throw an error");
+ i = 0;
+ do {
+ i++;
+ recv = xrecv_nonblock(server, recvbuf, sizeof(recvbuf), 0);
+ } while (recv > 0 && i < 15);
+
+ /* Pop past end of complex send starting in middle of last sg */
+ skel->bss->pop_start = 0;
+ skel->bss->pop_len = 0;
+ for (i = 0; i < 14; i++)
+ xsend(client, buf, sizeof(buf), MSG_MORE);
+ skel->bss->pop_start = (sizeof(buf) * 14) + sizeof(buf) - 5;
+ skel->bss->pop_len = 10;
+ xsend(client, buf, sizeof(buf), MSG_MORE);
+ ASSERT_ERR(skel->bss->err, "popping past end from offset of sg msg did not throw an error");
+ i = 0;
+ do {
+ i++;
+ recv = xrecv_nonblock(server, recvbuf, sizeof(recvbuf), 0);
+ } while (recv > 0 && i < 15);
+
+close_sockets:
+ close(client);
+ close(server);
+close_loopback:
+ close(s);
+out:
+ test_sockmap_msg_helpers__destroy(skel);
+}
+
+void test_sockmap_msg_helpers(void)
+{
+ if (test__start_subtest("sockmap pop"))
+ test_sockmap_pop();
+ if (test__start_subtest("sockmap pop errors"))
+ test_sockmap_pop_errors();
+}
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c b/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
new file mode 100644
index 000000000000..c721a00b6001
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020 Cloudflare
+
+#include <errno.h>
+#include <stdbool.h>
+#include <linux/bpf.h>
+
+#include <bpf/bpf_helpers.h>
+
+struct {
+ __uint(type, BPF_MAP_TYPE_SOCKMAP);
+ __uint(max_entries, 2);
+ __type(key, __u32);
+ __type(value, __u64);
+} sock_map SEC(".maps");
+
+int cork = 0;
+
+bool pull = false;
+bool push = false;
+bool pop = false;
+
+int pull_start = 0;
+int pull_end = 0;
+
+int push_start = 0;
+int push_end = 0;
+
+int pop_start = 0;
+int pop_len = 0;
+
+int err;
+
+SEC("sk_msg")
+int msg_helpers(struct sk_msg_md *msg)
+{
+ if (cork)
+ err = bpf_msg_cork_bytes(msg, cork);
+
+ if (pull)
+ err = bpf_msg_pull_data(msg, pull_start, pull_end, 0);
+
+ if (push)
+ err = bpf_msg_push_data(msg, push_start, push_end, 0);
+
+ if (pop)
+ err = bpf_msg_pop_data(msg, pop_start, pop_len, 0);
+
+ return SK_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path
2024-01-24 18:53 [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs John Fastabend
2024-01-24 18:54 ` [PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper John Fastabend
@ 2024-01-24 18:54 ` John Fastabend
2024-01-26 12:17 ` Jakub Sitnicki
2024-01-24 18:54 ` [PATCH bpf-next v2 3/4] bpf: sockmap, add a cork to force buffering of the scatterlist John Fastabend
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: John Fastabend @ 2024-01-24 18:54 UTC (permalink / raw)
To: jakub, bpf; +Cc: netdev, john.fastabend, andrii
Sendmsg path with multiple buffers is slightly different from a single
send in how we have to handle and walk the sg when doing pops. Lets
ensure this walk is correct.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
.../bpf/prog_tests/sockmap_helpers.h | 8 +++
.../bpf/prog_tests/sockmap_msg_helpers.c | 53 +++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
index 781cbdf01d7b..4d8d24482032 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
@@ -103,6 +103,14 @@
__ret; \
})
+#define xsendmsg(fd, msg, flags) \
+ ({ \
+ ssize_t __ret = sendmsg((fd), (msg), (flags)); \
+ if (__ret == -1) \
+ FAIL_ERRNO("sendmsg"); \
+ __ret; \
+ })
+
#define xrecv_nonblock(fd, buf, len, flags) \
({ \
ssize_t __ret = recv_timeout((fd), (buf), (len), (flags), \
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
index 9ffe02f45808..e5e618e84950 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
@@ -52,6 +52,50 @@ static void pop_simple_send(struct msg_test_opts *opts, int start, int len)
ASSERT_OK(cmp, "pop cmp end bytes failed");
}
+static void pop_complex_send(struct msg_test_opts *opts, int start, int len)
+{
+ struct test_sockmap_msg_helpers *skel = opts->skel;
+ char buf[] = "abcdefghijklmnopqrstuvwxyz";
+ size_t sent, recv, total = 0;
+ struct msghdr msg = {0};
+ struct iovec iov[15];
+ char *recvbuf;
+ int i;
+
+ for (i = 0; i < 15; i++) {
+ iov[i].iov_base = buf;
+ iov[i].iov_len = sizeof(buf);
+ total += sizeof(buf);
+ }
+
+ recvbuf = malloc(total);
+ if (!recvbuf)
+ FAIL("pop complex send malloc failure\n");
+
+ msg.msg_iov = iov;
+ msg.msg_iovlen = 15;
+
+ skel->bss->pop = true;
+
+ if (start == -1)
+ start = sizeof(buf) - len - 1;
+
+ skel->bss->pop_start = start;
+ skel->bss->pop_len = len;
+
+ sent = xsendmsg(opts->client, &msg, 0);
+ if (sent != total)
+ FAIL("xsend failed");
+
+ ASSERT_OK(skel->bss->err, "pop error");
+
+ recv = xrecv_nonblock(opts->server, recvbuf, total, 0);
+ if (recv != sent - skel->bss->pop_len)
+ FAIL("Received incorrect number number of bytes after pop");
+
+ free(recvbuf);
+}
+
static void test_sockmap_pop(void)
{
struct msg_test_opts opts;
@@ -92,6 +136,15 @@ static void test_sockmap_pop(void)
/* Pop from end */
pop_simple_send(&opts, POP_END, 5);
+ /* Empty pop from start of sendmsg */
+ pop_complex_send(&opts, 0, 0);
+ /* Pop from start of sendmsg */
+ pop_complex_send(&opts, 0, 10);
+ /* Pop from middle of sendmsg */
+ pop_complex_send(&opts, 100, 10);
+ /* Pop from end of sendmsg */
+ pop_complex_send(&opts, 394, 10);
+
close_sockets:
close(client);
close(server);
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH bpf-next v2 3/4] bpf: sockmap, add a cork to force buffering of the scatterlist
2024-01-24 18:53 [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs John Fastabend
2024-01-24 18:54 ` [PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper John Fastabend
2024-01-24 18:54 ` [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path John Fastabend
@ 2024-01-24 18:54 ` John Fastabend
2024-01-26 14:19 ` Jakub Sitnicki
2024-01-24 18:54 ` [PATCH bpf-next v2 4/4] bpf: sockmap test cork and pop combined John Fastabend
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: John Fastabend @ 2024-01-24 18:54 UTC (permalink / raw)
To: jakub, bpf; +Cc: netdev, john.fastabend, andrii
By using cork we can force multiple sends into a single scatterlist
and test that first the cork gives us the correct number of bytes,
but then also test the pop over the corked data.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
.../bpf/prog_tests/sockmap_msg_helpers.c | 81 +++++++++++++++++++
.../bpf/progs/test_sockmap_msg_helpers.c | 3 +
2 files changed, 84 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
index e5e618e84950..8ced54fe1a0b 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
@@ -21,6 +21,85 @@ struct msg_test_opts {
#define POP_END -1
+static void cork_send(struct msg_test_opts *opts, int cork)
+{
+ struct test_sockmap_msg_helpers *skel = opts->skel;
+ char buf[] = "abcdefghijklmnopqrstuvwxyz";
+ size_t sent, total = 0, recv;
+ char *recvbuf;
+ int i;
+
+ skel->bss->pop = false;
+ skel->bss->cork = cork;
+
+ /* Send N bytes in 27B chunks */
+ for (i = 0; i < cork / sizeof(buf); i++) {
+ sent = xsend(opts->client, buf, sizeof(buf), 0);
+ if (sent < sizeof(buf))
+ FAIL("xsend failed");
+ total += sent;
+ }
+
+ recvbuf = malloc(total);
+ if (!recvbuf)
+ FAIL("cork send malloc failure\n");
+
+ ASSERT_OK(skel->bss->err, "cork error");
+ ASSERT_EQ(skel->bss->size, cork, "cork did not receive all bytes");
+
+ recv = xrecv_nonblock(opts->server, recvbuf, total, 0);
+ if (recv != total)
+ FAIL("Received incorrect number of bytes");
+
+ free(recvbuf);
+}
+
+static void test_sockmap_cork()
+{
+ struct test_sockmap_msg_helpers *skel;
+ struct msg_test_opts opts;
+ int s, client, server;
+ int err, map, prog;
+
+ skel = test_sockmap_msg_helpers__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+
+ map = bpf_map__fd(skel->maps.sock_map);
+ prog = bpf_program__fd(skel->progs.msg_helpers);
+ err = bpf_prog_attach(prog, map, BPF_SK_MSG_VERDICT, 0);
+ if (!ASSERT_OK(err, "bpf_prog_attach"))
+ goto out;
+
+ s = socket_loopback(AF_INET, SOCK_STREAM);
+ if (s < 0)
+ goto out;
+
+ err = create_pair(s, AF_INET, SOCK_STREAM, &client, &server);
+ if (err < 0)
+ goto close_loopback;
+
+ err = add_to_sockmap(map, client, server);
+ if (err < 0)
+ goto close_sockets;
+
+ opts.client = client;
+ opts.server = server;
+ opts.skel = skel;
+
+ /* Small cork */
+ cork_send(&opts, 54);
+ /* Full cork */
+ cork_send(&opts, 270);
+close_sockets:
+ close(client);
+ close(server);
+close_loopback:
+ close(s);
+out:
+ test_sockmap_msg_helpers__destroy(skel);
+}
+
static void pop_simple_send(struct msg_test_opts *opts, int start, int len)
{
struct test_sockmap_msg_helpers *skel = opts->skel;
@@ -260,4 +339,6 @@ void test_sockmap_msg_helpers(void)
test_sockmap_pop();
if (test__start_subtest("sockmap pop errors"))
test_sockmap_pop_errors();
+ if (test__start_subtest("sockmap cork"))
+ test_sockmap_cork();
}
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c b/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
index c721a00b6001..9622f154d016 100644
--- a/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
@@ -30,10 +30,13 @@ int pop_start = 0;
int pop_len = 0;
int err;
+int size;
SEC("sk_msg")
int msg_helpers(struct sk_msg_md *msg)
{
+ size = msg->size;
+
if (cork)
err = bpf_msg_cork_bytes(msg, cork);
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH bpf-next v2 4/4] bpf: sockmap test cork and pop combined
2024-01-24 18:53 [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs John Fastabend
` (2 preceding siblings ...)
2024-01-24 18:54 ` [PATCH bpf-next v2 3/4] bpf: sockmap, add a cork to force buffering of the scatterlist John Fastabend
@ 2024-01-24 18:54 ` John Fastabend
2024-01-24 22:58 ` [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs John Fastabend
2024-01-26 14:39 ` Jakub Sitnicki
5 siblings, 0 replies; 15+ messages in thread
From: John Fastabend @ 2024-01-24 18:54 UTC (permalink / raw)
To: jakub, bpf; +Cc: netdev, john.fastabend, andrii
Its possible to cork data for some N bytes and then pop
a some bytes off that scatterlist. Test combining cork
and pop here.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
.../bpf/prog_tests/sockmap_msg_helpers.c | 19 ++++++++++++++-----
.../bpf/progs/test_sockmap_msg_helpers.c | 14 +++++++++++++-
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
index 8ced54fe1a0b..cfb965f6832f 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
@@ -21,7 +21,7 @@ struct msg_test_opts {
#define POP_END -1
-static void cork_send(struct msg_test_opts *opts, int cork)
+static void cork_send(struct msg_test_opts *opts, int cork, int start, int len)
{
struct test_sockmap_msg_helpers *skel = opts->skel;
char buf[] = "abcdefghijklmnopqrstuvwxyz";
@@ -29,9 +29,12 @@ static void cork_send(struct msg_test_opts *opts, int cork)
char *recvbuf;
int i;
- skel->bss->pop = false;
+ skel->bss->pop = !!len;
skel->bss->cork = cork;
+ skel->bss->pop_start = start;
+ skel->bss->pop_len = len;
+
/* Send N bytes in 27B chunks */
for (i = 0; i < cork / sizeof(buf); i++) {
sent = xsend(opts->client, buf, sizeof(buf), 0);
@@ -48,7 +51,7 @@ static void cork_send(struct msg_test_opts *opts, int cork)
ASSERT_EQ(skel->bss->size, cork, "cork did not receive all bytes");
recv = xrecv_nonblock(opts->server, recvbuf, total, 0);
- if (recv != total)
+ if (recv != total - len)
FAIL("Received incorrect number of bytes");
free(recvbuf);
@@ -88,9 +91,15 @@ static void test_sockmap_cork()
opts.skel = skel;
/* Small cork */
- cork_send(&opts, 54);
+ cork_send(&opts, 54, 0, 0);
/* Full cork */
- cork_send(&opts, 270);
+ cork_send(&opts, 270, 0, 0);
+
+ /* Combine cork and pop small */
+ cork_send(&opts, 54, 0, 10);
+ /* Full cork and pop */
+ cork_send(&opts, 270, 200, 50);
+
close_sockets:
close(client);
close(server);
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c b/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
index 9622f154d016..4c7e70367e35 100644
--- a/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
@@ -37,8 +37,19 @@ int msg_helpers(struct sk_msg_md *msg)
{
size = msg->size;
- if (cork)
+ /* If message is not yet fully cork'ed skip push, pull, pop */
+ if (cork && cork > msg->size) {
err = bpf_msg_cork_bytes(msg, cork);
+ goto out;
+ } else if (cork) {
+ /* If we previously corked the msg we need to clear the cork
+ * otherwise next pop would cause datapath to wait for the
+ * popped bytes to actually do the send.
+ */
+ err = bpf_msg_cork_bytes(msg, 0);
+ if (err)
+ goto out;
+ }
if (pull)
err = bpf_msg_pull_data(msg, pull_start, pull_end, 0);
@@ -49,6 +60,7 @@ int msg_helpers(struct sk_msg_md *msg)
if (pop)
err = bpf_msg_pop_data(msg, pop_start, pop_len, 0);
+out:
return SK_PASS;
}
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* RE: [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs
2024-01-24 18:53 [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs John Fastabend
` (3 preceding siblings ...)
2024-01-24 18:54 ` [PATCH bpf-next v2 4/4] bpf: sockmap test cork and pop combined John Fastabend
@ 2024-01-24 22:58 ` John Fastabend
2024-01-26 14:39 ` Jakub Sitnicki
5 siblings, 0 replies; 15+ messages in thread
From: John Fastabend @ 2024-01-24 22:58 UTC (permalink / raw)
To: John Fastabend, jakub, bpf; +Cc: netdev, john.fastabend, andrii
John Fastabend wrote:
> Its much easier to write and read tests than it was when sockmap was
> originally created. At that time we created a test_sockmap prog that
> did sockmap tests. But, its showing its age now. For example it reads
> user vars out of maps, is hard to run targetted tests, has a different
> format from the familiar test_progs and so on.
>
> I recently thought there was an issue with pop helpers so I created
> some tests to try and track it down. It turns out it was a bug in the
> BPF program we had not the kernel. But, I think it makes sense to
> start deprecating test_sockmap and converting these to the nicer
> test_progs.
>
> So this is a first round of test_prog tests for sockmap cork and
> pop helpers. I'll add push and pull tests shortly. I think its fine,
> maybe preferred to review smaller patchsets, to send these
> incrementally as I get them created.
>
> Thanks!
>
> v2: fix unint vars in some branches from `make RELEASE=1`
I'll wait a bit to see if there is any additional feedback, but on
bpf-next these tests were stable. When we backported to 6.1
they became a bit flaky because recv() would sometimes only get
part of the msg. I'll take a look, but this should be fine adding
a retry logic to the recv() so it does a few recv's before giving
up allows it to recv partial messages, but still pass the test.
Thanks,
John
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper
2024-01-24 18:54 ` [PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper John Fastabend
@ 2024-01-26 11:48 ` Jakub Sitnicki
2024-01-26 15:37 ` John Fastabend
0 siblings, 1 reply; 15+ messages in thread
From: Jakub Sitnicki @ 2024-01-26 11:48 UTC (permalink / raw)
To: John Fastabend; +Cc: bpf, netdev, andrii
On Wed, Jan 24, 2024 at 10:54 AM -08, John Fastabend wrote:
> For msg_pop sk_msg helpers we only have older tests in test_sockmap, but
> these are showing their age. They don't use any of the newer style BPF
> and also require running test_sockmap. Lets use the prog_test framework
> and add a test for msg_pop.
>
> This is a much nicer test env using newer style BPF. We can
> extend this to support all the other helpers shortly.
>
> The bpf program is a template that lets us run through all the helpers
> so we can cover not just pop, but all the other helpers as well.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> .../bpf/prog_tests/sockmap_helpers.h | 10 +
> .../bpf/prog_tests/sockmap_msg_helpers.c | 210 ++++++++++++++++++
> .../bpf/progs/test_sockmap_msg_helpers.c | 52 +++++
> 3 files changed, 272 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
>
[...]
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> new file mode 100644
> index 000000000000..9ffe02f45808
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2020 Cloudflare
Thanks, but we can't take the credit for this brand new file ;-)
> +#include <error.h>
> +#include <netinet/tcp.h>
> +#include <sys/epoll.h>
> +
> +#include "test_progs.h"
> +#include "test_sockmap_msg_helpers.skel.h"
> +#include "sockmap_helpers.h"
> +
> +#define TCP_REPAIR 19 /* TCP sock is under repair right now */
> +
> +#define TCP_REPAIR_ON 1
> +#define TCP_REPAIR_OFF_NO_WP -1 /* Turn off without window probes */
These defines are not unused by this module. Copy-pasted by mistake?
[...]
> diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c b/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
> new file mode 100644
> index 000000000000..c721a00b6001
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2020 Cloudflare
^ :-)
[...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path
2024-01-24 18:54 ` [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path John Fastabend
@ 2024-01-26 12:17 ` Jakub Sitnicki
2024-01-26 14:24 ` Jakub Sitnicki
2024-01-26 15:38 ` John Fastabend
0 siblings, 2 replies; 15+ messages in thread
From: Jakub Sitnicki @ 2024-01-26 12:17 UTC (permalink / raw)
To: John Fastabend; +Cc: bpf, netdev, andrii
On Wed, Jan 24, 2024 at 10:54 AM -08, John Fastabend wrote:
> Sendmsg path with multiple buffers is slightly different from a single
> send in how we have to handle and walk the sg when doing pops. Lets
> ensure this walk is correct.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> .../bpf/prog_tests/sockmap_helpers.h | 8 +++
> .../bpf/prog_tests/sockmap_msg_helpers.c | 53 +++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
> index 781cbdf01d7b..4d8d24482032 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
> @@ -103,6 +103,14 @@
> __ret; \
> })
>
> +#define xsendmsg(fd, msg, flags) \
> + ({ \
> + ssize_t __ret = sendmsg((fd), (msg), (flags)); \
> + if (__ret == -1) \
> + FAIL_ERRNO("sendmsg"); \
> + __ret; \
> + })
> +
> #define xrecv_nonblock(fd, buf, len, flags) \
> ({ \
> ssize_t __ret = recv_timeout((fd), (buf), (len), (flags), \
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> index 9ffe02f45808..e5e618e84950 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> @@ -52,6 +52,50 @@ static void pop_simple_send(struct msg_test_opts *opts, int start, int len)
> ASSERT_OK(cmp, "pop cmp end bytes failed");
> }
>
> +static void pop_complex_send(struct msg_test_opts *opts, int start, int len)
> +{
> + struct test_sockmap_msg_helpers *skel = opts->skel;
> + char buf[] = "abcdefghijklmnopqrstuvwxyz";
> + size_t sent, recv, total = 0;
> + struct msghdr msg = {0};
> + struct iovec iov[15];
> + char *recvbuf;
> + int i;
> +
> + for (i = 0; i < 15; i++) {
Always nice to use ARRAY_SIZE.
> + iov[i].iov_base = buf;
> + iov[i].iov_len = sizeof(buf);
> + total += sizeof(buf);
> + }
> +
> + recvbuf = malloc(total);
> + if (!recvbuf)
> + FAIL("pop complex send malloc failure\n");
390 bytes, why not have it on stack?
> +
> + msg.msg_iov = iov;
> + msg.msg_iovlen = 15;
> +
> + skel->bss->pop = true;
> +
> + if (start == -1)
> + start = sizeof(buf) - len - 1;
> +
> + skel->bss->pop_start = start;
> + skel->bss->pop_len = len;
> +
> + sent = xsendmsg(opts->client, &msg, 0);
> + if (sent != total)
> + FAIL("xsend failed");
> +
> + ASSERT_OK(skel->bss->err, "pop error");
> +
> + recv = xrecv_nonblock(opts->server, recvbuf, total, 0);
> + if (recv != sent - skel->bss->pop_len)
> + FAIL("Received incorrect number number of bytes after pop");
> +
> + free(recvbuf);
> +}
> +
> static void test_sockmap_pop(void)
> {
> struct msg_test_opts opts;
> @@ -92,6 +136,15 @@ static void test_sockmap_pop(void)
> /* Pop from end */
> pop_simple_send(&opts, POP_END, 5);
>
> + /* Empty pop from start of sendmsg */
> + pop_complex_send(&opts, 0, 0);
> + /* Pop from start of sendmsg */
> + pop_complex_send(&opts, 0, 10);
> + /* Pop from middle of sendmsg */
> + pop_complex_send(&opts, 100, 10);
> + /* Pop from end of sendmsg */
> + pop_complex_send(&opts, 394, 10);
Isn't the start offset here past the end? 15*26=390?
> +
> close_sockets:
> close(client);
> close(server);
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 3/4] bpf: sockmap, add a cork to force buffering of the scatterlist
2024-01-24 18:54 ` [PATCH bpf-next v2 3/4] bpf: sockmap, add a cork to force buffering of the scatterlist John Fastabend
@ 2024-01-26 14:19 ` Jakub Sitnicki
2024-01-26 15:38 ` John Fastabend
0 siblings, 1 reply; 15+ messages in thread
From: Jakub Sitnicki @ 2024-01-26 14:19 UTC (permalink / raw)
To: John Fastabend; +Cc: bpf, netdev, andrii
On Wed, Jan 24, 2024 at 10:54 AM -08, John Fastabend wrote:
> By using cork we can force multiple sends into a single scatterlist
> and test that first the cork gives us the correct number of bytes,
> but then also test the pop over the corked data.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> .../bpf/prog_tests/sockmap_msg_helpers.c | 81 +++++++++++++++++++
> .../bpf/progs/test_sockmap_msg_helpers.c | 3 +
> 2 files changed, 84 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> index e5e618e84950..8ced54fe1a0b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> @@ -21,6 +21,85 @@ struct msg_test_opts {
>
> #define POP_END -1
>
> +static void cork_send(struct msg_test_opts *opts, int cork)
> +{
> + struct test_sockmap_msg_helpers *skel = opts->skel;
> + char buf[] = "abcdefghijklmnopqrstuvwxyz";
> + size_t sent, total = 0, recv;
> + char *recvbuf;
> + int i;
> +
> + skel->bss->pop = false;
Why reset it? Every subtest loads new program & creates new maps.
> + skel->bss->cork = cork;
> +
> + /* Send N bytes in 27B chunks */
> + for (i = 0; i < cork / sizeof(buf); i++) {
> + sent = xsend(opts->client, buf, sizeof(buf), 0);
> + if (sent < sizeof(buf))
> + FAIL("xsend failed");
> + total += sent;
> + }
> +
> + recvbuf = malloc(total);
> + if (!recvbuf)
> + FAIL("cork send malloc failure\n");
> +
> + ASSERT_OK(skel->bss->err, "cork error");
> + ASSERT_EQ(skel->bss->size, cork, "cork did not receive all bytes");
> +
> + recv = xrecv_nonblock(opts->server, recvbuf, total, 0);
> + if (recv != total)
> + FAIL("Received incorrect number of bytes");
> +
> + free(recvbuf);
> +}
[...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path
2024-01-26 12:17 ` Jakub Sitnicki
@ 2024-01-26 14:24 ` Jakub Sitnicki
2024-01-26 15:38 ` John Fastabend
1 sibling, 0 replies; 15+ messages in thread
From: Jakub Sitnicki @ 2024-01-26 14:24 UTC (permalink / raw)
To: John Fastabend; +Cc: bpf, netdev, andrii
On Fri, Jan 26, 2024 at 01:17 PM +01, Jakub Sitnicki wrote:
> On Wed, Jan 24, 2024 at 10:54 AM -08, John Fastabend wrote:
[...]
>> @@ -92,6 +136,15 @@ static void test_sockmap_pop(void)
>> /* Pop from end */
>> pop_simple_send(&opts, POP_END, 5);
>>
>> + /* Empty pop from start of sendmsg */
>> + pop_complex_send(&opts, 0, 0);
>> + /* Pop from start of sendmsg */
>> + pop_complex_send(&opts, 0, 10);
>> + /* Pop from middle of sendmsg */
>> + pop_complex_send(&opts, 100, 10);
>> + /* Pop from end of sendmsg */
>> + pop_complex_send(&opts, 394, 10);
>
> Isn't the start offset here past the end? 15*26=390?
Plus terminating null bytes. Nevermind. I can't count.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs
2024-01-24 18:53 [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs John Fastabend
` (4 preceding siblings ...)
2024-01-24 22:58 ` [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs John Fastabend
@ 2024-01-26 14:39 ` Jakub Sitnicki
2024-01-26 15:40 ` John Fastabend
5 siblings, 1 reply; 15+ messages in thread
From: Jakub Sitnicki @ 2024-01-26 14:39 UTC (permalink / raw)
To: John Fastabend; +Cc: bpf, netdev, andrii
On Wed, Jan 24, 2024 at 10:53 AM -08, John Fastabend wrote:
> Its much easier to write and read tests than it was when sockmap was
> originally created. At that time we created a test_sockmap prog that
> did sockmap tests. But, its showing its age now. For example it reads
> user vars out of maps, is hard to run targetted tests, has a different
> format from the familiar test_progs and so on.
>
> I recently thought there was an issue with pop helpers so I created
> some tests to try and track it down. It turns out it was a bug in the
> BPF program we had not the kernel. But, I think it makes sense to
> start deprecating test_sockmap and converting these to the nicer
> test_progs.
>
> So this is a first round of test_prog tests for sockmap cork and
> pop helpers. I'll add push and pull tests shortly. I think its fine,
> maybe preferred to review smaller patchsets, to send these
> incrementally as I get them created.
Cool to see this transition starting.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper
2024-01-26 11:48 ` Jakub Sitnicki
@ 2024-01-26 15:37 ` John Fastabend
0 siblings, 0 replies; 15+ messages in thread
From: John Fastabend @ 2024-01-26 15:37 UTC (permalink / raw)
To: Jakub Sitnicki, John Fastabend; +Cc: bpf, netdev, andrii
Jakub Sitnicki wrote:
> On Wed, Jan 24, 2024 at 10:54 AM -08, John Fastabend wrote:
> > For msg_pop sk_msg helpers we only have older tests in test_sockmap, but
> > these are showing their age. They don't use any of the newer style BPF
> > and also require running test_sockmap. Lets use the prog_test framework
> > and add a test for msg_pop.
> >
> > This is a much nicer test env using newer style BPF. We can
> > extend this to support all the other helpers shortly.
> >
> > The bpf program is a template that lets us run through all the helpers
> > so we can cover not just pop, but all the other helpers as well.
> >
> > Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> > ---
> > .../bpf/prog_tests/sockmap_helpers.h | 10 +
> > .../bpf/prog_tests/sockmap_msg_helpers.c | 210 ++++++++++++++++++
> > .../bpf/progs/test_sockmap_msg_helpers.c | 52 +++++
> > 3 files changed, 272 insertions(+)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> > create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
> >
>
> [...]
>
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> > new file mode 100644
> > index 000000000000..9ffe02f45808
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> > @@ -0,0 +1,210 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2020 Cloudflare
>
> Thanks, but we can't take the credit for this brand new file ;-)
Ah yep seems I cut'n'pasted it from one of your files. I'll
update it.
>
> > +#include <error.h>
> > +#include <netinet/tcp.h>
> > +#include <sys/epoll.h>
> > +
> > +#include "test_progs.h"
> > +#include "test_sockmap_msg_helpers.skel.h"
> > +#include "sockmap_helpers.h"
> > +
> > +#define TCP_REPAIR 19 /* TCP sock is under repair right now */
> > +
> > +#define TCP_REPAIR_ON 1
> > +#define TCP_REPAIR_OFF_NO_WP -1 /* Turn off without window probes */
>
> These defines are not unused by this module. Copy-pasted by mistake?
Yep first iteration I used these will drop them. Thanks.
>
> [...]
>
> > diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c b/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
> > new file mode 100644
> > index 000000000000..c721a00b6001
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/test_sockmap_msg_helpers.c
> > @@ -0,0 +1,52 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2020 Cloudflare
>
> ^ :-)
>
> [...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path
2024-01-26 12:17 ` Jakub Sitnicki
2024-01-26 14:24 ` Jakub Sitnicki
@ 2024-01-26 15:38 ` John Fastabend
1 sibling, 0 replies; 15+ messages in thread
From: John Fastabend @ 2024-01-26 15:38 UTC (permalink / raw)
To: Jakub Sitnicki, John Fastabend; +Cc: bpf, netdev, andrii
Jakub Sitnicki wrote:
> On Wed, Jan 24, 2024 at 10:54 AM -08, John Fastabend wrote:
> > Sendmsg path with multiple buffers is slightly different from a single
> > send in how we have to handle and walk the sg when doing pops. Lets
> > ensure this walk is correct.
> >
> > Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> > ---
> > .../bpf/prog_tests/sockmap_helpers.h | 8 +++
> > .../bpf/prog_tests/sockmap_msg_helpers.c | 53 +++++++++++++++++++
> > 2 files changed, 61 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
> > index 781cbdf01d7b..4d8d24482032 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
> > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
> > @@ -103,6 +103,14 @@
> > __ret; \
> > })
> >
> > +#define xsendmsg(fd, msg, flags) \
> > + ({ \
> > + ssize_t __ret = sendmsg((fd), (msg), (flags)); \
> > + if (__ret == -1) \
> > + FAIL_ERRNO("sendmsg"); \
> > + __ret; \
> > + })
> > +
> > #define xrecv_nonblock(fd, buf, len, flags) \
> > ({ \
> > ssize_t __ret = recv_timeout((fd), (buf), (len), (flags), \
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> > index 9ffe02f45808..e5e618e84950 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> > @@ -52,6 +52,50 @@ static void pop_simple_send(struct msg_test_opts *opts, int start, int len)
> > ASSERT_OK(cmp, "pop cmp end bytes failed");
> > }
> >
> > +static void pop_complex_send(struct msg_test_opts *opts, int start, int len)
> > +{
> > + struct test_sockmap_msg_helpers *skel = opts->skel;
> > + char buf[] = "abcdefghijklmnopqrstuvwxyz";
> > + size_t sent, recv, total = 0;
> > + struct msghdr msg = {0};
> > + struct iovec iov[15];
> > + char *recvbuf;
> > + int i;
> > +
> > + for (i = 0; i < 15; i++) {
>
> Always nice to use ARRAY_SIZE.
Agree will do.
>
> > + iov[i].iov_base = buf;
> > + iov[i].iov_len = sizeof(buf);
> > + total += sizeof(buf);
> > + }
> > +
> > + recvbuf = malloc(total);
> > + if (!recvbuf)
> > + FAIL("pop complex send malloc failure\n");
>
> 390 bytes, why not have it on stack?
Sure one less thing that could fail seems like a win.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 3/4] bpf: sockmap, add a cork to force buffering of the scatterlist
2024-01-26 14:19 ` Jakub Sitnicki
@ 2024-01-26 15:38 ` John Fastabend
0 siblings, 0 replies; 15+ messages in thread
From: John Fastabend @ 2024-01-26 15:38 UTC (permalink / raw)
To: Jakub Sitnicki, John Fastabend; +Cc: bpf, netdev, andrii
Jakub Sitnicki wrote:
> On Wed, Jan 24, 2024 at 10:54 AM -08, John Fastabend wrote:
> > By using cork we can force multiple sends into a single scatterlist
> > and test that first the cork gives us the correct number of bytes,
> > but then also test the pop over the corked data.
> >
> > Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> > ---
> > .../bpf/prog_tests/sockmap_msg_helpers.c | 81 +++++++++++++++++++
> > .../bpf/progs/test_sockmap_msg_helpers.c | 3 +
> > 2 files changed, 84 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> > index e5e618e84950..8ced54fe1a0b 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_msg_helpers.c
> > @@ -21,6 +21,85 @@ struct msg_test_opts {
> >
> > #define POP_END -1
> >
> > +static void cork_send(struct msg_test_opts *opts, int cork)
> > +{
> > + struct test_sockmap_msg_helpers *skel = opts->skel;
> > + char buf[] = "abcdefghijklmnopqrstuvwxyz";
> > + size_t sent, total = 0, recv;
> > + char *recvbuf;
> > + int i;
> > +
> > + skel->bss->pop = false;
>
> Why reset it? Every subtest loads new program & creates new maps.
Agree I'lld elete it.
>
> > + skel->bss->cork = cork;
> > +
> > + /* Send N bytes in 27B chunks */
> > + for (i = 0; i < cork / sizeof(buf); i++) {
> > + sent = xsend(opts->client, buf, sizeof(buf), 0);
> > + if (sent < sizeof(buf))
> > + FAIL("xsend failed");
> > + total += sent;
> > + }
> > +
> > + recvbuf = malloc(total);
> > + if (!recvbuf)
> > + FAIL("cork send malloc failure\n");
> > +
> > + ASSERT_OK(skel->bss->err, "cork error");
> > + ASSERT_EQ(skel->bss->size, cork, "cork did not receive all bytes");
> > +
> > + recv = xrecv_nonblock(opts->server, recvbuf, total, 0);
> > + if (recv != total)
> > + FAIL("Received incorrect number of bytes");
> > +
> > + free(recvbuf);
> > +}
>
> [...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs
2024-01-26 14:39 ` Jakub Sitnicki
@ 2024-01-26 15:40 ` John Fastabend
0 siblings, 0 replies; 15+ messages in thread
From: John Fastabend @ 2024-01-26 15:40 UTC (permalink / raw)
To: Jakub Sitnicki, John Fastabend; +Cc: bpf, netdev, andrii
Jakub Sitnicki wrote:
> On Wed, Jan 24, 2024 at 10:53 AM -08, John Fastabend wrote:
> > Its much easier to write and read tests than it was when sockmap was
> > originally created. At that time we created a test_sockmap prog that
> > did sockmap tests. But, its showing its age now. For example it reads
> > user vars out of maps, is hard to run targetted tests, has a different
> > format from the familiar test_progs and so on.
> >
> > I recently thought there was an issue with pop helpers so I created
> > some tests to try and track it down. It turns out it was a bug in the
> > BPF program we had not the kernel. But, I think it makes sense to
> > start deprecating test_sockmap and converting these to the nicer
> > test_progs.
> >
> > So this is a first round of test_prog tests for sockmap cork and
> > pop helpers. I'll add push and pull tests shortly. I think its fine,
> > maybe preferred to review smaller patchsets, to send these
> > incrementally as I get them created.
>
> Cool to see this transition starting.
Thanks for the review.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-01-26 15:40 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-24 18:53 [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs John Fastabend
2024-01-24 18:54 ` [PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper John Fastabend
2024-01-26 11:48 ` Jakub Sitnicki
2024-01-26 15:37 ` John Fastabend
2024-01-24 18:54 ` [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path John Fastabend
2024-01-26 12:17 ` Jakub Sitnicki
2024-01-26 14:24 ` Jakub Sitnicki
2024-01-26 15:38 ` John Fastabend
2024-01-24 18:54 ` [PATCH bpf-next v2 3/4] bpf: sockmap, add a cork to force buffering of the scatterlist John Fastabend
2024-01-26 14:19 ` Jakub Sitnicki
2024-01-26 15:38 ` John Fastabend
2024-01-24 18:54 ` [PATCH bpf-next v2 4/4] bpf: sockmap test cork and pop combined John Fastabend
2024-01-24 22:58 ` [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs John Fastabend
2024-01-26 14:39 ` Jakub Sitnicki
2024-01-26 15:40 ` John Fastabend
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.