[PATCHv2,bpf-next,13/15] selftests/bpf: Test syscall command parsing

Commit Message

Joe Stringer March 2, 2021, 5:19 p.m. UTC

Add building of the bpf(2) syscall commands documentation as part of the
docs building step in the build. This allows us to pick up on potential
parse errors from the docs generator script as part of selftests.

The generated manual pages here are not intended for distribution, they
are just a fragment that can be integrated into the other static text of
bpf(2) to form the full manual page.

Acked-by: Toke Høiland-Jørgensen <[e-mail]>
Reviewed-by: Quentin Monnet <[e-mail]>
Signed-off-by: Joe Stringer <[e-mail]>
---
 tools/testing/selftests/bpf/.gitignore    |  1 +
 tools/testing/selftests/bpf/Makefile.docs | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

Comments

Yonghong Song March 3, 2021, 10:23 p.m. UTC | #1

On 3/2/21 9:19 AM, Joe Stringer wrote:
> Add building of the bpf(2) syscall commands documentation as part of the
> docs building step in the build. This allows us to pick up on potential
> parse errors from the docs generator script as part of selftests.
> 
> The generated manual pages here are not intended for distribution, they
> are just a fragment that can be integrated into the other static text of
> bpf(2) to form the full manual page.

I tried and the generated bpf(2) man page looks like:

BPF(2) 
 
                          BPF(2)

NAME
        bpf - Perform a command on an extended BPF object

COMMANDS
        BPF_MAP_CREATE

               Description
                      Create  a map and return a file descriptor that 
refers to the map. The close-on-exec file descriptor flag (see fcntl(2)) 
is automatically enabled for the
                      new file descriptor.

                      Applying close(2) to the file descriptor returned 
by BPF_MAP_CREATE will delete the map (but see NOTES).

               Return A new file descriptor (a nonnegative integer), or 
-1 if an error occurred (in which case, errno is set appropriately).

        BPF_MAP_LOOKUP_ELEM
...
        BPF_PROG_BIND_MAP

               Description
                      Bind a map to the lifetime of an eBPF program.

                      The map identified by map_fd is bound to the 
program identified by prog_fd and only released when prog_fd is 
released. This may be used  in  cases  where
                      metadata  should  be  associated  with  a  program 
  which otherwise does not contain any references to the map (for 
example, embedded in the eBPF program
                      instructions).

               Return Returns zero on success. On error, -1 is returned 
and errno is set appropriately.


Yes, this needs to be integrated into the real man page. But this is 
already great so people can see latest bpf latest features without
going to the source code. Thanks!

> 
> Acked-by: Toke Høiland-Jørgensen <[e-mail]>
> Reviewed-by: Quentin Monnet <[e-mail]>
> Signed-off-by: Joe Stringer <[e-mail]>

Acked-by: Yonghong Song <[e-mail]>

Joe Stringer March 3, 2021, 11:50 p.m. UTC | #2

On Wed, Mar 3, 2021 at 2:23 PM Yonghong Song <[e-mail]> wrote:
>
>
>
> On 3/2/21 9:19 AM, Joe Stringer wrote:
> > Add building of the bpf(2) syscall commands documentation as part of the
> > docs building step in the build. This allows us to pick up on potential
> > parse errors from the docs generator script as part of selftests.
> >
> > The generated manual pages here are not intended for distribution, they
> > are just a fragment that can be integrated into the other static text of
> > bpf(2) to form the full manual page.
>
> I tried and the generated bpf(2) man page looks like:
>
> BPF(2)
>
>                           BPF(2)
>
> NAME
>         bpf - Perform a command on an extended BPF object
>
> COMMANDS
>         BPF_MAP_CREATE
>
>                Description
>                       Create  a map and return a file descriptor that
> refers to the map. The close-on-exec file descriptor flag (see fcntl(2))
> is automatically enabled for the
>                       new file descriptor.
>
>                       Applying close(2) to the file descriptor returned
> by BPF_MAP_CREATE will delete the map (but see NOTES).
>
>                Return A new file descriptor (a nonnegative integer), or
> -1 if an error occurred (in which case, errno is set appropriately).
>
>         BPF_MAP_LOOKUP_ELEM
> ...
>         BPF_PROG_BIND_MAP
>
>                Description
>                       Bind a map to the lifetime of an eBPF program.
>
>                       The map identified by map_fd is bound to the
> program identified by prog_fd and only released when prog_fd is
> released. This may be used  in  cases  where
>                       metadata  should  be  associated  with  a  program
>   which otherwise does not contain any references to the map (for
> example, embedded in the eBPF program
>                       instructions).
>
>                Return Returns zero on success. On error, -1 is returned
> and errno is set appropriately.
>
>
> Yes, this needs to be integrated into the real man page. But this is
> already great so people can see latest bpf latest features without
> going to the source code. Thanks!

:party-parrot:

Yeah I think the step around integrating into the real man page is an
area that folks may have opinions on and I'm CC'ing Michael here in
particular, but ultimately that manual is currently managed in the
upstream manpages tree so nothing actionable from this series,
absolute simplest is for someone (is that me?) to just package this
hunk up and submit it to the upstream tree every once in a while.
Slightly more elaborate would involve integrating the static text with
this generated text somewhere (either kernel tree or manpages tree)
and have someone run a cron job to generate & submit the changes.
Could be another option, these are the obvious ones that come to mind.

> >
> > Acked-by: Toke Høiland-Jørgensen <[e-mail]>
> > Reviewed-by: Quentin Monnet <[e-mail]>
> > Signed-off-by: Joe Stringer <[e-mail]>
>
> Acked-by: Yonghong Song <[e-mail]>

Patch

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index a0d5ec3cfc24..4866f6a21901 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -1,5 +1,6 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 bpf-helpers*
+bpf-syscall*
 test_verifier
 test_maps
 test_lru_map
diff --git a/tools/testing/selftests/bpf/Makefile.docs b/tools/testing/selftests/bpf/Makefile.docs
index f39ad19317c8..ccf260021e83 100644
--- a/tools/testing/selftests/bpf/Makefile.docs
+++ b/tools/testing/selftests/bpf/Makefile.docs
@@ -15,18 +15,27 @@  endif
 
 prefix ?= /usr/local
 mandir ?= $(prefix)/man
+man2dir = $(mandir)/man2
 man7dir = $(mandir)/man7
 
+SYSCALL_RST = bpf-syscall.rst
+MAN2_RST = $(SYSCALL_RST)
+
 HELPERS_RST = bpf-helpers.rst
 MAN7_RST = $(HELPERS_RST)
 
+_DOC_MAN2 = $(patsubst %.rst,%.2,$(MAN2_RST))
+DOC_MAN2 = $(addprefix $(OUTPUT),$(_DOC_MAN2))
+
 _DOC_MAN7 = $(patsubst %.rst,%.7,$(MAN7_RST))
 DOC_MAN7 = $(addprefix $(OUTPUT),$(_DOC_MAN7))
 
-DOCTARGETS := helpers
+DOCTARGETS := helpers syscall
 
 docs: $(DOCTARGETS)
+syscall: man2
 helpers: man7
+man2: $(DOC_MAN2)
 man7: $(DOC_MAN7)
 
 RST2MAN_DEP := $(shell command -v rst2man 2>/dev/null)
@@ -64,9 +73,10 @@  endef
 
 # Create the make targets to generate manual pages by name and section
 $(eval $(call DOCS_RULES,helpers,7))
+$(eval $(call DOCS_RULES,syscall,2))
 
 docs-clean: $(foreach doctarget,$(DOCTARGETS), docs-clean-$(doctarget))
 docs-install: $(foreach doctarget,$(DOCTARGETS), docs-install-$(doctarget))
 docs-uninstall: $(foreach doctarget,$(DOCTARGETS), docs-uninstall-$(doctarget))
 
-.PHONY: docs docs-clean docs-install docs-uninstall man7
+.PHONY: docs docs-clean docs-install docs-uninstall man2 man7