Re: [PATCH net-next 3/4] bpf: add support for persistent maps/progs

[Daniel Borkmann]

On 10/19/2015 10:48 PM, Alexei Starovoitov wrote:
> On 10/19/15 1:03 PM, Hannes Frederic Sowa wrote:
> > I doubt it will stay a lightweight feature as it should not be in the
> > responsibility of user space to provide those debug facilities.
>
> It feels we're talking past each other.
> I want to solve 'persistent map' problem.
> debugging of maps/progs, hierarchy, etc are all nice to have,
> but different issues.

Ok, so you are saying that this file system will have *only* regular
files that are to be considered nodes for eBPF maps and progs. Nothing
else will ever be added to it? Yet, eBPF map and prog nodes are *not*
*regular files* to me, this seems odd.

As soon as you are starting to add additional folders that contain
files dumping additional meta data, etc, you basically end up on a
bit of similar concept to sysfs, no?

> In case of persistent maps I imagine unprivileged process would want
> to use it eventually as well, so this requirement already kills cdev
> approach for me, since I don't think we ever let unprivileged apps
> create cdev with syscall.

Hmm, I see. So far the discussion was only about having this for privileged
users (also in this fs patch). F.e. privileged system daemons could setup
and distribute progs/maps to consumers, etc (f.e. seccomp and tc case).

When we start lifting this, eBPF maps by its own will become a real kernel
IPC facility for unprivileged Linux applications (independently whether
they are connected to an actual eBPF program). Those kernel IPC facilities
that are anchored in the file system like named pipes and Unix domain sockets
are indicated as such as special files, no?

> > The bpf syscall is still used to create the pseudo nodes. If they should
> > be persistent they just get registered in the sysfs class hierarchy.
>
> nope. they should not. sysfs is debugging/tunning facility.
> There is absolutely no need for bpf to plug into sysfs.
>
> > > Doing 'resource stats' via sysfs requires bpf to add to sysfs, which
> > > is not this cdev approach.
> >
> > This is not yet part of the patch, but I think this would be added.
> > Daniel?
>
> please don't. I'm strongly against adding unnecessary bloat.
>
> > I don't think there are broad differences. But in case a namespaces uses
> > huge number of maps with tons of data, the admin in the initial
> > namespace might want to debug that without searching all mountpoints and
> > find dependencies between processes etc. IMHO sysfs approach can be
> > better extended here.
>
> sure, then we can force all bpffs to have the same hierarchy and mounted
> in /sys/kernel/bpf location. That would be the same.

That would imply to have a mount_single() file system (like f.e. tracefs and
securityfs), right?

So you'd loose having various mounts in different namespaces. And if you
allow various mount points, how would that /facilitate/ to an admin to
identify all eBPF objects/resources currently present in the system?

Or to an application developer finding possible mount points for his own
application so that bpf(2) syscall to create these nodes succeeds? Would
you make mounting also unprivileged? What if various distros have these
mount points at different locations? What should unprivileged applications
do to know that they can use these locations for themselves?

Also, since they are only regular files, one can only try and find these
objects based on their naming schemes, which seems to get a bit odd in case
this file system also carries (perhaps future?) other regular files that
are not eBPF map and program-special.

> It feels you're pushing for cdev only because of that potential
> debugging need. Did you actually face that need? I didn't and
> don't like to add 'nice to have' feature until real need comes.

I think this discussion arose, because the question of how flexible we are
in future to extend this facility. Nothing more.

I still don't consider the cdev approach as a hack. Despite a bit "old
school" as you say, the semantics seem to be better defined, imho. It
integrates better into existing facilities.

Cheers,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/