* [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support
@ 2022-11-09 7:44 Sahid Orentino Ferdjaoui
2022-11-09 7:45 ` [PATCH bpf-next v2 1/5] bpftool: remove support of --legacy option for bpftool Sahid Orentino Ferdjaoui
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sahid Orentino Ferdjaoui @ 2022-11-09 7:44 UTC (permalink / raw)
To: bpf, ast, daniel, andrii, quentin
Cc: martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo,
jolsa, Sahid Orentino Ferdjaoui
As part of commit 93b8952d223a ("libbpf: deprecate legacy BPF map
definitions") and commit bd054102a8c7 ("libbpf: enforce strict libbpf
1.0 behaviors") The --legacy option is not relevant anymore. #1 is
removing it. #4 is cleaning the code from using libbpf_get_error().
About patches #2 and #3 They are changes discovered while working on
this series (credits to Quentin Monnet). #2 is cleaning-up usage of an
unnecessary PTR_ERR(NULL), finally #3 is fixing an invalid value
passed to strerror().
v1 -> v2:
- Addressed review comments from Yonghong Song on patch #4
- Added a patch #5 that removes unwanted function noticed by
Yonghong Song
Sahid Orentino Ferdjaoui (5):
bpftool: remove support of --legacy option for bpftool
bpftool: replace return value PTR_ERR(NULL) with 0
bpftool: fix error message when function can't register struct_ops
bpftool: clean-up usage of libbpf_get_error()
bpftool: remove function free_btf_vmlinux()
.../bpftool/Documentation/common_options.rst | 9 --------
.../bpftool/Documentation/substitutions.rst | 2 +-
tools/bpf/bpftool/bash-completion/bpftool | 2 +-
tools/bpf/bpftool/btf.c | 17 ++++++--------
tools/bpf/bpftool/btf_dumper.c | 2 +-
tools/bpf/bpftool/gen.c | 11 ++++------
tools/bpf/bpftool/iter.c | 6 ++---
tools/bpf/bpftool/main.c | 22 +++----------------
tools/bpf/bpftool/main.h | 3 +--
tools/bpf/bpftool/map.c | 20 ++++++-----------
tools/bpf/bpftool/prog.c | 15 +++++--------
tools/bpf/bpftool/struct_ops.c | 22 ++++++++-----------
.../selftests/bpf/test_bpftool_synctypes.py | 6 ++---
13 files changed, 44 insertions(+), 93 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH bpf-next v2 1/5] bpftool: remove support of --legacy option for bpftool 2022-11-09 7:44 [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support Sahid Orentino Ferdjaoui @ 2022-11-09 7:45 ` Sahid Orentino Ferdjaoui 2022-11-09 7:45 ` [PATCH bpf-next v2 2/5] bpftool: replace return value PTR_ERR(NULL) with 0 Sahid Orentino Ferdjaoui 2022-11-09 15:54 ` [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support Yonghong Song 2 siblings, 0 replies; 6+ messages in thread From: Sahid Orentino Ferdjaoui @ 2022-11-09 7:45 UTC (permalink / raw) To: bpf, ast, daniel, andrii, quentin Cc: martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, Sahid Orentino Ferdjaoui Following: commit bd054102a8c7 ("libbpf: enforce strict libbpf 1.0 behaviors") commit 93b8952d223a ("libbpf: deprecate legacy BPF map definitions") The --legacy option is no longer relevant as libbpf no longer supports it. libbpf_set_strict_mode() is a no-op operation. Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@industrialdiscipline.com> --- .../bpf/bpftool/Documentation/common_options.rst | 9 --------- tools/bpf/bpftool/Documentation/substitutions.rst | 2 +- tools/bpf/bpftool/bash-completion/bpftool | 2 +- tools/bpf/bpftool/main.c | 15 --------------- tools/bpf/bpftool/main.h | 3 +-- tools/bpf/bpftool/prog.c | 5 ----- .../selftests/bpf/test_bpftool_synctypes.py | 6 +++--- 7 files changed, 6 insertions(+), 36 deletions(-) diff --git a/tools/bpf/bpftool/Documentation/common_options.rst b/tools/bpf/bpftool/Documentation/common_options.rst index 05350a1aadf9..30df7a707f02 100644 --- a/tools/bpf/bpftool/Documentation/common_options.rst +++ b/tools/bpf/bpftool/Documentation/common_options.rst @@ -23,12 +23,3 @@ Print all logs available, even debug-level information. This includes logs from libbpf as well as from the verifier, when attempting to load programs. - --l, --legacy - Use legacy libbpf mode which has more relaxed BPF program - requirements. By default, bpftool has more strict requirements - about section names, changes pinning logic and doesn't support - some of the older non-BTF map declarations. - - See https://github.com/libbpf/libbpf/wiki/Libbpf:-the-road-to-v1.0 - for details. diff --git a/tools/bpf/bpftool/Documentation/substitutions.rst b/tools/bpf/bpftool/Documentation/substitutions.rst index ccf1ffa0686c..827e3ffb1766 100644 --- a/tools/bpf/bpftool/Documentation/substitutions.rst +++ b/tools/bpf/bpftool/Documentation/substitutions.rst @@ -1,3 +1,3 @@ .. SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) -.. |COMMON_OPTIONS| replace:: { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } | { **-l** | **--legacy** } +.. |COMMON_OPTIONS| replace:: { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-d** | **--debug** } diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool index 2957b42cab67..35f26f7c1124 100644 --- a/tools/bpf/bpftool/bash-completion/bpftool +++ b/tools/bpf/bpftool/bash-completion/bpftool @@ -261,7 +261,7 @@ _bpftool() # Deal with options if [[ ${words[cword]} == -* ]]; then local c='--version --json --pretty --bpffs --mapcompat --debug \ - --use-loader --base-btf --legacy' + --use-loader --base-btf' COMPREPLY=( $( compgen -W "$c" -- "$cur" ) ) return 0 fi diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index 741e50ee0b6c..87ceafa4b9b8 100644 --- a/tools/bpf/bpftool/main.c +++ b/tools/bpf/bpftool/main.c @@ -31,7 +31,6 @@ bool block_mount; bool verifier_logs; bool relaxed_maps; bool use_loader; -bool legacy_libbpf; struct btf *base_btf; struct hashmap *refs_table; @@ -160,7 +159,6 @@ static int do_version(int argc, char **argv) jsonw_start_object(json_wtr); /* features */ jsonw_bool_field(json_wtr, "libbfd", has_libbfd); jsonw_bool_field(json_wtr, "llvm", has_llvm); - jsonw_bool_field(json_wtr, "libbpf_strict", !legacy_libbpf); jsonw_bool_field(json_wtr, "skeletons", has_skeletons); jsonw_bool_field(json_wtr, "bootstrap", bootstrap); jsonw_end_object(json_wtr); /* features */ @@ -179,7 +177,6 @@ static int do_version(int argc, char **argv) printf("features:"); print_feature("libbfd", has_libbfd, &nb_features); print_feature("llvm", has_llvm, &nb_features); - print_feature("libbpf_strict", !legacy_libbpf, &nb_features); print_feature("skeletons", has_skeletons, &nb_features); print_feature("bootstrap", bootstrap, &nb_features); printf("\n"); @@ -451,7 +448,6 @@ int main(int argc, char **argv) { "debug", no_argument, NULL, 'd' }, { "use-loader", no_argument, NULL, 'L' }, { "base-btf", required_argument, NULL, 'B' }, - { "legacy", no_argument, NULL, 'l' }, { 0 } }; bool version_requested = false; @@ -524,9 +520,6 @@ int main(int argc, char **argv) case 'L': use_loader = true; break; - case 'l': - legacy_libbpf = true; - break; default: p_err("unrecognized option '%s'", argv[optind - 1]); if (json_output) @@ -536,14 +529,6 @@ int main(int argc, char **argv) } } - if (!legacy_libbpf) { - /* Allow legacy map definitions for skeleton generation. - * It will still be rejected if users use LIBBPF_STRICT_ALL - * mode for loading generated skeleton. - */ - libbpf_set_strict_mode(LIBBPF_STRICT_ALL & ~LIBBPF_STRICT_MAP_DEFINITIONS); - } - argc -= optind; argv += optind; if (argc < 0) diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h index 467d8472df0c..2d1f4d7211cd 100644 --- a/tools/bpf/bpftool/main.h +++ b/tools/bpf/bpftool/main.h @@ -57,7 +57,7 @@ static inline void *u64_to_ptr(__u64 ptr) #define HELP_SPEC_PROGRAM \ "PROG := { id PROG_ID | pinned FILE | tag PROG_TAG | name PROG_NAME }" #define HELP_SPEC_OPTIONS \ - "OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug} | {-l|--legacy}" + "OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug}" #define HELP_SPEC_MAP \ "MAP := { id MAP_ID | pinned FILE | name MAP_NAME }" #define HELP_SPEC_LINK \ @@ -82,7 +82,6 @@ extern bool block_mount; extern bool verifier_logs; extern bool relaxed_maps; extern bool use_loader; -extern bool legacy_libbpf; extern struct btf *base_btf; extern struct hashmap *refs_table; diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index a858b907da16..b6b62b3ef49b 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -1804,11 +1804,6 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) else bpf_object__unpin_programs(obj, pinfile); err_close_obj: - if (!legacy_libbpf) { - p_info("Warning: bpftool is now running in libbpf strict mode and has more stringent requirements about BPF programs.\n" - "If it used to work for this object file but now doesn't, see --legacy option for more details.\n"); - } - bpf_object__close(obj); err_free_reuse_maps: for (i = 0; i < old_map_fds; i++) diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py b/tools/testing/selftests/bpf/test_bpftool_synctypes.py index 9fe4c9336c6f..0cfece7ff4f8 100755 --- a/tools/testing/selftests/bpf/test_bpftool_synctypes.py +++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py @@ -309,11 +309,11 @@ class MainHeaderFileExtractor(SourceFileExtractor): 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}" + "OPTIONS := { {-j|--json} [{-p|--pretty}] | {-d|--debug}" Return a set containing all options, such as: - {'-p', '-d', '--legacy', '--pretty', '--debug', '--json', '-l', '-j'} + {'-p', '-d', '--pretty', '--debug', '--json', '-j'} """ start_marker = re.compile(f'"OPTIONS :=') pattern = re.compile('([\w-]+) ?(?:\||}[ }\]"])') @@ -336,7 +336,7 @@ class ManSubstitutionsExtractor(SourceFileExtractor): Return a set containing all options, such as: - {'-p', '-d', '--legacy', '--pretty', '--debug', '--json', '-l', '-j'} + {'-p', '-d', '--pretty', '--debug', '--json', '-j'} """ start_marker = re.compile('\|COMMON_OPTIONS\| replace:: {') pattern = re.compile('\*\*([\w/-]+)\*\*') -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next v2 2/5] bpftool: replace return value PTR_ERR(NULL) with 0 2022-11-09 7:44 [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support Sahid Orentino Ferdjaoui 2022-11-09 7:45 ` [PATCH bpf-next v2 1/5] bpftool: remove support of --legacy option for bpftool Sahid Orentino Ferdjaoui @ 2022-11-09 7:45 ` Sahid Orentino Ferdjaoui 2022-11-09 23:58 ` Andrii Nakryiko 2022-11-09 15:54 ` [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support Yonghong Song 2 siblings, 1 reply; 6+ messages in thread From: Sahid Orentino Ferdjaoui @ 2022-11-09 7:45 UTC (permalink / raw) To: bpf, ast, daniel, andrii, quentin Cc: martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa, Sahid Orentino Ferdjaoui There is no reasons to keep PTR_ERR() when kern_btf=NULL, let's just return 0. Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@industrialdiscipline.com> --- tools/bpf/bpftool/struct_ops.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/bpf/bpftool/struct_ops.c b/tools/bpf/bpftool/struct_ops.c index e08a6ff2866c..68281f9b7a0e 100644 --- a/tools/bpf/bpftool/struct_ops.c +++ b/tools/bpf/bpftool/struct_ops.c @@ -63,10 +63,8 @@ static __s32 get_map_info_type_id(void) return map_info_type_id; kern_btf = get_btf_vmlinux(); - if (libbpf_get_error(kern_btf)) { - map_info_type_id = PTR_ERR(kern_btf); - return map_info_type_id; - } + if (libbpf_get_error(kern_btf)) + return 0; map_info_type_id = btf__find_by_name_kind(kern_btf, "bpf_map_info", BTF_KIND_STRUCT); -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2 2/5] bpftool: replace return value PTR_ERR(NULL) with 0 2022-11-09 7:45 ` [PATCH bpf-next v2 2/5] bpftool: replace return value PTR_ERR(NULL) with 0 Sahid Orentino Ferdjaoui @ 2022-11-09 23:58 ` Andrii Nakryiko 0 siblings, 0 replies; 6+ messages in thread From: Andrii Nakryiko @ 2022-11-09 23:58 UTC (permalink / raw) To: Sahid Orentino Ferdjaoui Cc: bpf, ast, daniel, andrii, quentin, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa On Tue, Nov 8, 2022 at 11:45 PM Sahid Orentino Ferdjaoui <sahid.ferdjaoui@industrialdiscipline.com> wrote: > > There is no reasons to keep PTR_ERR() when kern_btf=NULL, let's just > return 0. > > Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@industrialdiscipline.com> > --- > tools/bpf/bpftool/struct_ops.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/tools/bpf/bpftool/struct_ops.c b/tools/bpf/bpftool/struct_ops.c > index e08a6ff2866c..68281f9b7a0e 100644 > --- a/tools/bpf/bpftool/struct_ops.c > +++ b/tools/bpf/bpftool/struct_ops.c > @@ -63,10 +63,8 @@ static __s32 get_map_info_type_id(void) > return map_info_type_id; > > kern_btf = get_btf_vmlinux(); > - if (libbpf_get_error(kern_btf)) { > - map_info_type_id = PTR_ERR(kern_btf); > - return map_info_type_id; > - } > + if (libbpf_get_error(kern_btf)) > + return 0; > if (!kern_btf) return 0; > map_info_type_id = btf__find_by_name_kind(kern_btf, "bpf_map_info", > BTF_KIND_STRUCT); > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support 2022-11-09 7:44 [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support Sahid Orentino Ferdjaoui 2022-11-09 7:45 ` [PATCH bpf-next v2 1/5] bpftool: remove support of --legacy option for bpftool Sahid Orentino Ferdjaoui 2022-11-09 7:45 ` [PATCH bpf-next v2 2/5] bpftool: replace return value PTR_ERR(NULL) with 0 Sahid Orentino Ferdjaoui @ 2022-11-09 15:54 ` Yonghong Song 2022-11-13 8:43 ` Sahid Orentino Ferdjaoui 2 siblings, 1 reply; 6+ messages in thread From: Yonghong Song @ 2022-11-09 15:54 UTC (permalink / raw) To: Sahid Orentino Ferdjaoui, bpf, ast, daniel, andrii, quentin Cc: martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa On 11/8/22 11:44 PM, Sahid Orentino Ferdjaoui wrote: > As part of commit 93b8952d223a ("libbpf: deprecate legacy BPF map > definitions") and commit bd054102a8c7 ("libbpf: enforce strict libbpf > 1.0 behaviors") The --legacy option is not relevant anymore. #1 is > removing it. #4 is cleaning the code from using libbpf_get_error(). > > About patches #2 and #3 They are changes discovered while working on > this series (credits to Quentin Monnet). #2 is cleaning-up usage of an > unnecessary PTR_ERR(NULL), finally #3 is fixing an invalid value > passed to strerror(). > > v1 -> v2: > - Addressed review comments from Yonghong Song on patch #4 > - Added a patch #5 that removes unwanted function noticed by > Yonghong Song > > Sahid Orentino Ferdjaoui (5): > bpftool: remove support of --legacy option for bpftool > bpftool: replace return value PTR_ERR(NULL) with 0 > bpftool: fix error message when function can't register struct_ops > bpftool: clean-up usage of libbpf_get_error() > bpftool: remove function free_btf_vmlinux() > > .../bpftool/Documentation/common_options.rst | 9 -------- > .../bpftool/Documentation/substitutions.rst | 2 +- > tools/bpf/bpftool/bash-completion/bpftool | 2 +- > tools/bpf/bpftool/btf.c | 17 ++++++-------- > tools/bpf/bpftool/btf_dumper.c | 2 +- > tools/bpf/bpftool/gen.c | 11 ++++------ > tools/bpf/bpftool/iter.c | 6 ++--- > tools/bpf/bpftool/main.c | 22 +++---------------- > tools/bpf/bpftool/main.h | 3 +-- > tools/bpf/bpftool/map.c | 20 ++++++----------- > tools/bpf/bpftool/prog.c | 15 +++++-------- > tools/bpf/bpftool/struct_ops.c | 22 ++++++++----------- > .../selftests/bpf/test_bpftool_synctypes.py | 6 ++--- > 13 files changed, 44 insertions(+), 93 deletions(-) > > -- > 2.34.1 > You can carry the 'Ack' if no significant change for each patch. Ack for the whole series. Acked-by: Yonghong Song <yhs@fb.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support 2022-11-09 15:54 ` [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support Yonghong Song @ 2022-11-13 8:43 ` Sahid Orentino Ferdjaoui 0 siblings, 0 replies; 6+ messages in thread From: Sahid Orentino Ferdjaoui @ 2022-11-13 8:43 UTC (permalink / raw) To: Yonghong Song Cc: bpf, ast, daniel, andrii, quentin, martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo, jolsa On 11/09/ , Yonghong Song wrote: > > > On 11/8/22 11:44 PM, Sahid Orentino Ferdjaoui wrote: > > As part of commit 93b8952d223a ("libbpf: deprecate legacy BPF map > > definitions") and commit bd054102a8c7 ("libbpf: enforce strict libbpf > > 1.0 behaviors") The --legacy option is not relevant anymore. #1 is > > removing it. #4 is cleaning the code from using libbpf_get_error(). > > > > About patches #2 and #3 They are changes discovered while working on > > this series (credits to Quentin Monnet). #2 is cleaning-up usage of an > > unnecessary PTR_ERR(NULL), finally #3 is fixing an invalid value > > passed to strerror(). > > > > v1 -> v2: > > - Addressed review comments from Yonghong Song on patch #4 > > - Added a patch #5 that removes unwanted function noticed by > > Yonghong Song > > > > Sahid Orentino Ferdjaoui (5): > > bpftool: remove support of --legacy option for bpftool > > bpftool: replace return value PTR_ERR(NULL) with 0 > > bpftool: fix error message when function can't register struct_ops > > bpftool: clean-up usage of libbpf_get_error() > > bpftool: remove function free_btf_vmlinux() > > > > .../bpftool/Documentation/common_options.rst | 9 -------- > > .../bpftool/Documentation/substitutions.rst | 2 +- > > tools/bpf/bpftool/bash-completion/bpftool | 2 +- > > tools/bpf/bpftool/btf.c | 17 ++++++-------- > > tools/bpf/bpftool/btf_dumper.c | 2 +- > > tools/bpf/bpftool/gen.c | 11 ++++------ > > tools/bpf/bpftool/iter.c | 6 ++--- > > tools/bpf/bpftool/main.c | 22 +++---------------- > > tools/bpf/bpftool/main.h | 3 +-- > > tools/bpf/bpftool/map.c | 20 ++++++----------- > > tools/bpf/bpftool/prog.c | 15 +++++-------- > > tools/bpf/bpftool/struct_ops.c | 22 ++++++++----------- > > .../selftests/bpf/test_bpftool_synctypes.py | 6 ++--- > > 13 files changed, 44 insertions(+), 93 deletions(-) > > > > -- > > 2.34.1 > > > > You can carry the 'Ack' if no significant change for > each patch. Thanks a lot for this advice. I will use this in my v3 when addressing Andrii comments. > Ack for the whole series. > > Acked-by: Yonghong Song <yhs@fb.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-13 8:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-09 7:44 [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support Sahid Orentino Ferdjaoui 2022-11-09 7:45 ` [PATCH bpf-next v2 1/5] bpftool: remove support of --legacy option for bpftool Sahid Orentino Ferdjaoui 2022-11-09 7:45 ` [PATCH bpf-next v2 2/5] bpftool: replace return value PTR_ERR(NULL) with 0 Sahid Orentino Ferdjaoui 2022-11-09 23:58 ` Andrii Nakryiko 2022-11-09 15:54 ` [PATCH bpf-next v2 0/5] clean-up bpftool from legacy support Yonghong Song 2022-11-13 8:43 ` Sahid Orentino Ferdjaoui
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox