From: Song Liu <songliubraving@fb.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
"davem@davemloft.net" <davem@davemloft.net>,
"daniel@iogearbox.net" <daniel@iogearbox.net>,
"x86@kernel.org" <x86@kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
"Kernel Team" <Kernel-team@fb.com>
Subject: Re: [PATCH v2 bpf-next 03/17] bpf: Introduce BPF trampoline
Date: Thu, 7 Nov 2019 23:07:21 +0000 [thread overview]
Message-ID: <FABEB3EB-2AC4-43F8-984B-EFD1DA621A3E@fb.com> (raw)
In-Reply-To: <20191107225553.vnnos6nblxlwx24a@ast-mbp.dhcp.thefacebook.com>
> On Nov 7, 2019, at 2:55 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Nov 07, 2019 at 10:37:19PM +0000, Song Liu wrote:
>>
>>
>>> On Nov 6, 2019, at 9:46 PM, Alexei Starovoitov <ast@kernel.org> wrote:
>>>
>>
>> [...]
>>
>>>
>>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>>> Acked-by: Andrii Nakryiko <andriin@fb.com>
>>> ---
>>> arch/x86/net/bpf_jit_comp.c | 227 ++++++++++++++++++++++++++++++--
>>> include/linux/bpf.h | 98 ++++++++++++++
>>> include/uapi/linux/bpf.h | 2 +
>>> kernel/bpf/Makefile | 1 +
>>> kernel/bpf/btf.c | 77 ++++++++++-
>>> kernel/bpf/core.c | 1 +
>>> kernel/bpf/syscall.c | 53 +++++++-
>>> kernel/bpf/trampoline.c | 252 ++++++++++++++++++++++++++++++++++++
>>> kernel/bpf/verifier.c | 39 ++++++
>>> 9 files changed, 732 insertions(+), 18 deletions(-)
>>> create mode 100644 kernel/bpf/trampoline.c
>>>
>>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>>> index 8631d3bd637f..44169e8bffc0 100644
>>> --- a/arch/x86/net/bpf_jit_comp.c
>>> +++ b/arch/x86/net/bpf_jit_comp.c
>>> @@ -98,6 +98,7 @@ static int bpf_size_to_x86_bytes(int bpf_size)
>>>
>>> /* Pick a register outside of BPF range for JIT internal work */
>>> #define AUX_REG (MAX_BPF_JIT_REG + 1)
>>> +#define X86_REG_R9 (MAX_BPF_JIT_REG + 2)
>>>
>>> /*
>>> * The following table maps BPF registers to x86-64 registers.
>>> @@ -123,6 +124,7 @@ static const int reg2hex[] = {
>>> [BPF_REG_FP] = 5, /* RBP readonly */
>>> [BPF_REG_AX] = 2, /* R10 temp register */
>>> [AUX_REG] = 3, /* R11 temp register */
>>> + [X86_REG_R9] = 1, /* R9 register, 6th function argument */
>>
>> We should update the comment above this:
>>
>> * Also x86-64 register R9 is unused. ...
>
> good point. fixed.
>
>>> + /* One half of the page has active running trampoline.
>>> + * Another half is an area for next trampoline.
>>> + * Make sure the trampoline generation logic doesn't overflow.
>>> + */
>>> + if (WARN_ON_ONCE(prog - (u8 *)image > PAGE_SIZE / 2 - BPF_INSN_SAFETY))
>>> + return -EFAULT;
>>
>> Given max number of args, can we catch this error at compile time?
>
> I don't see how to do that. I was thinking about having fake __init function
> that would call it with flags that can generate the longest trampoline, but
> it's not fool proof either.
> So I've added a test for it instead. See patch 10.
>
>>> +
>>> +static int bpf_trampoline_update(struct bpf_prog *prog)
>>
>> Seems argument "prog" is not used at all?
>
> like one below ? ;)
e... I was really dumb... sorry..
Maybe we should just pass the tr in?
>
>>> +{
>>> + struct bpf_trampoline *tr = prog->aux->trampoline;
>>> + void *old_image = tr->image + ((tr->selector + 1) & 1) * PAGE_SIZE/2;
>>> + void *new_image = tr->image + (tr->selector & 1) * PAGE_SIZE/2;
>>> + if (err)
>>> + goto out;
>>> + tr->selector++;
>>
>> Shall we do selector-- for unlink?
>
> It's a bit flip. I think it would be more confusing with --
Right.. Maybe should use int instead of u64 for selector?
next prev parent reply other threads:[~2019-11-07 23:07 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-07 5:46 [PATCH v2 bpf-next 00/17] Introduce BPF trampoline Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 01/17] bpf: refactor x86 JIT into helpers Alexei Starovoitov
2019-11-07 17:05 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 02/17] bpf: Add bpf_arch_text_poke() helper Alexei Starovoitov
2019-11-07 17:20 ` Song Liu
2019-11-07 17:50 ` Alexei Starovoitov
2019-11-07 17:54 ` Alexei Starovoitov
2019-11-07 18:04 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 03/17] bpf: Introduce BPF trampoline Alexei Starovoitov
2019-11-07 22:37 ` Song Liu
2019-11-07 22:55 ` Alexei Starovoitov
2019-11-07 23:07 ` Song Liu [this message]
2019-11-07 23:09 ` Alexei Starovoitov
2019-11-07 23:16 ` Song Liu
2019-11-08 0:09 ` Alexei Starovoitov
2019-11-08 1:10 ` Song Liu
2019-11-08 3:11 ` Alexei Starovoitov
2019-11-08 4:06 ` Song Liu
2019-11-08 4:08 ` Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 04/17] libbpf: Add support to attach to fentry/fexit tracing progs Alexei Starovoitov
2019-11-07 22:51 ` Song Liu
2019-11-07 23:07 ` Alexei Starovoitov
2019-11-07 23:13 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 05/17] selftest/bpf: Simple test for fentry/fexit Alexei Starovoitov
2019-11-07 23:22 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 06/17] bpf: Add kernel test functions for fentry testing Alexei Starovoitov
2019-11-07 23:28 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 07/17] selftests/bpf: Add test for BPF trampoline Alexei Starovoitov
2019-11-08 1:17 ` Song Liu
2019-11-08 2:33 ` Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 08/17] selftests/bpf: Add fexit tests " Alexei Starovoitov
2019-11-08 1:22 ` Song Liu
2019-11-07 5:46 ` [PATCH v2 bpf-next 09/17] selftests/bpf: Add combined fentry/fexit test Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 10/17] selftests/bpf: Add stress test for maximum number of progs Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 11/17] bpf: Reserver space for BPF trampoline in BPF programs Alexei Starovoitov
2019-11-08 5:03 ` Andrii Nakryiko
2019-11-07 5:46 ` [PATCH v2 bpf-next 12/17] bpf: Fix race in btf_resolve_helper_id() Alexei Starovoitov
2019-11-08 5:13 ` Andrii Nakryiko
2019-11-08 5:31 ` Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 13/17] bpf: Compare BTF types of functions arguments with actual types Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 14/17] bpf: Support attaching tracing BPF program to other BPF programs Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 15/17] selftests/bpf: Extend test_pkt_access test Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 16/17] libbpf: Add support for attaching BPF programs to other BPF programs Alexei Starovoitov
2019-11-07 14:33 ` Alexei Starovoitov
2019-11-07 5:46 ` [PATCH v2 bpf-next 17/17] selftests/bpf: Add a test for attaching BPF prog to another BPF prog and subprog Alexei Starovoitov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=FABEB3EB-2AC4-43F8-984B-EFD1DA621A3E@fb.com \
--to=songliubraving@fb.com \
--cc=Kernel-team@fb.com \
--cc=alexei.starovoitov@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).