Gilad Reti <gilad.reti@xxxxxxxxx> writes:
> On Mon, Feb 8, 2021 at 7:55 PM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>>
>> Gilad Reti <gilad.reti@xxxxxxxxx> writes:
>>
>> > On Mon, Feb 8, 2021 at 5:09 PM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>> >>
>> >> Gilad Reti <gilad.reti@xxxxxxxxx> writes:
>> >>
>> >> > On Mon, Feb 8, 2021 at 4:28 PM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>> >> >>
>> >> >> Gilad Reti <gilad.reti@xxxxxxxxx> writes:
>> >> >>
>> >> >> > On Mon, Feb 8, 2021 at 1:42 PM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>> >> >> >>
>> >> >> >> Gilad Reti <gilad.reti@xxxxxxxxx> writes:
>> >> >> >>
>> >> >> >> > Also, is there a way to set the pin path to all maps/programs at once?
>> >> >> >> > For example, bpf_object__pin_maps pins all maps at a specific path,
>> >> >> >> > but as far as I was able to find there is no similar function to set
>> >> >> >> > the pin path for all maps only (without pinning) so that at loading
>> >> >> >> > time libbpf will try to reuse all maps. The only way to achieve a
>> >> >> >> > complete reuse of all maps that I could find is to "reverse engineer"
>> >> >> >> > libbpf's pin path generation algorithm (i.e. <path>/<map_name>) and
>> >> >> >> > set the pin path on each map before load.
>> >> >> >>
>> >> >> >> You can set the 'pinning' attribute in the map definition - add
>> >> >> >> '__uint(pinning, LIBBPF_PIN_BY_NAME);' to the map struct. By default
>> >> >> >> this will pin beneath /sys/fs/bpf, but you can customise that by setting
>> >> >> >> the pin_root_path attribute in bpf_object_open_opts.
>> >> >> >
>> >> >> > Yes, I am familiar with that feature, but it has some downsides:
>> >> >> > 1. I need to set it manually on every map (and in cases that I have
>> >> >> > only the compiled object file that would be hard).
>> >> >> > 2. It only works for bpf maps and not bpf programs.
>> >> >> > 3. It only works for bpf maps that are defined explicitly in the bpf
>> >> >> > code and not for implicit (inner) bpf maps (bss, rodata, etc).
>> >> >>
>> >> >> Ah, right. Well, other than that I don't think there's a way to set pin
>> >> >> paths in bulk, other than by manually iterating and setting them one at
>> >> >> a time. But, erm, can't you just do that? :)
>> >> >>
>> >> >
>> >> > Sure, I can, but I think we should avoid that. As I said this forces
>> >> > the user to know libbpf's pin path naming algorithm, which is not part
>> >> > of the libbpf api afaik.
>> >>
>> >> Why? If you set the pin path from your application, libbpf will also try
>> >> to reuse the map from that path. So you don't need to know libbpf's
>> >> algorithm if you just override it with your own paths?
>> >>
>> >
>> > If I do bpf_object__pin_maps then libbpf decides where it wants to pin
>> > them. I can set each path by my own, but then why do we need this
>> > function?
>>
>> Erm, what do you mean, "libbpf decides". bpf_object__pin_maps(obj, path)
>> does exactly what you're asking for: If you supply the path, all maps
>> are going to be pinned by name underneath that directory...
>
> They are pinned under this directory, but with which filename? Today
> libbpf builds the filename by taking the map name and escaping it, but
> what will happen if this will have to change?
Then that would have to be done in a way that was backwards-compatible
so as not to break user code :)
>> > For example, libbpf today uses <path>/<map_name> as the pin path, but
>> > it is also doing sanitize_pin_path on each path. This means that after
>> > if use bpf_object__pin_maps I also need to know how libbpf sanitizes
>> > its paths and mimic that behavior on my side.
>>
>> The paths are sanitised so the kernel will accept them. If you're using
>> invalid paths your pinning is not going to work at all. If you just want
>> the paths that the maps are pinned under, use bpf_map__get_pin_path().
>>
>
> I am not saying that sanitization is redundant, but rather that it
> needs to be properly defined (i.e. all dots will always be replaced
> with underscores), so either expose it in the api or document it so
> that users don't have to look after the specific implementation.
Lack of documentation is a perennial problem, and patches are always
welcome. But it is API: libbpf does things a certain way today, and if
it changes in a way that will break user programs, that is an API break.
>> >> > I think that if we have a method to pin all maps at a specific path
>> >> > there should also be a method for reusing them all from this path,
>> >> > either by exposing the function that builds the pin path, or a
>> >> > function that sets all the paths from a root path.
>> >>
>> >> What you're asking for is basically a function
>> >> bpf_object__set_all_pin_paths(obj, path)
>> >>
>> >> instead of having to do
>> >>
>> >> bpf_object__for_each_map(map, obj) {
>> >> sprintf(path, "path/%s", bpf_map__name(map));
>> >> bpf_map__set_pin_path(map, path);
>> >> }
>> >>
>> >> or? Is that really needed?
>> >>
>> >
>> > Yes, that is what I am asking for. Either that or a
>> > bpf_map__build_pin_path(path, map) That will return a pin path that is
>> > compatible with libbpf's one, and then I can iterate over all maps.
>>
>> See above; this is what bpf_object__pin_maps() does today?
>
> I didn't get this last comment. What I meant is that I want something
> like the bpf_object__pin_maps but that doesn't pin the maps, just
> exposing its naming part.
Right, OK. Why, though? I can kinda see how it could be convenient to
(basically )make libbpf behave as if all maps has the 'pinning'
attribute set, for map reuse. But I'm not sure I can think any concrete
use cases where this would be needed. What's yours?
-Toke
