diff mbox series

[RFC,bpf-next] bpf: allow JIT debugging if CONFIG_BPF_JIT_ALWAYS_ON is set

Message ID 20191106161204.87261-1-iii@linux.ibm.com
State RFC
Delegated to: BPF Maintainers
Headers show
Series [RFC,bpf-next] bpf: allow JIT debugging if CONFIG_BPF_JIT_ALWAYS_ON is set | expand

Commit Message

Ilya Leoshkevich Nov. 6, 2019, 4:12 p.m. UTC
Currently it's not possible to set bpf_jit_enable = 2 when
CONFIG_BPF_JIT_ALWAYS_ON is set, which makes debugging certain problems
harder.

It looks as if it's safe to allow this, because setting this knob
requires root anyway, but I'm not sure about all the security
implications, so sending this as an RFC.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 net/core/sysctl_net_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alexei Starovoitov Nov. 6, 2019, 4:15 p.m. UTC | #1
On Wed, Nov 6, 2019 at 8:12 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> Currently it's not possible to set bpf_jit_enable = 2 when
> CONFIG_BPF_JIT_ALWAYS_ON is set, which makes debugging certain problems
> harder.

This is obsolete way of debugging.
Please use bpftool dump jited instead.
Ilya Leoshkevich Nov. 6, 2019, 4:28 p.m. UTC | #2
> Am 06.11.2019 um 17:15 schrieb Alexei Starovoitov <alexei.starovoitov@gmail.com>:
> 
> On Wed, Nov 6, 2019 at 8:12 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>> 
>> Currently it's not possible to set bpf_jit_enable = 2 when
>> CONFIG_BPF_JIT_ALWAYS_ON is set, which makes debugging certain problems
>> harder.
> 
> This is obsolete way of debugging.
> Please use bpftool dump jited instead.

Is there a way to integrate bpftool nicely with e.g. test_verifier?
With bpf_jit_enable = 2, I can see JITed code for each test right away,
without pausing it (via gdb or rebuilding with added sleep()) and
running bpftool.
John Fastabend Nov. 6, 2019, 4:50 p.m. UTC | #3
Ilya Leoshkevich wrote:
> > Am 06.11.2019 um 17:15 schrieb Alexei Starovoitov <alexei.starovoitov@gmail.com>:
> > 
> > On Wed, Nov 6, 2019 at 8:12 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> >> 
> >> Currently it's not possible to set bpf_jit_enable = 2 when
> >> CONFIG_BPF_JIT_ALWAYS_ON is set, which makes debugging certain problems
> >> harder.
> > 
> > This is obsolete way of debugging.
> > Please use bpftool dump jited instead.
> 
> Is there a way to integrate bpftool nicely with e.g. test_verifier?
> With bpf_jit_enable = 2, I can see JITed code for each test right away,
> without pausing it (via gdb or rebuilding with added sleep()) and
> running bpftool.

On the library side we can set the log_level causing the verifier logic
steps to be printed. I guess adding it to bpftool might be nice. At least
I would find it useful. I'll probably get to it sometime if its not
already there somewhere and/or someone doesn't beat me to it.
Daniel Borkmann Nov. 6, 2019, 11:07 p.m. UTC | #4
On 11/6/19 5:50 PM, John Fastabend wrote:
> Ilya Leoshkevich wrote:
>>> Am 06.11.2019 um 17:15 schrieb Alexei Starovoitov <alexei.starovoitov@gmail.com>:
>>> On Wed, Nov 6, 2019 at 8:12 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>>>
>>>> Currently it's not possible to set bpf_jit_enable = 2 when
>>>> CONFIG_BPF_JIT_ALWAYS_ON is set, which makes debugging certain problems
>>>> harder.
>>>
>>> This is obsolete way of debugging.
>>> Please use bpftool dump jited instead.
>>
>> Is there a way to integrate bpftool nicely with e.g. test_verifier?
>> With bpf_jit_enable = 2, I can see JITed code for each test right away,
>> without pausing it (via gdb or rebuilding with added sleep()) and
>> running bpftool.
> 
> On the library side we can set the log_level causing the verifier logic
> steps to be printed. I guess adding it to bpftool might be nice. At least
> I would find it useful. I'll probably get to it sometime if its not
> already there somewhere and/or someone doesn't beat me to it.

+1

Was wondering whether it may be worth it moving parts of the logic from bpftool
into libbpf wrt jit dump as a higher level api, so it could be used directly for
checking out the jit disasm + opcodes for specific tests given we have the fd
there as well, but that might be too specific perhaps and would bring one more
lib dependency to libbpf for a rather narrow use case. Adding sleep before prog
fd close and/or shelling out to bpftool etc all is a crude temporary hack as
well (currently using something long these lines locally). Would it make sense
to dump some meta data and generated opcodes per test case to a file as opt-in
e.g. ./test_verifier 711 --dump produces 711.opcodes out of bpf_obj_get_info_by_fd()
which then bpftool could dump this artifact through its own disasm?

Thanks,
Daniel
Ilya Leoshkevich Nov. 7, 2019, 3:30 p.m. UTC | #5
> Am 07.11.2019 um 00:07 schrieb Daniel Borkmann <daniel@iogearbox.net>:
> 
> On 11/6/19 5:50 PM, John Fastabend wrote:
>> Ilya Leoshkevich wrote:
>>>> Am 06.11.2019 um 17:15 schrieb Alexei Starovoitov <alexei.starovoitov@gmail.com>:
>>>> On Wed, Nov 6, 2019 at 8:12 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>>>> 
>>>>> Currently it's not possible to set bpf_jit_enable = 2 when
>>>>> CONFIG_BPF_JIT_ALWAYS_ON is set, which makes debugging certain problems
>>>>> harder.
>>>> 
>>>> This is obsolete way of debugging.
>>>> Please use bpftool dump jited instead.
>>> 
>>> Is there a way to integrate bpftool nicely with e.g. test_verifier?
>>> With bpf_jit_enable = 2, I can see JITed code for each test right away,
>>> without pausing it (via gdb or rebuilding with added sleep()) and
>>> running bpftool.
>> On the library side we can set the log_level causing the verifier logic
>> steps to be printed. I guess adding it to bpftool might be nice. At least
>> I would find it useful. I'll probably get to it sometime if its not
>> already there somewhere and/or someone doesn't beat me to it.
> 
> +1
> 
> Was wondering whether it may be worth it moving parts of the logic from bpftool
> into libbpf wrt jit dump as a higher level api, so it could be used directly for
> checking out the jit disasm + opcodes for specific tests given we have the fd
> there as well, but that might be too specific perhaps and would bring one more
> lib dependency to libbpf for a rather narrow use case. Adding sleep before prog
> fd close and/or shelling out to bpftool etc all is a crude temporary hack as
> well (currently using something long these lines locally). Would it make sense
> to dump some meta data and generated opcodes per test case to a file as opt-in
> e.g. ./test_verifier 711 --dump produces 711.opcodes out of bpf_obj_get_info_by_fd()
> which then bpftool could dump this artifact through its own disasm?
> 
> Thanks,
> Daniel

Yes, this sounds fine - if the test fails or behaves strangely, I won't
have to re-run it using a special setup, but rather just disasm the
dumped JITted image (maybe even without bpftool, just with objdump).

Another question though: what about seccomp? It looks as if those
programs are not shown by bpftool, since they are not created using bpf
syscall.

Best regards,
Ilya
Daniel Borkmann Nov. 7, 2019, 4:19 p.m. UTC | #6
On 11/7/19 4:30 PM, Ilya Leoshkevich wrote:
>> Am 07.11.2019 um 00:07 schrieb Daniel Borkmann <daniel@iogearbox.net>:
>>
>> On 11/6/19 5:50 PM, John Fastabend wrote:
>>> Ilya Leoshkevich wrote:
>>>>> Am 06.11.2019 um 17:15 schrieb Alexei Starovoitov <alexei.starovoitov@gmail.com>:
>>>>> On Wed, Nov 6, 2019 at 8:12 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>>>>>
>>>>>> Currently it's not possible to set bpf_jit_enable = 2 when
>>>>>> CONFIG_BPF_JIT_ALWAYS_ON is set, which makes debugging certain problems
>>>>>> harder.
>>>>>
>>>>> This is obsolete way of debugging.
>>>>> Please use bpftool dump jited instead.
>>>>
>>>> Is there a way to integrate bpftool nicely with e.g. test_verifier?
>>>> With bpf_jit_enable = 2, I can see JITed code for each test right away,
>>>> without pausing it (via gdb or rebuilding with added sleep()) and
>>>> running bpftool.
>>> On the library side we can set the log_level causing the verifier logic
>>> steps to be printed. I guess adding it to bpftool might be nice. At least
>>> I would find it useful. I'll probably get to it sometime if its not
>>> already there somewhere and/or someone doesn't beat me to it.
>>
>> +1
>>
>> Was wondering whether it may be worth it moving parts of the logic from bpftool
>> into libbpf wrt jit dump as a higher level api, so it could be used directly for
>> checking out the jit disasm + opcodes for specific tests given we have the fd
>> there as well, but that might be too specific perhaps and would bring one more
>> lib dependency to libbpf for a rather narrow use case. Adding sleep before prog
>> fd close and/or shelling out to bpftool etc all is a crude temporary hack as
>> well (currently using something long these lines locally). Would it make sense
>> to dump some meta data and generated opcodes per test case to a file as opt-in
>> e.g. ./test_verifier 711 --dump produces 711.opcodes out of bpf_obj_get_info_by_fd()
>> which then bpftool could dump this artifact through its own disasm?
> 
> Yes, this sounds fine - if the test fails or behaves strangely, I won't
> have to re-run it using a special setup, but rather just disasm the
> dumped JITted image (maybe even without bpftool, just with objdump).
> 
> Another question though: what about seccomp? It looks as if those
> programs are not shown by bpftool, since they are not created using bpf
> syscall.

Correct, they are not shown because they are not native (e)BPF.

The criu folks are using PTRACE_SECCOMP_GET_FILTER as one option to dump (not
sure if that helps you much though):

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f8e529ed941ba2bbcbf310b575d968159ce7e895
diff mbox series

Patch

diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index eb29e5adc84d..09f1218b5656 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -389,7 +389,7 @@  static struct ctl_table net_core_table[] = {
 		.proc_handler	= proc_dointvec_minmax_bpf_enable,
 # ifdef CONFIG_BPF_JIT_ALWAYS_ON
 		.extra1		= SYSCTL_ONE,
-		.extra2		= SYSCTL_ONE,
+		.extra2		= &two,
 # else
 		.extra1		= SYSCTL_ZERO,
 		.extra2		= &two,