2018-04-25 17:43:04

by Jiri Olsa

[permalink] [raw]
Subject: [PATCHv2 0/3] bpf: Store/dump license string for loaded program

hi,
sending the change to store and dump the license
info for loaded BPF programs. It's important for
us get the license info, when investigating on
screwed up machine.

v2 changes:
- dumping only the GPL compatible bool, without
storing the whole license string

Adding change to bpftool to dump the license GPL
compatible info via:

# bpftool prog list
3: kprobe name func_begin tag 57cd311f2e27366b license GPL NON compatible
loaded_at Apr 25/11:20 uid 0
xlated 16B not jited memlock 4096B

# bpftool prog list
4: kprobe name func_begin tag 57cd311f2e27366b license GPL compatible
loaded_at Apr 25/11:20 uid 0
xlated 16B not jited memlock 4096B

# bpftool prog show --json
[{"id":3,"type":"kprobe","name":"func ... ,"gpl_compatible":false,"loade...

Also available at:
https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
bpf/license

thanks,
jirka


---
Jiri Olsa (3):
bpf: Add gpl_compatible flag to struct bpf_prog_info
tools bpf: Sync bpf.h uapi header
tools bpftool: Display license GPL compatible in prog show/list

include/uapi/linux/bpf.h | 1 +
kernel/bpf/syscall.c | 1 +
tools/bpf/bpftool/prog.c | 3 +++
tools/include/uapi/linux/bpf.h | 1 +
4 files changed, 6 insertions(+)


2018-04-25 17:43:06

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list

Display the license GPL NON/compatible string in bpftool
prog command, like:

# bpftool prog list
3: kprobe name func_begin tag 57cd311f2e27366b license GPL NON compatible
loaded_at Apr 25/11:20 uid 0
xlated 16B not jited memlock 4096B

# bpftool prog list
4: kprobe name func_begin tag 57cd311f2e27366b license GPL compatible
loaded_at Apr 25/11:20 uid 0
xlated 16B not jited memlock 4096B

# bpftool prog show --json
[{"id":3,"type":"kprobe","name":"func ... ,"gpl_compatible":false,"loade...

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/bpf/bpftool/prog.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 548adb9b7317..b8b4341a1342 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -235,6 +235,8 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
info->tag[0], info->tag[1], info->tag[2], info->tag[3],
info->tag[4], info->tag[5], info->tag[6], info->tag[7]);

+ jsonw_bool_field(json_wtr, "gpl_compatible", info->gpl_compatible);
+
print_dev_json(info->ifindex, info->netns_dev, info->netns_ino);

if (info->load_time) {
@@ -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 ");
printf("\n");

if (info->load_time) {
--
2.13.6


2018-04-25 17:43:17

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 1/3] bpf: Add gpl_compatible flag to struct bpf_prog_info

Adding gpl_compatible flag to struct bpf_prog_info
so it can be dumped via bpf_prog_get_info_by_fd and
displayed via bpftool progs dump.

Alexei noticed 4-byte hole in struct bpf_prog_info,
so we put the u32 flags field in there, and we can
keep adding bit fields in there without breaking
user space.

Signed-off-by: Jiri Olsa <[email protected]>
---
include/uapi/linux/bpf.h | 1 +
kernel/bpf/syscall.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e6679393b687..da8801860c7d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1060,6 +1060,7 @@ struct bpf_prog_info {
__aligned_u64 map_ids;
char name[BPF_OBJ_NAME_LEN];
__u32 ifindex;
+ __u32 gpl_compatible:1;
__u64 netns_dev;
__u64 netns_ino;
} __attribute__((aligned(8)));
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index fe23dc5a3ec4..7bb4ff1c770a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1914,6 +1914,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
info.load_time = prog->aux->load_time;
info.created_by_uid = from_kuid_munged(current_user_ns(),
prog->aux->user->uid);
+ info.gpl_compatible = prog->gpl_compatible;

memcpy(info.tag, prog->tag, sizeof(prog->tag));
memcpy(info.name, prog->aux->name, sizeof(prog->aux->name));
--
2.13.6


2018-04-25 17:44:18

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 2/3] tools bpf: Sync bpf.h uapi header

Syncing the bpf.h uapi header with tools.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/include/uapi/linux/bpf.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index e6679393b687..da8801860c7d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1060,6 +1060,7 @@ struct bpf_prog_info {
__aligned_u64 map_ids;
char name[BPF_OBJ_NAME_LEN];
__u32 ifindex;
+ __u32 gpl_compatible:1;
__u64 netns_dev;
__u64 netns_ino;
} __attribute__((aligned(8)));
--
2.13.6


2018-04-25 21:05:56

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list

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..

2018-04-25 21:16:00

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list

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.

Thanks,
Daniel

2018-04-26 07:40:48

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list

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?

thanks,
jirka

2018-04-26 07:41:29

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list

On Wed, Apr 25, 2018 at 02:03:46PM -0700, 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
> ^^ ^^
>

will change

> Could you also update the example outputs in the man page:
>
> tools/bpf/bpftool/Documentation/bpftool-prog.rst

oops, I always forget that.. will do

thanks,
jirka

2018-04-26 07:56:06

by Daniel Borkmann

[permalink] [raw]
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!

2018-04-26 08:19:33

by Jiri Olsa

[permalink] [raw]
Subject: [PATCHv3 3/3] tools bpftool: Display license GPL compatible in prog show/list

On Thu, Apr 26, 2018 at 09:53:26AM +0200, Daniel Borkmann wrote:
> 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!

v3 of the last patch attached, the branch is also updated

thanks,
jirka


---
Display the license "gpl" string in bpftool prog command, like:

# bpftool prog list
5: tracepoint name func tag 57cd311f2e27366b gpl
loaded_at Apr 26/09:37 uid 0
xlated 16B not jited memlock 4096B

# bpftool --json --pretty prog show
[{
"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
}
]

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 3 ++-
tools/bpf/bpftool/prog.c | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 67ca6c69376c..43d34a5c3ec5 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -95,7 +95,7 @@ EXAMPLES
**# bpftool prog show**
::

- 10: xdp name some_prog tag 005a3d2123620c8b
+ 10: xdp name some_prog tag 005a3d2123620c8b gpl
loaded_at Sep 29/20:11 uid 0
xlated 528B jited 370B memlock 4096B map_ids 10

@@ -108,6 +108,7 @@ EXAMPLES
"id": 10,
"type": "xdp",
"tag": "005a3d2123620c8b",
+ "gpl_compatible": true,
"loaded_at": "Sep 29/20:11",
"uid": 0,
"bytes_xlated": 528,
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 548adb9b7317..e71a0a11afde 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -235,6 +235,8 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
info->tag[0], info->tag[1], info->tag[2], info->tag[3],
info->tag[4], info->tag[5], info->tag[6], info->tag[7]);

+ jsonw_bool_field(json_wtr, "gpl_compatible", info->gpl_compatible);
+
print_dev_json(info->ifindex, info->netns_dev, info->netns_ino);

if (info->load_time) {
@@ -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("%s", info->gpl_compatible ? " gpl" : "");
printf("\n");

if (info->load_time) {
--
2.13.6


2018-04-26 20:51:01

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCHv3 3/3] tools bpftool: Display license GPL compatible in prog show/list

On 04/26/2018 10:18 AM, Jiri Olsa wrote:
[...]
> v3 of the last patch attached, the branch is also updated
>
> thanks,
> jirka
>
>
> ---
> Display the license "gpl" string in bpftool prog command, like:
>
> # bpftool prog list
> 5: tracepoint name func tag 57cd311f2e27366b gpl
> loaded_at Apr 26/09:37 uid 0
> xlated 16B not jited memlock 4096B
>
> # bpftool --json --pretty prog show
> [{
> "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
> }
> ]
>
> Signed-off-by: Jiri Olsa <[email protected]>

Ok, v2 from prior two patches and v3 of this one applied to bpf-next. Please
next time always submit a fresh new series at once, thanks Jiri.

2018-04-27 09:00:19

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCHv3 3/3] tools bpftool: Display license GPL compatible in prog show/list

On Thu, Apr 26, 2018 at 10:49:25PM +0200, Daniel Borkmann wrote:
> On 04/26/2018 10:18 AM, Jiri Olsa wrote:
> [...]
> > v3 of the last patch attached, the branch is also updated
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > Display the license "gpl" string in bpftool prog command, like:
> >
> > # bpftool prog list
> > 5: tracepoint name func tag 57cd311f2e27366b gpl
> > loaded_at Apr 26/09:37 uid 0
> > xlated 16B not jited memlock 4096B
> >
> > # bpftool --json --pretty prog show
> > [{
> > "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
> > }
> > ]
> >
> > Signed-off-by: Jiri Olsa <[email protected]>
>
> Ok, v2 from prior two patches and v3 of this one applied to bpf-next. Please
> next time always submit a fresh new series at once, thanks Jiri.

noted, thanks a lot

jirka