All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Yonghong Song <yhs@fb.com>
Cc: Jiri Olsa <jolsa@kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
	Andrii Nakryiko <andriin@fb.com>, Martin Lau <kafai@fb.com>,
	Jakub Kicinski <jakub.kicinski@netronome.com>,
	David Miller <davem@redhat.com>
Subject: Re: [PATCH 1/5] bpf: Allow non struct type for btf ctx access
Date: Tue, 7 Jan 2020 13:13:19 +0100	[thread overview]
Message-ID: <20200107121319.GG290055@krava> (raw)
In-Reply-To: <d7b8cecf-d28a-1f4d-eb2b-eb8a601b9914@fb.com>

On Mon, Jan 06, 2020 at 09:36:17PM +0000, Yonghong Song wrote:
> 
> 
> On 12/29/19 6:37 AM, Jiri Olsa wrote:
> > I'm not sure why the restriction was added,
> > but I can't access pointers to POD types like
> > const char * when probing vfs_read function.
> > 
> > Removing the check and allow non struct type
> > access in context.
> > 
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >   kernel/bpf/btf.c | 6 ------
> >   1 file changed, 6 deletions(-)
> > 
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index ed2075884724..ae90f60ac1b8 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -3712,12 +3712,6 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> >   	/* skip modifiers */
> >   	while (btf_type_is_modifier(t))
> >   		t = btf_type_by_id(btf, t->type);
> > -	if (!btf_type_is_struct(t)) {
> > -		bpf_log(log,
> > -			"func '%s' arg%d type %s is not a struct\n",
> > -			tname, arg, btf_kind_str[BTF_INFO_KIND(t->info)]);
> > -		return false;
> > -	}
> 
> Hi, Jiri, the RFC looks great! Especially, you also referenced this will
> give great performance boost for bcc scripts.
> 
> Could you provide more context on why the above change is needed?
> The function btf_ctx_access is used to check validity of accessing
> function parameters which are wrapped inside a structure, I am wondering
> what kinds of accesses you tried to address here.

when I was transforming opensnoop.py to use this I got fail in
there when I tried to access filename arg in do_sys_open

but actualy it seems this should get recognized earlier by:

          if (btf_type_is_int(t))
                /* accessing a scalar */
                return true;

I'm not sure why it did not pass for const char*, I'll check

jirka


  reply	other threads:[~2020-01-07 12:13 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-29 14:37 [RFC 0/5] bpf: Add trampoline helpers Jiri Olsa
2019-12-29 14:37 ` [PATCH 1/5] bpf: Allow non struct type for btf ctx access Jiri Olsa
2020-01-06 21:36   ` Yonghong Song
2020-01-07 12:13     ` Jiri Olsa [this message]
2020-01-07 15:50       ` Jiri Olsa
2020-01-07 17:55         ` Yonghong Song
2020-01-07 18:28           ` Yonghong Song
2020-01-08 14:38             ` Jiri Olsa
2019-12-29 14:37 ` [PATCH 2/5] bpf: Add bpf_perf_event_output_kfunc Jiri Olsa
2020-01-06 23:27   ` Alexei Starovoitov
2020-01-07 12:25     ` Jiri Olsa
2020-01-07 22:13       ` Alexei Starovoitov
2020-01-08 10:24         ` Jiri Olsa
2019-12-29 14:37 ` [PATCH 3/5] bpf: Add bpf_get_stackid_kfunc Jiri Olsa
2019-12-29 14:37 ` [PATCH 4/5] bpf: Add bpf_get_stack_kfunc Jiri Olsa
2019-12-29 14:37 ` [PATCH 5/5] bpf: Allow to resolve bpf trampoline in unwind Jiri Olsa
2020-01-06 23:46   ` Alexei Starovoitov
2020-01-07  8:30     ` Daniel Borkmann
2020-01-07 13:15       ` Jiri Olsa
2020-01-07 19:30         ` Daniel Borkmann
2020-01-07 13:05     ` Jiri Olsa
2020-01-07 13:30       ` Björn Töpel
2020-01-13  9:43         ` Jiri Olsa
2020-01-13 12:21           ` Björn Töpel
2020-01-13 12:31             ` Björn Töpel
2020-01-13 12:37               ` Jiri Olsa
2020-02-03 19:58                 ` Jiri Olsa
2020-02-03 20:27                   ` Björn Töpel
2020-02-03 20:45                     ` Toke Høiland-Jørgensen
2020-02-04  8:18                     ` 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=20200107121319.GG290055@krava \
    --to=jolsa@redhat.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@redhat.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=jolsa@kernel.org \
    --cc=kafai@fb.com \
    --cc=netdev@vger.kernel.org \
    --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 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.