netdev - Re: [PATCH bpf-next 1/3] bpf: add bpf queue map

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 8 Aug 2018 21:48:08 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Mauricio Vasquez <mauricio.vasquez@...ito.it>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next 1/3] bpf: add bpf queue map

On Wed, Aug 08, 2018 at 10:08:47PM -0500, Mauricio Vasquez wrote:
> 
> > And how about adding three new helpers: push/pop/peek as well?
> > Reusing lookup/update is neat, but does lookup == pop
> > or does lookup == peek ?
> > I suspect it will be confusing.
> > Three new helpers cost nothing, but will make bpf progs easier to read.
> I agree. I have one doubt here, update/lookup/delete is implemented in all
> map types, if the operation is not supported it returns -EINVAL.
> For push/pop/peek, should we implement it in all maps or is it better to
> check the map type before invoking map->ops->push/pop/seek?
> (Maybe checking if map->ops->xxx is NULL will also work)

Since push/pop/peak are only for this queue/stack I thought we won't
be adding 'ops' for them and just call the helpers from progs.
But now I'm having second thoughts, since 3 new commands for syscall
feels like overkill.
At the same time I still don't feel that lookup == pop is the right alias.
Also what peak is going to alias to ?
May be let's go back to your original idea with a tweak:
push == update
peak == lookup
pop = lookup + delete
In other words in case of stack the bpf_map_lookup_elem will return
the pointer to value of top element in the stack and
bpf_map_delete_elem will delete that top element.
Then in user space we can introduce push/pop always_inline functions like:
void bpf_push(struct bpf_map *map, void *value)
{
  bpf_map_update_elem(map, NULL/*key*/, value);
}

void *bpf_pop(struct bpf_map *map)
{
  void * val = bpf_map_lookup_elem(map, NULL/*key*/);
  bpf_map_delete_elem(map, NULL/*key*/);
  return val;
}

Thoughts?