bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: "Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Alexei Starovoitov" <ast@fb.com>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii.nakryiko@gmail.com>,
	"Andrii Nakryiko" <andriin@fb.com>, bpf <bpf@vger.kernel.org>,
	Networking <netdev@vger.kernel.org>,
	"Kernel Team" <kernel-team@fb.com>
Subject: Re: [PATCH bpf-next 0/3] Introduce pinnable bpf_link kernel abstraction
Date: Wed, 4 Mar 2020 13:24:39 -0800	[thread overview]
Message-ID: <20200304132439.6abadbe3@kicinski-fedora-PC1C0HJN> (raw)
In-Reply-To: <20200304204506.wli3enu5w25b35h7@ast-mbp>

On Wed, 4 Mar 2020 12:45:07 -0800 Alexei Starovoitov wrote:
> On Wed, Mar 04, 2020 at 11:41:58AM -0800, Jakub Kicinski wrote:
> > On Tue, 3 Mar 2020 20:36:45 -0800 Alexei Starovoitov wrote:  
> > > > > libxdp can choose to pin it in some libxdp specific location, so other
> > > > > libxdp-enabled applications can find it in the same location, detach,
> > > > > replace, modify, but random app that wants to hack an xdp prog won't
> > > > > be able to mess with it.    
> > > > 
> > > > What if that "random app" comes first, and keeps holding on to the link
> > > > fd? Then the admin essentially has to start killing processes until they
> > > > find the one that has the device locked, no?    
> > > 
> > > Of course not. We have to provide an api to make it easy to discover
> > > what process holds that link and where it's pinned.  
> > 
> > That API to discover ownership would be useful but it's on the BPF side.  
> 
> it's on bpf side because it's bpf specific.
> 
> > We have netlink notifications in networking world. The application
> > which doesn't want its program replaced should simply listen to the
> > netlink notifications and act if something goes wrong.  
> 
> instead of locking the bike let's setup a camera and monitor the bike
> when somebody steals it.
> and then what? chase the thief and bring the bike back?

:) Is the bike the BPF program? It's more like thief is stealing our
parking spot, we still have the program :)

Maybe also the thief should not have CAP_ADMIN in the first place?
And ask a daemon to perform its actions..

> > > But if we go with notifier approach none of it is an issue.  
> > 
> > Sorry, what's the notifier approach? You mean netdev notifier chain 
> > or something new?  
> 
> that's tbd.
> 
> > > Whether target obj is held or notifier is used everything I said before still
> > > stands. "random app" that uses netlink after libdispatcher got its link FD will
> > > not be able to mess with carefully orchestrated setup done by libdispatcher.
> > > 
> > > Also either approach will guarantee that infamous message:
> > > "unregister_netdevice: waiting for %s to become free. Usage count"
> > > users will never see.
> > >  
> > > > And what about the case where the link fd is pinned on a bpffs that is
> > > > no longer available? I.e., if a netdevice with an XDP program moves
> > > > namespaces and no longer has access to the original bpffs, that XDP
> > > > program would essentially become immutable?    
> > > 
> > > 'immutable' will not be possible.
> > > I'm not clear to me how bpffs is going to disappear. What do you mean
> > > exactly?
> > >   
> > > > > We didn't come up with these design choices overnight. It came from
> > > > > hard lessons learned while deploying xdp, tc and cgroup in production.
> > > > > Legacy apis will not be deprecated, of course.    
> > 
> > This sounds like a version of devm_* helpers for configuration.
> > Why are current user space APIs insufficient?   
> 
> current xdp, tc, cgroup apis don't have the concept of the link
> and owner of that link.

Why do the attachment points have to have a concept of an owner and 
not the program itself?

Link is a very overloaded term, I may not comprehend very well that 
it models because of that.

> > Surely all of this can 
> > be done from user space.  
> 
> with a camera for theft monitoring. that will work well.
> 
> > And we will need a centralized daemon for XDP
> > dispatch, so why is it not a part of a daemon?  
> 
> current design of libdispatcher doesn't need the deamon.

Which is flawed. Why do we want to solve a distributed problem 
of multiple applications with potentially a different version 
of a library cooperating. When we can make it a daemon.

  reply	other threads:[~2020-03-04 21:24 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-28 22:39 [PATCH bpf-next 0/3] Introduce pinnable bpf_link kernel abstraction Andrii Nakryiko
2020-02-28 22:39 ` [PATCH bpf-next 1/3] bpf: introduce pinnable bpf_link abstraction Andrii Nakryiko
2020-03-02 10:13   ` Toke Høiland-Jørgensen
2020-03-02 18:06     ` Andrii Nakryiko
2020-03-02 21:40       ` Toke Høiland-Jørgensen
2020-03-02 23:37         ` Andrii Nakryiko
2020-03-03  2:50   ` Alexei Starovoitov
2020-03-03  4:18     ` Andrii Nakryiko
2020-02-28 22:39 ` [PATCH bpf-next 2/3] libbpf: add bpf_link pinning/unpinning Andrii Nakryiko
2020-03-02 10:16   ` Toke Høiland-Jørgensen
2020-03-02 18:09     ` Andrii Nakryiko
2020-03-02 21:45       ` Toke Høiland-Jørgensen
2020-02-28 22:39 ` [PATCH bpf-next 3/3] selftests/bpf: add link pinning selftests Andrii Nakryiko
2020-03-02 10:11 ` [PATCH bpf-next 0/3] Introduce pinnable bpf_link kernel abstraction Toke Høiland-Jørgensen
2020-03-02 18:05   ` Andrii Nakryiko
2020-03-02 22:24     ` Toke Høiland-Jørgensen
2020-03-02 23:35       ` Andrii Nakryiko
2020-03-03  8:12         ` Toke Høiland-Jørgensen
2020-03-03  8:12       ` Daniel Borkmann
2020-03-03 15:46         ` Alexei Starovoitov
2020-03-03 19:23           ` Daniel Borkmann
2020-03-03 19:46             ` Andrii Nakryiko
2020-03-03 20:24               ` Toke Høiland-Jørgensen
2020-03-03 20:53                 ` Daniel Borkmann
2020-03-03 22:01                   ` Alexei Starovoitov
2020-03-03 22:27                     ` Toke Høiland-Jørgensen
2020-03-04  4:36                       ` Alexei Starovoitov
2020-03-04  7:47                         ` Toke Høiland-Jørgensen
2020-03-04 15:47                           ` Alexei Starovoitov
2020-03-05 10:37                             ` Toke Høiland-Jørgensen
2020-03-05 16:34                               ` Alexei Starovoitov
2020-03-05 22:34                                 ` Daniel Borkmann
2020-03-05 22:50                                   ` Alexei Starovoitov
2020-03-05 23:42                                     ` Daniel Borkmann
2020-03-06  8:31                                       ` Toke Høiland-Jørgensen
2020-03-06 10:25                                         ` Daniel Borkmann
2020-03-06 10:42                                           ` Toke Høiland-Jørgensen
2020-03-06 18:09                                           ` David Ahern
2020-03-04 19:41                         ` Jakub Kicinski
2020-03-04 20:45                           ` Alexei Starovoitov
2020-03-04 21:24                             ` Jakub Kicinski [this message]
2020-03-05  1:07                               ` Alexei Starovoitov
2020-03-05  8:16                                 ` Jakub Kicinski
2020-03-05 11:05                                   ` Toke Høiland-Jørgensen
2020-03-05 18:13                                     ` Jakub Kicinski
2020-03-09 11:41                                       ` Toke Høiland-Jørgensen
2020-03-09 18:50                                         ` Jakub Kicinski
2020-03-10 12:22                                           ` Toke Høiland-Jørgensen
2020-03-05 16:39                                   ` Alexei Starovoitov
2020-03-03 22:40                 ` Jakub Kicinski

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=20200304132439.6abadbe3@kicinski-fedora-PC1C0HJN \
    --to=kuba@kernel.org \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.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 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).