Re: [PATCH v2 net-next 3/4] bpftool: implement prog load command

[Quentin Monnet <quentin.monnet@netronome.com>]

2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> Add the prog load command to load a bpf program from a specified
> binary file and pin it to bpffs.
> 
> Usage description and examples are given in the corresponding man
> page.
> 
> Syntax:
> $ bpftool prog load SOURCE_FILE FILE
> 
> FILE is a non-existing file on bpffs.
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Cc: David Ahern <dsahern@gmail.com>
> ---
>  tools/bpf/bpftool/Documentation/bpftool-prog.rst | 10 +++-
>  tools/bpf/bpftool/Documentation/bpftool.rst      |  2 +-
>  tools/bpf/bpftool/common.c                       | 71 +++++++++++++-----------
>  tools/bpf/bpftool/main.h                         |  1 +
>  tools/bpf/bpftool/prog.c                         | 31 ++++++++++-
>  5 files changed, 81 insertions(+), 34 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index 36e8d1c3c40d..827b415f8ab6 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -15,7 +15,7 @@ SYNOPSIS
>  	*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
>  
>  	*COMMANDS* :=
> -	{ **show** | **dump xlated** | **dump jited** | **pin** | **help** }
> +	{ **show** | **dump xlated** | **dump jited** | **pin** | **load** | **help** }
>  
>  MAP COMMANDS
>  =============
> @@ -24,6 +24,7 @@ MAP COMMANDS
>  |	**bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes**}]
>  |	**bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | **opcodes**}]
>  |	**bpftool** **prog pin** *PROG* *FILE*
> +|	**bpftool** **prog load** *SRC* *FILE*
>  |	**bpftool** **prog help**
>  |
>  |	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
> @@ -57,6 +58,11 @@ DESCRIPTION
>  
>  		  Note: *FILE* must be located in *bpffs* mount.
>  
> +	**bpftool prog load** *SRC* *FILE*
> +		  Load bpf program from binary *SRC* and pin as *FILE*.
> +
> +		  Note: *FILE* must be located in *bpffs* mount.
> +
>  	**bpftool prog help**
>  		  Print short help message.
>  
> @@ -126,8 +132,10 @@ EXAMPLES
>  |
>  | **# mount -t bpf none /sys/fs/bpf/**
>  | **# bpftool prog pin id 10 /sys/fs/bpf/prog**
> +| **# bpftool prog load ./my_prog.o /sys/fs/bpf/prog2**
>  | **# ls -l /sys/fs/bpf/**
>  |   -rw------- 1 root root 0 Jul 22 01:43 prog
> +|   -rw------- 1 root root 0 Dec 07 17:23 prog2

Nit: would you mind using a date closer to the first one? Just from a
reader's point of view it might look strange to see the files apparently
created with two successive commands having such a difference between
their creation times.

>  
>  **# bpftool prog dum jited pinned /sys/fs/bpf/prog opcodes**
>  
> diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
> index 926c03d5a8da..f547a0c0aa34 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool.rst
> @@ -26,7 +26,7 @@ SYNOPSIS
>  	| **pin** | **help** }
>  
>  	*PROG-COMMANDS* := { **show** | **dump jited** | **dump xlated** | **pin**
> -	| **help** }
> +	| **load** | **help** }
>  
>  DESCRIPTION
>  ===========

[…]

> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index ad619b96c276..bac5d81e2ff0 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -45,6 +45,7 @@
>  #include <sys/stat.h>
>  
>  #include <bpf.h>
> +#include <libbpf.h>
>  
>  #include "main.h"
>  #include "disasm.h"
> @@ -635,6 +636,32 @@ static int do_pin(int argc, char **argv)
>  	return err;
>  }
>  
> +static int do_load(int argc, char **argv)
> +{
> +	struct bpf_object *obj;
> +	int prog_fd;
> +
> +	if (argc != 2) {
> +		usage();
> +		return -1;
> +	}

No need to return here, usage() exits from the program.

> +
> +	if (bpf_prog_load(argv[0], BPF_PROG_TYPE_UNSPEC, &obj, &prog_fd)) {
> +		p_err("failed to load program\n");
> +		return -1;
> +	}
> +
> +	if (do_pin_fd(prog_fd, argv[1])) {
> +		p_err("failed to pin program\n");
> +		return -1;
> +	}
> +
> +	if (json_output)
> +		jsonw_null(json_wtr);
> +
> +	return 0;
> +}
> +
>  static int do_help(int argc, char **argv)
>  {
>  	if (json_output) {
> @@ -647,13 +674,14 @@ static int do_help(int argc, char **argv)
>  		"       %s %s dump xlated PROG [{ file FILE | opcodes }]\n"
>  		"       %s %s dump jited  PROG [{ file FILE | opcodes }]\n"
>  		"       %s %s pin   PROG FILE\n"
> +		"       %s %s load  SRC  FILE\n"
>  		"       %s %s help\n"
>  		"\n"
>  		"       " HELP_SPEC_PROGRAM "\n"
>  		"       " HELP_SPEC_OPTIONS "\n"
>  		"",
>  		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
> -		bin_name, argv[-2], bin_name, argv[-2]);
> +		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
>  
>  	return 0;
>  }
> @@ -663,6 +691,7 @@ static const struct cmd cmds[] = {
>  	{ "help",	do_help },
>  	{ "dump",	do_dump },
>  	{ "pin",	do_pin },
> +	{ "load",	do_load },
>  	{ 0 }
>  };
>  
>