From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stanislav Fomichev Subject: Re: [PATCH bpf-next 1/6] selftests/bpf: add map/prog type probe helpers Date: Fri, 14 Dec 2018 11:16:28 -0800 Message-ID: <20181214191628.GA20955@mini-arch.hsd1.ca.comcast.net> References: <20181213190301.65816-1-sdf@google.com> <20181213190301.65816-2-sdf@google.com> <20181214181649.GA32470@mini-arch.hsd1.ca.comcast.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Stanislav Fomichev , netdev@vger.kernel.org, ast@kernel.org, davem@davemloft.net, daniel@iogearbox.net, ecree@solarflare.com, OSS-drivers Netronome To: Quentin Monnet Return-path: Received: from mail-pf1-f195.google.com ([209.85.210.195]:39043 "EHLO mail-pf1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730123AbeLNTQb (ORCPT ); Fri, 14 Dec 2018 14:16:31 -0500 Received: by mail-pf1-f195.google.com with SMTP id c72so3275282pfc.6 for ; Fri, 14 Dec 2018 11:16:30 -0800 (PST) Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On 12/14, Quentin Monnet wrote: > 2018-12-14 10:16 UTC-0800 ~ Stanislav Fomichev > > On 12/14, Quentin Monnet wrote: > >> Hi Stanislav, > >> > >> 2018-12-13 11:02 UTC-0800 ~ Stanislav Fomichev > >>> 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).