public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Stanislav Fomichev <sdf@google.com>
To: "Toke Høiland-Jørgensen" <toke@kernel.org>
Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	 andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
	yhs@fb.com,  john.fastabend@gmail.com, kpsingh@kernel.org,
	haoluo@google.com,  jolsa@kernel.org, willemb@google.com,
	dsahern@kernel.org,  magnus.karlsson@intel.com, bjorn@kernel.org,
	maciej.fijalkowski@intel.com,  netdev@vger.kernel.org
Subject: Re: [RFC bpf-next 0/7] bpf: netdev TX metadata
Date: Tue, 13 Jun 2023 09:32:29 -0700	[thread overview]
Message-ID: <ZIiaHXr9M0LGQ0Ht@google.com> (raw)
In-Reply-To: <87cz20xunt.fsf@toke.dk>

On 06/12, Toke Høiland-Jørgensen wrote:
> Some immediate thoughts after glancing through this:
> 
> > --- Use cases ---
> >
> > The goal of this series is to add two new standard-ish places
> > in the transmit path:
> >
> > 1. Right before the packet is transmitted (with access to TX
> >    descriptors)
> > 2. Right after the packet is actually transmitted and we've received the
> >    completion (again, with access to TX completion descriptors)
> >
> > Accessing TX descriptors unlocks the following use-cases:
> >
> > - Setting device hints at TX: XDP/AF_XDP might use these new hooks to
> > use device offloads. The existing case implements TX timestamp.
> > - Observability: global per-netdev hooks can be used for tracing
> > the packets and exploring completion descriptors for all sorts of
> > device errors.
> >
> > Accessing TX descriptors also means that the hooks have to be called
> > from the drivers.
> >
> > The hooks are a light-weight alternative to XDP at egress and currently
> > don't provide any packet modification abilities. However, eventually,
> > can expose new kfuncs to operate on the packet (or, rather, the actual
> > descriptors; for performance sake).
> 
> dynptr?

Haven't considered, let me explore, but not sure what it buys us
here?

> > --- UAPI ---
> >
> > The hooks are implemented in a HID-BPF style. Meaning they don't
> > expose any UAPI and are implemented as tracing programs that call
> > a bunch of kfuncs. The attach/detach operation happen via BPF syscall
> > programs. The series expands device-bound infrastructure to tracing
> > programs.
> 
> Not a fan of the "attach from BPF syscall program" thing. These are part
> of the XDP data path API, and I think we should expose them as proper
> bpf_link attachments from userspace with introspection etc. But I guess
> the bpf_mprog thing will give us that?

bpf_mprog will just make those attach kfuncs return the link fd. The
syscall program will still stay :-(

> > --- skb vs xdp ---
> >
> > The hooks operate on a new light-weight devtx_frame which contains:
> > - data
> > - len
> > - sinfo
> >
> > This should allow us to have a unified (from BPF POW) place at TX
> > and not be super-taxing (we need to copy 2 pointers + len to the stack
> > for each invocation).
> 
> Not sure what I think about this one. At the very least I think we
> should expose xdp->data_meta as well. I'm not sure what the use case for
> accessing skbs is? If that *is* indeed useful, probably there will also
> end up being a use case for accessing the full skb?

skb_shared_info has meta_len, buf afaik, xdp doesn't use it. Maybe I
a good opportunity to unify? Or probably won't work because if
xdf_frame doesn't have frags, it won't have sinfo?

> > --- Multiprog attachment ---
> >
> > Currently, attach/detach don't expose links and don't support multiple
> > programs. I'm planning to use Daniel's bpf_mprog once it lands.
> >
> > --- TODO ---
> >
> > Things that I'm planning to do for the non-RFC series:
> > - have some real device support to verify xdp_hw_metadata works
> 
> Would be good to see some performance numbers as well :)

+1 :-)

> > - freplace
> > - Documentation/networking/xdp-rx-metadata.rst - like documentation
> >
> > --- CC ---
> >
> > CC'ing people only on the cover letter. Hopefully can find the rest via
> > lore.
> 
> Well, I found it there, even though I was apparently left off the Cc
> list :(
> 
> -Toke

Sure, I'll CC you explicitly next time! But I know you diligently follow bpf
list, so decided to explicitly cc mostly netdev folks that might miss
it otherwise.

  reply	other threads:[~2023-06-13 16:32 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-12 17:23 [RFC bpf-next 0/7] bpf: netdev TX metadata Stanislav Fomichev
2023-06-12 17:23 ` [RFC bpf-next 1/7] bpf: rename some xdp-metadata functions into dev-bound Stanislav Fomichev
2023-06-12 17:23 ` [RFC bpf-next 2/7] bpf: resolve single typedef when walking structs Stanislav Fomichev
2023-06-12 17:23 ` [RFC bpf-next 3/7] bpf: implement devtx hook points Stanislav Fomichev
2023-06-13 14:54   ` Willem de Bruijn
2023-06-13 19:00     ` Stanislav Fomichev
2023-06-13 19:29       ` Willem de Bruijn
2023-06-13 15:08   ` Simon Horman
2023-06-13 19:00     ` Stanislav Fomichev
2023-06-14  7:02       ` Simon Horman
2023-06-14 17:18         ` Stanislav Fomichev
2023-06-16  5:46   ` Kui-Feng Lee
2023-06-16 17:32     ` Stanislav Fomichev
2023-06-12 17:23 ` [RFC bpf-next 4/7] bpf: implement devtx timestamp kfunc Stanislav Fomichev
2023-06-13 15:14   ` Simon Horman
2023-06-13 18:39     ` Stanislav Fomichev
2023-06-12 17:23 ` [RFC bpf-next 5/7] net: veth: implement devtx timestamp kfuncs Stanislav Fomichev
2023-06-12 17:23 ` [RFC bpf-next 6/7] selftests/bpf: extend xdp_metadata with devtx kfuncs Stanislav Fomichev
2023-06-13 14:47   ` Willem de Bruijn
2023-06-13 19:00     ` Stanislav Fomichev
2023-06-12 17:23 ` [RFC bpf-next 7/7] selftests/bpf: extend xdp_hw_metadata " Stanislav Fomichev
2023-06-13 15:03   ` Willem de Bruijn
2023-06-13 19:00     ` Stanislav Fomichev
2023-06-12 21:00 ` [RFC bpf-next 0/7] bpf: netdev TX metadata Toke Høiland-Jørgensen
2023-06-13 16:32   ` Stanislav Fomichev [this message]
2023-06-13 17:18     ` Toke Høiland-Jørgensen
2023-06-13 18:39       ` Stanislav Fomichev
2023-06-13 19:10         ` Toke Høiland-Jørgensen
2023-06-13 21:17           ` Stanislav Fomichev
2023-06-13 22:32             ` Alexei Starovoitov
2023-06-13 23:16               ` Stanislav Fomichev
2023-06-14  4:19                 ` Alexei Starovoitov
2023-06-14 11:59                   ` Toke Høiland-Jørgensen
2023-06-14 16:27                     ` Alexei Starovoitov
2023-06-15 12:36                       ` Toke Høiland-Jørgensen
2023-06-15 16:10                         ` Alexei Starovoitov
2023-06-15 16:31                           ` Stanislav Fomichev
2023-06-16  1:50                             ` Jakub Kicinski
2023-06-16  0:09   ` Stanislav Fomichev
2023-06-16  8:12     ` Magnus Karlsson
2023-06-16 17:32       ` Stanislav Fomichev
2023-06-16 23:10         ` Stanislav Fomichev
2023-06-19  7:15           ` Magnus Karlsson
2023-06-14  3:31 ` Jakub Kicinski
2023-06-14  3:54   ` David Ahern
2023-06-14  5:05     ` Jakub Kicinski
2023-06-14 17:17       ` Stanislav Fomichev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZIiaHXr9M0LGQ0Ht@google.com \
    --to=sdf@google.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bjorn@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dsahern@kernel.org \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=martin.lau@linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=song@kernel.org \
    --cc=toke@kernel.org \
    --cc=willemb@google.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox