On 20/03/2025 20:34, Alan Maguire wrote: > On 20/03/2025 16:32, Ihor Solodrai wrote: >> On 2/28/25 11:46 AM, Ihor Solodrai wrote: >>> This patch series implements emitting appropriate BTF type tags for >>> argument and return types of kfuncs marked with KF_ARENA_* flags. >>> >>> For additional context see the description of BPF patch >>> "bpf: define KF_ARENA_* flags for bpf_arena kfuncs" [1]. >>> >>> The feature depends on recent changes in libbpf [2]. >>> >>> [1] https://lore.kernel.org/bpf/20250206003148.2308659-1-ihor.solodrai@xxxxxxxxx/ >>> [2] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@xxxxxxxxx/ >>> >>> v3->v4: >>> * Add a patch (#2) replacing compile-time libbpf version checks with >>> runtime checks for symbol availablility >>> * Add a patch (#3) bumping libbpf submodule commit to latest master >>> * Modify "btf_encoder: emit type tags for bpf_arena pointers" >>> (#2->#4) to not use compile time libbpf version checks >>> >>> v2->v3: >>> * Nits in patch #1 >>> >>> v1->v2: >>> * Rewrite patch #1 refactoring btf_encoder__tag_kfuncs(): now the >>> post-processing step is removed entirely, and kfuncs are tagged in >>> btf_encoder__add_func(). >>> * Nits and renames in patch #2 >>> * Add patch #4 editing man pages >>> >>> v2: https://lore.kernel.org/dwarves/20250212201552.1431219-1-ihor.solodrai@xxxxxxxxx/ >>> v1: https://lore.kernel.org/dwarves/20250207021442.155703-1-ihor.solodrai@xxxxxxxxx/ >>> >>> Ihor Solodrai (6): >>> btf_encoder: refactor btf_encoder__tag_kfuncs() >>> btf_encoder: use __weak declarations of version-dependent libbpf API >>> pahole: sync with libbpf mainline >>> btf_encoder: emit type tags for bpf_arena pointers >>> pahole: introduce --btf_feature=attributes >>> man-pages: describe attributes and remove reproducible_build >> >> Hi Alan, Arnaldo. >> >> This series hasn't received any comments in a while. >> Do you plan to review/land this? >> > > Yep, thanks for the reminder; I hit a wall last time I looked a this > when testing with a shared library libbpf versus embedded but I can get > around that now so I should have the testing done for both modes tomorrow. > hi Ihor, I took a look at the series and merged it with latest next branch; results are in https://web.git.kernel.org/pub/scm/devel/pahole/pahole.git/log/?h=next.attributes-v4 ...if you want to take a look. There are a few small things I think that it would be good to resolve before landing this. First, when testing this with -DLIBBPF_EMBEDDED=OFF and a packaged libbpf 1.5 - which means we wouldn't have the latest attributes-related libbpf function; I saw: BTF .tmp_vmlinux1.btf.o btf__add_type_attr is not available, is libbpf < 1.6? error: failed to encode function 'bbr_cwnd_event': invalid proto Failed to encode BTF NM .tmp_vmlinux1.syms ...and we got no BTF as a result. Ideally we'd like pahole to encode but without the attributes feature if not available. Related, we also report features that are not present, i.e. attributes with "--supported_btf_features". So I propose that we make use of the weak declarations being NULL in an optional feature check function. It is optionally declared for a feature, and if declared must return true if the feature is available. Something like the below works (it's in the next.attributes-v4 branch too for reference) and it resolves the issue of BTF generation failure and accurate supported_btf_features reporting. What do you think? Thanks! Alan From: Alan Maguire <alan.maguire@xxxxxxxxxx> Date: Sun, 23 Mar 2025 11:06:18 +0000 Subject: [PATCH] pahole: add a BTF feature check function It is used to see if functions that BTF features rely on are really there; weak declarations mean they will be NULL if not in non-embedded linked libbpf. This gives us more accurate --supported_btf_features reporting also. Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx> --- pahole.c | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/pahole.c b/pahole.c index 4a2b1ce..8304ba4 100644 --- a/pahole.c +++ b/pahole.c @@ -1183,10 +1183,31 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; * floats, etc. This ensures backwards compatibility. */ #define BTF_DEFAULT_FEATURE(name, alias, initial_value) \ - { #name, #alias, &conf_load.alias, initial_value, true } + { #name, #alias, &conf_load.alias, initial_value, true, NULL } + +#define BTF_DEFAULT_FEATURE_CHECK(name, alias, initial_value, feature_check) \ + { #name, #alias, &conf_load.alias, initial_value, true, feature_check } #define BTF_NON_DEFAULT_FEATURE(name, alias, initial_value) \ - { #name, #alias, &conf_load.alias, initial_value, false } + { #name, #alias, &conf_load.alias, initial_value, false, NULL } + +#define BTF_NON_DEFAULT_FEATURE_CHECK(name, alias, initial_value, feature_check) \ + { #name, #alias, &conf_load.alias, initial_value, false, feature_check } + +static bool enum64_check(void) +{ + return btf__add_enum64 != NULL; +} + +static bool distilled_base_check(void) +{ + return btf__distill_base != NULL; +} + +static bool attributes_check(void) +{ + return btf__add_type_attr != NULL; +} struct btf_feature { const char *name; @@ -1196,20 +1217,23 @@ struct btf_feature { bool default_enabled; /* some nonstandard features may not * be enabled for --btf_features=default */ + bool (*feature_check)(void); } btf_features[] = { BTF_DEFAULT_FEATURE(encode_force, btf_encode_force, false), BTF_DEFAULT_FEATURE(var, skip_encoding_btf_vars, true), BTF_DEFAULT_FEATURE(float, btf_gen_floats, false), BTF_DEFAULT_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true), BTF_DEFAULT_FEATURE(type_tag, skip_encoding_btf_type_tag, true), - BTF_DEFAULT_FEATURE(enum64, skip_encoding_btf_enum64, true), + BTF_DEFAULT_FEATURE_CHECK(enum64, skip_encoding_btf_enum64, true, enum64_check), BTF_DEFAULT_FEATURE(optimized_func, btf_gen_optimized, false), BTF_DEFAULT_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false), BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false), BTF_NON_DEFAULT_FEATURE(reproducible_build, reproducible_build, false), - BTF_NON_DEFAULT_FEATURE(distilled_base, btf_gen_distilled_base, false), + BTF_NON_DEFAULT_FEATURE_CHECK(distilled_base, btf_gen_distilled_base, false, + distilled_base_check), BTF_NON_DEFAULT_FEATURE(global_var, encode_btf_global_vars, false), - BTF_NON_DEFAULT_FEATURE(attributes, btf_attributes, false), + BTF_NON_DEFAULT_FEATURE_CHECK(attributes, btf_attributes, false, + attributes_check), }; #define BTF_MAX_FEATURE_STR 1024 @@ -1248,7 +1272,8 @@ static void enable_btf_feature(struct btf_feature *feature) /* switch "initial-off" features on, and "initial-on" features * off; i.e. negate the initial value. */ - *feature->conf_value = !feature->initial_value; + if (!feature->feature_check || feature->feature_check()) + *feature->conf_value = !feature->initial_value; } static void show_supported_btf_features(FILE *output) @@ -1256,6 +1281,8 @@ static void show_supported_btf_features(FILE *output) int i; for (i = 0; i < ARRAY_SIZE(btf_features); i++) { + if (btf_features[i].feature_check && !btf_features[i].feature_check()) + continue; if (i > 0) fprintf(output, ","); fprintf(output, "%s", btf_features[i].name); -- 2.39.3
