[PATCH] bpf: program printed in bpf_debug compatible format

[Elazar Leibovich]

Thanks for the quick reply.
Inline:

On Tue, Jun 26, 2018 at 7:29 PM Chen Jingpiao <chenjingpiao at gmail.com>
wrote:

> On Wed, Jun 6, 2018 at 2:35 AM Elazar Leibovich <elazarl at gmail.com> wrote:
>
> $ git apply 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch
>
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:10: trailing
> whitespace.
> int i;
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:11: trailing
> whitespace.
> int n = MIN(len, BPF_MAXINSNS);
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:12: trailing
> whitespace.
> tprintf("%hu", n); // bpf_debug requires <len>, prefix
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:13: trailing
> whitespace.
> for (i=0; i<n; i++) {
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:14: trailing
> whitespace.
> struct bpf_filter_block filter;
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:15: trailing
> whitespace.
> tfetch_mem(tcp, addr + i*sizeof(filter),
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:16: trailing
> whitespace.
> sizeof(filter), &filter);
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:17: trailing
> whitespace.
> tprintf(",%u %u %u %u", filter.code, filter.jt,
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:18: trailing
> whitespace.
> filter.jf, filter.k);
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:19: trailing
> whitespace.
> }
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:20: trailing
> whitespace.
> if (n < len)
> 0001-bpf-program-printed-in-bpf_debug-compatible-format.patch:21: trailing
> whitespace.
> tprints("...");
> fatal: 12 lines add whitespace errors.
>
> please update the patch and read guide for new contributors [1].
>

Will do. I read it, and tried to follow it IIRC I used the command
specified there, but would revise.


>
> > diff --git a/bpf_filter.c b/bpf_filter.c
> > index a102e14..97f202b 100644
> > --- a/bpf_filter.c
> > +++ b/bpf_filter.c
> > @@ -177,7 +177,18 @@ print_bpf_fprog(struct tcb *const tcp, const
> kernel_ulong_t addr,
> >  const unsigned short len, const print_bpf_filter_fn print_k)
> >  {
> >  if (abbrev(tcp)) {
> > - printaddr(addr);
> > + int i;
> > + int n = MIN(len, BPF_MAXINSNS);
> > + tprintf("%hu", n); // bpf_debug requires <len>, prefix
>
> Why you use %hu print int variable?
>

Good point. Logically it cannot be larger thhan BPF_MAXINSN, so I can
either use uint, or just printf with %d.
Using %d is preferred, am I correct?


>
> > + for (i=0; i<n; i++) {
> > + struct bpf_filter_block filter;
> > + tfetch_mem(tcp, addr + i*sizeof(filter),
> > + sizeof(filter), &filter);
> > + tprintf(",%u %u %u %u", filter.code, filter.jt,
> > + filter.jf, filter.k);
> > + }
> > + if (n < len)
> > + tprints("...");
>
> We can call the print_array to do the same thing.  Why you want to print
> in this
> raw format?
>
Because:
1. We want a raw format with minimal interpretation, like in other cases.
2. This is the only format accepted by the kernel's tools. User can easily
copy and paste it to bpf_debug, and then disassemble it, run it, or print a
C array enabling him to compile it to his own program. It is not better or
worse than any other raw format, but it enable interoperability with
kernel's current bpf tooling.
It was useful for me, when I had to debug the output of libseccomp once.

>
> >  } else {
> >  struct bpf_filter_block_data fbd = { .fn = print_k };
> >  struct bpf_filter_block filter;
> >
>
> [1] https://strace.io/wiki/NewContributorGuide
> --
> Chen Jingpiao
> --
> Strace-devel mailing list
> Strace-devel at lists.strace.io
> https://lists.strace.io/mailman/listinfo/strace-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20180626/7c5f036c/attachment.html>