On Tue, Feb 13, 2024 at 3:37 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Tue, 2024-02-13 at 15:17 -0800, Andrii Nakryiko wrote:
>
> [...]
>
> > > So, at first I thought that having two maps is a bit of a hack.
> >
> > yep, that was my instinct as well
> >
> > > However, after trying to make it work with only one map I don't really
> > > like that either :)
> >
> > Can you elaborate? see my reply to Alexei, I wonder how did you think
> > about doing this?
>
> Relocations in the ELF file are against a new section: ".arena.1".
> This works nicely with logic in bpf_program__record_reloc().
> If single map is used, we effectively need to track two indexes for
> the map section:
> - one used for relocations against map variables themselves
> (named "generic map reference relocation" in the function code);
> - one used for relocations against ".arena.1"
> (named "global data map relocation" in the function code).
>
> This spooked me off:
> - either bpf_object__init_internal_map() would have a specialized
> branch for arenas, as with current approach;
> - or bpf_program__record_reloc() would have a specialized branch for arenas,
> as with one map approach.
Yes, relocations would know about .arena.1, but it's a pretty simple
check in a few places. We basically have arena *definition* sec_idx
(corresponding to SEC(".maps")) and arena *data* sec_idx. The latter
is what is recorded for global variables in .arena.1. We can remember
this arena data sec_idx in struct bpf_object once during ELF
processing, and then just special case it internally in a few places.
The "fake" bpf_map for __arena_internal is user-visible and requires
autocreate=false tricks, etc. I feel like it's a worse tradeoff from a
user API perspective than a few extra ARENA-specific internal checks
(which we already have a few anyways, ARENA is not completely
transparent internally anyways).
>
> Additionally, skel generation logic currently assumes that mmapable
> bindings would be generated only for internal maps,
> but that is probably not a big deal.
yeah, it's not, we will have STRUCT_OPS maps handled special anyways
(Kui-Feng posted an RFC already), so ARENA won't be the only one
special case
