On Mon, Oct 01, 2018 at 08:11:43AM -0500, Mauricio Vasquez wrote:
> > > > +BPF_CALL_3(bpf_map_pop_elem, struct bpf_map *, map, void *,
> > > > value, u32, size)
> > > > +{
> > > > + void *ptr;
> > > > +
> > > > + if (map->value_size != size)
> > > > + return -EINVAL;
> > > > +
> > > > + ptr = map->ops->map_lookup_and_delete_elem(map, NULL);
> > > > + if (!ptr)
> > > > + return -ENOENT;
> > > > +
> > > > + switch (size) {
> > > > + case 1:
> > > > + *(u8 *) value = *(u8 *) ptr;
> > > > + break;
> > > > + case 2:
> > > > + *(u16 *) value = *(u16 *) ptr;
> > > > + break;
> > > > + case 4:
> > > > + *(u32 *) value = *(u32 *) ptr;
> > > > + break;
> > > > + case 8:
> > > > + *(u64 *) value = *(u64 *) ptr;
> > > > + break;
> > > this is inefficient. can we pass value ptr into ops and let it
> > > populate it?
> >
> > I don't think so, doing that implies that look_and_delete will be a
> > per-value op, while other ops in maps are per-reference.
> > For instance, how to change it in the case of peek helper that is using
> > the lookup operation?, we cannot change the signature of the lookup
> > operation.
> >
> > This is something that worries me a little bit, we are creating new
> > per-value helpers based on already existing per-reference operations,
> > this is not probably the cleanest way. Here we are at the beginning of
> > the discussion once again, how should we map helpers and syscalls to
> > ops.
> >
> > What about creating pop/peek/push ops, mapping helpers one to one and
> > adding some logic into syscall.c to call the correct operation in case
> > the map is stack/queue?
> > Syscall mapping would be:
> > bpf_map_lookup_elem() -> peek
> > bpf_map_lookup_and_delete_elem() -> pop
> > bpf_map_update_elem() -> push
> >
> > Does it make sense?
>
> Hello Alexei,
>
> Do you have any feedback on this specific part?
Indeed. It seems push/pop ops will be cleaner.
I still think that peek() is useless due to races.
So BPF_MAP_UPDATE_ELEM syscall cmd will map to 'push' ops
and new BPF_MAP_LOOKUP_AND_DELETE_ELEM will map to 'pop' ops.
right?
