Re: [PATCH net] bpf: split eBPF out of NET

[Daniel Borkmann]

On 10/24/2014 10:11 AM, Josh Triplett wrote:
> On Thu, Oct 23, 2014 at 10:32:50PM -0700, Alexei Starovoitov wrote:
> > On Thu, Oct 23, 2014 at 8:23 PM, Josh Triplett <josh@xxxxxxxxxxxxxxxx> wrote:
> > > On Thu, Oct 23, 2014 at 06:41:08PM -0700, Alexei Starovoitov wrote:
> > > > introduce two configs:
> > > > - hidden CONFIG_BPF to select eBPF interpreter that classic socket filters
> > > >    depend on
> > > > - visible CONFIG_BPF_SYSCALL (default off) that tracing and sockets can use
> > > >
> > > > that solves several problems:
> > > > - tracing and others that wish to use eBPF don't need to depend on NET.
> > > >    They can use BPF_SYSCALL to allow loading from userspace or select BPF
> > > >    to use it directly from kernel in NET-less configs.
> > > > - in 3.18 programs cannot be attached to events yet, so don't force it on
> > > > - when the rest of eBPF infra is there in 3.19+, it's still useful to
> > > >    switch it off to minimize kernel size
> > > >
> > > > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxxxx>
> > >
> > > Thanks for working on this!  A few nits below, but otherwise this looks
> > > good to me.  Once this gets appropriate reviews from net and bpf folks,
> > > please let me know if you want this to go through the net tree, the tiny
> > > tree, or some other tree.
> >
> > Thanks :)
> > I've sent it to Dave and marked it as 'net', so it's for
> > his net tree. I don't mind if he decides to steer it into net-next
> > when it opens, since changing Kconfig is always tricky.
> > I just felt that this patch deserves to be in 'net' and in 3.18-rc
>
> Ah, nice; yes, getting it into 3.18-rc would be excellent if possible.

Fully agreed, BPF_SYSCALL defaulting to 'n' _for the time being_
would also give an option for reducing exposure until the API is
further stabilized and in a ready-to-use state.

> > > > bloat-o-meter on x64 shows:
> > > > add/remove: 0/60 grow/shrink: 0/2 up/down: 0/-15601 (-15601)
> > >
> > > Very nice!  Please do include the bloat-o-meter stats in the commit
> > > message.
> >
> > I don't think that's necessary. eBPF is in early stages of adoption.
> > More things to come, so bloat-o-meter stats will be obsolete
> > very quickly.
>
> I don't mean the full list of symbols, just the summary saying this
> saves 15k.

It might probably help to more easily identify from the log which
commits are related to a tinyfication perspective. Perhaps Dave can
still squash that into the commit log.

> > > > +# interpreter that classic socket filters depend on
> > > > +config BPF
> > > > +     boolean
> > >
> > > s/boolean/bool/
> >
> > Is there a difference? I thought it's an alias.
>
> It's an alias, but almost everything uses "bool":
>
> ~/src/linux$ git grep -w bool -- '*Kconfig*' | wc -l
> 7064
> ~/src/linux$ git grep -w boolean -- '*Kconfig*' | wc -l
> 94

Actually, shouldn't we get rid of the alias then? Same accounts
for def_bool and def_boolean ... it would help to avoid confusion
to just have a single term for each.

Anyway, the rest looks good to me, thanks.

I am totally fine of having it under EXPERT for now for the reasons
mentioned above. This can still be lifted later on.

Acked-by: Daniel Borkmann <dborkman@xxxxxxxxxx>
--
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/