Message-ID: <20180706174043.14a8145a@cakuba.netronome.com>
Date:   Fri, 6 Jul 2018 17:40:43 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     "Waskiewicz Jr, Peter" <peter.waskiewicz.jr@...el.com>
Cc:     "alexei.starovoitov@...il.com" <alexei.starovoitov@...il.com>,
        "Duyck, Alexander H" <alexander.h.duyck@...el.com>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "saeedm@...lanox.com" <saeedm@...lanox.com>,
        "brouer@...hat.com" <brouer@...hat.com>,
        "borkmann@...earbox.net" <borkmann@...earbox.net>,
        "tariqt@...lanox.com" <tariqt@...lanox.com>,
        "john.fastabend@...il.com" <john.fastabend@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "saeedm@....mellanox.co.il" <saeedm@....mellanox.co.il>
Subject: Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure

On Fri, 6 Jul 2018 23:49:55 +0000, Waskiewicz Jr, Peter wrote:
> On Fri, 2018-07-06 at 16:38 -0700, Alexei Starovoitov wrote:
> > On Fri, Jul 06, 2018 at 08:44:24PM +0000, Waskiewicz Jr, Peter wrote:  
> > > On Fri, 2018-07-06 at 09:30 -0700, Alexei Starovoitov wrote:  
> > > > On Thu, Jul 05, 2018 at 10:18:23AM -0700, Jakub Kicinski wrote:  
> > > > > 
> > > > > 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?  
> > > > 
> > > > let's table configuration of metadata aside for a second.  
> > > 
> > > I agree that this should/could be NIC-specific and shouldn't weigh
> > > on
> > > the metadata interface between the drivers and XDP layer.
> > >   
> > > > Describing metedata layout in BTF allows NICs to disclose
> > > > everything
> > > > NIC has to users in a standard and generic way.
> > > > Whether firmware is reconfigurable on the fly or has to reflashed
> > > > and hw powercycled to have new md layout (and corresponding BTF
> > > > description)
> > > > is a separate discussion.
> > > > Saeed's proposal introduces the concept of 'offset' inside
> > > > 'struct
> > > > xdp_md_info'
> > > > to reach 'hash' value in metadata.
> > > > Essentially it's a run-time way to access 'hash' instead of
> > > > build-
> > > > time.
> > > > So bpf program would need two loads to read csum or hash field
> > > > instead of one.
> > > > With BTF the layout of metadata is known to the program at build-
> > > > time.
> > > > 
> > > > To reiterate the proposal:
> > > > - driver+firmware keep layout of the metadata in BTF format
> > > > (either
> > > > in the driver
> > > >   or driver can read it from firmware)
> > > > - 'bpftool read-metadata-desc eth0 > md_desc.h' command will
> > > > query
> > > > the driver and
> > > >   generate normal C header file based on BTF in the given NIC
> > > > - user does #include "md_desc.h" and bpf program can access md-  
> > > > >csum  
> > > > or md->hash
> > > >   with direct single load out of metadata area in front of the
> > > > packet  
> > > 
> > > This piece is where I'd like to discuss more.  When we discussed
> > > this
> > > in Seoul, the initial proposal was a static struct that we'd try to
> > > hammer out a common layout between the interested parties.  That
> > > obviously wasn't going to scale, and we wanted to pursue something
> > > more
> > > dynamic.  But I thought the goal was the XDP/eBPF program wouldn't
> > > want
> > > to care what the underlying device is, and could just ask for
> > > metadata
> > > that it's interested in.  With this approach, your eBPF program is
> > > now
> > > bound/tied to the NIC/driver, and if you switched to a differen
> > > NIC/driver combo, then you'd have to rewrite part of your eBPF
> > > program
> > > to comprehend that.  I thought we were trying to avoid that.  
> > 
> > It looks to me that NICs have a lot more differences instead of
> > common pieces.
> > If we focus the architecture on making common things as generic
> > as possible we miss the bigger picture of covering distinct
> > and unique features that hw provides.
> > 
> > In the last email when I said "would be good to standardize at least
> > a few common fields"
> > I meant that it make sense only at the later phases.
> > I don't think we should put things into uapi upfront.
> > Otherwise we will end up with likely useless fields.
> > Like vlan field. Surely a lot of nics can store it into metadata, but
> > why?

FWIW vlan is extracted from the packet because of cBPF uABI.  tcpdump
breaks badly if VLAN header is in the packet buffer.  It may not be
entirely without merit to expose the field for writing, perhaps that can
simplify some EVPN use cases?  But that's most likely off-topic.

> > bpf program can easily read it from the packet.
> > What's the benefit of adding it to md?
> > This metadata concept we're discussing in the context of program
> > acceleration.
> > If metadata doesn't give perf benefits it should not be done by
> > firmware, it should not be supported by the driver and certainly
> > should not be part of uapi.
> > Hence I'd like to see pure BTF description without any
> > common/standard
> > fields across the drivers and figure out what (if anything) is useful
> > to accelerate xdp programs (and af_xdp in the future).  
> 
> I guess I didn't write my response very well before, apologies.  I
> actually really like the BTF description and the dynamic-ness of it. 
> It solves lots of problems and keeps things out of the UAPI.
> 
> What I was getting at was using the BTF description and dumping a
> header file to be used in the program to be loaded.  If we have an
> application developer include that header, it will be the Intel BTF
> description, or Mellanox BTF description, etc.  I'm fine with that, but
> it makes your program not as portable, since you'd need to get a
> different header and recompile it if you change the NIC.  Unless I'm
> missing something.

Could we just say that at the start we expose only existing SKB fields
(csum, hash, mark) and the meaning of the is the same as in the SKB?
That covers a lot of use cases and provides clear semantics (at least
to the driver developer) while naturally allowing to communicate the
fields from XDP to the stack with XDP_PASS?  It covers all Saeed did 
in his RFC, for example.

Powered by blists - more mailing lists