All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Marek Majtyka <alardam@gmail.com>
Cc: "Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Saeed Mahameed" <saeed@kernel.org>,
	"David Ahern" <dsahern@gmail.com>,
	"Maciej Fijalkowski" <maciej.fijalkowski@intel.com>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Jesper Dangaard Brouer" <jbrouer@redhat.com>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Maciej Fijalkowski" <maciejromanfijalkowski@gmail.com>,
	"Björn Töpel" <bjorn.topel@intel.com>,
	"Andrii Nakryiko" <andrii.nakryiko@gmail.com>,
	"Jonathan Lemon" <jonathan.lemon@gmail.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Network Development" <netdev@vger.kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	hawk@kernel.org, bpf <bpf@vger.kernel.org>,
	intel-wired-lan <intel-wired-lan@lists.osuosl.org>,
	"Karlsson, Magnus" <magnus.karlsson@intel.com>,
	jeffrey.t.kirsher@intel.com
Subject: Re: [PATCH v2 bpf 1/5] net: ethtool: add xdp properties flag set
Date: Wed, 3 Feb 2021 09:02:32 -0800	[thread overview]
Message-ID: <20210203090232.4a259958@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <CAAOQfrGqcsn3wu5oxzHYxtE8iK3=gFdTka5HSh5Fe9Hc6HWRWA@mail.gmail.com>

On Wed, 3 Feb 2021 13:50:59 +0100 Marek Majtyka wrote:
> On Tue, Feb 2, 2021 at 8:34 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > On Tue, 02 Feb 2021 13:05:34 +0100 Toke Høiland-Jørgensen wrote:  
> > > Awesome! And sorry for not replying straight away - I hate it when I
> > > send out something myself and receive no replies, so I suppose I should
> > > get better at not doing that myself :)
> > >
> > > As for the inclusion of the XDP_BASE / XDP_LIMITED_BASE sets (which I
> > > just realised I didn't reply to), I am fine with defining XDP_BASE as a
> > > shortcut for TX/ABORTED/PASS/DROP, but think we should skip
> > > XDP_LIMITED_BASE and instead require all new drivers to implement the
> > > full XDP_BASE set straight away. As long as we're talking about
> > > features *implemented* by the driver, at least; i.e., it should still be
> > > possible to *deactivate* XDP_TX if you don't want to use the HW
> > > resources, but I don't think there's much benefit from defining the
> > > LIMITED_BASE set as a shortcut for this mode...  
> >
> > I still have mixed feelings about these flags. The first step IMO
> > should be adding validation tests. I bet^W pray every vendor has
> > validation tests but since they are not unified we don't know what
> > level of interoperability we're achieving in practice. That doesn't
> > matter for trivial feature like base actions, but we'll inevitably
> > move on to defining more advanced capabilities and the question of
> > "what supporting X actually mean" will come up (3 years later, when
> > we don't remember ourselves).  
> 
> I am a bit confused now. Did you mean validation tests of those XDP
> flags, which I am working on or some other validation tests?
> What should these tests verify? Can you please elaborate more on the
> topic, please - just a few sentences how are you see it?

Conformance tests can be written for all features, whether they have 
an explicit capability in the uAPI or not. But for those that do IMO
the tests should be required.

Let me give you an example. This set adds a bit that says Intel NICs 
can do XDP_TX and XDP_REDIRECT, yet we both know of the Tx queue
shenanigans. So can i40e do XDP_REDIRECT or can it not?

If we have exhaustive conformance tests we can confidently answer that
question. And the answer may not be "yes" or "no", it may actually be
"we need more options because many implementations fall in between".

I think readable (IOW not written in some insane DSL) tests can also 
be useful for users who want to check which features their program /
deployment will require.

WARNING: multiple messages have this Message-ID (diff)
From: Jakub Kicinski <kuba@kernel.org>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH v2 bpf 1/5] net: ethtool: add xdp properties flag set
Date: Wed, 3 Feb 2021 09:02:32 -0800	[thread overview]
Message-ID: <20210203090232.4a259958@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <CAAOQfrGqcsn3wu5oxzHYxtE8iK3=gFdTka5HSh5Fe9Hc6HWRWA@mail.gmail.com>

On Wed, 3 Feb 2021 13:50:59 +0100 Marek Majtyka wrote:
> On Tue, Feb 2, 2021 at 8:34 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > On Tue, 02 Feb 2021 13:05:34 +0100 Toke H?iland-J?rgensen wrote:  
> > > Awesome! And sorry for not replying straight away - I hate it when I
> > > send out something myself and receive no replies, so I suppose I should
> > > get better at not doing that myself :)
> > >
> > > As for the inclusion of the XDP_BASE / XDP_LIMITED_BASE sets (which I
> > > just realised I didn't reply to), I am fine with defining XDP_BASE as a
> > > shortcut for TX/ABORTED/PASS/DROP, but think we should skip
> > > XDP_LIMITED_BASE and instead require all new drivers to implement the
> > > full XDP_BASE set straight away. As long as we're talking about
> > > features *implemented* by the driver, at least; i.e., it should still be
> > > possible to *deactivate* XDP_TX if you don't want to use the HW
> > > resources, but I don't think there's much benefit from defining the
> > > LIMITED_BASE set as a shortcut for this mode...  
> >
> > I still have mixed feelings about these flags. The first step IMO
> > should be adding validation tests. I bet^W pray every vendor has
> > validation tests but since they are not unified we don't know what
> > level of interoperability we're achieving in practice. That doesn't
> > matter for trivial feature like base actions, but we'll inevitably
> > move on to defining more advanced capabilities and the question of
> > "what supporting X actually mean" will come up (3 years later, when
> > we don't remember ourselves).  
> 
> I am a bit confused now. Did you mean validation tests of those XDP
> flags, which I am working on or some other validation tests?
> What should these tests verify? Can you please elaborate more on the
> topic, please - just a few sentences how are you see it?

Conformance tests can be written for all features, whether they have 
an explicit capability in the uAPI or not. But for those that do IMO
the tests should be required.

Let me give you an example. This set adds a bit that says Intel NICs 
can do XDP_TX and XDP_REDIRECT, yet we both know of the Tx queue
shenanigans. So can i40e do XDP_REDIRECT or can it not?

If we have exhaustive conformance tests we can confidently answer that
question. And the answer may not be "yes" or "no", it may actually be
"we need more options because many implementations fall in between".

I think readable (IOW not written in some insane DSL) tests can also 
be useful for users who want to check which features their program /
deployment will require.

  reply	other threads:[~2021-02-03 17:04 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-04 10:28 [PATCH v2 bpf 0/5] New netdev feature flags for XDP alardam
2020-12-04 10:28 ` [Intel-wired-lan] " alardam
2020-12-04 10:28 ` [PATCH v2 bpf 1/5] net: ethtool: add xdp properties flag set alardam
2020-12-04 10:28   ` [Intel-wired-lan] " alardam
2020-12-04 12:18   ` Toke Høiland-Jørgensen
2020-12-04 12:18     ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2020-12-04 12:46     ` Maciej Fijalkowski
2020-12-04 12:46       ` [Intel-wired-lan] " Maciej Fijalkowski
2020-12-04 15:21       ` Daniel Borkmann
2020-12-04 15:21         ` [Intel-wired-lan] " Daniel Borkmann
2020-12-04 17:20         ` Toke Høiland-Jørgensen
2020-12-04 17:20           ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2020-12-04 22:19           ` Daniel Borkmann
2020-12-04 22:19             ` [Intel-wired-lan] " Daniel Borkmann
2020-12-07 11:54             ` Jesper Dangaard Brouer
2020-12-07 11:54               ` [Intel-wired-lan] " Jesper Dangaard Brouer
2020-12-07 12:08               ` Toke Høiland-Jørgensen
2020-12-07 12:08                 ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2020-12-07 12:03             ` Toke Høiland-Jørgensen
2020-12-07 12:03               ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2020-12-07 12:54         ` Jesper Dangaard Brouer
2020-12-07 12:54           ` [Intel-wired-lan] " Jesper Dangaard Brouer
2020-12-07 20:52           ` John Fastabend
2020-12-07 20:52             ` [Intel-wired-lan] " John Fastabend
2020-12-07 22:38             ` Saeed Mahameed
2020-12-07 22:38               ` [Intel-wired-lan] " Saeed Mahameed
2020-12-07 23:07             ` Maciej Fijalkowski
2020-12-07 23:07               ` [Intel-wired-lan] " Maciej Fijalkowski
2020-12-09  6:03               ` John Fastabend
2020-12-09  6:03                 ` [Intel-wired-lan] " John Fastabend
2020-12-09  9:54                 ` Maciej Fijalkowski
2020-12-09  9:54                   ` [Intel-wired-lan] " Maciej Fijalkowski
2020-12-09 11:52                   ` Jesper Dangaard Brouer
2020-12-09 11:52                     ` [Intel-wired-lan] " Jesper Dangaard Brouer
2020-12-09 15:41                     ` David Ahern
2020-12-09 15:41                       ` [Intel-wired-lan] " David Ahern
2020-12-09 17:15                       ` Saeed Mahameed
2020-12-09 17:15                         ` [Intel-wired-lan] " Saeed Mahameed
2020-12-10  3:34                         ` David Ahern
2020-12-10  3:34                           ` [Intel-wired-lan] " David Ahern
2020-12-10  6:48                           ` Saeed Mahameed
2020-12-10  6:48                             ` [Intel-wired-lan] " Saeed Mahameed
2020-12-10 15:30                             ` David Ahern
2020-12-10 15:30                               ` [Intel-wired-lan] " David Ahern
2020-12-10 18:58                               ` Saeed Mahameed
2020-12-10 18:58                                 ` [Intel-wired-lan] " Saeed Mahameed
2021-01-05 11:56                                 ` Marek Majtyka
2021-01-05 11:56                                   ` [Intel-wired-lan] " Marek Majtyka
2021-02-01 16:16                                   ` Toke Høiland-Jørgensen
2021-02-01 16:16                                     ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2021-02-02 11:26                                     ` Marek Majtyka
2021-02-02 11:26                                       ` [Intel-wired-lan] " Marek Majtyka
2021-02-02 12:05                                       ` Toke Høiland-Jørgensen
2021-02-02 12:05                                         ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2021-02-02 19:34                                         ` Jakub Kicinski
2021-02-02 19:34                                           ` [Intel-wired-lan] " Jakub Kicinski
2021-02-03 12:50                                           ` Marek Majtyka
2021-02-03 12:50                                             ` [Intel-wired-lan] " Marek Majtyka
2021-02-03 17:02                                             ` Jakub Kicinski [this message]
2021-02-03 17:02                                               ` Jakub Kicinski
2021-02-10 10:53                                               ` Toke Høiland-Jørgensen
2021-02-10 10:53                                                 ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2021-02-10 18:31                                                 ` Jakub Kicinski
2021-02-10 18:31                                                   ` [Intel-wired-lan] " Jakub Kicinski
2021-02-10 22:52                                                   ` Toke Høiland-Jørgensen
2021-02-10 22:52                                                     ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2021-02-12  1:26                                                     ` Jakub Kicinski
2021-02-12  1:26                                                       ` [Intel-wired-lan] " Jakub Kicinski
2021-02-12  2:05                                                       ` Alexei Starovoitov
2021-02-12  2:05                                                         ` [Intel-wired-lan] " Alexei Starovoitov
2021-02-12  7:02                                                         ` Marek Majtyka
2021-02-12  7:02                                                           ` [Intel-wired-lan] " Marek Majtyka
2021-02-16 14:30                                                           ` Toke Høiland-Jørgensen
2021-02-16 14:30                                                             ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2020-12-09 15:44                     ` David Ahern
2020-12-09 15:44                       ` [Intel-wired-lan] " David Ahern
2020-12-10 13:32                       ` Explaining XDP redirect bulk size design (Was: [PATCH v2 bpf 1/5] net: ethtool: add xdp properties flag set) Jesper Dangaard Brouer
2020-12-10 13:32                         ` [Intel-wired-lan] " Jesper Dangaard Brouer
2020-12-10 14:14                         ` Magnus Karlsson
2020-12-10 14:14                           ` Magnus Karlsson
2020-12-10 17:30                           ` Jesper Dangaard Brouer
2020-12-10 17:30                             ` Jesper Dangaard Brouer
2020-12-10 19:20                         ` Saeed Mahameed
2020-12-10 19:20                           ` [Intel-wired-lan] " Saeed Mahameed
2020-12-08  1:01             ` [PATCH v2 bpf 1/5] net: ethtool: add xdp properties flag set David Ahern
2020-12-08  1:01               ` [Intel-wired-lan] " David Ahern
2020-12-08  8:28               ` Jesper Dangaard Brouer
2020-12-08  8:28                 ` [Intel-wired-lan] " Jesper Dangaard Brouer
2020-12-08 11:58                 ` Toke Høiland-Jørgensen
2020-12-08 11:58                   ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2020-12-09  5:50                   ` John Fastabend
2020-12-09  5:50                     ` [Intel-wired-lan] " John Fastabend
2020-12-09 10:26                     ` Toke Høiland-Jørgensen
2020-12-09 10:26                       ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2020-12-08  9:00             ` Jesper Dangaard Brouer
2020-12-08  9:00               ` [Intel-wired-lan] " Jesper Dangaard Brouer
2020-12-08  9:42               ` Daniel Borkmann
2020-12-08  9:42                 ` [Intel-wired-lan] " Daniel Borkmann
2020-12-04 12:57   ` Maciej Fijalkowski
2020-12-04 12:57     ` [Intel-wired-lan] " Maciej Fijalkowski
2020-12-04 10:28 ` [PATCH v2 bpf 2/5] drivers/net: turn XDP properties on alardam
2020-12-04 10:28   ` [Intel-wired-lan] " alardam
2020-12-04 12:19   ` Toke Høiland-Jørgensen
2020-12-04 12:19     ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2020-12-09 19:05   ` kernel test robot
2020-12-09 19:05     ` kernel test robot
2020-12-04 10:28 ` [PATCH v2 bpf 3/5] xsk: add usage of xdp properties flags alardam
2020-12-04 10:28   ` [Intel-wired-lan] " alardam
2020-12-04 10:29 ` [PATCH v2 bpf 4/5] xsk: add check for full support of XDP in bind alardam
2020-12-04 10:29   ` [Intel-wired-lan] " alardam
2020-12-04 10:29 ` [PATCH v2 bpf 5/5] ethtool: provide xdp info with XDP_PROPERTIES_GET alardam
2020-12-04 10:29   ` [Intel-wired-lan] " alardam
2020-12-04 17:20 ` [PATCH v2 bpf 0/5] New netdev feature flags for XDP Jakub Kicinski
2020-12-04 17:20   ` [Intel-wired-lan] " Jakub Kicinski
2020-12-04 17:26   ` Toke Høiland-Jørgensen
2020-12-04 17:26     ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=
2020-12-04 19:22     ` Jakub Kicinski
2020-12-04 19:22       ` [Intel-wired-lan] " Jakub Kicinski
2020-12-07 12:04       ` Toke Høiland-Jørgensen
2020-12-07 12:04         ` [Intel-wired-lan] " Toke =?unknown-8bit?q?H=C3=B8iland-J=C3=B8rgensen?=

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=20210203090232.4a259958@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=alardam@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=ast@kernel.org \
    --cc=bjorn.topel@intel.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=hawk@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jbrouer@redhat.com \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=john.fastabend@gmail.com \
    --cc=jonathan.lemon@gmail.com \
    --cc=maciej.fijalkowski@intel.com \
    --cc=maciejromanfijalkowski@gmail.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=saeed@kernel.org \
    --cc=toke@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.