Received: by 2002:a25:ad19:0:0:0:0:0 with SMTP id y25csp1155929ybi; Fri, 12 Jul 2019 10:39:27 -0700 (PDT) X-Google-Smtp-Source: APXvYqzkW0jFavYZlTTEXTfyr7Nz/88TIjN5CVna6sRHFO8nAtsKdZgzEsTAm9GHisMJ6AW0dhcV X-Received: by 2002:a17:902:9896:: with SMTP id s22mr12364881plp.4.1562953167462; Fri, 12 Jul 2019 10:39:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1562953167; cv=none; d=google.com; s=arc-20160816; b=iQY2F/T+2jyZlaIC/lb3ZvPi8dcnFpwIfb8yCdX71TAFix87Uy5Of3g0bySX+bGly1 KaJMSV4DTzuzB61+wjG6vXG4EGgzOtv9N8u3cWNf05Z2mo5Leov/AeJP5Nk8QGTOAhBc QG2rig9KnDSqUKvdXcpmYyf+jH37ww3D3c6pHSaKaLNl25n3AzrkLJYQ2cQRKhHmlzKb zttz/7AJ1WqKchYiYdA4Fy2sYGWYzmEY07yF9rHMA1b89to3fXjyQVXwprdG2tn7Oxrd vC42hC27SgQvVne4FXyrcrBM+zTQ16IA5qOvFg15WxGmkZJhs0cGbef0mS2DctzIP7ZX fajQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:cc:to:subject :message-id:date:from:in-reply-to:references:mime-version :dkim-signature; bh=ECOowTnmo6hEt1xt/TfDln7M7jHsu79iCyx4EjgLRmk=; b=0ArONBA5I2MJf0sYhasaWyED+iV7qQIoMDcrS4J6ETB2JCoztLMzxUYOHFHQ/TGt2I 0lQWj8pHlsvZEJ4BZlF/xr/PivYrWJnjvZ5ngQEm3lnHKjFH0s3VrTyOSuWCwqbpstJO Xg+FQRF5KQ9A990Miy3XytWaqJQnoDUKeyb4bBxPXECA/RmmH6hNEkP6nDYoA4qSqy/h fZ7mf33hynRDjLX/6eLnIT61onWjeOuxvPNMayimDbxe185O5tnnhJhaWV6l/UZTyirM 08NZv97ezCS6FUVcUo+XsJ0QEzNQwRkf3MOYQX/Uul3/krQE+TyrpTIY2lELyjHZx5/M gh9Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kinvolk.io header.s=google header.b=at082DvK; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id n14si8927042pgc.133.2019.07.12.10.39.11; Fri, 12 Jul 2019 10:39:27 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kinvolk.io header.s=google header.b=at082DvK; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727356AbfGLRh1 (ORCPT + 99 others); Fri, 12 Jul 2019 13:37:27 -0400 Received: from mail-lf1-f66.google.com ([209.85.167.66]:43875 "EHLO mail-lf1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727028AbfGLRh0 (ORCPT ); Fri, 12 Jul 2019 13:37:26 -0400 Received: by mail-lf1-f66.google.com with SMTP id c19so7003699lfm.10 for ; Fri, 12 Jul 2019 10:37:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kinvolk.io; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=ECOowTnmo6hEt1xt/TfDln7M7jHsu79iCyx4EjgLRmk=; b=at082DvKhiTPqjz4ap9YPlMHYMCJY9h7UyXg+zOJb5BkByXK7uzj2jv8q0G+JOyCkv 20HT/VP9yGNlE9ubQAloFeetKGUZValrHYc8baWhk+iTSr4ypaYqEz2qvEJBscBTsqv4 K2VuuRLoKqTkY8lw+ZQpVGS9xLW8TleV/CWyA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=ECOowTnmo6hEt1xt/TfDln7M7jHsu79iCyx4EjgLRmk=; b=WQSxxdCB+Gbnn9QJXVlTpmfTkMXXdxe/i9dRMOqVpTqBmcR59PWTjYFtV6dnB4cHnf C+qqXuH0d1mlWMuJTg0lCrHVXVGehBN/LUwyVa0ZAOFHoLVjfM5JS8uqmW4Hg3F4ChYR UMYbH0x6aQ5+2ZeDXjE/inmuAr/cvKBCkem1EtAVcPUMq+qtVJwE3g4pvMxs1a3wvgZW UuoR6sdwi/W9V/GZvOSyMqGEaKyb3+gL3sYaw8IMz1Xh1VE+bNpPlcKfgyML/fEcUXBQ tJB1cY8PgOnaQ0wplZwN8CgoVf+cU0n0IWkA8WhhT+Hk7tpprt3D+TbrgWB+n4ZbS9nM n+6Q== X-Gm-Message-State: APjAAAU0keyxxbUItdCA9uOR3vKx3tYUxMaCg1/HTvSAgzj0zFeQZf5O IfRP+1sI3sQkFU7T1C/GuRRDujzY8njvsn3hav5WyA== X-Received: by 2002:a19:8c08:: with SMTP id o8mr5348268lfd.57.1562953043592; Fri, 12 Jul 2019 10:37:23 -0700 (PDT) MIME-Version: 1.0 References: <20190708163121.18477-1-krzesimir@kinvolk.io> <20190708163121.18477-12-krzesimir@kinvolk.io> In-Reply-To: From: Krzesimir Nowak Date: Fri, 12 Jul 2019 19:37:12 +0200 Message-ID: Subject: Re: [bpf-next v3 11/12] selftests/bpf: Add tests for bpf_prog_test_run for perf events progs To: Andrii Nakryiko Cc: open list , Alban Crequy , =?UTF-8?Q?Iago_L=C3=B3pez_Galeiras?= , Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , "David S. Miller" , Jakub Kicinski , Jesper Dangaard Brouer , John Fastabend , Stanislav Fomichev , Networking , bpf , xdp-newbies@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jul 12, 2019 at 2:38 AM Andrii Nakryiko wrote: > > On Mon, Jul 8, 2019 at 3:42 PM Krzesimir Nowak wro= te: > > > > The tests check if ctx and data are correctly prepared from ctx_in and > > data_in, so accessing the ctx and using the bpf_perf_prog_read_value > > work as expected. > > > > These are x86_64-specific tests, aren't they? Should probably guard > them behind #ifdef's. Yeah, they are x86_64 specific, because pt_regs are arch specific. I was wondering what to do here in the cover letter. Ifdef? Ifdef and cover also other arches (please no)? Do some weird tricks with overriding the definition of pt_regs? Else? > > > Signed-off-by: Krzesimir Nowak > > --- > > tools/testing/selftests/bpf/test_verifier.c | 48 ++++++++++ > > .../selftests/bpf/verifier/perf_event_run.c | 96 +++++++++++++++++++ > > 2 files changed, 144 insertions(+) > > create mode 100644 tools/testing/selftests/bpf/verifier/perf_event_run= .c > > > > diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testin= g/selftests/bpf/test_verifier.c > > index 6f124cc4ee34..484ea8842b06 100644 > > --- a/tools/testing/selftests/bpf/test_verifier.c > > +++ b/tools/testing/selftests/bpf/test_verifier.c > > @@ -295,6 +295,54 @@ static void bpf_fill_scale(struct bpf_test *self) > > } > > } > > > > +static void bpf_fill_perf_event_test_run_check(struct bpf_test *self) > > +{ > > + compiletime_assert( > > + sizeof(struct bpf_perf_event_data) <=3D TEST_CTX_LEN, > > + "buffer for ctx is too short to fit struct bpf_perf_eve= nt_data"); > > + compiletime_assert( > > + sizeof(struct bpf_perf_event_value) <=3D TEST_DATA_LEN, > > + "buffer for data is too short to fit struct bpf_perf_ev= ent_value"); > > + > > + struct bpf_perf_event_data ctx =3D { > > + .regs =3D (bpf_user_pt_regs_t) { > > + .r15 =3D 1, > > + .r14 =3D 2, > > + .r13 =3D 3, > > + .r12 =3D 4, > > + .rbp =3D 5, > > + .rbx =3D 6, > > + .r11 =3D 7, > > + .r10 =3D 8, > > + .r9 =3D 9, > > + .r8 =3D 10, > > + .rax =3D 11, > > + .rcx =3D 12, > > + .rdx =3D 13, > > + .rsi =3D 14, > > + .rdi =3D 15, > > + .orig_rax =3D 16, > > + .rip =3D 17, > > + .cs =3D 18, > > + .eflags =3D 19, > > + .rsp =3D 20, > > + .ss =3D 21, > > + }, > > + .sample_period =3D 1, > > + .addr =3D 2, > > + }; > > + struct bpf_perf_event_value data =3D { > > + .counter =3D 1, > > + .enabled =3D 2, > > + .running =3D 3, > > + }; > > + > > + memcpy(self->ctx, &ctx, sizeof(ctx)); > > + memcpy(self->data, &data, sizeof(data)); > > Just curious, just assignment didn't work? > > > + free(self->fill_insns); > > + self->fill_insns =3D NULL; > > +} > > + > > /* BPF_SK_LOOKUP contains 13 instructions, if you need to fix up maps = */ > > #define BPF_SK_LOOKUP(func) = \ > > /* struct bpf_sock_tuple tuple =3D {} */ = \ > > diff --git a/tools/testing/selftests/bpf/verifier/perf_event_run.c b/to= ols/testing/selftests/bpf/verifier/perf_event_run.c > > new file mode 100644 > > index 000000000000..3f877458a7f8 > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/verifier/perf_event_run.c > > @@ -0,0 +1,96 @@ > > +#define PER_LOAD_AND_CHECK_PTREG(PT_REG_FIELD, VALUE) = \ > > + PER_LOAD_AND_CHECK_CTX(offsetof(bpf_user_pt_regs_t, PT_REG_FIEL= D), VALUE) > > +#define PER_LOAD_AND_CHECK_EVENT(PED_FIELD, VALUE) = \ > > + PER_LOAD_AND_CHECK_CTX(offsetof(struct bpf_perf_event_data, PED= _FIELD), VALUE) > > +#define PER_LOAD_AND_CHECK_CTX(OFFSET, VALUE) = \ > > + PER_LOAD_AND_CHECK_64(BPF_REG_4, BPF_REG_1, OFFSET, VALUE) > > +#define PER_LOAD_AND_CHECK_VALUE(PEV_FIELD, VALUE) = \ > > + PER_LOAD_AND_CHECK_64(BPF_REG_7, BPF_REG_6, offsetof(struct bpf= _perf_event_value, PEV_FIELD), VALUE) > > Wrap long lines? Try also running scripts/checkpatch.pl again these > files you are modifying. Will wrap. Checkpatch was also complaining about complex macro not being inside parens, but I can't see how to wrap it in parens and keep it working at the same time. > > > +#define PER_LOAD_AND_CHECK_64(DST, SRC, OFFSET, VALUE) = \ > > + BPF_LDX_MEM(BPF_DW, DST, SRC, OFFSET), = \ > > + BPF_JMP_IMM(BPF_JEQ, DST, VALUE, 2), = \ > > + BPF_MOV64_IMM(BPF_REG_0, VALUE), = \ > > + BPF_EXIT_INSN() > > + > > +{ > > + "check if regs contain expected values", > > + .insns =3D { > > + PER_LOAD_AND_CHECK_PTREG(r15, 1), > > + PER_LOAD_AND_CHECK_PTREG(r14, 2), > > + PER_LOAD_AND_CHECK_PTREG(r13, 3), > > + PER_LOAD_AND_CHECK_PTREG(r12, 4), > > + PER_LOAD_AND_CHECK_PTREG(rbp, 5), > > + PER_LOAD_AND_CHECK_PTREG(rbx, 6), > > + PER_LOAD_AND_CHECK_PTREG(r11, 7), > > + PER_LOAD_AND_CHECK_PTREG(r10, 8), > > + PER_LOAD_AND_CHECK_PTREG(r9, 9), > > + PER_LOAD_AND_CHECK_PTREG(r8, 10), > > + PER_LOAD_AND_CHECK_PTREG(rax, 11), > > + PER_LOAD_AND_CHECK_PTREG(rcx, 12), > > + PER_LOAD_AND_CHECK_PTREG(rdx, 13), > > + PER_LOAD_AND_CHECK_PTREG(rsi, 14), > > + PER_LOAD_AND_CHECK_PTREG(rdi, 15), > > + PER_LOAD_AND_CHECK_PTREG(orig_rax, 16), > > + PER_LOAD_AND_CHECK_PTREG(rip, 17), > > + PER_LOAD_AND_CHECK_PTREG(cs, 18), > > + PER_LOAD_AND_CHECK_PTREG(eflags, 19), > > + PER_LOAD_AND_CHECK_PTREG(rsp, 20), > > + PER_LOAD_AND_CHECK_PTREG(ss, 21), > > + BPF_MOV64_IMM(BPF_REG_0, 0), > > + BPF_EXIT_INSN(), > > + }, > > + .result =3D ACCEPT, > > + .prog_type =3D BPF_PROG_TYPE_PERF_EVENT, > > + .ctx_len =3D sizeof(struct bpf_perf_event_data), > > + .data_len =3D sizeof(struct bpf_perf_event_value), > > + .fill_helper =3D bpf_fill_perf_event_test_run_check, > > + .override_data_out_len =3D true, > > +}, > > +{ > > + "check if sample period and addr contain expected values", > > + .insns =3D { > > + PER_LOAD_AND_CHECK_EVENT(sample_period, 1), > > + PER_LOAD_AND_CHECK_EVENT(addr, 2), > > + BPF_MOV64_IMM(BPF_REG_0, 0), > > + BPF_EXIT_INSN(), > > + }, > > + .result =3D ACCEPT, > > + .prog_type =3D BPF_PROG_TYPE_PERF_EVENT, > > + .ctx_len =3D sizeof(struct bpf_perf_event_data), > > + .data_len =3D sizeof(struct bpf_perf_event_value), > > + .fill_helper =3D bpf_fill_perf_event_test_run_check, > > + .override_data_out_len =3D true, > > +}, > > +{ > > + "check if bpf_perf_prog_read_value returns expected data", > > + .insns =3D { > > + // allocate space for a struct bpf_perf_event_value > > + BPF_MOV64_REG(BPF_REG_6, BPF_REG_10), > > + BPF_ALU64_IMM(BPF_ADD, BPF_REG_6, -(int)sizeof(struct bpf_perf_= event_value)), > > + // prepare parameters for bpf_perf_prog_read_value(ctx, struct = bpf_perf_event_value*, u32) > > + // BPF_REG_1 already contains the context > > + BPF_MOV64_REG(BPF_REG_2, BPF_REG_6), > > + BPF_MOV64_IMM(BPF_REG_3, sizeof(struct bpf_perf_event_value)), > > + BPF_EMIT_CALL(BPF_FUNC_perf_prog_read_value), > > + // check the return value > > + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1), > > + BPF_EXIT_INSN(), > > + // check if the fields match the expected values > > Use /* */ comments. Oops. Will fix. > > > + PER_LOAD_AND_CHECK_VALUE(counter, 1), > > + PER_LOAD_AND_CHECK_VALUE(enabled, 2), > > + PER_LOAD_AND_CHECK_VALUE(running, 3), > > + BPF_MOV64_IMM(BPF_REG_0, 0), > > + BPF_EXIT_INSN(), > > + }, > > + .result =3D ACCEPT, > > + .prog_type =3D BPF_PROG_TYPE_PERF_EVENT, > > + .ctx_len =3D sizeof(struct bpf_perf_event_data), > > + .data_len =3D sizeof(struct bpf_perf_event_value), > > + .fill_helper =3D bpf_fill_perf_event_test_run_check, > > + .override_data_out_len =3D true, > > +}, > > +#undef PER_LOAD_AND_CHECK_64 > > +#undef PER_LOAD_AND_CHECK_VALUE > > +#undef PER_LOAD_AND_CHECK_CTX > > +#undef PER_LOAD_AND_CHECK_EVENT > > +#undef PER_LOAD_AND_CHECK_PTREG > > -- > > 2.20.1 > > -- Kinvolk GmbH | Adalbertstr.6a, 10999 Berlin | tel: +491755589364 Gesch=C3=A4ftsf=C3=BChrer/Directors: Alban Crequy, Chris K=C3=BChl, Iago L= =C3=B3pez Galeiras Registergericht/Court of registration: Amtsgericht Charlottenburg Registernummer/Registration number: HRB 171414 B Ust-ID-Nummer/VAT ID number: DE302207000