Re: [PATCH dwarves v4 0/6] btf_encoder: emit type tags for bpf_arena pointers

Alan Maguire
2025-03-23T11:12:03Z

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@linux.dev/
>>> [2] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/
>>>
>>> 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@linux.dev/
>>> v1: https://lore.kernel.org/dwarves/20250207021442.155703-1-ihor.solodrai@linux.dev/
>>>
>>> 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@oracle.com>
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@oracle.com>
---
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