Re: HASH_OF_MAPS inner map allocation from BPF

[Borna Cafuk <borna.cafuk@sartura.hr>]

On Wed, Sep 9, 2020 at 12:36 PM KP Singh <kpsingh@google.com> wrote:
>
> On Wed, Sep 9, 2020 at 12:24 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >
> > Borna Cafuk <borna.cafuk@sartura.hr> writes:
> >
> > > On Mon, Sep 7, 2020 at 3:33 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >>
> > >> Borna Cafuk <borna.cafuk@sartura.hr> writes:
> > >>
> > >> > On Sat, Sep 5, 2020 at 12:47 AM Alexei Starovoitov
> > >> > <alexei.starovoitov@gmail.com> wrote:
>
> [...]
>
> > >> >
> > >> > The idea is to have an outer map where the keys are PIDs, and inner maps where
> > >> > the keys are system call numbers. This would enable tracking the number of
> > >> > syscalls made by each process and the makeup of those calls for all processes
> > >> > simultaneously.
> > >> >
> > >> > [1] https://github.com/iovisor/bcc/blob/master/libbpf-tools/syscount.bpf.c
> > >>
> > >> Well, if you just want to count, map-in-map seems a bit overkill? You
> > >> could just do:
> > >>
> > >> struct {
> > >>   u32 pid;
> > >>   u32 syscall;
> > >> } map_key;
> > >>
> > >> and use that?
> > >>
> > >> -Toke
> > >>
> > >
> > > I have considered that, but maps in maps seem better for when I need to get the
> > > data about a single process's syscalls: It requires reading only one of the
> > > inner maps in its entirety. If I have a composite key like that, I don't see
> > > any way, other than:
> > >  * either iterating through all the possible keys for a process
> > >    (i.e. over all syscalls) and looking them up in the map, or
> > >  * iterating over all entries in the map and filtering them.
> > >
> > > Looking at it again, the first option does not seem _that_ bad,
> >
> > You could even use BPF_MAP_LOOKUP_BATCH to do this in one operation, I
> > suppose...
> >
> > > but just iterating over one (inner) map would be easier to fit into
> > > our use-case.
> >
> > ...but yeah, I see what you mean. Well, maybe BPF local storage per
> > process would also be a nice fit here?

Thank you for the insight.

>
> Yes, task local storage does seem like a good fit and is the next one I was
> thinking of implementing.
>
> - KP

I'm looking forward to the patches.

>
> >
> > -Toke
> >