linux-kernel - Re: [PATCH 1/5] bpf tools: Improve libbpf error reporting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20151105153647.GU13236@kernel.org>
Date:	Thu, 5 Nov 2015 12:36:47 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Wang Nan <wangnan0@...wei.com>
Cc:	namhyung@...nel.org, lizefan@...wei.com, pi3orama@....com,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
	Jiri Olsa <jolsa@...nel.org>,
	Alexei Starovoitov <ast@...mgrid.com>
Subject: Re: [PATCH 1/5] bpf tools: Improve libbpf error reporting

Em Thu, Nov 05, 2015 at 12:35:11PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Nov 05, 2015 at 12:26:07PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Nov 05, 2015 at 04:26:58AM +0000, Wang Nan escreveu:
> > > In this patch, a series libbpf specific error numbers and
> > > libbpf_strerror() are created to help reporting error to caller.
> > > Functions are updated to pass correct error number through macro
> > > CHECK_ERR().
> > > 
> > > All users of bpf_object__open{_buffer}() and bpf_program__title()
> > > in perf are modified accordingly. In addition, due to error code
> > > changing, bpf__strerror_load() also modified to use new error code.
> > > 
> > > bpf__strerror_head() is also changed accordingly so it can parse
> > > libbpf error. bpf_loader_strerror() is introduced for it, and will
> > > be improved by following patch.
> > 
> > I am applying this, test shows a improvement in error reporting, but
> > please look below for some suggestions:
> 
> I take that back, better fix this now, see the other message about the
> BPF loader part too.

And please add something like this so that we can see how it was before
your change and how it became, to make it clear the value of your patch,
i.e. what is that the user will see before and after your patch:


    Committer note:
    
    Before:
    
      the foo.o object has an invalid kernel version:
    
      # /tmp/oldperf record -e /tmp/foo.o sleep 1
      event syntax error: '/tmp/foo.o'
                           \___ Invalid argument: Are you root and runing a CONFIG_BPF_SYSCALL kernel?
    
      (add -v to see detail)
      Run 'perf list' for a list of valid events
    
       Usage: perf record [<options>] [<command>]
          or: perf record [<options>] -- <command> [<options>]
    
          -e, --event <event>   event selector. use 'perf list' to list available events
    
    After:
    
      # perf record -e /tmp/foo.o sleep 1
      event syntax error: '/tmp/foo.o'
                           \___ Failed to load program: Validate your program and check 'license'/'version' sections in your object
    
      (add -v to see detail)
      Run 'perf list' for a list of valid events
    
       Usage: perf record [<options>] [<command>]
          or: perf record [<options>] -- <command> [<options>]
    
          -e, --event <event>   event selector. use 'perf list' to list available events
      #

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/