netdev - Re: [PATCH bpf-next 1/6] selftests/bpf: add map/prog type probe helpers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181214191628.GA20955@mini-arch.hsd1.ca.comcast.net>
Date:   Fri, 14 Dec 2018 11:16:28 -0800
From:   Stanislav Fomichev <sdf@...ichev.me>
To:     Quentin Monnet <quentin.monnet@...ronome.com>
Cc:     Stanislav Fomichev <sdf@...gle.com>, netdev@...r.kernel.org,
        ast@...nel.org, davem@...emloft.net, daniel@...earbox.net,
        ecree@...arflare.com,
        OSS-drivers Netronome <oss-drivers@...ronome.com>
Subject: Re: [PATCH bpf-next 1/6] selftests/bpf: add map/prog type probe
 helpers

On 12/14, Quentin Monnet wrote:
> 2018-12-14 10:16 UTC-0800 ~ Stanislav Fomichev <sdf@...ichev.me>
> > On 12/14, Quentin Monnet wrote:
> >> Hi Stanislav,
> >>
> >> 2018-12-13 11:02 UTC-0800 ~ Stanislav Fomichev <sdf@...gle.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).