[PATCH v2 bpf-next 0/6] selftests/bpf: Add sockaddr tests for kernel networking

[PATCH v2 bpf-next 0/6] selftests/bpf: Add sockaddr tests for kernel networking

[Jordan Rife]

This patch series adds test coverage for BPF sockaddr hooks and their
interactions with kernel socket functions (i.e. kernel_bind(),
kernel_connect(), kernel_sendmsg(), sock_sendmsg(),
kernel_getpeername(), and kernel_getsockname()) while also rounding out
IPv4 and IPv6 sockaddr hook coverage in prog_tests/sock_addr.c.

As with v1 of this patch series, we add regression coverage for the
issues addressed by these patches,

- commit 0bdf399342c5("net: Avoid address overwrite in kernel_connect")
- commit 86a7e0b69bd5("net: prevent rewrite of msg_name in sock_sendmsg()")
- commit c889a99a21bf("net: prevent address rewrite in kernel_bind()")
- commit 01b2885d9415("net: Save and restore msg_namelen in sock_sendmsg")

but broaden the focus a bit.

In order to extend prog_tests/sock_addr.c to test these kernel
functions, we add a set of new kfuncs that wrap individual socket
operations to bpf_testmod and invoke them through set of corresponding
SYSCALL programs (progs/sock_addr_kern.c). Each test case can be
configured to use a different set of "sock_ops" depending on whether it
is testing kernel calls (kernel_bind(), kernel_connect(), etc.) or
system calls (bind(), connect(), etc.).

=======
Patches
=======
* Patch 1 fixes the sock_addr bind test program to work for big endian
  architectures such as s390x.
* Patch 2 introduces the new kfuncs to bpf_testmod.
* Patch 3 introduces the BPF program which allows us to invoke these
  kfuncs invividually from the test program.
* Patch 4 lays the groundwork for IPv4 and IPv6 sockaddr hook coverage
  by migrating much of the environment setup logic from
  bpf/test_sock_addr.sh into prog_tests/sock_addr.c and adds test cases
  to cover bind4/6, connect4/6, sendmsg4/6 and recvmsg4/6 hooks.
* Patch 5 makes the set of socket operations for each test case
  configurable, laying the groundwork for Patch 6.
* Patch 6 introduces two sets of sock_ops that invoke the kernel
  equivalents of connect(), bind(), etc. and uses these to add coverage
  for the kernel socket functions.

=======
Changes
=======
v1->v2
------
* Dropped test_progs/sock_addr_kern.c and the sock_addr_kern test module
  in favor of simply expanding bpf_testmod and test_progs/sock_addr.c.
* Migrated environment setup logic from bpf/test_sock_addr.sh into
  prog_tests/sock_addr.c rather than invoking the script from the test
  program.
* Added kfuncs to bpf_testmod as well as the sock_addr_kern BPF program
  to enable us to invoke kernel socket functions from
  test_progs/sock_addr.c.
* Added test coverage for kernel socket functions to
  test_progs/sock_addr.c.

Link: https://lore.kernel.org/bpf/20240329191907.1808635-1-jrife@google.com/T/#u

Jordan Rife (6):
  selftests/bpf: Fix bind program for big endian systems
  selftests/bpf: Implement socket kfuncs for bpf_testmod
  selftests/bpf: Implement BPF programs for kernel socket operations
  selftests/bpf: Add IPv4 and IPv6 sockaddr test cases
  selftests/bpf: Make sock configurable for each test case
  selftests/bpf: Add kernel socket operation tests

 .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 139 +++
 .../bpf/bpf_testmod/bpf_testmod_kfunc.h       |  27 +
 .../selftests/bpf/prog_tests/sock_addr.c      | 940 +++++++++++++++---
 .../testing/selftests/bpf/progs/bind4_prog.c  |  18 +-
 .../testing/selftests/bpf/progs/bind6_prog.c  |  18 +-
 tools/testing/selftests/bpf/progs/bind_prog.h |  19 +
 .../selftests/bpf/progs/sock_addr_kern.c      |  65 ++
 7 files changed, 1077 insertions(+), 149 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/bind_prog.h
 create mode 100644 tools/testing/selftests/bpf/progs/sock_addr_kern.c

-- 
2.44.0.478.gd926399ef9-goog

[PATCH v2 bpf-next 1/6] selftests/bpf: Fix bind program for big endian systems

[Jordan Rife]

Without this fix, the bind4 and bind6 programs will reject bind attempts
on big endian systems. This patch ensures that CI tests pass for the
s390x architecture.

Signed-off-by: Jordan Rife <jrife@google.com>
---
 .../testing/selftests/bpf/progs/bind4_prog.c  | 18 ++++++++++--------
 .../testing/selftests/bpf/progs/bind6_prog.c  | 18 ++++++++++--------
 tools/testing/selftests/bpf/progs/bind_prog.h | 19 +++++++++++++++++++
 3 files changed, 39 insertions(+), 16 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/bind_prog.h

diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
index a487f60b73ac4..2bc052ecb6eef 100644
--- a/tools/testing/selftests/bpf/progs/bind4_prog.c
+++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
@@ -12,6 +12,8 @@
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_endian.h>
 
+#include "bind_prog.h"
+
 #define SERV4_IP		0xc0a801feU /* 192.168.1.254 */
 #define SERV4_PORT		4040
 #define SERV4_REWRITE_IP	0x7f000001U /* 127.0.0.1 */
@@ -118,23 +120,23 @@ int bind_v4_prog(struct bpf_sock_addr *ctx)
 
 	// u8 narrow loads:
 	user_ip4 = 0;
-	user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[0] << 0;
-	user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[1] << 8;
-	user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[2] << 16;
-	user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[3] << 24;
+	user_ip4 |= load_byte_ntoh(ctx->user_ip4, 0, sizeof(user_ip4));
+	user_ip4 |= load_byte_ntoh(ctx->user_ip4, 1, sizeof(user_ip4));
+	user_ip4 |= load_byte_ntoh(ctx->user_ip4, 2, sizeof(user_ip4));
+	user_ip4 |= load_byte_ntoh(ctx->user_ip4, 3, sizeof(user_ip4));
 	if (ctx->user_ip4 != user_ip4)
 		return 0;
 
 	user_port = 0;
-	user_port |= ((volatile __u8 *)&ctx->user_port)[0] << 0;
-	user_port |= ((volatile __u8 *)&ctx->user_port)[1] << 8;
+	user_port |= load_byte_ntoh(ctx->user_port, 0, sizeof(user_port));
+	user_port |= load_byte_ntoh(ctx->user_port, 1, sizeof(user_port));
 	if (ctx->user_port != user_port)
 		return 0;
 
 	// u16 narrow loads:
 	user_ip4 = 0;
-	user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[0] << 0;
-	user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[1] << 16;
+	user_ip4 |= load_word_ntoh(ctx->user_ip4, 0, sizeof(user_ip4));
+	user_ip4 |= load_word_ntoh(ctx->user_ip4, 1, sizeof(user_ip4));
 	if (ctx->user_ip4 != user_ip4)
 		return 0;
 
diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c
index d62cd9e9cf0ea..194583e3375bf 100644
--- a/tools/testing/selftests/bpf/progs/bind6_prog.c
+++ b/tools/testing/selftests/bpf/progs/bind6_prog.c
@@ -12,6 +12,8 @@
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_endian.h>
 
+#include "bind_prog.h"
+
 #define SERV6_IP_0		0xfaceb00c /* face:b00c:1234:5678::abcd */
 #define SERV6_IP_1		0x12345678
 #define SERV6_IP_2		0x00000000
@@ -129,25 +131,25 @@ int bind_v6_prog(struct bpf_sock_addr *ctx)
 	// u8 narrow loads:
 	for (i = 0; i < 4; i++) {
 		user_ip6 = 0;
-		user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[0] << 0;
-		user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[1] << 8;
-		user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[2] << 16;
-		user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[3] << 24;
+		user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 0, sizeof(user_ip6));
+		user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 1, sizeof(user_ip6));
+		user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 2, sizeof(user_ip6));
+		user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 3, sizeof(user_ip6));
 		if (ctx->user_ip6[i] != user_ip6)
 			return 0;
 	}
 
 	user_port = 0;
-	user_port |= ((volatile __u8 *)&ctx->user_port)[0] << 0;
-	user_port |= ((volatile __u8 *)&ctx->user_port)[1] << 8;
+	user_port |= load_byte_ntoh(ctx->user_port, 0, sizeof(user_port));
+	user_port |= load_byte_ntoh(ctx->user_port, 1, sizeof(user_port));
 	if (ctx->user_port != user_port)
 		return 0;
 
 	// u16 narrow loads:
 	for (i = 0; i < 4; i++) {
 		user_ip6 = 0;
-		user_ip6 |= ((volatile __u16 *)&ctx->user_ip6[i])[0] << 0;
-		user_ip6 |= ((volatile __u16 *)&ctx->user_ip6[i])[1] << 16;
+		user_ip6 |= load_word_ntoh(ctx->user_ip6[i], 0, sizeof(user_ip6));
+		user_ip6 |= load_word_ntoh(ctx->user_ip6[i], 1, sizeof(user_ip6));
 		if (ctx->user_ip6[i] != user_ip6)
 			return 0;
 	}
diff --git a/tools/testing/selftests/bpf/progs/bind_prog.h b/tools/testing/selftests/bpf/progs/bind_prog.h
new file mode 100644
index 0000000000000..0fdc466aec346
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bind_prog.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __BIND_PROG_H__
+#define __BIND_PROG_H__
+
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define load_byte_ntoh(src, b, s) \
+	(((volatile __u8 *)&(src))[b] << 8 * b)
+#define load_word_ntoh(src, w, s) \
+	(((volatile __u16 *)&(src))[w] << 16 * w)
+#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+#define load_byte_ntoh(src, b, s) \
+	(((volatile __u8 *)&(src))[(b) + (sizeof(src) - (s))] << 8 * ((s) - (b) - 1))
+#define load_word_ntoh(src, w, s) \
+	(((volatile __u16 *)&(src))[w] << 16 * (((s) / 2) - (w) - 1))
+#else
+# error "Fix your compiler's __BYTE_ORDER__?!"
+#endif
+
+#endif
-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCH v2 bpf-next 1/6] selftests/bpf: Fix bind program for big endian systems

[Kui-Feng Lee]

On 4/12/24 09:52, Jordan Rife wrote:
> Without this fix, the bind4 and bind6 programs will reject bind attempts
> on big endian systems. This patch ensures that CI tests pass for the
> s390x architecture.
> 
> Signed-off-by: Jordan Rife <jrife@google.com>
> ---
>   .../testing/selftests/bpf/progs/bind4_prog.c  | 18 ++++++++++--------
>   .../testing/selftests/bpf/progs/bind6_prog.c  | 18 ++++++++++--------
>   tools/testing/selftests/bpf/progs/bind_prog.h | 19 +++++++++++++++++++
>   3 files changed, 39 insertions(+), 16 deletions(-)
>   create mode 100644 tools/testing/selftests/bpf/progs/bind_prog.h
> 
> diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
> index a487f60b73ac4..2bc052ecb6eef 100644
> --- a/tools/testing/selftests/bpf/progs/bind4_prog.c
> +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
> @@ -12,6 +12,8 @@
>   #include <bpf/bpf_helpers.h>
>   #include <bpf/bpf_endian.h>
>   
> +#include "bind_prog.h"
> +
>   #define SERV4_IP		0xc0a801feU /* 192.168.1.254 */
>   #define SERV4_PORT		4040
>   #define SERV4_REWRITE_IP	0x7f000001U /* 127.0.0.1 */
> @@ -118,23 +120,23 @@ int bind_v4_prog(struct bpf_sock_addr *ctx)
>   
>   	// u8 narrow loads:
>   	user_ip4 = 0;
> -	user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[0] << 0;
> -	user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[1] << 8;
> -	user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[2] << 16;
> -	user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[3] << 24;
> +	user_ip4 |= load_byte_ntoh(ctx->user_ip4, 0, sizeof(user_ip4));
> +	user_ip4 |= load_byte_ntoh(ctx->user_ip4, 1, sizeof(user_ip4));
> +	user_ip4 |= load_byte_ntoh(ctx->user_ip4, 2, sizeof(user_ip4));
> +	user_ip4 |= load_byte_ntoh(ctx->user_ip4, 3, sizeof(user_ip4));
>   	if (ctx->user_ip4 != user_ip4)
>   		return 0;
>   
>   	user_port = 0;
> -	user_port |= ((volatile __u8 *)&ctx->user_port)[0] << 0;
> -	user_port |= ((volatile __u8 *)&ctx->user_port)[1] << 8;
> +	user_port |= load_byte_ntoh(ctx->user_port, 0, sizeof(user_port));
> +	user_port |= load_byte_ntoh(ctx->user_port, 1, sizeof(user_port));
>   	if (ctx->user_port != user_port)
>   		return 0;
>   
>   	// u16 narrow loads:
>   	user_ip4 = 0;
> -	user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[0] << 0;
> -	user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[1] << 16;
> +	user_ip4 |= load_word_ntoh(ctx->user_ip4, 0, sizeof(user_ip4));
> +	user_ip4 |= load_word_ntoh(ctx->user_ip4, 1, sizeof(user_ip4));
>   	if (ctx->user_ip4 != user_ip4)
>   		return 0;
>   
> diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c
> index d62cd9e9cf0ea..194583e3375bf 100644
> --- a/tools/testing/selftests/bpf/progs/bind6_prog.c
> +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c
> @@ -12,6 +12,8 @@
>   #include <bpf/bpf_helpers.h>
>   #include <bpf/bpf_endian.h>
>   
> +#include "bind_prog.h"
> +
>   #define SERV6_IP_0		0xfaceb00c /* face:b00c:1234:5678::abcd */
>   #define SERV6_IP_1		0x12345678
>   #define SERV6_IP_2		0x00000000
> @@ -129,25 +131,25 @@ int bind_v6_prog(struct bpf_sock_addr *ctx)
>   	// u8 narrow loads:
>   	for (i = 0; i < 4; i++) {
>   		user_ip6 = 0;
> -		user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[0] << 0;
> -		user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[1] << 8;
> -		user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[2] << 16;
> -		user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[3] << 24;
> +		user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 0, sizeof(user_ip6));
> +		user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 1, sizeof(user_ip6));
> +		user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 2, sizeof(user_ip6));
> +		user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 3, sizeof(user_ip6));
>   		if (ctx->user_ip6[i] != user_ip6)
>   			return 0;
>   	}
>   
>   	user_port = 0;
> -	user_port |= ((volatile __u8 *)&ctx->user_port)[0] << 0;
> -	user_port |= ((volatile __u8 *)&ctx->user_port)[1] << 8;
> +	user_port |= load_byte_ntoh(ctx->user_port, 0, sizeof(user_port));
> +	user_port |= load_byte_ntoh(ctx->user_port, 1, sizeof(user_port));
>   	if (ctx->user_port != user_port)
>   		return 0;
>   
>   	// u16 narrow loads:
>   	for (i = 0; i < 4; i++) {
>   		user_ip6 = 0;
> -		user_ip6 |= ((volatile __u16 *)&ctx->user_ip6[i])[0] << 0;
> -		user_ip6 |= ((volatile __u16 *)&ctx->user_ip6[i])[1] << 16;
> +		user_ip6 |= load_word_ntoh(ctx->user_ip6[i], 0, sizeof(user_ip6));
> +		user_ip6 |= load_word_ntoh(ctx->user_ip6[i], 1, sizeof(user_ip6));
>   		if (ctx->user_ip6[i] != user_ip6)
>   			return 0;
>   	}
> diff --git a/tools/testing/selftests/bpf/progs/bind_prog.h b/tools/testing/selftests/bpf/progs/bind_prog.h
> new file mode 100644
> index 0000000000000..0fdc466aec346
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bind_prog.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __BIND_PROG_H__
> +#define __BIND_PROG_H__
> +
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +#define load_byte_ntoh(src, b, s) \
> +	(((volatile __u8 *)&(src))[b] << 8 * b)
> +#define load_word_ntoh(src, w, s) \
> +	(((volatile __u16 *)&(src))[w] << 16 * w)
> +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> +#define load_byte_ntoh(src, b, s) \
> +	(((volatile __u8 *)&(src))[(b) + (sizeof(src) - (s))] << 8 * ((s) - (b) - 1))
> +#define load_word_ntoh(src, w, s) \
> +	(((volatile __u16 *)&(src))[w] << 16 * (((s) / 2) - (w) - 1))
These names, load_byte_ntoh() and load_word_ntoh(), are miss-leading.

They don't actually do byte-order conversion from network order to host
order. Network order is big endian. 0xdeadbeef in u32 should be stored
as the sequence of

   0xde, 0xad, 0xbe, 0xef

The little endian implementation of load_word_ntoh() provided here will
return 0xadde and 0xefbe0000. However, a network order to host order
conversion should return 0xbeef and 0xdead0000 for little endian.

The little endian implementation of load_byte_ntoh() here returns 0xde,
0xad00, 0xbe0000, and 0xef000000. However, a network to host order
conversion should return 0xef, 0xbe00, 0xad0000, and 0xde00000.

So, they just access raw data following the host byte order, not
providing any byte order conversion.


> +#else
> +# error "Fix your compiler's __BYTE_ORDER__?!"
> +#endif
> +
> +#endif

Re: [PATCH v2 bpf-next 1/6] selftests/bpf: Fix bind program for big endian systems

[Jordan Rife]

Kui-Feng,

You are right. Maybe simply "load_word" and "load_byte" would be a
better name here. WDYT?

-Jordan


On Fri, Apr 12, 2024 at 6:01 PM Kui-Feng Lee <sinquersw@gmail.com> wrote:
>
>
>
> On 4/12/24 09:52, Jordan Rife wrote:
> > Without this fix, the bind4 and bind6 programs will reject bind attempts
> > on big endian systems. This patch ensures that CI tests pass for the
> > s390x architecture.
> >
> > Signed-off-by: Jordan Rife <jrife@google.com>
> > ---
> >   .../testing/selftests/bpf/progs/bind4_prog.c  | 18 ++++++++++--------
> >   .../testing/selftests/bpf/progs/bind6_prog.c  | 18 ++++++++++--------
> >   tools/testing/selftests/bpf/progs/bind_prog.h | 19 +++++++++++++++++++
> >   3 files changed, 39 insertions(+), 16 deletions(-)
> >   create mode 100644 tools/testing/selftests/bpf/progs/bind_prog.h
> >
> > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
> > index a487f60b73ac4..2bc052ecb6eef 100644
> > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c
> > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
> > @@ -12,6 +12,8 @@
> >   #include <bpf/bpf_helpers.h>
> >   #include <bpf/bpf_endian.h>
> >
> > +#include "bind_prog.h"
> > +
> >   #define SERV4_IP            0xc0a801feU /* 192.168.1.254 */
> >   #define SERV4_PORT          4040
> >   #define SERV4_REWRITE_IP    0x7f000001U /* 127.0.0.1 */
> > @@ -118,23 +120,23 @@ int bind_v4_prog(struct bpf_sock_addr *ctx)
> >
> >       // u8 narrow loads:
> >       user_ip4 = 0;
> > -     user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[0] << 0;
> > -     user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[1] << 8;
> > -     user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[2] << 16;
> > -     user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[3] << 24;
> > +     user_ip4 |= load_byte_ntoh(ctx->user_ip4, 0, sizeof(user_ip4));
> > +     user_ip4 |= load_byte_ntoh(ctx->user_ip4, 1, sizeof(user_ip4));
> > +     user_ip4 |= load_byte_ntoh(ctx->user_ip4, 2, sizeof(user_ip4));
> > +     user_ip4 |= load_byte_ntoh(ctx->user_ip4, 3, sizeof(user_ip4));
> >       if (ctx->user_ip4 != user_ip4)
> >               return 0;
> >
> >       user_port = 0;
> > -     user_port |= ((volatile __u8 *)&ctx->user_port)[0] << 0;
> > -     user_port |= ((volatile __u8 *)&ctx->user_port)[1] << 8;
> > +     user_port |= load_byte_ntoh(ctx->user_port, 0, sizeof(user_port));
> > +     user_port |= load_byte_ntoh(ctx->user_port, 1, sizeof(user_port));
> >       if (ctx->user_port != user_port)
> >               return 0;
> >
> >       // u16 narrow loads:
> >       user_ip4 = 0;
> > -     user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[0] << 0;
> > -     user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[1] << 16;
> > +     user_ip4 |= load_word_ntoh(ctx->user_ip4, 0, sizeof(user_ip4));
> > +     user_ip4 |= load_word_ntoh(ctx->user_ip4, 1, sizeof(user_ip4));
> >       if (ctx->user_ip4 != user_ip4)
> >               return 0;
> >
> > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c
> > index d62cd9e9cf0ea..194583e3375bf 100644
> > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c
> > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c
> > @@ -12,6 +12,8 @@
> >   #include <bpf/bpf_helpers.h>
> >   #include <bpf/bpf_endian.h>
> >
> > +#include "bind_prog.h"
> > +
> >   #define SERV6_IP_0          0xfaceb00c /* face:b00c:1234:5678::abcd */
> >   #define SERV6_IP_1          0x12345678
> >   #define SERV6_IP_2          0x00000000
> > @@ -129,25 +131,25 @@ int bind_v6_prog(struct bpf_sock_addr *ctx)
> >       // u8 narrow loads:
> >       for (i = 0; i < 4; i++) {
> >               user_ip6 = 0;
> > -             user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[0] << 0;
> > -             user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[1] << 8;
> > -             user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[2] << 16;
> > -             user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[3] << 24;
> > +             user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 0, sizeof(user_ip6));
> > +             user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 1, sizeof(user_ip6));
> > +             user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 2, sizeof(user_ip6));
> > +             user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 3, sizeof(user_ip6));
> >               if (ctx->user_ip6[i] != user_ip6)
> >                       return 0;
> >       }
> >
> >       user_port = 0;
> > -     user_port |= ((volatile __u8 *)&ctx->user_port)[0] << 0;
> > -     user_port |= ((volatile __u8 *)&ctx->user_port)[1] << 8;
> > +     user_port |= load_byte_ntoh(ctx->user_port, 0, sizeof(user_port));
> > +     user_port |= load_byte_ntoh(ctx->user_port, 1, sizeof(user_port));
> >       if (ctx->user_port != user_port)
> >               return 0;
> >
> >       // u16 narrow loads:
> >       for (i = 0; i < 4; i++) {
> >               user_ip6 = 0;
> > -             user_ip6 |= ((volatile __u16 *)&ctx->user_ip6[i])[0] << 0;
> > -             user_ip6 |= ((volatile __u16 *)&ctx->user_ip6[i])[1] << 16;
> > +             user_ip6 |= load_word_ntoh(ctx->user_ip6[i], 0, sizeof(user_ip6));
> > +             user_ip6 |= load_word_ntoh(ctx->user_ip6[i], 1, sizeof(user_ip6));
> >               if (ctx->user_ip6[i] != user_ip6)
> >                       return 0;
> >       }
> > diff --git a/tools/testing/selftests/bpf/progs/bind_prog.h b/tools/testing/selftests/bpf/progs/bind_prog.h
> > new file mode 100644
> > index 0000000000000..0fdc466aec346
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/bind_prog.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __BIND_PROG_H__
> > +#define __BIND_PROG_H__
> > +
> > +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> > +#define load_byte_ntoh(src, b, s) \
> > +     (((volatile __u8 *)&(src))[b] << 8 * b)
> > +#define load_word_ntoh(src, w, s) \
> > +     (((volatile __u16 *)&(src))[w] << 16 * w)
> > +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> > +#define load_byte_ntoh(src, b, s) \
> > +     (((volatile __u8 *)&(src))[(b) + (sizeof(src) - (s))] << 8 * ((s) - (b) - 1))
> > +#define load_word_ntoh(src, w, s) \
> > +     (((volatile __u16 *)&(src))[w] << 16 * (((s) / 2) - (w) - 1))
> These names, load_byte_ntoh() and load_word_ntoh(), are miss-leading.
>
> They don't actually do byte-order conversion from network order to host
> order. Network order is big endian. 0xdeadbeef in u32 should be stored
> as the sequence of
>
>    0xde, 0xad, 0xbe, 0xef
>
> The little endian implementation of load_word_ntoh() provided here will
> return 0xadde and 0xefbe0000. However, a network order to host order
> conversion should return 0xbeef and 0xdead0000 for little endian.
>
> The little endian implementation of load_byte_ntoh() here returns 0xde,
> 0xad00, 0xbe0000, and 0xef000000. However, a network to host order
> conversion should return 0xef, 0xbe00, 0xad0000, and 0xde00000.
>
> So, they just access raw data following the host byte order, not
> providing any byte order conversion.
>
>
> > +#else
> > +# error "Fix your compiler's __BYTE_ORDER__?!"
> > +#endif
> > +
> > +#endif

Re: [PATCH v2 bpf-next 1/6] selftests/bpf: Fix bind program for big endian systems

[Kui-Feng Lee]

On 4/12/24 18:17, Jordan Rife wrote:
> Kui-Feng,
> 
> You are right. Maybe simply "load_word" and "load_byte" would be a 
> better name here. WDYT?

Agree!


> 
> -Jordan
> 
> 
> On Fri, Apr 12, 2024 at 6:01 PM Kui-Feng Lee <sinquersw@gmail.com 
> <mailto:sinquersw@gmail.com>> wrote:
> 
> 
> 
>     On 4/12/24 09:52, Jordan Rife wrote:
>      > Without this fix, the bind4 and bind6 programs will reject bind
>     attempts
>      > on big endian systems. This patch ensures that CI tests pass for the
>      > s390x architecture.
>      >
>      > Signed-off-by: Jordan Rife <jrife@google.com
>     <mailto:jrife@google.com>>
>      > ---
>      >   .../testing/selftests/bpf/progs/bind4_prog.c  | 18
>     ++++++++++--------
>      >   .../testing/selftests/bpf/progs/bind6_prog.c  | 18
>     ++++++++++--------
>      >   tools/testing/selftests/bpf/progs/bind_prog.h | 19
>     +++++++++++++++++++
>      >   3 files changed, 39 insertions(+), 16 deletions(-)
>      >   create mode 100644 tools/testing/selftests/bpf/progs/bind_prog.h
>      >
>      > diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c
>     b/tools/testing/selftests/bpf/progs/bind4_prog.c
>      > index a487f60b73ac4..2bc052ecb6eef 100644
>      > --- a/tools/testing/selftests/bpf/progs/bind4_prog.c
>      > +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
>      > @@ -12,6 +12,8 @@
>      >   #include <bpf/bpf_helpers.h>
>      >   #include <bpf/bpf_endian.h>
>      >
>      > +#include "bind_prog.h"
>      > +
>      >   #define SERV4_IP            0xc0a801feU /* 192.168.1.254 */
>      >   #define SERV4_PORT          4040
>      >   #define SERV4_REWRITE_IP    0x7f000001U /* 127.0.0.1 */
>      > @@ -118,23 +120,23 @@ int bind_v4_prog(struct bpf_sock_addr *ctx)
>      >
>      >       // u8 narrow loads:
>      >       user_ip4 = 0;
>      > -     user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[0] << 0;
>      > -     user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[1] << 8;
>      > -     user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[2] << 16;
>      > -     user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[3] << 24;
>      > +     user_ip4 |= load_byte_ntoh(ctx->user_ip4, 0, sizeof(user_ip4));
>      > +     user_ip4 |= load_byte_ntoh(ctx->user_ip4, 1, sizeof(user_ip4));
>      > +     user_ip4 |= load_byte_ntoh(ctx->user_ip4, 2, sizeof(user_ip4));
>      > +     user_ip4 |= load_byte_ntoh(ctx->user_ip4, 3, sizeof(user_ip4));
>      >       if (ctx->user_ip4 != user_ip4)
>      >               return 0;
>      >
>      >       user_port = 0;
>      > -     user_port |= ((volatile __u8 *)&ctx->user_port)[0] << 0;
>      > -     user_port |= ((volatile __u8 *)&ctx->user_port)[1] << 8;
>      > +     user_port |= load_byte_ntoh(ctx->user_port, 0,
>     sizeof(user_port));
>      > +     user_port |= load_byte_ntoh(ctx->user_port, 1,
>     sizeof(user_port));
>      >       if (ctx->user_port != user_port)
>      >               return 0;
>      >
>      >       // u16 narrow loads:
>      >       user_ip4 = 0;
>      > -     user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[0] << 0;
>      > -     user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[1] << 16;
>      > +     user_ip4 |= load_word_ntoh(ctx->user_ip4, 0, sizeof(user_ip4));
>      > +     user_ip4 |= load_word_ntoh(ctx->user_ip4, 1, sizeof(user_ip4));
>      >       if (ctx->user_ip4 != user_ip4)
>      >               return 0;
>      >
>      > diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c
>     b/tools/testing/selftests/bpf/progs/bind6_prog.c
>      > index d62cd9e9cf0ea..194583e3375bf 100644
>      > --- a/tools/testing/selftests/bpf/progs/bind6_prog.c
>      > +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c
>      > @@ -12,6 +12,8 @@
>      >   #include <bpf/bpf_helpers.h>
>      >   #include <bpf/bpf_endian.h>
>      >
>      > +#include "bind_prog.h"
>      > +
>      >   #define SERV6_IP_0          0xfaceb00c /*
>     face:b00c:1234:5678::abcd */
>      >   #define SERV6_IP_1          0x12345678
>      >   #define SERV6_IP_2          0x00000000
>      > @@ -129,25 +131,25 @@ int bind_v6_prog(struct bpf_sock_addr *ctx)
>      >       // u8 narrow loads:
>      >       for (i = 0; i < 4; i++) {
>      >               user_ip6 = 0;
>      > -             user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[0]
>     << 0;
>      > -             user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[1]
>     << 8;
>      > -             user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[2]
>     << 16;
>      > -             user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[3]
>     << 24;
>      > +             user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 0,
>     sizeof(user_ip6));
>      > +             user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 1,
>     sizeof(user_ip6));
>      > +             user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 2,
>     sizeof(user_ip6));
>      > +             user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 3,
>     sizeof(user_ip6));
>      >               if (ctx->user_ip6[i] != user_ip6)
>      >                       return 0;
>      >       }
>      >
>      >       user_port = 0;
>      > -     user_port |= ((volatile __u8 *)&ctx->user_port)[0] << 0;
>      > -     user_port |= ((volatile __u8 *)&ctx->user_port)[1] << 8;
>      > +     user_port |= load_byte_ntoh(ctx->user_port, 0,
>     sizeof(user_port));
>      > +     user_port |= load_byte_ntoh(ctx->user_port, 1,
>     sizeof(user_port));
>      >       if (ctx->user_port != user_port)
>      >               return 0;
>      >
>      >       // u16 narrow loads:
>      >       for (i = 0; i < 4; i++) {
>      >               user_ip6 = 0;
>      > -             user_ip6 |= ((volatile __u16
>     *)&ctx->user_ip6[i])[0] << 0;
>      > -             user_ip6 |= ((volatile __u16
>     *)&ctx->user_ip6[i])[1] << 16;
>      > +             user_ip6 |= load_word_ntoh(ctx->user_ip6[i], 0,
>     sizeof(user_ip6));
>      > +             user_ip6 |= load_word_ntoh(ctx->user_ip6[i], 1,
>     sizeof(user_ip6));
>      >               if (ctx->user_ip6[i] != user_ip6)
>      >                       return 0;
>      >       }
>      > diff --git a/tools/testing/selftests/bpf/progs/bind_prog.h
>     b/tools/testing/selftests/bpf/progs/bind_prog.h
>      > new file mode 100644
>      > index 0000000000000..0fdc466aec346
>      > --- /dev/null
>      > +++ b/tools/testing/selftests/bpf/progs/bind_prog.h
>      > @@ -0,0 +1,19 @@
>      > +/* SPDX-License-Identifier: GPL-2.0 */
>      > +#ifndef __BIND_PROG_H__
>      > +#define __BIND_PROG_H__
>      > +
>      > +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>      > +#define load_byte_ntoh(src, b, s) \
>      > +     (((volatile __u8 *)&(src))[b] << 8 * b)
>      > +#define load_word_ntoh(src, w, s) \
>      > +     (((volatile __u16 *)&(src))[w] << 16 * w)
>      > +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
>      > +#define load_byte_ntoh(src, b, s) \
>      > +     (((volatile __u8 *)&(src))[(b) + (sizeof(src) - (s))] << 8
>     * ((s) - (b) - 1))
>      > +#define load_word_ntoh(src, w, s) \
>      > +     (((volatile __u16 *)&(src))[w] << 16 * (((s) / 2) - (w) - 1))
>     These names, load_byte_ntoh() and load_word_ntoh(), are miss-leading.
> 
>     They don't actually do byte-order conversion from network order to host
>     order. Network order is big endian. 0xdeadbeef in u32 should be stored
>     as the sequence of
> 
>         0xde, 0xad, 0xbe, 0xef
> 
>     The little endian implementation of load_word_ntoh() provided here will
>     return 0xadde and 0xefbe0000. However, a network order to host order
>     conversion should return 0xbeef and 0xdead0000 for little endian.
> 
>     The little endian implementation of load_byte_ntoh() here returns 0xde,
>     0xad00, 0xbe0000, and 0xef000000. However, a network to host order
>     conversion should return 0xef, 0xbe00, 0xad0000, and 0xde00000.
> 
>     So, they just access raw data following the host byte order, not
>     providing any byte order conversion.
> 
> 
>      > +#else
>      > +# error "Fix your compiler's __BYTE_ORDER__?!"
>      > +#endif
>      > +
>      > +#endif
> 

[PATCH v2 bpf-next 2/6] selftests/bpf: Implement socket kfuncs for bpf_testmod

[Jordan Rife]

This patch adds a set of kfuncs to bpf_testmod that can be used to
manipulate a socket from kernel space.

Signed-off-by: Jordan Rife <jrife@google.com>
---
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 139 ++++++++++++++++++
 .../bpf/bpf_testmod/bpf_testmod_kfunc.h       |  27 ++++
 2 files changed, 166 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 39ad96a18123f..663df8148097e 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -10,18 +10,29 @@
 #include <linux/percpu-defs.h>
 #include <linux/sysfs.h>
 #include <linux/tracepoint.h>
+#include <linux/net.h>
+#include <linux/socket.h>
+#include <linux/nsproxy.h>
+#include <linux/inet.h>
+#include <linux/in.h>
+#include <linux/in6.h>
+#include <linux/un.h>
+#include <net/sock.h>
 #include "bpf_testmod.h"
 #include "bpf_testmod_kfunc.h"
 
 #define CREATE_TRACE_POINTS
 #include "bpf_testmod-events.h"
 
+#define CONNECT_TIMEOUT_SEC 1
+
 typedef int (*func_proto_typedef)(long);
 typedef int (*func_proto_typedef_nested1)(func_proto_typedef);
 typedef int (*func_proto_typedef_nested2)(func_proto_typedef_nested1);
 
 DEFINE_PER_CPU(int, bpf_testmod_ksym_percpu) = 123;
 long bpf_testmod_test_struct_arg_result;
+static struct socket *sock;
 
 struct bpf_testmod_struct_arg_1 {
 	int a;
@@ -494,6 +505,124 @@ __bpf_kfunc static u32 bpf_kfunc_call_test_static_unused_arg(u32 arg, u32 unused
 	return arg;
 }
 
+__bpf_kfunc int bpf_kfunc_init_sock(struct init_sock_args *args)
+{
+	int proto;
+
+	if (sock)
+		pr_warn("%s called without releasing old sock", __func__);
+
+	switch (args->af) {
+	case AF_INET:
+	case AF_INET6:
+		proto = args->type == SOCK_STREAM ? IPPROTO_TCP : IPPROTO_UDP;
+		break;
+	case AF_UNIX:
+		proto = PF_UNIX;
+		break;
+	default:
+		pr_err("invalid address family %d\n", args->af);
+		return -EINVAL;
+	}
+
+	return sock_create_kern(&init_net, args->af, args->type, proto, &sock);
+}
+
+__bpf_kfunc void bpf_kfunc_close_sock(void)
+{
+	if (sock) {
+		sock_release(sock);
+		sock = NULL;
+	}
+}
+
+__bpf_kfunc int bpf_kfunc_call_kernel_connect(struct addr_args *args)
+{
+	/* Set timeout for call to kernel_connect() to prevent it from hanging,
+	 * and consider the connection attempt failed if it returns
+	 * -EINPROGRESS.
+	 */
+	sock->sk->sk_sndtimeo = CONNECT_TIMEOUT_SEC * HZ;
+
+	return kernel_connect(sock, (struct sockaddr *)&args->addr,
+			      args->addrlen, 0);
+}
+
+__bpf_kfunc int bpf_kfunc_call_kernel_bind(struct addr_args *args)
+{
+	return kernel_bind(sock, (struct sockaddr *)&args->addr, args->addrlen);
+}
+
+__bpf_kfunc int bpf_kfunc_call_kernel_listen(void)
+{
+	return kernel_listen(sock, 128);
+}
+
+__bpf_kfunc int bpf_kfunc_call_kernel_sendmsg(struct sendmsg_args *args)
+{
+	struct msghdr msg = {
+		.msg_name	= &args->addr.addr,
+		.msg_namelen	= args->addr.addrlen,
+	};
+	struct kvec iov;
+	int err;
+
+	iov.iov_base = args->msg;
+	iov.iov_len  = args->msglen;
+
+	err = kernel_sendmsg(sock, &msg, &iov, 1, args->msglen);
+	args->addr.addrlen = msg.msg_namelen;
+
+	return err;
+}
+
+__bpf_kfunc int bpf_kfunc_call_sock_sendmsg(struct sendmsg_args *args)
+{
+	struct msghdr msg = {
+		.msg_name	= &args->addr.addr,
+		.msg_namelen	= args->addr.addrlen,
+	};
+	struct kvec iov;
+	int err;
+
+	iov.iov_base = args->msg;
+	iov.iov_len  = args->msglen;
+
+	iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, &iov, 1, args->msglen);
+	err = sock_sendmsg(sock, &msg);
+	args->addr.addrlen = msg.msg_namelen;
+
+	return err;
+}
+
+__bpf_kfunc int bpf_kfunc_call_kernel_getsockname(struct addr_args *args)
+{
+	int err;
+
+	err = kernel_getsockname(sock, (struct sockaddr *)&args->addr);
+	if (err < 0)
+		goto out;
+
+	args->addrlen = err;
+	err = 0;
+out:
+	return err;
+}
+
+__bpf_kfunc int bpf_kfunc_call_kernel_getpeername(struct addr_args *args)
+{
+	int err;
+
+	err = kernel_getpeername(sock, (struct sockaddr *)&args->addr);
+	if (err < 0)
+		goto out;
+
+	args->addrlen = err;
+	err = 0;
+out:
+	return err;
+}
+
 BTF_KFUNCS_START(bpf_testmod_check_kfunc_ids)
 BTF_ID_FLAGS(func, bpf_testmod_test_mod_kfunc)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
@@ -520,6 +649,15 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS | KF_RCU)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_static_unused_arg)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_offset)
+BTF_ID_FLAGS(func, bpf_kfunc_init_sock)
+BTF_ID_FLAGS(func, bpf_kfunc_close_sock)
+BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_connect)
+BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_bind)
+BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_listen)
+BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_sendmsg)
+BTF_ID_FLAGS(func, bpf_kfunc_call_sock_sendmsg)
+BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_getsockname)
+BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_getpeername)
 BTF_KFUNCS_END(bpf_testmod_check_kfunc_ids)
 
 static int bpf_testmod_ops_init(struct btf *btf)
@@ -650,6 +788,7 @@ static int bpf_testmod_init(void)
 		return ret;
 	if (bpf_fentry_test1(0) < 0)
 		return -EINVAL;
+	sock = NULL;
 	return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
 }
 
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
index 7c664dd610597..cdf7769a7d8ca 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
@@ -64,6 +64,22 @@ struct prog_test_fail3 {
 	char arr2[];
 };
 
+struct init_sock_args {
+	int af;
+	int type;
+};
+
+struct addr_args {
+	char addr[sizeof(struct __kernel_sockaddr_storage)];
+	int addrlen;
+};
+
+struct sendmsg_args {
+	struct addr_args addr;
+	char msg[10];
+	int msglen;
+};
+
 struct prog_test_ref_kfunc *
 bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) __ksym;
 void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
@@ -106,4 +122,15 @@ void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p);
 void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len);
 
 void bpf_kfunc_common_test(void) __ksym;
+
+int bpf_kfunc_init_sock(struct init_sock_args *args) __ksym;
+void bpf_kfunc_close_sock(void) __ksym;
+int bpf_kfunc_call_kernel_connect(struct addr_args *args) __ksym;
+int bpf_kfunc_call_kernel_bind(struct addr_args *args) __ksym;
+int bpf_kfunc_call_kernel_listen(void) __ksym;
+int bpf_kfunc_call_kernel_sendmsg(struct sendmsg_args *args) __ksym;
+int bpf_kfunc_call_sock_sendmsg(struct sendmsg_args *args) __ksym;
+int bpf_kfunc_call_kernel_getsockname(struct addr_args *args) __ksym;
+int bpf_kfunc_call_kernel_getpeername(struct addr_args *args) __ksym;
+
 #endif /* _BPF_TESTMOD_KFUNC_H */
-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCH v2 bpf-next 2/6] selftests/bpf: Implement socket kfuncs for bpf_testmod

[Kui-Feng Lee]

On 4/12/24 09:52, Jordan Rife wrote:
> This patch adds a set of kfuncs to bpf_testmod that can be used to
> manipulate a socket from kernel space.
> 
> Signed-off-by: Jordan Rife <jrife@google.com>
> ---
>   .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 139 ++++++++++++++++++
>   .../bpf/bpf_testmod/bpf_testmod_kfunc.h       |  27 ++++
>   2 files changed, 166 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index 39ad96a18123f..663df8148097e 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -10,18 +10,29 @@
>   #include <linux/percpu-defs.h>
>   #include <linux/sysfs.h>
>   #include <linux/tracepoint.h>
> +#include <linux/net.h>
> +#include <linux/socket.h>
> +#include <linux/nsproxy.h>
> +#include <linux/inet.h>
> +#include <linux/in.h>
> +#include <linux/in6.h>
> +#include <linux/un.h>
> +#include <net/sock.h>
>   #include "bpf_testmod.h"
>   #include "bpf_testmod_kfunc.h"
>   
>   #define CREATE_TRACE_POINTS
>   #include "bpf_testmod-events.h"
>   
> +#define CONNECT_TIMEOUT_SEC 1
> +
>   typedef int (*func_proto_typedef)(long);
>   typedef int (*func_proto_typedef_nested1)(func_proto_typedef);
>   typedef int (*func_proto_typedef_nested2)(func_proto_typedef_nested1);
>   
>   DEFINE_PER_CPU(int, bpf_testmod_ksym_percpu) = 123;
>   long bpf_testmod_test_struct_arg_result;
> +static struct socket *sock;
>   
>   struct bpf_testmod_struct_arg_1 {
>   	int a;
> @@ -494,6 +505,124 @@ __bpf_kfunc static u32 bpf_kfunc_call_test_static_unused_arg(u32 arg, u32 unused
>   	return arg;
>   }
>   
> +__bpf_kfunc int bpf_kfunc_init_sock(struct init_sock_args *args)
> +{
> +	int proto;
> +
> +	if (sock)
> +		pr_warn("%s called without releasing old sock", __func__);
> +
> +	switch (args->af) {
> +	case AF_INET:
> +	case AF_INET6:
> +		proto = args->type == SOCK_STREAM ? IPPROTO_TCP : IPPROTO_UDP;
> +		break;
> +	case AF_UNIX:
> +		proto = PF_UNIX;
> +		break;
> +	default:
> +		pr_err("invalid address family %d\n", args->af);
> +		return -EINVAL;
> +	}
> +
> +	return sock_create_kern(&init_net, args->af, args->type, proto, &sock);
> +}
> +
> +__bpf_kfunc void bpf_kfunc_close_sock(void)
> +{
> +	if (sock) {
> +		sock_release(sock);
> +		sock = NULL;
> +	}
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_connect(struct addr_args *args)
> +{
> +	/* Set timeout for call to kernel_connect() to prevent it from hanging,
> +	 * and consider the connection attempt failed if it returns
> +	 * -EINPROGRESS.
> +	 */
> +	sock->sk->sk_sndtimeo = CONNECT_TIMEOUT_SEC * HZ;
> +
> +	return kernel_connect(sock, (struct sockaddr *)&args->addr,
> +			      args->addrlen, 0);
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_bind(struct addr_args *args)
> +{
> +	return kernel_bind(sock, (struct sockaddr *)&args->addr, args->addrlen);
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_listen(void)
> +{
> +	return kernel_listen(sock, 128);
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_sendmsg(struct sendmsg_args *args)
> +{
> +	struct msghdr msg = {
> +		.msg_name	= &args->addr.addr,
> +		.msg_namelen	= args->addr.addrlen,
> +	};
> +	struct kvec iov;
> +	int err;
> +
> +	iov.iov_base = args->msg;
> +	iov.iov_len  = args->msglen;

It would be better to check if args->msglen > sizeof(arg->msg) although
this function is just for test cases. Same for args->addr.addrlen.

> +
> +	err = kernel_sendmsg(sock, &msg, &iov, 1, args->msglen);
> +	args->addr.addrlen = msg.msg_namelen;
> +
> +	return err;
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_sock_sendmsg(struct sendmsg_args *args)
> +{
> +	struct msghdr msg = {
> +		.msg_name	= &args->addr.addr,
> +		.msg_namelen	= args->addr.addrlen,
> +	};
> +	struct kvec iov;
> +	int err;
> +
> +	iov.iov_base = args->msg;
> +	iov.iov_len  = args->msglen;
> +
> +	iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, &iov, 1, args->msglen);
> +	err = sock_sendmsg(sock, &msg);
> +	args->addr.addrlen = msg.msg_namelen;
> +
> +	return err;
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_getsockname(struct addr_args *args)
> +{
> +	int err;
> +
> +	err = kernel_getsockname(sock, (struct sockaddr *)&args->addr);
> +	if (err < 0)
> +		goto out;
> +
> +	args->addrlen = err;
> +	err = 0;
> +out:
> +	return err;
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_getpeername(struct addr_args *args)
> +{
> +	int err;
> +
> +	err = kernel_getpeername(sock, (struct sockaddr *)&args->addr);
> +	if (err < 0)
> +		goto out;
> +
> +	args->addrlen = err;
> +	err = 0;
> +out:
> +	return err;
> +}
> +
>   BTF_KFUNCS_START(bpf_testmod_check_kfunc_ids)
>   BTF_ID_FLAGS(func, bpf_testmod_test_mod_kfunc)
>   BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
> @@ -520,6 +649,15 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS | KF_RCU)
>   BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE)
>   BTF_ID_FLAGS(func, bpf_kfunc_call_test_static_unused_arg)
>   BTF_ID_FLAGS(func, bpf_kfunc_call_test_offset)
> +BTF_ID_FLAGS(func, bpf_kfunc_init_sock)
> +BTF_ID_FLAGS(func, bpf_kfunc_close_sock)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_connect)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_bind)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_listen)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_sendmsg)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_sock_sendmsg)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_getsockname)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_getpeername)
>   BTF_KFUNCS_END(bpf_testmod_check_kfunc_ids)
>   
>   static int bpf_testmod_ops_init(struct btf *btf)
> @@ -650,6 +788,7 @@ static int bpf_testmod_init(void)
>   		return ret;
>   	if (bpf_fentry_test1(0) < 0)
>   		return -EINVAL;
> +	sock = NULL;
>   	return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
>   }
>   
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> index 7c664dd610597..cdf7769a7d8ca 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> @@ -64,6 +64,22 @@ struct prog_test_fail3 {
>   	char arr2[];
>   };
>   
> +struct init_sock_args {
> +	int af;
> +	int type;
> +};
> +
> +struct addr_args {
> +	char addr[sizeof(struct __kernel_sockaddr_storage)];
> +	int addrlen;
> +};
> +
> +struct sendmsg_args {
> +	struct addr_args addr;
> +	char msg[10];
> +	int msglen;
> +};
> +
>   struct prog_test_ref_kfunc *
>   bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) __ksym;
>   void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> @@ -106,4 +122,15 @@ void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p);
>   void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len);
>   
>   void bpf_kfunc_common_test(void) __ksym;
> +
> +int bpf_kfunc_init_sock(struct init_sock_args *args) __ksym;
> +void bpf_kfunc_close_sock(void) __ksym;
> +int bpf_kfunc_call_kernel_connect(struct addr_args *args) __ksym;
> +int bpf_kfunc_call_kernel_bind(struct addr_args *args) __ksym;
> +int bpf_kfunc_call_kernel_listen(void) __ksym;
> +int bpf_kfunc_call_kernel_sendmsg(struct sendmsg_args *args) __ksym;
> +int bpf_kfunc_call_sock_sendmsg(struct sendmsg_args *args) __ksym;
> +int bpf_kfunc_call_kernel_getsockname(struct addr_args *args) __ksym;
> +int bpf_kfunc_call_kernel_getpeername(struct addr_args *args) __ksym;
> +
>   #endif /* _BPF_TESTMOD_KFUNC_H */

Re: [PATCH v2 bpf-next 2/6] selftests/bpf: Implement socket kfuncs for bpf_testmod

[Jordan Rife]

> It would be better to check if args->msglen > sizeof(arg->msg) although
> this function is just for test cases. Same for args->addr.addrlen.

Ack. I will add this.

Thanks,
Jordan


On Fri, Apr 12, 2024 at 6:26 PM Kui-Feng Lee <sinquersw@gmail.com> wrote:
>
>
>
> On 4/12/24 09:52, Jordan Rife wrote:
> > This patch adds a set of kfuncs to bpf_testmod that can be used to
> > manipulate a socket from kernel space.
> >
> > Signed-off-by: Jordan Rife <jrife@google.com>
> > ---
> >   .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 139 ++++++++++++++++++
> >   .../bpf/bpf_testmod/bpf_testmod_kfunc.h       |  27 ++++
> >   2 files changed, 166 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > index 39ad96a18123f..663df8148097e 100644
> > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > @@ -10,18 +10,29 @@
> >   #include <linux/percpu-defs.h>
> >   #include <linux/sysfs.h>
> >   #include <linux/tracepoint.h>
> > +#include <linux/net.h>
> > +#include <linux/socket.h>
> > +#include <linux/nsproxy.h>
> > +#include <linux/inet.h>
> > +#include <linux/in.h>
> > +#include <linux/in6.h>
> > +#include <linux/un.h>
> > +#include <net/sock.h>
> >   #include "bpf_testmod.h"
> >   #include "bpf_testmod_kfunc.h"
> >
> >   #define CREATE_TRACE_POINTS
> >   #include "bpf_testmod-events.h"
> >
> > +#define CONNECT_TIMEOUT_SEC 1
> > +
> >   typedef int (*func_proto_typedef)(long);
> >   typedef int (*func_proto_typedef_nested1)(func_proto_typedef);
> >   typedef int (*func_proto_typedef_nested2)(func_proto_typedef_nested1);
> >
> >   DEFINE_PER_CPU(int, bpf_testmod_ksym_percpu) = 123;
> >   long bpf_testmod_test_struct_arg_result;
> > +static struct socket *sock;
> >
> >   struct bpf_testmod_struct_arg_1 {
> >       int a;
> > @@ -494,6 +505,124 @@ __bpf_kfunc static u32 bpf_kfunc_call_test_static_unused_arg(u32 arg, u32 unused
> >       return arg;
> >   }
> >
> > +__bpf_kfunc int bpf_kfunc_init_sock(struct init_sock_args *args)
> > +{
> > +     int proto;
> > +
> > +     if (sock)
> > +             pr_warn("%s called without releasing old sock", __func__);
> > +
> > +     switch (args->af) {
> > +     case AF_INET:
> > +     case AF_INET6:
> > +             proto = args->type == SOCK_STREAM ? IPPROTO_TCP : IPPROTO_UDP;
> > +             break;
> > +     case AF_UNIX:
> > +             proto = PF_UNIX;
> > +             break;
> > +     default:
> > +             pr_err("invalid address family %d\n", args->af);
> > +             return -EINVAL;
> > +     }
> > +
> > +     return sock_create_kern(&init_net, args->af, args->type, proto, &sock);
> > +}
> > +
> > +__bpf_kfunc void bpf_kfunc_close_sock(void)
> > +{
> > +     if (sock) {
> > +             sock_release(sock);
> > +             sock = NULL;
> > +     }
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_connect(struct addr_args *args)
> > +{
> > +     /* Set timeout for call to kernel_connect() to prevent it from hanging,
> > +      * and consider the connection attempt failed if it returns
> > +      * -EINPROGRESS.
> > +      */
> > +     sock->sk->sk_sndtimeo = CONNECT_TIMEOUT_SEC * HZ;
> > +
> > +     return kernel_connect(sock, (struct sockaddr *)&args->addr,
> > +                           args->addrlen, 0);
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_bind(struct addr_args *args)
> > +{
> > +     return kernel_bind(sock, (struct sockaddr *)&args->addr, args->addrlen);
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_listen(void)
> > +{
> > +     return kernel_listen(sock, 128);
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_sendmsg(struct sendmsg_args *args)
> > +{
> > +     struct msghdr msg = {
> > +             .msg_name       = &args->addr.addr,
> > +             .msg_namelen    = args->addr.addrlen,
> > +     };
> > +     struct kvec iov;
> > +     int err;
> > +
> > +     iov.iov_base = args->msg;
> > +     iov.iov_len  = args->msglen;
>
> It would be better to check if args->msglen > sizeof(arg->msg) although
> this function is just for test cases. Same for args->addr.addrlen.
>
> > +
> > +     err = kernel_sendmsg(sock, &msg, &iov, 1, args->msglen);
> > +     args->addr.addrlen = msg.msg_namelen;
> > +
> > +     return err;
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_sock_sendmsg(struct sendmsg_args *args)
> > +{
> > +     struct msghdr msg = {
> > +             .msg_name       = &args->addr.addr,
> > +             .msg_namelen    = args->addr.addrlen,
> > +     };
> > +     struct kvec iov;
> > +     int err;
> > +
> > +     iov.iov_base = args->msg;
> > +     iov.iov_len  = args->msglen;
> > +
> > +     iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, &iov, 1, args->msglen);
> > +     err = sock_sendmsg(sock, &msg);
> > +     args->addr.addrlen = msg.msg_namelen;
> > +
> > +     return err;
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_getsockname(struct addr_args *args)
> > +{
> > +     int err;
> > +
> > +     err = kernel_getsockname(sock, (struct sockaddr *)&args->addr);
> > +     if (err < 0)
> > +             goto out;
> > +
> > +     args->addrlen = err;
> > +     err = 0;
> > +out:
> > +     return err;
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_getpeername(struct addr_args *args)
> > +{
> > +     int err;
> > +
> > +     err = kernel_getpeername(sock, (struct sockaddr *)&args->addr);
> > +     if (err < 0)
> > +             goto out;
> > +
> > +     args->addrlen = err;
> > +     err = 0;
> > +out:
> > +     return err;
> > +}
> > +
> >   BTF_KFUNCS_START(bpf_testmod_check_kfunc_ids)
> >   BTF_ID_FLAGS(func, bpf_testmod_test_mod_kfunc)
> >   BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
> > @@ -520,6 +649,15 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS | KF_RCU)
> >   BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE)
> >   BTF_ID_FLAGS(func, bpf_kfunc_call_test_static_unused_arg)
> >   BTF_ID_FLAGS(func, bpf_kfunc_call_test_offset)
> > +BTF_ID_FLAGS(func, bpf_kfunc_init_sock)
> > +BTF_ID_FLAGS(func, bpf_kfunc_close_sock)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_connect)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_bind)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_listen)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_sendmsg)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_sock_sendmsg)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_getsockname)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_getpeername)
> >   BTF_KFUNCS_END(bpf_testmod_check_kfunc_ids)
> >
> >   static int bpf_testmod_ops_init(struct btf *btf)
> > @@ -650,6 +788,7 @@ static int bpf_testmod_init(void)
> >               return ret;
> >       if (bpf_fentry_test1(0) < 0)
> >               return -EINVAL;
> > +     sock = NULL;
> >       return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
> >   }
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> > index 7c664dd610597..cdf7769a7d8ca 100644
> > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> > @@ -64,6 +64,22 @@ struct prog_test_fail3 {
> >       char arr2[];
> >   };
> >
> > +struct init_sock_args {
> > +     int af;
> > +     int type;
> > +};
> > +
> > +struct addr_args {
> > +     char addr[sizeof(struct __kernel_sockaddr_storage)];
> > +     int addrlen;
> > +};
> > +
> > +struct sendmsg_args {
> > +     struct addr_args addr;
> > +     char msg[10];
> > +     int msglen;
> > +};
> > +
> >   struct prog_test_ref_kfunc *
> >   bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) __ksym;
> >   void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> > @@ -106,4 +122,15 @@ void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p);
> >   void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len);
> >
> >   void bpf_kfunc_common_test(void) __ksym;
> > +
> > +int bpf_kfunc_init_sock(struct init_sock_args *args) __ksym;
> > +void bpf_kfunc_close_sock(void) __ksym;
> > +int bpf_kfunc_call_kernel_connect(struct addr_args *args) __ksym;
> > +int bpf_kfunc_call_kernel_bind(struct addr_args *args) __ksym;
> > +int bpf_kfunc_call_kernel_listen(void) __ksym;
> > +int bpf_kfunc_call_kernel_sendmsg(struct sendmsg_args *args) __ksym;
> > +int bpf_kfunc_call_sock_sendmsg(struct sendmsg_args *args) __ksym;
> > +int bpf_kfunc_call_kernel_getsockname(struct addr_args *args) __ksym;
> > +int bpf_kfunc_call_kernel_getpeername(struct addr_args *args) __ksym;
> > +
> >   #endif /* _BPF_TESTMOD_KFUNC_H */

Re: [PATCH v2 bpf-next 2/6] selftests/bpf: Implement socket kfuncs for bpf_testmod

[Martin KaFai Lau]

On 4/12/24 9:52 AM, Jordan Rife wrote:
> This patch adds a set of kfuncs to bpf_testmod that can be used to
> manipulate a socket from kernel space.
> 
> Signed-off-by: Jordan Rife <jrife@google.com>
> ---
>   .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 139 ++++++++++++++++++
>   .../bpf/bpf_testmod/bpf_testmod_kfunc.h       |  27 ++++
>   2 files changed, 166 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index 39ad96a18123f..663df8148097e 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -10,18 +10,29 @@
>   #include <linux/percpu-defs.h>
>   #include <linux/sysfs.h>
>   #include <linux/tracepoint.h>
> +#include <linux/net.h>
> +#include <linux/socket.h>
> +#include <linux/nsproxy.h>
> +#include <linux/inet.h>
> +#include <linux/in.h>
> +#include <linux/in6.h>
> +#include <linux/un.h>
> +#include <net/sock.h>
>   #include "bpf_testmod.h"
>   #include "bpf_testmod_kfunc.h"
>   
>   #define CREATE_TRACE_POINTS
>   #include "bpf_testmod-events.h"
>   
> +#define CONNECT_TIMEOUT_SEC 1
> +
>   typedef int (*func_proto_typedef)(long);
>   typedef int (*func_proto_typedef_nested1)(func_proto_typedef);
>   typedef int (*func_proto_typedef_nested2)(func_proto_typedef_nested1);
>   
>   DEFINE_PER_CPU(int, bpf_testmod_ksym_percpu) = 123;
>   long bpf_testmod_test_struct_arg_result;
> +static struct socket *sock;
>   
>   struct bpf_testmod_struct_arg_1 {
>   	int a;
> @@ -494,6 +505,124 @@ __bpf_kfunc static u32 bpf_kfunc_call_test_static_unused_arg(u32 arg, u32 unused
>   	return arg;
>   }
>   
> +__bpf_kfunc int bpf_kfunc_init_sock(struct init_sock_args *args)
> +{
> +	int proto;
> +
> +	if (sock)
> +		pr_warn("%s called without releasing old sock", __func__);

hmm...this global sock pointer is quite unease. e.g. what if multiple tasks 
trying to use init/close/connect... in parallel.

Storing sock in a bpf map will be better but that may be overkill for testing. 
Can a separate global lock/mutex (not the lock_sock) be acquired first before 
using the sock pointer in the kfuncs?

> +
> +	switch (args->af) {
> +	case AF_INET:
> +	case AF_INET6:
> +		proto = args->type == SOCK_STREAM ? IPPROTO_TCP : IPPROTO_UDP;
> +		break;
> +	case AF_UNIX:
> +		proto = PF_UNIX;
> +		break;
> +	default:
> +		pr_err("invalid address family %d\n", args->af);
> +		return -EINVAL;
> +	}
> +
> +	return sock_create_kern(&init_net, args->af, args->type, proto, &sock);
> +}
> +
> +__bpf_kfunc void bpf_kfunc_close_sock(void)
> +{
> +	if (sock) {
> +		sock_release(sock);

bpf_testmod_exit() should probably do this NULL check and sock_release() also.

> +		sock = NULL;
> +	}
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_connect(struct addr_args *args)
> +{
> +	/* Set timeout for call to kernel_connect() to prevent it from hanging,
> +	 * and consider the connection attempt failed if it returns
> +	 * -EINPROGRESS.
> +	 */
> +	sock->sk->sk_sndtimeo = CONNECT_TIMEOUT_SEC * HZ;

Is it better to set sk_sndtimeo in bpf_kfunc_init_sock() ?

> +
> +	return kernel_connect(sock, (struct sockaddr *)&args->addr,
> +			      args->addrlen, 0);
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_bind(struct addr_args *args)
> +{
> +	return kernel_bind(sock, (struct sockaddr *)&args->addr, args->addrlen);
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_listen(void)
> +{
> +	return kernel_listen(sock, 128);
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_sendmsg(struct sendmsg_args *args)
> +{
> +	struct msghdr msg = {
> +		.msg_name	= &args->addr.addr,
> +		.msg_namelen	= args->addr.addrlen,
> +	};
> +	struct kvec iov;
> +	int err;
> +
> +	iov.iov_base = args->msg;
> +	iov.iov_len  = args->msglen;
> +
> +	err = kernel_sendmsg(sock, &msg, &iov, 1, args->msglen);
> +	args->addr.addrlen = msg.msg_namelen;
> +
> +	return err;
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_sock_sendmsg(struct sendmsg_args *args)
> +{
> +	struct msghdr msg = {
> +		.msg_name	= &args->addr.addr,
> +		.msg_namelen	= args->addr.addrlen,
> +	};
> +	struct kvec iov;
> +	int err;
> +
> +	iov.iov_base = args->msg;
> +	iov.iov_len  = args->msglen;
> +
> +	iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, &iov, 1, args->msglen);
> +	err = sock_sendmsg(sock, &msg);
> +	args->addr.addrlen = msg.msg_namelen;
> +
> +	return err;
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_getsockname(struct addr_args *args)
> +{
> +	int err;
> +
> +	err = kernel_getsockname(sock, (struct sockaddr *)&args->addr);
> +	if (err < 0)
> +		goto out;
> +
> +	args->addrlen = err;
> +	err = 0;
> +out:
> +	return err;
> +}
> +
> +__bpf_kfunc int bpf_kfunc_call_kernel_getpeername(struct addr_args *args)
> +{
> +	int err;
> +
> +	err = kernel_getpeername(sock, (struct sockaddr *)&args->addr);
> +	if (err < 0)
> +		goto out;
> +
> +	args->addrlen = err;
> +	err = 0;
> +out:
> +	return err;
> +}
> +
>   BTF_KFUNCS_START(bpf_testmod_check_kfunc_ids)
>   BTF_ID_FLAGS(func, bpf_testmod_test_mod_kfunc)
>   BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
> @@ -520,6 +649,15 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS | KF_RCU)
>   BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE)
>   BTF_ID_FLAGS(func, bpf_kfunc_call_test_static_unused_arg)
>   BTF_ID_FLAGS(func, bpf_kfunc_call_test_offset)
> +BTF_ID_FLAGS(func, bpf_kfunc_init_sock)
> +BTF_ID_FLAGS(func, bpf_kfunc_close_sock)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_connect)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_bind)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_listen)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_sendmsg)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_sock_sendmsg)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_getsockname)
> +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_getpeername)

All these new kfunc should have the KF_SLEEPABLE flag.

>   BTF_KFUNCS_END(bpf_testmod_check_kfunc_ids)
>   
>   static int bpf_testmod_ops_init(struct btf *btf)
> @@ -650,6 +788,7 @@ static int bpf_testmod_init(void)
>   		return ret;
>   	if (bpf_fentry_test1(0) < 0)
>   		return -EINVAL;
> +	sock = NULL;
>   	return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
>   }
>   
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> index 7c664dd610597..cdf7769a7d8ca 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> @@ -64,6 +64,22 @@ struct prog_test_fail3 {
>   	char arr2[];
>   };
>   
> +struct init_sock_args {
> +	int af;
> +	int type;
> +};
> +
> +struct addr_args {
> +	char addr[sizeof(struct __kernel_sockaddr_storage)];

nit. Can "struct sockaddr_storage addr;" be directly used instead of a char array?

> +	int addrlen;
> +};
> +
> +struct sendmsg_args {
> +	struct addr_args addr;
> +	char msg[10];
> +	int msglen;
> +};
> +
>   struct prog_test_ref_kfunc *
>   bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) __ksym;
>   void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> @@ -106,4 +122,15 @@ void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p);
>   void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len);
>   
>   void bpf_kfunc_common_test(void) __ksym;
> +
> +int bpf_kfunc_init_sock(struct init_sock_args *args) __ksym;
> +void bpf_kfunc_close_sock(void) __ksym;
> +int bpf_kfunc_call_kernel_connect(struct addr_args *args) __ksym;
> +int bpf_kfunc_call_kernel_bind(struct addr_args *args) __ksym;
> +int bpf_kfunc_call_kernel_listen(void) __ksym;
> +int bpf_kfunc_call_kernel_sendmsg(struct sendmsg_args *args) __ksym;
> +int bpf_kfunc_call_sock_sendmsg(struct sendmsg_args *args) __ksym;
> +int bpf_kfunc_call_kernel_getsockname(struct addr_args *args) __ksym;
> +int bpf_kfunc_call_kernel_getpeername(struct addr_args *args) __ksym;
> +
>   #endif /* _BPF_TESTMOD_KFUNC_H */

Re: [PATCH v2 bpf-next 2/6] selftests/bpf: Implement socket kfuncs for bpf_testmod

[Jordan Rife]

Martin,

Thank you for the detailed feedback.

> Can a separate global lock/mutex (not the lock_sock) be acquired first before
> using the sock pointer in the kfuncs?

Sure. I will add the mutex around the socket operations. As for the
single global sock pointer, I wanted to keep it simple in this patch
series to fulfill the current use case. I agree it might be overkill
for now to add the bpf map and such.

> Is it better to set sk_sndtimeo in bpf_kfunc_init_sock() ?
> All these new kfunc should have the KF_SLEEPABLE flag.
> bpf_testmod_exit() should probably do this NULL check and sock_release() also.

Ack. I will add this.

> nit. Can "struct sockaddr_storage addr;" be directly used instead of a char array?

When using "struct sockaddr_storage addr;" directly, the BPF program
fails to load with the following error message.

> libbpf: prog 'kernel_connect': BPF program load failed: Invalid argument
> libbpf: prog 'kernel_connect': -- BEGIN PROG LOAD LOG --
> 0: R1=ctx() R10=fp0
> ; return bpf_kfunc_call_kernel_connect(args); @ sock_addr_kern.c:26
> 0: (85) call bpf_kfunc_call_kernel_connect#99994
> arg#0 pointer type STRUCT addr_args must point to scalar, or struct with scalar
> processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
> -- END PROG LOAD LOG --
> libbpf: prog 'kernel_connect': failed to load: -22
> libbpf: failed to load object 'sock_addr_kern'
> libbpf: failed to load BPF skeleton 'sock_addr_kern': -22
> load_sock_addr_kern:FAIL:skel unexpected error: -22
> test_sock_addr:FAIL:load_sock_addr_kern unexpected error: -1 (errno 22)
> #288 sock_addr:FAIL

-Jordan

On Tue, Apr 16, 2024 at 2:43 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 4/12/24 9:52 AM, Jordan Rife wrote:
> > This patch adds a set of kfuncs to bpf_testmod that can be used to
> > manipulate a socket from kernel space.
> >
> > Signed-off-by: Jordan Rife <jrife@google.com>
> > ---
> >   .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 139 ++++++++++++++++++
> >   .../bpf/bpf_testmod/bpf_testmod_kfunc.h       |  27 ++++
> >   2 files changed, 166 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > index 39ad96a18123f..663df8148097e 100644
> > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > @@ -10,18 +10,29 @@
> >   #include <linux/percpu-defs.h>
> >   #include <linux/sysfs.h>
> >   #include <linux/tracepoint.h>
> > +#include <linux/net.h>
> > +#include <linux/socket.h>
> > +#include <linux/nsproxy.h>
> > +#include <linux/inet.h>
> > +#include <linux/in.h>
> > +#include <linux/in6.h>
> > +#include <linux/un.h>
> > +#include <net/sock.h>
> >   #include "bpf_testmod.h"
> >   #include "bpf_testmod_kfunc.h"
> >
> >   #define CREATE_TRACE_POINTS
> >   #include "bpf_testmod-events.h"
> >
> > +#define CONNECT_TIMEOUT_SEC 1
> > +
> >   typedef int (*func_proto_typedef)(long);
> >   typedef int (*func_proto_typedef_nested1)(func_proto_typedef);
> >   typedef int (*func_proto_typedef_nested2)(func_proto_typedef_nested1);
> >
> >   DEFINE_PER_CPU(int, bpf_testmod_ksym_percpu) = 123;
> >   long bpf_testmod_test_struct_arg_result;
> > +static struct socket *sock;
> >
> >   struct bpf_testmod_struct_arg_1 {
> >       int a;
> > @@ -494,6 +505,124 @@ __bpf_kfunc static u32 bpf_kfunc_call_test_static_unused_arg(u32 arg, u32 unused
> >       return arg;
> >   }
> >
> > +__bpf_kfunc int bpf_kfunc_init_sock(struct init_sock_args *args)
> > +{
> > +     int proto;
> > +
> > +     if (sock)
> > +             pr_warn("%s called without releasing old sock", __func__);
>
> hmm...this global sock pointer is quite unease. e.g. what if multiple tasks
> trying to use init/close/connect... in parallel.
>
> Storing sock in a bpf map will be better but that may be overkill for testing.
> Can a separate global lock/mutex (not the lock_sock) be acquired first before
> using the sock pointer in the kfuncs?
>
> > +
> > +     switch (args->af) {
> > +     case AF_INET:
> > +     case AF_INET6:
> > +             proto = args->type == SOCK_STREAM ? IPPROTO_TCP : IPPROTO_UDP;
> > +             break;
> > +     case AF_UNIX:
> > +             proto = PF_UNIX;
> > +             break;
> > +     default:
> > +             pr_err("invalid address family %d\n", args->af);
> > +             return -EINVAL;
> > +     }
> > +
> > +     return sock_create_kern(&init_net, args->af, args->type, proto, &sock);
> > +}
> > +
> > +__bpf_kfunc void bpf_kfunc_close_sock(void)
> > +{
> > +     if (sock) {
> > +             sock_release(sock);
>
> bpf_testmod_exit() should probably do this NULL check and sock_release() also.
>
> > +             sock = NULL;
> > +     }
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_connect(struct addr_args *args)
> > +{
> > +     /* Set timeout for call to kernel_connect() to prevent it from hanging,
> > +      * and consider the connection attempt failed if it returns
> > +      * -EINPROGRESS.
> > +      */
> > +     sock->sk->sk_sndtimeo = CONNECT_TIMEOUT_SEC * HZ;
>
> Is it better to set sk_sndtimeo in bpf_kfunc_init_sock() ?
>
> > +
> > +     return kernel_connect(sock, (struct sockaddr *)&args->addr,
> > +                           args->addrlen, 0);
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_bind(struct addr_args *args)
> > +{
> > +     return kernel_bind(sock, (struct sockaddr *)&args->addr, args->addrlen);
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_listen(void)
> > +{
> > +     return kernel_listen(sock, 128);
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_sendmsg(struct sendmsg_args *args)
> > +{
> > +     struct msghdr msg = {
> > +             .msg_name       = &args->addr.addr,
> > +             .msg_namelen    = args->addr.addrlen,
> > +     };
> > +     struct kvec iov;
> > +     int err;
> > +
> > +     iov.iov_base = args->msg;
> > +     iov.iov_len  = args->msglen;
> > +
> > +     err = kernel_sendmsg(sock, &msg, &iov, 1, args->msglen);
> > +     args->addr.addrlen = msg.msg_namelen;
> > +
> > +     return err;
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_sock_sendmsg(struct sendmsg_args *args)
> > +{
> > +     struct msghdr msg = {
> > +             .msg_name       = &args->addr.addr,
> > +             .msg_namelen    = args->addr.addrlen,
> > +     };
> > +     struct kvec iov;
> > +     int err;
> > +
> > +     iov.iov_base = args->msg;
> > +     iov.iov_len  = args->msglen;
> > +
> > +     iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, &iov, 1, args->msglen);
> > +     err = sock_sendmsg(sock, &msg);
> > +     args->addr.addrlen = msg.msg_namelen;
> > +
> > +     return err;
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_getsockname(struct addr_args *args)
> > +{
> > +     int err;
> > +
> > +     err = kernel_getsockname(sock, (struct sockaddr *)&args->addr);
> > +     if (err < 0)
> > +             goto out;
> > +
> > +     args->addrlen = err;
> > +     err = 0;
> > +out:
> > +     return err;
> > +}
> > +
> > +__bpf_kfunc int bpf_kfunc_call_kernel_getpeername(struct addr_args *args)
> > +{
> > +     int err;
> > +
> > +     err = kernel_getpeername(sock, (struct sockaddr *)&args->addr);
> > +     if (err < 0)
> > +             goto out;
> > +
> > +     args->addrlen = err;
> > +     err = 0;
> > +out:
> > +     return err;
> > +}
> > +
> >   BTF_KFUNCS_START(bpf_testmod_check_kfunc_ids)
> >   BTF_ID_FLAGS(func, bpf_testmod_test_mod_kfunc)
> >   BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
> > @@ -520,6 +649,15 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS | KF_RCU)
> >   BTF_ID_FLAGS(func, bpf_kfunc_call_test_destructive, KF_DESTRUCTIVE)
> >   BTF_ID_FLAGS(func, bpf_kfunc_call_test_static_unused_arg)
> >   BTF_ID_FLAGS(func, bpf_kfunc_call_test_offset)
> > +BTF_ID_FLAGS(func, bpf_kfunc_init_sock)
> > +BTF_ID_FLAGS(func, bpf_kfunc_close_sock)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_connect)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_bind)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_listen)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_sendmsg)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_sock_sendmsg)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_getsockname)
> > +BTF_ID_FLAGS(func, bpf_kfunc_call_kernel_getpeername)
>
> All these new kfunc should have the KF_SLEEPABLE flag.
>
> >   BTF_KFUNCS_END(bpf_testmod_check_kfunc_ids)
> >
> >   static int bpf_testmod_ops_init(struct btf *btf)
> > @@ -650,6 +788,7 @@ static int bpf_testmod_init(void)
> >               return ret;
> >       if (bpf_fentry_test1(0) < 0)
> >               return -EINVAL;
> > +     sock = NULL;
> >       return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
> >   }
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> > index 7c664dd610597..cdf7769a7d8ca 100644
> > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod_kfunc.h
> > @@ -64,6 +64,22 @@ struct prog_test_fail3 {
> >       char arr2[];
> >   };
> >
> > +struct init_sock_args {
> > +     int af;
> > +     int type;
> > +};
> > +
> > +struct addr_args {
> > +     char addr[sizeof(struct __kernel_sockaddr_storage)];
>
> nit. Can "struct sockaddr_storage addr;" be directly used instead of a char array?
>
> > +     int addrlen;
> > +};
> > +
> > +struct sendmsg_args {
> > +     struct addr_args addr;
> > +     char msg[10];
> > +     int msglen;
> > +};
> > +
> >   struct prog_test_ref_kfunc *
> >   bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) __ksym;
> >   void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
> > @@ -106,4 +122,15 @@ void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p);
> >   void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len);
> >
> >   void bpf_kfunc_common_test(void) __ksym;
> > +
> > +int bpf_kfunc_init_sock(struct init_sock_args *args) __ksym;
> > +void bpf_kfunc_close_sock(void) __ksym;
> > +int bpf_kfunc_call_kernel_connect(struct addr_args *args) __ksym;
> > +int bpf_kfunc_call_kernel_bind(struct addr_args *args) __ksym;
> > +int bpf_kfunc_call_kernel_listen(void) __ksym;
> > +int bpf_kfunc_call_kernel_sendmsg(struct sendmsg_args *args) __ksym;
> > +int bpf_kfunc_call_sock_sendmsg(struct sendmsg_args *args) __ksym;
> > +int bpf_kfunc_call_kernel_getsockname(struct addr_args *args) __ksym;
> > +int bpf_kfunc_call_kernel_getpeername(struct addr_args *args) __ksym;
> > +
> >   #endif /* _BPF_TESTMOD_KFUNC_H */
>

Re: [PATCH v2 bpf-next 2/6] selftests/bpf: Implement socket kfuncs for bpf_testmod

[Kui-Feng Lee]

On 4/17/24 09:59, Jordan Rife wrote:
>> nit. Can "struct sockaddr_storage addr;" be directly used instead of a char array?
> When using "struct sockaddr_storage addr;" directly, the BPF program
> fails to load with the following error message.
> 
>> libbpf: prog 'kernel_connect': BPF program load failed: Invalid argument
>> libbpf: prog 'kernel_connect': -- BEGIN PROG LOAD LOG --
>> 0: R1=ctx() R10=fp0
>> ; return bpf_kfunc_call_kernel_connect(args); @ sock_addr_kern.c:26
>> 0: (85) call bpf_kfunc_call_kernel_connect#99994
>> arg#0 pointer type STRUCT addr_args must point to scalar, or struct with scalar
>> processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
>> -- END PROG LOAD LOG --
>> libbpf: prog 'kernel_connect': failed to load: -22
>> libbpf: failed to load object 'sock_addr_kern'
>> libbpf: failed to load BPF skeleton 'sock_addr_kern': -22
>> load_sock_addr_kern:FAIL:skel unexpected error: -22
>> test_sock_addr:FAIL:load_sock_addr_kern unexpected error: -1 (errno 22)
>> #288 sock_addr:FAIL

I just looked into the definition of struct __kernel_sockaddr_sotrage
and the change log of this type. It has a pointer in it, causing this
error. According to the commit log, the pointer is there to fix an
alignment issue. I am curious if we can replace the pointer with
intptr_t to fix this error.

Of course, this should not block this patch set.

[PATCH v2 bpf-next 3/6] selftests/bpf: Implement BPF programs for kernel socket operations

[Jordan Rife]

This patch lays out a set of SYSCALL programs that can be used to invoke
the socket operation kfuncs in bpf_testmod, allowing a test program to
manipulate kernel socket operations from userspace.

Signed-off-by: Jordan Rife <jrife@google.com>
---
 .../selftests/bpf/progs/sock_addr_kern.c      | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/sock_addr_kern.c

diff --git a/tools/testing/selftests/bpf/progs/sock_addr_kern.c b/tools/testing/selftests/bpf/progs/sock_addr_kern.c
new file mode 100644
index 0000000000000..8386bb15ccdc1
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/sock_addr_kern.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Google LLC */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "../bpf_testmod/bpf_testmod_kfunc.h"
+
+SEC("syscall")
+int init_sock(struct init_sock_args *args)
+{
+	bpf_kfunc_init_sock(args);
+
+	return 0;
+}
+
+SEC("syscall")
+int close_sock(void *ctx)
+{
+	bpf_kfunc_close_sock();
+
+	return 0;
+}
+
+SEC("syscall")
+int kernel_connect(struct addr_args *args)
+{
+	return bpf_kfunc_call_kernel_connect(args);
+}
+
+SEC("syscall")
+int kernel_bind(struct addr_args *args)
+{
+	return bpf_kfunc_call_kernel_bind(args);
+}
+
+SEC("syscall")
+int kernel_listen(struct addr_args *args)
+{
+	return bpf_kfunc_call_kernel_listen();
+}
+
+SEC("syscall")
+int kernel_sendmsg(struct sendmsg_args *args)
+{
+	return bpf_kfunc_call_kernel_sendmsg(args);
+}
+
+SEC("syscall")
+int sock_sendmsg(struct sendmsg_args *args)
+{
+	return bpf_kfunc_call_sock_sendmsg(args);
+}
+
+SEC("syscall")
+int kernel_getsockname(struct addr_args *args)
+{
+	return bpf_kfunc_call_kernel_getsockname(args);
+}
+
+SEC("syscall")
+int kernel_getpeername(struct addr_args *args)
+{
+	return bpf_kfunc_call_kernel_getpeername(args);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.44.0.478.gd926399ef9-goog

[PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Jordan Rife]

This patch lays the groundwork for testing IPv4 and IPv6 sockaddr hooks
and their interaction with both socket syscalls and kernel functions
(e.g. kernel_connect, kernel_bind, etc.) and moves the test cases from
the old-style bpf/test_sock_addr.c self test into the sock_addr
prog_test.

Signed-off-by: Jordan Rife <jrife@google.com>
---
 .../selftests/bpf/prog_tests/sock_addr.c      | 391 ++++++++++++------
 1 file changed, 269 insertions(+), 122 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sock_addr.c b/tools/testing/selftests/bpf/prog_tests/sock_addr.c
index 5fd6177189915..92879b971a098 100644
--- a/tools/testing/selftests/bpf/prog_tests/sock_addr.c
+++ b/tools/testing/selftests/bpf/prog_tests/sock_addr.c
@@ -3,16 +3,43 @@
 
 #include "test_progs.h"
 
+#include "bind4_prog.skel.h"
+#include "bind6_prog.skel.h"
 #include "connect_unix_prog.skel.h"
+#include "connect4_prog.skel.h"
+#include "connect6_prog.skel.h"
+#include "sendmsg4_prog.skel.h"
+#include "sendmsg6_prog.skel.h"
+#include "recvmsg4_prog.skel.h"
+#include "recvmsg6_prog.skel.h"
 #include "sendmsg_unix_prog.skel.h"
 #include "recvmsg_unix_prog.skel.h"
 #include "getsockname_unix_prog.skel.h"
 #include "getpeername_unix_prog.skel.h"
 #include "network_helpers.h"
 
+#define TEST_IF_PREFIX          "test_sock_addr"
+#define TEST_IPV4               "127.0.0.4"
+#define TEST_IPV6               "::6"
+
+#define SERV4_IP                "192.168.1.254"
+#define SERV4_REWRITE_IP        "127.0.0.1"
+#define SRC4_IP                 "172.16.0.1"
+#define SRC4_REWRITE_IP         TEST_IPV4
+#define SERV4_PORT              4040
+#define SERV4_REWRITE_PORT      4444
+
+#define SERV6_IP                "face:b00c:1234:5678::abcd"
+#define SERV6_REWRITE_IP        "::1"
+#define SERV6_V4MAPPED_IP       "::ffff:192.168.0.4"
+#define SRC6_IP                 "::1"
+#define SRC6_REWRITE_IP         TEST_IPV6
+#define SERV6_PORT              6060
+#define SERV6_REWRITE_PORT      6666
+
 #define SERVUN_ADDRESS         "bpf_cgroup_unix_test"
 #define SERVUN_REWRITE_ADDRESS "bpf_cgroup_unix_test_rewrite"
-#define SRCUN_ADDRESS	       "bpf_cgroup_unix_test_src"
+#define SRCUN_ADDRESS          "bpf_cgroup_unix_test_src"
 
 enum sock_addr_test_type {
 	SOCK_ADDR_TEST_BIND,
@@ -43,130 +70,148 @@ struct sock_addr_test {
 	const char *expected_src_addr;
 };
 
-static void *connect_unix_prog_load(int cgroup_fd)
-{
-	struct connect_unix_prog *skel;
-
-	skel = connect_unix_prog__open_and_load();
-	if (!ASSERT_OK_PTR(skel, "skel_open"))
-		goto cleanup;
-
-	skel->links.connect_unix_prog = bpf_program__attach_cgroup(
-		skel->progs.connect_unix_prog, cgroup_fd);
-	if (!ASSERT_OK_PTR(skel->links.connect_unix_prog, "prog_attach"))
-		goto cleanup;
-
-	return skel;
-cleanup:
-	connect_unix_prog__destroy(skel);
-	return NULL;
-}
-
-static void connect_unix_prog_destroy(void *skel)
-{
-	connect_unix_prog__destroy(skel);
-}
-
-static void *sendmsg_unix_prog_load(int cgroup_fd)
-{
-	struct sendmsg_unix_prog *skel;
-
-	skel = sendmsg_unix_prog__open_and_load();
-	if (!ASSERT_OK_PTR(skel, "skel_open"))
-		goto cleanup;
-
-	skel->links.sendmsg_unix_prog = bpf_program__attach_cgroup(
-		skel->progs.sendmsg_unix_prog, cgroup_fd);
-	if (!ASSERT_OK_PTR(skel->links.sendmsg_unix_prog, "prog_attach"))
-		goto cleanup;
-
-	return skel;
-cleanup:
-	sendmsg_unix_prog__destroy(skel);
-	return NULL;
-}
-
-static void sendmsg_unix_prog_destroy(void *skel)
-{
-	sendmsg_unix_prog__destroy(skel);
-}
-
-static void *recvmsg_unix_prog_load(int cgroup_fd)
-{
-	struct recvmsg_unix_prog *skel;
-
-	skel = recvmsg_unix_prog__open_and_load();
-	if (!ASSERT_OK_PTR(skel, "skel_open"))
-		goto cleanup;
-
-	skel->links.recvmsg_unix_prog = bpf_program__attach_cgroup(
-		skel->progs.recvmsg_unix_prog, cgroup_fd);
-	if (!ASSERT_OK_PTR(skel->links.recvmsg_unix_prog, "prog_attach"))
-		goto cleanup;
-
-	return skel;
-cleanup:
-	recvmsg_unix_prog__destroy(skel);
-	return NULL;
-}
-
-static void recvmsg_unix_prog_destroy(void *skel)
-{
-	recvmsg_unix_prog__destroy(skel);
-}
-
-static void *getsockname_unix_prog_load(int cgroup_fd)
-{
-	struct getsockname_unix_prog *skel;
-
-	skel = getsockname_unix_prog__open_and_load();
-	if (!ASSERT_OK_PTR(skel, "skel_open"))
-		goto cleanup;
-
-	skel->links.getsockname_unix_prog = bpf_program__attach_cgroup(
-		skel->progs.getsockname_unix_prog, cgroup_fd);
-	if (!ASSERT_OK_PTR(skel->links.getsockname_unix_prog, "prog_attach"))
-		goto cleanup;
-
-	return skel;
-cleanup:
-	getsockname_unix_prog__destroy(skel);
-	return NULL;
+#define BPF_SKEL_FUNCS(skel_name, prog_name) \
+static void *skel_name##_load(int cgroup_fd) \
+{ \
+	struct skel_name *skel; \
+	skel = skel_name##__open_and_load(); \
+	if (!ASSERT_OK_PTR(skel, "skel_open")) \
+		goto cleanup; \
+	skel->links.prog_name = bpf_program__attach_cgroup( \
+		skel->progs.prog_name, cgroup_fd); \
+	if (!ASSERT_OK_PTR(skel->links.prog_name, "prog_attach")) \
+		goto cleanup; \
+	return skel; \
+cleanup: \
+	skel_name##__destroy(skel); \
+	return NULL; \
+} \
+static void skel_name##_destroy(void *skel) \
+{ \
+	skel_name##__destroy(skel); \
 }
 
-static void getsockname_unix_prog_destroy(void *skel)
-{
-	getsockname_unix_prog__destroy(skel);
-}
-
-static void *getpeername_unix_prog_load(int cgroup_fd)
-{
-	struct getpeername_unix_prog *skel;
-
-	skel = getpeername_unix_prog__open_and_load();
-	if (!ASSERT_OK_PTR(skel, "skel_open"))
-		goto cleanup;
-
-	skel->links.getpeername_unix_prog = bpf_program__attach_cgroup(
-		skel->progs.getpeername_unix_prog, cgroup_fd);
-	if (!ASSERT_OK_PTR(skel->links.getpeername_unix_prog, "prog_attach"))
-		goto cleanup;
-
-	return skel;
-cleanup:
-	getpeername_unix_prog__destroy(skel);
-	return NULL;
-}
-
-static void getpeername_unix_prog_destroy(void *skel)
-{
-	getpeername_unix_prog__destroy(skel);
-}
+BPF_SKEL_FUNCS(bind4_prog, bind_v4_prog);
+BPF_SKEL_FUNCS(bind6_prog, bind_v6_prog);
+BPF_SKEL_FUNCS(connect4_prog, connect_v4_prog);
+BPF_SKEL_FUNCS(connect6_prog, connect_v6_prog);
+BPF_SKEL_FUNCS(connect_unix_prog, connect_unix_prog);
+BPF_SKEL_FUNCS(sendmsg4_prog, sendmsg_v4_prog);
+BPF_SKEL_FUNCS(sendmsg6_prog, sendmsg_v6_prog);
+BPF_SKEL_FUNCS(sendmsg_unix_prog, sendmsg_unix_prog);
+BPF_SKEL_FUNCS(recvmsg4_prog, recvmsg4_prog);
+BPF_SKEL_FUNCS(recvmsg6_prog, recvmsg6_prog);
+BPF_SKEL_FUNCS(recvmsg_unix_prog, recvmsg_unix_prog);
+BPF_SKEL_FUNCS(getsockname_unix_prog, getsockname_unix_prog);
+BPF_SKEL_FUNCS(getpeername_unix_prog, getpeername_unix_prog);
 
 static struct sock_addr_test tests[] = {
+	/* bind - system calls */
+	{
+		SOCK_ADDR_TEST_BIND,
+		"bind4: bind (stream)",
+		bind4_prog_load,
+		bind4_prog_destroy,
+		AF_INET,
+		SOCK_STREAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+	},
+	{
+		SOCK_ADDR_TEST_BIND,
+		"bind4: bind (dgram)",
+		bind4_prog_load,
+		bind4_prog_destroy,
+		AF_INET,
+		SOCK_DGRAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+	},
+	{
+		SOCK_ADDR_TEST_BIND,
+		"bind6: bind (stream)",
+		bind6_prog_load,
+		bind6_prog_destroy,
+		AF_INET6,
+		SOCK_STREAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+	},
+	{
+		SOCK_ADDR_TEST_BIND,
+		"bind6: bind (dgram)",
+		bind6_prog_load,
+		bind6_prog_destroy,
+		AF_INET6,
+		SOCK_DGRAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+	},
+
+	/* connect - system calls */
+	{
+		SOCK_ADDR_TEST_CONNECT,
+		"connect4: connect (stream)",
+		connect4_prog_load,
+		connect4_prog_destroy,
+		AF_INET,
+		SOCK_STREAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+		SRC4_REWRITE_IP,
+	},
 	{
 		SOCK_ADDR_TEST_CONNECT,
-		"connect_unix",
+		"connect4: connect (dgram)",
+		connect4_prog_load,
+		connect4_prog_destroy,
+		AF_INET,
+		SOCK_DGRAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+		SRC4_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_CONNECT,
+		"connect6: connect (stream)",
+		connect6_prog_load,
+		connect6_prog_destroy,
+		AF_INET6,
+		SOCK_STREAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+		SRC6_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_CONNECT,
+		"connect6: connect (dgram)",
+		connect6_prog_load,
+		connect6_prog_destroy,
+		AF_INET6,
+		SOCK_DGRAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+		SRC6_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_CONNECT,
+		"connect_unix: connect (stream)",
 		connect_unix_prog_load,
 		connect_unix_prog_destroy,
 		AF_UNIX,
@@ -177,9 +222,37 @@ static struct sock_addr_test tests[] = {
 		0,
 		NULL,
 	},
+
+	/* sendmsg - system calls */
 	{
 		SOCK_ADDR_TEST_SENDMSG,
-		"sendmsg_unix",
+		"sendmsg4: sendmsg (dgram)",
+		sendmsg4_prog_load,
+		sendmsg4_prog_destroy,
+		AF_INET,
+		SOCK_DGRAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+		SRC4_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_SENDMSG,
+		"sendmsg6: sendmsg (dgram)",
+		sendmsg6_prog_load,
+		sendmsg6_prog_destroy,
+		AF_INET6,
+		SOCK_DGRAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+		SRC6_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_SENDMSG,
+		"sendmsg_unix: sendmsg (dgram)",
 		sendmsg_unix_prog_load,
 		sendmsg_unix_prog_destroy,
 		AF_UNIX,
@@ -190,9 +263,37 @@ static struct sock_addr_test tests[] = {
 		0,
 		NULL,
 	},
+
+	/* recvmsg - system calls */
+	{
+		SOCK_ADDR_TEST_RECVMSG,
+		"recvmsg4: recvfrom (dgram)",
+		recvmsg4_prog_load,
+		recvmsg4_prog_destroy,
+		AF_INET,
+		SOCK_DGRAM,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+		SERV4_IP,
+	},
 	{
 		SOCK_ADDR_TEST_RECVMSG,
-		"recvmsg_unix-dgram",
+		"recvmsg6: recvfrom (dgram)",
+		recvmsg6_prog_load,
+		recvmsg6_prog_destroy,
+		AF_INET6,
+		SOCK_DGRAM,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+		SERV6_IP,
+	},
+	{
+		SOCK_ADDR_TEST_RECVMSG,
+		"recvmsg_unix: recvfrom (dgram)",
 		recvmsg_unix_prog_load,
 		recvmsg_unix_prog_destroy,
 		AF_UNIX,
@@ -205,7 +306,7 @@ static struct sock_addr_test tests[] = {
 	},
 	{
 		SOCK_ADDR_TEST_RECVMSG,
-		"recvmsg_unix-stream",
+		"recvmsg_unix: recvfrom (stream)",
 		recvmsg_unix_prog_load,
 		recvmsg_unix_prog_destroy,
 		AF_UNIX,
@@ -216,6 +317,8 @@ static struct sock_addr_test tests[] = {
 		0,
 		SERVUN_ADDRESS,
 	},
+
+	/* getsockname - system calls */
 	{
 		SOCK_ADDR_TEST_GETSOCKNAME,
 		"getsockname_unix",
@@ -229,6 +332,8 @@ static struct sock_addr_test tests[] = {
 		0,
 		NULL,
 	},
+
+	/* getpeername - system calls */
 	{
 		SOCK_ADDR_TEST_GETPEERNAME,
 		"getpeername_unix",
@@ -558,11 +663,52 @@ static void test_getpeername(struct sock_addr_test *test)
 		close(serv);
 }
 
+static int ping_once(int ipv, const char *addr)
+{
+	const char *ping_cmd_prefix = "ping -";
+
+	if (!SYS_NOFAIL("type ping%d >/dev/null 2>&1", ipv))
+		ping_cmd_prefix = "ping";
+
+	return SYS_NOFAIL("%s%d -q -c 1 -W 1 %s >/dev/null 2>&1",
+			  ping_cmd_prefix, ipv, addr);
+}
+
+static int setup_test_env(void)
+{
+	SYS(err, "ip link add dev %s1 type veth peer name %s2", TEST_IF_PREFIX,
+	    TEST_IF_PREFIX);
+	SYS(err, "ip link set %s1 up", TEST_IF_PREFIX);
+	SYS(err, "ip link set %s2 up", TEST_IF_PREFIX);
+	SYS(err, "ip -4 addr add %s/8 dev %s1", TEST_IPV4, TEST_IF_PREFIX);
+	SYS(err, "ip -6 addr add %s/128 dev %s1", TEST_IPV6, TEST_IF_PREFIX);
+
+	int i;
+
+	for (i = 0; i < 5; i++) {
+		if (!ping_once(4, TEST_IPV4) && !ping_once(6, TEST_IPV6))
+			return 0;
+	}
+
+	ASSERT_FAIL("Timed out waiting for test IP to become available.");
+err:
+	return -1;
+}
+
+static void cleanup_test_env(void)
+{
+	SYS_NOFAIL("ip link del %s1 2>/dev/null", TEST_IF_PREFIX);
+	SYS_NOFAIL("ip link del %s2 2>/dev/null", TEST_IF_PREFIX);
+}
+
 void test_sock_addr(void)
 {
 	int cgroup_fd = -1;
 	void *skel;
 
+	if (!ASSERT_OK(setup_test_env(), "setup_test_env"))
+		goto cleanup;
+
 	cgroup_fd = test__join_cgroup("/sock_addr");
 	if (!ASSERT_GE(cgroup_fd, 0, "join_cgroup"))
 		goto cleanup;
@@ -609,4 +755,5 @@ void test_sock_addr(void)
 cleanup:
 	if (cgroup_fd >= 0)
 		close(cgroup_fd);
+	cleanup_test_env();
 }
-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Martin KaFai Lau]

On 4/12/24 9:52 AM, Jordan Rife wrote:
> This patch lays the groundwork for testing IPv4 and IPv6 sockaddr hooks
> and their interaction with both socket syscalls and kernel functions
> (e.g. kernel_connect, kernel_bind, etc.) and moves the test cases from
> the old-style bpf/test_sock_addr.c self test into the sock_addr
> prog_test.

Thanks for moving the existing sock_addr test to the test_progs.

> +static int ping_once(int ipv, const char *addr)
> +{
> +	const char *ping_cmd_prefix = "ping -";
> +
> +	if (!SYS_NOFAIL("type ping%d >/dev/null 2>&1", ipv))
> +		ping_cmd_prefix = "ping";
> +
> +	return SYS_NOFAIL("%s%d -q -c 1 -W 1 %s >/dev/null 2>&1",
> +			  ping_cmd_prefix, ipv, addr);
> +}
> +
> +static int setup_test_env(void)
> +{
> +	SYS(err, "ip link add dev %s1 type veth peer name %s2", TEST_IF_PREFIX,
> +	    TEST_IF_PREFIX);

I would like to take this chance to simplify the setup.

Does it need a veth pair? The %s2 interface is not used.

Can it be done in lo alone?

Also, all this setup (and test) has to be done in a new netns. Anything blocking 
the kfunc in patch 2 using the current task netns instead of the init_net?


> +	SYS(err, "ip link set %s1 up", TEST_IF_PREFIX);
> +	SYS(err, "ip link set %s2 up", TEST_IF_PREFIX);
> +	SYS(err, "ip -4 addr add %s/8 dev %s1", TEST_IPV4, TEST_IF_PREFIX);
> +	SYS(err, "ip -6 addr add %s/128 dev %s1", TEST_IPV6, TEST_IF_PREFIX);

Add nodad to the "ip -6 addr add...". just in case it may add unnecessary delay.

> +
> +	int i;
> +
> +	for (i = 0; i < 5; i++) {
> +		if (!ping_once(4, TEST_IPV4) && !ping_once(6, TEST_IPV6))

This interface/address ping should not be needed. Other tests under prog_tests/ 
don't need this interface/address ping also.

> +			return 0;
> +	}
> +
> +	ASSERT_FAIL("Timed out waiting for test IP to become available.");
> +err:
> +	return -1;
> +}
> +
> +static void cleanup_test_env(void)
> +{
> +	SYS_NOFAIL("ip link del %s1 2>/dev/null", TEST_IF_PREFIX);
> +	SYS_NOFAIL("ip link del %s2 2>/dev/null", TEST_IF_PREFIX);

Using lo will make this easier. Regardless, the link should go away with the netns.

> +}
> +
>   void test_sock_addr(void)
>   {
>   	int cgroup_fd = -1;
>   	void *skel;
>   
> +	if (!ASSERT_OK(setup_test_env(), "setup_test_env"))

This will probably have to be called after the test__join_cgroup() if it also 
creates a new netns.

pw-bot: cr

> +		goto cleanup;
> +
>   	cgroup_fd = test__join_cgroup("/sock_addr");
>   	if (!ASSERT_GE(cgroup_fd, 0, "join_cgroup"))
>   		goto cleanup;
> @@ -609,4 +755,5 @@ void test_sock_addr(void)
>   cleanup:
>   	if (cgroup_fd >= 0)
>   		close(cgroup_fd);
> +	cleanup_test_env();
>   }

Re: [PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Martin KaFai Lau]

On 4/12/24 9:52 AM, Jordan Rife wrote:
> This patch lays the groundwork for testing IPv4 and IPv6 sockaddr hooks
> and their interaction with both socket syscalls and kernel functions
> (e.g. kernel_connect, kernel_bind, etc.) and moves the test cases from
> the old-style bpf/test_sock_addr.c self test into the sock_addr
> prog_test.

Can the test_sock_addr.{c,sh} be retired after this patch?

If that is the case, please create another patch to remove them in the next respin.

Re: [PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Jordan Rife]

> I would like to take this chance to simplify the setup.
>
> Does it need a veth pair? The %s2 interface is not used.
>
> Can it be done in lo alone?

I will look into this for the next spin of the patch series.

> Also, all this setup (and test) has to be done in a new netns. Anything blocking
> the kfunc in patch 2 using the current task netns instead of the init_net?

This should be doable.

> Add nodad to the "ip -6 addr add...". just in case it may add unnecessary delay.
> This interface/address ping should not be needed. Other tests under prog_tests/
> don't need this interface/address ping also.

Ack.

> Can the test_sock_addr.{c,sh} be retired after this patch?

I know it's not used in the BPF CI tests, but is it still used in any
other contexts?

- Jordan

Re: [PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Martin KaFai Lau]

On 4/17/24 10:08 AM, Jordan Rife wrote:
>> Can the test_sock_addr.{c,sh} be retired after this patch?
> I know it's not used in the BPF CI tests, but is it still used in any
> other contexts?

If anyone depends on the test_sock_addr binary, it will have to start using 
"./test_progs -t sock_addr".

The test_sock_addr.{c,sh} can be retired as long as all its tests are migrated 
to sock_addr.c

Re: [PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Jordan Rife]

> The test_sock_addr.{c,sh} can be retired as long as all its tests are migrated
> to sock_addr.c

test_sock_addr.c has a few more test dimensions than
prog_tests/sock_addr.c currently does, so it covers a few more
scenarios.

struct sock_addr_test {
    const char *descr;
    /* BPF prog properties */
    load_fn loadfn;
    enum bpf_attach_type expected_attach_type;
    enum bpf_attach_type attach_type;
    /* Socket properties */
    int domain;
    int type;
    /* IP:port pairs for BPF prog to override */
    const char *requested_ip;
    unsigned short requested_port;
    const char *expected_ip;
    unsigned short expected_port;
    const char *expected_src_ip;
    /* Expected test result */
    enum {
        LOAD_REJECT,
        ATTACH_REJECT,
        ATTACH_OKAY,
        SYSCALL_EPERM,
        SYSCALL_ENOTSUPP,
        SUCCESS,
    } expected_result;
};

We focus on the "happy path" scenarios currently in
prog_tests/sock_addr.c while test_sock_addr.c has test cases that
cover a range of scenarios where loading or attaching a BPF program
should fail. There are also a few asm tests that use program loader
functions like sendmsg4_rw_asm_prog_load which specifies a series of
BPF instructions directly rather than loading one of the skeletons.
Adding in these test dimensions and migrating the test cases is a
slightly bigger lift for this patch series. Do we want to try to
migrate all of these to prog_tests/sock_addr.c in order to fully
retire it?

-Jordan

Re: [PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Martin KaFai Lau]

On 4/18/24 9:37 AM, Jordan Rife wrote:
>> The test_sock_addr.{c,sh} can be retired as long as all its tests are migrated
>> to sock_addr.c
> 
> test_sock_addr.c has a few more test dimensions than
> prog_tests/sock_addr.c currently does, so it covers a few more
> scenarios.
> 
> struct sock_addr_test {
>      const char *descr;
>      /* BPF prog properties */
>      load_fn loadfn;
>      enum bpf_attach_type expected_attach_type;
>      enum bpf_attach_type attach_type;
>      /* Socket properties */
>      int domain;
>      int type;
>      /* IP:port pairs for BPF prog to override */
>      const char *requested_ip;
>      unsigned short requested_port;
>      const char *expected_ip;
>      unsigned short expected_port;
>      const char *expected_src_ip;
>      /* Expected test result */
>      enum {
>          LOAD_REJECT,
>          ATTACH_REJECT,
>          ATTACH_OKAY,
>          SYSCALL_EPERM,
>          SYSCALL_ENOTSUPP,
>          SUCCESS,
>      } expected_result;
> };
> 
> We focus on the "happy path" scenarios currently in
> prog_tests/sock_addr.c while test_sock_addr.c has test cases that
> cover a range of scenarios where loading or attaching a BPF program
> should fail. There are also a few asm tests that use program loader
> functions like sendmsg4_rw_asm_prog_load which specifies a series of
> BPF instructions directly rather than loading one of the skeletons.
> Adding in these test dimensions and migrating the test cases is a
> slightly bigger lift for this patch series. Do we want to try to
> migrate all of these to prog_tests/sock_addr.c in order to fully
> retire it?

I don't want to keep this set hostage too much until everything is migrated from 
test_sock_addr.c. As long as for the tests you find useful in test_sock_addr.c 
in this patch set and moved them to prog_tests/sock_addr.c, it is heading in the 
right direction. For the moved test, please remove them from test_sock_addr.c so 
that it is clear what else needs to be done.

[ Side note for future migration attempt, at least for the LOAD_REJECT one, it 
probably makes sense to write it like progs/verifier_*.c ]

Re: [PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Jordan Rife]

> Also, all this setup (and test) has to be done in a new netns. Anything blocking
> the kfunc in patch 2 using the current task netns instead of the init_net?
> Add nodad to the "ip -6 addr add...". just in case it may add unnecessary delay.
> This interface/address ping should not be needed. Other tests under prog_tests/
> don't need this interface/address ping also.

I was able to make these changes.

> Does it need a veth pair? The %s2 interface is not used.
>
> Can it be done in lo alone?

I think it may be better to keep it as-is for now with the veth pair.
It turns out that these BPF programs (progs/bind6_prog.c,
progs/bind4_prog.c, and progs/connect4_prog.c) expect the veth pair
setup with these names (test_sock_addr1, test_sock_addr2). We may be
able to update the logic in these BPF programs to allow us to just use
lo, but I'm not sure if we'd be losing out on important test coverage.
Additionally, since we aren't fully retiring test_sock_addr.c yet we'd
also need to change test_sock_addr.sh if we changed
progs/bind6_prog.c, progs/bind4_prog.c, and progs/connect4_prog.c. If
there are no objections to leaving things as-is here, I will send out
v3 with the rest of the changes listed above.

-Jordan

Re: [PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Martin KaFai Lau]

On 4/28/24 10:47 AM, Jordan Rife wrote:
>> Also, all this setup (and test) has to be done in a new netns. Anything blocking
>> the kfunc in patch 2 using the current task netns instead of the init_net?
>> Add nodad to the "ip -6 addr add...". just in case it may add unnecessary delay.
>> This interface/address ping should not be needed. Other tests under prog_tests/
>> don't need this interface/address ping also.
> 
> I was able to make these changes.
> 
>> Does it need a veth pair? The %s2 interface is not used.
>>
>> Can it be done in lo alone?
> 
> I think it may be better to keep it as-is for now with the veth pair.
> It turns out that these BPF programs (progs/bind6_prog.c,
> progs/bind4_prog.c, and progs/connect4_prog.c) expect the veth pair
> setup with these names (test_sock_addr1, test_sock_addr2). We may be
> able to update the logic in these BPF programs to allow us to just use
> lo, but I'm not sure if we'd be losing out on important test coverage.
> Additionally, since we aren't fully retiring test_sock_addr.c yet we'd
> also need to change test_sock_addr.sh if we changed
> progs/bind6_prog.c, progs/bind4_prog.c, and progs/connect4_prog.c. If
> there are no objections to leaving things as-is here, I will send out
> v3 with the rest of the changes listed above.

Yep, the veth cleanup could be done when the test_sock_addr.c is fully retired. 
Thanks for checking.

For the tests that moved to sock_addr.c, please also remove them from 
test_sock_addr.c.

Re: [PATCH v2 bpf-next 4/6] selftests/bpf: Add IPv4 and IPv6 sockaddr test cases

[Jordan Rife]

> For the tests that moved to sock_addr.c, please also remove them from
> test_sock_addr.c.

Done in v3 (https://lore.kernel.org/bpf/20240429214529.2644801-1-jrife@google.com/T/#m560606260cda41652a7f305a0acff7fc8975d10a).

-Jordan

[PATCH v2 bpf-next 5/6] selftests/bpf: Make sock configurable for each test case

[Jordan Rife]

In order to reuse the same test code for both socket system calls (e.g.
connect(), bind(), etc.) and kernel socket functions (e.g.
kernel_connect(), kernel_bind(), etc.), this patch introduces the "ops"
field to sock_addr_test. This field allows each test cases to configure
the set of functions used in the test case to create, manipulate, and
tear down a socket.

Signed-off-by: Jordan Rife <jrife@google.com>
---
 .../selftests/bpf/prog_tests/sock_addr.c      | 137 ++++++++++++------
 1 file changed, 95 insertions(+), 42 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/sock_addr.c b/tools/testing/selftests/bpf/prog_tests/sock_addr.c
index 92879b971a098..78bcc147f09c4 100644
--- a/tools/testing/selftests/bpf/prog_tests/sock_addr.c
+++ b/tools/testing/selftests/bpf/prog_tests/sock_addr.c
@@ -53,12 +53,63 @@ enum sock_addr_test_type {
 typedef void *(*load_fn)(int cgroup_fd);
 typedef void (*destroy_fn)(void *skel);
 
+struct sock_ops {
+	int (*connect_to_addr)(const struct sockaddr_storage *addr,
+			       socklen_t addrlen, int type);
+	int (*start_server)(int family, int type, const char *addr_str,
+			    __u16 port, int timeout_ms);
+	int (*socket)(int famil, int type, int protocol);
+	int (*bind)(int fd, struct sockaddr *addr, socklen_t addrlen);
+	int (*getsockname)(int fd, struct sockaddr *addr, socklen_t *addrlen);
+	int (*getpeername)(int fd, struct sockaddr *addr, socklen_t *addrlen);
+	int (*sendmsg)(int fd, struct sockaddr *addr, socklen_t addrlen,
+		       char *msg, int msglen);
+	int (*close)(int fd);
+};
+
+static int user_sendmsg(int fd, struct sockaddr *addr, socklen_t addrlen,
+			char *msg, int msglen)
+{
+	struct msghdr hdr;
+	struct iovec iov;
+
+	memset(&iov, 0, sizeof(iov));
+	iov.iov_base = msg;
+	iov.iov_len = msglen;
+
+	memset(&hdr, 0, sizeof(hdr));
+	hdr.msg_name = (void *)addr;
+	hdr.msg_namelen = addrlen;
+	hdr.msg_iov = &iov;
+	hdr.msg_iovlen = 1;
+
+	return sendmsg(fd, &hdr, 0);
+}
+
+static int user_bind(int fd, struct sockaddr *addr, socklen_t addrlen)
+{
+	return bind(fd, (const struct sockaddr *)addr, addrlen);
+}
+
+struct sock_ops user_ops = {
+	.connect_to_addr = connect_to_addr,
+	.start_server = start_server,
+	.socket = socket,
+	.bind = user_bind,
+	.getsockname = getsockname,
+	.getpeername = getpeername,
+	.sendmsg = user_sendmsg,
+	.close = close,
+};
+
 struct sock_addr_test {
 	enum sock_addr_test_type type;
 	const char *name;
 	/* BPF prog properties */
 	load_fn loadfn;
 	destroy_fn destroyfn;
+	/* Socket operations */
+	struct sock_ops *ops;
 	/* Socket properties */
 	int socket_family;
 	int socket_type;
@@ -112,6 +163,7 @@ static struct sock_addr_test tests[] = {
 		"bind4: bind (stream)",
 		bind4_prog_load,
 		bind4_prog_destroy,
+		&user_ops,
 		AF_INET,
 		SOCK_STREAM,
 		SERV4_IP,
@@ -124,6 +176,7 @@ static struct sock_addr_test tests[] = {
 		"bind4: bind (dgram)",
 		bind4_prog_load,
 		bind4_prog_destroy,
+		&user_ops,
 		AF_INET,
 		SOCK_DGRAM,
 		SERV4_IP,
@@ -136,6 +189,7 @@ static struct sock_addr_test tests[] = {
 		"bind6: bind (stream)",
 		bind6_prog_load,
 		bind6_prog_destroy,
+		&user_ops,
 		AF_INET6,
 		SOCK_STREAM,
 		SERV6_IP,
@@ -148,6 +202,7 @@ static struct sock_addr_test tests[] = {
 		"bind6: bind (dgram)",
 		bind6_prog_load,
 		bind6_prog_destroy,
+		&user_ops,
 		AF_INET6,
 		SOCK_DGRAM,
 		SERV6_IP,
@@ -162,6 +217,7 @@ static struct sock_addr_test tests[] = {
 		"connect4: connect (stream)",
 		connect4_prog_load,
 		connect4_prog_destroy,
+		&user_ops,
 		AF_INET,
 		SOCK_STREAM,
 		SERV4_IP,
@@ -175,6 +231,7 @@ static struct sock_addr_test tests[] = {
 		"connect4: connect (dgram)",
 		connect4_prog_load,
 		connect4_prog_destroy,
+		&user_ops,
 		AF_INET,
 		SOCK_DGRAM,
 		SERV4_IP,
@@ -188,6 +245,7 @@ static struct sock_addr_test tests[] = {
 		"connect6: connect (stream)",
 		connect6_prog_load,
 		connect6_prog_destroy,
+		&user_ops,
 		AF_INET6,
 		SOCK_STREAM,
 		SERV6_IP,
@@ -201,6 +259,7 @@ static struct sock_addr_test tests[] = {
 		"connect6: connect (dgram)",
 		connect6_prog_load,
 		connect6_prog_destroy,
+		&user_ops,
 		AF_INET6,
 		SOCK_DGRAM,
 		SERV6_IP,
@@ -214,6 +273,7 @@ static struct sock_addr_test tests[] = {
 		"connect_unix: connect (stream)",
 		connect_unix_prog_load,
 		connect_unix_prog_destroy,
+		&user_ops,
 		AF_UNIX,
 		SOCK_STREAM,
 		SERVUN_ADDRESS,
@@ -229,6 +289,7 @@ static struct sock_addr_test tests[] = {
 		"sendmsg4: sendmsg (dgram)",
 		sendmsg4_prog_load,
 		sendmsg4_prog_destroy,
+		&user_ops,
 		AF_INET,
 		SOCK_DGRAM,
 		SERV4_IP,
@@ -242,6 +303,7 @@ static struct sock_addr_test tests[] = {
 		"sendmsg6: sendmsg (dgram)",
 		sendmsg6_prog_load,
 		sendmsg6_prog_destroy,
+		&user_ops,
 		AF_INET6,
 		SOCK_DGRAM,
 		SERV6_IP,
@@ -255,6 +317,7 @@ static struct sock_addr_test tests[] = {
 		"sendmsg_unix: sendmsg (dgram)",
 		sendmsg_unix_prog_load,
 		sendmsg_unix_prog_destroy,
+		&user_ops,
 		AF_UNIX,
 		SOCK_DGRAM,
 		SERVUN_ADDRESS,
@@ -270,6 +333,7 @@ static struct sock_addr_test tests[] = {
 		"recvmsg4: recvfrom (dgram)",
 		recvmsg4_prog_load,
 		recvmsg4_prog_destroy,
+		&user_ops,
 		AF_INET,
 		SOCK_DGRAM,
 		SERV4_REWRITE_IP,
@@ -283,6 +347,7 @@ static struct sock_addr_test tests[] = {
 		"recvmsg6: recvfrom (dgram)",
 		recvmsg6_prog_load,
 		recvmsg6_prog_destroy,
+		&user_ops,
 		AF_INET6,
 		SOCK_DGRAM,
 		SERV6_REWRITE_IP,
@@ -296,6 +361,7 @@ static struct sock_addr_test tests[] = {
 		"recvmsg_unix: recvfrom (dgram)",
 		recvmsg_unix_prog_load,
 		recvmsg_unix_prog_destroy,
+		&user_ops,
 		AF_UNIX,
 		SOCK_DGRAM,
 		SERVUN_REWRITE_ADDRESS,
@@ -309,6 +375,7 @@ static struct sock_addr_test tests[] = {
 		"recvmsg_unix: recvfrom (stream)",
 		recvmsg_unix_prog_load,
 		recvmsg_unix_prog_destroy,
+		&user_ops,
 		AF_UNIX,
 		SOCK_STREAM,
 		SERVUN_REWRITE_ADDRESS,
@@ -324,6 +391,7 @@ static struct sock_addr_test tests[] = {
 		"getsockname_unix",
 		getsockname_unix_prog_load,
 		getsockname_unix_prog_destroy,
+		&user_ops,
 		AF_UNIX,
 		SOCK_STREAM,
 		SERVUN_ADDRESS,
@@ -339,6 +407,7 @@ static struct sock_addr_test tests[] = {
 		"getpeername_unix",
 		getpeername_unix_prog_load,
 		getpeername_unix_prog_destroy,
+		&user_ops,
 		AF_UNIX,
 		SOCK_STREAM,
 		SERVUN_ADDRESS,
@@ -399,26 +468,15 @@ static int cmp_sock_addr(info_fn fn, int sock1,
 	return cmp_addr(&addr1, len1, addr2, addr2_len, cmp_port);
 }
 
-static int cmp_local_addr(int sock1, const struct sockaddr_storage *addr2,
-			  socklen_t addr2_len, bool cmp_port)
-{
-	return cmp_sock_addr(getsockname, sock1, addr2, addr2_len, cmp_port);
-}
-
-static int cmp_peer_addr(int sock1, const struct sockaddr_storage *addr2,
-			 socklen_t addr2_len, bool cmp_port)
-{
-	return cmp_sock_addr(getpeername, sock1, addr2, addr2_len, cmp_port);
-}
-
 static void test_bind(struct sock_addr_test *test)
 {
 	struct sockaddr_storage expected_addr;
 	socklen_t expected_addr_len = sizeof(struct sockaddr_storage);
 	int serv = -1, client = -1, err;
 
-	serv = start_server(test->socket_family, test->socket_type,
-			    test->requested_addr, test->requested_port, 0);
+	serv = test->ops->start_server(test->socket_family, test->socket_type,
+				       test->requested_addr,
+				       test->requested_port, 0);
 	if (!ASSERT_GE(serv, 0, "start_server"))
 		goto cleanup;
 
@@ -428,7 +486,8 @@ static void test_bind(struct sock_addr_test *test)
 	if (!ASSERT_EQ(err, 0, "make_sockaddr"))
 		goto cleanup;
 
-	err = cmp_local_addr(serv, &expected_addr, expected_addr_len, true);
+	err = cmp_sock_addr(test->ops->getsockname, serv, &expected_addr,
+			    expected_addr_len, true);
 	if (!ASSERT_EQ(err, 0, "cmp_local_addr"))
 		goto cleanup;
 
@@ -441,7 +500,7 @@ static void test_bind(struct sock_addr_test *test)
 	if (client != -1)
 		close(client);
 	if (serv != -1)
-		close(serv);
+		test->ops->close(serv);
 }
 
 static void test_connect(struct sock_addr_test *test)
@@ -462,7 +521,7 @@ static void test_connect(struct sock_addr_test *test)
 	if (!ASSERT_EQ(err, 0, "make_sockaddr"))
 		goto cleanup;
 
-	client = connect_to_addr(&addr, addr_len, test->socket_type);
+	client = test->ops->connect_to_addr(&addr, addr_len, test->socket_type);
 	if (!ASSERT_GE(client, 0, "connect_to_addr"))
 		goto cleanup;
 
@@ -478,18 +537,21 @@ static void test_connect(struct sock_addr_test *test)
 			goto cleanup;
 	}
 
-	err = cmp_peer_addr(client, &expected_addr, expected_addr_len, true);
+	err = cmp_sock_addr(test->ops->getpeername, client, &expected_addr,
+			    expected_addr_len, true);
 	if (!ASSERT_EQ(err, 0, "cmp_peer_addr"))
 		goto cleanup;
 
 	if (test->expected_src_addr) {
-		err = cmp_local_addr(client, &expected_src_addr, expected_src_addr_len, false);
+		err = cmp_sock_addr(test->ops->getsockname, client,
+				    &expected_src_addr, expected_src_addr_len,
+				    false);
 		if (!ASSERT_EQ(err, 0, "cmp_local_addr"))
 			goto cleanup;
 	}
 cleanup:
 	if (client != -1)
-		close(client);
+		test->ops->close(client);
 	if (serv != -1)
 		close(serv);
 }
@@ -499,8 +561,6 @@ static void test_xmsg(struct sock_addr_test *test)
 	struct sockaddr_storage addr, src_addr;
 	socklen_t addr_len = sizeof(struct sockaddr_storage),
 		  src_addr_len = sizeof(struct sockaddr_storage);
-	struct msghdr hdr;
-	struct iovec iov;
 	char data = 'a';
 	int serv = -1, client = -1, err;
 
@@ -513,7 +573,7 @@ static void test_xmsg(struct sock_addr_test *test)
 	if (!ASSERT_GE(serv, 0, "start_server"))
 		goto cleanup;
 
-	client = socket(test->socket_family, test->socket_type, 0);
+	client = test->ops->socket(test->socket_family, test->socket_type, 0);
 	if (!ASSERT_GE(client, 0, "socket"))
 		goto cleanup;
 
@@ -523,7 +583,8 @@ static void test_xmsg(struct sock_addr_test *test)
 		if (!ASSERT_EQ(err, 0, "make_sockaddr"))
 			goto cleanup;
 
-		err = bind(client, (const struct sockaddr *) &src_addr, src_addr_len);
+		err = test->ops->bind(client, (struct sockaddr *) &src_addr,
+				      src_addr_len);
 		if (!ASSERT_OK(err, "bind"))
 			goto cleanup;
 	}
@@ -534,17 +595,8 @@ static void test_xmsg(struct sock_addr_test *test)
 		goto cleanup;
 
 	if (test->socket_type == SOCK_DGRAM) {
-		memset(&iov, 0, sizeof(iov));
-		iov.iov_base = &data;
-		iov.iov_len = sizeof(data);
-
-		memset(&hdr, 0, sizeof(hdr));
-		hdr.msg_name = (void *)&addr;
-		hdr.msg_namelen = addr_len;
-		hdr.msg_iov = &iov;
-		hdr.msg_iovlen = 1;
-
-		err = sendmsg(client, &hdr, 0);
+		err = test->ops->sendmsg(client, (struct sockaddr *)&addr,
+					 addr_len, &data, sizeof(data));
 		if (!ASSERT_EQ(err, sizeof(data), "sendmsg"))
 			goto cleanup;
 	} else {
@@ -595,7 +647,7 @@ static void test_xmsg(struct sock_addr_test *test)
 
 cleanup:
 	if (client != -1)
-		close(client);
+		test->ops->close(client);
 	if (serv != -1)
 		close(serv);
 }
@@ -606,7 +658,7 @@ static void test_getsockname(struct sock_addr_test *test)
 	socklen_t expected_addr_len = sizeof(struct sockaddr_storage);
 	int serv = -1, err;
 
-	serv = start_server(test->socket_family, test->socket_type,
+	serv = test->ops->start_server(test->socket_family, test->socket_type,
 			    test->requested_addr, test->requested_port, 0);
 	if (!ASSERT_GE(serv, 0, "start_server"))
 		goto cleanup;
@@ -617,13 +669,13 @@ static void test_getsockname(struct sock_addr_test *test)
 	if (!ASSERT_EQ(err, 0, "make_sockaddr"))
 		goto cleanup;
 
-	err = cmp_local_addr(serv, &expected_addr, expected_addr_len, true);
+	err = cmp_sock_addr(test->ops->getsockname, serv, &expected_addr, expected_addr_len, true);
 	if (!ASSERT_EQ(err, 0, "cmp_local_addr"))
 		goto cleanup;
 
 cleanup:
 	if (serv != -1)
-		close(serv);
+		test->ops->close(serv);
 }
 
 static void test_getpeername(struct sock_addr_test *test)
@@ -643,7 +695,7 @@ static void test_getpeername(struct sock_addr_test *test)
 	if (!ASSERT_EQ(err, 0, "make_sockaddr"))
 		goto cleanup;
 
-	client = connect_to_addr(&addr, addr_len, test->socket_type);
+	client = test->ops->connect_to_addr(&addr, addr_len, test->socket_type);
 	if (!ASSERT_GE(client, 0, "connect_to_addr"))
 		goto cleanup;
 
@@ -652,13 +704,14 @@ static void test_getpeername(struct sock_addr_test *test)
 	if (!ASSERT_EQ(err, 0, "make_sockaddr"))
 		goto cleanup;
 
-	err = cmp_peer_addr(client, &expected_addr, expected_addr_len, true);
+	err = cmp_sock_addr(test->ops->getpeername, client, &expected_addr,
+			    expected_addr_len, true);
 	if (!ASSERT_EQ(err, 0, "cmp_peer_addr"))
 		goto cleanup;
 
 cleanup:
 	if (client != -1)
-		close(client);
+		test->ops->close(client);
 	if (serv != -1)
 		close(serv);
 }
-- 
2.44.0.478.gd926399ef9-goog

[PATCH v2 bpf-next 6/6] selftests/bpf: Add kernel socket operation tests

[Jordan Rife]

This patch creates two sets of sock_ops that call out to the SYSCALL
hooks in the sock_addr_kern BPF program and uses them to construct
test cases for the range of supported operations (kernel_connect(),
kernel_bind(), kernel_sendms(), sock_sendmsg(), kernel_getsockname(),
kenel_getpeername()). This ensures that these interact with BPF sockaddr
hooks as intended.

Beyond this it also ensures that these operations do not modify their
address parameter, providing regression coverage for the issues
addressed by this set of patches:

- commit 0bdf399342c5("net: Avoid address overwrite in kernel_connect")
- commit 86a7e0b69bd5("net: prevent rewrite of msg_name in sock_sendmsg()")
- commit c889a99a21bf("net: prevent address rewrite in kernel_bind()")
- commit 01b2885d9415("net: Save and restore msg_namelen in sock_sendmsg")

Signed-off-by: Jordan Rife <jrife@google.com>
---
 .../selftests/bpf/prog_tests/sock_addr.c      | 474 ++++++++++++++++++
 1 file changed, 474 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/sock_addr.c b/tools/testing/selftests/bpf/prog_tests/sock_addr.c
index 78bcc147f09c4..621df9b593ec8 100644
--- a/tools/testing/selftests/bpf/prog_tests/sock_addr.c
+++ b/tools/testing/selftests/bpf/prog_tests/sock_addr.c
@@ -3,6 +3,7 @@
 
 #include "test_progs.h"
 
+#include "sock_addr_kern.skel.h"
 #include "bind4_prog.skel.h"
 #include "bind6_prog.skel.h"
 #include "connect_unix_prog.skel.h"
@@ -53,6 +54,218 @@ enum sock_addr_test_type {
 typedef void *(*load_fn)(int cgroup_fd);
 typedef void (*destroy_fn)(void *skel);
 
+static int cmp_addr(const struct sockaddr_storage *addr1, socklen_t addr1_len,
+		    const struct sockaddr_storage *addr2, socklen_t addr2_len,
+		    bool cmp_port);
+
+struct init_sock_args {
+	int af;
+	int type;
+};
+
+struct addr_args {
+	char addr[sizeof(struct sockaddr_storage)];
+	int addrlen;
+};
+
+struct sendmsg_args {
+	struct addr_args addr;
+	char msg[10];
+	int msglen;
+};
+
+static struct sock_addr_kern *skel;
+
+static int run_bpf_prog(const char *prog_name, void *ctx, int ctx_size)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	struct bpf_program *prog;
+	int prog_fd, err;
+
+	topts.ctx_in = ctx;
+	topts.ctx_size_in = ctx_size;
+
+	prog = bpf_object__find_program_by_name(skel->obj, prog_name);
+	if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name"))
+		goto err;
+
+	prog_fd = bpf_program__fd(prog);
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	if (!ASSERT_OK(err, prog_name))
+		goto err;
+
+	err = topts.retval;
+	goto out;
+err:
+	err = -1;
+out:
+	return err;
+}
+
+static int kernel_init_sock(int af, int type, int protocol)
+{
+	struct init_sock_args args = {
+		.af = af,
+		.type = type,
+	};
+
+	return run_bpf_prog("init_sock", &args, sizeof(args));
+}
+
+static int kernel_close_sock(int fd)
+{
+	return run_bpf_prog("close_sock", NULL, 0);
+}
+
+static int sock_addr_op(const char *name, struct sockaddr *addr,
+			socklen_t *addrlen, bool expect_change)
+{
+	struct addr_args args;
+	int err;
+
+	if (addrlen)
+		args.addrlen = *addrlen;
+
+	if (addr)
+		memcpy(&args.addr, addr, *addrlen);
+
+	err = run_bpf_prog(name, &args, sizeof(args));
+
+	if (!expect_change && addr)
+		if (!ASSERT_EQ(cmp_addr((struct sockaddr_storage *)addr,
+					*addrlen,
+					(struct sockaddr_storage *)&args.addr,
+					args.addrlen, 1),
+			       0, "address_param_modified"))
+			return -1;
+
+	if (addrlen)
+		*addrlen = args.addrlen;
+
+	if (addr)
+		memcpy(addr, &args.addr, *addrlen);
+
+	return err;
+}
+
+static int send_msg_op(const char *name, struct sockaddr *addr,
+		       socklen_t addrlen, const char *msg, int msglen,
+		       bool expect_change)
+{
+	struct sendmsg_args args;
+	int err;
+
+	memset(&args, 0, sizeof(args));
+	memcpy(&args.addr.addr, addr, addrlen);
+	args.addr.addrlen = addrlen;
+	memcpy(args.msg, msg, msglen);
+	args.msglen = msglen;
+
+	err = run_bpf_prog(name, &args, sizeof(args));
+
+	if (!expect_change && addr)
+		if (!ASSERT_EQ(cmp_addr((struct sockaddr_storage *)addr,
+					addrlen,
+					(struct sockaddr_storage *)&args.addr.addr,
+					args.addr.addrlen, 1),
+			       0, "address_param_modified"))
+			return -1;
+
+	return err;
+}
+
+static int kernel_connect(struct sockaddr *addr, socklen_t addrlen)
+{
+	return sock_addr_op("kernel_connect", addr, &addrlen, false);
+}
+
+static int kernel_bind(int fd, struct sockaddr *addr, socklen_t addrlen)
+{
+	return sock_addr_op("kernel_bind", addr, &addrlen, false);
+}
+
+static int kernel_listen(void)
+{
+	return sock_addr_op("kernel_listen", NULL, NULL, false);
+}
+
+static int kernel_sendmsg(int fd, struct sockaddr *addr, socklen_t addrlen,
+			  char *msg, int msglen)
+{
+	return send_msg_op("kernel_sendmsg", addr, addrlen, msg, msglen, false);
+}
+
+static int sock_sendmsg(int fd, struct sockaddr *addr, socklen_t addrlen,
+			char *msg, int msglen)
+{
+	return send_msg_op("sock_sendmsg", addr, addrlen, msg, msglen, false);
+}
+
+static int kernel_getsockname(int fd, struct sockaddr *addr, socklen_t *addrlen)
+{
+	return sock_addr_op("kernel_getsockname", addr, addrlen, true);
+}
+
+static int kernel_getpeername(int fd, struct sockaddr *addr, socklen_t *addrlen)
+{
+	return sock_addr_op("kernel_getpeername", addr, addrlen, true);
+}
+
+int kernel_connect_to_addr(const struct sockaddr_storage *addr,
+			   socklen_t addrlen, int type)
+{
+	int err;
+
+	if (!ASSERT_OK(kernel_init_sock(addr->ss_family, type, 0),
+		       "kernel_init_sock"))
+		goto err;
+
+	if (!ASSERT_OK(kernel_connect((struct sockaddr *)addr, addrlen),
+		       "kernel_connect"))
+		goto err;
+
+	/* Test code expects a "file descriptor" on success. */
+	err = 1;
+	goto out;
+err:
+	err = -1;
+	ASSERT_OK(kernel_close_sock(0), "kernel_close_sock");
+out:
+	return err;
+}
+
+int kernel_start_server(int family, int type, const char *addr_str, __u16 port,
+			int timeout_ms)
+{
+	struct sockaddr_storage addr;
+	socklen_t addrlen;
+	int err;
+
+	if (!ASSERT_OK(kernel_init_sock(family, type, 0), "kernel_init_sock"))
+		goto err;
+
+	if (make_sockaddr(family, addr_str, port, &addr, &addrlen))
+		goto err;
+
+	if (!ASSERT_OK(kernel_bind(0, (struct sockaddr *)&addr, addrlen),
+		       "kernel_bind"))
+		goto err;
+
+	if (type == SOCK_STREAM) {
+		if (!ASSERT_OK(kernel_listen(), "kernel_listen"))
+			goto err;
+	}
+
+	/* Test code expects a "file descriptor" on success. */
+	err = 1;
+	goto out;
+err:
+	err = -1;
+	ASSERT_OK(kernel_close_sock(0), "kernel_close_sock");
+out:
+	return err;
+}
+
 struct sock_ops {
 	int (*connect_to_addr)(const struct sockaddr_storage *addr,
 			       socklen_t addrlen, int type);
@@ -102,6 +315,28 @@ struct sock_ops user_ops = {
 	.close = close,
 };
 
+struct sock_ops kern_ops_sock_sendmsg = {
+	.connect_to_addr = kernel_connect_to_addr,
+	.start_server = kernel_start_server,
+	.socket = kernel_init_sock,
+	.bind = kernel_bind,
+	.getsockname = kernel_getsockname,
+	.getpeername = kernel_getpeername,
+	.sendmsg = sock_sendmsg,
+	.close = kernel_close_sock,
+};
+
+struct sock_ops kern_ops_kernel_sendmsg = {
+	.connect_to_addr = kernel_connect_to_addr,
+	.start_server = kernel_start_server,
+	.socket = kernel_init_sock,
+	.bind = kernel_bind,
+	.getsockname = kernel_getsockname,
+	.getpeername = kernel_getpeername,
+	.sendmsg = kernel_sendmsg,
+	.close = kernel_close_sock,
+};
+
 struct sock_addr_test {
 	enum sock_addr_test_type type;
 	const char *name;
@@ -211,6 +446,60 @@ static struct sock_addr_test tests[] = {
 		SERV6_REWRITE_PORT,
 	},
 
+	/* bind - kernel calls */
+	{
+		SOCK_ADDR_TEST_BIND,
+		"bind4: kernel_bind (stream)",
+		bind4_prog_load,
+		bind4_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_INET,
+		SOCK_STREAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+	},
+	{
+		SOCK_ADDR_TEST_BIND,
+		"bind4: kernel_bind (dgram)",
+		bind4_prog_load,
+		bind4_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_INET,
+		SOCK_DGRAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+	},
+	{
+		SOCK_ADDR_TEST_BIND,
+		"bind6: kernel_bind (stream)",
+		bind6_prog_load,
+		bind6_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_INET6,
+		SOCK_STREAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+	},
+	{
+		SOCK_ADDR_TEST_BIND,
+		"bind6: kernel_bind (dgram)",
+		bind6_prog_load,
+		bind6_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_INET6,
+		SOCK_DGRAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+	},
+
 	/* connect - system calls */
 	{
 		SOCK_ADDR_TEST_CONNECT,
@@ -283,6 +572,78 @@ static struct sock_addr_test tests[] = {
 		NULL,
 	},
 
+	/* connect - kernel calls */
+	{
+		SOCK_ADDR_TEST_CONNECT,
+		"connect4: kernel_connect (stream)",
+		connect4_prog_load,
+		connect4_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_INET,
+		SOCK_STREAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+		SRC4_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_CONNECT,
+		"connect4: kernel_connect (dgram)",
+		connect4_prog_load,
+		connect4_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_INET,
+		SOCK_DGRAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+		SRC4_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_CONNECT,
+		"connect6: kernel_connect (stream)",
+		connect6_prog_load,
+		connect6_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_INET6,
+		SOCK_STREAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+		SRC6_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_CONNECT,
+		"connect6: kernel_connect (dgram)",
+		connect6_prog_load,
+		connect6_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_INET6,
+		SOCK_DGRAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+		SRC6_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_CONNECT,
+		"connect_unix: kernel_connect (dgram)",
+		connect_unix_prog_load,
+		connect_unix_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_UNIX,
+		SOCK_STREAM,
+		SERVUN_ADDRESS,
+		0,
+		SERVUN_REWRITE_ADDRESS,
+		0,
+		NULL,
+	},
+
 	/* sendmsg - system calls */
 	{
 		SOCK_ADDR_TEST_SENDMSG,
@@ -327,6 +688,94 @@ static struct sock_addr_test tests[] = {
 		NULL,
 	},
 
+	/* sendmsg - kernel calls (sock_sendmsg) */
+	{
+		SOCK_ADDR_TEST_SENDMSG,
+		"sendmsg4: sock_sendmsg (dgram)",
+		sendmsg4_prog_load,
+		sendmsg4_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_INET,
+		SOCK_DGRAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+		SRC4_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_SENDMSG,
+		"sendmsg6: sock_sendmsg (dgram)",
+		sendmsg6_prog_load,
+		sendmsg6_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_INET6,
+		SOCK_DGRAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+		SRC6_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_SENDMSG,
+		"sendmsg_unix: sock_sendmsg (dgram)",
+		sendmsg_unix_prog_load,
+		sendmsg_unix_prog_destroy,
+		&kern_ops_sock_sendmsg,
+		AF_UNIX,
+		SOCK_DGRAM,
+		SERVUN_ADDRESS,
+		0,
+		SERVUN_REWRITE_ADDRESS,
+		0,
+		NULL,
+	},
+
+	/* sendmsg - kernel calls (kernel_sendmsg) */
+	{
+		SOCK_ADDR_TEST_SENDMSG,
+		"sendmsg4: kernel_sendmsg (dgram)",
+		sendmsg4_prog_load,
+		sendmsg4_prog_destroy,
+		&kern_ops_kernel_sendmsg,
+		AF_INET,
+		SOCK_DGRAM,
+		SERV4_IP,
+		SERV4_PORT,
+		SERV4_REWRITE_IP,
+		SERV4_REWRITE_PORT,
+		SRC4_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_SENDMSG,
+		"sendmsg6: kernel_sendmsg (dgram)",
+		sendmsg6_prog_load,
+		sendmsg6_prog_destroy,
+		&kern_ops_kernel_sendmsg,
+		AF_INET6,
+		SOCK_DGRAM,
+		SERV6_IP,
+		SERV6_PORT,
+		SERV6_REWRITE_IP,
+		SERV6_REWRITE_PORT,
+		SRC6_REWRITE_IP,
+	},
+	{
+		SOCK_ADDR_TEST_SENDMSG,
+		"sendmsg_unix: sock_sendmsg (dgram)",
+		sendmsg_unix_prog_load,
+		sendmsg_unix_prog_destroy,
+		&kern_ops_kernel_sendmsg,
+		AF_UNIX,
+		SOCK_DGRAM,
+		SERVUN_ADDRESS,
+		0,
+		SERVUN_REWRITE_ADDRESS,
+		0,
+		NULL,
+	},
+
 	/* recvmsg - system calls */
 	{
 		SOCK_ADDR_TEST_RECVMSG,
@@ -468,6 +917,27 @@ static int cmp_sock_addr(info_fn fn, int sock1,
 	return cmp_addr(&addr1, len1, addr2, addr2_len, cmp_port);
 }
 
+static int load_sock_addr_kern(void)
+{
+	int err;
+
+	skel = sock_addr_kern__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel"))
+		goto err;
+
+	err = 0;
+	goto out;
+err:
+	err = -1;
+out:
+	return err;
+}
+
+static void unload_sock_addr_kern(void)
+{
+	sock_addr_kern__destroy(skel);
+}
+
 static void test_bind(struct sock_addr_test *test)
 {
 	struct sockaddr_storage expected_addr;
@@ -766,6 +1236,9 @@ void test_sock_addr(void)
 	if (!ASSERT_GE(cgroup_fd, 0, "join_cgroup"))
 		goto cleanup;
 
+	if (!ASSERT_OK(load_sock_addr_kern(), "load_sock_addr_kern"))
+		goto cleanup;
+
 	for (size_t i = 0; i < ARRAY_SIZE(tests); ++i) {
 		struct sock_addr_test *test = &tests[i];
 
@@ -806,6 +1279,7 @@ void test_sock_addr(void)
 	}
 
 cleanup:
+	unload_sock_addr_kern();
 	if (cgroup_fd >= 0)
 		close(cgroup_fd);
 	cleanup_test_env();
-- 
2.44.0.683.g7961c838ac-goog