Date: Mon, 26 Jun 2023 15:37:44 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Stanislav Fomichev <sdf@...gle.com>
Cc: Jakub Kicinski <kuba@...nel.org>, John Fastabend <john.fastabend@...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 Mon, Jun 26, 2023 at 2:36 PM Stanislav Fomichev <sdf@...gle.com> wrote:
>
> > >
> > > I'd think HW TX csum is actually simpler than dealing with time,
> > > will you change your mind if Stan posts Tx csum within a few days? :)

Absolutely :) Happy to change my mind.

> > > The set of offloads is barely changing, the lack of clarity
> > > on what is needed seems overstated. IMHO AF_XDP is getting no use
> > > today, because everything remotely complex was stripped out of
> > > the implementation to get it merged. Aren't we hand waving the
> > > complexity away simply because we don't want to deal with it?
> > >
> > > These are the features today's devices support (rx/tx is a mirror):
> > >  - L4 csum
> > >  - segmentation
> > >  - time reporting
> > >
> > > Some may also support:
> > >  - forwarding md tagging
> > >  - Tx launch time
> > >  - no fcs
> > > Legacy / irrelevant:
> > >  - VLAN insertion
> >
> > Right, the goal of the series is to lay out the foundation to support
> > AF_XDP offloads. I'm starting with tx timestamp because that's more
> > pressing. But, as I mentioned in another thread, we do have other
> > users that want to adopt AF_XDP, but due to missing tx offloads, they
> > aren't able to.
> >
> > IMHO, with pre-tx/post-tx hooks, it's pretty easy to go from TX
> > timestamp to TX checksum offload, we don't need a lot:
> > - define another generic kfunc bpf_request_tx_csum(from, to)
> > - drivers implement it
> > - af_xdp users call this kfunc from devtx hook
> >
> > We seem to be arguing over start-with-my-specific-narrow-use-case vs
> > start-with-generic implementation, so maybe time for the office hours?
> > I can try to present some cohesive plan of how we start with the framework
> > plus tx-timestamp and expand with tx-checksum/etc. There is a lot of
> > commonality in these offloads, so I'm probably not communicating it
> > properly..
>
> Or, maybe a better suggestion: let me try to implement TX checksum
> kfunc in the v3 (to show how to build on top this series).
> Having code is better than doing slides :-D

That would certainly help :)
What I got out of your lsfmmbpf talk is that timestamp is your
main and only use case. tx checksum for af_xdp is the other use case,
but it's not yours, so you sort-of use it as an extra justification
for timestamp. Hence my negative reaction to 'generality'.
I think we'll have better results in designing an api
when we look at these two use cases independently.
And implement them in patches solving specifically timestamp
with normal skb traffic and tx checksum for af_xdp as two independent apis.
If it looks like we can extract a common framework out of them. Great.
But trying to generalize before truly addressing both cases
is likely to cripple both apis.

It doesn't have to be only two use cases.
I completely agree with Kuba that:
 - L4 csum
 - segmentation
 - time reporting
are universal HW NIC features and we need to have an api
that exposes these features in programmable way to bpf progs in the kernel
and through af_xdp to user space.
I mainly suggest addressing them one by one and look
for common code bits and api similarities later.

Powered by blists - more mailing lists