Re: [PATCH bpf-next v4 2/3] bpftool: conditionally append / to the progtype
From: Quentin Monnet <hidden>
Date: 2021-10-21 19:55:58
Also in:
bpf
2021-10-21 09:56 UTC-0700 ~ Stanislav Fomichev [off-list ref]
quoted hunk ↗ jump to hunk
Otherwise, attaching with bpftool doesn't work with strict section names. Also, switch to libbpf strict mode to use the latest conventions (note, I don't think we have any cli api guarantees?). Cc: Quentin Monnet <redacted> Signed-off-by: Stanislav Fomichev <redacted> --- tools/bpf/bpftool/main.c | 4 ++++ tools/bpf/bpftool/prog.c | 9 +++++++-- 2 files changed, 11 insertions(+), 2 deletions(-)diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index 02eaaf065f65..8223bac1e401 100644 --- a/tools/bpf/bpftool/main.c +++ b/tools/bpf/bpftool/main.c@@ -409,6 +409,10 @@ int main(int argc, char **argv) block_mount = false; bin_name = argv[0]; + ret = libbpf_set_strict_mode(LIBBPF_STRICT_ALL); + if (ret) + p_err("failed to enable libbpf strict mode: %d", ret); + hash_init(prog_table.table); hash_init(map_table.table); hash_init(link_table.table);diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index 277d51c4c5d9..b04990588ccf 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c@@ -1420,8 +1420,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only) err = get_prog_type_by_name(type, &common_prog_type, &expected_attach_type); free(type); - if (err < 0) - goto err_free_reuse_maps;
Thanks a lot for the change! Can you please test it for e.g. an XDP program? You should see that "bpftool prog load prog.o <path> type xdp" prints a debug message from libbpf about the first attempt (above) failing, before the second attempt (below) succeeds. We need to get rid of this message. I think it should be easy, because we explicitly "ask" for that message in get_prog_type_by_name(), in the same file, if it fails to load in the first place. Could you please update get_prog_type_by_name() to take an additional switch as an argument, to tell if the debug-info should be retrieved (then first attempt here would skip it, second would keep it)? An alternative could be to move all the '/' and retries handling to that function, and I think it would end up in bpftool keeping support for the legacy object files with the former convention - but that would somewhat defeat the objectives of the strict mode, so maybe not the best option.
+ if (err < 0) {We could run the second attempt only on libbpf returning -ESRCH, maybe?
+ err = get_prog_type_by_name(*argv, &common_prog_type,
+ &expected_attach_type);
+ if (err < 0)
+
+ goto err_free_reuse_maps;
+ }
NEXT_ARG();
} else if (is_prefix(*argv, "map")) {