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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAMEtUuw1q_=CuZOHqd1WDd8ZE+bo-CNsbQ4rFPgRRVy7H1Q4uA@mail.gmail.com>
Date:	Thu, 20 Nov 2014 13:07:29 -0800
From:	Alexei Starovoitov <ast@...mgrid.com>
To:	Denis Kirjanov <kda@...ux-powerpc.org>
Cc:	Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] filter: add bpf_optimize_div()

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.

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.

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