On Mon, May 12, 2025 at 1:51 PM Kumar Kartikeya Dwivedi
<memxor@xxxxxxxxx> wrote:
>
> On Fri, 9 May 2025 at 17:33, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote:
> >
> > On Fri, May 9, 2025 at 11:48 AM Alexei Starovoitov
> > <alexei.starovoitov@xxxxxxxxx> wrote:
> > >
> > > On Fri, May 9, 2025 at 11:31 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
> > > >
> > > > On Fri, 2025-05-09 at 10:31 -0700, Alexei Starovoitov wrote:
> > > >
> > > > [...]
> > > >
> > > > > How about we extend BPF_OBJ_GET_INFO_BY_FD to return stream data?
> > > > > Or add a new command ?
> > > >
> > > > You mean like this:
> > > >
> > > > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> > > > index 71d5ac83cf5d..25ac28d11af5 100644
> > > > --- a/tools/include/uapi/linux/bpf.h
> > > > +++ b/tools/include/uapi/linux/bpf.h
> > > > @@ -6610,6 +6610,10 @@ struct bpf_prog_info {
> > > > __u32 verified_insns;
> > > > __u32 attach_btf_obj_id;
> > > > __u32 attach_btf_id;
> > > > + __u32 stdout_len; /* length of the buffer passed in 'stdout' */
> > > > + __u32 stderr_len; /* length of the buffer passed in 'stderr' */
> > > > + __aligned_u64 stdout;
> > > > + __aligned_u64 stderr;
> > > > } __attribute__((aligned(8)));
> > > >
> > > > And return -EAGAIN if there is more data to read?
> > >
> > > Exactly.
> > > The only concern that all other __aligned_u64 will probably be zero,
> > > but kernel will still fill in all other non-pointer fields and
> > > that information will be re-populated again and again,
> > > so new command might be cleaner.
> >
> > +1, but I'd allow reading only either stdout or stderr per each
> > command invocation to keep things simple API-wise (e.g., which stream
> > got EAGAIN, if you asked for both?) I haven't read carefully enough to
> > know if we'll allow creating custom streams beyond stderr/stdout, but
> > this would scale to that more naturally as well.
> >
>
> What's your preference/concerns re: pseudo files in sysfs?
> That does seem like it would be simplest for someone using this
> (read() on a file vs special BPF syscall).
sysfs is abi.
If we start creating directories:
/sys/kernel/bpf/<prog_id>/stdout
it will be permanent.
Though I'd like to see it, I feel we're not quite ready
to cross that bridge.
Let's add a new sys_bpf command for now,
some trivial helper function in libbpf,
and corresponding bpftool support.
