[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <639b2b78-af9e-4605-2b33-9bd1af8e760a@iogearbox.net>
Date: Thu, 26 Apr 2018 09:53:26 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Jiri Olsa <jolsa@...hat.com>
Cc: Jakub Kicinski <kubakici@...pl>, Jiri Olsa <jolsa@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
lkml <linux-kernel@...r.kernel.org>, netdev@...r.kernel.org,
Quentin Monnet <quentin.monnet@...ronome.com>
Subject: Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog
show/list
On 04/26/2018 09:39 AM, Jiri Olsa wrote:
> On Wed, Apr 25, 2018 at 11:14:30PM +0200, Daniel Borkmann wrote:
>> On 04/25/2018 11:03 PM, Jakub Kicinski wrote:
>>> On Wed, 25 Apr 2018 19:41:08 +0200, Jiri Olsa wrote:
>>>> @@ -295,6 +297,7 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
>>>> printf("tag ");
>>>> fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
>>>> print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
>>>> + printf(" license GPL %scompatible", info->gpl_compatible ? "" : "NON ");
>>>
>>> 3 nit picks:
>>>
>>> Other "fields" are separated by two spaces between each other:
>>>
>>> 4: kprobe name func_begin tag 57cd311f2e27366b license GPL compatible
>>> ^^ ^^ X
>>> loaded_at Apr 25/11:20 uid 0
>>> ^^
>>> xlated 16B not jited memlock 4096B
>>> ^^ ^^
>>>
>>> Could you also update the example outputs in the man page:
>>>
>>> tools/bpf/bpftool/Documentation/bpftool-prog.rst
>>>
>>> Sorry about the bike shedding but I would also vote for:
>>>
>>> "[not] GPL compatible"
>>>
>>> rather than
>>>
>>> "license GPL [NON] compatible"
>>>
>>> for brevity..
>>
>> While we're at it, can we also squeeze this whole thing a bit? Feels like
>> huge string wasted for very little information compared to the rest of the
>> dump. Just append the string "gpl" at the end of the line if info->gpl_compatible
>> is set, otherwise just add nothing. This also allows to naturally grep
>> for it e.g. `bpftool p | grep gpl` if you need a quick summary.
>
> that's fine with me.. so 'gpl' in here:
>
> 5: tracepoint name func tag 57cd311f2e27366b gpl
> loaded_at Apr 26/09:37 uid 0
> xlated 16B not jited memlock 4096B
>
> and keeping tyhe whole name in json output:
>
> [{
> "id": 5,
> "type": "tracepoint",
> "name": "func",
> "tag": "57cd311f2e27366b",
> "gpl_compatible": true,
> "loaded_at": "Apr 26/09:37",
> "uid": 0,
> "bytes_xlated": 16,
> "jited": false,
> "bytes_memlock": 4096
> }
> ]
>
> how about that?
Sounds good, thanks Jiri!
Powered by blists - more mailing lists