bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Borkmann <daniel@iogearbox.net>
To: Andrii Nakryiko <andriin@fb.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org, ast@fb.com
Cc: andrii.nakryiko@gmail.com, kernel-team@fb.com,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH bpf 2/2] selftests/bpf: add variable-length data concatenation pattern test
Date: Tue, 16 Jun 2020 22:21:05 +0200	[thread overview]
Message-ID: <5fed920d-aeb6-c8de-18c0-7c046bbfb242@iogearbox.net> (raw)
In-Reply-To: <20200616050432.1902042-2-andriin@fb.com>

On 6/16/20 7:04 AM, Andrii Nakryiko wrote:
> Add selftest that validates variable-length data reading and concatentation
> with one big shared data array. This is a common pattern in production use for
> monitoring and tracing applications, that potentially can read a lot of data,
> but usually reads much less. Such pattern allows to determine precisely what
> amount of data needs to be sent over perfbuf/ringbuf and maximize efficiency.
> 
> This is the first BPF selftest that at all looks at and tests
> bpf_probe_read_str()-like helper's return value, closing a major gap in BPF
> testing. It surfaced the problem with bpf_probe_read_kernel_str() returning
> 0 on success, instead of amount of bytes successfully read.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Fix looks good, but I'm seeing an issue in the selftest on my side. With latest
Clang/LLVM I'm getting:

# ./test_progs -t varlen
#86 varlen:OK
Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

All good, however, the test_progs-no_alu32 fails for me with:

# ./test_progs-no_alu32 -t varlen
Switching to flavor 'no_alu32' subdirectory...
libbpf: load bpf program failed: Invalid argument
libbpf: -- BEGIN DUMP LOG ---
libbpf:
arg#0 type is not a struct
Unrecognized arg#0 type PTR
; int pid = bpf_get_current_pid_tgid() >> 32;
0: (85) call bpf_get_current_pid_tgid#14
; int pid = bpf_get_current_pid_tgid() >> 32;
1: (77) r0 >>= 32
; if (test_pid != pid || !capture)
2: (18) r1 = 0xffffb14a4010c200
4: (61) r1 = *(u32 *)(r1 +0)
  R0_w=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R1_w=map_value(id=0,off=512,ks=4,vs=1056,imm=0) R10=fp0
; if (test_pid != pid || !capture)
5: (5d) if r1 != r0 goto pc+43
  R0_w=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R1_w=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R10=fp0
6: (18) r1 = 0xffffb14a4010c204
8: (71) r1 = *(u8 *)(r1 +0)
  R0_w=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R1_w=map_value(id=0,off=516,ks=4,vs=1056,imm=0) R10=fp0
9: (15) if r1 == 0x0 goto pc+39
  R0=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R1=inv(id=0,umax_value=255,var_off=(0x0; 0xff)) R10=fp0
; len = bpf_probe_read_kernel_str(payload, MAX_LEN, &buf_in1[0]);
10: (18) r6 = 0xffffb14a4010c220
12: (18) r1 = 0xffffb14a4010c220
14: (b7) r2 = 256
15: (18) r3 = 0xffffb14a4010c000
17: (85) call bpf_probe_read_kernel_str#115
  R0=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R1_w=map_value(id=0,off=544,ks=4,vs=1056,imm=0) R2_w=inv256 R3_w=map_value(id=0,off=0,ks=4,vs=1056,imm=0) R6_w=map_value(id=0,off=544,ks=4,vs=1056,imm=0) R10=fp0
last_idx 17 first_idx 9
regs=4 stack=0 before 15: (18) r3 = 0xffffb14a4010c000
regs=4 stack=0 before 14: (b7) r2 = 256
18: (67) r0 <<= 32
19: (bf) r1 = r0
20: (77) r1 >>= 32
; if (len <= MAX_LEN) {
21: (25) if r1 > 0x100 goto pc+7
  R0=inv(id=0,smax_value=1099511627776,umax_value=18446744069414584320,var_off=(0x0; 0xffffffff00000000),s32_min_value=0,s32_max_value=0,u32_max_value=0) R1=inv(id=0,umax_value=256,var_off=(0x0; 0x1ff)) R6=map_value(id=0,off=544,ks=4,vs=1056,imm=0) R10=fp0
;
22: (c7) r0 s>>= 32
; payload1_len1 = len;
23: (18) r1 = 0xffffb14a4010c208
25: (7b) *(u64 *)(r1 +0) = r0
  R0_w=inv(id=0,smin_value=-2147483648,smax_value=256) R1_w=map_value(id=0,off=520,ks=4,vs=1056,imm=0) R6=map_value(id=0,off=544,ks=4,vs=1056,imm=0) R10=fp0
; payload += len;
26: (18) r6 = 0xffffb14a4010c220
28: (0f) r6 += r0
last_idx 28 first_idx 21
regs=1 stack=0 before 26: (18) r6 = 0xffffb14a4010c220
regs=1 stack=0 before 25: (7b) *(u64 *)(r1 +0) = r0
regs=1 stack=0 before 23: (18) r1 = 0xffffb14a4010c208
regs=1 stack=0 before 22: (c7) r0 s>>= 32
regs=1 stack=0 before 21: (25) if r1 > 0x100 goto pc+7
  R0_rw=invP(id=0,smax_value=1099511627776,umax_value=18446744069414584320,var_off=(0x0; 0xffffffff00000000),s32_min_value=0,s32_max_value=0,u32_max_value=0) R1_rw=inv(id=0,umax_value=4294967295,var_off=(0x0; 0xffffffff)) R6_w=map_value(id=0,off=544,ks=4,vs=1056,imm=0) R10=fp0
parent didn't have regs=1 stack=0 marks
last_idx 20 first_idx 9
regs=1 stack=0 before 20: (77) r1 >>= 32
regs=1 stack=0 before 19: (bf) r1 = r0
regs=1 stack=0 before 18: (67) r0 <<= 32
regs=1 stack=0 before 17: (85) call bpf_probe_read_kernel_str#115
value -2147483648 makes map_value pointer be out of bounds
processed 22 insns (limit 1000000) max_states_per_insn 0 total_states 2 peak_states 2 mark_read 1

libbpf: -- END LOG --
libbpf: failed to load program 'raw_tp/sys_enter'
libbpf: failed to load object 'test_varlen'
libbpf: failed to load BPF skeleton 'test_varlen': -4007
test_varlen:FAIL:skel_open failed to open skeleton
#86 varlen:FAIL
Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED

  reply	other threads:[~2020-06-16 20:21 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-16  5:04 [PATCH bpf 1/2] bpf: bpf_probe_read_kernel_str() has to return amount of data read on success Andrii Nakryiko
2020-06-16  5:04 ` [PATCH bpf 2/2] selftests/bpf: add variable-length data concatenation pattern test Andrii Nakryiko
2020-06-16 20:21   ` Daniel Borkmann [this message]
2020-06-16 21:27     ` Andrii Nakryiko
2020-06-16 22:23       ` Daniel Borkmann
2020-06-16 23:14         ` Andrii Nakryiko
2020-06-17 15:51           ` Daniel Borkmann
2020-06-18 19:09   ` John Fastabend
2020-06-18 21:59     ` Andrii Nakryiko
2020-06-18 23:48       ` John Fastabend
2020-06-19  0:09         ` Andrii Nakryiko
2020-06-16  6:54 ` [PATCH bpf 1/2] bpf: bpf_probe_read_kernel_str() has to return amount of data read on success Christoph Hellwig
2020-06-16  7:01 ` John Fastabend

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=5fed920d-aeb6-c8de-18c0-7c046bbfb242@iogearbox.net \
    --to=daniel@iogearbox.net \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=hch@lst.de \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.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).