diff options
author | Alexei Starovoitov <ast@kernel.org> | 2022-11-21 03:17:46 +0300 |
---|---|---|
committer | Alexei Starovoitov <ast@kernel.org> | 2022-11-21 03:18:25 +0300 |
commit | 35ffb1d9bff01cf3e2a55fcc8ab001cbb087c9cb (patch) | |
tree | 485001b6e4ef11db75fb1a4bd2e896bbb8742fe9 | |
parent | 99429b224f6107f691c365ef9bdc1bbe4ee43ede (diff) | |
parent | 52df1a8aabadeba1e4c2fe157784637ddec76301 (diff) | |
download | linux-35ffb1d9bff01cf3e2a55fcc8ab001cbb087c9cb.tar.xz |
Merge branch 'clean-up bpftool from legacy support'
Sahid Orentino Ferdjaoui says:
====================
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
v2 -> v3
- Addressed review comments from Andrii Nakryiko on patch #2, #3, #4
* clean-up usage of libbpf_get_error() (#2, #3)
* fix possible return of an uninitialized local variable err
* fix returned errors using errno
v3 -> v4
- Addressed review comments from Quentin Monnet
* fix line moved from patch #2 to patch #3
* fix missing returned errors using errno
* fix some returned values to errno instead of -1
====================
Reviewed-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
-rw-r--r-- | tools/bpf/bpftool/Documentation/common_options.rst | 9 | ||||
-rw-r--r-- | tools/bpf/bpftool/Documentation/substitutions.rst | 2 | ||||
-rw-r--r-- | tools/bpf/bpftool/bash-completion/bpftool | 2 | ||||
-rw-r--r-- | tools/bpf/bpftool/btf.c | 19 | ||||
-rw-r--r-- | tools/bpf/bpftool/btf_dumper.c | 2 | ||||
-rw-r--r-- | tools/bpf/bpftool/gen.c | 10 | ||||
-rw-r--r-- | tools/bpf/bpftool/iter.c | 10 | ||||
-rw-r--r-- | tools/bpf/bpftool/main.c | 22 | ||||
-rw-r--r-- | tools/bpf/bpftool/main.h | 3 | ||||
-rw-r--r-- | tools/bpf/bpftool/map.c | 20 | ||||
-rw-r--r-- | tools/bpf/bpftool/prog.c | 15 | ||||
-rw-r--r-- | tools/bpf/bpftool/struct_ops.c | 22 | ||||
-rwxr-xr-x | tools/testing/selftests/bpf/test_bpftool_synctypes.py | 6 |
13 files changed, 49 insertions, 93 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/btf.c b/tools/bpf/bpftool/btf.c index b87e4a7fd689..352290ba7b29 100644 --- a/tools/bpf/bpftool/btf.c +++ b/tools/bpf/bpftool/btf.c @@ -467,9 +467,8 @@ static int dump_btf_c(const struct btf *btf, int err = 0, i; d = btf_dump__new(btf, btf_dump_printf, NULL, NULL); - err = libbpf_get_error(d); - if (err) - return err; + if (!d) + return -errno; printf("#ifndef __VMLINUX_H__\n"); printf("#define __VMLINUX_H__\n"); @@ -512,11 +511,9 @@ static struct btf *get_vmlinux_btf_from_sysfs(void) struct btf *base; base = btf__parse(sysfs_vmlinux, NULL); - if (libbpf_get_error(base)) { - p_err("failed to parse vmlinux BTF at '%s': %ld\n", - sysfs_vmlinux, libbpf_get_error(base)); - base = NULL; - } + if (!base) + p_err("failed to parse vmlinux BTF at '%s': %d\n", + sysfs_vmlinux, -errno); return base; } @@ -559,7 +556,7 @@ static int do_dump(int argc, char **argv) __u32 btf_id = -1; const char *src; int fd = -1; - int err; + int err = 0; if (!REQ_ARGS(2)) { usage(); @@ -634,8 +631,8 @@ static int do_dump(int argc, char **argv) base = get_vmlinux_btf_from_sysfs(); btf = btf__parse_split(*argv, base ?: base_btf); - err = libbpf_get_error(btf); if (!btf) { + err = -errno; p_err("failed to load BTF from %s: %s", *argv, strerror(errno)); goto done; @@ -681,8 +678,8 @@ static int do_dump(int argc, char **argv) } btf = btf__load_from_kernel_by_id_split(btf_id, base_btf); - err = libbpf_get_error(btf); if (!btf) { + err = -errno; p_err("get btf by id (%u): %s", btf_id, strerror(errno)); goto done; } diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c index 19924b6ce796..eda71fdfe95a 100644 --- a/tools/bpf/bpftool/btf_dumper.c +++ b/tools/bpf/bpftool/btf_dumper.c @@ -75,7 +75,7 @@ static int dump_prog_id_as_func_ptr(const struct btf_dumper *d, goto print; prog_btf = btf__load_from_kernel_by_id(info.btf_id); - if (libbpf_get_error(prog_btf)) + if (!prog_btf) goto print; func_type = btf__type_by_id(prog_btf, finfo.type_id); if (!func_type || !btf_is_func(func_type)) diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c index 01bb8d8f5568..2883660d6b67 100644 --- a/tools/bpf/bpftool/gen.c +++ b/tools/bpf/bpftool/gen.c @@ -252,9 +252,8 @@ static int codegen_datasecs(struct bpf_object *obj, const char *obj_name) int err = 0; d = btf_dump__new(btf, codegen_btf_dump_printf, NULL, NULL); - err = libbpf_get_error(d); - if (err) - return err; + if (!d) + return -errno; bpf_object__for_each_map(map, obj) { /* only generate definitions for memory-mapped internal maps */ @@ -976,13 +975,12 @@ static int do_skeleton(int argc, char **argv) /* log_level1 + log_level2 + stats, but not stable UAPI */ opts.kernel_log_level = 1 + 2 + 4; obj = bpf_object__open_mem(obj_data, file_sz, &opts); - err = libbpf_get_error(obj); - if (err) { + if (!obj) { char err_buf[256]; + err = -errno; libbpf_strerror(err, err_buf, sizeof(err_buf)); p_err("failed to open BPF object file: %s", err_buf); - obj = NULL; goto out; } diff --git a/tools/bpf/bpftool/iter.c b/tools/bpf/bpftool/iter.c index a3e6b167153d..9a1d2365a297 100644 --- a/tools/bpf/bpftool/iter.c +++ b/tools/bpf/bpftool/iter.c @@ -4,6 +4,7 @@ #ifndef _GNU_SOURCE #define _GNU_SOURCE #endif +#include <errno.h> #include <unistd.h> #include <linux/err.h> #include <bpf/libbpf.h> @@ -48,8 +49,8 @@ static int do_pin(int argc, char **argv) } obj = bpf_object__open(objfile); - err = libbpf_get_error(obj); - if (err) { + if (!obj) { + err = -errno; p_err("can't open objfile %s", objfile); goto close_map_fd; } @@ -62,13 +63,14 @@ static int do_pin(int argc, char **argv) prog = bpf_object__next_program(obj, NULL); if (!prog) { + err = -errno; p_err("can't find bpf program in objfile %s", objfile); goto close_obj; } link = bpf_program__attach_iter(prog, &iter_opts); - err = libbpf_get_error(link); - if (err) { + if (!link) { + err = -errno; p_err("attach_iter failed for program %s", bpf_program__name(prog)); goto close_obj; diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index 337ab7977ea4..08d0ac543c67 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; @@ -514,19 +510,15 @@ int main(int argc, char **argv) break; case 'B': base_btf = btf__parse(optarg, NULL); - if (libbpf_get_error(base_btf)) { - p_err("failed to parse base BTF at '%s': %ld\n", - optarg, libbpf_get_error(base_btf)); - base_btf = NULL; + if (!base_btf) { + p_err("failed to parse base BTF at '%s': %d\n", + optarg, -errno); return -1; } break; 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 +528,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 d4e8a1aef787..a84224b6a604 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/map.c b/tools/bpf/bpftool/map.c index d884070a2314..88911d3aa2d9 100644 --- a/tools/bpf/bpftool/map.c +++ b/tools/bpf/bpftool/map.c @@ -786,18 +786,18 @@ static int get_map_kv_btf(const struct bpf_map_info *info, struct btf **btf) if (info->btf_vmlinux_value_type_id) { if (!btf_vmlinux) { btf_vmlinux = libbpf_find_kernel_btf(); - err = libbpf_get_error(btf_vmlinux); - if (err) { + if (!btf_vmlinux) { p_err("failed to get kernel btf"); - return err; + return -errno; } } *btf = btf_vmlinux; } else if (info->btf_value_type_id) { *btf = btf__load_from_kernel_by_id(info->btf_id); - err = libbpf_get_error(*btf); - if (err) + if (!*btf) { + err = -errno; p_err("failed to get btf"); + } } else { *btf = NULL; } @@ -807,16 +807,10 @@ static int get_map_kv_btf(const struct bpf_map_info *info, struct btf **btf) static void free_map_kv_btf(struct btf *btf) { - if (!libbpf_get_error(btf) && btf != btf_vmlinux) + if (btf != btf_vmlinux) btf__free(btf); } -static void free_btf_vmlinux(void) -{ - if (!libbpf_get_error(btf_vmlinux)) - btf__free(btf_vmlinux); -} - static int map_dump(int fd, struct bpf_map_info *info, json_writer_t *wtr, bool show_header) @@ -953,7 +947,7 @@ exit_close: close(fds[i]); exit_free: free(fds); - free_btf_vmlinux(); + btf__free(btf_vmlinux); return err; } diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index 9d32ffb9f22e..cfc9fdc1e863 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -322,7 +322,7 @@ static void show_prog_metadata(int fd, __u32 num_maps) return; btf = btf__load_from_kernel_by_id(map_info.btf_id); - if (libbpf_get_error(btf)) + if (!btf) goto out_free; t_datasec = btf__type_by_id(btf, map_info.btf_value_type_id); @@ -726,7 +726,7 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode, if (info->btf_id) { btf = btf__load_from_kernel_by_id(info->btf_id); - if (libbpf_get_error(btf)) { + if (!btf) { p_err("failed to get btf"); return -1; } @@ -1663,7 +1663,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) open_opts.kernel_log_level = 1 + 2 + 4; obj = bpf_object__open_file(file, &open_opts); - if (libbpf_get_error(obj)) { + if (!obj) { p_err("failed to open object file"); goto err_free_reuse_maps; } @@ -1802,11 +1802,6 @@ err_unpin: 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++) @@ -1887,7 +1882,7 @@ static int do_loader(int argc, char **argv) open_opts.kernel_log_level = 1 + 2 + 4; obj = bpf_object__open_file(file, &open_opts); - if (libbpf_get_error(obj)) { + if (!obj) { p_err("failed to open object file"); goto err_close_obj; } @@ -2204,7 +2199,7 @@ static char *profile_target_name(int tgt_fd) } btf = btf__load_from_kernel_by_id(info.btf_id); - if (libbpf_get_error(btf)) { + if (!btf) { p_err("failed to load btf for prog FD %d", tgt_fd); goto out; } diff --git a/tools/bpf/bpftool/struct_ops.c b/tools/bpf/bpftool/struct_ops.c index e08a6ff2866c..903b80ff4e9a 100644 --- a/tools/bpf/bpftool/struct_ops.c +++ b/tools/bpf/bpftool/struct_ops.c @@ -32,7 +32,7 @@ static const struct btf *get_btf_vmlinux(void) return btf_vmlinux; btf_vmlinux = libbpf_find_kernel_btf(); - if (libbpf_get_error(btf_vmlinux)) + if (!btf_vmlinux) p_err("struct_ops requires kernel CONFIG_DEBUG_INFO_BTF=y"); return btf_vmlinux; @@ -45,7 +45,7 @@ static const char *get_kern_struct_ops_name(const struct bpf_map_info *info) const char *st_ops_name; kern_btf = get_btf_vmlinux(); - if (libbpf_get_error(kern_btf)) + if (!kern_btf) return "<btf_vmlinux_not_found>"; t = btf__type_by_id(kern_btf, info->btf_vmlinux_value_type_id); @@ -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 (!kern_btf) + return 0; map_info_type_id = btf__find_by_name_kind(kern_btf, "bpf_map_info", BTF_KIND_STRUCT); @@ -415,7 +413,7 @@ static int do_dump(int argc, char **argv) } kern_btf = get_btf_vmlinux(); - if (libbpf_get_error(kern_btf)) + if (!kern_btf) return -1; if (!json_output) { @@ -498,7 +496,7 @@ static int do_register(int argc, char **argv) open_opts.kernel_log_level = 1 + 2 + 4; obj = bpf_object__open_file(file, &open_opts); - if (libbpf_get_error(obj)) + if (!obj) return -1; set_max_rlimit(); @@ -513,10 +511,9 @@ static int do_register(int argc, char **argv) continue; link = bpf_map__attach_struct_ops(map); - if (libbpf_get_error(link)) { + if (!link) { p_err("can't register struct_ops %s: %s", - bpf_map__name(map), - strerror(-PTR_ERR(link))); + bpf_map__name(map), strerror(errno)); nr_errs++; continue; } @@ -593,8 +590,7 @@ int do_struct_ops(int argc, char **argv) err = cmd_select(cmds, argc, argv, do_help); - if (!libbpf_get_error(btf_vmlinux)) - btf__free(btf_vmlinux); + btf__free(btf_vmlinux); return err; } 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/-]+)\*\*') |