Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Alan Maguire <alan.maguire@oracle.com>
Cc: ast@kernel.org, daniel@iogearbox.net, andriin@fb.com, yhs@fb.com,
	linux@rasmusvillemoes.dk, andriy.shevchenko@linux.intel.com,
	pmladek@suse.com, kafai@fb.com, songliubraving@fb.com,
	john.fastabend@gmail.com, kpsingh@chromium.org, shuah@kernel.org,
	rdna@fb.com, scott.branden@broadcom.com, quentin@isovalent.com,
	cneirabustos@gmail.com, jakub@cloudflare.com, mingo@redhat.com,
	rostedt@goodmis.org, bpf@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [RFC PATCH bpf-next 2/4] bpf: make BTF show support generic, apply to seq files/bpf_trace_printk
Date: Thu, 20 Aug 2020 15:16:36 -0700	[thread overview]
Message-ID: <20200820221636.zkvxx64n3ij72ud5@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <alpine.LRH.2.21.2008180945380.3461@localhost>

On Tue, Aug 18, 2020 at 10:12:05AM +0100, Alan Maguire wrote:
> 
> Fair enough. I'm thinking a helper like
> 
> long bpf_btf_snprintf(char *str, u32 str_size, struct btf_ptr *ptr,
> 		      u32 ptr_size, u64 flags);
> 
> Then the user can choose perf event or ringbuf interfaces
> to share the results with userspace.

makes sense.

> 
> > If the user happen to use bpf_trace_printk("%s", buf);
> > after that to print that string buffer to trace_pipe that's user's choice.
> > I can see such use case when program author wants to debug
> > their bpf program. That's fine. But for kernel debugging, on demand and
> > "always on" logging and tracing the documentation should point
> > to sustainable interfaces that don't interfere with each other,
> > can be run in parallel by multiple users, etc.
> > 
> 
> The problem with bpf_trace_printk() under this approach is
> that the string size for %s arguments is very limited;
> bpf_trace_printk() restricts these to 64 bytes in size.
> Looks like bpf_seq_printf() restricts a %s string to 128
> bytes also.  We could add an additional helper for the 
> bpf_seq case which calls bpf_seq_printf() for each component
> in the object, i.e.
> 
> long bpf_seq_btf_printf(struct seq_file *m, struct btf_ptr *ptr,
> 			u32 ptr_size, u64 flags);

yeah. this one is needed as well.
Please double check that it works out of bpf iterator too.
Especially the case when output is not power of 2.
bpf_iter.c keeps page sized buffer and will restart
iteration on overflow.
Could you please modify progs/bpf_iter_task.c to do
bpf_seq_btf_printf(seq, {task, }, 0);
and check that the output doesn't contain repeated/garbled lines.

Also I'm not sure why you see 64 limit of bpf_trace_printk as a blocker.
We could do:
if (in_nmi())
  use 64 byte on stack
else
  spin_lock_irqsave and use 1k static buffer.
and/or we can introduce bpf_trace_puts(const char *buf, u32 size_of_buf);
with
        .arg1_type      = ARG_PTR_TO_MEM,
        .arg2_type      = ARG_CONST_SIZE,
add null termination check and call trace_bpf_trace_printk() on that
buffer. That will avoid a bunch of copies.
But that still doesn't make bpf_trace_puts() a good interface for
dumping stuff to user space. It's still debug only feature.

  reply	other threads:[~2020-08-20 22:16 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-06 14:42 [RFC PATCH bpf-next 0/4] bpf: add bpf-based bpf_trace_printk()-like support Alan Maguire
2020-08-06 14:42 ` [RFC PATCH bpf-next 1/4] bpf: provide function to get vmlinux BTF information Alan Maguire
2020-08-06 14:42 ` [RFC PATCH bpf-next 2/4] bpf: make BTF show support generic, apply to seq files/bpf_trace_printk Alan Maguire
2020-08-13  1:46   ` Alexei Starovoitov
2020-08-14 13:06     ` Alan Maguire
2020-08-14 17:01       ` Alexei Starovoitov
2020-08-18  9:12         ` Alan Maguire
2020-08-20 22:16           ` Alexei Starovoitov [this message]
2020-08-06 14:42 ` [RFC PATCH bpf-next 3/4] bpf: add bpf_trace_btf helper Alan Maguire
2020-08-20  6:36   ` Andrii Nakryiko
2020-08-06 14:42 ` [RFC PATCH bpf-next 4/4] selftests/bpf: add bpf_trace_btf helper tests Alan Maguire

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=20200820221636.zkvxx64n3ij72ud5@ast-mbp.dhcp.thefacebook.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=alan.maguire@oracle.com \
    --cc=andriin@fb.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=cneirabustos@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=jakub@cloudflare.com \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mingo@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=quentin@isovalent.com \
    --cc=rdna@fb.com \
    --cc=rostedt@goodmis.org \
    --cc=scott.branden@broadcom.com \
    --cc=shuah@kernel.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    --subject='Re: [RFC PATCH bpf-next 2/4] bpf: make BTF show support generic, apply to seq files/bpf_trace_printk' \
    /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

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).