Re: [PATCH bpf-next 2/2] selftests/bpf: Attach to socketcall() in test_probe_user

[Jiri Olsa <olsajiri@gmail.com>]

On Sat, Jul 23, 2022 at 04:03:44AM +0200, Ilya Leoshkevich wrote:
> test_probe_user fails on architectures where libc uses
> socketcall(SYS_CONNECT) instead of connect(). Fix by attaching to
> socketcall as well.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  .../selftests/bpf/prog_tests/probe_user.c     | 35 +++++++++++++------
>  .../selftests/bpf/progs/test_probe_user.c     | 28 +++++++++++++--
>  2 files changed, 50 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/probe_user.c b/tools/testing/selftests/bpf/prog_tests/probe_user.c
> index abf890d066eb..76c8e06b0357 100644
> --- a/tools/testing/selftests/bpf/prog_tests/probe_user.c
> +++ b/tools/testing/selftests/bpf/prog_tests/probe_user.c
> @@ -4,25 +4,35 @@
>  /* TODO: corrupts other tests uses connect() */
>  void serial_test_probe_user(void)
>  {
> -	const char *prog_name = "handle_sys_connect";
> +	const char *prog_names[] = {
> +		"handle_sys_connect",
> +#if defined(__s390x__)
> +		"handle_sys_socketcall",
> +#endif
> +	};
> +	const size_t prog_count = ARRAY_SIZE(prog_names);
>  	const char *obj_file = "./test_probe_user.o";
>  	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts, );
>  	int err, results_map_fd, sock_fd, duration = 0;
>  	struct sockaddr curr, orig, tmp;
>  	struct sockaddr_in *in = (struct sockaddr_in *)&curr;
> -	struct bpf_link *kprobe_link = NULL;
> -	struct bpf_program *kprobe_prog;
> +	struct bpf_link *kprobe_links[ARRAY_SIZE(prog_names)] = {};
> +	struct bpf_program *kprobe_progs[ARRAY_SIZE(prog_names)];
>  	struct bpf_object *obj;
>  	static const int zero = 0;
> +	size_t i;
>  
>  	obj = bpf_object__open_file(obj_file, &opts);
>  	if (!ASSERT_OK_PTR(obj, "obj_open_file"))
>  		return;
>  
> -	kprobe_prog = bpf_object__find_program_by_name(obj, prog_name);
> -	if (CHECK(!kprobe_prog, "find_probe",
> -		  "prog '%s' not found\n", prog_name))
> -		goto cleanup;
> +	for (i = 0; i < prog_count; i++) {
> +		kprobe_progs[i] =
> +			bpf_object__find_program_by_name(obj, prog_names[i]);
> +		if (CHECK(!kprobe_progs[i], "find_probe",
> +			  "prog '%s' not found\n", prog_names[i]))
> +			goto cleanup;
> +	}
>  
>  	err = bpf_object__load(obj);
>  	if (CHECK(err, "obj_load", "err %d\n", err))
> @@ -33,9 +43,11 @@ void serial_test_probe_user(void)
>  		  "err %d\n", results_map_fd))
>  		goto cleanup;
>  
> -	kprobe_link = bpf_program__attach(kprobe_prog);
> -	if (!ASSERT_OK_PTR(kprobe_link, "attach_kprobe"))
> -		goto cleanup;
> +	for (i = 0; i < prog_count; i++) {
> +		kprobe_links[i] = bpf_program__attach(kprobe_progs[i]);
> +		if (!ASSERT_OK_PTR(kprobe_links[i], "attach_kprobe"))
> +			goto cleanup;
> +	}
>  
>  	memset(&curr, 0, sizeof(curr));
>  	in->sin_family = AF_INET;
> @@ -69,6 +81,7 @@ void serial_test_probe_user(void)
>  		  inet_ntoa(in->sin_addr), ntohs(in->sin_port)))
>  		goto cleanup;
>  cleanup:
> -	bpf_link__destroy(kprobe_link);
> +	for (i = 0; i < ARRAY_SIZE(prog_names); i++)

nit, you used prog_count in all places, could be also here

> +		bpf_link__destroy(kprobe_links[i]);
>  	bpf_object__close(obj);
>  }
> diff --git a/tools/testing/selftests/bpf/progs/test_probe_user.c b/tools/testing/selftests/bpf/progs/test_probe_user.c
> index 8e1495008e4d..78e50c37fa21 100644
> --- a/tools/testing/selftests/bpf/progs/test_probe_user.c
> +++ b/tools/testing/selftests/bpf/progs/test_probe_user.c
> @@ -5,10 +5,13 @@
>  #include <bpf/bpf_core_read.h>
>  #include "bpf_misc.h"
>  
> +#ifndef SYS_CONNECT
> +#define SYS_CONNECT 3
> +#endif
> +
>  static struct sockaddr_in old;
>  
> -SEC("ksyscall/connect")
> -int BPF_KSYSCALL(handle_sys_connect, int fd, struct sockaddr_in *uservaddr, int addrlen)
> +static int handle_sys_connect_common(struct sockaddr_in *uservaddr)
>  {
>  	struct sockaddr_in new;
>  
> @@ -19,4 +22,25 @@ int BPF_KSYSCALL(handle_sys_connect, int fd, struct sockaddr_in *uservaddr, int
>  	return 0;
>  }
>  
> +SEC("ksyscall/connect")
> +int BPF_KSYSCALL(handle_sys_connect, int fd, struct sockaddr_in *uservaddr,
> +		 int addrlen)
> +{
> +	return handle_sys_connect_common(uservaddr);
> +}
> +
> +SEC("ksyscall/socketcall")
> +int BPF_KSYSCALL(handle_sys_socketcall, int call, unsigned long *args)
> +{
> +	if (call == SYS_CONNECT) {
> +		struct sockaddr_in *uservaddr;
> +
> +		bpf_probe_read_user(&uservaddr, sizeof(uservaddr), &args[1]);
> +
> +		return handle_sys_connect_common(uservaddr);
> +	}
> +
> +	return 0;
> +}

should this function be under __s390x__ ifdef same as in the user side?

jirka