bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Song Liu <songliubraving@fb.com>
Cc: Andrii Nakryiko <andriin@fb.com>, bpf <bpf@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Alexei Starovoitov <ast@fb.com>,
	"daniel@iogearbox.net" <daniel@iogearbox.net>,
	Kernel Team <Kernel-team@fb.com>
Subject: Re: [PATCH bpf-next 1/5] selftests/bpf: convert test_get_stack_raw_tp to perf_buffer API
Date: Tue, 23 Jul 2019 10:38:51 -0700	[thread overview]
Message-ID: <CAEf4BzYKFTgsf982SEZotZ5+UgP+ErieKXSUoKyj5_gCKrHxTg@mail.gmail.com> (raw)
In-Reply-To: <EBDB05E7-C10F-479F-B2A7-62D59EE4887E@fb.com>

On Tue, Jul 23, 2019 at 2:25 AM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Jul 22, 2019, at 9:31 PM, Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > Convert test_get_stack_raw_tp test to new perf_buffer API.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > .../bpf/prog_tests/get_stack_raw_tp.c         | 78 ++++++++++---------
> > .../bpf/progs/test_get_stack_rawtp.c          |  2 +-
> > 2 files changed, 44 insertions(+), 36 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
> > index c2a0a9d5591b..473889e1b219 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
> > @@ -1,8 +1,15 @@
> > // SPDX-License-Identifier: GPL-2.0
> > +#define _GNU_SOURCE
> > +#include <pthread.h>
> > +#include <sched.h>
> > +#include <sys/socket.h>
> > #include <test_progs.h>
> >
> > #define MAX_CNT_RAWTP 10ull
> > #define MAX_STACK_RAWTP       100
> > +
> > +static int duration = 0;
> > +
>
> Are we using "duration" at all?

Yes, unfortunately in test_progs CHECK macro expects "duration"
variable to be defined. It's very annoying and I'm going to work on
cleaning up and streamlining how we do selftests in bpf, so hopefully
we'll get rid of some of those "artifacts". But for now, yeah,
duration has to be defined somewhere.

>
> > struct get_stack_trace_t {
> >       int pid;
> >       int kern_stack_size;
> > @@ -13,7 +20,7 @@ struct get_stack_trace_t {
> >       struct bpf_stack_build_id user_stack_buildid[MAX_STACK_RAWTP];
> > };
> >
> > -static int get_stack_print_output(void *data, int size)
> > +static void get_stack_print_output(void *ctx, int cpu, void *data, __u32 size)
> > {
> >       bool good_kern_stack = false, good_user_stack = false;
> >       const char *nonjit_func = "___bpf_prog_run";
> > @@ -65,75 +72,76 @@ static int get_stack_print_output(void *data, int size)
> >               if (e->user_stack_size > 0 && e->user_stack_buildid_size > 0)
> >                       good_user_stack = true;
> >       }
> > -     if (!good_kern_stack || !good_user_stack)
> > -             return LIBBPF_PERF_EVENT_ERROR;
> >
> > -     if (cnt == MAX_CNT_RAWTP)
> > -             return LIBBPF_PERF_EVENT_DONE;
> > -
> > -     return LIBBPF_PERF_EVENT_CONT;
> > +     if (!good_kern_stack)
> > +         CHECK(!good_kern_stack, "bad_kern_stack", "bad\n");
>
> Two "bad" is a little weird. How about "kern stack", "bad"?

Heh :) I'll add something more human-readable, like "failed to get
kernel stack".

>
> > +     if (!good_user_stack)
> > +         CHECK(!good_user_stack, "bad_user_stack", "bad\n");
> > }
> >
> > void test_get_stack_raw_tp(void)
> > {

[...]

  reply	other threads:[~2019-07-23 17:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-23  4:31 [PATCH bpf-next 0/5] switch samples and tests to libbpf perf buffer API Andrii Nakryiko
2019-07-23  4:31 ` [PATCH bpf-next 1/5] selftests/bpf: convert test_get_stack_raw_tp to perf_buffer API Andrii Nakryiko
2019-07-23  9:25   ` Song Liu
2019-07-23 17:38     ` Andrii Nakryiko [this message]
2019-07-23  4:31 ` [PATCH bpf-next 2/5] selftests/bpf: switch test_tcpnotify " Andrii Nakryiko
2019-07-23  9:35   ` Song Liu
2019-07-23  4:31 ` [PATCH bpf-next 3/5] samples/bpf: convert xdp_sample_pkts_user " Andrii Nakryiko
2019-07-23  9:39   ` Song Liu
2019-07-23 12:34   ` Toke Høiland-Jørgensen
2019-07-23  4:31 ` [PATCH bpf-next 4/5] samples/bpf: switch trace_output sample " Andrii Nakryiko
2019-07-23  9:29   ` Song Liu
2019-07-23  4:31 ` [PATCH bpf-next 5/5] selftests/bpf: remove perf buffer helpers Andrii Nakryiko
2019-07-23  9:39   ` Song Liu

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=CAEf4BzYKFTgsf982SEZotZ5+UgP+ErieKXSUoKyj5_gCKrHxTg@mail.gmail.com \
    --to=andrii.nakryiko@gmail.com \
    --cc=Kernel-team@fb.com \
    --cc=andriin@fb.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    /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).