On 6/6/19 5:10 PM, Jakub Kicinski wrote: > On Thu, 6 Jun 2019 23:27:36 +0000, Alexei Starovoitov wrote: >> On 6/6/19 4:02 PM, Andrii Nakryiko wrote: >>>> struct { >>>> int type; >>>> int max_entries; >>>> } my_map __attribute__((map(int,struct my_value))) = { >>>> .type = BPF_MAP_TYPE_ARRAY, >>>> .max_entries = 16, >>>> }; >>>> >>>> Of course this would need BPF backend support, but at least that approach >>>> would be more C like. Thus this would define types where we can automatically >>> I guess it's technically possible (not a compiler guru, but I don't >>> see why it wouldn't be possible). But it will require at least two >>> things: >>> 1. Compiler support, obviously, as you mentioned. >> >> every time we're doing llvm common change it takes many months. >> Adding BTF took 6 month, though the common changes were trivial. >> Now we're already 1+ month into adding 4 intrinsics to support CO-RE. >> >> In the past I was very much in favor of extending __attribute__ >> with bpf specific stuff. Now not so much. >> __attribute__((map(int,struct my_value))) cannot be done as strings. >> clang has to process the types, create new objects inside debug info. >> It's not clear to me how this modified debug info will be associated >> with the variable my_map. >> So I suspect doing __attribute__ with actual C type inside (()) >> will not be possible. >> I think in the future we might still add string based attributes, >> but it's not going to be easy. >> So... Unless somebody in the community who is doing full time llvm work >> will not step in right now and says "I will code the above attr stuff", >> we should not count on such clang+llvm feature. > > If nobody has resources to commit to this, perhaps we can just stick > to BPF_ANNOTATE_KV_PAIR()? > > Apologies, but I think I missed the memo on why that's considered > a hack. Could someone point me to the relevant discussion? > > We could conceivably add BTF-based map_def for other features, and > solve the K/V problem once a clean solution becomes apparent and > tractable? BPF_ANNOTATE_KV_PAIR() is not great, but we kinda already > have it.. > > Perhaps I'm not thinking clearly about this and I should stay quiet :) the solution we're discussing should solve BPF_ANNOTATE_KV_PAIR too. That hack must go. If I understood your objections to Andrii's format is that you don't like pointer part of key/value while Andrii explained why we picked the pointer, right? So how about: struct { int type; int max_entries; struct { __u32 key; struct my_value value; } types[]; } ...