Date:   Tue, 12 Mar 2019 12:20:24 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Song Liu <songliubraving@...com>, bpf@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        ast@...nel.org, kernel-team@...com, peterz@...radead.org,
        acme@...hat.com, jolsa@...nel.org, namhyung@...nel.org
Subject: Re: [PATCH v7 perf,bpf 02/15] bpf: libbpf: introduce
 bpf_program__get_prog_info_linear()

On 03/11/2019 09:57 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 11, 2019 at 09:45:49PM +0100, Daniel Borkmann escreveu:
>> On 03/11/2019 07:26 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Thu, Mar 07, 2019 at 09:57:57AM -0800, Song Liu escreveu:
>>>> Currently, bpf_prog_info includes 9 arrays. The user has the option to
>>>> fetch any combination of these arrays. However, this requires a lot of
>>>> handling of these arrays. This work becomes more tricky when we need to
>>>> store bpf_prog_info to a file, because these arrays are allocated
>>>> independently.
>>>>
>>>> This patch introduces struct bpf_prog_info_linear, which stores arrays
>>>> of bpf_prog_info in continues memory. Helper functions are introduced
>>>> to unify the work to get different information of bpf_prog_info.
>>>> Specifically, bpf_program__get_prog_info_linear() allows the user to
>>>> select which arrays to fetch, and handles details for the user.
>>>>
>>>> Plesae see the comments before enum bpf_prog_info_array for more details
>>>> and examples.
>>>>
>>>> Cc: Daniel Borkmann <daniel@...earbox.net>
>>>> Cc: Alexei Starovoitov <ast@...nel.org>
>>>
>>> Daniel, are you ok with these changes to libbpf and bpftool? Perhaps
>>> those should be detached from this patchkit and submitted sooner,
>>> eroding the size of this kit.
>>>
>>> Alternatively, if you're ok with it, please provide your Acked-by and
>>> I'll process as soon as I get back to it after Jiri is done reviewing.
>>
>> Overall looks okay. Are you planning to get these in for 5.1 window? If
> 
> I hope so.
> 
>> yes, that would be great, otherwise we might need to cherry-pick the libbpf
> 
> So I'll try and grab those for my next pull req to Ingo, this week,
> will add your Acked-by, ok?

Please do, thanks.

>> and bpftool ones from your tree into bpf-next as well since there's just
>> too much going on in this area where we'd potentially run into complex
>> merge conflicts. In the latter case, libbpf.map would need to be fixed up
>> to LIBBPF_0.0.3 as convention is that this is in line with kernel release.
> 
> BTW, while running my build tests I had to add this to today's batch,
> should be trivial when merging, I think, ok?

That's fine as well, ack, lets shoot for getting it in during merge window
such that we don't need to duplicate these dependencies in bpf trees.

> commit dfcbc2f2994b8a3af3605a26dc29c07ad7378bf4
> Author: Arnaldo Carvalho de Melo <acme@...hat.com>
> Date:   Mon Mar 11 17:07:52 2019 -0300
> 
>     tools lib bpf: Fix the build by adding a missing stdarg.h include
>     
>     The libbpf_print_fn_t typedef uses va_list without including the header
>     where that type is defined, stdarg.h, breaking in places where we're
>     unlucky for that type not to be already defined by some previously
>     included header.
>     
>     Noticed while building on fedora 24 cross building tools/perf to the ARC
>     architecture using the uClibc C library:
>     
>       28 fedora:24-x-ARC-uClibc   : FAIL arc-linux-gcc (ARCompact ISA Linux uClibc toolchain 2017.09-rc2) 7.1.1 20170710
>     
>         CC       /tmp/build/perf/tests/llvm.o
>       In file included from tests/llvm.c:3:0:
>       /git/linux/tools/lib/bpf/libbpf.h:57:20: error: unknown type name 'va_list'
>             const char *, va_list ap);
>                           ^~~~~~~
>       /git/linux/tools/lib/bpf/libbpf.h:59:34: error: unknown type name 'libbpf_print_fn_t'
>        LIBBPF_API void libbpf_set_print(libbpf_print_fn_t fn);
>                                         ^~~~~~~~~~~~~~~~~
>       mv: cannot stat '/tmp/build/perf/tests/.llvm.o.tmp': No such file or directory
>     
>     Cc: Alexei Starovoitov <ast@...nel.org>
>     Cc: Daniel Borkmann <daniel@...earbox.net>
>     Cc: Jakub Kicinski <jakub.kicinski@...ronome.com>
>     Cc: Jiri Olsa <jolsa@...nel.org>
>     Cc: Namhyung Kim <namhyung@...nel.org>
>     Cc: Quentin Monnet <quentin.monnet@...ronome.com>
>     Cc: Stanislav Fomichev <sdf@...gle.com>
>     Cc: Yonghong Song <yhs@...com>
>     Fixes: a8a1f7d09cfc ("libbpf: fix libbpf_print")
>     Link: https://lkml.kernel.org/n/tip-5270n2quu2gqz22o7itfdx00@git.kernel.org
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> 
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index b4652aa1a58a..aa1521a51687 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -10,6 +10,7 @@
>  #ifndef __LIBBPF_LIBBPF_H
>  #define __LIBBPF_LIBBPF_H
>  
> +#include <stdarg.h>
>  #include <stdio.h>
>  #include <stdint.h>
>  #include <stdbool.h>
> 

Thanks,
Daniel

Powered by blists - more mailing lists