From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Subject: Re: [PATCH bpf-next 1/3] bpf: add bpf queue map
Date: Wed, 8 Aug 2018 21:48:08 -0700
Message-ID: <20180809044806.bbvzyp3sqh6orcnl@ast-mbp>
References: <153356387977.6981.12236150594041620482.stgit@kernel>
 <153356390770.6981.4228793745105954649.stgit@kernel>
 <20180807144226.pmalhe3mjvc3a45y@ast-mbp.dhcp.thefacebook.com>
 <cf74d278-180b-1db1-4f5b-10f4177a4361@polito.it>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>, netdev@vger.kernel.org
To: Mauricio Vasquez <mauricio.vasquez@polito.it>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pl0-f66.google.com ([209.85.160.66]:44453 "EHLO
        mail-pl0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1727211AbeHIHLF (ORCPT
        <rfc822;netdev@vger.kernel.org>); Thu, 9 Aug 2018 03:11:05 -0400
Received: by mail-pl0-f66.google.com with SMTP id ba4-v6so1995658plb.11
        for <netdev@vger.kernel.org>; Wed, 08 Aug 2018 21:48:11 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <cf74d278-180b-1db1-4f5b-10f4177a4361@polito.it>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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?