On 12/14, Quentin Monnet wrote:
> 2018-12-14 10:16 UTC-0800 ~ Stanislav Fomichev <s...@fomichev.me>
> > On 12/14, Quentin Monnet wrote:
> >> Hi Stanislav,
> >>
> >> 2018-12-13 11:02 UTC-0800 ~ Stanislav Fomichev <s...@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 :).
No worries, I was just scratching my own itch with these (wanted to have
a simple non-controversial probers for the test cases, I can migrate
to your libbpf helpers whenever they are available).
