On 17/09/2024 05:40, Eduard Zingerman wrote:
> On Mon, 2024-09-16 at 11:16 +0100, Alan Maguire wrote:
>
> [...]
>
>> hi Eduard,
>>
>> you've added support for multiple declaration tags as part of this, but
>> I wonder if we could go slightly further to simplify any additional
>> future KF_* flags -> decl tag needs?
>>
>> Specifically if we had an array of <set8 flags, tag name> mappings such
>> that we can add support for new declaration tags by simply adding a new
>> flag and declaration tag string. When checking flags value in
>> btf_encoder__tag_kfunc(), we'd just walk the array entries, and for each
>> matching flag add the associated decl tag. Would that work?
>
> Hi Alan,
>
> That would be something like below.
> It works, but looks a bit over-complicated for my taste, wdyt?
yeah, I guess I was thinking the kfunc flag would be checked along with
KF_FASTCALL, but the code already knows it's a kfunc, so no need. We
can add something like this if/when other flags are added.
Sorry for the noise!
Reviewed-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
>
> --- 8< ----------------------------------------
> iff --git a/btf_encoder.c b/btf_encoder.c
> index ae059e0..b6178c3 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -39,7 +39,6 @@
> #define BTF_ID_SET8_PFX "__BTF_ID__set8__"
> #define BTF_SET8_KFUNCS (1 << 0)
> #define BTF_KFUNC_TYPE_TAG "bpf_kfunc"
> -#define BTF_FASTCALL_TAG "bpf_fastcall"
> #define KF_FASTCALL (1 << 12)
>
> struct btf_id_and_flag {
> @@ -1534,6 +1533,15 @@ static int add_kfunc_decl_tag(struct btf *btf, const char *tag, __u32 id, const
> return 0;
> }
>
> +enum kf_bit_nums {
> + KF_BIT_NUM_FASTCALL = 12,
> + KF_BIT_NUM_FASTCALL_NR
> +};
> +
> +static const char *kfunc_tags[KF_BIT_NUM_FASTCALL_NR] = {
> + [KF_BIT_NUM_FASTCALL] = "bpf_fastcall"
> +};
> +
> static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *funcs, const char *kfunc, __u32 flags)
> {
> struct btf_func key = { .name = kfunc };
> @@ -1559,8 +1567,11 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *
> err = add_kfunc_decl_tag(btf, BTF_KFUNC_TYPE_TAG, target->type_id, kfunc);
> if (err < 0)
> return err;
> - if (flags & KF_FASTCALL) {
> - err = add_kfunc_decl_tag(btf, BTF_FASTCALL_TAG, target->type_id, kfunc);
> +
> + for (uint32_t i = 0; i < KF_BIT_NUM_FASTCALL_NR; i++) {
> + if (!(flags & (1u << i)) || !kfunc_tags[i])
> + continue;
> + err = add_kfunc_decl_tag(btf, kfunc_tags[i], target->type_id, kfunc);
> if (err < 0)
> return err;
> }
> ---------------------------------------- >8 ---
>
>>
>>> ---
>>> btf_encoder.c | 59 +++++++++++++++++++++++++++++++++++++--------------
>>> 1 file changed, 43 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/btf_encoder.c b/btf_encoder.c
>>> index 8a2d92e..ae059e0 100644
>>> --- a/btf_encoder.c
>>> +++ b/btf_encoder.c
>>> @@ -39,15 +39,19 @@
>>> #define BTF_ID_SET8_PFX "__BTF_ID__set8__"
>>> #define BTF_SET8_KFUNCS (1 << 0)
>>> #define BTF_KFUNC_TYPE_TAG "bpf_kfunc"
>>> +#define BTF_FASTCALL_TAG "bpf_fastcall"
>>> +#define KF_FASTCALL (1 << 12)
>>> +
>>
>> probably need an #ifndef KF_FASTCALL/#endif here once this makes it into
>> uapi.
>
> kfunc flags are defined in include/linux/btf.h so these should not be
> visible in the uapi/linux/btf.h, unless I'm confused.
>
ah, ok sorry I thought it was UAPI, never mind.
>>
>>
>>> +struct btf_id_and_flag {
>>> + uint32_t id;
>>> + uint32_t flags;
>>> +};
>
> [...]
>
