netdev - Re: [PATCH net-next] filter: add bpf_optimize

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAOJe8K13=RXfUccVqLk-73-STJC9b2n5OL=ZRwySeOCtVmi6kw@mail.gmail.com>
Date:	Fri, 21 Nov 2014 13:22:03 +0400
From:	Denis Kirjanov <kda@...ux-powerpc.org>
To:	Alexei Starovoitov <ast@...mgrid.com>
Cc:	Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] filter: add bpf_optimize_div()

On 11/21/14, Alexei Starovoitov <ast@...mgrid.com> wrote:
> On Thu, Nov 20, 2014 at 11:42 AM, Denis Kirjanov <kda@...ux-powerpc.org>
> wrote:
>> optimize_div() found in  mips bpf jit is really usefull
>> for other arches. So let's put it in filter.h
>>
>> CC: Alexei Starovoitov <ast@...mgrid.com>
>> Signed-off-by: Denis Kirjanov <kda@...ux-powerpc.org>
>> ---
>>  include/linux/filter.h | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index ca95abd..b385637 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> +
>> +static inline int bpf_optimize_div(u32 *k)
>
> 'inline' is unnecessary
>
>> +{
>> +       /* power of 2 divides can be implemented with right shift */
>> +       if (!(*k & (*k-1))) {
>> +               *k = ilog2(*k);
>> +               return 1;
>> +       }
>> +
>> +       return 0;
>> +}
>
> I don't think it makes sense to add a helper function
> without first user. If you really want to use it
> in ppc, make this change as part of the series.
> In the 1st patch move it out of mips and use it in
> mips jit and 2nd patch use it in ppc.

Heh, actually we already have it: is_power_of_2()

>
> Also since you mentioned mips...
> if (k == 1 || optimize_div(&k)) {
>   ctx->flags |= SEEN_A;
>   emit_jit_reg_move(r_A, r_zero, ctx);
> this is definitely wrong in there.
>
I'll check it

> Also with such helper function we'd need to be
> careful not to modify 'k' in place, otherwise
> multi-pass jit algorithm would be broken.
>
> In general I don't think such optimizations
> belong in kernel at all. User space should be doing this.
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html