netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stanislav Fomichev <sdf@fomichev.me>
To: Quentin Monnet <quentin.monnet@netronome.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Edward Cree <ecree@solarflare.com>,
	Stanislav Fomichev <sdf@google.com>,
	netdev@vger.kernel.org, davem@davemloft.net, ast@kernel.org
Subject: Re: [PATCH bpf-next 1/2] selftests/bpf: skip verifier tests that depend on CONFIG_CGROUP_BPF
Date: Thu, 13 Dec 2018 08:37:49 -0800	[thread overview]
Message-ID: <20181213163749.GH9107@mini-arch.hsd1.ca.comcast.net> (raw)
In-Reply-To: <e8fa879f-96d5-4dc0-57a2-2eee9bf08a00@netronome.com>

On 12/13, Quentin Monnet wrote:
> 2018-12-13 12:52 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net>
> > On 12/13/2018 07:06 AM, Alexei Starovoitov wrote:
> >> On Wed, Dec 12, 2018 at 02:32:01PM -0800, Stanislav Fomichev wrote:
> >>>
> >>> To summarize, I like your idea about doing runtime tests and I think I
> >>> can make it work quite nicely without any config_disabled ugliness by
> >>> looking at the prog_type of each test.
> >>> I can send an RFC patch series out if there still a small chance you could
> >>> take it, but if you've already set you mind, I'd just keep them
> >>> internally. So let me know if you have a hard NACK on the runtime probing
> >>> approach or there is still some wiggle room.
> >>
> >> If there is no uapi/bpf.h change, it's likely fine.
> >> Like if test_verifier tries to load 'foo() {return 0;}' prog
> >> for the .prog_type in the test that failed to confirm that
> >> such prog type is supported by the kernel...
> >> that is fine, since no extra prog_loads are happening for the default case.
> > 
> > I think this would kind of go along the lines of what Quentin is working on.
> > Idea [0] is to consolidate effort into bpftool so that one can do something
> > like `bpftool kernel probe` and it generates a header file with CONFIG_*
> > defines for features where bpftool was able to successfully probe the
> > underlying kernel with. This would allow developers to include this header
> > generation as part of the build workflow and avoid having to implement
> > similar probing mechanism in various projects over and over again which aim
> > to run on different kernel versions. I'm wondering whether it would make sense
> > to split the probing part and put it into libbpf where then bpftool is only
> > responsible to call the API and write out the defines? That way, the runtime
> > probing could potentially be reused for selftests as well?
+1

Keeping those low-level probing details in the libbpf seems like a
good idea. `bpftool feature` can then be just a simple a frontend to those
probes to dump them in C/JSON. Tests and other tools can use the
probes on the target host via libbpf to degrade some functionality or
print nice error messages instead of 'EINVAL: Invalid argument'.

> > 
> > Thanks,
> > Daniel
> > 
> >  [0] slide 8,12: http://vger.kernel.org/lpc_bpf2018_talks/qmo-bpf-slides-v2.pdf
> 
> Hi Daniel, Stanislav,
> 
> Thanks for the Cc. I got somewhat delayed in my series, but I just
> finished it and was about to post the patches to the mailing list. Since
> the code is ready to go I'll send it in its current shape, i.e. all
> probes implemented on bpftool side, and we can hopefully use it as a
> support for further discussion.
Just out of curiosity: what's the usecase of generating C defines via
bpftool? Is it for the BCC case where we have the complier on the target
host and can run bpftool+bcc there?

> 
> Quentin

  reply	other threads:[~2018-12-13 16:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-12 18:27 [PATCH bpf-next 1/2] selftests/bpf: skip verifier tests that depend on CONFIG_CGROUP_BPF Stanislav Fomichev
2018-12-12 18:27 ` [PATCH bpf-next 2/2] selftests/bpf: skip verifier tests that depend on CONFIG_BPF_EVENTS Stanislav Fomichev
2018-12-12 18:51 ` [PATCH bpf-next 1/2] selftests/bpf: skip verifier tests that depend on CONFIG_CGROUP_BPF Alexei Starovoitov
2018-12-12 18:59   ` Stanislav Fomichev
2018-12-12 19:04     ` Alexei Starovoitov
2018-12-12 19:24       ` Stanislav Fomichev
2018-12-12 19:54         ` Stanislav Fomichev
2018-12-12 20:13           ` Alexei Starovoitov
2018-12-12 21:23             ` Edward Cree
2018-12-12 22:07               ` Alexei Starovoitov
2018-12-12 22:32                 ` Stanislav Fomichev
2018-12-13  6:06                   ` Alexei Starovoitov
2018-12-13 11:52                     ` Daniel Borkmann
2018-12-13 12:18                       ` Quentin Monnet
2018-12-13 16:37                         ` Stanislav Fomichev [this message]
2018-12-13 17:02                           ` Quentin Monnet
2018-12-13 17:10                             ` Stanislav Fomichev
2018-12-14 11:43                               ` Quentin Monnet

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=20181213163749.GH9107@mini-arch.hsd1.ca.comcast.net \
    --to=sdf@fomichev.me \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=ecree@solarflare.com \
    --cc=netdev@vger.kernel.org \
    --cc=quentin.monnet@netronome.com \
    --cc=sdf@google.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).