bpftool: Update doc (use susbtitutions) and test_bpftool_synctypes.py
authorQuentin Monnet <quentin@isovalent.com>
Mon, 15 Nov 2021 22:58:43 +0000 (22:58 +0000)
committerDaniel Borkmann <daniel@iogearbox.net>
Tue, 16 Nov 2021 12:56:22 +0000 (13:56 +0100)
test_bpftool_synctypes.py helps detecting inconsistencies in bpftool
between the different list of types and options scattered in the
sources, the documentation, and the bash completion. For options that
apply to all bpftool commands, the script had a hardcoded list of
values, and would use them to check whether the man pages are
up-to-date. When writing the script, it felt acceptable to have this
list in order to avoid to open and parse bpftool's main.h every time,
and because the list of global options in bpftool doesn't change so
often.

However, this is prone to omissions, and we recently added a new
-l|--legacy option which was described in common_options.rst, but not
listed in the options summary of each manual page. The script did not
complain, because it keeps comparing the hardcoded list to the (now)
outdated list in the header file.

To address the issue, this commit brings the following changes:

- Options that are common to all bpftool commands (--json, --pretty, and
  --debug) are moved to a dedicated file, and used in the definition of
  a RST substitution. This substitution is used in the sources of all
  the man pages.

- This list of common options is updated, with the addition of the new
  -l|--legacy option.

- The script test_bpftool_synctypes.py is updated to compare:
    - Options specific to a command, found in C files, for the
      interactive help messages, with the same specific options from the
      relevant man page for that command.
    - Common options, checked just once: the list in main.h is
      compared with the new list in substitutions.rst.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20211115225844.33943-3-quentin@isovalent.com
14 files changed:
tools/bpf/bpftool/Documentation/bpftool-btf.rst
tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
tools/bpf/bpftool/Documentation/bpftool-feature.rst
tools/bpf/bpftool/Documentation/bpftool-gen.rst
tools/bpf/bpftool/Documentation/bpftool-iter.rst
tools/bpf/bpftool/Documentation/bpftool-link.rst
tools/bpf/bpftool/Documentation/bpftool-map.rst
tools/bpf/bpftool/Documentation/bpftool-net.rst
tools/bpf/bpftool/Documentation/bpftool-perf.rst
tools/bpf/bpftool/Documentation/bpftool-prog.rst
tools/bpf/bpftool/Documentation/bpftool-struct_ops.rst
tools/bpf/bpftool/Documentation/bpftool.rst
tools/bpf/bpftool/Documentation/substitutions.rst [new file with mode: 0644]
tools/testing/selftests/bpf/test_bpftool_synctypes.py

index 2d2ceb7163f6883ee3d4335489d26ee8c2c39362..342716f74ec4844532cd11b8e228b745191f12d8 100644 (file)
@@ -9,13 +9,14 @@ tool for inspection of BTF data
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **btf** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | {**-d** | **--debug** } |
-       { **-B** | **--base-btf** } }
+       *OPTIONS* := { |COMMON_OPTIONS| | { **-B** | **--base-btf** } }
 
        *COMMANDS* := { **dump** | **help** }
 
index b954faeb0f07c25e6d2624b2267b87cdeb1e62ec..a17e9aa314fd070c482145e80bd00649dd010cf9 100644 (file)
@@ -9,13 +9,14 @@ tool for inspection and simple manipulation of eBPF progs
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **cgroup** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
-       { **-f** | **--bpffs** } }
+       *OPTIONS* := { |COMMON_OPTIONS| | { **-f** | **--bpffs** } }
 
        *COMMANDS* :=
        { **show** | **list** | **tree** | **attach** | **detach** | **help** }
index b1471788a15fdf32612556ce166786c4d0ee2573..4ce9a77bc1e06f73800f80609de92797ce82d515 100644 (file)
@@ -9,12 +9,14 @@ tool for inspection of eBPF-related parameters for Linux kernel or net device
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **feature** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
+       *OPTIONS* := { |COMMON_OPTIONS| }
 
        *COMMANDS* := { **probe** | **help** }
 
index 51e2e8de5208e70078d81cd51952bf0cd5ccce76..bc276388f43222d035d04e4a88622889824e7271 100644 (file)
@@ -9,13 +9,14 @@ tool for BPF code-generation
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **gen** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
-       { **-L** | **--use-loader** } }
+       *OPTIONS* := { |COMMON_OPTIONS| | { **-L** | **--use-loader** } }
 
        *COMMAND* := { **object** | **skeleton** | **help** }
 
index 51914c9e8a54dd5692a41e2450f2ff189ab63986..84839d488621e6dbf8736f13b40dc629b9496067 100644 (file)
@@ -9,12 +9,14 @@ tool to create BPF iterators
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **iter** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
+       *OPTIONS* := { |COMMON_OPTIONS| }
 
        *COMMANDS* := { **pin** | **help** }
 
index 31371bcf605a1527e8dd15bed3cf39a8b17953b4..52a4eee4af546fcb1c368ec7c1e06c12ecbe368e 100644 (file)
@@ -9,13 +9,14 @@ tool for inspection and simple manipulation of eBPF links
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **link** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
-       { **-f** | **--bpffs** } | { **-n** | **--nomount** } }
+       *OPTIONS* := { |COMMON_OPTIONS| | { **-f** | **--bpffs** } | { **-n** | **--nomount** } }
 
        *COMMANDS* := { **show** | **list** | **pin** | **help** }
 
index e22c918c069cc571aad1479f75c0135fe3ef75f6..7c188a598444c463c2ea6d06711788b9810f0337 100644 (file)
@@ -9,13 +9,14 @@ tool for inspection and simple manipulation of eBPF maps
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **map** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
-       { **-f** | **--bpffs** } | { **-n** | **--nomount** } }
+       *OPTIONS* := { |COMMON_OPTIONS| | { **-f** | **--bpffs** } | { **-n** | **--nomount** } }
 
        *COMMANDS* :=
        { **show** | **list** | **create** | **dump** | **update** | **lookup** | **getnext** |
index 6d1aa374529f4570570cdce90663f27d3067696e..f4e0a516335acdaebbf345bc883c020b6cd0a57b 100644 (file)
@@ -9,12 +9,14 @@ tool for inspection of netdev/tc related bpf prog attachments
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **net** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
+       *OPTIONS* := { |COMMON_OPTIONS| }
 
        *COMMANDS* :=
        { **show** | **list** | **attach** | **detach** | **help** }
index ad554806faa2ce3f2289cd1ae8765b7e44ce6211..5fea633a82f171fc18455ad7e47cea90a9f686c9 100644 (file)
@@ -9,12 +9,14 @@ tool for inspection of perf related bpf prog attachments
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **perf** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
+       *OPTIONS* := { |COMMON_OPTIONS| }
 
        *COMMANDS* :=
        { **show** | **list** | **help** }
index d31148571403a5343d971331d199c0f9a9f45629..a2e9359e554c8d936a1aa4cc2e91fbc16dfe2a91 100644 (file)
@@ -9,12 +9,14 @@ tool for inspection and simple manipulation of eBPF progs
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **prog** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } |
+       *OPTIONS* := { |COMMON_OPTIONS| |
        { **-f** | **--bpffs** } | { **-m** | **--mapcompat** } | { **-n** | **--nomount** } |
        { **-L** | **--use-loader** } }
 
index 77b845b5ac61af3b007ddf2f85409b609d1733c6..ee53a122c0c780b3ef268ac0d2facd877bead267 100644 (file)
@@ -9,12 +9,14 @@ tool to register/unregister/introspect BPF struct_ops
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
        **bpftool** [*OPTIONS*] **struct_ops** *COMMAND*
 
-       *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
+       *OPTIONS* := { |COMMON_OPTIONS| }
 
        *COMMANDS* :=
        { **show** | **list** | **dump** | **register** | **unregister** | **help** }
index 1248b35e67aedfddc3e8da849b8e886a36587b0e..7084dd9fa2f8224c45b24baba5299d5a5da0b58b 100644 (file)
@@ -9,6 +9,8 @@ tool for inspection and simple manipulation of eBPF programs and maps
 
 :Manual section: 8
 
+.. include:: substitutions.rst
+
 SYNOPSIS
 ========
 
@@ -20,8 +22,7 @@ SYNOPSIS
 
        *OBJECT* := { **map** | **program** | **cgroup** | **perf** | **net** | **feature** }
 
-       *OPTIONS* := { { **-V** | **--version** } |
-       { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } }
+       *OPTIONS* := { { **-V** | **--version** } | |COMMON_OPTIONS| }
 
        *MAP-COMMANDS* :=
        { **show** | **list** | **create** | **dump** | **update** | **lookup** | **getnext** |
diff --git a/tools/bpf/bpftool/Documentation/substitutions.rst b/tools/bpf/bpftool/Documentation/substitutions.rst
new file mode 100644 (file)
index 0000000..ccf1ffa
--- /dev/null
@@ -0,0 +1,3 @@
+.. SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+.. |COMMON_OPTIONS| replace:: { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } | { **-l** | **--legacy** }
index be54b7335a76e87424bd008744afb5b98c008c4b..3f6e562565ec55380cb49d24e7676ecfd5164fcb 100755 (executable)
@@ -242,12 +242,6 @@ class FileExtractor(object):
         end_marker = re.compile('}\\\\n')
         return self.__get_description_list(start_marker, pattern, end_marker)
 
-    def default_options(self):
-        """
-        Return the default options contained in HELP_SPEC_OPTIONS
-        """
-        return { '-j', '--json', '-p', '--pretty', '-d', '--debug' }
-
     def get_bashcomp_list(self, block_name):
         """
         Search for and parse a list of type names from a variable in bash
@@ -274,7 +268,56 @@ class SourceFileExtractor(FileExtractor):
     defined in children classes.
     """
     def get_options(self):
-        return self.default_options().union(self.get_help_list_macro('HELP_SPEC_OPTIONS'))
+        return self.get_help_list_macro('HELP_SPEC_OPTIONS')
+
+class MainHeaderFileExtractor(SourceFileExtractor):
+    """
+    An extractor for bpftool's main.h
+    """
+    filename = os.path.join(BPFTOOL_DIR, 'main.h')
+
+    def get_common_options(self):
+        """
+        Parse the list of common options in main.h (options that apply to all
+        commands), which looks to the lists of options in other source files
+        but has different start and end markers:
+
+            "OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy}"
+
+        Return a set containing all options, such as:
+
+            {'-p', '-d', '--legacy', '--pretty', '--debug', '--json', '-l', '-j'}
+        """
+        start_marker = re.compile(f'"OPTIONS :=')
+        pattern = re.compile('([\w-]+) ?(?:\||}[ }\]"])')
+        end_marker = re.compile('#define')
+
+        parser = InlineListParser(self.reader)
+        parser.search_block(start_marker)
+        return parser.parse(pattern, end_marker)
+
+class ManSubstitutionsExtractor(SourceFileExtractor):
+    """
+    An extractor for substitutions.rst
+    """
+    filename = os.path.join(BPFTOOL_DIR, 'Documentation/substitutions.rst')
+
+    def get_common_options(self):
+        """
+        Parse the list of common options in substitutions.rst (options that
+        apply to all commands).
+
+        Return a set containing all options, such as:
+
+            {'-p', '-d', '--legacy', '--pretty', '--debug', '--json', '-l', '-j'}
+        """
+        start_marker = re.compile('\|COMMON_OPTIONS\| replace:: {')
+        pattern = re.compile('\*\*([\w/-]+)\*\*')
+        end_marker = re.compile('}$')
+
+        parser = InlineListParser(self.reader)
+        parser.search_block(start_marker)
+        return parser.parse(pattern, end_marker)
 
 class ProgFileExtractor(SourceFileExtractor):
     """
@@ -580,6 +623,19 @@ def main():
     verify(help_main_options, man_main_options,
             f'Comparing {source_main_info.filename} (do_help() OPTIONS) and {man_main_info.filename} (OPTIONS):')
 
+    # Compare common options (options that apply to all commands)
+
+    main_hdr_info = MainHeaderFileExtractor()
+    source_common_options = main_hdr_info.get_common_options()
+    main_hdr_info.close()
+
+    man_substitutions = ManSubstitutionsExtractor()
+    man_common_options = man_substitutions.get_common_options()
+    man_substitutions.close()
+
+    verify(source_common_options, man_common_options,
+            f'Comparing common options from {main_hdr_info.filename} (HELP_SPEC_OPTIONS) and {man_substitutions.filename}:')
+
     sys.exit(retval)
 
 if __name__ == "__main__":