All of lore.kernel.org
 help / color / mirror / Atom feed
From: Quentin Monnet <quentin.monnet@netronome.com>
To: Stanislav Fomichev <sdf@fomichev.me>
Cc: Stanislav Fomichev <sdf@google.com>,
	netdev@vger.kernel.org, ast@kernel.org, davem@davemloft.net,
	daniel@iogearbox.net, ecree@solarflare.com,
	OSS-drivers Netronome <oss-drivers@netronome.com>
Subject: Re: [PATCH bpf-next 1/6] selftests/bpf: add map/prog type probe helpers
Date: Fri, 14 Dec 2018 18:37:33 +0000	[thread overview]
Message-ID: <a01d0e36-68bf-a559-48cd-7400156e9caa@netronome.com> (raw)
In-Reply-To: <20181214181649.GA32470@mini-arch.hsd1.ca.comcast.net>

2018-12-14 10:16 UTC-0800 ~ Stanislav Fomichev <sdf@fomichev.me>
> On 12/14, Quentin Monnet wrote:
>> Hi Stanislav,
>>
>> 2018-12-13 11:02 UTC-0800 ~ Stanislav Fomichev <sdf@google.com>
>>> Export bpf_map_type_supported() and bpf_prog_type_supported() which
>>> return true/false to indicate kernel support for the appropriate
>>> program or map type. These helpers will be used in the next commits
>>> to selectively skip test_verifier/test_maps tests.
>>>
>>> bpf_map_type_supported() supports only limited set of maps for which we
>>> do fixups in the test_verifier, for unknown maps it falls back to
>>> 'supported'.
>>
>> Why would you fall back on “supported” if it does not know about them?
>> Would that be worth having an enum as a return type (..._SUPPORTED,
>> ..._UNSUPPORTED, ..._UNKNOWN) maybe? Or default to not supported?
> I thought that it's safer for verifier to FAIL in case we forgot to add
> a specific map support to bpf_map_type_supported(). This is not the case
> if we were to use your version where you try to support every map type.
> 
>> Not related - We would need to put a warning somewhere, maybe a comment
>> in the header, that using probes repeatedly in a short amount of time
>> needs to update resources limits (setrlimit()), otherwise probes won't
>> work correctly.
> If we were to move this to libbpf, yes. For tests, I think we include
> bpr_rlimit.h everywhere and things just work :-)

Hmm. I was so focused on bpftool and libbpf that somehow I read you
patch as a proposal to include these probes directly into libbpf. Which,
as you explain (and as I should have read), is not the case. So please
accept my apologies, in this case your decisions (here and in the rest
of the patch) make sense to me :).

  reply	other threads:[~2018-12-14 18:37 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-13 19:02 [PATCH bpf-next 0/6] skip verifier/map tests if kernel support is missing Stanislav Fomichev
2018-12-13 19:02 ` [PATCH bpf-next 1/6] selftests/bpf: add map/prog type probe helpers Stanislav Fomichev
2018-12-14 12:32   ` Quentin Monnet
2018-12-14 18:16     ` Stanislav Fomichev
2018-12-14 18:37       ` Quentin Monnet [this message]
2018-12-14 19:16         ` Stanislav Fomichev
2018-12-13 19:02 ` [PATCH bpf-next 2/6] selftests/bpf: skip sockmap in test_maps if kernel doesn't have support Stanislav Fomichev
2018-12-13 19:02 ` [PATCH bpf-next 3/6] selftests/bpf: skip verifier tests for unsupported program types Stanislav Fomichev
2018-12-13 19:02 ` [PATCH bpf-next 4/6] selftests/bpf: skip verifier tests for unsupported map types Stanislav Fomichev
2018-12-13 19:03 ` [PATCH bpf-next 5/6] selftests/bpf: mark verifier test that uses bpf_trace_printk as BPF_PROG_TYPE_TRACEPOINT Stanislav Fomichev
2018-12-13 19:03 ` [PATCH bpf-next 6/6] bpf: BPF_PROG_TYPE_CGROUP_{SKB,SOCK,SOCK_ADDR} require cgroups enabled Stanislav Fomichev
2018-12-15  1:11   ` Alexei Starovoitov
2018-12-15 20:40     ` Stanislav Fomichev
2018-12-17 18:16       ` Stanislav Fomichev

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=a01d0e36-68bf-a559-48cd-7400156e9caa@netronome.com \
    --to=quentin.monnet@netronome.com \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=ecree@solarflare.com \
    --cc=netdev@vger.kernel.org \
    --cc=oss-drivers@netronome.com \
    --cc=sdf@fomichev.me \
    --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 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.