All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Kicinski <kubakici@wp.pl>, Jiri Olsa <jolsa@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	netdev@vger.kernel.org,
	Quentin Monnet <quentin.monnet@netronome.com>
Subject: Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list
Date: Thu, 26 Apr 2018 09:39:05 +0200	[thread overview]
Message-ID: <20180426073905.GI3396@krava> (raw)
In-Reply-To: <0e84fe67-59c9-a419-5ff7-05be2aa1991e@iogearbox.net>

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

  reply	other threads:[~2018-04-26  7:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-25 17:41 [PATCHv2 0/3] bpf: Store/dump license string for loaded program Jiri Olsa
2018-04-25 17:41 ` [PATCH 1/3] bpf: Add gpl_compatible flag to struct bpf_prog_info Jiri Olsa
2018-04-25 17:41 ` [PATCH 2/3] tools bpf: Sync bpf.h uapi header Jiri Olsa
2018-04-25 17:41 ` [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list Jiri Olsa
2018-04-25 21:03   ` Jakub Kicinski
2018-04-25 21:14     ` Daniel Borkmann
2018-04-26  7:39       ` Jiri Olsa [this message]
2018-04-26  7:53         ` Daniel Borkmann
2018-04-26  8:18           ` [PATCHv3 " Jiri Olsa
2018-04-26 20:49             ` Daniel Borkmann
2018-04-27  8:58               ` Jiri Olsa
2018-04-26  7:39     ` [PATCH " Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180426073905.GI3396@krava \
    --to=jolsa@redhat.com \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jolsa@kernel.org \
    --cc=kubakici@wp.pl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=quentin.monnet@netronome.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.