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?
