netdev - Re: [PATCH/RFC bpf-next 06/16] bpf: new sysctl "bpf_jit_32bit

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: <352EED46-6368-4A25-B0DC-23D1E736C7A1@netronome.com>
Date:   Wed, 27 Mar 2019 17:06:01 +0000
From:   Jiong Wang <jiong.wang@...ronome.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Daniel Borkmann <daniel@...earbox.net>, bpf@...r.kernel.org,
        netdev@...r.kernel.org, oss-drivers@...ronome.com
Subject: Re: [PATCH/RFC bpf-next 06/16] bpf: new sysctl "bpf_jit_32bit_opt"


> On 27 Mar 2019, at 17:00, Alexei Starovoitov <alexei.starovoitov@...il.com> wrote:
> 
> On Tue, Mar 26, 2019 at 06:05:29PM +0000, Jiong Wang wrote:
>> After previous patches, verifier has marked those instructions that really
>> need zero extension on dst_reg.
>> 
>> It is then for all back-ends to decide how to use such information to
>> eliminate unnecessary zero extension codegen during JIT compilation.
>> 
>> One approach is:
>>  1. Verifier insert explicit zero extension for those instructions that
>>     need zero extension.
>>  2. All JIT back-ends do NOT generate zero extension for sub-register
>>     write any more.
>> 
>> The good thing for this approach is no major change on JIT back-end
>> interface, all back-ends could get this optimization.
>> 
>> However, only those back-ends that do not have hardware zero extension
>> want this optimization. For back-ends like x86_64 and AArch64, there is
>> hardware support, so this optimization should be disabled.
>> 
>> This patch introduces new sysctl "bpf_jit_32bit_opt" which is the control
>> variable for whether the optimization should be enabled.
>> 
>> It is initialized using target hook bpf_jit_hardware_zext which is default
>> true, meaning the underlying hardware will do zero extension automatically,
>> therefore the optimization will be disabled.
>> 
>> Offload targets do not use this native target hook, instead, they could
>> get the optimization results using bpf_prog_offload_ops.finalize.
>> 
>> The user could always enable or disable the optimization by using:
>> 
>>   sysctl net/core/bpf_jit_32bit_opt=[0 | 1]
> 
> I don't think there should be a sysctl for this.

The sysctl introduced mostly because I think it could be useful for testing.
For example on x86_64, with this sysctl, we can enable the optimisation and
can run selftest.

Does this make sense?

Or when one insn is marked, we print verbose info, so the tester could catch
it from log?
 

> 32-bit archs and ppc/sparc should always do it, since there is no downside
> and for x64/arm64 the verifier won't be inserting shifts because
> bpf_jit_hardware_zext==true.
> We probably need a new insn for this.

Agree, and I mentioned this issue on the ToDo in cover letter.

> I'm guessing shifts are not always
> optimal on all archs. There could be arch specific insns that do it.
> I'd love to hear from arch folks though.
> 

Regards,
Jiong