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

Groups > linux.kernel > #1651425 > unrolled thread

Re: [PATCH 5/5] MIPS: Add support for eBPF JIT.

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 5/5] MIPS: Add support for eBPF JIT. Daniel Borkmann <daniel@iogearbox.net> - 2017-05-26 17:20 +0200
    Re: [PATCH 5/5] MIPS: Add support for eBPF JIT. Daniel Borkmann <daniel@iogearbox.net> - 2017-05-26 17:40 +0200
    Re: [PATCH 5/5] MIPS: Add support for eBPF JIT. Daniel Borkmann <daniel@iogearbox.net> - 2017-05-26 21:20 +0200
      Re: [PATCH 5/5] MIPS: Add support for eBPF JIT. Daniel Borkmann <daniel@iogearbox.net> - 2017-05-26 21:30 +0200

#1651425 — Re: [PATCH 5/5] MIPS: Add support for eBPF JIT.

On 05/26/2017 02:38 AM, David Daney wrote:
> Since the eBPF machine has 64-bit registers, we only support this in
> 64-bit kernels.  As of the writing of this commit log test-bpf is showing:
>
>    test_bpf: Summary: 316 PASSED, 0 FAILED, [308/308 JIT'ed]
>
> All current test cases are successfully compiled.
>
> Signed-off-by: David Daney <david.daney@cavium.com>

Awesome work!

Did you also manage to run tools/testing/selftests/bpf/ fine with
the JIT enabled?

[...]
> +struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
> +{
> +	struct jit_ctx ctx;
> +	unsigned int alloc_size;
> +
> +	/* Only 64-bit kernel supports eBPF */
> +	if (!IS_ENABLED(CONFIG_64BIT) || !bpf_jit_enable)

Isn't this already reflected by the following?

   select HAVE_EBPF_JIT if (64BIT && !CPU_MICROMIPS)

> +		return prog;
> +
> +	memset(&ctx, 0, sizeof(ctx));
> +
> +	ctx.offsets = kcalloc(prog->len + 1, sizeof(*ctx.offsets), GFP_KERNEL);
> +	if (ctx.offsets == NULL)
> +		goto out;
> +
> +	ctx.reg_val_types = kcalloc(prog->len + 1, sizeof(*ctx.reg_val_types), GFP_KERNEL);
> +	if (ctx.reg_val_types == NULL)
> +		goto out;
> +
> +	ctx.skf = prog;
> +
> +	if (reg_val_propagate(&ctx))
> +		goto out;
> +
> +	/* First pass discovers used resources */
> +	if (build_int_body(&ctx))
> +		goto out;
> +
> +	/* Second pass generates offsets */
> +	ctx.idx = 0;
> +	if (gen_int_prologue(&ctx))
> +		goto out;
> +	if (build_int_body(&ctx))
> +		goto out;
> +	if (build_int_epilogue(&ctx))
> +		goto out;
> +
> +	alloc_size = 4 * ctx.idx;
> +
> +	ctx.target = module_alloc(alloc_size);

You would need to use bpf_jit_binary_alloc() like all other
eBPF JITs do, otherwise kallsyms of the JITed progs would
break.

> +	if (ctx.target == NULL)
> +		goto out;
> +
> +	/* Clean it */
> +	memset(ctx.target, 0, alloc_size);
> +
> +	/* Third pass generates the code */
> +	ctx.idx = 0;
> +	if (gen_int_prologue(&ctx))
> +		goto out;
> +	if (build_int_body(&ctx))
> +		goto out;
> +	if (build_int_epilogue(&ctx))
> +		goto out;
> +	/* Update the icache */
> +	flush_icache_range((ptr)ctx.target, (ptr)(ctx.target + ctx.idx));
> +
> +	if (bpf_jit_enable > 1)
> +		/* Dump JIT code */
> +		bpf_jit_dump(prog->len, alloc_size, 2, ctx.target);
> +
> +	prog->bpf_func = (void *)ctx.target;
> +	prog->jited = 1;
> +
> +out:
> +	kfree(ctx.offsets);
> +	kfree(ctx.reg_val_types);
> +
> +	return prog;
> +}

[toc] | [next] | [standalone]

#1651433

On 05/26/2017 05:14 PM, Daniel Borkmann wrote:
> On 05/26/2017 02:38 AM, David Daney wrote:
>> Since the eBPF machine has 64-bit registers, we only support this in
>> 64-bit kernels.  As of the writing of this commit log test-bpf is showing:
>>
>>    test_bpf: Summary: 316 PASSED, 0 FAILED, [308/308 JIT'ed]
>>
>> All current test cases are successfully compiled.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>
> Awesome work!
>
> Did you also manage to run tools/testing/selftests/bpf/ fine with
> the JIT enabled?
>
> [...]
>> +struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>> +{
>> +    struct jit_ctx ctx;
>> +    unsigned int alloc_size;
>> +
>> +    /* Only 64-bit kernel supports eBPF */
>> +    if (!IS_ENABLED(CONFIG_64BIT) || !bpf_jit_enable)
>
> Isn't this already reflected by the following?
>
>    select HAVE_EBPF_JIT if (64BIT && !CPU_MICROMIPS)

Oh, overlooked that you keep both JITs in the same file. ppc and
sparc also carry cBPF JITs, but strictly separated at compile time,
x86 threw out the cBPF one and only uses eBPF. Have you considered
separating them as well (which the current model assumes right now)?
(Need to double check all assumption we currently make and whether
they would still hold, but separation like all others do would
definitely be preferred.)

>> +        return prog;
>> +
>> +    memset(&ctx, 0, sizeof(ctx));
>> +
>> +    ctx.offsets = kcalloc(prog->len + 1, sizeof(*ctx.offsets), GFP_KERNEL);
>> +    if (ctx.offsets == NULL)
>> +        goto out;
>> +
>> +    ctx.reg_val_types = kcalloc(prog->len + 1, sizeof(*ctx.reg_val_types), GFP_KERNEL);
>> +    if (ctx.reg_val_types == NULL)
>> +        goto out;
>> +
>> +    ctx.skf = prog;
>> +
>> +    if (reg_val_propagate(&ctx))
>> +        goto out;
>> +
>> +    /* First pass discovers used resources */
>> +    if (build_int_body(&ctx))
>> +        goto out;
>> +
>> +    /* Second pass generates offsets */
>> +    ctx.idx = 0;
>> +    if (gen_int_prologue(&ctx))
>> +        goto out;
>> +    if (build_int_body(&ctx))
>> +        goto out;
>> +    if (build_int_epilogue(&ctx))
>> +        goto out;
>> +
>> +    alloc_size = 4 * ctx.idx;
>> +
>> +    ctx.target = module_alloc(alloc_size);
>
> You would need to use bpf_jit_binary_alloc() like all other
> eBPF JITs do, otherwise kallsyms of the JITed progs would
> break.
>
>> +    if (ctx.target == NULL)
>> +        goto out;
>> +
>> +    /* Clean it */
>> +    memset(ctx.target, 0, alloc_size);
>> +
>> +    /* Third pass generates the code */
>> +    ctx.idx = 0;
>> +    if (gen_int_prologue(&ctx))
>> +        goto out;
>> +    if (build_int_body(&ctx))
>> +        goto out;
>> +    if (build_int_epilogue(&ctx))
>> +        goto out;
>> +    /* Update the icache */
>> +    flush_icache_range((ptr)ctx.target, (ptr)(ctx.target + ctx.idx));
>> +
>> +    if (bpf_jit_enable > 1)
>> +        /* Dump JIT code */
>> +        bpf_jit_dump(prog->len, alloc_size, 2, ctx.target);
>> +
>> +    prog->bpf_func = (void *)ctx.target;
>> +    prog->jited = 1;
>> +
>> +out:
>> +    kfree(ctx.offsets);
>> +    kfree(ctx.reg_val_types);
>> +
>> +    return prog;
>> +}

[toc] | [prev] | [next] | [standalone]

#1651581

On 05/26/2017 05:39 PM, David Daney wrote:
> On 05/26/2017 08:14 AM, Daniel Borkmann wrote:
>> On 05/26/2017 02:38 AM, David Daney wrote:
>>> Since the eBPF machine has 64-bit registers, we only support this in
>>> 64-bit kernels.  As of the writing of this commit log test-bpf is showing:
>>>
>>>    test_bpf: Summary: 316 PASSED, 0 FAILED, [308/308 JIT'ed]
>>>
>>> All current test cases are successfully compiled.
>>>
>>> Signed-off-by: David Daney <david.daney@cavium.com>
>>
>> Awesome work!
>>
>> Did you also manage to run tools/testing/selftests/bpf/ fine with
>> the JIT enabled?
>
> I haven't done that yet, I will before the next revision.
>
>> [...]
>>> +struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>> +{
>>> +    struct jit_ctx ctx;
>>> +    unsigned int alloc_size;
>>> +
>>> +    /* Only 64-bit kernel supports eBPF */
>>> +    if (!IS_ENABLED(CONFIG_64BIT) || !bpf_jit_enable)
>>
>> Isn't this already reflected by the following?
>>
>>    select HAVE_EBPF_JIT if (64BIT && !CPU_MICROMIPS)
>
> Not exactly.  The eBPF JIT is in the same file as the classic-BPF JIT, so when HAVE_EBPF_JIT is false this will indeed never be called.  But the kernel would otherwise contain all the JIT code.
>
> By putting in !IS_ENABLED(CONFIG_64BIT) we allow gcc to eliminate all the dead code when compiling the JITs.

Side-effect would still be that for cBPF you go through the cBPF
JIT instead of letting the kernel convert all cBPF to eBPF and
later on go through your eBPF JIT. If you still prefer to have
everything in one single file and let gcc eliminate dead code
then you can just do single line change ...

void bpf_jit_compile(struct bpf_prog *fp)
{
         struct jit_ctx ctx;
         unsigned int alloc_size, tmp_idx;

         if (IS_ENABLED(CONFIG_HAVE_EBPF_JIT) || !bpf_jit_enable)
                 return;
         [...]
}

... and bpf_prog_ebpf_jited() et al wouldn't need to be changed
in the core, which are used in kallsyms, and kernel will then
also be able to automatically JIT all of seccomp-BPF and the
missing cBPF extensions we have through the eBPF JIT w/o extra
work.

[toc] | [prev] | [next] | [standalone]

#1651586

On 05/26/2017 09:20 PM, David Daney wrote:
> On 05/26/2017 12:09 PM, Daniel Borkmann wrote:
>> On 05/26/2017 05:39 PM, David Daney wrote:
>>> On 05/26/2017 08:14 AM, Daniel Borkmann wrote:
>>>> On 05/26/2017 02:38 AM, David Daney wrote:
>>>>> Since the eBPF machine has 64-bit registers, we only support this in
>>>>> 64-bit kernels.  As of the writing of this commit log test-bpf is showing:
>>>>>
>>>>>    test_bpf: Summary: 316 PASSED, 0 FAILED, [308/308 JIT'ed]
>>>>>
>>>>> All current test cases are successfully compiled.
>>>>>
>>>>> Signed-off-by: David Daney <david.daney@cavium.com>
>>>>
>>>> Awesome work!
>>>>
>>>> Did you also manage to run tools/testing/selftests/bpf/ fine with
>>>> the JIT enabled?
>>>
>>> I haven't done that yet, I will before the next revision.
>>>
>>>> [...]
>>>>> +struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
>>>>> +{
>>>>> +    struct jit_ctx ctx;
>>>>> +    unsigned int alloc_size;
>>>>> +
>>>>> +    /* Only 64-bit kernel supports eBPF */
>>>>> +    if (!IS_ENABLED(CONFIG_64BIT) || !bpf_jit_enable)
>>>>
>>>> Isn't this already reflected by the following?
>>>>
>>>>    select HAVE_EBPF_JIT if (64BIT && !CPU_MICROMIPS)
>>>
>>> Not exactly.  The eBPF JIT is in the same file as the classic-BPF JIT, so when HAVE_EBPF_JIT is false this will indeed never be called.  But the kernel would otherwise contain all the JIT code.
>>>
>>> By putting in !IS_ENABLED(CONFIG_64BIT) we allow gcc to eliminate all the dead code when compiling the JITs.
>>
>> Side-effect would still be that for cBPF you go through the cBPF
>> JIT instead of letting the kernel convert all cBPF to eBPF and
>> later on go through your eBPF JIT. If you still prefer to have
>> everything in one single file and let gcc eliminate dead code
>> then you can just do single line change ...
>>
>> void bpf_jit_compile(struct bpf_prog *fp)
>> {
>>          struct jit_ctx ctx;
>>          unsigned int alloc_size, tmp_idx;
>>
>>          if (IS_ENABLED(CONFIG_HAVE_EBPF_JIT) || !bpf_jit_enable)
>>                  return;
>
> Yes.  In fact I did that for testing.
>
> The cBPF JIT generates smaller code for:
>
> test_bpf: #274 BPF_MAXINSNS: ld_abs+get_processor_id jited:1 44128 PASS
>
> When we attempt to use the eBPF JIT for this, some of the MIPS branch instructions cannot reach their targets (+- 32K instructions).  I didn't feel like fixing the code generation quite yet to handle branches that span more than 32K instructions, so I left the cBPF in place so I could claim that all of the test cases were JITed :-)
>
> For the next revision of the patch I will revisit this.

Okay, sounds good!

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web