From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [PATCH bpf-next 2/3] libbpf: cleanup after partial failure in bpf_object__pin Date: Wed, 7 Nov 2018 14:56:48 -0800 Message-ID: <20181107145648.66f47037@cakuba.netronome.com> References: <20181107224356.73080-1-sdf@google.com> <20181107224356.73080-3-sdf@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, shuah@kernel.org, quentin.monnet@netronome.com, guro@fb.com, jiong.wang@netronome.com, bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com, jbenc@redhat.com, treeze.taeung@gmail.com, yhs@fb.com, osk@fb.com, sandipan@linux.vnet.ibm.com To: Stanislav Fomichev Return-path: Received: from mail-pl1-f196.google.com ([209.85.214.196]:38564 "EHLO mail-pl1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726896AbeKHI30 (ORCPT ); Thu, 8 Nov 2018 03:29:26 -0500 Received: by mail-pl1-f196.google.com with SMTP id p4-v6so6157434plo.5 for ; Wed, 07 Nov 2018 14:56:53 -0800 (PST) In-Reply-To: <20181107224356.73080-3-sdf@google.com> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, 7 Nov 2018 14:43:55 -0800, Stanislav Fomichev wrote: > bpftool will use bpf_object__pin in the next commit to pin all programs > and maps from the file; in case of a partial failure, we need to get > back to the clean state (undo previous program/map pins). > > Signed-off-by: Stanislav Fomichev > --- > tools/lib/bpf/libbpf.c | 58 ++++++++++++++++++++++++++++++++++-------- > 1 file changed, 48 insertions(+), 10 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index d6e62e90e8d4..309abe7196f3 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -1803,14 +1803,17 @@ int bpf_object__pin(struct bpf_object *obj, const char *path) > > len = snprintf(buf, PATH_MAX, "%s/%s", path, > bpf_map__name(map)); > - if (len < 0) > - return -EINVAL; > - else if (len >= PATH_MAX) > - return -ENAMETOOLONG; > + if (len < 0) { > + err = -EINVAL; > + goto err_unpin_maps; > + } else if (len >= PATH_MAX) { > + err = -ENAMETOOLONG; > + goto err_unpin_maps; > + } > > err = bpf_map__pin(map, buf); > if (err) > - return err; > + goto err_unpin_maps; > } > > bpf_object__for_each_program(prog, obj) { > @@ -1819,17 +1822,52 @@ int bpf_object__pin(struct bpf_object *obj, const char *path) > > len = snprintf(buf, PATH_MAX, "%s/%s", path, > prog->section_name); > - if (len < 0) > - return -EINVAL; > - else if (len >= PATH_MAX) > - return -ENAMETOOLONG; > + if (len < 0) { > + err = -EINVAL; > + goto err_unpin_programs; > + } else if (len >= PATH_MAX) { > + err = -ENAMETOOLONG; > + goto err_unpin_programs; > + } > > err = bpf_program__pin(prog, buf); > if (err) > - return err; > + goto err_unpin_programs; > } > > return 0; > + > +err_unpin_programs: > + bpf_object__for_each_program(prog, obj) { > + char buf[PATH_MAX]; > + int len; > + > + len = snprintf(buf, PATH_MAX, "%s/%s", path, > + prog->section_name); > + if (len < 0) > + continue; > + else if (len >= PATH_MAX) > + continue; > + > + unlink(buf); I think that's no bueno, if pin failed because the file already exists you'll now remove that already existing file. > + } > + > +err_unpin_maps: > + bpf_map__for_each(map, obj) { > + char buf[PATH_MAX]; > + int len; > + > + len = snprintf(buf, PATH_MAX, "%s/%s", path, > + bpf_map__name(map)); > + if (len < 0) > + continue; > + else if (len >= PATH_MAX) > + continue; > + > + unlink(buf); > + } > + > + return err; > } > > void bpf_object__close(struct bpf_object *obj)