[bpf,0/3] bpf: avoid using/returning file descriptor value zero

Make it easier to handle UAPI/kABI extensions by avoid BPF using/returning
file descriptor value zero. Use this in recent devmap extension to keep
older applications compatible with newer kernels.

For special type maps (e.g. devmap and cpumap) the map-value data-layout is
a configuration interface. This is a kernel Application Binary Interface
(kABI) that can only be tail extended. Thus, new members (and thus features)
can only be added to the end of this structure, and the kernel uses the
map->value_size from userspace to determine feature set 'version'.

For this kind of kABI to be extensible and backward compatible, is it common
that new members/fields (that represent a new feature) in the struct are
initialised as zero, which indicate that the feature isn't used. This makes
it possible to write userspace applications that are unaware of new kernel
features, but just include latest uapi headers, zero-init struct and
populate features it knows about.

The recent extension of devmap with a bpf_prog.fd requires end-user to
supply the file-descriptor value minus-1 to communicate that the features
isn't used. This isn't compatible with the described kABI extension model.

---

Jesper Dangaard Brouer (3):
      bpf: syscall to start at file-descriptor 1
      bpf: devmap adjust uapi for attach bpf program
      bpf: selftests and tools use struct bpf_devmap_val from uapi


 fs/file.c                                          |    2 +
 include/linux/file.h                               |    1 +
 include/uapi/linux/bpf.h                           |   13 +++++++
 kernel/bpf/devmap.c                                |   17 ++-------
 kernel/bpf/syscall.c                               |   38 +++++++++++++++++---
 tools/include/uapi/linux/bpf.h                     |   13 +++++++
 .../selftests/bpf/prog_tests/xdp_devmap_attach.c   |    8 ----
 .../selftests/bpf/progs/test_xdp_devmap_helpers.c  |    2 +
 .../bpf/progs/test_xdp_with_devmap_helpers.c       |    3 +-
 9 files changed, 66 insertions(+), 31 deletions(-)

--

On Mon, Jun 08, 2020 at 06:51:12PM +0200, Jesper Dangaard Brouer wrote:
> Make it easier to handle UAPI/kABI extensions by avoid BPF using/returning
> file descriptor value zero. Use this in recent devmap extension to keep
> older applications compatible with newer kernels.
> 
> For special type maps (e.g. devmap and cpumap) the map-value data-layout is
> a configuration interface. This is a kernel Application Binary Interface
> (kABI) that can only be tail extended. Thus, new members (and thus features)
> can only be added to the end of this structure, and the kernel uses the
> map->value_size from userspace to determine feature set 'version'.

please drop these kabi references. As far as I know kabi is a redhat invention
and I'm not even sure what exactly it means.
'struct bpf_devmap_val' is uapi. No need to invent new names for existing concept.

> The recent extension of devmap with a bpf_prog.fd requires end-user to
> supply the file-descriptor value minus-1 to communicate that the features
> isn't used. This isn't compatible with the described kABI extension model.

non-zero prog_fd requirement exists already in bpf syscall. It's not recent.
So I don't think patch 1 is appropriate at this point. Certainly not
for bpf tree. We can argue about it usefulness when bpf-next reopens.
For now I think patches 2 and 3 are good to go.
Don't delete 'enum sk_action' in patch 2 though.
The rest looks good to me.
Thanks!

On Mon, 8 Jun 2020 18:34:10 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Mon, Jun 08, 2020 at 06:51:12PM +0200, Jesper Dangaard Brouer wrote:
> > Make it easier to handle UAPI/kABI extensions by avoid BPF using/returning
> > file descriptor value zero. Use this in recent devmap extension to keep
> > older applications compatible with newer kernels.
> > 
> > For special type maps (e.g. devmap and cpumap) the map-value data-layout is
> > a configuration interface. This is a kernel Application Binary Interface
> > (kABI) that can only be tail extended. Thus, new members (and thus features)
> > can only be added to the end of this structure, and the kernel uses the
> > map->value_size from userspace to determine feature set 'version'.  
> 
> please drop these kabi references. As far as I know kabi is a redhat invention
> and I'm not even sure what exactly it means.
> 'struct bpf_devmap_val' is uapi. No need to invent new names for existing concept.

Sure I can call it UAPI.

I was alluding to the difference between API and ABI, but it doesn't matter.
For the record, Red Hat didn't invent ABI (Application Binary Interface):
 https://en.wikipedia.org/wiki/Application_binary_interface


> > The recent extension of devmap with a bpf_prog.fd requires end-user to
> > supply the file-descriptor value minus-1 to communicate that the features
> > isn't used. This isn't compatible with the described kABI extension model.  
> 
> non-zero prog_fd requirement exists already in bpf syscall. It's not recent.
> So I don't think patch 1 is appropriate at this point. Certainly not
> for bpf tree. We can argue about it usefulness when bpf-next reopens.
> For now I think patches 2 and 3 are good to go.

Great.

> Don't delete 'enum sk_action' in patch 2 though.

Sorry, yes, that was a mistake.

> The rest looks good to me.

Thanks!

For special type maps (e.g. devmap and cpumap) the map-value data-layout is
a configuration interface. This is uapi that can only be tail extended.
Thus, new members (and thus features) can only be added to the end of this
structure, and the kernel uses the map->value_size from userspace to
determine feature set 'version'.

For this kind of uapi to be extensible and backward compatible, is it common
that new members/fields (that represent a new feature) in the struct are
initialized as zero, which indicate that the feature isn't used. This makes
it possible to write userspace applications that are unaware of new kernel
features, but just include latest uapi headers, zero-init struct and
populate features it knows about.

The recent extension of devmap with a bpf_prog.fd requires end-user to
supply the file-descriptor value minus-1 to communicate that the features
isn't used. This isn't compatible with the described kABI extension model.

V2: Drop patch-1 that changed BPF-syscall to start at file-descriptor 1

---

Jesper Dangaard Brouer (2):
      bpf: devmap adjust uapi for attach bpf program
      bpf: selftests and tools use struct bpf_devmap_val from uapi


 include/uapi/linux/bpf.h                           |   13 +++++++++++++
 kernel/bpf/devmap.c                                |   17 ++++-------------
 tools/include/uapi/linux/bpf.h                     |   13 +++++++++++++
 .../selftests/bpf/prog_tests/xdp_devmap_attach.c   |    8 --------
 .../selftests/bpf/progs/test_xdp_devmap_helpers.c  |    2 +-
 .../bpf/progs/test_xdp_with_devmap_helpers.c       |    3 +--
 6 files changed, 32 insertions(+), 24 deletions(-)

--