netdev - Re: [PATCH v9 2/6] cgroup: add support for eBPF programs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0f73e359-e3d9-4344-ecae-7a03945aaf8b@zonque.org>
Date:   Thu, 24 Nov 2016 09:21:58 +0100
From:   Daniel Mack <daniel@...que.org>
To:     Rami Rosen <roszenrami@...il.com>
Cc:     htejun@...com, daniel@...earbox.net, ast@...com,
        David Miller <davem@...emloft.net>, kafai@...com, fw@...len.de,
        pablo@...filter.org, harald@...hat.com,
        Netdev <netdev@...r.kernel.org>,
        Sargun Dhillon <sargun@...gun.me>, cgroups@...r.kernel.org
Subject: Re: [PATCH v9 2/6] cgroup: add support for eBPF programs

Hi Rami,

On 11/23/2016 11:46 PM, Rami Rosen wrote:
> A minor comment:
> 
>> +/**
>> + * __cgroup_bpf_update() - Update the pinned program of a cgroup, and
>> + *                         propagate the change to descendants
>> + * @cgrp: The cgroup which descendants to traverse
>> + * @parent: The parent of @cgrp, or %NULL if @cgrp is the root
>> + * @prog: A new program to pin
>> + * @type: Type of pinning operation (ingress/egress)
>> + *
>> + * Each cgroup has a set of two pointers for bpf programs; one for eBPF
>> + * programs it owns, and which is effective for execution.
>> + *
> You have in the following section twice identical checks, for If @prog
> is %NULL".
> Shouldn't it be here (in the first case only) "If @prog is not %NULL"
> instead "If @prog is %NULL"?

Yes, you're right, thanks for spotting.

If possible, I would like to not send a v10 just for this one missing
word in the comments though, but rather fix that up in a separate patch
afterwards.


Thanks,
Daniel


> 
>> + * If @prog is %NULL, this function attaches a new program to the cgroup and
>> + * releases the one that is currently attached, if any. @prog is then made
>> + * the effective program of type @type in that cgroup.
>> + *
>> + * If @prog is %NULL, the currently attached program of type @type is released,
>> + * and the effective program of the parent cgroup (if any) is inherited to
>> + * @cgrp.
>> + *
> 
> 
> Regard,
> Rami Rosen
>