Message-ID: <60d2cb1fd2bf9_2052b20886@john-XPS-13-9370.notmuch>
Date:   Tue, 22 Jun 2021 22:48:15 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     David Ahern <dsahern@...il.com>,
        John Fastabend <john.fastabend@...il.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>, bpf@...r.kernel.org,
        netdev@...r.kernel.org
Cc:     lorenzo.bianconi@...hat.com, davem@...emloft.net, kuba@...nel.org,
        ast@...nel.org, daniel@...earbox.net, shayagr@...zon.com,
        sameehj@...zon.com, dsahern@...nel.org, brouer@...hat.com,
        echaudro@...hat.com, jasowang@...hat.com,
        alexander.duyck@...il.com, saeed@...nel.org,
        maciej.fijalkowski@...el.com, magnus.karlsson@...el.com,
        tirthendu.sarkar@...el.com
Subject: Re: [PATCH v9 bpf-next 00/14] mvneta: introduce XDP multi-buffer
 support

David Ahern wrote:
> On 6/22/21 5:18 PM, John Fastabend wrote:
> > At this point I don't think we can have a partial implementation. At
> > the moment we have packet capture applications and protocol parsers
> > running in production. If we allow this to go in staged we are going
> > to break those applications that make the fundamental assumption they
> > have access to all the data in the packet.
> 
> What about cases like netgpu where headers are accessible but data is
> not (e.g., gpu memory)? If the API indicates limited buffer access, is
> that sufficient?

I never consider netgpus and I guess I don't fully understand the
architecture to say. But, I would try to argue that an XDP API
should allow XDP to reach into the payload of these GPU packets as well.
Of course it might be slow.

I'm not really convinced just indicating its a limited buffer is enough.
I think we want to be able to read/write any byte in the packet. I see
two ways to do it,

  /* xdp_pull_data moves data and data_end pointers into the frag
   * containing the byte offset start.
   *
   * returns negative value on error otherwise returns offset of
   * data pointer into payload.
   */
  int xdp_pull_data(int start)

This would be a helper call to push the xdp->data{_end} pointers into
the correct frag and then normal verification should work. From my
side this works because I can always find the next frag by starting
at 'xdp_pull_data(xdp->data_end+1)'. And by returning offset we can
always figure out where we are in the payload. This is the easiest
thing I could come up with. And hopefully for _most_ cases the bytes
we need are in the initial data. Also I don't see how extending tail
works without something like this.

My other thought, but requires some verifier work would be to extend
'struct xdp_md' with a frags[] pointer.

 struct xdp_md {
   __u32 data;
   __u32 data_end;
   __u32 data_meta;
   /* metadata stuff */
  struct _xdp_md frags[] 
  __u32 frags_end;
 }

Then a XDP program could read access a frag like so,

  if (i < xdp->frags_end) {
     frag = xdp->frags[i];
     if (offset + hdr_size < frag->data_end)
         memcpy(dst, frag->data[offset], hdr_size);
  }

The nice bit about above is you avoid the call, but maybe it doesn't
matter if you are already looking into frags pps is probably not at
64B sizes anyways.

My main concern here is we hit a case where the driver doesn't pull in
the bytes we need and then we are stuck without a workaround. The helper
looks fairly straightforward to me could we try that?

Also I thought we had another driver in the works? Any ideas where
that went...

Last, I'll add thanks for working on this everyone.

.John

Powered by blists - more mailing lists