linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexei Starovoitov <ast@plumgrid.com>
To: Daniel Borkmann <daniel@iogearbox.net>,
	Hannes Frederic Sowa <hannes@stressinduktion.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Cc: davem@davemloft.net, viro@ZenIV.linux.org.uk, tgraf@suug.ch,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Alexei Starovoitov <ast@kernel.org>
Subject: Re: [PATCH net-next 3/4] bpf: add support for persistent maps/progs
Date: Mon, 19 Oct 2015 11:15:17 -0700	[thread overview]
Message-ID: <56253335.9000206@plumgrid.com> (raw)
In-Reply-To: <56252A43.3000706@iogearbox.net>

On 10/19/15 10:37 AM, Daniel Borkmann wrote:
> An eBPF program or map loading/destruction is *not* by any means to be
> considered fast-path. We currently hold a global mutex during loading.
> So, how can that be considered fast-path? Similarly, socket creation/
> destruction is also not fast-path, etc. Do you expect that applications
> would create/destroy these devices within milliseconds? I'd argue that
> something would be seriously wrong with that application, then. Such
> persistent maps are to be considered rather mid-long living objects in
> the system. The fast-path surely is the data-path of them.

consider seccomp that loads several programs for every tab, then
container use case where we're loading two for each as well.
Who knows what use cases will come up in the future.
It's obviously not a fast path that is being hit million times a second,
but why add overhead when it's unnecessary?

>> completely unnecessary here. The kernel will consume more memory for
>> no real reason other than cdev are used to keep prog/maps around.
>
> I don't consider this a big issue, and well worth the trade-off. You'll
> have an infrastructure that integrates *nicely* into the *existing* kernel
> model *and* tooling with the proposed patch. This is a HUGE plus. The
> UAPI of this is simple and minimal. And to me, these are in-fact special
> files, not regular ones.

Seriously? Syscall to create/destory cdevs is a nice api?
Not by any means. We can argue in circles, but it doesn't fit.
Using cdev to hold maps is a hack.
Telling kernel that fake device is created only to hold a map?
This fake device doesn't have any of the device properties.
Look at fops->open. Surely it's a smart trick, but it's not behaving
like device.

uapi for fs adds only two commands, whereas cdev needs three.

>> imo fs is cleaner and we can tailor it to be similar to cdev style.
>
> Really, IMHO I think this is over-designed, and much much more hacky. We
> design a whole new file system that works *exactly* like cdevs, takes
> likely more than twice the code and complexity to realize but just to
> save a few bytes ...? I don't understand that.

Let's argue with facts. Your fs patch 758 lines vs cdev 483.
In fs the cost is single alloc_inode(), whereas in cdev the struct cdev
and mutex will add to all maps and progs (even when they're not going
to be pinned), plus kobj allocation, plus gigantic struct device, plus
sysfs inodes, etc. Way more than few bytes of difference.

where do you see 'over-design' in fs? It's a straightforward code and
most of it is boilerplate like all other fs.
Kill rmdir/mkdir ops, term bit and options and the fs diff will shrink
by another 200+ lines.


  reply	other threads:[~2015-10-19 18:15 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-16  1:09 [PATCH net-next 0/4] BPF updates Daniel Borkmann
2015-10-16  1:09 ` [PATCH net-next 1/4] bpf: abstract anon_inode_getfd invocations Daniel Borkmann
2015-10-16  1:09 ` [PATCH net-next 2/4] bpf: align and clean bpf_{map,prog}_get helpers Daniel Borkmann
2015-10-16  1:09 ` [PATCH net-next 3/4] bpf: add support for persistent maps/progs Daniel Borkmann
2015-10-16 10:25   ` Hannes Frederic Sowa
2015-10-16 13:36     ` Daniel Borkmann
2015-10-16 16:36       ` Hannes Frederic Sowa
2015-10-16 17:27         ` Daniel Borkmann
2015-10-16 17:37           ` Alexei Starovoitov
2015-10-16 16:18     ` Alexei Starovoitov
2015-10-16 16:43       ` Hannes Frederic Sowa
2015-10-16 17:32         ` Alexei Starovoitov
2015-10-16 17:37           ` Thomas Graf
2015-10-16 17:21   ` Hannes Frederic Sowa
2015-10-16 17:42     ` Alexei Starovoitov
2015-10-16 17:56       ` Daniel Borkmann
2015-10-16 18:41         ` Eric W. Biederman
2015-10-16 19:27           ` Alexei Starovoitov
2015-10-16 19:53             ` Eric W. Biederman
2015-10-16 20:56               ` Alexei Starovoitov
2015-10-16 23:44                 ` Eric W. Biederman
2015-10-17  2:43                   ` Alexei Starovoitov
2015-10-17 12:28                     ` Daniel Borkmann
2015-10-18  2:20                       ` Alexei Starovoitov
2015-10-18 15:03                         ` Daniel Borkmann
2015-10-18 16:49                           ` Daniel Borkmann
2015-10-18 20:59                             ` Alexei Starovoitov
2015-10-19  7:36                               ` Hannes Frederic Sowa
2015-10-19  9:51                                 ` Daniel Borkmann
2015-10-19 14:23                                   ` Daniel Borkmann
2015-10-19 16:22                                     ` Alexei Starovoitov
2015-10-19 17:37                                       ` Daniel Borkmann
2015-10-19 18:15                                         ` Alexei Starovoitov [this message]
2015-10-19 18:46                                           ` Hannes Frederic Sowa
2015-10-19 19:34                                             ` Alexei Starovoitov
2015-10-19 20:03                                               ` Hannes Frederic Sowa
2015-10-19 20:48                                                 ` Alexei Starovoitov
2015-10-19 22:17                                                   ` Daniel Borkmann
2015-10-20  0:30                                                     ` Alexei Starovoitov
2015-10-20  8:46                                                       ` Daniel Borkmann
2015-10-20 17:53                                                         ` Alexei Starovoitov
2015-10-20 18:56                                                           ` Eric W. Biederman
2015-10-21 15:17                                                             ` Daniel Borkmann
2015-10-21 18:34                                                               ` Thomas Graf
2015-10-21 22:44                                                                 ` Alexei Starovoitov
2015-10-22 13:22                                                                   ` Daniel Borkmann
2015-10-22 19:35                                                               ` Eric W. Biederman
2015-10-23 13:47                                                                 ` Daniel Borkmann
2015-10-20  9:43                                                       ` Hannes Frederic Sowa
2015-10-19 23:02                                                   ` Hannes Frederic Sowa
2015-10-20  1:09                                                     ` Alexei Starovoitov
2015-10-20 10:07                                                       ` Hannes Frederic Sowa
2015-10-20 18:44                                                         ` Alexei Starovoitov
2015-10-16 19:54             ` Daniel Borkmann
2015-10-16  1:09 ` [PATCH net-next 4/4] bpf: add sample usages " Daniel Borkmann
2015-10-19  2:53 ` [PATCH net-next 0/4] BPF updates David Miller

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=56253335.9000206@plumgrid.com \
    --to=ast@plumgrid.com \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=hannes@stressinduktion.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tgraf@suug.ch \
    --cc=viro@ZenIV.linux.org.uk \
    /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).