From: "Ihor Solodrai" <ihor.solodrai@linux.dev>
To: "Alan Maguire" <alan.maguire@oracle.com>,
dwarves@vger.kernel.org, bpf@vger.kernel.org
Cc: acme@kernel.org, ast@kernel.org, andrii@kernel.org,
eddyz87@gmail.com, mykolal@fb.com, kernel-team@meta.com
Subject: Re: [PATCH dwarves v4 0/6] btf_encoder: emit type tags for bpf_arena pointers
Date: Mon, 24 Mar 2025 18:07:06 +0000 [thread overview]
Message-ID: <68a594e38c00ff3dd30d0a13fb1e1de71f19954c@linux.dev> (raw)
In-Reply-To: <b1a23727-098e-473b-8282-8fb0cbf97603@oracle.com>
On 3/23/25 4:11 AM, Alan Maguire wrote:
> [...]
>
> 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
Hi Alan. Thanks for testing. This is my mistake, I should've checked
for attributes feature here:
@@ -731,6 +812,10 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f
assert(ftype != NULL || state != NULL);
+ if (is_kfunc_state(state) && encoder->tag_kfuncs)
+ if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
+ return -1;
>
> ...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!
I think failing fast is a good approach here: check that all
requested/default features are available on startup.
>
> 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(-)
>
Acked-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> 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);
next prev parent reply other threads:[~2025-03-24 18:07 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-28 19:46 [PATCH dwarves v4 0/6] btf_encoder: emit type tags for bpf_arena pointers Ihor Solodrai
2025-02-28 19:46 ` [PATCH dwarves v4 1/6] btf_encoder: refactor btf_encoder__tag_kfuncs() Ihor Solodrai
2025-02-28 19:46 ` [PATCH dwarves v4 2/6] btf_encoder: use __weak declarations of version-dependent libbpf API Ihor Solodrai
2025-02-28 19:53 ` Ihor Solodrai
2025-03-06 17:14 ` Alan Maguire
2025-02-28 19:46 ` [PATCH dwarves v4 3/6] pahole: sync with libbpf mainline Ihor Solodrai
2025-02-28 19:46 ` [PATCH dwarves v4 4/6] btf_encoder: emit type tags for bpf_arena pointers Ihor Solodrai
2025-02-28 19:46 ` [PATCH dwarves v4 5/6] pahole: introduce --btf_feature=attributes Ihor Solodrai
2025-02-28 19:46 ` [PATCH dwarves v4 6/6] man-pages: describe attributes and remove reproducible_build Ihor Solodrai
2025-03-20 16:32 ` [PATCH dwarves v4 0/6] btf_encoder: emit type tags for bpf_arena pointers Ihor Solodrai
2025-03-20 20:34 ` Alan Maguire
2025-03-23 11:11 ` Alan Maguire
2025-03-24 18:07 ` Ihor Solodrai [this message]
2025-03-24 18:47 ` Ihor Solodrai
2025-03-25 9:59 ` Alan Maguire
2025-03-26 17:41 ` Ihor Solodrai
2025-03-27 8:22 ` Alan Maguire
2025-03-27 15:33 ` Ihor Solodrai
2025-03-27 15:40 ` Alan Maguire
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=68a594e38c00ff3dd30d0a13fb1e1de71f19954c@linux.dev \
--to=ihor.solodrai@linux.dev \
--cc=acme@kernel.org \
--cc=alan.maguire@oracle.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=dwarves@vger.kernel.org \
--cc=eddyz87@gmail.com \
--cc=kernel-team@meta.com \
--cc=mykolal@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox