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

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

[Jiri Olsa]

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(+)

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

Syncing the bpf.h uapi header with tools.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Jakub Kicinski]

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

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

[Daniel Borkmann]

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

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

[Jiri Olsa]

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

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

[Daniel Borkmann]

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!

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

[Jiri Olsa]

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 <jolsa@kernel.org>
---
 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

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

[Daniel Borkmann]

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 <jolsa@kernel.org>

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.

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

[Jiri Olsa]

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 <jolsa@kernel.org>
> 
> 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

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

[Jiri Olsa]

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