netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: "John Fastabend" <john.fastabend@gmail.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Andrii Nakryiko" <andrii.nakryiko@gmail.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Martin KaFai Lau" <kafai@fb.com>,
	"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
	"Andrii Nakryiko" <andriin@fb.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Jesper Dangaard Brouer" <brouer@redhat.com>,
	"Lorenz Bauer" <lmb@cloudflare.com>,
	"Andrey Ignatov" <rdna@fb.com>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 1/4] xdp: Support specifying expected existing program when attaching XDP
Date: Tue, 24 Mar 2020 19:15:54 -0700	[thread overview]
Message-ID: <20200324191554.46a7e0c2@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <20200325013631.vuncsvkivexdb3fr@ast-mbp>

On Tue, 24 Mar 2020 18:36:31 -0700 Alexei Starovoitov wrote:
> On Tue, Mar 24, 2020 at 12:22:47PM -0700, John Fastabend wrote:
> > > Well, I wasn't talking about any of those subsystems, I was talking
> > > about networking :)  
> > 
> > My experience has been that networking in the strict sense of XDP no
> > longer exists on its own without cgroups, flow dissector, sockops,
> > sockmap, tracing, etc. All of these pieces are built, patched, loaded,
> > pinned and otherwise managed and manipulated as BPF objects via libbpf.
> > 
> > Because I have all this infra in place for other items its a bit odd
> > imo to drop out of BPF apis to then swap a program differently in the
> > XDP case from how I would swap a program in any other place. I'm
> > assuming ability to swap links will be enabled at some point.
> > 
> > Granted it just means I have some extra functions on the side to manage
> > the swap similar to how 'qdisc' would be handled today but still not as
> > nice an experience in my case as if it was handled natively.
> > 
> > Anyways the netlink API is going to have to call into the BPF infra
> > on the kernel side for verification, etc so its already not pure
> > networking.
> >   
> > > 
> > > In particular, networking already has a consistent and fairly
> > > well-designed configuration mechanism (i.e., netlink) that we are
> > > generally trying to move more functionality *towards* not *away from*
> > > (see, e.g., converting ethtool to use netlink).  
> > 
> > True. But BPF programs are going to exist and interop with other
> > programs not exactly in the networking space. Actually library calls
> > might be used in tracing, cgroups, and XDP side. It gets a bit more
> > interesting if the "same" object file (with some patching) runs in both
> > XDP and sockops land for example.  
> 
> Thanks John for summarizing it very well.
> It looks to me that netlink proponents fail to realize that "bpf for
> networking" goes way beyond what netlink is doing and capable of doing in the
> future. BPF_*_INET_* progs do core networking without any smell of netlink
> anywhere. "But, but, but, netlink is the way to configure networking"... is
> simply not true. Even in years before BPF sockets and syscalls were the way to
> do it. netlink has plenty of awesome properties, but arguing that it's the
> only true way to do networking is not matching the reality.

It is the way to configure XDP today, so it's only natural to
scrutinize the attempts to replace it. 

Also I personally don't think you'd see this much push back trying to
add bpf_link-based stuff to cls_bpf, that's an add-on. XDP is
integrated very fundamentally with the networking stack at this point.

> Details are important and every case is different. So imo:
> converting ethtool to netlink - great stuff.
> converting netdev irq/queue management to netlink - great stuff too.
> adding more netlink api for xdp - really bad idea.

Why is it a bad idea?

There are plenty things which will only be available over netlink.
Configuring the interface so installing the XDP program is possible
(disabling features, configuring queues etc.). Chances are user gets
the ifindex of the interface to attach to over netlink in the first
place. The queue configuration (which you agree belongs in netlink)
will definitely get more complex to allow REDIRECTs to work more
smoothly. AF_XDP needs all sort of netlink stuff.

Netlink gives us the notification mechanism which is how we solve
coordination across daemons (something that BPF subsystem is only 
now trying to solve).

BPF subsystem has a proven track record of reimplementing things devs
don't like or haven't studied (bpftool net, netlink library). So it is
a real concern to allow duplicating parts of the kernel netlink API.

  reply	other threads:[~2020-03-25  2:15 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-19 13:13 [PATCH bpf-next 0/4] XDP: Support atomic replacement of XDP interface attachments Toke Høiland-Jørgensen
2020-03-19 13:13 ` [PATCH bpf-next 1/4] xdp: Support specifying expected existing program when attaching XDP Toke Høiland-Jørgensen
2020-03-19 22:52   ` Jakub Kicinski
2020-03-20  8:48     ` Toke Høiland-Jørgensen
2020-03-20 17:35       ` Jakub Kicinski
2020-03-20 18:17         ` Toke Høiland-Jørgensen
2020-03-20 18:35           ` Jakub Kicinski
2020-03-20 18:30         ` John Fastabend
2020-03-20 20:24           ` Andrii Nakryiko
2020-03-23 11:24             ` Toke Høiland-Jørgensen
2020-03-23 16:54               ` Jakub Kicinski
2020-03-23 18:14               ` Andrii Nakryiko
2020-03-23 19:23                 ` Toke Høiland-Jørgensen
2020-03-24  1:01                   ` David Ahern
2020-03-24  4:53                     ` Andrii Nakryiko
2020-03-24 20:55                       ` David Ahern
2020-03-24 22:56                         ` Andrii Nakryiko
2020-03-24  5:00                   ` Andrii Nakryiko
2020-03-24 10:57                     ` Toke Høiland-Jørgensen
2020-03-24 18:53                       ` Jakub Kicinski
2020-03-24 22:30                         ` Andrii Nakryiko
2020-03-25  1:25                           ` Jakub Kicinski
2020-03-24 19:22                       ` John Fastabend
2020-03-25  1:36                         ` Alexei Starovoitov
2020-03-25  2:15                           ` Jakub Kicinski [this message]
2020-03-25 18:06                             ` Alexei Starovoitov
2020-03-25 18:20                               ` Jakub Kicinski
2020-03-25 19:14                                 ` Alexei Starovoitov
2020-03-25 10:42                           ` Toke Høiland-Jørgensen
2020-03-25 18:11                             ` Alexei Starovoitov
2020-03-25 10:30                         ` Toke Høiland-Jørgensen
2020-03-25 17:56                           ` Alexei Starovoitov
2020-03-24 22:25                       ` Andrii Nakryiko
2020-03-25  9:38                         ` Toke Høiland-Jørgensen
2020-03-25 17:55                           ` Alexei Starovoitov
2020-03-26  0:16                           ` Andrii Nakryiko
2020-03-26  5:13                             ` Jakub Kicinski
2020-03-26 18:09                               ` Andrii Nakryiko
2020-03-26 19:40                               ` Alexei Starovoitov
2020-03-26 20:05                                 ` Edward Cree
2020-03-27 11:09                                   ` Lorenz Bauer
2020-03-27 23:11                                   ` Alexei Starovoitov
2020-03-26 10:04                             ` Lorenz Bauer
2020-03-26 17:47                               ` Jakub Kicinski
2020-03-26 19:45                                 ` Alexei Starovoitov
2020-03-26 18:18                               ` Andrii Nakryiko
2020-03-26 19:53                               ` Alexei Starovoitov
2020-03-27 11:11                                 ` Toke Høiland-Jørgensen
2020-04-02 20:21                                   ` bpf: ability to attach freplace to multiple parents Alexei Starovoitov
2020-04-02 21:23                                     ` Toke Høiland-Jørgensen
2020-04-02 21:54                                       ` Alexei Starovoitov
2020-04-03  8:38                                         ` Toke Høiland-Jørgensen
2020-04-07  1:44                                           ` Alexei Starovoitov
2020-04-07  9:20                                             ` Toke Høiland-Jørgensen
2020-05-12  8:34                                         ` Toke Høiland-Jørgensen
2020-05-12  9:53                                           ` Alan Maguire
2020-05-12 13:02                                             ` Toke Høiland-Jørgensen
2020-05-12 23:18                                             ` Alexei Starovoitov
2020-05-12 23:06                                           ` Alexei Starovoitov
2020-05-13 10:25                                             ` Toke Høiland-Jørgensen
2020-04-02 21:24                                     ` Andrey Ignatov
2020-04-02 22:01                                       ` Alexei Starovoitov
2020-03-26 12:35                             ` [PATCH bpf-next 1/4] xdp: Support specifying expected existing program when attaching XDP Toke Høiland-Jørgensen
2020-03-26 19:06                               ` Andrii Nakryiko
2020-03-27 11:06                                 ` Lorenz Bauer
2020-03-27 16:12                                   ` David Ahern
2020-03-27 20:10                                     ` Andrii Nakryiko
2020-03-27 23:02                                     ` Alexei Starovoitov
2020-03-30 15:25                                       ` Edward Cree
2020-03-31  3:43                                         ` Alexei Starovoitov
2020-03-31 22:05                                           ` Edward Cree
2020-03-31 22:16                                             ` Alexei Starovoitov
2020-03-27 19:42                                   ` Andrii Nakryiko
2020-03-27 19:45                                   ` Andrii Nakryiko
2020-03-27 23:09                                   ` Alexei Starovoitov
2020-03-27 11:46                                 ` Toke Høiland-Jørgensen
2020-03-27 20:07                                   ` Andrii Nakryiko
2020-03-27 22:16                                     ` Toke Høiland-Jørgensen
2020-03-27 22:54                                       ` Andrii Nakryiko
2020-03-28  1:09                                         ` Toke Høiland-Jørgensen
2020-03-28  1:44                                           ` Andrii Nakryiko
2020-03-28 19:43                                             ` Toke Høiland-Jørgensen
2020-03-26 19:58                               ` Alexei Starovoitov
2020-03-27 12:06                                 ` Toke Høiland-Jørgensen
2020-03-27 23:00                                   ` Alexei Starovoitov
2020-03-28  1:43                                     ` Toke Høiland-Jørgensen
2020-03-28  2:26                                       ` Alexei Starovoitov
2020-03-28 19:34                                         ` Toke Høiland-Jørgensen
2020-03-28 23:35                                           ` Alexei Starovoitov
2020-03-29 10:39                                             ` Toke Høiland-Jørgensen
2020-03-29 19:26                                               ` Alexei Starovoitov
2020-03-30 10:19                                                 ` Toke Høiland-Jørgensen
2020-03-29 20:23                                           ` Andrii Nakryiko
2020-03-30 13:53                                             ` Toke Høiland-Jørgensen
2020-03-30 20:17                                               ` Andrii Nakryiko
2020-03-31 10:13                                                 ` Toke Høiland-Jørgensen
2020-03-31 13:48                                                   ` Daniel Borkmann
2020-03-31 15:00                                                     ` Toke Høiland-Jørgensen
2020-03-31 20:19                                                       ` Andrii Nakryiko
2020-03-31 20:15                                                     ` Andrii Nakryiko
2020-03-30 15:41                                             ` Edward Cree
2020-03-30 19:13                                               ` Jakub Kicinski
2020-03-31  4:01                                               ` Alexei Starovoitov
2020-03-31 11:34                                                 ` Toke Høiland-Jørgensen
2020-03-31 18:52                                                   ` Alexei Starovoitov
2020-03-20 20:30       ` Daniel Borkmann
2020-03-20 20:40         ` Daniel Borkmann
2020-03-20 21:30           ` Jakub Kicinski
2020-03-20 21:55             ` Daniel Borkmann
2020-03-20 23:35               ` Jakub Kicinski
2020-03-20 20:39       ` Andrii Nakryiko
2020-03-23 11:25         ` Toke Høiland-Jørgensen
2020-03-23 18:07           ` Andrii Nakryiko
2020-03-23 23:54           ` Andrey Ignatov
2020-03-24 10:16             ` Toke Høiland-Jørgensen
2020-03-20  2:13   ` Yonghong Song
2020-03-20  8:48     ` Toke Høiland-Jørgensen
2020-03-19 13:13 ` [PATCH bpf-next 2/4] tools: Add EXPECTED_FD-related definitions in if_link.h Toke Høiland-Jørgensen
2020-03-19 13:13 ` [PATCH bpf-next 3/4] libbpf: Add function to set link XDP fd while specifying old fd Toke Høiland-Jørgensen
2020-03-19 13:13 ` [PATCH bpf-next 4/4] selftests/bpf: Add tests for attaching XDP programs Toke Høiland-Jørgensen

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=20200324191554.46a7e0c2@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=lmb@cloudflare.com \
    --cc=netdev@vger.kernel.org \
    --cc=rdna@fb.com \
    --cc=songliubraving@fb.com \
    --cc=toke@redhat.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;
as well as URLs for NNTP newsgroup(s).