All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net
Cc: andrii@kernel.org, kernel-team@fb.com
Subject: RE: [PATCH bpf-next 1/2] selftests/bpf: add generic BPF program verification tester
Date: Tue, 06 Dec 2022 22:21:19 -0800	[thread overview]
Message-ID: <639030df9ec4e_bb36208e6@john.notmuch> (raw)
In-Reply-To: <20221206011159.1208452-1-andrii@kernel.org>

Andrii Nakryiko wrote:
> It's become a common pattern to have a collection of small BPF programs
> in one BPF object file, each representing one test case. On user-space
> side of such tests we maintain a table of program names and expected
> failure or success, along with optional expected verifier log message.
> 
> This works, but each set of tests reimplement this mundane code over and
> over again, which is a waste of time for anyone trying to add a new set
> of tests. Furthermore, it's quite error prone as it's way too easy to miss
> some entries in these manually maintained test tables (as evidences by
> dynptr_fail tests, in which ringbuf_release_uninit_dynptr subtest was
> accidentally missed; this is fixed in next patch).
> 
> So this patch implements generic verification_tester, which accepts
> skeleton name and handles the rest of details: opens and loads BPF
> object file, making sure each program is tested in isolation. Optionally
> each test case can specify expected BPF verifier log message. In case of
> failure, tester makes sure to report verifier log, but it also reports
> verifier log in verbose mode unconditionally.
> 
> Now, the interesting deviation from existing custom implementations is
> the use of btf_decl_tag attribute to specify expected-to-fail vs
> expected-to-succeed markers and, optionally, expected log message
> directly next to BPF program source code, eliminating the need to
> manually create and update table of tests.
> 
> We define few macros wrapping btf_decl_tag with a convention that all
> values of btf_decl_tag start with "comment:" prefix, and then utilizing
> a very simple "just_some_text_tag" or "some_key_name=<value>" pattern to
> define things like expected success/failure, expected verifier message,
> extra verifier log level (if necessary). This approach is demonstrated
> by next patch in which two existing sets of failure tests are converted.
> 
> Tester supports both expected-to-fail and expected-to-succeed programs,
> though this patch set didn't convert any existing expected-to-succeed
> programs yet, as existing tests couple BPF program loading with their
> further execution through attach or test_prog_run. One way to allow
> testing scenarios like this would be ability to specify custom callback,
> executed for each successfully loaded BPF program. This is left for
> follow up patches, after some more analysis of existing test cases.
> 
> This verification_tester is, hopefully, a start of a test_verifier
> runner, which allows much better "user experience" of defining low-level
> verification types that can take advantage of all the libbpf-provided
> nicety features on BPF side: global variables, declarative maps, etc.
> All while having a choice of defining it in C or as BPF assembly
> (through __attribute__((naked)) functions and using embedded asm). This
> will be explored in follow up patches as well.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---

LGTM.

Acked-by: John Fastabend <john.fastabend@gmail.com>

      parent reply	other threads:[~2022-12-07  6:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-06  1:11 [PATCH bpf-next 1/2] selftests/bpf: add generic BPF program verification tester Andrii Nakryiko
2022-12-06  1:11 ` [PATCH bpf-next 2/2] selftests/bpf: convert dynptr_fail and map_kptr_fail subtests to generic tester Andrii Nakryiko
2022-12-07  6:27   ` John Fastabend
2022-12-07  4:43 ` [PATCH bpf-next 1/2] selftests/bpf: add generic BPF program verification tester Alexei Starovoitov
2022-12-07 19:46   ` Andrii Nakryiko
2022-12-07  6:21 ` John Fastabend [this message]

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=639030df9ec4e_bb36208e6@john.notmuch \
    --to=john.fastabend@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.