Date: Fri, 30 Jun 2023 11:54:23 -0700
From: Stanislav Fomichev <sdf@...gle.com>
To: Toke Høiland-Jørgensen <toke@...hat.com>
Cc: Jakub Kicinski <kuba@...nel.org>, John Fastabend <john.fastabend@...il.com>, 
	Alexei Starovoitov <alexei.starovoitov@...il.com>, Donald Hunter <donald.hunter@...il.com>, 
	bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, 
	Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>, 
	KP Singh <kpsingh@...nel.org>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	Network Development <netdev@...r.kernel.org>
Subject: Re: [RFC bpf-next v2 11/11] net/mlx5e: Support TX timestamp metadata

On Thu, Jun 29, 2023 at 4:43 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> Jakub Kicinski <kuba@...nel.org> writes:
>
> > On Tue, 27 Jun 2023 14:43:57 -0700 John Fastabend wrote:
> >> What I think would be the most straight-forward thing and most flexible
> >> is to create a <drvname>_devtx_submit_skb(<drivname>descriptor, sk_buff)
> >> and <drvname>_devtx_submit_xdp(<drvname>descriptor, xdp_frame) and then
> >> corresponding calls for <drvname>_devtx_complete_{skb|xdp}() Then you
> >> don't spend any cycles building the metadata thing or have to even
> >> worry about read kfuncs. The BPF program has read access to any
> >> fields they need. And with the skb, xdp pointer we have the context
> >> that created the descriptor and generate meaningful metrics.
> >
> > Sorry but this is not going to happen without my nack. DPDK was a much
> > cleaner bifurcation point than trying to write datapath drivers in BPF.
> > Users having to learn how to render descriptors for all the NICs
> > and queue formats out there is not reasonable. Isovalent hired
> > a lot of former driver developers so you may feel like it's a good
> > idea, as a middleware provider. But for the rest of us the matrix
> > of HW x queue format x people writing BPF is too large. If we can
> > write some poor man's DPDK / common BPF driver library to be selected
> > at linking time - we can as well provide a generic interface in
> > the kernel itself. Again, we never merged explicit DPDK support,
> > your idea is strictly worse.
>
> I agree: we're writing an operating system kernel here. The *whole
> point* of an operating system is to provide an abstraction over
> different types of hardware and provide a common API so users don't have
> to deal with the hardware details.
>
> I feel like there's some tension between "BPF as a dataplane API" and
> "BPF as a kernel extension language" here, especially as the BPF
> subsystem has grown more features in the latter direction. In my mind,
> XDP is still very much a dataplane API; in fact that's one of the main
> selling points wrt DPDK: you can get high performance networking but
> still take advantage of the kernel drivers and other abstractions that
> the kernel provides. If you're going for raw performance and the ability
> to twiddle every tiny detail of the hardware, DPDK fills that niche
> quite nicely (and also shows us the pains of going that route).

Since the thread has been quiet for a day, here is how I'm planning to proceed:
- remove most of the devtx_frame context (but still keep it for
stashing descriptor pointers and having a common kfunc api)
- keep common kfunc interface for common abstractions
- separate skb/xdp hooks - this is probably a good idea anyway to not
mix them up (we are focusing mostly on xdp here)
- continue using raw fentry for now, let's reconsider later, depending
on where we end up with generic apis vs non-generic ones
- add tx checksum to show how this tx-dev-bound framework can be
extended (and show similarities between the timestamp and checksum)

Iow, I'll largely keep the same approach but will try to expose raw
skb/xdp_frame + add tx-csum. Let's reconvene once I send out v3. Thank
you all for the valuable feedback!

Powered by blists - more mailing lists