[PATCH bpf-next v2 0/4] transition sockmap testing to test_progs

[PATCH bpf-next v2 0/4] transition sockmap testing to test_progs

[John Fastabend]

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

[PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper

[John Fastabend]

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

Re: [PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper

[Jakub Sitnicki]

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

^ :-)

[...]

Re: [PATCH bpf-next v2 1/4] bpf: sockmap, add test for sk_msg prog pop msg helper

[John Fastabend]

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
> 
> ^ :-)
> 
> [...]

[PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path

[John Fastabend]

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

Re: [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path

[Jakub Sitnicki]

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);

Re: [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path

[Jakub Sitnicki]

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.

Re: [PATCH bpf-next v2 2/4] bpf: sockmap, add a sendmsg test so we can check that path

[John Fastabend]

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.

[PATCH bpf-next v2 3/4] bpf: sockmap, add a cork to force buffering of the scatterlist

[John Fastabend]

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

Re: [PATCH bpf-next v2 3/4] bpf: sockmap, add a cork to force buffering of the scatterlist

[Jakub Sitnicki]

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);
> +}

[...]

Re: [PATCH bpf-next v2 3/4] bpf: sockmap, add a cork to force buffering of the scatterlist

[John Fastabend]

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);
> > +}
> 
> [...]

[PATCH bpf-next v2 4/4] bpf: sockmap test cork and pop combined

[John Fastabend]

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

RE: [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs

[John Fastabend]

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

Re: [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs

[Jakub Sitnicki]

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.

Re: [PATCH bpf-next v2 0/4] transition sockmap testing to test_progs

[John Fastabend]

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.