On 24/03/2025 18:47, Ihor Solodrai wrote: > On 3/24/25 11:07 AM, Ihor Solodrai wrote: >> 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; > > Actually, I added this check in a different patch so the failure must > have happened in a different place. > > In any case, the point remains that it's better to check for feature > availability (hence for API availability) in one place. Your > suggestion to add a feature check makes sense to me. Thank you. > Great; so let's do this to land the series. Could you either - check I merged your patches correctly in the above branch, and if they look good I'll merge them into next and I'll officially send the feature check patch; or if you'd prefer - send a v5 (perhaps including my feature check patch?) ...whichever approach is easiest for you. Thanks! Alan
