Netdev List
 help / color / mirror / Atom feed
From: Stanislav Fomichev <sdf@fomichev.me>
To: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Stanislav Fomichev <sdf@google.com>,
	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
Subject: Re: [PATCH bpf-next 2/3] libbpf: cleanup after partial failure in bpf_object__pin
Date: Wed, 7 Nov 2018 15:25:16 -0800	[thread overview]
Message-ID: <20181107232516.qiy24ccbq3hh4ail@mini-arch> (raw)
In-Reply-To: <20181107150605.59a8ac1c@cakuba.netronome.com>

On 11/07, Jakub Kicinski wrote:
> On Wed, 7 Nov 2018 15:00:21 -0800, Stanislav Fomichev wrote:
> > > > +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.  
> >
> > How about we check beforehand and bail early if we are going to
> > overwrite something?
> 
> Possible, although the most common way to handle situation like this in
> the kernel is to "continue the iteration in reverse" over the list.
> I.e. walk the list back.  I think the objects are on a double linked
> list.  You may need to add the appropriate foreach macro and helper..
That sounds more complicated than just ensuring that the top directory
for the pins doesn't exist and then rm -rf it on failure.
I'm thinking about copy-pasting rm_rf from perf
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/util/util.c#n119).
Thoughts?

Btw, current patch won't work because of those /0 added by bpf_program__pin.

  reply	other threads:[~2018-11-08  8:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-07 22:43 [PATCH v2 bpf-next 0/3] bpftool: support loading flow dissector Stanislav Fomichev
2018-11-07 22:43 ` [PATCH bpf-next 1/3] selftests/bpf: rename flow dissector section to flow_dissector Stanislav Fomichev
2018-11-07 22:43 ` [PATCH bpf-next 2/3] libbpf: cleanup after partial failure in bpf_object__pin Stanislav Fomichev
2018-11-07 22:56   ` Jakub Kicinski
2018-11-07 23:00     ` Stanislav Fomichev
2018-11-07 23:06       ` Jakub Kicinski
2018-11-07 23:25         ` Stanislav Fomichev [this message]
2018-11-07 23:38           ` Jakub Kicinski
2018-11-07 22:43 ` [PATCH bpf-next 3/3] bpftool: support loading flow dissector Stanislav Fomichev
2018-11-07 23:09   ` Jakub Kicinski
2018-11-07 23:13     ` Stanislav Fomichev
2018-11-07 23:29       ` Jakub Kicinski
2018-11-07 23:34         ` Stanislav Fomichev
2018-11-07 23:41           ` Jakub Kicinski
2018-11-08  0:40             ` Stanislav Fomichev

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=20181107232516.qiy24ccbq3hh4ail@mini-arch \
    --to=sdf@fomichev.me \
    --cc=ast@kernel.org \
    --cc=bhole_prashant_q7@lab.ntt.co.jp \
    --cc=daniel@iogearbox.net \
    --cc=guro@fb.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=jbenc@redhat.com \
    --cc=jiong.wang@netronome.com \
    --cc=john.fastabend@gmail.com \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=osk@fb.com \
    --cc=quentin.monnet@netronome.com \
    --cc=sandipan@linux.vnet.ibm.com \
    --cc=sdf@google.com \
    --cc=shuah@kernel.org \
    --cc=treeze.taeung@gmail.com \
    --cc=yhs@fb.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox