Message-ID: <CAEf4Bzar-KgCjUEfKVeWzcB77xvXDagZFRQKDvWo1o9-JzCirw@mail.gmail.com>
Date:   Fri, 2 Aug 2019 00:19:29 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Björn Töpel <bjorn.topel@...il.com>
Cc:     Kevin Laatz <kevin.laatz@...el.com>,
        Netdev <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Björn Töpel <bjorn.topel@...el.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Maxim Mikityanskiy <maximmi@...lanox.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Bruce Richardson <bruce.richardson@...el.com>,
        ciara.loftus@...el.com,
        intel-wired-lan <intel-wired-lan@...ts.osuosl.org>,
        bpf <bpf@...r.kernel.org>
Subject: Re: [Intel-wired-lan] [PATCH bpf-next v4 03/11] libbpf: add flags to
 umem config

On Thu, Aug 1, 2019 at 12:34 AM Björn Töpel <bjorn.topel@...il.com> wrote:
>
> On Thu, 1 Aug 2019 at 08:59, Andrii Nakryiko <andrii.nakryiko@...il.com> wrote:
> >
> > On Wed, Jul 31, 2019 at 8:21 AM Björn Töpel <bjorn.topel@...il.com> wrote:
> > >
> > > On Tue, 30 Jul 2019 at 19:43, Kevin Laatz <kevin.laatz@...el.com> wrote:
> > > >
> > > > This patch adds a 'flags' field to the umem_config and umem_reg structs.
> > > > This will allow for more options to be added for configuring umems.
> > > >
> > > > The first use for the flags field is to add a flag for unaligned chunks
> > > > mode. These flags can either be user-provided or filled with a default.
> > > >
> > > > Signed-off-by: Kevin Laatz <kevin.laatz@...el.com>
> > > > Signed-off-by: Ciara Loftus <ciara.loftus@...el.com>
> > > >
> > > > ---
> > > > v2:
> > > >   - Removed the headroom check from this patch. It has moved to the
> > > >     previous patch.
> > > >
> > > > v4:
> > > >   - modified chunk flag define
> > > > ---
> >
> > [...]
> >
> > > > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> > > > index 833a6e60d065..44a03d8c34b9 100644
> > > > --- a/tools/lib/bpf/xsk.h
> > > > +++ b/tools/lib/bpf/xsk.h
> > > > @@ -170,12 +170,14 @@ LIBBPF_API int xsk_socket__fd(const struct xsk_socket *xsk);
> > > >  #define XSK_UMEM__DEFAULT_FRAME_SHIFT    12 /* 4096 bytes */
> > > >  #define XSK_UMEM__DEFAULT_FRAME_SIZE     (1 << XSK_UMEM__DEFAULT_FRAME_SHIFT)
> > > >  #define XSK_UMEM__DEFAULT_FRAME_HEADROOM 0
> > > > +#define XSK_UMEM__DEFAULT_FLAGS 0
> > > >
> > > >  struct xsk_umem_config {
> > > >         __u32 fill_size;
> > > >         __u32 comp_size;
> > > >         __u32 frame_size;
> > > >         __u32 frame_headroom;
> > > > +       __u32 flags;
> > >
> > > And the flags addition here, unfortunately, requires symbol versioning
> > > of xsk_umem__create(). That'll be the first in libbpf! :-)
> >
> > xsk_umem_config is passed by pointer to xsk_umem__create(), so this
> > doesn't break ABI, does it?
> >
>
> Old application, dynamically linked to new libbpf.so will crash,
> right? Old application passes old version of xsk_umem_config, and new
> library accesses (non-existing) flag struct member.

I think we have similar problems for all the _xattr type of commands
(as well some of btf stuff accepting extra opts structs). How is this
problem solved in general? Do we version same function multiple times,
one for each added field? It feels like there should be some better
way to handle this...

>
>
> Björn
>
>
> > >
> > >
> > > Björn
> > >
> > > >  };
> > > >
> > > >  /* Flags for the libbpf_flags field. */
> > > > --
> > > > 2.17.1
> > > >
> > > > _______________________________________________
> > > > Intel-wired-lan mailing list
> > > > Intel-wired-lan@...osl.org
> > > > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Powered by blists - more mailing lists