Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1471244

Re: [RFC v2 09/10] landlock: Handle cgroups (performance)

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On 27/08/2016 20:06, Alexei Starovoitov wrote:
> On Sat, Aug 27, 2016 at 04:06:38PM +0200, Mickaël Salaün wrote:
>>
>> On 27/08/2016 01:05, Alexei Starovoitov wrote:
>>> On Fri, Aug 26, 2016 at 05:10:40PM +0200, Mickaël Salaün wrote:
>>>>
>>>>>
>>>>> - I don't think such 'for' loop can scale. The solution needs to work
>>>>> with thousands of containers and thousands of cgroups.
>>>>> In the patch 06/10 the proposal is to use 'current' as holder of
>>>>> the programs:
>>>>> +   for (prog = current->seccomp.landlock_prog;
>>>>> +                   prog; prog = prog->prev) {
>>>>> +           if (prog->filter == landlock_ret->filter) {
>>>>> +                   cur_ret = BPF_PROG_RUN(prog->prog, (void *)&ctx);
>>>>> +                   break;
>>>>> +           }
>>>>> +   }
>>>>> imo that's the root of scalability issue.
>>>>> I think to be able to scale the bpf programs have to be attached to
>>>>> cgroups instead of tasks.
>>>>> That would be very different api. seccomp doesn't need to be touched.
>>>>> But that is the only way I see to be able to scale.
>>>>
>>>> Landlock is inspired from seccomp which also use a BPF program per
>>>> thread. For seccomp, each BPF programs are executed for each syscall.
>>>> For Landlock, some BPF programs are executed for some LSM hooks. I don't
>>>> see why it is a scale issue for Landlock comparing to seccomp. I also
>>>> don't see why storing the BPF program list pointer in the cgroup struct
>>>> instead of the task struct change a lot here. The BPF programs execution
>>>> will be the same anyway (for each LSM hook). Kees should probably have a
>>>> better opinion on this.
>>>
>>> seccomp has its own issues and copying them doesn't make this lsm any better.
>>> Like seccomp bpf programs are all gigantic switch statement that looks
>>> for interesting syscall numbers. All syscalls of a task are paying
>>> non-trivial seccomp penalty due to such design. If bpf was attached per
>>> syscall it would have been much cheaper. Of course doing it this way
>>> for seccomp is not easy, but for lsm such facility is already there.
>>> Blank call of a single bpf prog for all lsm hooks is unnecessary
>>> overhead that can and should be avoided.
>>
>> It's probably a misunderstanding. Contrary to seccomp which run all the
>> thread's BPF programs for any syscall, Landlock only run eBPF programs
>> for the triggered LSM hooks, if their type match. Indeed, thanks to the
>> multiple eBPF program types and contrary to seccomp, Landlock only run
>> an eBPF program when needed. Landlock will have almost no performance
>> overhead if the syscalls do not trigger the watched LSM hooks for the
>> current process.
> 
> that's not what I see in the patch 06/10:
> all lsm_hooks in 'static struct security_hook_list landlock_hooks'
> (which eventually means all lsm hooks) will call
> static inline int landlock_hook_##NAME
> which will call landlock_run_prog()
> which does:
> + for (landlock_ret = current->seccomp.landlock_ret;
> +      landlock_ret; landlock_ret = landlock_ret->prev) {
> +    if (landlock_ret->triggered) {
> +       ctx.cookie = landlock_ret->cookie;
> +       for (prog = current->seccomp.landlock_prog;
> +            prog; prog = prog->prev) {
> +               if (prog->filter == landlock_ret->filter) {
> +                       cur_ret = BPF_PROG_RUN(prog->prog, (void *)&ctx);
> +                       break;
> +               }
> +       }
> 
> that is unacceptable overhead and not a scalable design.
> It kinda works for 3 lsm_hooks as in patch 6, but doesn't scale
> as soon as more lsm hooks are added.

Good catch! I forgot to check the program (sub)type in the loop to only
run the needed programs for the current hook. I will fix this.


> 
>> As said above, Landlock will not run an eBPF programs when not strictly
>> needed. Attaching to a cgroup will have the same performance impact as
>> attaching to a process hierarchy.
> 
> Having a prog per cgroup per lsm_hook is the only scalable way I
> could come up with. If you see another way, please propose.
> current->seccomp.landlock_prog is not the answer.

Hum, I don't see the difference from a performance point of view between
a cgroup-based or a process hierarchy-based system.

Maybe a better option should be to use an array of pointers with N
entries, one for each supported hook, instead of a unique pointer list?

Anyway, being able to attach an LSM hook program to a cgroup thanks to
the new BPF_PROG_ATTACH seems a good idea (while keeping the possibility
to use a process hierarchy). The downside will be to handle an LSM hook
program which is not triggered by a seccomp-filter, but this should be
needed anyway to handle interruptions.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC v2 09/10] landlock: Handle cgroups Mickaël Salaün <mic@digikod.net> - 2016-08-25 12:50 +0200
  Re: [RFC v2 09/10] landlock: Handle cgroups Andy Lutomirski <luto@amacapital.net> - 2016-08-25 13:20 +0200
    Re: [RFC v2 09/10] landlock: Handle cgroups Mickaël Salaün <mic@digikod.net> - 2016-08-25 16:50 +0200
      Re: [RFC v2 09/10] landlock: Handle cgroups Tejun Heo <tj@kernel.org> - 2016-08-26 15:00 +0200
      Re: [RFC v2 09/10] landlock: Handle cgroups Andy Lutomirski <luto@amacapital.net> - 2016-08-26 16:30 +0200
        Re: [RFC v2 09/10] landlock: Handle cgroups Tejun Heo <tj@kernel.org> - 2016-08-26 18:00 +0200
  Re: [RFC v2 09/10] landlock: Handle cgroups Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-26 04:30 +0200
    Re: [RFC v2 09/10] landlock: Handle cgroups Mickaël Salaün <mic@digikod.net> - 2016-08-26 17:20 +0200
      Re: [RFC v2 09/10] landlock: Handle cgroups Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 01:10 +0200
        Re: [RFC v2 09/10] landlock: Handle cgroups Andy Lutomirski <luto@amacapital.net> - 2016-08-27 09:40 +0200
          Re: [RFC v2 09/10] landlock: Handle cgroups Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 20:20 +0200
            Re: [RFC v2 09/10] landlock: Handle cgroups Andy Lutomirski <luto@amacapital.net> - 2016-08-28 10:20 +0200
        Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-27 16:20 +0200
          Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 20:10 +0200
            Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-27 21:40 +0200
              Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 22:50 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-27 23:20 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Andy Lutomirski <luto@amacapital.net> - 2016-08-28 10:20 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-28 11:50 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Andy Lutomirski <luto@amacapital.net> - 2016-08-30 21:00 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-30 22:30 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Andy Lutomirski <luto@amacapital.net> - 2016-08-30 22:30 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Mickaël Salaün <mic@digikod.net> - 2016-08-30 22:40 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-30 23:00 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Andy Lutomirski <luto@amacapital.net> - 2016-08-30 23:50 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-31 03:40 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (performance) Andy Lutomirski <luto@amacapital.net> - 2016-08-31 05:30 +0200
        Re: [RFC v2 09/10] landlock: Handle cgroups (netfilter match) Mickaël Salaün <mic@digikod.net> - 2016-08-27 16:30 +0200
          Re: [RFC v2 09/10] landlock: Handle cgroups (netfilter match) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 20:40 +0200
        Re: [RFC v2 09/10] landlock: Handle cgroups (program types) Mickaël Salaün <mic@digikod.net> - 2016-08-27 16:40 +0200
          Re: [RFC v2 09/10] landlock: Handle cgroups (program types) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 20:30 +0200
            Re: [RFC v2 09/10] landlock: Handle cgroups (program types) Mickaël Salaün <mic@digikod.net> - 2016-08-27 22:00 +0200
              Re: [RFC v2 09/10] landlock: Handle cgroups (program types) Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-27 23:00 +0200
                Re: [RFC v2 09/10] landlock: Handle cgroups (program types) Mickaël Salaün <mic@digikod.net> - 2016-08-27 23:20 +0200

csiph-web