Re: [PATCH v4 bpf-next 5/7] bpftool: add loadall command
On 11/09, Quentin Monnet wrote:
> 2018-11-08 16:22 UTC-0800 ~ Stanislav Fomichev
> > From: Stanislav Fomichev
> >
> > This patch adds new *loadall* command which slightly differs from the
> > existing *load*. *load* command loads all programs from the obj file,
> > but pins only the first programs. *loadall* pins all programs from the
> > obj file under specified directory.
> >
> > The intended usecase is flow_dissector, where we want to load a bunch
> > of progs, pin them all and after that construct a jump table.
> >
> > Signed-off-by: Stanislav Fomichev
> > ---
> > .../bpftool/Documentation/bpftool-prog.rst| 14 +++-
> > tools/bpf/bpftool/bash-completion/bpftool | 4 +-
> > tools/bpf/bpftool/common.c| 31
> > tools/bpf/bpftool/main.h | 1 +
> > tools/bpf/bpftool/prog.c | 74 ++-
> > 5 files changed, 82 insertions(+), 42 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > index ac4e904b10fb..d943d9b67a1d 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>
> > @@ -24,7 +25,7 @@ MAP COMMANDS
> > | **bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes**
> > | **visual**}]
> > | **bpftool** **prog dump jited** *PROG* [{**file** *FILE* |
> > **opcodes**}]
> > | **bpftool** **prog pin** *PROG* *FILE*
> > -| **bpftool** **prog load** *OBJ* *FILE* [**type** *TYPE*] [**map**
> > {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > +| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*]
> > [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > | **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
> > | **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
> > | **bpftool** **prog help**
> > @@ -79,8 +80,13 @@ DESCRIPTION
> > contain a dot character ('.'), which is reserved for future
> > extensions of *bpffs*.
> >
> > - **bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx**
> > *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > - Load bpf program from binary *OBJ* and pin as *FILE*.
> > + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*]
> > [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> > + Both **bpftool prog load** and **bpftool prog loadall** load
> > + all maps and programs from the *OBJ* and differ only in
> > + pinning. **load** pins only the first program from the *OBJ*
> > + as *FILE*. **loadall** pins all programs from the *OBJ*
> > + under *FILE* directory.
> > **type** is optional, if not specified program type will be
> > inferred from section names.
> > By default bpftool will create new maps as declared in the ELF
>
> Thanks a lot for all the changes! The series looks really good to me
> now. The last nit I might have is that we could maybe replace "FILE"
> with "PATH" (as it can now be a directory), in the doc an below. No need
> to respin just for this, though.
Agreed, makes sense, will do another respin to address Jakub's comments
anyway. Thanks for a review!
> > @@ -1035,7 +1067,8 @@ static int do_help(int argc, char **argv)
> > " %s %s dump xlated PROG [{ file FILE | opcodes | visual
> > }]\n"
> > " %s %s dump jited PROG [{ file FILE | opcodes }]\n"
> > " %s %s pin PROG FILE\n"
> > - " %s %s load OBJ FILE [type TYPE] [dev NAME] \\\n"
> > + " %s %s { load | loadall } OBJ FILE \\\n"
> > + " [type TYPE] [dev NAME] \\\n"
> > " [map { idx IDX | name NAME } MAP]\n"
> > " %s %s attach PROG ATTACH_TYPE MAP\n"
> > " %s %s detach PROG ATTACH_TYPE MAP\n"
Re: [PATCH v4 bpf-next 5/7] bpftool: add loadall command
2018-11-08 16:22 UTC-0800 ~ Stanislav Fomichev
> From: Stanislav Fomichev
>
> This patch adds new *loadall* command which slightly differs from the
> existing *load*. *load* command loads all programs from the obj file,
> but pins only the first programs. *loadall* pins all programs from the
> obj file under specified directory.
>
> The intended usecase is flow_dissector, where we want to load a bunch
> of progs, pin them all and after that construct a jump table.
>
> Signed-off-by: Stanislav Fomichev
> ---
> .../bpftool/Documentation/bpftool-prog.rst| 14 +++-
> tools/bpf/bpftool/bash-completion/bpftool | 4 +-
> tools/bpf/bpftool/common.c| 31
> tools/bpf/bpftool/main.h | 1 +
> tools/bpf/bpftool/prog.c | 74 ++-
> 5 files changed, 82 insertions(+), 42 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index ac4e904b10fb..d943d9b67a1d 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -24,7 +25,7 @@ MAP COMMANDS
> |**bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes**
> | **visual**}]
> |**bpftool** **prog dump jited** *PROG* [{**file** *FILE* |
> **opcodes**}]
> |**bpftool** **prog pin** *PROG* *FILE*
> -|**bpftool** **prog load** *OBJ* *FILE* [**type** *TYPE*] [**map**
> {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> +|**bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*]
> [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> | **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
> | **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
> |**bpftool** **prog help**
> @@ -79,8 +80,13 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx**
> *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> - Load bpf program from binary *OBJ* and pin as *FILE*.
> + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*]
> [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> + Both **bpftool prog load** and **bpftool prog loadall** load
> + all maps and programs from the *OBJ* and differ only in
> + pinning. **load** pins only the first program from the *OBJ*
> + as *FILE*. **loadall** pins all programs from the *OBJ*
> + under *FILE* directory.
> **type** is optional, if not specified program type will be
> inferred from section names.
> By default bpftool will create new maps as declared in the ELF
Thanks a lot for all the changes! The series looks really good to me
now. The last nit I might have is that we could maybe replace "FILE"
with "PATH" (as it can now be a directory), in the doc an below. No need
to respin just for this, though.
> @@ -1035,7 +1067,8 @@ static int do_help(int argc, char **argv)
> " %s %s dump xlated PROG [{ file FILE | opcodes | visual
> }]\n"
> " %s %s dump jited PROG [{ file FILE | opcodes }]\n"
> " %s %s pin PROG FILE\n"
> - " %s %s load OBJ FILE [type TYPE] [dev NAME] \\\n"
> + " %s %s { load | loadall } OBJ FILE \\\n"
> + " [type TYPE] [dev NAME] \\\n"
> " [map { idx IDX | name NAME } MAP]\n"
> " %s %s attach PROG ATTACH_TYPE MAP\n"
> " %s %s detach PROG ATTACH_TYPE MAP\n"
Re: [PATCH v4 bpf-next 5/7] bpftool: add loadall command
On Thu, 8 Nov 2018 16:22:11 -0800, Stanislav Fomichev wrote:
> @@ -79,8 +80,13 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx**
> *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> - Load bpf program from binary *OBJ* and pin as *FILE*.
> + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*]
> [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> + Both **bpftool prog load** and **bpftool prog loadall** load
> + all maps and programs from the *OBJ* and differ only in
> + pinning. **load** pins only the first program from the *OBJ*
> + as *FILE*. **loadall** pins all programs from the *OBJ*
> + under *FILE* directory.
> **type** is optional, if not specified program type will be
> inferred from section names.
> By default bpftool will create new maps as declared in the ELF
As I said the fact that we load all always is a libbpf limitation,
I wouldn't put it in documentation as it may change.
With that removed looks good to me:
Acked-by: Jakub Kicinski
[PATCH v4 bpf-next 5/7] bpftool: add loadall command
From: Stanislav Fomichev
This patch adds new *loadall* command which slightly differs from the
existing *load*. *load* command loads all programs from the obj file,
but pins only the first programs. *loadall* pins all programs from the
obj file under specified directory.
The intended usecase is flow_dissector, where we want to load a bunch
of progs, pin them all and after that construct a jump table.
Signed-off-by: Stanislav Fomichev
---
.../bpftool/Documentation/bpftool-prog.rst| 14 +++-
tools/bpf/bpftool/bash-completion/bpftool | 4 +-
tools/bpf/bpftool/common.c| 31
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 74 ++-
5 files changed, 82 insertions(+), 42 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index ac4e904b10fb..d943d9b67a1d 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -15,7 +15,8 @@ SYNOPSIS
*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | {
**-f** | **--bpffs** } }
*COMMANDS* :=
- { **show** | **list** | **dump xlated** | **dump jited** | **pin** |
**load** | **help** }
+ { **show** | **list** | **dump xlated** | **dump jited** | **pin** |
**load**
+ | **loadall** | **help** }
MAP COMMANDS
=
@@ -24,7 +25,7 @@ MAP COMMANDS
| **bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes**
| **visual**}]
| **bpftool** **prog dump jited** *PROG* [{**file** *FILE* |
**opcodes**}]
| **bpftool** **prog pin** *PROG* *FILE*
-| **bpftool** **prog load** *OBJ* *FILE* [**type** *TYPE*] [**map**
{**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*]
[**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
| **bpftool** **prog help**
@@ -79,8 +80,13 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx**
*IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
- Load bpf program from binary *OBJ* and pin as *FILE*.
+ **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*]
[**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ Load bpf program(s) from binary *OBJ* and pin as *FILE*.
+ Both **bpftool prog load** and **bpftool prog loadall** load
+ all maps and programs from the *OBJ* and differ only in
+ pinning. **load** pins only the first program from the *OBJ*
+ as *FILE*. **loadall** pins all programs from the *OBJ*
+ under *FILE* directory.
**type** is optional, if not specified program type will be
inferred from section names.
By default bpftool will create new maps as declared in the ELF
diff --git a/tools/bpf/bpftool/bash-completion/bpftool
b/tools/bpf/bpftool/bash-completion/bpftool
index 3f78e6404589..780ebafb756a 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -243,7 +243,7 @@ _bpftool()
# Completion depends on object and command in use
case $object in
prog)
-if [[ $command != "load" ]]; then
+if [[ $command != "load" && $command != "loadall" ]]; then
case $prev in
id)
_bpftool_get_prog_ids
@@ -309,7 +309,7 @@ _bpftool()
fi
return 0
;;
-load)
+load|loadall)
local obj
if [[ ${#words[@]} -lt 6 ]]; then
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 25af85304ebe..21ce556c15e1 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -169,34 +169,23 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type
exp_type)
return fd;
}
-int do_pin_fd(int fd, const char *name)
+int mount_bpffs_for_pin(const char *name)
{
char err_str[ERR_MAX_LEN];
char *file;
char *dir;
int err = 0;
- err = bpf_obj_pin(fd, name);
- if (!err)
- goto out;
-
file = malloc(strlen(name) + 1);
strcpy(file, name);
dir = dirname(file);
- if (errno != EPERM || is_bpffs(dir)) {
- p_err("can't pin the object (%s): %s", name, strerror(errno));
+ if (is_bpffs(dir))
+ /* nothing to do if already mounted */
