| Message ID | 20210414200146.2663044-13-andrii@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | BPF static linker: support externs | expand |
On 4/14/21 1:01 PM, Andrii Nakryiko wrote: > Add extra logic to handle map externs (only BTF-defined maps are supported for > linking). Re-use the map parsing logic used during bpf_object__open(). Map > externs are currently restricted to always and only specify map type, key > type and/or size, and value type and/or size. Nothing extra is allowed. If any > of those attributes are mismatched between extern and actual map definition, > linker will report an error. I don't get the motivation for this. It seems cumbersome to force users to do: +extern struct { + __uint(type, BPF_MAP_TYPE_HASH); + __type(key, key_type); + __type(value, value_type); + /* no max_entries on extern map definitions */ +} map1 SEC(".maps"); > The original intent was to allow for extern to specify attributes that matters > (to user) to enforce. E.g., if you specify just key information and omit > value, then any value fits. Similarly, it should have been possible to enforce > map_flags, pinning, and any other possible map attribute. Unfortunately, that > means that multiple externs can be only partially overlapping with each other, > which means linker would need to combine their type definitions to end up with > the most restrictive and fullest map definition. but there is only one such full map definition. Can all externs to be: extern struct {} map1 SEC(".maps"); They can be in multiple .o files, but one true global map def should have all the fields and will take the precedence during the linking. The map type, key size, value size, max entries are all irrelevant during compilation. They're relevant during loading, but libbpf is not going to load every .o individually. So "extern map" can have any fields it wouldn't change the end result after linking. May be enforce that 'extern struct {} map' doesn't have any fields defined instead? It seems asking users to copy-paste map defs in one file and in all of extern is just extra hassle. The users wouldn't want to copy-paste them for production code, but will put map def into .h and include in multiple .c, but adding "extern " in many .c-s and not adding that "extern " is the main .c is another macro hassle. Actually forcing "no max_entries in extern" is killing this idea. So it's mandatory copy-paste or even more macro magic with partial defs of maps? What am I missing?
On Wed, Apr 14, 2021 at 3:00 PM Alexei Starovoitov <ast@fb.com> wrote: > > On 4/14/21 1:01 PM, Andrii Nakryiko wrote: > > Add extra logic to handle map externs (only BTF-defined maps are supported for > > linking). Re-use the map parsing logic used during bpf_object__open(). Map > > externs are currently restricted to always and only specify map type, key > > type and/or size, and value type and/or size. Nothing extra is allowed. If any > > of those attributes are mismatched between extern and actual map definition, > > linker will report an error. > > I don't get the motivation for this. > It seems cumbersome to force users to do: > +extern struct { > + __uint(type, BPF_MAP_TYPE_HASH); > + __type(key, key_type); > + __type(value, value_type); > + /* no max_entries on extern map definitions */ > +} map1 SEC(".maps"); The intent was to simulate what you'd have in a language with generics. E.g., if you were declaring extern for a map in C++: extern std::map<key_type, value_type> my_map; You'd want a linker to make sure that actual my_map definition to conform to your expectations, no? > > > The original intent was to allow for extern to specify attributes that matters > > (to user) to enforce. E.g., if you specify just key information and omit > > value, then any value fits. Similarly, it should have been possible to enforce > > map_flags, pinning, and any other possible map attribute. Unfortunately, that > > means that multiple externs can be only partially overlapping with each other, > > which means linker would need to combine their type definitions to end up with > > the most restrictive and fullest map definition. > > but there is only one such full map definition. > Can all externs to be: > extern struct {} map1 SEC(".maps"); I can certainly modify logic to allow this. But for variables and funcs we want to enforce type information, right? So I'm not sure why you think it's bad for maps. > > They can be in multiple .o files, but one true global map def > should have all the fields and will take the precedence during > the linking. So if it's just a multi-file application and you don't care which file declares that map, you can do a single __weak definition in a header and forget about it. But imagine a BPF library, maintained separately from some BPF application that is using it. And imagine that for some reason that BPF library wants/needs to "export" its map directly. In such case, I'd imagine BPF library author to provide a header with pre-defined correct extern definition of that map. It's the same situation as with extern functions. You are either copy/pasting exact function signature or providing it through some common header. BPF map definition is just slightly more verbose. > > The map type, key size, value size, max entries are all irrelevant > during compilation. They're relevant during loading, but libbpf is > not going to load every .o individually. So "extern map" can > have any fields it wouldn't change the end result after linking. > May be enforce that 'extern struct {} map' doesn't have > any fields defined instead? It's easy for me to do that as well, it's just a question of what behavior makes more sense and what are we trying to achieve. Of course during the compilation itself it doesn't matter that's the type of map is or what key/value type/size is. But from the programmer's point of view, when I do lookup/update, I'd like to know that my map corresponds to my understanding. So if I assume 4-byte key, and 16-byte value and allocate stack variables according to that understanding, yet something changes about BPF map definition, I'd rather notice that during linking, than maybe notice during BPF verification. So that was the only motivation: catch mismatch earlier. I originally wanted to let users define which attributes matter and enforce them (as I mention in the commit description), but that requires some more work on merging BTF. Now that I'm done with all the rest logic, I guess I can go and address that as well. So that would support cases from: extern struct {} my_map SEC(".maps"); to extern struct { __uint(type, BPF_MAP_TYPE_ARRAY); __type(key, int); __type(value, struct value_type); __uint(map_flags, BPF_F_MMAPABLE); /* because I care for whatever reason */ __uint(pinning, LIBBPF_PIN_BY_NAME); /* because I can */ } my_peculiar_map SEC(".maps"); But basically, if we allow only `extern struct {} my_map SEC(".maps");`, why do I even bother with BTF in that case? > It seems asking users to copy-paste map defs in one file and in all > of extern is just extra hassle. So see above about __weak. As for the BPF library providers, that felt unavoidable (and actually desirable), because that's what they would do with extern func and extern vars anyways. And that's what we do with C code today, except linker is oblivious to types (because no BTF in user-space C world). > The users wouldn't want to copy-paste them for production code, > but will put map def into .h and include in multiple .c, > but adding "extern " in many .c-s and not > adding that "extern " is the main .c is another macro hassle. > Actually forcing "no max_entries in extern" is killing this idea. so forcing to type+key+value is to make sure that currently all externs (if there are many) are exactly the same. Because as soon as I allow some to specify max_entries and some don't, then depending on the order in which I see those externs (before actual definition), I'll need to merge their definitions (worst case), or at least pick the most complete one. It's doable, but felt unnecessary for the first iteration. > So it's mandatory copy-paste or even more macro magic with partial > defs of maps? > What am I missing? Maybe nothing, just there is no single right answer (except the aspirational implementation I explained above). I'm open to discussion, btw, not claiming my way is the best way.
On Wed, Apr 14, 2021 at 04:48:25PM -0700, Andrii Nakryiko wrote: > On Wed, Apr 14, 2021 at 3:00 PM Alexei Starovoitov <ast@fb.com> wrote: > > > > On 4/14/21 1:01 PM, Andrii Nakryiko wrote: > > > Add extra logic to handle map externs (only BTF-defined maps are supported for > > > linking). Re-use the map parsing logic used during bpf_object__open(). Map > > > externs are currently restricted to always and only specify map type, key > > > type and/or size, and value type and/or size. Nothing extra is allowed. If any > > > of those attributes are mismatched between extern and actual map definition, > > > linker will report an error. > > > > I don't get the motivation for this. > > It seems cumbersome to force users to do: > > +extern struct { > > + __uint(type, BPF_MAP_TYPE_HASH); > > + __type(key, key_type); > > + __type(value, value_type); > > + /* no max_entries on extern map definitions */ > > +} map1 SEC(".maps"); > > The intent was to simulate what you'd have in a language with > generics. E.g., if you were declaring extern for a map in C++: > > extern std::map<key_type, value_type> my_map; right, because C++ will mangle types into names. When llvm bpf backend will support C++ front-end it will do the mangling too. I think BPF is ready for C++, but it's a separate discussion, of course. > > but there is only one such full map definition. > > Can all externs to be: > > extern struct {} map1 SEC(".maps"); > > I can certainly modify logic to allow this. But for variables and > funcs we want to enforce type information, right? So I'm not sure why > you think it's bad for maps. I'm not saying it's bad. Traditional linker only deals with names, since we're in C domain, so far, I figured it's an option, but more below. C++ is good analogy too. > So if it's just a multi-file application and you don't care which file > declares that map, you can do a single __weak definition in a header > and forget about it. > > But imagine a BPF library, maintained separately from some BPF > application that is using it. And imagine that for some reason that > BPF library wants/needs to "export" its map directly. In such case, > I'd imagine BPF library author to provide a header with pre-defined > correct extern definition of that map. I'm mainly looking at patch 17 and thinking how that copy paste can be avoided. In C and C++ world the user would do: defs.h: struct S { ... }; extern struct S s; file.c: #include "defs.h" struct S s; and it would work, but afaics it won't work for BPF C in patch 17. If the user does: defs.h: struct my_map { __uint(type, BPF_MAP_TYPE_HASH); __type(key, struct my_key); __type(value, struct my_value); __uint(max_entries, 16); }; extern struct my_map map1 SEC(".maps"); file.c: #include "defs.h" struct my_map map1; // do we need SEC here too? probably not? It won't work for another_filer.c since max_entries are not allowed? Why, btw? So how the user suppose to do this? With __weak in .h ? But if that's the only reasonable choice whe bother supporting extern in the linker? > I originally wanted to let users define which attributes matter and > enforce them (as I mention in the commit description), but that > requires some more work on merging BTF. Now that I'm done with all the > rest logic, I guess I can go and address that as well. I think that would be overkill. It won't match neither C style nor C++. Let's pick one. > So see above about __weak. As for the BPF library providers, that felt > unavoidable (and actually desirable), because that's what they would > do with extern func and extern vars anyways. As far as supporting __weak for map defs, I think __weak in one file.c should be weak for all attributes. Another_file.c should be able to define the same map name without __weak and different types, value/type sizes. Because why not? Sort-of C++ style of override. > so forcing to type+key+value is to make sure that currently all > externs (if there are many) are exactly the same. Because as soon as I > allow some to specify max_entries and some don't, I don't get why max_entries is special. They can be overridden in typical skeleton usage. After open and before load. So max_entries is a default value in map init. Whether it's part of extern or not why should that matter? > Maybe nothing, just there is no single right answer (except the > aspirational implementation I explained above). I'm open to > discussion, btw, not claiming my way is the best way. I'm not suggesting that extern struct {} my_map; is the right answer either. Mainly looking into how user code will look like and trying to make it look the most similar to how C, C++ code traditionally looks. BPF C is reduced and extended C at the same time. BPF C++ will be similar. Certain features will be supported right away, some others will take time. I'm looking at BTF as a language independent concept. Both C and C++ will rely on it. To summarize if max_entries can be supported and ingored in extern when the definition has a different value then it's probably good to enforce that the rest of map fields are the same. Then my .h/.c example above will work. In case of __weak probably all map fields can change. It can be seen as a weak definition of the whole map. Not just weak of the variable. It's a default for everything that can be overridden. While non-weak can override max_entries only. btw for signed progs I'm thinking to allow override of max_entries only, since this attribute doesn't affect safety, correctness, behavior. Meaning max_entries will and will not be part of a signature at the same time. In other words it's necessary to support existing bcc/libbpf-tools. If we go with 'allow max_entries in extern' that would match that behavior.
On Wed, Apr 14, 2021 at 7:01 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Wed, Apr 14, 2021 at 04:48:25PM -0700, Andrii Nakryiko wrote: > > On Wed, Apr 14, 2021 at 3:00 PM Alexei Starovoitov <ast@fb.com> wrote: > > > > > > On 4/14/21 1:01 PM, Andrii Nakryiko wrote: > > > > Add extra logic to handle map externs (only BTF-defined maps are supported for > > > > linking). Re-use the map parsing logic used during bpf_object__open(). Map > > > > externs are currently restricted to always and only specify map type, key > > > > type and/or size, and value type and/or size. Nothing extra is allowed. If any > > > > of those attributes are mismatched between extern and actual map definition, > > > > linker will report an error. > > > > > > I don't get the motivation for this. > > > It seems cumbersome to force users to do: > > > +extern struct { > > > + __uint(type, BPF_MAP_TYPE_HASH); > > > + __type(key, key_type); > > > + __type(value, value_type); > > > + /* no max_entries on extern map definitions */ > > > +} map1 SEC(".maps"); > > > > The intent was to simulate what you'd have in a language with > > generics. E.g., if you were declaring extern for a map in C++: > > > > extern std::map<key_type, value_type> my_map; > > right, because C++ will mangle types into names. > When llvm bpf backend will support C++ front-end it will do the mangling too. > I think BPF is ready for C++, but it's a separate discussion, of course. > > > > but there is only one such full map definition. > > > Can all externs to be: > > > extern struct {} map1 SEC(".maps"); > > > > I can certainly modify logic to allow this. But for variables and > > funcs we want to enforce type information, right? So I'm not sure why > > you think it's bad for maps. > > I'm not saying it's bad. > Traditional linker only deals with names, since we're in C domain, so far, > I figured it's an option, but more below. > C++ is good analogy too. > > > So if it's just a multi-file application and you don't care which file > > declares that map, you can do a single __weak definition in a header > > and forget about it. > > > > But imagine a BPF library, maintained separately from some BPF > > application that is using it. And imagine that for some reason that > > BPF library wants/needs to "export" its map directly. In such case, > > I'd imagine BPF library author to provide a header with pre-defined > > correct extern definition of that map. > > I'm mainly looking at patch 17 and thinking how that copy paste can be avoided. > In C and C++ world the user would do: > defs.h: > struct S { > ... > }; > extern struct S s; > file.c: > #include "defs.h" > struct S s; > and it would work, but afaics it won't work for BPF C in patch 17. Yes, you are right, there is no clean way to avoid defining extern and full map definition. Which is the case for functions and variables, except those type signatures tend to be shorter. E.g., if you had void my_func(int arg) { ... } you'd still have to duplicate it as at least: extern void my_func(int); I don't think you can use typedef for this either. > If the user does: > defs.h: > struct my_map { > __uint(type, BPF_MAP_TYPE_HASH); > __type(key, struct my_key); > __type(value, struct my_value); > __uint(max_entries, 16); > }; > extern struct my_map map1 SEC(".maps"); > file.c: > #include "defs.h" > struct my_map map1; // do we need SEC here too? probably not? yeah, we do, all map "variables" are designated with .maps section, otherwise they'll be treated as just a normal global variable (there is no way to distinguish two, generally speaking). > > It won't work for another_filer.c since max_entries are not allowed? > Why, btw? So the idea was that for consumers of extern map definition map type and key/value info was the only thing they should care about and linker should enforce (at least that's how I thought about this and what I think the typical use case would be). E.g., caring about exact max_entries, or numa_node, or pinning, or map_flags, etc, shouldn't be the concern of the consumer of the map. > > So how the user suppose to do this? With __weak in .h ? > But if that's the only reasonable choice whe bother supporting extern in the linker? > > > I originally wanted to let users define which attributes matter and > > enforce them (as I mention in the commit description), but that > > requires some more work on merging BTF. Now that I'm done with all the > > rest logic, I guess I can go and address that as well. > > I think that would be overkill. It won't match neither C style nor C++. > Let's pick one. So I still think we might want to implement it down the road, but let's stick to something simpler for now. See below. > > > So see above about __weak. As for the BPF library providers, that felt > > unavoidable (and actually desirable), because that's what they would > > do with extern func and extern vars anyways. > > As far as supporting __weak for map defs, I think __weak in one file.c > should be weak for all attributes. Another_file.c should be able > to define the same map name without __weak and different types, value/type > sizes. Because why not? Sort-of C++ style of override. That's a significant deviation from semantics of weak variables and functions, but I can see how it's useful. E.g., we can have some potentially compiled out big map definition, but a __weak fallback to a small, but compatible one. > > > so forcing to type+key+value is to make sure that currently all > > externs (if there are many) are exactly the same. Because as soon as I > > allow some to specify max_entries and some don't, > > I don't get why max_entries is special. > They can be overridden in typical skeleton usage. After open and before load. > So max_entries is a default value in map init. Whether it's part of > extern or not why should that matter? This is just a source code-level contract. You can override any attribute of the map at runtime from user-space code. You can even replace one map with an entirely different map (using bpf_map__reuse_fd()). You can change type, etc, etc. The goal here is to not prevent the abuse (I don't think it's possible at linking stage, but at least BPF verifier will stop from something totally stupid), rather for a typical case to make sure that there is no accidental mismatch at the C code level. > > > Maybe nothing, just there is no single right answer (except the > > aspirational implementation I explained above). I'm open to > > discussion, btw, not claiming my way is the best way. > > I'm not suggesting that extern struct {} my_map; is the right answer either. > Mainly looking into how user code will look like and trying to > make it look the most similar to how C, C++ code traditionally looks. > BPF C is reduced and extended C at the same time. > BPF C++ will be similar. Certain features will be supported right away, > some others will take time. > I'm looking at BTF as a language independent concept. > Both C and C++ will rely on it. > > To summarize if max_entries can be supported and ingored in extern > when the definition has a different value then it's probably good to enforce > that the rest of map fields are the same. Then my .h/.c example above will work. > In case of __weak probably all map fields can change. > It can be seen as a weak definition of the whole map. Not just weak of the variable. > It's a default for everything that can be overridden. > While non-weak can override max_entries only. How about we start in the most restrictive way first. Each extern would need to specify all the attributes that should match the map definition. That includes max_entries. That way the typedef struct { ... } my_map_t re-use will work right out of the box. Later, if we see this is not sufficient, we can start relaxing the rules. Ultimately what I proposed above with selective extern attributes enforcement would allow to cover any scenario. Granted it's special compared to C linking style, but given the __weak map definition proposal above also deviates significantly, we can just treat maps specially, but in a "makes sense" way. > > btw for signed progs I'm thinking to allow override of max_entries only, > since this attribute doesn't affect safety, correctness, behavior. > Meaning max_entries will and will not be part of a signature at the same time. > In other words it's necessary to support existing bcc/libbpf-tools. > If we go with 'allow max_entries in extern' that would match that behavior. Ok, unless I misunderstood, allowing and checking all map attributes as a starting point should work, right?
On 4/15/21 1:35 PM, Andrii Nakryiko wrote: > > How about we start in the most restrictive way first. Each extern > would need to specify all the attributes that should match the map > definition. That includes max_entries. That way the typedef struct { > ... } my_map_t re-use will work right out of the box. Later, if we see > this is not sufficient, we can start relaxing the rules. +1 >> >> btw for signed progs I'm thinking to allow override of max_entries only, >> since this attribute doesn't affect safety, correctness, behavior. >> Meaning max_entries will and will not be part of a signature at the same time. >> In other words it's necessary to support existing bcc/libbpf-tools. >> If we go with 'allow max_entries in extern' that would match that behavior. > > Ok, unless I misunderstood, allowing and checking all map attributes > as a starting point should work, right? yes. thanks!
diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c index 7f9b91760462..9432c125fa43 100644 --- a/tools/lib/bpf/linker.c +++ b/tools/lib/bpf/linker.c @@ -1467,6 +1467,169 @@ static bool glob_sym_btf_matches(const char *sym_name, bool exact, } } +static bool map_defs_match(const char *sym_name, bool full_match, + const struct btf *main_btf, + const struct btf_map_def *main_def, + const struct btf_map_def *main_inner_def, + const struct btf *extra_btf, + const struct btf_map_def *extra_def, + const struct btf_map_def *extra_inner_def) +{ + const char *reason; + + if (main_def->map_type != extra_def->map_type) { + reason = "type"; + goto mismatch; + } + + /* check key type/size match */ + if (main_def->key_size != extra_def->key_size) { + reason = "key_size"; + goto mismatch; + } + if (!!main_def->key_type_id != !!extra_def->key_type_id) { + reason = "key type"; + goto mismatch; + } + if ((main_def->parts & MAP_DEF_KEY_TYPE) + && !glob_sym_btf_matches(sym_name, true /*exact*/, + main_btf, main_def->key_type_id, + extra_btf, extra_def->key_type_id)) { + reason = "key type"; + goto mismatch; + } + + /* validate value type/size match */ + if (main_def->value_size != extra_def->value_size) { + reason = "value_size"; + goto mismatch; + } + if (!!main_def->value_type_id != !!extra_def->value_type_id) { + reason = "value type"; + goto mismatch; + } + if ((main_def->parts & MAP_DEF_VALUE_TYPE) + && !glob_sym_btf_matches(sym_name, true /*exact*/, + main_btf, main_def->value_type_id, + extra_btf, extra_def->value_type_id)) { + reason = "key type"; + goto mismatch; + } + + /* when having non-extern and extern, we don't compare the rest, + * because externs are currently enforced to only specify map type, + * key, and value info + */ + if (!full_match) + return true; + + if (main_def->max_entries != extra_def->max_entries) { + reason = "max_entries"; + goto mismatch; + } + if (main_def->map_flags != extra_def->map_flags) { + reason = "map_flags"; + goto mismatch; + } + if (main_def->numa_node != extra_def->numa_node) { + reason = "numa_node"; + goto mismatch; + } + if (main_def->pinning != extra_def->pinning) { + reason = "pinning"; + goto mismatch; + } + + if ((main_def->parts & MAP_DEF_INNER_MAP) != (extra_def->parts & MAP_DEF_INNER_MAP)) { + reason = "inner map"; + goto mismatch; + } + + if (main_def->parts & MAP_DEF_INNER_MAP) { + char inner_map_name[128]; + + snprintf(inner_map_name, sizeof(inner_map_name), "%s.inner", sym_name); + + return map_defs_match(inner_map_name, true /*full_match*/, + main_btf, main_inner_def, NULL, + extra_btf, extra_inner_def, NULL); + } + + return true; + +mismatch: + pr_warn("global '%s': map %s mismatch\n", sym_name, reason); + return false; +} + +#define MAP_DEF_EXTERN_PARTS (MAP_DEF_MAP_TYPE \ + | MAP_DEF_KEY_SIZE | MAP_DEF_KEY_TYPE \ + | MAP_DEF_VALUE_SIZE | MAP_DEF_VALUE_TYPE) + +static bool glob_map_defs_match(const char *sym_name, + struct bpf_linker *linker, struct glob_sym *glob_sym, + struct src_obj *obj, Elf64_Sym *sym, int btf_id) +{ + bool sym_is_extern = sym->st_shndx == SHN_UNDEF; + struct btf_map_def dst_def = {}, dst_inner_def = {}; + struct btf_map_def src_def = {}, src_inner_def = {}; + const struct btf_type *t; + int err; + + t = btf__type_by_id(obj->btf, btf_id); + if (!btf_is_var(t)) { + pr_warn("global '%s': invalid map definition type [%d]\n", sym_name, btf_id); + return false; + } + t = skip_mods_and_typedefs(obj->btf, t->type, NULL); + + err = parse_btf_map_def(sym_name, obj->btf, t, true /*strict*/, &src_def, &src_inner_def); + if (err) { + pr_warn("global '%s': invalid map definition\n", sym_name); + return false; + } + + /* We restict extern map defs to only specify map type and key/value + * type or size. Inner map definitions are prohibited for now as well. + */ + if (sym_is_extern && (src_def.parts & ~MAP_DEF_EXTERN_PARTS)) { + pr_warn("global '%s': extern map can specify only map type and key/value info\n", + sym_name); + return false; + } + + /* re-parse existing map definition */ + t = btf__type_by_id(linker->btf, glob_sym->btf_id); + t = skip_mods_and_typedefs(linker->btf, t->type, NULL); + err = parse_btf_map_def(sym_name, linker->btf, t, true /*strict*/, &dst_def, &dst_inner_def); + if (err) { + /* this should not happen, because we already validated it */ + pr_warn("global '%s': invalid dst map definition\n", sym_name); + return false; + } + + if (glob_sym->is_extern != sym_is_extern) { + /* extern map def should be a subset of non-extern one */ + if (sym_is_extern) + /* existing map def is the main one */ + return map_defs_match(sym_name, false /*full_match*/, + linker->btf, &dst_def, &dst_inner_def, + obj->btf, &src_def, &src_inner_def); + else + /* new map def is the main one */ + return map_defs_match(sym_name, false /*full_match*/, + obj->btf, &src_def, &src_inner_def, + linker->btf, &dst_def, &dst_inner_def); + } else { + /* map defs should match exactly regardless of extern/extern + * or non-extern/non-extern case + */ + return map_defs_match(sym_name, true /*full_match*/, + linker->btf, &dst_def, &dst_inner_def, + obj->btf, &src_def, &src_inner_def); + } +} + static bool glob_syms_match(const char *sym_name, struct bpf_linker *linker, struct glob_sym *glob_sym, struct src_obj *obj, Elf64_Sym *sym, size_t sym_idx, int btf_id) @@ -1488,6 +1651,10 @@ static bool glob_syms_match(const char *sym_name, return false; } + /* deal with .maps definitions specially */ + if (glob_sym->sec_id && strcmp(linker->secs[glob_sym->sec_id].sec_name, MAPS_ELF_SEC) == 0) + return glob_map_defs_match(sym_name, linker, glob_sym, obj, sym, btf_id); + if (!glob_sym_btf_matches(sym_name, true /*exact*/, linker->btf, glob_sym->btf_id, obj->btf, btf_id)) return false;
Add extra logic to handle map externs (only BTF-defined maps are supported for linking). Re-use the map parsing logic used during bpf_object__open(). Map externs are currently restricted to always and only specify map type, key type and/or size, and value type and/or size. Nothing extra is allowed. If any of those attributes are mismatched between extern and actual map definition, linker will report an error. The original intent was to allow for extern to specify attributes that matters (to user) to enforce. E.g., if you specify just key information and omit value, then any value fits. Similarly, it should have been possible to enforce map_flags, pinning, and any other possible map attribute. Unfortunately, that means that multiple externs can be only partially overlapping with each other, which means linker would need to combine their type definitions to end up with the most restrictive and fullest map definition. This requires an extra amount of BTF manipulation which at this time was deemed unnecessary and would require further extending generic BTF writer APIs. So that is left for future follow ups, if there will be demand for that. But the idea seems intresting and useful, so I want to document it here. Otherwise extern maps behave intuitively, just like extern vars and funcs. Weak definitions are also supported. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/lib/bpf/linker.c | 167 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+)