[PATCH bpf 0/6] nfp fix for calls, bpftool completions and doc fixes

STALE3134d

8 messages, 2 authors, 2018-02-08 · open the first message on its own page

[PATCH bpf 0/6] nfp fix for calls, bpftool completions and doc fixes

From: Jakub Kicinski <hidden>
Date: 2018-02-08 04:27:36

Hi!

First patch in this series fixes applying the relocation to immediate
load instructions in the NFP JIT.

The remaining patches come from Quentin.  Small addition to libbpf
makes sure it recognizes all standard section names.  Makefile in
bpftool/Documentation is improved to explicitly check for rst2man
being installed on the system, otherwise we risk installing empty
files.  Man page for bpftool-map is corrected to include program
as a potential value for map of programs.

Last two patches are slightly longer, those update bash completions to
include this release cycle's additions from Roman.  Maybe the use of
Fixes tags is slightly frivolous there, but having bash completions
which don't cover all commands and options could be disruptive to work
flow for users.

Jakub Kicinski (1):
  nfp: bpf: fix immed relocation for larger offsets

Quentin Monnet (5):
  libbpf: complete list of strings for guessing program type
  tools: bpftool: exit doc Makefile early if rst2man is not available
  tools: bpftool: make syntax for program map update explicit in man
    page
  tools: bpftool: add bash completion for `bpftool prog load`
  tools: bpftool: add bash completion for cgroup commands

 drivers/net/ethernet/netronome/nfp/nfp_asm.c       |  2 +-
 tools/bpf/bpftool/Documentation/Makefile           |  5 ++
 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst |  4 +-
 tools/bpf/bpftool/Documentation/bpftool-map.rst    |  3 +-
 tools/bpf/bpftool/bash-completion/bpftool          | 72 ++++++++++++++++++++--
 tools/lib/bpf/libbpf.c                             |  5 ++
 6 files changed, 81 insertions(+), 10 deletions(-)

-- 
2.15.1

[PATCH bpf 1/6] nfp: bpf: fix immed relocation for larger offsets

From: Jakub Kicinski <hidden>
Date: 2018-02-08 04:27:37

Immed relocation is missing a shift which means for larger
offsets the lower and higher part of the address would be
ORed together.

Fixes: ce4ebfd859c3 ("nfp: bpf: add helpers for updating immediate instructions")
Signed-off-by: Jakub Kicinski <redacted>
Reviewed-by: Jiong Wang <redacted>
---
 drivers/net/ethernet/netronome/nfp/nfp_asm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.c b/drivers/net/ethernet/netronome/nfp/nfp_asm.c
index 3f6952b66a49..1e597600c693 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.c

@@ -107,7 +107,7 @@ u16 immed_get_value(u64 instr)
 	if (!unreg_is_imm(reg))
 		reg = FIELD_GET(OP_IMMED_B_SRC, instr);
 
-	return (reg & 0xff) | FIELD_GET(OP_IMMED_IMM, instr);
+	return (reg & 0xff) | FIELD_GET(OP_IMMED_IMM, instr) << 8;
 }
 
 void immed_set_value(u64 *instr, u16 immed)

-- 
2.15.1

[PATCH bpf 2/6] libbpf: complete list of strings for guessing program type

From: Jakub Kicinski <hidden>
Date: 2018-02-08 04:27:38

From: Quentin Monnet <redacted>

It seems that the type guessing feature for libbpf, based on the name of
the ELF section the program is located in, was inspired from
samples/bpf/prog_load.c, which was not used by any sample for loading
programs of certain types such as TC actions and classifiers, or
LWT-related types. As a consequence, libbpf is not able to guess the
type of such programs and to load them automatically if type is not
provided to the `bpf_load_prog()` function.

Add ELF section names associated to those eBPF program types so that
they can be loaded with e.g. bpftool as well.

Signed-off-by: Quentin Monnet <redacted>
Reviewed-by: Jakub Kicinski <redacted>
---
 tools/lib/bpf/libbpf.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 71ddc481f349..c64840365433 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c

@@ -1816,12 +1816,17 @@ static const struct {
 	BPF_PROG_SEC("socket",		BPF_PROG_TYPE_SOCKET_FILTER),
 	BPF_PROG_SEC("kprobe/",		BPF_PROG_TYPE_KPROBE),
 	BPF_PROG_SEC("kretprobe/",	BPF_PROG_TYPE_KPROBE),
+	BPF_PROG_SEC("classifier",	BPF_PROG_TYPE_SCHED_CLS),
+	BPF_PROG_SEC("action",		BPF_PROG_TYPE_SCHED_ACT),
 	BPF_PROG_SEC("tracepoint/",	BPF_PROG_TYPE_TRACEPOINT),
 	BPF_PROG_SEC("xdp",		BPF_PROG_TYPE_XDP),
 	BPF_PROG_SEC("perf_event",	BPF_PROG_TYPE_PERF_EVENT),
 	BPF_PROG_SEC("cgroup/skb",	BPF_PROG_TYPE_CGROUP_SKB),
 	BPF_PROG_SEC("cgroup/sock",	BPF_PROG_TYPE_CGROUP_SOCK),
 	BPF_PROG_SEC("cgroup/dev",	BPF_PROG_TYPE_CGROUP_DEVICE),
+	BPF_PROG_SEC("lwt_in",		BPF_PROG_TYPE_LWT_IN),
+	BPF_PROG_SEC("lwt_out",		BPF_PROG_TYPE_LWT_OUT),
+	BPF_PROG_SEC("lwt_xmit",	BPF_PROG_TYPE_LWT_XMIT),
 	BPF_PROG_SEC("sockops",		BPF_PROG_TYPE_SOCK_OPS),
 	BPF_PROG_SEC("sk_skb",		BPF_PROG_TYPE_SK_SKB),
 };

-- 
2.15.1

[PATCH bpf 3/6] tools: bpftool: exit doc Makefile early if rst2man is not available

From: Jakub Kicinski <hidden>
Date: 2018-02-08 04:27:39

From: Quentin Monnet <redacted>

If rst2man is not available on the system, running `make doc` from the
bpftool directory fails with an error message. However, it creates empty
manual pages (.8 files in this case). A subsequent call to `make
doc-install` would then succeed and install those empty man pages on the
system.

To prevent this, raise a Makefile error and exit immediately if rst2man
is not available before generating the pages from the rst documentation.

Fixes: ff69c21a85a4 ("tools: bpftool: add documentation")
Reported-by: Jason van Aaardt <redacted>
Signed-off-by: Quentin Monnet <redacted>
Reviewed-by: Jakub Kicinski <redacted>
---
 tools/bpf/bpftool/Documentation/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/bpf/bpftool/Documentation/Makefile b/tools/bpf/bpftool/Documentation/Makefile
index c462a928e03d..a9d47c1558bb 100644
--- a/tools/bpf/bpftool/Documentation/Makefile
+++ b/tools/bpf/bpftool/Documentation/Makefile

@@ -23,7 +23,12 @@ DOC_MAN8 = $(addprefix $(OUTPUT),$(_DOC_MAN8))
 man: man8
 man8: $(DOC_MAN8)
 
+RST2MAN_DEP := $(shell command -v rst2man 2>/dev/null)
+
 $(OUTPUT)%.8: %.rst
+ifndef RST2MAN_DEP
+	$(error "rst2man not found, but required to generate man pages")
+endif
 	$(QUIET_GEN)rst2man $< > $@
 
 clean:

-- 
2.15.1

[PATCH bpf 4/6] tools: bpftool: make syntax for program map update explicit in man page

From: Jakub Kicinski <hidden>
Date: 2018-02-08 04:27:40

From: Quentin Monnet <redacted>

Specify in the documentation that when using bpftool to update a map of
type BPF_MAP_TYPE_PROG_ARRAY, the syntax for the program used as a value
should use the "id|tag|pinned" keywords convention, as used with
"bpftool prog" commands.

Fixes: ff69c21a85a4 ("tools: bpftool: add documentation")
Signed-off-by: Quentin Monnet <redacted>
Reviewed-by: Jakub Kicinski <redacted>
---
 tools/bpf/bpftool/Documentation/bpftool-map.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 0ab32b312aec..457e868bd32f 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst

@@ -31,7 +31,8 @@ MAP COMMANDS
 |	**bpftool** **map help**
 |
 |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
-|	*VALUE* := { *BYTES* | *MAP* | *PROGRAM* }
+|	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
+|	*VALUE* := { *BYTES* | *MAP* | *PROG* }
 |	*UPDATE_FLAGS* := { **any** | **exist** | **noexist** }
 
 DESCRIPTION

-- 
2.15.1

[PATCH bpf 5/6] tools: bpftool: add bash completion for `bpftool prog load`

From: Jakub Kicinski <hidden>
Date: 2018-02-08 04:27:41

From: Quentin Monnet <redacted>

Add bash completion for bpftool command `prog load`. Completion for this
command is easy, as it only takes existing file paths as arguments.

Fixes: 49a086c201a9 ("bpftool: implement prog load command")
Signed-off-by: Quentin Monnet <redacted>
Reviewed-by: Jakub Kicinski <redacted>
---
 tools/bpf/bpftool/bash-completion/bpftool | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 0137866bb8f6..17d246418348 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool

@@ -230,10 +230,14 @@ _bpftool()
                     fi
                     return 0
                     ;;
+                load)
+                    _filedir
+                    return 0
+                    ;;
                 *)
                     [[ $prev == $object ]] && \
-                        COMPREPLY=( $( compgen -W 'dump help pin show list' -- \
-                            "$cur" ) )
+                        COMPREPLY=( $( compgen -W 'dump help pin load \
+                            show list' -- "$cur" ) )
                     ;;
             esac
             ;;

-- 
2.15.1

[PATCH bpf 6/6] tools: bpftool: add bash completion for cgroup commands

From: Jakub Kicinski <hidden>
Date: 2018-02-08 04:27:42

From: Quentin Monnet <redacted>

Add bash completion for "bpftool cgroup" command family. While at it,
also fix the formatting of some keywords in the man page for cgroups.

Fixes: 5ccda64d38cc ("bpftool: implement cgroup bpf operations")
Signed-off-by: Quentin Monnet <redacted>
Reviewed-by: Jakub Kicinski <redacted>
---
 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst |  4 +-
 tools/bpf/bpftool/bash-completion/bpftool          | 64 ++++++++++++++++++++--
 2 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
index 2fe2a1bdbe3e..0e4e923235b6 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst

@@ -26,8 +26,8 @@ MAP COMMANDS
 |	**bpftool** **cgroup help**
 |
 |	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
-|	*ATTACH_TYPE* := { *ingress* | *egress* | *sock_create* | *sock_ops* | *device* }
-|	*ATTACH_FLAGS* := { *multi* | *override* }
+|	*ATTACH_TYPE* := { **ingress** | **egress** | **sock_create** | **sock_ops** | **device** }
+|	*ATTACH_FLAGS* := { **multi** | **override** }
 
 DESCRIPTION
 ===========

diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 17d246418348..08719c54a614 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool

@@ -52,16 +52,24 @@ _bpftool_once_attr()
     done
 }
 
-# Takes a list of words in argument; adds them all to COMPREPLY if none of them
-# is already present on the command line. Returns no value.
-_bpftool_one_of_list()
+# Takes a list of words as argument; if any of those words is present on the
+# command line, return 0. Otherwise, return 1.
+_bpftool_search_list()
 {
     local w idx
     for w in $*; do
         for (( idx=3; idx < ${#words[@]}-1; idx++ )); do
-            [[ $w == ${words[idx]} ]] && return 1
+            [[ $w == ${words[idx]} ]] && return 0
         done
     done
+    return 1
+}
+
+# Takes a list of words in argument; adds them all to COMPREPLY if none of them
+# is already present on the command line. Returns no value.
+_bpftool_one_of_list()
+{
+    _bpftool_search_list $* && return 1
     COMPREPLY+=( $( compgen -W "$*" -- "$cur" ) )
 }

@@ -351,6 +359,54 @@ _bpftool()
                     ;;
             esac
             ;;
+        cgroup)
+            case $command in
+                show|list)
+                    _filedir
+                    return 0
+                    ;;
+                attach|detach)
+                    local ATTACH_TYPES='ingress egress sock_create sock_ops \
+                        device'
+                    local ATTACH_FLAGS='multi override'
+                    local PROG_TYPE='id pinned tag'
+                    case $prev in
+                        $command)
+                            _filedir
+                            return 0
+                            ;;
+                        ingress|egress|sock_create|sock_ops|device)
+                            COMPREPLY=( $( compgen -W "$PROG_TYPE" -- \
+                                "$cur" ) )
+                            return 0
+                            ;;
+                        id)
+                            _bpftool_get_prog_ids
+                            return 0
+                            ;;
+                        *)
+                            if ! _bpftool_search_list "$ATTACH_TYPES"; then
+                                COMPREPLY=( $( compgen -W "$ATTACH_TYPES" -- \
+                                    "$cur" ) )
+                            elif [[ "$command" == "attach" ]]; then
+                                # We have an attach type on the command line,
+                                # but it is not the previous word, or
+                                # "id|pinned|tag" (we already checked for
+                                # that). This should only leave the case when
+                                # we need attach flags for "attach" commamnd.
+                                _bpftool_one_of_list "$ATTACH_FLAGS"
+                            fi
+                            return 0
+                            ;;
+                    esac
+                    ;;
+                *)
+                    [[ $prev == $object ]] && \
+                        COMPREPLY=( $( compgen -W 'help attach detach \
+                            show list' -- "$cur" ) )
+                    ;;
+            esac
+            ;;
     esac
 } &&
 complete -F _bpftool bpftool

-- 
2.15.1

Re: [PATCH bpf 0/6] nfp fix for calls, bpftool completions and doc fixes

From: Daniel Borkmann <daniel@iogearbox.net>
Date: 2018-02-08 11:08:10

On 02/08/2018 05:27 AM, Jakub Kicinski wrote:

Hi!

First patch in this series fixes applying the relocation to immediate
load instructions in the NFP JIT.

The remaining patches come from Quentin.  Small addition to libbpf
makes sure it recognizes all standard section names.  Makefile in
bpftool/Documentation is improved to explicitly check for rst2man
being installed on the system, otherwise we risk installing empty
files.  Man page for bpftool-map is corrected to include program
as a potential value for map of programs.

Last two patches are slightly longer, those update bash completions to
include this release cycle's additions from Roman.  Maybe the use of
Fixes tags is slightly frivolous there, but having bash completions
which don't cover all commands and options could be disruptive to work
flow for users.

Series applied to bpf, thanks Jakub and Quentin!

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help