Message-ID: <20180705101800.3c5d6af0@cakuba.netronome.com>
Date:   Thu, 5 Jul 2018 10:18:23 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Saeed Mahameed <saeedm@...lanox.com>,
        "alexei.starovoitov@...il.com" <alexei.starovoitov@...il.com>,
        "saeedm@....mellanox.co.il" <saeedm@....mellanox.co.il>,
        "alexander.h.duyck@...el.com" <alexander.h.duyck@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Tariq Toukan <tariqt@...lanox.com>,
        "john.fastabend@...il.com" <john.fastabend@...il.com>,
        "brouer@...hat.com" <brouer@...hat.com>,
        "borkmann@...earbox.net" <borkmann@...earbox.net>,
        "peter.waskiewicz.jr@...el.com" <peter.waskiewicz.jr@...el.com>
Subject: Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure

On Wed, 4 Jul 2018 09:51:54 +0200, Daniel Borkmann wrote:
> On 07/04/2018 02:57 AM, Saeed Mahameed wrote:
> > On Tue, 2018-07-03 at 16:01 -0700, Alexei Starovoitov wrote:  
> >> How about we make driver+firmware provide a BTF definition of
> >> metadata that they
> >> can provide? There can be multiple definitions of such structs.
> >> Then in userpsace we can have BTF->plain C converter.
> >> (bpftool practically ready to do that already).
> >> Then the programmer can take such generated C definition, add it to
> >> .h and include
> >> it in their programs. llvm will compile the whole thing and will
> >> include BTF
> >> of maps, progs and this md struct in the target elf file.
> >> During loading the kernel can check that BTF in elf is matching one-
> >> to-one
> >> to what driver+firmware are saying they support.  
> 
> I do like the above idea of utilizing BTF for this, seems like a good fit.
>
> > Just thinking out loud, can't we do this at program load ? just run a
> > setup function in the xdp program to load nic md BTF definition into
> > the elf section ?
> >   
> >> No ambiguity and no possibility of mistake, since offsets and field
> >> names
> >> are verified.  
> > 
> > But what about the dynamic nature of this feature ? Sometimes you only
> > want HW/Driver to provide a subset of whatever the HW can provide and
> > save md buffer for other stuff.
> > 
> > Yes a well defined format is favorable here, but we need to make sure
> > there is no computational overhead in data path just to extract each
> > field! for example if i want to know what is the offset of the hash
> > will i need to go parse (for every packet) the whole BTF definition of
> > metadata just to find the offset of type=hash ?  
> 
> I don't think this would be the case that you'd need to walk BTF in fast
> path here. In the ideal case, the only thing that driver would need to do
> in fast path would be to set proper xdp->data_meta offset and _that_ would
> be it. For the rest, program would know how to access the data since it's
> already aware of it from BTF definition the driver provided. Other drivers
> which would be less flexible on that regard would internally prep the buffer
> based on the progs needs more or less similar as in mlx5e_xdp_fill_data_meta(),
> but it would be really up to the driver how to handle this internally. The
> BTF it would check at XDP setup time to do the configuration needed in the
> driver. Verifier would only check BTF, pass it along for XDP setup, prog
> rewrites in verifier aren't even needed since LLVM compiled everything
> already.

I don't think we should force drivers to place such meta data in the
buffer.  The moment that happens we loose the "zero-touch" abilities
Jesper was trying to achieve.

Besides what happens to the meta data after XDP is finished.  We really
need the ability to communicate the modified fields further to the
stack.  With meta data in the buffer we don't really know if the
information place there after XDP finishes is still valid or did the
program overwrite it with something completely different.

I'm also not 100% on board with the argument that "future" FW can
reshuffle things whatever way it wants to.  Is the assumption that
future ASICs/FW will be designed to always use the "blessed" BTF
format?  Or will it be reconfigurable at runtime?

> >> Every driver can have their own BTF for md and their own special
> >> features.
> >> We can try to standardize the names (like vlan and csum), so xdp
> >> programs
> >> can stay relatively portable across NICs.  
> > 
> > Yes this is a must.  
> 
> Agree, there needs to be a basic common set that would be provided by
> every XDP aware driver.

I'm sorry to bring this up again, but can we really not let drivers
define their own "get_XYZ/set_XYZ" helpers, and link those to the
program at attachment time?  Sure we'd have to create a new copy of the
program for each driver it's used with, but is that really a problem?
That'd have the lowest impact on the performance and complexity of the
driver fast path.  The BTF solution already has all the same problems
WRT tail calls and not being sure which driver the program is attached
to.

> >> Such api will address exposing asic+firmware metadata to the xdp
> >> program.
> >> Once we tackle this problem, we'll think how to do the backward
> >> config
> >> (to do firmware reconfig for specific BTF definition of md supplied
> >> by the prog).
> >> What people think?  
> > 
> > For legacy HW, we can do it already in the driver, provide whatever the
> > prog requested, its only a matter of translation to the BTF format in
> > the driver xdp setup and pushing the values accordingly into the md
> > offsets on data path.
> > 
> > Question: how can you share the md BTF from the driver/HW with the xdp
> > program ?
> 
> I think this would likely be a new query as in XDP_QUERY_META_BTF
> implemented in ndo_bpf callback and then exported e.g. via bpf(2)
> or netlink such that bpftool can generate BTF -> C from there for the
> program to include later in compilation.

Powered by blists - more mailing lists