From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 05754C282C0 for ; Wed, 23 Jan 2019 14:11:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D2B002133D for ; Wed, 23 Jan 2019 14:11:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726984AbfAWOLN (ORCPT ); Wed, 23 Jan 2019 09:11:13 -0500 Received: from www62.your-server.de ([213.133.104.62]:35492 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726126AbfAWOLM (ORCPT ); Wed, 23 Jan 2019 09:11:12 -0500 Received: from [78.46.172.2] (helo=sslproxy05.your-server.de) by www62.your-server.de with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89_1) (envelope-from ) id 1gmJFG-0007bR-Ax; Wed, 23 Jan 2019 15:11:10 +0100 Received: from [62.203.87.61] (helo=linux.home) by sslproxy05.your-server.de with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1gmJFG-000I34-4A; Wed, 23 Jan 2019 15:11:10 +0100 Subject: Re: [PATCH bpf-next v2 2/8] libbpf: Add a helper for retrieving a prog via index To: Maciej Fijalkowski Cc: ast@kernel.org, netdev@vger.kernel.org, jakub.kicinski@netronome.com, brouer@redhat.com, quentin.monnet@netronome.com References: <20190121091041.14666-1-maciejromanfijalkowski@gmail.com> <20190121091041.14666-3-maciejromanfijalkowski@gmail.com> <91d162e0-3d15-c1d8-1e80-8d0a4f561540@iogearbox.net> <20190123144159.000051bd@gmail.com> From: Daniel Borkmann Message-ID: <82ab385e-fabc-87eb-9816-176fa9edba1d@iogearbox.net> Date: Wed, 23 Jan 2019 15:11:09 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20190123144159.000051bd@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.100.2/25324/Wed Jan 23 12:01:28 2019) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 01/23/2019 02:41 PM, Maciej Fijalkowski wrote: > On Wed, 23 Jan 2019 11:41:11 +0100 > Daniel Borkmann wrote: >> On 01/21/2019 10:10 AM, Maciej Fijalkowski wrote: >>> xdp_redirect_cpu has a 6 different XDP programs that can be attached to >>> network interface. This sample has a option --prognum that allows user >>> for specifying which particular program from a given set will be >>> attached to network interface. >>> In order to make it easier when converting the mentioned sample to >>> libbpf usage, add a function to libbpf that will return program's fd for >>> a given index. >>> >>> Note that there is already a bpf_object__find_prog_by_idx, which could >>> be exported and might be used for that purpose, but it operates on the >>> number of ELF section and here we need an index from a programs array >>> within the bpf_object. >> >> Series in general looks good to me. Few minor comments, mainly in relation >> to the need for libbpf extensions. >> >> Would it not be a better interface to the user to instead choose the prog >> based on section name and then retrieve it via bpf_object__find_program_by_title() >> instead of prognum (which feels less user friendly) at least? > > I couldn't decide which one from: > * adding a libbpf helper > * changing the xdp_redirect_cpu behaviour > would be more invasive when I was converting this sample to libbpf support. > > Your suggestion sounds good, but I'm wondering about the actual implementation. > I suppose that we would choose the program via command line. Some program Yes, selection for loading prog could be done through that. > section names in this sample are a bit long and it might be irritating for > user to type in for example "xdp_cpu_map5_lb_hash_ip_pairs", no? Or maybe we I think that's an implementation detail of this specific sample. For the prog number, the user would need to first look up the source and figure out the prog number correlation to the name that should be loaded which is a bit ugly (unless I missed something). So we might as well make it easier and allow to choose by name. Figuring out which programs are available in an object file is a separate tooling issue, imho. readelf is one way but not optimal, and it would be a nice addition to bpftool as brought up at plumbers [0] to dump the contents of a bpf obj file. Latter might also be nice to enrich with BTF info when present (e.g. types, line annotation etc) so this would be much more powerful than plain introspection via readelf. [0] http://vger.kernel.org/lpc-bpf2018.html#session-6 > can live with this. In case of typo and program being not found it would be > good to print out the section names to choose from in usage(). Agree, might be nice as well via libbpf. >>> Signed-off-by: Maciej Fijalkowski >>> Reviewed-by: Jakub Kicinski >>> --- >>> tools/lib/bpf/libbpf.c | 8 ++++++++ >>> tools/lib/bpf/libbpf.h | 3 +++ >>> tools/lib/bpf/libbpf.map | 1 + >>> 3 files changed, 12 insertions(+) >>> >>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c >>> index dc838bea403f..21c84d0f6128 100644 >>> --- a/tools/lib/bpf/libbpf.c >>> +++ b/tools/lib/bpf/libbpf.c >>> @@ -935,6 +935,14 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags) >>> return err; >>> } >>> >>> +int >>> +bpf_object__get_prog_fd_by_num(struct bpf_object *obj, int idx) >>> +{ >>> + if (idx >= 0 && idx < obj->nr_programs) >>> + return bpf_program__fd(&obj->programs[idx]); >>> + return -ENOENT; >>> +} >>> + >>> static struct bpf_program * >>> bpf_object__find_prog_by_idx(struct bpf_object *obj, int idx) >>> { >>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h >>> index 7f10d36abdde..ca1b381cb3ad 100644 >>> --- a/tools/lib/bpf/libbpf.h >>> +++ b/tools/lib/bpf/libbpf.h >>> @@ -95,6 +95,9 @@ LIBBPF_API int bpf_object__btf_fd(const struct bpf_object *obj); >>> LIBBPF_API struct bpf_program * >>> bpf_object__find_program_by_title(struct bpf_object *obj, const char *title); >>> >>> +LIBBPF_API int >>> +bpf_object__get_prog_fd_by_num(struct bpf_object *obj, int idx); >>> + >>> LIBBPF_API struct bpf_object *bpf_object__next(struct bpf_object *prev); >>> #define bpf_object__for_each_safe(pos, tmp) \ >>> for ((pos) = bpf_object__next(NULL), \ >>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map >>> index 7c59e4f64082..871d2fc07150 100644 >>> --- a/tools/lib/bpf/libbpf.map >>> +++ b/tools/lib/bpf/libbpf.map >>> @@ -127,4 +127,5 @@ LIBBPF_0.0.1 { >>> LIBBPF_0.0.2 { >>> global: >>> bpf_object__find_map_fd_by_name; >>> + bpf_object__get_prog_fd_by_num; >>> } LIBBPF_0.0.1; >>> >> >