diff mbox series

[RFC,bpf-next,1/4] bpf: add struct largest member size in func model

Message ID 20250411-many_args_arm64-v1-1-0a32fe72339e@bootlin.com
State New
Headers show
Series bpf, arm64: support up to 12 arguments | expand

Commit Message

Alexis Lothoré (eBPF Foundation) April 11, 2025, 8:32 p.m. UTC
In order to properly JIT the trampolines needed to attach BPF programs
to functions, some architectures like ARM64 need to know about the
alignment needed for the function arguments. Such alignment can
generally be deduced from the argument size, but that's not completely
true for composite types. In the specific case of ARM64, the AAPCS64 ABI
defines that a composite type which needs to be passed through stack
must be aligned on the maximum between 8 and the largest alignment
constraint of its first-level members. So the JIT compiler needs more
information about the arguments to make sure to generate code that
respects those alignment constraints.

For struct arguments, add information about the size of the largest
first-level member in the struct btf_func_model to allow the JIT
compiler to guess the needed alignment. The information is quite
specific, but it allows to keep arch-specific concerns (ie: guessing the
final needed alignment for an argument) isolated in each JIT compiler.

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
 include/linux/bpf.h |  1 +
 kernel/bpf/btf.c    | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

Comments

Alexis Lothoré (eBPF Foundation) April 14, 2025, 8:27 p.m. UTC | #1
Hello Jiri,

On Mon Apr 14, 2025 at 1:04 PM CEST, Jiri Olsa wrote:
> On Fri, Apr 11, 2025 at 10:32:10PM +0200, Alexis Lothoré (eBPF Foundation) wrote:

[...]

>> +	for_each_member(i, t, member) {
>> +		mtype = btf_type_by_id(btf, member->type);
>> +		while (mtype && btf_type_is_modifier(mtype))
>> +			mtype = btf_type_by_id(btf, mtype->type);
>> +		if (!mtype)
>> +			return -EINVAL;
>
> should we use __get_type_size for member->type instead ?

Ah, yes, thanks for the hint, that will allow to get rid of the manual
modifiers skip.

Alexis
> jirka
Andrii Nakryiko April 16, 2025, 9:24 p.m. UTC | #2
On Fri, Apr 11, 2025 at 1:32 PM Alexis Lothoré (eBPF Foundation)
<alexis.lothore@bootlin.com> wrote:
>
> In order to properly JIT the trampolines needed to attach BPF programs
> to functions, some architectures like ARM64 need to know about the
> alignment needed for the function arguments. Such alignment can
> generally be deduced from the argument size, but that's not completely
> true for composite types. In the specific case of ARM64, the AAPCS64 ABI
> defines that a composite type which needs to be passed through stack
> must be aligned on the maximum between 8 and the largest alignment
> constraint of its first-level members. So the JIT compiler needs more
> information about the arguments to make sure to generate code that
> respects those alignment constraints.
>
> For struct arguments, add information about the size of the largest
> first-level member in the struct btf_func_model to allow the JIT
> compiler to guess the needed alignment. The information is quite

I might be missing something, but how can the *size* of the field be
used to calculate that argument's *alignment*? i.e., I don't
understand why arg_largest_member_size needs to be calculated instead
of arg_largest_member_alignment...

> specific, but it allows to keep arch-specific concerns (ie: guessing the
> final needed alignment for an argument) isolated in each JIT compiler.

couldn't all this information be calculated in the JIT compiler (if
JIT needs that) from BTF?

>
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
>  include/linux/bpf.h |  1 +
>  kernel/bpf/btf.c    | 25 +++++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 3f0cc89c0622cb1a097999afb78c17102593b6bb..8b34dcf60a0ce09228ff761b962ab67b6a3e2263 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1106,6 +1106,7 @@ struct btf_func_model {
>         u8 nr_args;
>         u8 arg_size[MAX_BPF_FUNC_ARGS];
>         u8 arg_flags[MAX_BPF_FUNC_ARGS];
> +       u8 arg_largest_member_size[MAX_BPF_FUNC_ARGS];
>  };
>
>  /* Restore arguments before returning from trampoline to let original function
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 16ba36f34dfab7531babf5753cab9f368cddefa3..5d40911ec90210086a6175d569abb6e52d75ad17 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7318,6 +7318,29 @@ static int __get_type_size(struct btf *btf, u32 btf_id,
>         return -EINVAL;
>  }
>
> +static u8 __get_largest_member_size(struct btf *btf, const struct btf_type *t)
> +{
> +       const struct btf_member *member;
> +       const struct btf_type *mtype;
> +       u8 largest_member_size = 0;
> +       int i;
> +
> +       if (!__btf_type_is_struct(t))
> +               return largest_member_size;
> +
> +       for_each_member(i, t, member) {
> +               mtype = btf_type_by_id(btf, member->type);
> +               while (mtype && btf_type_is_modifier(mtype))
> +                       mtype = btf_type_by_id(btf, mtype->type);
> +               if (!mtype)
> +                       return -EINVAL;
> +               if (mtype->size > largest_member_size)
> +                       largest_member_size = mtype->size;
> +       }
> +
> +       return largest_member_size;
> +}
> +
>  static u8 __get_type_fmodel_flags(const struct btf_type *t)
>  {
>         u8 flags = 0;
> @@ -7396,6 +7419,8 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
>                 }
>                 m->arg_size[i] = ret;
>                 m->arg_flags[i] = __get_type_fmodel_flags(t);
> +               m->arg_largest_member_size[i] =
> +                       __get_largest_member_size(btf, t);
>         }
>         m->nr_args = nargs;
>         return 0;
>
> --
> 2.49.0
>
Xu Kuohai April 17, 2025, 2:10 p.m. UTC | #3
On 4/17/2025 3:14 PM, Alexis Lothoré wrote:
> Hi Andrii,
> 
> On Wed Apr 16, 2025 at 11:24 PM CEST, Andrii Nakryiko wrote:
>> On Fri, Apr 11, 2025 at 1:32 PM Alexis Lothoré (eBPF Foundation)
>> <alexis.lothore@bootlin.com> wrote:
>>>
>>> In order to properly JIT the trampolines needed to attach BPF programs
>>> to functions, some architectures like ARM64 need to know about the
>>> alignment needed for the function arguments. Such alignment can
>>> generally be deduced from the argument size, but that's not completely
>>> true for composite types. In the specific case of ARM64, the AAPCS64 ABI
>>> defines that a composite type which needs to be passed through stack
>>> must be aligned on the maximum between 8 and the largest alignment
>>> constraint of its first-level members. So the JIT compiler needs more
>>> information about the arguments to make sure to generate code that
>>> respects those alignment constraints.
>>>
>>> For struct arguments, add information about the size of the largest
>>> first-level member in the struct btf_func_model to allow the JIT
>>> compiler to guess the needed alignment. The information is quite
>>
>> I might be missing something, but how can the *size* of the field be
>> used to calculate that argument's *alignment*? i.e., I don't
>> understand why arg_largest_member_size needs to be calculated instead
>> of arg_largest_member_alignment...
> 
> Indeed I initially checked whether I could return directly some alignment
> info from btf, but it then involves the alignment computation in the btf
> module. Since there could be minor differences between architectures about
> alignment requirements, I though it would be better to in fact keep alignment
> computation out of the btf module. For example, I see that 128 bits values
> are aligned on 16 bytes on ARM64, while being aligned on 8 bytes on S390.
> 
> And since for ARM64, all needed alignments are somehow derived from size
> (it is either directly size for fundamental types, or alignment of the
> largest member for structs, which is then size of largest member),
> returning the size seems to be enough to allow the JIT side to compute
> alignments.
>

Not exactly. The compiler's "packed" and "alignment" attributes cause a
structure to be aligned differently from its natural alignment.

For example, with the following three structures:

struct s0 {
     __int128 x;
};

struct s1 {
     __int128 x;
} __attribute__((packed));

struct s2 {
     __int128 x;
} __attribute__((aligned(64)));

Even though the largest member size is the same, s0 will be aligned to 16
bytes, s1 and s2 are not aligned the same way. s1 has no alignment due to
the "packed" attribute, while s2 will be aligned to 64 bytes.

When these three structures are passed as function arguments, they will be
located on different positions on the stack.

For the following three functions:

int f0(__int128 a, __int128 b, __int128 c, int64_t d, __int128 e, int64_t f, struct s0 g);
int f1(__int128 a, __int128 b, __int128 c, int64_t d, __int128 e, int64_t f, struct s1 g);
int f2(__int128 a, __int128 b, __int128 c, int64_t d, __int128 e, int64_t f, struct s2 g);

g will be located at sp+32 in f0, sp + 24 in f1, and some 64-byte aligned
stack address in f2.

>>> specific, but it allows to keep arch-specific concerns (ie: guessing the
>>> final needed alignment for an argument) isolated in each JIT compiler.
>>
>> couldn't all this information be calculated in the JIT compiler (if
>> JIT needs that) from BTF?
> 
>>From what I understand, the JIT compiler does not have access to BTF info,
> only a substract from it, arranged in a struct btf_func_model ? This
> struct btf_func_model already has size info for standard types, but for
> structs we need some additional info about the members, hence this
> arg_largest_member_alignment addition in btf_func_model.
> 
> Thanks,
> 
> Alexis
>
Alexis Lothoré (eBPF Foundation) April 23, 2025, 3:38 p.m. UTC | #4
On Mon Apr 21, 2025 at 4:14 AM CEST, Xu Kuohai wrote:
> On 4/21/2025 12:02 AM, Alexis Lothoré wrote:
>> Hi Xu,
>> 
>> On Thu Apr 17, 2025 at 4:10 PM CEST, Xu Kuohai wrote:
>>> On 4/17/2025 3:14 PM, Alexis Lothoré wrote:
>>>> Hi Andrii,
>>>>
>>>> On Wed Apr 16, 2025 at 11:24 PM CEST, Andrii Nakryiko wrote:
>>>>> On Fri, Apr 11, 2025 at 1:32 PM Alexis Lothoré (eBPF Foundation)
>>>>> <alexis.lothore@bootlin.com> wrote:

[...]

>> Ah, thanks for those clear examples, I completely overlooked this
>> possibility. And now that you mention it, I feel a bit dumb because I now
>> remember that you mentioned this in Puranjay's series...
>> 
>> I took a quick look at the x86 JIT compiler for reference, and saw no code
>> related to this specific case neither. So I searched in the kernel for
>> actual functions taking struct arguments by value AND being declared with some
>> packed or aligned attribute. I only found a handful of those, and none
>> seems to take enough arguments to have the corresponding struct passed on the
>> stack. So rather than supporting this very specific case, I am tempted
>> to just return an error for now during trampoline creation if we detect such
>> structure (and then the JIT compiler can keep using data size to compute
>> alignment, now that it is sure not to receive custom alignments). Or am I
>> missing some actual cases involving those very specific alignments ?
>> 
>
> How can we reliably 'detect' the case? If a function has such a parameter
> but we fail to detect it, the BPF trampoline will pass an incorrect value
> to the function, which is also unacceptable.

That's a question I still have to answer :) I imagined being able to detect
it thanks to some info somewhere in BTF, but I have to dig further to find
how.


Alexis
Alexis Lothoré (eBPF Foundation) April 24, 2025, 1:38 p.m. UTC | #5
Hi Xu,

On Thu Apr 24, 2025 at 2:00 PM CEST, Xu Kuohai wrote:
> On 4/24/2025 3:24 AM, Alexis Lothoré wrote:
>> Hi Andrii,
>> 
>> On Wed Apr 23, 2025 at 7:15 PM CEST, Andrii Nakryiko wrote:
>>> On Thu, Apr 17, 2025 at 12:14 AM Alexis Lothoré
>>> <alexis.lothore@bootlin.com> wrote:
>>>>
>>>> Hi Andrii,
>>>>
>>>> On Wed Apr 16, 2025 at 11:24 PM CEST, Andrii Nakryiko wrote:
>>>>> On Fri, Apr 11, 2025 at 1:32 PM Alexis Lothoré (eBPF Foundation)
>>>>> <alexis.lothore@bootlin.com> wrote:

[...]

>> Thanks for the pointer, I'll take a look at it. The more we discuss this
>> series, the less member size sounds relevant for what I'm trying to achieve
>> here.
>> 
>> Following Xu's comments, I have been thinking about how I could detect the
>> custom alignments and packing on structures, and I was wondering if I could
>> somehow benefit from __attribute__ encoding in BTF info ([1]). But
>> following your hint, I also see some btf_is_struct_packed() in
>> tools/lib/bpf/btf_dump.c that could help. I'll dig this further and see if
>> I can manage to make something work with all of this.
>>
>
> With DWARF info, we might not need to detect the structure alignment anymore,
> since the DW_AT_location attribute tells us where the structure parameter is
> located on the stack, and DW_AT_byte_size gives us the size of the structure.

I am not sure to follow you here, because DWARF info is not accessible
from kernel at runtime, right ? Or are you meaning that we could, at build
time, enrich the BTF info embedded in the kernel thanks to DWARF info ?

Thanks,

Alexis
Alexei Starovoitov April 24, 2025, 11:14 p.m. UTC | #6
On Thu, Apr 24, 2025 at 6:38 AM Alexis Lothoré
<alexis.lothore@bootlin.com> wrote:
>
> Hi Xu,
>
> On Thu Apr 24, 2025 at 2:00 PM CEST, Xu Kuohai wrote:
> > On 4/24/2025 3:24 AM, Alexis Lothoré wrote:
> >> Hi Andrii,
> >>
> >> On Wed Apr 23, 2025 at 7:15 PM CEST, Andrii Nakryiko wrote:
> >>> On Thu, Apr 17, 2025 at 12:14 AM Alexis Lothoré
> >>> <alexis.lothore@bootlin.com> wrote:
> >>>>
> >>>> Hi Andrii,
> >>>>
> >>>> On Wed Apr 16, 2025 at 11:24 PM CEST, Andrii Nakryiko wrote:
> >>>>> On Fri, Apr 11, 2025 at 1:32 PM Alexis Lothoré (eBPF Foundation)
> >>>>> <alexis.lothore@bootlin.com> wrote:
>
> [...]
>
> >> Thanks for the pointer, I'll take a look at it. The more we discuss this
> >> series, the less member size sounds relevant for what I'm trying to achieve
> >> here.
> >>
> >> Following Xu's comments, I have been thinking about how I could detect the
> >> custom alignments and packing on structures, and I was wondering if I could
> >> somehow benefit from __attribute__ encoding in BTF info ([1]). But
> >> following your hint, I also see some btf_is_struct_packed() in
> >> tools/lib/bpf/btf_dump.c that could help. I'll dig this further and see if
> >> I can manage to make something work with all of this.
> >>
> >
> > With DWARF info, we might not need to detect the structure alignment anymore,
> > since the DW_AT_location attribute tells us where the structure parameter is
> > located on the stack, and DW_AT_byte_size gives us the size of the structure.
>
> I am not sure to follow you here, because DWARF info is not accessible
> from kernel at runtime, right ? Or are you meaning that we could, at build
> time, enrich the BTF info embedded in the kernel thanks to DWARF info ?

Sounds like arm64 has complicated rules for stack alignment and
stack offset computation for passing 9th+ argument.

Since your analysis shows:
"there are about 200 functions accept 9 to 12 arguments, so adding support
for up to 12 function arguments."
I say, let's keep the existing limitation:
        if (nregs > 8)
                return -ENOTSUPP;

If there is a simple and dumb way to detect that arg9+ are scalars
with simple stack passing rules, then, sure, let's support those too,
but fancy packed/align(x)/etc let's ignore.
Alexis Lothoré (eBPF Foundation) April 25, 2025, 8:47 a.m. UTC | #7
Hello Alexei,

On Fri Apr 25, 2025 at 1:14 AM CEST, Alexei Starovoitov wrote:
> On Thu, Apr 24, 2025 at 6:38 AM Alexis Lothoré
> <alexis.lothore@bootlin.com> wrote:

[...]

>> > With DWARF info, we might not need to detect the structure alignment anymore,
>> > since the DW_AT_location attribute tells us where the structure parameter is
>> > located on the stack, and DW_AT_byte_size gives us the size of the structure.
>>
>> I am not sure to follow you here, because DWARF info is not accessible
>> from kernel at runtime, right ? Or are you meaning that we could, at build
>> time, enrich the BTF info embedded in the kernel thanks to DWARF info ?
>
> Sounds like arm64 has complicated rules for stack alignment and
> stack offset computation for passing 9th+ argument.

AFAICT, arm64 has some specificities for large types, but not that much
compared to x86 for example. If I take a look at System V ABI ([1]), I see
pretty much the same constraints:
- p.18: "Arguments of type __int128 offer the same operations as INTEGERs,
  [...] with the exception that arguments of type __int128 that are stored
  in memory must be aligned on a 16-byte boundary"
- p.13: "Structures and unions assume the alignment of their most strictly
  aligned component"
- the custom packing and alignments attributes will end up having the same
  consequence on both architectures

As I mentioned in my cover letter, the new tests covering those same
alignment constraints for ARM64 break on x86, which makes me think other
archs are also silently ignoring those cases.

> Since your analysis shows:
> "there are about 200 functions accept 9 to 12 arguments, so adding support
> for up to 12 function arguments."
> I say, let's keep the existing limitation:
>         if (nregs > 8)
>                 return -ENOTSUPP;
>
> If there is a simple and dumb way to detect that arg9+ are scalars
> with simple stack passing rules, then, sure, let's support those too,
> but fancy packed/align(x)/etc let's ignore.


[1] https://refspecs.linuxbase.org/elf/x86_64-abi-0.99.pdf
Alexis Lothoré (eBPF Foundation) June 4, 2025, 9:02 a.m. UTC | #8
Hi all,
a simpler version of this series has been merged, and so I am now taking a
look at the issue I have put aside in the merged version: dealing with more
specific data layout for arguments passed on stack. For example, a function
can pass small structs on stack, but this need special care when generating
the corresponding bpf trampolines. Those structs have specific alignment
specified by the target ABI, but it can also be altered with attributes
packing the structure or modifying the alignment.

Some platforms already support structs on stack (see
tracing_struct_many_args test), but as discussed earlier, those may suffer
from the same kind of issue mentioned above.

On Wed Apr 23, 2025 at 9:24 PM CEST, Alexis Lothoré wrote:
> Hi Andrii,
>
> On Wed Apr 23, 2025 at 7:15 PM CEST, Andrii Nakryiko wrote:
>> On Thu, Apr 17, 2025 at 12:14 AM Alexis Lothoré
>> <alexis.lothore@bootlin.com> wrote:
>>>
>>> Hi Andrii,
>>>
>>> On Wed Apr 16, 2025 at 11:24 PM CEST, Andrii Nakryiko wrote:

[...]

>> I'd suggest looking at btf__align_of() in libbpf (tools/lib/bpf/btf.c)
>> to see how we calculate alignment there. It seems to work decently
>> enough. It won't cover any arch-specific extra rules like double
>> needing 16-byte alignment (I vaguely remember something like that for
>> some architectures, but I might be misremembering), or anything
>> similar. It also won't detect (I don't think it's possible without
>> DWARF) artificially increased alignment with attribute((aligned(N))).
>
> Thanks for the pointer, I'll take a look at it. The more we discuss this
> series, the less member size sounds relevant for what I'm trying to achieve
> here.
>
> Following Xu's comments, I have been thinking about how I could detect the
> custom alignments and packing on structures, and I was wondering if I could
> somehow benefit from __attribute__ encoding in BTF info ([1]). But
> following your hint, I also see some btf_is_struct_packed() in
> tools/lib/bpf/btf_dump.c that could help. I'll dig this further and see if
> I can manage to make something work with all of this.

Andrii's comment above illustrates well my current issue: when functions
pass arguments on stack, we are missing info for some of them to correctly
build trampolines, especially for struct, which can have attributes like
__attribute__((packed)) or __attribute__((align(x))). [1] seems to be a
recent solution implemented for BTF to cover this need. IIUC it encodes any
arbitratry attribute affecting a data type or function, so if I have some
struct like this one in my kernel or a module:

struct foo {
    short b
    int a;
} __packed;

I would expect the corresponding BTF data to have some BTF_KIND_DECL_TAG
describing the "packed" attribute for the corresponding structure, but I
fail to find any of those when running:

$ bpftool btf dump file vmlinux format raw

In there I see some DECL_TAG but those are mostly 'bpf_kfunc', I see not
arbitrary attribute like 'packed' or 'aligned(x)'.

What I really need to do in the end is to be able to parse those alignments
attributes info in the kernel at runtime when generating trampolines, but I
hoped to be able to see them with bpftool first to validate the concept.

I started taking a look further at this and stumbled upon [2] in which Alan
gives many more details about the feature, so I did the following checks:
- kernel version 6.15.0-rc4 from bpf-next_base: it contains the updated
  Makefile.btf calling pahole with `--btf_features=attributes`
- pahole v1.30
  $ pahole --supported_btf_features
  encode_force,var,float,decl_tag,type_tag,enum64,optimized_func,consistent_func,decl_tag_kfuncs,reproducible_build,distilled_base,global_var,attributes
  This pahole comes from my distro pkg manager, but I have also done the
  same test with a freshly built pahole, while taking care of pulling the
  libbpf submodule.
- bpftool v7.6.0
    bpftool v7.6.0
    using libbpf v1.6
    features: llvm, skeletons

Could I be missing something obvious ? Or did I misunderstand the actual
attribute encoding feature ?

Thanks,

Alexis

[1] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/
[2] https://lore.kernel.org/all/CA+icZUW31vpS=R3zM6G4FMkzuiQovqtd+e-8ihwsK_A-QtSSYg@mail.gmail.com/
Ihor Solodrai June 4, 2025, 5:31 p.m. UTC | #9
On 6/4/25 2:02 AM, Alexis Lothoré wrote:
> Hi all,
> a simpler version of this series has been merged, and so I am now taking a
> look at the issue I have put aside in the merged version: dealing with more
> specific data layout for arguments passed on stack. For example, a function
> can pass small structs on stack, but this need special care when generating
> the corresponding bpf trampolines. Those structs have specific alignment
> specified by the target ABI, but it can also be altered with attributes
> packing the structure or modifying the alignment.
> 
> Some platforms already support structs on stack (see
> tracing_struct_many_args test), but as discussed earlier, those may suffer
> from the same kind of issue mentioned above.
> 
> On Wed Apr 23, 2025 at 9:24 PM CEST, Alexis Lothoré wrote:
>> Hi Andrii,
>>
>> On Wed Apr 23, 2025 at 7:15 PM CEST, Andrii Nakryiko wrote:
>>> On Thu, Apr 17, 2025 at 12:14 AM Alexis Lothoré
>>> <alexis.lothore@bootlin.com> wrote:
>>>>
>>>> Hi Andrii,
>>>>
>>>> On Wed Apr 16, 2025 at 11:24 PM CEST, Andrii Nakryiko wrote:
> 
> [...]
> 
>>> I'd suggest looking at btf__align_of() in libbpf (tools/lib/bpf/btf.c)
>>> to see how we calculate alignment there. It seems to work decently
>>> enough. It won't cover any arch-specific extra rules like double
>>> needing 16-byte alignment (I vaguely remember something like that for
>>> some architectures, but I might be misremembering), or anything
>>> similar. It also won't detect (I don't think it's possible without
>>> DWARF) artificially increased alignment with attribute((aligned(N))).
>>
>> Thanks for the pointer, I'll take a look at it. The more we discuss this
>> series, the less member size sounds relevant for what I'm trying to achieve
>> here.
>>
>> Following Xu's comments, I have been thinking about how I could detect the
>> custom alignments and packing on structures, and I was wondering if I could
>> somehow benefit from __attribute__ encoding in BTF info ([1]). But
>> following your hint, I also see some btf_is_struct_packed() in
>> tools/lib/bpf/btf_dump.c that could help. I'll dig this further and see if
>> I can manage to make something work with all of this.
> 
> Andrii's comment above illustrates well my current issue: when functions
> pass arguments on stack, we are missing info for some of them to correctly
> build trampolines, especially for struct, which can have attributes like
> __attribute__((packed)) or __attribute__((align(x))). [1] seems to be a
> recent solution implemented for BTF to cover this need. IIUC it encodes any
> arbitratry attribute affecting a data type or function, so if I have some
> struct like this one in my kernel or a module:
> 
> struct foo {
>      short b
>      int a;
> } __packed;
> 
> I would expect the corresponding BTF data to have some BTF_KIND_DECL_TAG
> describing the "packed" attribute for the corresponding structure, but I
> fail to find any of those when running:
> 
> $ bpftool btf dump file vmlinux format raw
> 
> In there I see some DECL_TAG but those are mostly 'bpf_kfunc', I see not
> arbitrary attribute like 'packed' or 'aligned(x)'.
> 
> What I really need to do in the end is to be able to parse those alignments
> attributes info in the kernel at runtime when generating trampolines, but I
> hoped to be able to see them with bpftool first to validate the concept.
> 
> I started taking a look further at this and stumbled upon [2] in which Alan
> gives many more details about the feature, so I did the following checks:
> - kernel version 6.15.0-rc4 from bpf-next_base: it contains the updated
>    Makefile.btf calling pahole with `--btf_features=attributes`
> - pahole v1.30
>    $ pahole --supported_btf_features
>    encode_force,var,float,decl_tag,type_tag,enum64,optimized_func,consistent_func,decl_tag_kfuncs,reproducible_build,distilled_base,global_var,attributes
>    This pahole comes from my distro pkg manager, but I have also done the
>    same test with a freshly built pahole, while taking care of pulling the
>    libbpf submodule.
> - bpftool v7.6.0
>      bpftool v7.6.0
>      using libbpf v1.6
>      features: llvm, skeletons
> 
> Could I be missing something obvious ? Or did I misunderstand the actual
> attribute encoding feature ?

Hi Alexis.

The changes recently landed in pahole and libbpf re attributes had a 
very narrow goal: passing through particular attributes for some BPF 
kfuncs from the kernel source to vmlinux.h

BTF now has a way of encoding any attribute (as opposed to only bpf 
type/decl tags) by setting type/decl tag kind flag [1]. So it is 
possible to represent attributes like packed and aligned in BTF.

However, the BTF tags need to be generated by something, in case of 
vmlinux by pahole. Pahole generates BTF by parsing DWARF. And, as far as 
I understand, attributes are not (can not be?) represented in DWARF in a 
generic way, it really depends on specifics of the attribute.

In order to support packed/aligned, pahole needs to know how to figure 
them out from DWARF input and add the tags to BTF. And this does not 
happen right now, which is why you don't see anything in bpftool output.

[1] 
https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/

> 
> Thanks,
> 
> Alexis
> 
> [1] https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/
> [2] https://lore.kernel.org/all/CA+icZUW31vpS=R3zM6G4FMkzuiQovqtd+e-8ihwsK_A-QtSSYg@mail.gmail.com/
>
Alexis Lothoré (eBPF Foundation) June 5, 2025, 7:35 a.m. UTC | #10
Hi Ihor,

On Wed Jun 4, 2025 at 7:31 PM CEST, Ihor Solodrai wrote:
> On 6/4/25 2:02 AM, Alexis Lothoré wrote:

[...]

>> Could I be missing something obvious ? Or did I misunderstand the actual
>> attribute encoding feature ?
>
> Hi Alexis.
>
> The changes recently landed in pahole and libbpf re attributes had a 
> very narrow goal: passing through particular attributes for some BPF 
> kfuncs from the kernel source to vmlinux.h
>
> BTF now has a way of encoding any attribute (as opposed to only bpf 
> type/decl tags) by setting type/decl tag kind flag [1]. So it is 
> possible to represent attributes like packed and aligned in BTF.
>
> However, the BTF tags need to be generated by something, in case of 
> vmlinux by pahole. Pahole generates BTF by parsing DWARF. And, as far as 
> I understand, attributes are not (can not be?) represented in DWARF in a 
> generic way, it really depends on specifics of the attribute.
>
> In order to support packed/aligned, pahole needs to know how to figure 
> them out from DWARF input and add the tags to BTF. And this does not 
> happen right now, which is why you don't see anything in bpftool output.
>
> [1] 
> https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/

Thanks for the details ! I have missed this possibility, as I have been
assuming that DWARF info was exposing the needed info. I'll take a look at
it, but if those attributes can not be represented by DWARF, I'll have to
find another way of getting those packing/alignment modifications on data
type (eg: re-use/share btf__align_of from libbpf, as suggested by Andrii,
but it may not able to cover all cases).

Thanks,

Alexis
Alexei Starovoitov June 5, 2025, 4:09 p.m. UTC | #11
On Thu, Jun 5, 2025 at 12:35 AM Alexis Lothoré
<alexis.lothore@bootlin.com> wrote:
>
> Hi Ihor,
>
> On Wed Jun 4, 2025 at 7:31 PM CEST, Ihor Solodrai wrote:
> > On 6/4/25 2:02 AM, Alexis Lothoré wrote:
>
> [...]
>
> >> Could I be missing something obvious ? Or did I misunderstand the actual
> >> attribute encoding feature ?
> >
> > Hi Alexis.
> >
> > The changes recently landed in pahole and libbpf re attributes had a
> > very narrow goal: passing through particular attributes for some BPF
> > kfuncs from the kernel source to vmlinux.h
> >
> > BTF now has a way of encoding any attribute (as opposed to only bpf
> > type/decl tags) by setting type/decl tag kind flag [1]. So it is
> > possible to represent attributes like packed and aligned in BTF.
> >
> > However, the BTF tags need to be generated by something, in case of
> > vmlinux by pahole. Pahole generates BTF by parsing DWARF. And, as far as
> > I understand, attributes are not (can not be?) represented in DWARF in a
> > generic way, it really depends on specifics of the attribute.
> >
> > In order to support packed/aligned, pahole needs to know how to figure
> > them out from DWARF input and add the tags to BTF. And this does not
> > happen right now, which is why you don't see anything in bpftool output.
> >
> > [1]
> > https://lore.kernel.org/bpf/20250130201239.1429648-1-ihor.solodrai@linux.dev/
>
> Thanks for the details ! I have missed this possibility, as I have been
> assuming that DWARF info was exposing the needed info. I'll take a look at
> it, but if those attributes can not be represented by DWARF, I'll have to
> find another way of getting those packing/alignment modifications on data
> type (eg: re-use/share btf__align_of from libbpf, as suggested by Andrii,
> but it may not able to cover all cases).

Not sure all the trouble is worth it.
I feel it's a corner case. Something we don't need to fix.
Alexis Lothoré (eBPF Foundation) June 6, 2025, 7:45 a.m. UTC | #12
Hi Alexei,

On Thu Jun 5, 2025 at 6:09 PM CEST, Alexei Starovoitov wrote:
> On Thu, Jun 5, 2025 at 12:35 AM Alexis Lothoré
> <alexis.lothore@bootlin.com> wrote:
>>
>> Hi Ihor,
>>
>> On Wed Jun 4, 2025 at 7:31 PM CEST, Ihor Solodrai wrote:
>> > On 6/4/25 2:02 AM, Alexis Lothoré wrote:

[...]

>> Thanks for the details ! I have missed this possibility, as I have been
>> assuming that DWARF info was exposing the needed info. I'll take a look at
>> it, but if those attributes can not be represented by DWARF, I'll have to
>> find another way of getting those packing/alignment modifications on data
>> type (eg: re-use/share btf__align_of from libbpf, as suggested by Andrii,
>> but it may not able to cover all cases).
>
> Not sure all the trouble is worth it.
> I feel it's a corner case. Something we don't need to fix.

TBH I don't own any specific use case really needing this handling, so if
it does not feel worth the trouble, I'm fine with not trying to support
this. My effort is rather motivated by the goal of aligning the ARM64
features with other platform, and so of getting rid of
tools/testing/selftests/bpf/DENYLIST.aarch64.

For the record, this effort also showed that the same kind of issue affects
other platforms already supporting many args + structs passed by value ([1])
- structs alignment with specific alignment constraints are not
  specifically handled (eg: a struct with an __int128 as a top-level
  member, leading to a 16 byte alignment requirement)
- packing and custom alignment is not handled
diff mbox series

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 3f0cc89c0622cb1a097999afb78c17102593b6bb..8b34dcf60a0ce09228ff761b962ab67b6a3e2263 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1106,6 +1106,7 @@  struct btf_func_model {
 	u8 nr_args;
 	u8 arg_size[MAX_BPF_FUNC_ARGS];
 	u8 arg_flags[MAX_BPF_FUNC_ARGS];
+	u8 arg_largest_member_size[MAX_BPF_FUNC_ARGS];
 };
 
 /* Restore arguments before returning from trampoline to let original function
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 16ba36f34dfab7531babf5753cab9f368cddefa3..5d40911ec90210086a6175d569abb6e52d75ad17 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7318,6 +7318,29 @@  static int __get_type_size(struct btf *btf, u32 btf_id,
 	return -EINVAL;
 }
 
+static u8 __get_largest_member_size(struct btf *btf, const struct btf_type *t)
+{
+	const struct btf_member *member;
+	const struct btf_type *mtype;
+	u8 largest_member_size = 0;
+	int i;
+
+	if (!__btf_type_is_struct(t))
+		return largest_member_size;
+
+	for_each_member(i, t, member) {
+		mtype = btf_type_by_id(btf, member->type);
+		while (mtype && btf_type_is_modifier(mtype))
+			mtype = btf_type_by_id(btf, mtype->type);
+		if (!mtype)
+			return -EINVAL;
+		if (mtype->size > largest_member_size)
+			largest_member_size = mtype->size;
+	}
+
+	return largest_member_size;
+}
+
 static u8 __get_type_fmodel_flags(const struct btf_type *t)
 {
 	u8 flags = 0;
@@ -7396,6 +7419,8 @@  int btf_distill_func_proto(struct bpf_verifier_log *log,
 		}
 		m->arg_size[i] = ret;
 		m->arg_flags[i] = __get_type_fmodel_flags(t);
+		m->arg_largest_member_size[i] =
+			__get_largest_member_size(btf, t);
 	}
 	m->nr_args = nargs;
 	return 0;