On Tue, Nov 1, 2022 at 12:21 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Tue, 2022-11-01 at 11:35 -0700, Alexei Starovoitov wrote: > > On Tue, Nov 1, 2022 at 9:02 AM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote: > > > > > > > > Yes, we discussed this before. This will need to add additional work > > > > > in preprocessor. I just made a discussion topic in llvm discourse > > > > > > > > > > https://discourse.llvm.org/t/add-a-type-checking-macro-is-type-defined-type/66268 > > > > That would be a great clang feature. > > Thanks Yonghong! > > > > > > > > > > > > Let us see whether we can get some upstream agreement or not. > > > > > > > > > > > > > Thanks for starting the conversation! I'll be following along. > > > > > > > > > > > > > I think this sort of approach assumes that vmlinux.h is included after > > > any uapi headers, and would guard type definitions with > > > > > > #if type_is_defined(foo) > > > struct foo { > > > > > > }; > > > #endif > > > > > > ...is that right? My concern is that the vmlinux.h definitions have > > > the CO-RE attributes. From a BPF perspective, would we like the vmlinux.h > > > definitions to dominate over UAPI definitions do you think, or does it > > > matter? > > > > I think it's totally fine to require #include "vmlinux.h" to be last. > > The attr(preserve_access_index) is only useful for kernel internal > > structs. uapi structs don't need it. > > > > > > > > I was wondering if there might be yet another way to crack this; > > > if we did want the vmlinux.h type definitions to be authoritative > > > because they have the preserve access index attribute, and because > > > bpftool knows all vmlinux types, it could use that info to selectively > > > redefine those type names such that we avoid name clashes when later > > > including UAPI headers. Something like > > > > > > #ifdef __VMLINUX_H__ > > > //usual vmlinux.h type definitions > > > #endif /* __VMLINUX_H__ */ > > > > > > #ifdef __VMLINUX_ALIAS__ > > > if !defined(timespec64) > > > #define timespec64 __VMLINUX_ALIAS__timespec64 > > > #endif > > > // rest of the types define aliases here > > > #undef __VMLINUX_ALIAS__ > > > #else /* unalias */ > > > #if defined(timespec64) > > > #undef timespec64 > > > #endif > > > // rest of types undef aliases here > > > #endif /* __VMLINUX_ALIAS__ */ > > > > > > > > > Then the consumer does this: > > > > > > #define __VMLINUX_ALIAS__ > > > #include "vmlinux.h" > > > // include uapi headers > > > #include "vmlinux.h" > > > > > > (the latter include of vmlinux.h is needed to undef all the type aliases) > > > > Sounds like a bunch of complexity for the use case that is not > > clear to me. > > Well, my RFC is not shy of complexity :) > What Alan suggests should solve the confilicts described in [1] or any > other confilicts of such kind. > > [1] https://lore.kernel.org/bpf/999da51bdf050f155ba299500061b3eb6e0dcd0d.camel@xxxxxxxxx/ I don't quite see how renaming all types in the 1st vmlinux.h will help with name collisions inside enum {}. The enums will conflict with 2nd vmlinux.h too. Unless the proposal is to rename them as well, but then what's the point?
