Re: [RFC v2 2/3] bpftool: add attribute preserve_static_offset for context types

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/12/23 8:07 AM, Quentin Monnet wrote:
2023-12-12 15:58 UTC+0000 ~ Eduard Zingerman <eddyz87@xxxxxxxxx>
On Tue, 2023-12-12 at 11:39 +0000, Quentin Monnet wrote:
[...]
Hi, and thanks for this!

Apologies for missing the discussion on v1. Reading through the previous
thread I see that they were votes in favour of the hard-coded approach,
but I would ask folks to please reconsider.

I'm not keen on taking this list in bpftool, it doesn't seem to be the
right place for that. I understand there's no plan to add new mirror
context structs, but if we change policy for whatever reason, we're sure
to miss the update in this list and that's a bug in bpftool. If bpftool
ever gets ported to Windows and Windows needs support for new structs,
that's more juggling to do to support multiple platforms. And if any
tool other than bpftool needs to generate vmlinux.h headers in the
future, it's back to square one - although by then there might be extra
pushback for changing the BTF, if bpftool already does the work.

Like Alan, I rather share your own inclination towards the more generic
declaration tags approach, if you don't mind the additional work.
Understood, thank you for feedback.
The second option is to:

1. Define __bpf_ctx macro in linux headers as follows:

     #if __has_attribute(preserve_static_offset) && defined(__bpf__)
     #define __bpf_ctx __attribute__((preserve_static_offset)) \
                       __attribute__((btf_decl_tag(preserve_static_offset)))
     #else
     #define __bpf_ctx
     #endif

2a. Update libbpf to emit __attribute__((preserve_static_offset)) when
     corresponding decl tag is present in the BTF.

2b. Update bpftool to emit __attribute__((preserve_static_offset)) for
     types with corresponding decl tag. (Like in this patch but check
     for decl tag instead of name).
I don't have a strong opinion on that part, so...

I think that 2b is better, because emitting
BPF_NO_PRESERVE_STATIC_OFFSET from the same place where
BPF_NO_PRESERVE_ACCESS_INDEX makes more sense,
libbpf does not emit any macro definitions at the moment.
... the above makes sense, I'd say let's go for this if nobody else
objects (or wants it in libbpf instead - but bpftool is fine as far as
I'm concerned).

Thanks, Eduard and Quentin, my original proposal for hardcoded list is
for simplicity. Eduard, could you help also do an implementation like
you proposed in the above so we can then compare each other?

[...]





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux