bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Florent Revest <revest@chromium.org>
Cc: Jiri Olsa <jolsa@kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	Yonghong Song <yhs@fb.com>, Martin KaFai Lau <kafai@fb.com>,
	David Miller <davem@redhat.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	Wenbo Zhang <ethercflow@gmail.com>,
	KP Singh <kpsingh@chromium.org>, Andrii Nakryiko <andriin@fb.com>,
	bgregg@netflix.com, Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [RFC 0/3] bpf: Add d_path helper
Date: Fri, 3 Apr 2020 10:55:29 +0200	[thread overview]
Message-ID: <20200403085529.GD2784502@krava> (raw)
In-Reply-To: <5968eda68bfec39387c34ffaf0ecc3ed5d8afd6f.camel@chromium.org>

On Thu, Apr 02, 2020 at 04:03:00PM +0200, Florent Revest wrote:
> On Wed, 2020-04-01 at 13:09 +0200, Jiri Olsa wrote:
> > hi,
> > adding d_path helper to return full path for 'path' object.
> > 
> > I originally added and used 'file_path' helper, which did the same,
> > but used 'struct file' object. Then realized that file_path is just
> > a wrapper for d_path, so we'd cover more calling sites if we add
> > d_path helper and allowed resolving BTF object within another object,
> > so we could call d_path also with file pointer, like:
> > 
> >   bpf_d_path(&file->f_path, buf, size);
> > 
> > This feature is mainly to be able to add dpath (filepath originally)
> > function to bpftrace, which seems to work nicely now, like:
> > 
> >   # bpftrace -e 'kretfunc:fget { printf("%s\n", dpath(args->ret-
> > >f_path));  }' 
> > 
> > I'm not completely sure this is all safe and bullet proof and there's
> > no other way to do this, hence RFC post.
> > 
> > I'd be happy also with file_path function, but I thought it'd be
> > a shame not to try to add d_path with the verifier change.
> > I'm open to any suggestions ;-)
> 
> First of all I want to mention that we are really interested in this
> feature so thanks a lot for bringing it up Jiri! I have experimented
> with similar BPF helpers in the past few months so I hope my input can
> be helpful! :)
> 
> One of our use-cases is to gather information about execution events,
> including a bunch of paths (such as the executable command, the
> resolved executable file path and the current-working-directory) and
> then output them to Perf.
> Each of those paths can be up to PATH_MAX(one page) long so we would
> pre-allocate a data structure with a few identifiers (to later
> reassemble the event from userspace) and a page of data and then we
> would output it using bpf_perf_event_output. However, with three mostly
> empty pages per event, we would quickly fill up the ring buffer and
> loose many events.
> This might be a bit out-of-scope at this moment but one of the
> teachings we got from playing with such a helper is that we would also
> need a helper for outputting strings to Perf, pre-pended with a header
> buffer.

I think bpftrace uses fixed size as well at some point,
but very small one, which is still sufficent for tools usage,
but we can always send only data with the size of the path

thanks for info
jirka


  reply	other threads:[~2020-04-03  8:55 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-01 11:09 [RFC 0/3] bpf: Add d_path helper Jiri Olsa
2020-04-01 11:09 ` [PATCH 1/3] bpf: Add support to check if BTF object is nested in another object Jiri Olsa
2020-04-07  1:16   ` Alexei Starovoitov
2020-04-07  9:37     ` Jiri Olsa
2020-04-01 11:09 ` [PATCH 2/3] bpf: Add d_path helper Jiri Olsa
2020-04-02 14:02   ` Florent Revest
2020-04-03  9:01     ` Jiri Olsa
2020-04-06  2:49       ` Andrii Nakryiko
2020-04-01 11:09 ` [PATCH 3/3] selftests/bpf: Add test for " Jiri Olsa
2020-04-02 14:03 ` [RFC 0/3] bpf: Add " Florent Revest
2020-04-03  8:55   ` Jiri Olsa [this message]
2020-04-02 14:21 ` Al Viro
2020-04-03  9:08   ` Jiri Olsa
2020-04-06  3:16     ` Al Viro
2020-04-06  9:09       ` Jiri Olsa
2020-04-06 12:47         ` Al Viro
2020-04-07  1:10         ` Alexei Starovoitov
2020-04-07  8:53           ` Jiri Olsa
2020-04-07  9:27           ` KP Singh
2020-04-07  9:45             ` 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=20200403085529.GD2784502@krava \
    --to=jolsa@redhat.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bgregg@netflix.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@redhat.com \
    --cc=ethercflow@gmail.com \
    --cc=hawk@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kafai@fb.com \
    --cc=kpsingh@chromium.org \
    --cc=netdev@vger.kernel.org \
    --cc=revest@chromium.org \
    --cc=viro@zeniv.linux.org.uk \
    --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;
as well as URLs for NNTP newsgroup(s).