* [PATCH bpf-next v3 1/5] bpftool: Move out sections to separate functions
2020-02-25 19:44 [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional Michal Rostecki
@ 2020-02-25 19:44 ` Michal Rostecki
2020-02-25 19:44 ` [PATCH bpf-next v3 2/5] bpftool: Make probes which emit dmesg warnings optional Michal Rostecki
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Michal Rostecki @ 2020-02-25 19:44 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Quentin Monnet, Jakub Kicinski,
netdev, linux-kernel, Shuah Khan, David S. Miller,
Jesper Dangaard Brouer, John Fastabend, linux-kselftest
Remove all calls of print_end_then_start_section function and for loops
out from the do_probe function. Instead, provide separate functions for
each section (like i.e. section_helpers) which are called in do_probe.
This change is motivated by better readability.
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
---
tools/bpf/bpftool/feature.c | 219 +++++++++++++++++++++---------------
1 file changed, 126 insertions(+), 93 deletions(-)
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 941873d778d8..345e4a2b4f53 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -112,18 +112,12 @@ print_start_section(const char *json_title, const char *plain_title,
}
}
-static void
-print_end_then_start_section(const char *json_title, const char *plain_title,
- const char *define_comment,
- const char *define_prefix)
+static void print_end_section(void)
{
if (json_output)
jsonw_end_object(json_wtr);
else
printf("\n");
-
- print_start_section(json_title, plain_title, define_comment,
- define_prefix);
}
/* Probing functions */
@@ -584,13 +578,130 @@ probe_large_insn_limit(const char *define_prefix, __u32 ifindex)
res, define_prefix);
}
+static void
+section_system_config(enum probe_component target, const char *define_prefix)
+{
+ switch (target) {
+ case COMPONENT_KERNEL:
+ case COMPONENT_UNSPEC:
+ if (define_prefix)
+ break;
+
+ print_start_section("system_config",
+ "Scanning system configuration...",
+ NULL, /* define_comment never used here */
+ NULL); /* define_prefix always NULL here */
+ if (check_procfs()) {
+ probe_unprivileged_disabled();
+ probe_jit_enable();
+ probe_jit_harden();
+ probe_jit_kallsyms();
+ probe_jit_limit();
+ } else {
+ p_info("/* procfs not mounted, skipping related probes */");
+ }
+ probe_kernel_image_config();
+ print_end_section();
+ break;
+ default:
+ break;
+ }
+}
+
+static bool section_syscall_config(const char *define_prefix)
+{
+ bool res;
+
+ print_start_section("syscall_config",
+ "Scanning system call availability...",
+ "/*** System call availability ***/",
+ define_prefix);
+ res = probe_bpf_syscall(define_prefix);
+ print_end_section();
+
+ return res;
+}
+
+static void
+section_program_types(bool *supported_types, const char *define_prefix,
+ __u32 ifindex)
+{
+ unsigned int i;
+
+ print_start_section("program_types",
+ "Scanning eBPF program types...",
+ "/*** eBPF program types ***/",
+ define_prefix);
+
+ for (i = BPF_PROG_TYPE_UNSPEC + 1; i < ARRAY_SIZE(prog_type_name); i++)
+ probe_prog_type(i, supported_types, define_prefix, ifindex);
+
+ print_end_section();
+}
+
+static void section_map_types(const char *define_prefix, __u32 ifindex)
+{
+ unsigned int i;
+
+ print_start_section("map_types",
+ "Scanning eBPF map types...",
+ "/*** eBPF map types ***/",
+ define_prefix);
+
+ for (i = BPF_MAP_TYPE_UNSPEC + 1; i < map_type_name_size; i++)
+ probe_map_type(i, define_prefix, ifindex);
+
+ print_end_section();
+}
+
+static void
+section_helpers(bool *supported_types, const char *define_prefix, __u32 ifindex)
+{
+ unsigned int i;
+
+ print_start_section("helpers",
+ "Scanning eBPF helper functions...",
+ "/*** eBPF helper functions ***/",
+ define_prefix);
+
+ if (define_prefix)
+ printf("/*\n"
+ " * Use %sHAVE_PROG_TYPE_HELPER(prog_type_name, helper_name)\n"
+ " * to determine if <helper_name> is available for <prog_type_name>,\n"
+ " * e.g.\n"
+ " * #if %sHAVE_PROG_TYPE_HELPER(xdp, bpf_redirect)\n"
+ " * // do stuff with this helper\n"
+ " * #elif\n"
+ " * // use a workaround\n"
+ " * #endif\n"
+ " */\n"
+ "#define %sHAVE_PROG_TYPE_HELPER(prog_type, helper) \\\n"
+ " %sBPF__PROG_TYPE_ ## prog_type ## __HELPER_ ## helper\n",
+ define_prefix, define_prefix, define_prefix,
+ define_prefix);
+ for (i = BPF_PROG_TYPE_UNSPEC + 1; i < ARRAY_SIZE(prog_type_name); i++)
+ probe_helpers_for_progtype(i, supported_types[i],
+ define_prefix, ifindex);
+
+ print_end_section();
+}
+
+static void section_misc(const char *define_prefix, __u32 ifindex)
+{
+ print_start_section("misc",
+ "Scanning miscellaneous eBPF features...",
+ "/*** eBPF misc features ***/",
+ define_prefix);
+ probe_large_insn_limit(define_prefix, ifindex);
+ print_end_section();
+}
+
static int do_probe(int argc, char **argv)
{
enum probe_component target = COMPONENT_UNSPEC;
const char *define_prefix = NULL;
bool supported_types[128] = {};
__u32 ifindex = 0;
- unsigned int i;
char *ifname;
/* Detection assumes user has sufficient privileges (CAP_SYS_ADMIN).
@@ -658,97 +769,19 @@ static int do_probe(int argc, char **argv)
jsonw_start_object(json_wtr);
}
- switch (target) {
- case COMPONENT_KERNEL:
- case COMPONENT_UNSPEC:
- if (define_prefix)
- break;
-
- print_start_section("system_config",
- "Scanning system configuration...",
- NULL, /* define_comment never used here */
- NULL); /* define_prefix always NULL here */
- if (check_procfs()) {
- probe_unprivileged_disabled();
- probe_jit_enable();
- probe_jit_harden();
- probe_jit_kallsyms();
- probe_jit_limit();
- } else {
- p_info("/* procfs not mounted, skipping related probes */");
- }
- probe_kernel_image_config();
- if (json_output)
- jsonw_end_object(json_wtr);
- else
- printf("\n");
- break;
- default:
- break;
- }
-
- print_start_section("syscall_config",
- "Scanning system call availability...",
- "/*** System call availability ***/",
- define_prefix);
-
- if (!probe_bpf_syscall(define_prefix))
+ section_system_config(target, define_prefix);
+ if (!section_syscall_config(define_prefix))
/* bpf() syscall unavailable, don't probe other BPF features */
goto exit_close_json;
-
- print_end_then_start_section("program_types",
- "Scanning eBPF program types...",
- "/*** eBPF program types ***/",
- define_prefix);
-
- for (i = BPF_PROG_TYPE_UNSPEC + 1; i < ARRAY_SIZE(prog_type_name); i++)
- probe_prog_type(i, supported_types, define_prefix, ifindex);
-
- print_end_then_start_section("map_types",
- "Scanning eBPF map types...",
- "/*** eBPF map types ***/",
- define_prefix);
-
- for (i = BPF_MAP_TYPE_UNSPEC + 1; i < map_type_name_size; i++)
- probe_map_type(i, define_prefix, ifindex);
-
- print_end_then_start_section("helpers",
- "Scanning eBPF helper functions...",
- "/*** eBPF helper functions ***/",
- define_prefix);
-
- if (define_prefix)
- printf("/*\n"
- " * Use %sHAVE_PROG_TYPE_HELPER(prog_type_name, helper_name)\n"
- " * to determine if <helper_name> is available for <prog_type_name>,\n"
- " * e.g.\n"
- " * #if %sHAVE_PROG_TYPE_HELPER(xdp, bpf_redirect)\n"
- " * // do stuff with this helper\n"
- " * #elif\n"
- " * // use a workaround\n"
- " * #endif\n"
- " */\n"
- "#define %sHAVE_PROG_TYPE_HELPER(prog_type, helper) \\\n"
- " %sBPF__PROG_TYPE_ ## prog_type ## __HELPER_ ## helper\n",
- define_prefix, define_prefix, define_prefix,
- define_prefix);
- for (i = BPF_PROG_TYPE_UNSPEC + 1; i < ARRAY_SIZE(prog_type_name); i++)
- probe_helpers_for_progtype(i, supported_types[i],
- define_prefix, ifindex);
-
- print_end_then_start_section("misc",
- "Scanning miscellaneous eBPF features...",
- "/*** eBPF misc features ***/",
- define_prefix);
- probe_large_insn_limit(define_prefix, ifindex);
+ section_program_types(supported_types, define_prefix, ifindex);
+ section_map_types(define_prefix, ifindex);
+ section_helpers(supported_types, define_prefix, ifindex);
+ section_misc(define_prefix, ifindex);
exit_close_json:
- if (json_output) {
- /* End current "section" of probes */
- jsonw_end_object(json_wtr);
+ if (json_output)
/* End root object */
jsonw_end_object(json_wtr);
- }
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH bpf-next v3 2/5] bpftool: Make probes which emit dmesg warnings optional
2020-02-25 19:44 [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional Michal Rostecki
2020-02-25 19:44 ` [PATCH bpf-next v3 1/5] bpftool: Move out sections to separate functions Michal Rostecki
@ 2020-02-25 19:44 ` Michal Rostecki
2020-02-26 15:35 ` Daniel Borkmann
2020-02-25 19:44 ` [PATCH bpf-next v3 3/5] bpftool: Update documentation of "bpftool feature" command Michal Rostecki
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Michal Rostecki @ 2020-02-25 19:44 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Quentin Monnet, Jakub Kicinski,
netdev, linux-kernel, Shuah Khan, linux-kselftest
Probes related to bpf_probe_write_user and bpf_trace_printk helpers emit
dmesg warnings which might be confusing for people running bpftool on
production environments. This change filters them out by default and
introduces the new positional argument "full" which enables all
available probes.
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
---
tools/bpf/bpftool/feature.c | 70 ++++++++++++++++++++++++++-----------
1 file changed, 49 insertions(+), 21 deletions(-)
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 345e4a2b4f53..ada3e1502b45 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -513,14 +513,39 @@ probe_map_type(enum bpf_map_type map_type, const char *define_prefix,
define_prefix);
}
+static void
+probe_helper_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
+ const char *define_prefix, unsigned int id,
+ const char *ptype_name, __u32 ifindex)
+{
+ bool res;
+
+ if (!supported_type)
+ res = false;
+ else
+ res = bpf_probe_helper(id, prog_type, ifindex);
+
+ if (json_output) {
+ if (res)
+ jsonw_string(json_wtr, helper_name[id]);
+ } else if (define_prefix) {
+ printf("#define %sBPF__PROG_TYPE_%s__HELPER_%s %s\n",
+ define_prefix, ptype_name, helper_name[id],
+ res ? "1" : "0");
+ } else {
+ if (res)
+ printf("\n\t- %s", helper_name[id]);
+ }
+}
+
static void
probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
- const char *define_prefix, __u32 ifindex)
+ const char *define_prefix, bool full_mode,
+ __u32 ifindex)
{
const char *ptype_name = prog_type_name[prog_type];
char feat_name[128];
unsigned int id;
- bool res;
if (ifindex)
/* Only test helpers for offload-able program types */
@@ -542,21 +567,19 @@ probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
}
for (id = 1; id < ARRAY_SIZE(helper_name); id++) {
- if (!supported_type)
- res = false;
- else
- res = bpf_probe_helper(id, prog_type, ifindex);
-
- if (json_output) {
- if (res)
- jsonw_string(json_wtr, helper_name[id]);
- } else if (define_prefix) {
- printf("#define %sBPF__PROG_TYPE_%s__HELPER_%s %s\n",
- define_prefix, ptype_name, helper_name[id],
- res ? "1" : "0");
- } else {
- if (res)
- printf("\n\t- %s", helper_name[id]);
+ /* Skip helper functions which emit dmesg messages when not in
+ * the full mode.
+ */
+ switch (id) {
+ case 6: /* trace_printk */
+ case 36: /* probe_write_user */
+ if (!full_mode)
+ continue;
+ /* fallthrough */
+ default:
+ probe_helper_for_progtype(prog_type, supported_type,
+ define_prefix, id, ptype_name,
+ ifindex);
}
}
@@ -655,7 +678,8 @@ static void section_map_types(const char *define_prefix, __u32 ifindex)
}
static void
-section_helpers(bool *supported_types, const char *define_prefix, __u32 ifindex)
+section_helpers(bool *supported_types, const char *define_prefix,
+ bool full_mode, __u32 ifindex)
{
unsigned int i;
@@ -681,7 +705,7 @@ section_helpers(bool *supported_types, const char *define_prefix, __u32 ifindex)
define_prefix);
for (i = BPF_PROG_TYPE_UNSPEC + 1; i < ARRAY_SIZE(prog_type_name); i++)
probe_helpers_for_progtype(i, supported_types[i],
- define_prefix, ifindex);
+ define_prefix, full_mode, ifindex);
print_end_section();
}
@@ -701,6 +725,7 @@ static int do_probe(int argc, char **argv)
enum probe_component target = COMPONENT_UNSPEC;
const char *define_prefix = NULL;
bool supported_types[128] = {};
+ bool full_mode = false;
__u32 ifindex = 0;
char *ifname;
@@ -740,6 +765,9 @@ static int do_probe(int argc, char **argv)
strerror(errno));
return -1;
}
+ } else if (is_prefix(*argv, "full")) {
+ full_mode = true;
+ NEXT_ARG();
} else if (is_prefix(*argv, "macros") && !define_prefix) {
define_prefix = "";
NEXT_ARG();
@@ -775,7 +803,7 @@ static int do_probe(int argc, char **argv)
goto exit_close_json;
section_program_types(supported_types, define_prefix, ifindex);
section_map_types(define_prefix, ifindex);
- section_helpers(supported_types, define_prefix, ifindex);
+ section_helpers(supported_types, define_prefix, full_mode, ifindex);
section_misc(define_prefix, ifindex);
exit_close_json:
@@ -794,7 +822,7 @@ static int do_help(int argc, char **argv)
}
fprintf(stderr,
- "Usage: %s %s probe [COMPONENT] [macros [prefix PREFIX]]\n"
+ "Usage: %s %s probe [COMPONENT] [full] [macros [prefix PREFIX]]\n"
" %s %s help\n"
"\n"
" COMPONENT := { kernel | dev NAME }\n"
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH bpf-next v3 2/5] bpftool: Make probes which emit dmesg warnings optional
2020-02-25 19:44 ` [PATCH bpf-next v3 2/5] bpftool: Make probes which emit dmesg warnings optional Michal Rostecki
@ 2020-02-26 15:35 ` Daniel Borkmann
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Borkmann @ 2020-02-26 15:35 UTC (permalink / raw)
To: Michal Rostecki, bpf
Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, Quentin Monnet, Jakub Kicinski, netdev,
linux-kernel, Shuah Khan, linux-kselftest
On 2/25/20 8:44 PM, Michal Rostecki wrote:
> Probes related to bpf_probe_write_user and bpf_trace_printk helpers emit
> dmesg warnings which might be confusing for people running bpftool on
> production environments. This change filters them out by default and
> introduces the new positional argument "full" which enables all
> available probes.
>
> Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
[...]
> + /* Skip helper functions which emit dmesg messages when not in
> + * the full mode.
> + */
> + switch (id) {
> + case 6: /* trace_printk */
> + case 36: /* probe_write_user */
Please fix this into the actual enum above, then also the comment is not needed.
> + if (!full_mode)
> + continue;
> + /* fallthrough */
> + default:
> + probe_helper_for_progtype(prog_type, supported_type,
> + define_prefix, id, ptype_name,
> + ifindex);
> }
> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH bpf-next v3 3/5] bpftool: Update documentation of "bpftool feature" command
2020-02-25 19:44 [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional Michal Rostecki
2020-02-25 19:44 ` [PATCH bpf-next v3 1/5] bpftool: Move out sections to separate functions Michal Rostecki
2020-02-25 19:44 ` [PATCH bpf-next v3 2/5] bpftool: Make probes which emit dmesg warnings optional Michal Rostecki
@ 2020-02-25 19:44 ` Michal Rostecki
2020-02-25 19:44 ` [PATCH bpf-next v3 4/5] bpftool: Update bash completion for " Michal Rostecki
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Michal Rostecki @ 2020-02-25 19:44 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Quentin Monnet, Jakub Kicinski,
netdev, linux-kernel, Shuah Khan, linux-kselftest
Update documentation of "bpftool feature" command with information about
new arguments: "full".
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
---
.../bpftool/Documentation/bpftool-feature.rst | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
index 4d08f35034a2..b04156cfd7a3 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
@@ -19,19 +19,24 @@ SYNOPSIS
FEATURE COMMANDS
================
-| **bpftool** **feature probe** [*COMPONENT*] [**macros** [**prefix** *PREFIX*]]
+| **bpftool** **feature probe** [*COMPONENT*] [**full**] [**macros** [**prefix** *PREFIX*]]
| **bpftool** **feature help**
|
| *COMPONENT* := { **kernel** | **dev** *NAME* }
DESCRIPTION
===========
- **bpftool feature probe** [**kernel**] [**macros** [**prefix** *PREFIX*]]
+ **bpftool feature probe** [**kernel**] [**full**] [**macros** [**prefix** *PREFIX*]]
Probe the running kernel and dump a number of eBPF-related
parameters, such as availability of the **bpf()** system call,
JIT status, eBPF program types availability, eBPF helper
functions availability, and more.
+ By default, bpftool **does not run probes** for
+ **bpf_probe_write_user**\ () and **bpf_trace_printk**\()
+ helpers which print warnings to kernel logs. To enable them
+ and run all probes, the **full** keyword should be used.
+
If the **macros** keyword (but not the **-j** option) is
passed, a subset of the output is dumped as a list of
**#define** macros that are ready to be included in a C
@@ -44,16 +49,12 @@ DESCRIPTION
Keyword **kernel** can be omitted. If no probe target is
specified, probing the kernel is the default behaviour.
- Note that when probed, some eBPF helpers (e.g.
- **bpf_trace_printk**\ () or **bpf_probe_write_user**\ ()) may
- print warnings to kernel logs.
-
- **bpftool feature probe dev** *NAME* [**macros** [**prefix** *PREFIX*]]
+ **bpftool feature probe dev** *NAME* [**full**] [**macros** [**prefix** *PREFIX*]]
Probe network device for supported eBPF features and dump
results to the console.
- The two keywords **macros** and **prefix** have the same
- role as when probing the kernel.
+ The keywords **full**, **macros** and **prefix** have the
+ same role as when probing the kernel.
**bpftool feature help**
Print short help message.
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH bpf-next v3 4/5] bpftool: Update bash completion for "bpftool feature" command
2020-02-25 19:44 [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional Michal Rostecki
` (2 preceding siblings ...)
2020-02-25 19:44 ` [PATCH bpf-next v3 3/5] bpftool: Update documentation of "bpftool feature" command Michal Rostecki
@ 2020-02-25 19:44 ` Michal Rostecki
2020-02-25 19:44 ` [PATCH bpf-next v3 5/5] selftests/bpf: Add test " Michal Rostecki
2020-02-26 11:17 ` [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional Quentin Monnet
5 siblings, 0 replies; 13+ messages in thread
From: Michal Rostecki @ 2020-02-25 19:44 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Quentin Monnet, Jakub Kicinski,
netdev, linux-kernel, Shuah Khan, linux-kselftest
Update bash completion for "bpftool feature" command with the new
argument: "full".
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
---
tools/bpf/bpftool/bash-completion/bpftool | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 754d8395e451..acd5ea76c91d 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -983,11 +983,12 @@ _bpftool()
probe)
[[ $prev == "prefix" ]] && return 0
if _bpftool_search_list 'macros'; then
- COMPREPLY+=( $( compgen -W 'prefix' -- "$cur" ) )
+ _bpftool_once_attr 'prefix'
else
COMPREPLY+=( $( compgen -W 'macros' -- "$cur" ) )
fi
_bpftool_one_of_list 'kernel dev'
+ _bpftool_once_attr 'full'
return 0
;;
*)
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH bpf-next v3 5/5] selftests/bpf: Add test for "bpftool feature" command
2020-02-25 19:44 [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional Michal Rostecki
` (3 preceding siblings ...)
2020-02-25 19:44 ` [PATCH bpf-next v3 4/5] bpftool: Update bash completion for " Michal Rostecki
@ 2020-02-25 19:44 ` Michal Rostecki
2020-02-26 15:34 ` Daniel Borkmann
2020-02-26 11:17 ` [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional Quentin Monnet
5 siblings, 1 reply; 13+ messages in thread
From: Michal Rostecki @ 2020-02-25 19:44 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Quentin Monnet, Jakub Kicinski,
netdev, linux-kernel, Shuah Khan, David S. Miller,
Jesper Dangaard Brouer, John Fastabend, linux-kselftest
Add Python module with tests for "bpftool feature" command, which mainly
wheck whether the "full" option is working properly.
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
---
tools/testing/selftests/.gitignore | 5 +-
tools/testing/selftests/bpf/Makefile | 3 +-
tools/testing/selftests/bpf/test_bpftool.py | 179 ++++++++++++++++++++
tools/testing/selftests/bpf/test_bpftool.sh | 5 +
4 files changed, 190 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/test_bpftool.py
create mode 100755 tools/testing/selftests/bpf/test_bpftool.sh
diff --git a/tools/testing/selftests/.gitignore b/tools/testing/selftests/.gitignore
index 61df01cdf0b2..304fdf1a21dc 100644
--- a/tools/testing/selftests/.gitignore
+++ b/tools/testing/selftests/.gitignore
@@ -3,4 +3,7 @@ gpiogpio-hammer
gpioinclude/
gpiolsgpio
tpm2/SpaceTest.log
-tpm2/*.pyc
+
+# Python bytecode and cache
+__pycache__/
+*.py[cod]
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 2a583196fa51..2dffce6cc429 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -62,7 +62,8 @@ TEST_PROGS := test_kmod.sh \
test_tc_tunnel.sh \
test_tc_edt.sh \
test_xdping.sh \
- test_bpftool_build.sh
+ test_bpftool_build.sh \
+ test_bpftool.sh
TEST_PROGS_EXTENDED := with_addr.sh \
with_tunnels.sh \
diff --git a/tools/testing/selftests/bpf/test_bpftool.py b/tools/testing/selftests/bpf/test_bpftool.py
new file mode 100644
index 000000000000..c8e54843a487
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_bpftool.py
@@ -0,0 +1,179 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 SUSE LLC.
+
+import collections
+import functools
+import json
+import os
+import socket
+import subprocess
+import unittest
+
+
+# Add the source tree of bpftool and /usr/local/sbin to PATH
+cur_dir = os.path.dirname(os.path.realpath(__file__))
+bpftool_dir = os.path.abspath(os.path.join(cur_dir, "..", "..", "..", "..",
+ "tools", "bpf", "bpftool"))
+os.environ["PATH"] = bpftool_dir + ":/usr/local/sbin:" + os.environ["PATH"]
+
+
+class IfaceNotFoundError(Exception):
+ pass
+
+
+class UnprivilegedUserError(Exception):
+ pass
+
+
+def _bpftool(args, json=True):
+ _args = ["bpftool"]
+ if json:
+ _args.append("-j")
+ _args.extend(args)
+
+ res = subprocess.run(_args, capture_output=True)
+ return res.stdout
+
+
+def bpftool(args):
+ return _bpftool(args, json=False).decode("utf-8")
+
+
+def bpftool_json(args):
+ res = _bpftool(args)
+ return json.loads(res)
+
+
+def get_default_iface():
+ for iface in socket.if_nameindex():
+ if iface[1] != "lo":
+ return iface[1]
+ raise IfaceNotFoundError("Could not find any network interface to probe")
+
+
+def default_iface(f):
+ @functools.wraps(f)
+ def wrapper(*args, **kwargs):
+ iface = get_default_iface()
+ return f(*args, iface, **kwargs)
+ return wrapper
+
+
+class TestBpftool(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ if os.getuid() != 0:
+ raise UnprivilegedUserError(
+ "This test suite needs root privileges")
+
+ @default_iface
+ def test_feature_dev_json(self, iface):
+ unexpected_helpers = [
+ "bpf_probe_write_user",
+ "bpf_trace_printk",
+ ]
+ expected_keys = [
+ "syscall_config",
+ "program_types",
+ "map_types",
+ "helpers",
+ "misc",
+ ]
+
+ res = bpftool_json(["feature", "probe", "dev", iface])
+ # Check if the result has all expected keys.
+ self.assertCountEqual(res.keys(), expected_keys)
+ # Check if unexpected helpers are not included in helpers probes
+ # result.
+ for helpers in res["helpers"].values():
+ for unexpected_helper in unexpected_helpers:
+ self.assertNotIn(unexpected_helper, helpers)
+
+ def test_feature_kernel(self):
+ test_cases = [
+ bpftool_json(["feature", "probe", "kernel"]),
+ bpftool_json(["feature", "probe"]),
+ bpftool_json(["feature"]),
+ ]
+ unexpected_helpers = [
+ "bpf_probe_write_user",
+ "bpf_trace_printk",
+ ]
+ expected_keys = [
+ "syscall_config",
+ "system_config",
+ "program_types",
+ "map_types",
+ "helpers",
+ "misc",
+ ]
+
+ for tc in test_cases:
+ # Check if the result has all expected keys.
+ self.assertCountEqual(tc.keys(), expected_keys)
+ # Check if unexpected helpers are not included in helpers probes
+ # result.
+ for helpers in tc["helpers"].values():
+ for unexpected_helper in unexpected_helpers:
+ self.assertNotIn(unexpected_helper, helpers)
+
+ def test_feature_kernel_full(self):
+ test_cases = [
+ bpftool_json(["feature", "probe", "kernel", "full"]),
+ bpftool_json(["feature", "probe", "full"]),
+ ]
+ expected_helpers = [
+ "bpf_probe_write_user",
+ "bpf_trace_printk",
+ ]
+
+ for tc in test_cases:
+ # Check if expected helpers are included at least once in any
+ # helpers list for any program type. Unfortunately we cannot assume
+ # that they will be included in all program types or a specific
+ # subset of programs. It depends on the kernel version and
+ # configuration.
+ found_helpers = False
+
+ for helpers in tc["helpers"].values():
+ if all(expected_helper in helpers
+ for expected_helper in expected_helpers):
+ found_helpers = True
+ break
+
+ self.assertTrue(found_helpers)
+
+ def test_feature_kernel_full_vs_not_full(self):
+ full_res = bpftool_json(["feature", "probe", "full"])
+ not_full_res = bpftool_json(["feature", "probe"])
+ not_full_set = set()
+ full_set = set()
+
+ for helpers in full_res["helpers"].values():
+ for helper in helpers:
+ full_set.add(helper)
+
+ for helpers in not_full_res["helpers"].values():
+ for helper in helpers:
+ not_full_set.add(helper)
+
+ self.assertCountEqual(full_set - not_full_set,
+ {"bpf_probe_write_user", "bpf_trace_printk"})
+ self.assertCountEqual(not_full_set - full_set, set())
+
+ def test_feature_macros(self):
+ expected_patterns = [
+ r"/\*\*\* System call availability \*\*\*/",
+ r"#define HAVE_BPF_SYSCALL",
+ r"/\*\*\* eBPF program types \*\*\*/",
+ r"#define HAVE.*PROG_TYPE",
+ r"/\*\*\* eBPF map types \*\*\*/",
+ r"#define HAVE.*MAP_TYPE",
+ r"/\*\*\* eBPF helper functions \*\*\*/",
+ r"#define HAVE.*HELPER",
+ r"/\*\*\* eBPF misc features \*\*\*/",
+ ]
+
+ res = bpftool(["feature", "probe", "macros"])
+ for pattern in expected_patterns:
+ self.assertRegex(res, pattern)
diff --git a/tools/testing/selftests/bpf/test_bpftool.sh b/tools/testing/selftests/bpf/test_bpftool.sh
new file mode 100755
index 000000000000..66690778e36d
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_bpftool.sh
@@ -0,0 +1,5 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 SUSE LLC.
+
+python3 -m unittest -v test_bpftool.TestBpftool
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH bpf-next v3 5/5] selftests/bpf: Add test for "bpftool feature" command
2020-02-25 19:44 ` [PATCH bpf-next v3 5/5] selftests/bpf: Add test " Michal Rostecki
@ 2020-02-26 15:34 ` Daniel Borkmann
2020-02-26 15:42 ` Michal Rostecki
2020-02-26 15:43 ` Quentin Monnet
0 siblings, 2 replies; 13+ messages in thread
From: Daniel Borkmann @ 2020-02-26 15:34 UTC (permalink / raw)
To: Michal Rostecki, bpf
Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, Quentin Monnet, Jakub Kicinski, netdev,
linux-kernel, Shuah Khan, David S. Miller,
Jesper Dangaard Brouer, John Fastabend, linux-kselftest
On 2/25/20 8:44 PM, Michal Rostecki wrote:
> Add Python module with tests for "bpftool feature" command, which mainly
> wheck whether the "full" option is working properly.
nit, typo: wheck
>
> Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
Ptal, when running the test I'm getting the following error:
root@tank:~/bpf-next/tools/testing/selftests/bpf# ./test_bpftool.sh
test_feature_dev_json (test_bpftool.TestBpftool) ... ERROR
test_feature_kernel (test_bpftool.TestBpftool) ... ERROR
test_feature_kernel_full (test_bpftool.TestBpftool) ... ERROR
test_feature_kernel_full_vs_not_full (test_bpftool.TestBpftool) ... ERROR
test_feature_macros (test_bpftool.TestBpftool) ... ERROR
======================================================================
ERROR: test_feature_dev_json (test_bpftool.TestBpftool)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 58, in wrapper
return f(*args, iface, **kwargs)
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 83, in test_feature_dev_json
res = bpftool_json(["feature", "probe", "dev", iface])
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 43, in bpftool_json
res = _bpftool(args)
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 34, in _bpftool
res = subprocess.run(_args, capture_output=True)
File "/usr/lib/python3.6/subprocess.py", line 423, in run
with Popen(*popenargs, **kwargs) as process:
TypeError: __init__() got an unexpected keyword argument 'capture_output'
======================================================================
ERROR: test_feature_kernel (test_bpftool.TestBpftool)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 94, in test_feature_kernel
bpftool_json(["feature", "probe", "kernel"]),
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 43, in bpftool_json
res = _bpftool(args)
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 34, in _bpftool
res = subprocess.run(_args, capture_output=True)
File "/usr/lib/python3.6/subprocess.py", line 423, in run
with Popen(*popenargs, **kwargs) as process:
TypeError: __init__() got an unexpected keyword argument 'capture_output'
======================================================================
ERROR: test_feature_kernel_full (test_bpftool.TestBpftool)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 122, in test_feature_kernel_full
bpftool_json(["feature", "probe", "kernel", "full"]),
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 43, in bpftool_json
res = _bpftool(args)
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 34, in _bpftool
res = subprocess.run(_args, capture_output=True)
File "/usr/lib/python3.6/subprocess.py", line 423, in run
with Popen(*popenargs, **kwargs) as process:
TypeError: __init__() got an unexpected keyword argument 'capture_output'
======================================================================
ERROR: test_feature_kernel_full_vs_not_full (test_bpftool.TestBpftool)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 147, in test_feature_kernel_full_vs_not_full
full_res = bpftool_json(["feature", "probe", "full"])
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 43, in bpftool_json
res = _bpftool(args)
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 34, in _bpftool
res = subprocess.run(_args, capture_output=True)
File "/usr/lib/python3.6/subprocess.py", line 423, in run
with Popen(*popenargs, **kwargs) as process:
TypeError: __init__() got an unexpected keyword argument 'capture_output'
======================================================================
ERROR: test_feature_macros (test_bpftool.TestBpftool)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 177, in test_feature_macros
res = bpftool(["feature", "probe", "macros"])
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 39, in bpftool
return _bpftool(args, json=False).decode("utf-8")
File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py", line 34, in _bpftool
res = subprocess.run(_args, capture_output=True)
File "/usr/lib/python3.6/subprocess.py", line 423, in run
with Popen(*popenargs, **kwargs) as process:
TypeError: __init__() got an unexpected keyword argument 'capture_output'
----------------------------------------------------------------------
Ran 5 tests in 0.001s
FAILED (errors=5)
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH bpf-next v3 5/5] selftests/bpf: Add test for "bpftool feature" command
2020-02-26 15:34 ` Daniel Borkmann
@ 2020-02-26 15:42 ` Michal Rostecki
2020-02-26 15:43 ` Quentin Monnet
1 sibling, 0 replies; 13+ messages in thread
From: Michal Rostecki @ 2020-02-26 15:42 UTC (permalink / raw)
To: Daniel Borkmann, bpf
Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, Quentin Monnet, Jakub Kicinski, netdev,
linux-kernel, Shuah Khan, David S. Miller,
Jesper Dangaard Brouer, John Fastabend, linux-kselftest
On 2/26/20 4:34 PM, Daniel Borkmann wrote:
> Ptal, when running the test I'm getting the following error:
>
> root@tank:~/bpf-next/tools/testing/selftests/bpf# ./test_bpftool.sh
> test_feature_dev_json (test_bpftool.TestBpftool) ... ERROR
> test_feature_kernel (test_bpftool.TestBpftool) ... ERROR
> test_feature_kernel_full (test_bpftool.TestBpftool) ... ERROR
> test_feature_kernel_full_vs_not_full (test_bpftool.TestBpftool) ... ERROR
> test_feature_macros (test_bpftool.TestBpftool) ... ERROR
>
> ======================================================================
> ERROR: test_feature_dev_json (test_bpftool.TestBpftool)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py",
> line 58, in wrapper
> return f(*args, iface, **kwargs)
> File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py",
> line 83, in test_feature_dev_json
> res = bpftool_json(["feature", "probe", "dev", iface])
> File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py",
> line 43, in bpftool_json
> res = _bpftool(args)
> File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py",
> line 34, in _bpftool
> res = subprocess.run(_args, capture_output=True)
> File "/usr/lib/python3.6/subprocess.py", line 423, in run
> with Popen(*popenargs, **kwargs) as process:
> TypeError: __init__() got an unexpected keyword argument 'capture_output'
Seems like that kwarg in Popen was added in Python 3.7. I will drop it
and use the older way of getting combined output. Thanks for pointing
that out!
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v3 5/5] selftests/bpf: Add test for "bpftool feature" command
2020-02-26 15:34 ` Daniel Borkmann
2020-02-26 15:42 ` Michal Rostecki
@ 2020-02-26 15:43 ` Quentin Monnet
1 sibling, 0 replies; 13+ messages in thread
From: Quentin Monnet @ 2020-02-26 15:43 UTC (permalink / raw)
To: Daniel Borkmann, Michal Rostecki, bpf
Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, Jakub Kicinski, netdev, linux-kernel,
Shuah Khan, David S. Miller, Jesper Dangaard Brouer,
John Fastabend, linux-kselftest
2020-02-26 16:34 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net>
> On 2/25/20 8:44 PM, Michal Rostecki wrote:
>> Add Python module with tests for "bpftool feature" command, which mainly
>> wheck whether the "full" option is working properly.
>
> nit, typo: wheck
>
>>
>> Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
>
> Ptal, when running the test I'm getting the following error:
>
> root@tank:~/bpf-next/tools/testing/selftests/bpf# ./test_bpftool.sh
> test_feature_dev_json (test_bpftool.TestBpftool) ... ERROR
> test_feature_kernel (test_bpftool.TestBpftool) ... ERROR
> test_feature_kernel_full (test_bpftool.TestBpftool) ... ERROR
> test_feature_kernel_full_vs_not_full (test_bpftool.TestBpftool) ... ERROR
> test_feature_macros (test_bpftool.TestBpftool) ... ERROR
>
> ======================================================================
> ERROR: test_feature_dev_json (test_bpftool.TestBpftool)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py",
> line 58, in wrapper
> return f(*args, iface, **kwargs)
> File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py",
> line 83, in test_feature_dev_json
> res = bpftool_json(["feature", "probe", "dev", iface])
> File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py",
> line 43, in bpftool_json
> res = _bpftool(args)
> File "/root/bpf-next/tools/testing/selftests/bpf/test_bpftool.py",
> line 34, in _bpftool
> res = subprocess.run(_args, capture_output=True)
> File "/usr/lib/python3.6/subprocess.py", line 423, in run
> with Popen(*popenargs, **kwargs) as process:
> TypeError: __init__() got an unexpected keyword argument 'capture_output'
Apparently the “capture_output” option for subprocess was added to
python 3.7 [0]. It worked on my system (python 3.7.5) but didn't pass on
yours with 3.6.
Michal, can you change it to something less recent please, so that
people don't have to upgrade python to test?
Quentin
[0] https://docs.python.org/3/whatsnew/3.7.html#subprocess
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional
2020-02-25 19:44 [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional Michal Rostecki
` (4 preceding siblings ...)
2020-02-25 19:44 ` [PATCH bpf-next v3 5/5] selftests/bpf: Add test " Michal Rostecki
@ 2020-02-26 11:17 ` Quentin Monnet
2020-02-26 12:17 ` Michal Rostecki
5 siblings, 1 reply; 13+ messages in thread
From: Quentin Monnet @ 2020-02-26 11:17 UTC (permalink / raw)
To: Michal Rostecki, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Jakub Kicinski, netdev,
linux-kernel, Shuah Khan, linux-kselftest
2020-02-25 20:44 UTC+0100 ~ Michal Rostecki <mrostecki@opensuse.org>
> Feature probes in bpftool related to bpf_probe_write_user and
> bpf_trace_printk helpers emit dmesg warnings which might be confusing
> for people running bpftool on production environments. This patch series
> addresses that by filtering them out by default and introducing the new
> positional argument "full" which enables all available probes.
>
> The main motivation behind those changes is ability the fact that some
> probes (for example those related to "trace" or "write_user" helpers)
> emit dmesg messages which might be confusing for people who are running
> on production environments. For details see the Cilium issue[0].
>
> v1 -> v2:
> - Do not expose regex filters to users, keep filtering logic internal,
> expose only the "full" option for including probes which emit dmesg
> warnings.
>
> v2 -> v3:
> - Do not use regex for filtering out probes, use function IDs directly.
> - Fix bash completion - in v2 only "prefix" was proposed after "macros",
> "dev" and "kernel" were not.
> - Rephrase the man page paragraph, highlight helper function names.
> - Remove tests which parse the plain output of bpftool (except the
> header/macros test), focus on testing JSON output instead.
> - Add test which compares the output with and without "full" option.
>
> [0] https://github.com/cilium/cilium/issues/10048
>
> Michal Rostecki (5):
> bpftool: Move out sections to separate functions
> bpftool: Make probes which emit dmesg warnings optional
> bpftool: Update documentation of "bpftool feature" command
> bpftool: Update bash completion for "bpftool feature" command
> selftests/bpf: Add test for "bpftool feature" command
>
> .../bpftool/Documentation/bpftool-feature.rst | 19 +-
> tools/bpf/bpftool/bash-completion/bpftool | 3 +-
> tools/bpf/bpftool/feature.c | 283 +++++++++++-------
> tools/testing/selftests/.gitignore | 5 +-
> tools/testing/selftests/bpf/Makefile | 3 +-
> tools/testing/selftests/bpf/test_bpftool.py | 179 +++++++++++
> tools/testing/selftests/bpf/test_bpftool.sh | 5 +
> 7 files changed, 374 insertions(+), 123 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/test_bpftool.py
> create mode 100755 tools/testing/selftests/bpf/test_bpftool.sh
>
This version looks good to me, thanks!
Reviewed-by: Quentin Monnet <quentin@isovalent.com>
(Please keep Acked-by/Reviewed-by tags between versions if there are no
significant changes, here for patch 1.)
That's a lot of tests now that we don't have the regex and filtering is
very straightforward, but it does not hurt. I checked and they all pass
on my system.
Thanks,
Quentin
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional
2020-02-26 11:17 ` [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional Quentin Monnet
@ 2020-02-26 12:17 ` Michal Rostecki
2020-02-26 12:33 ` Quentin Monnet
0 siblings, 1 reply; 13+ messages in thread
From: Michal Rostecki @ 2020-02-26 12:17 UTC (permalink / raw)
To: Quentin Monnet, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Jakub Kicinski, netdev,
linux-kernel, Shuah Khan, linux-kselftest
On 2/26/20 12:17 PM, Quentin Monnet wrote:
> 2020-02-25 20:44 UTC+0100 ~ Michal Rostecki <mrostecki@opensuse.org>
>> Feature probes in bpftool related to bpf_probe_write_user and
>> bpf_trace_printk helpers emit dmesg warnings which might be confusing
>> for people running bpftool on production environments. This patch series
>> addresses that by filtering them out by default and introducing the new
>> positional argument "full" which enables all available probes.
>>
>> The main motivation behind those changes is ability the fact that some
>> probes (for example those related to "trace" or "write_user" helpers)
>> emit dmesg messages which might be confusing for people who are running
>> on production environments. For details see the Cilium issue[0].
>>
>> v1 -> v2:
>> - Do not expose regex filters to users, keep filtering logic internal,
>> expose only the "full" option for including probes which emit dmesg
>> warnings.
>>
>> v2 -> v3:
>> - Do not use regex for filtering out probes, use function IDs directly.
>> - Fix bash completion - in v2 only "prefix" was proposed after "macros",
>> "dev" and "kernel" were not.
>> - Rephrase the man page paragraph, highlight helper function names.
>> - Remove tests which parse the plain output of bpftool (except the
>> header/macros test), focus on testing JSON output instead.
>> - Add test which compares the output with and without "full" option.
>>
>> [0] https://github.com/cilium/cilium/issues/10048
>>
>> Michal Rostecki (5):
>> bpftool: Move out sections to separate functions
>> bpftool: Make probes which emit dmesg warnings optional
>> bpftool: Update documentation of "bpftool feature" command
>> bpftool: Update bash completion for "bpftool feature" command
>> selftests/bpf: Add test for "bpftool feature" command
>>
>> .../bpftool/Documentation/bpftool-feature.rst | 19 +-
>> tools/bpf/bpftool/bash-completion/bpftool | 3 +-
>> tools/bpf/bpftool/feature.c | 283 +++++++++++-------
>> tools/testing/selftests/.gitignore | 5 +-
>> tools/testing/selftests/bpf/Makefile | 3 +-
>> tools/testing/selftests/bpf/test_bpftool.py | 179 +++++++++++
>> tools/testing/selftests/bpf/test_bpftool.sh | 5 +
>> 7 files changed, 374 insertions(+), 123 deletions(-)
>> create mode 100644 tools/testing/selftests/bpf/test_bpftool.py
>> create mode 100755 tools/testing/selftests/bpf/test_bpftool.sh
>>
>
> This version looks good to me, thanks!
>
> Reviewed-by: Quentin Monnet <quentin@isovalent.com>
>
> (Please keep Acked-by/Reviewed-by tags between versions if there are no
> significant changes, here for patch 1.)
Sorry, I will do that next time.
> That's a lot of tests now that we don't have the regex and filtering is
> very straightforward, but it does not hurt. I checked and they all pass
> on my system.
I know that those tests were necessary with the regex implementation and
now they may seem to be an overkill. But on the other hand, I think that
having selftests for bpftool in general is a good thing, so I didn't
want throw them away despite the easier implementation of my patches. I
might follow up with some more tests covering the other subcommands in
future.
Cheers,
Michal
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v3 0/5] bpftool: Make probes which emit dmesg warnings optional
2020-02-26 12:17 ` Michal Rostecki
@ 2020-02-26 12:33 ` Quentin Monnet
0 siblings, 0 replies; 13+ messages in thread
From: Quentin Monnet @ 2020-02-26 12:33 UTC (permalink / raw)
To: Michal Rostecki, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Jakub Kicinski, netdev,
linux-kernel, Shuah Khan, linux-kselftest
2020-02-26 13:17 UTC+0100 ~ Michal Rostecki <mrostecki@opensuse.org>
[...]
> I
> might follow up with some more tests covering the other subcommands in
> future.
That would be great!
Thanks,
Quentin
^ permalink raw reply [flat|nested] 13+ messages in thread