Re: [PATCH bpf-next 3/9] samples: bpf: refactor test_cgrp2_sock2 program with libbpf

On Tue, Nov 17, 2020 at 02:56:38PM +0000, Daniel T. Lee wrote:
[ ... ]

> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 01449d767122..7a643595ac6c 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -82,7 +82,7 @@ test_overhead-objs := bpf_load.o test_overhead_user.o
>  test_cgrp2_array_pin-objs := test_cgrp2_array_pin.o
>  test_cgrp2_attach-objs := test_cgrp2_attach.o
>  test_cgrp2_sock-objs := test_cgrp2_sock.o
> -test_cgrp2_sock2-objs := bpf_load.o test_cgrp2_sock2.o
> +test_cgrp2_sock2-objs := test_cgrp2_sock2.o
>  xdp1-objs := xdp1_user.o
>  # reuse xdp1 source intentionally
>  xdp2-objs := xdp1_user.o
> diff --git a/samples/bpf/test_cgrp2_sock2.c b/samples/bpf/test_cgrp2_sock2.c
> index a9277b118c33..518526c7ce16 100644
> --- a/samples/bpf/test_cgrp2_sock2.c
> +++ b/samples/bpf/test_cgrp2_sock2.c
> @@ -20,9 +20,9 @@
>  #include <net/if.h>
>  #include <linux/bpf.h>
>  #include <bpf/bpf.h>
> +#include <bpf/libbpf.h>
>  
>  #include "bpf_insn.h"
> -#include "bpf_load.h"
>  
>  static int usage(const char *argv0)
>  {
> @@ -32,37 +32,66 @@ static int usage(const char *argv0)
>  
>  int main(int argc, char **argv)
>  {
> -	int cg_fd, ret, filter_id = 0;
> +	int cg_fd, err, ret = EXIT_FAILURE, filter_id = 0, prog_cnt = 0;
> +	const char *link_pin_path = "/sys/fs/bpf/test_cgrp2_sock2";
> +	struct bpf_link *link = NULL;
> +	struct bpf_program *progs[2];
> +	struct bpf_program *prog;
> +	struct bpf_object *obj;
>  
>  	if (argc < 3)
>  		return usage(argv[0]);
>  
> +	if (argc > 3)
> +		filter_id = atoi(argv[3]);
> +
>  	cg_fd = open(argv[1], O_DIRECTORY | O_RDONLY);
>  	if (cg_fd < 0) {
>  		printf("Failed to open cgroup path: '%s'\n", strerror(errno));
> -		return EXIT_FAILURE;
> +		return ret;
>  	}
>  
> -	if (load_bpf_file(argv[2]))
> -		return EXIT_FAILURE;
> -
> -	printf("Output from kernel verifier:\n%s\n-------\n", bpf_log_buf);
> +	obj = bpf_object__open_file(argv[2], NULL);
> +	if (libbpf_get_error(obj)) {
> +		printf("ERROR: opening BPF object file failed\n");
> +		return ret;
> +	}
>  
> -	if (argc > 3)
> -		filter_id = atoi(argv[3]);
> +	bpf_object__for_each_program(prog, obj) {
> +		progs[prog_cnt] = prog;
> +		prog_cnt++;
> +	}
>  
>  	if (filter_id >= prog_cnt) {
>  		printf("Invalid program id; program not found in file\n");
> -		return EXIT_FAILURE;
> +		goto cleanup;
> +	}
> +
> +	/* load BPF program */
> +	if (bpf_object__load(obj)) {
> +		printf("ERROR: loading BPF object file failed\n");
> +		goto cleanup;
>  	}
>  
> -	ret = bpf_prog_attach(prog_fd[filter_id], cg_fd,
> -			      BPF_CGROUP_INET_SOCK_CREATE, 0);
> -	if (ret < 0) {
> -		printf("Failed to attach prog to cgroup: '%s'\n",
> -		       strerror(errno));
> -		return EXIT_FAILURE;
> +	link = bpf_program__attach_cgroup(progs[filter_id], cg_fd);
> +	if (libbpf_get_error(link)) {
> +		printf("ERROR: bpf_program__attach failed\n");
> +		link = NULL;
> +		goto cleanup;
>  	}
>  
> -	return EXIT_SUCCESS;
> +	err = bpf_link__pin(link, link_pin_path);
> +	if (err < 0) {
> +		printf("err : %d\n", err);
> +		goto cleanup;
> +	}
> +
> +	ret = EXIT_SUCCESS;
> +
> +cleanup:
> +	if (ret != EXIT_SUCCESS)
> +		bpf_link__destroy(link);
This looks wrong.  cleanup should be done regardless.

> +
> +	bpf_object__close(obj);
> +	return ret;
>  }