Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1593787468-29931-1-git-send-email-alan.maguire@oracle.com> <1593787468-29931-3-git-send-email-alan.maguire@oracle.com>
In-Reply-To: <1593787468-29931-3-git-send-email-alan.maguire@oracle.com>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Tue, 7 Jul 2020 23:01:33 -0700
Message-ID: <CAEf4BzaUQT8399tvd+XsbjL+ZyC-sfH-B5ixrgzWC+q6Wnvh8w@mail.gmail.com>
Subject: Re: [PATCH bpf-next 2/2] selftests/bpf: add selftests verifying
 bpf_trace_printk() behaviour
To:     Alan Maguire <alan.maguire@oracle.com>
Cc:     Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@redhat.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Martin Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
        Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
        john fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@chromium.org>,
        open list <linux-kernel@vger.kernel.org>,
        Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jul 3, 2020 at 7:45 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Simple selftest that verifies bpf_trace_printk() returns a sensible
> value and tracing messages appear.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  .../selftests/bpf/prog_tests/trace_printk.c        | 71 ++++++++++++++++++++++
>  tools/testing/selftests/bpf/progs/trace_printk.c   | 21 +++++++
>  2 files changed, 92 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/trace_printk.c
>  create mode 100644 tools/testing/selftests/bpf/progs/trace_printk.c
>

[...]

> +       fd = open(TRACEBUF, O_RDONLY);
> +       if (CHECK(fd < 0, "could not open trace buffer",
> +                 "error %d opening %s", errno, TRACEBUF))
> +               goto cleanup;
> +
> +       /* We do not want to wait forever if this test fails... */
> +       fcntl(fd, F_SETFL, O_NONBLOCK);
> +
> +       /* wait for tracepoint to trigger */
> +       sleep(1);

that's a long sleep, it's better to use tp/raw_syscalls/sys_enter
tracepoint to trigger BPF program and then just usleep(1)

> +       trace_printk__detach(skel);
> +
> +       if (CHECK(bss->trace_printk_ran == 0,
> +                 "bpf_trace_printk never ran",
> +                 "ran == %d", bss->trace_printk_ran))
> +               goto cleanup;
> +

[...]

> +
> +int trace_printk_ret = 0;
> +int trace_printk_ran = 0;
> +
> +SEC("tracepoint/sched/sched_switch")

see above, probably better to stick to something like
tp/raw_syscalls/sys_enter or raw_tp/sys_enter. Also, to not overwhelm
trace_pipe output, might want to filter by PID and emit messages for
test_prog's PID only.


> +int sched_switch(void *ctx)
> +{
> +       static const char fmt[] = "testing,testing %d\n";
> +
> +       trace_printk_ret = bpf_trace_printk(fmt, sizeof(fmt),
> +                                           ++trace_printk_ran);
> +       return 0;
> +}
> --
> 1.8.3.1
>