[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200411231719.4nybod6ku524eawv@ast-mbp>
Date: Sat, 11 Apr 2020 16:17:19 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Yonghong Song <yhs@...com>
Cc: Andrii Nakryiko <andrii.nakryiko@...il.com>,
Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
Martin KaFai Lau <kafai@...com>,
Networking <netdev@...r.kernel.org>,
Alexei Starovoitov <ast@...com>,
Daniel Borkmann <daniel@...earbox.net>,
Kernel Team <kernel-team@...com>
Subject: Re: [RFC PATCH bpf-next 05/16] bpf: create file or anonymous dumpers
On Fri, Apr 10, 2020 at 05:23:30PM -0700, Yonghong Song wrote:
> >
> > So it seems like few things would be useful:
> >
> > 1. end flag for post-aggregation and/or footer printing (seq_num == 0
> > is providing similar means for start flag).
>
> the end flag is a problem. We could say hijack next or stop so we
> can detect the end, but passing a NULL pointer as the object
> to the bpf program may be problematic without verifier enforcement
> as it may cause a lot of exceptions... Although all these exception
> will be silenced by bpf infra, but still not sure whether this
> is acceptable or not.
I don't like passing NULL there just to indicate something to a program.
It's not too horrible to support from verifier side, but NULL is only
one such flag. What does it suppose to indicate? That dumper prog
is just starting? or ending? Let's pass (void*)1, and (void *)2 ?
I'm not a fan of such inband signaling.
imo it's cleaner and simpler when that object pointer is always valid.
> > 2. Some sort of "session id", so that bpfdumper can maintain
> > per-session intermediate state. Plus with this it would be possible to
> > detect restarts (if there is some state for the same session and
> > seq_num == 0, this is restart).
>
> I guess we can do this.
beyond seq_num passing session_id is a good idea. Though I don't quite see
the use case where you'd need bpfdumper prog to be stateful, but doesn't hurt.
Powered by blists - more mailing lists