* [PATCH bpf 0/2] tools: bpftool: small fixes for error handling in prog load
@ 2018-06-20 18:42 Jakub Kicinski
2018-06-20 18:42 ` [PATCH bpf 1/2] tools: bpftool: remove duplicated error message on " Jakub Kicinski
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jakub Kicinski @ 2018-06-20 18:42 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jakub Kicinski
Hi!
Two small fixes for error handling in bpftool prog load, first patch
removes a duplicated message, second one frees resources correctly.
Multiple error messages break JSON:
# bpftool -jp prog load tracex1_kern.o /sys/fs/bpf/a
{
"error": "can't pin the object (/sys/fs/bpf/a): File exists"
},{
"error": "failed to pin program"
}
Jakub Kicinski (2):
tools: bpftool: remove duplicated error message on prog load
tools: bpftool: remember to close the libbpf object on prog load error
tools/bpf/bpftool/prog.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf 1/2] tools: bpftool: remove duplicated error message on prog load
2018-06-20 18:42 [PATCH bpf 0/2] tools: bpftool: small fixes for error handling in prog load Jakub Kicinski
@ 2018-06-20 18:42 ` Jakub Kicinski
2018-06-21 5:36 ` Song Liu
2018-06-20 18:42 ` [PATCH bpf 2/2] tools: bpftool: remember to close the libbpf object after " Jakub Kicinski
2018-06-21 21:10 ` [PATCH bpf 0/2] tools: bpftool: small fixes for error handling in " Daniel Borkmann
2 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2018-06-20 18:42 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jakub Kicinski
do_pin_fd() will already print out an error message if something
goes wrong. Printing another error is unnecessary and will break
JSON output, since error messages are full objects:
$ bpftool -jp prog load tracex1_kern.o /sys/fs/bpf/a
{
"error": "can't pin the object (/sys/fs/bpf/a): File exists"
},{
"error": "failed to pin program"
}
Fixes: 49a086c201a9 ("bpftool: implement prog load command")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/bpf/bpftool/prog.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 05f42a46d6ed..12b694fe0404 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -694,10 +694,8 @@ static int do_load(int argc, char **argv)
return -1;
}
- if (do_pin_fd(prog_fd, argv[1])) {
- p_err("failed to pin program");
+ if (do_pin_fd(prog_fd, argv[1]))
return -1;
- }
if (json_output)
jsonw_null(json_wtr);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf 2/2] tools: bpftool: remember to close the libbpf object after prog load
2018-06-20 18:42 [PATCH bpf 0/2] tools: bpftool: small fixes for error handling in prog load Jakub Kicinski
2018-06-20 18:42 ` [PATCH bpf 1/2] tools: bpftool: remove duplicated error message on " Jakub Kicinski
@ 2018-06-20 18:42 ` Jakub Kicinski
2018-06-21 5:37 ` Song Liu
2018-06-21 21:10 ` [PATCH bpf 0/2] tools: bpftool: small fixes for error handling in " Daniel Borkmann
2 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2018-06-20 18:42 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, oss-drivers, Jakub Kicinski
Remembering to close all descriptors and free memory may not seem
important in a user space tool like bpftool, but if we were to run
in batch mode the consumed resources start to add up quickly. Make
sure program load closes the libbpf object (which unloads and frees
it).
Fixes: 49a086c201a9 ("bpftool: implement prog load command")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/bpf/bpftool/prog.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 12b694fe0404..959aa53ab678 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -695,12 +695,18 @@ static int do_load(int argc, char **argv)
}
if (do_pin_fd(prog_fd, argv[1]))
- return -1;
+ goto err_close_obj;
if (json_output)
jsonw_null(json_wtr);
+ bpf_object__close(obj);
+
return 0;
+
+err_close_obj:
+ bpf_object__close(obj);
+ return -1;
}
static int do_help(int argc, char **argv)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 1/2] tools: bpftool: remove duplicated error message on prog load
2018-06-20 18:42 ` [PATCH bpf 1/2] tools: bpftool: remove duplicated error message on " Jakub Kicinski
@ 2018-06-21 5:36 ` Song Liu
0 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2018-06-21 5:36 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Alexei Starovoitov, Daniel Borkmann, Networking, oss-drivers
On Wed, Jun 20, 2018 at 11:42 AM, Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
> do_pin_fd() will already print out an error message if something
> goes wrong. Printing another error is unnecessary and will break
> JSON output, since error messages are full objects:
>
> $ bpftool -jp prog load tracex1_kern.o /sys/fs/bpf/a
> {
> "error": "can't pin the object (/sys/fs/bpf/a): File exists"
> },{
> "error": "failed to pin program"
> }
>
> Fixes: 49a086c201a9 ("bpftool: implement prog load command")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/bpf/bpftool/prog.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 05f42a46d6ed..12b694fe0404 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -694,10 +694,8 @@ static int do_load(int argc, char **argv)
> return -1;
> }
>
> - if (do_pin_fd(prog_fd, argv[1])) {
> - p_err("failed to pin program");
> + if (do_pin_fd(prog_fd, argv[1]))
> return -1;
> - }
>
> if (json_output)
> jsonw_null(json_wtr);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 2/2] tools: bpftool: remember to close the libbpf object after prog load
2018-06-20 18:42 ` [PATCH bpf 2/2] tools: bpftool: remember to close the libbpf object after " Jakub Kicinski
@ 2018-06-21 5:37 ` Song Liu
0 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2018-06-21 5:37 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Alexei Starovoitov, Daniel Borkmann, Networking, oss-drivers
On Wed, Jun 20, 2018 at 11:42 AM, Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
> Remembering to close all descriptors and free memory may not seem
> important in a user space tool like bpftool, but if we were to run
> in batch mode the consumed resources start to add up quickly. Make
> sure program load closes the libbpf object (which unloads and frees
> it).
>
> Fixes: 49a086c201a9 ("bpftool: implement prog load command")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/bpf/bpftool/prog.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 12b694fe0404..959aa53ab678 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -695,12 +695,18 @@ static int do_load(int argc, char **argv)
> }
>
> if (do_pin_fd(prog_fd, argv[1]))
> - return -1;
> + goto err_close_obj;
>
> if (json_output)
> jsonw_null(json_wtr);
>
> + bpf_object__close(obj);
> +
> return 0;
> +
> +err_close_obj:
> + bpf_object__close(obj);
> + return -1;
> }
>
> static int do_help(int argc, char **argv)
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf 0/2] tools: bpftool: small fixes for error handling in prog load
2018-06-20 18:42 [PATCH bpf 0/2] tools: bpftool: small fixes for error handling in prog load Jakub Kicinski
2018-06-20 18:42 ` [PATCH bpf 1/2] tools: bpftool: remove duplicated error message on " Jakub Kicinski
2018-06-20 18:42 ` [PATCH bpf 2/2] tools: bpftool: remember to close the libbpf object after " Jakub Kicinski
@ 2018-06-21 21:10 ` Daniel Borkmann
2 siblings, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2018-06-21 21:10 UTC (permalink / raw)
To: Jakub Kicinski, alexei.starovoitov; +Cc: netdev, oss-drivers
On 06/20/2018 08:42 PM, Jakub Kicinski wrote:
> Hi!
>
> Two small fixes for error handling in bpftool prog load, first patch
> removes a duplicated message, second one frees resources correctly.
> Multiple error messages break JSON:
>
> # bpftool -jp prog load tracex1_kern.o /sys/fs/bpf/a
> {
> "error": "can't pin the object (/sys/fs/bpf/a): File exists"
> },{
> "error": "failed to pin program"
> }
>
> Jakub Kicinski (2):
> tools: bpftool: remove duplicated error message on prog load
> tools: bpftool: remember to close the libbpf object on prog load error
>
> tools/bpf/bpftool/prog.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
Applied to bpf, thanks Jakub!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-21 21:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-20 18:42 [PATCH bpf 0/2] tools: bpftool: small fixes for error handling in prog load Jakub Kicinski
2018-06-20 18:42 ` [PATCH bpf 1/2] tools: bpftool: remove duplicated error message on " Jakub Kicinski
2018-06-21 5:36 ` Song Liu
2018-06-20 18:42 ` [PATCH bpf 2/2] tools: bpftool: remember to close the libbpf object after " Jakub Kicinski
2018-06-21 5:37 ` Song Liu
2018-06-21 21:10 ` [PATCH bpf 0/2] tools: bpftool: small fixes for error handling in " Daniel Borkmann
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.