Re: [RESEND PATCH bpf-next v4 1/2] bpf, sockmap: Introduce tracing capability for sockmap

[John Fastabend <john.fastabend@gmail.com>]

On 2025-05-06 20:43:43, Alexei Starovoitov wrote:
> On Tue, May 6, 2025 at 8:37 PM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> >
> > May 7, 2025 at 04:24, "Martin KaFai Lau" <martin.lau@linux.dev> wrote:
> >
> > >
> > > On 5/5/25 7:51 PM, Jiayuan Chen wrote:
> > >
> > > >
> > > > Sockmap has the same high-performance forwarding capability as XDP, but
> > > >
> > > >  operates at Layer 7.
> > > >
> > > >  Introduce tracing capability for sockmap, to trace the execution results
> > > >
> > > >  of BPF programs without modifying the programs themselves, similar to
> > > >
> > > >  the existing trace_xdp_redirect{_map}.
> > > >
> > >
> > > There were advancements in bpf tracing since the trace_xdp_xxx additions.
> > >
> > > Have you considered the fexit bpf prog and why it is not sufficient ?
> > >
> >
> > 1.This patchset prints a large amount of information (e.g. inode ID, etc.),
> > some of which require kernel-internal helpers to access. These helpers are
> > not currently available as kfuncs, making it difficult to implement
> > equivalent functionality with fentry/fexit.

If the data is useful and can't be read normally having kfuncs/etc to
get the data makes a lot of sense to me. Then it would be useful for
everyone presumably.

> >
> > 2. skb->_sk_redir implicitly stores both a redir action and the socket address
> > in a single field. Decoding this structure in fentry/fexit would require
> > duplicating kernel-internal logic in BPF programs. This creates maintenance
> > risks, as any future changes to the kernel's internal representation would
> > necessitate corresponding updates to the BPF programs.

If its needed we could build BPF code somewhere that decoded these
correctly for all kernels.

> >
> > 3. Similar to the debate between using built-in tracepoints vs kprobes/fentry,
> > each approach has its tradeoffs. The key advantage of a built-in tracepoint is
> > seamless integration with existing tools like perf and bpftrace, which natively
> > support tracepoint-based tracing. For example, simply executing
> > 'perf trace -e 'sockmap:*' ./producer' could provide sufficient visibility
> > without custom BPF programs.

We could likely teach bpftrace a new syntax if we care?

bpftrace -e 'skmsg:sendmsg: { @[socket, pid] = count_bytes(); }'

might be interesting.


> Similar to Martin I don't buy these excuses.
> For your own debugging you can write bpftrace prog that will
> print exact same stats and numbers without adding any kernel code.
> 
> We add tracepoints when they're in the path that is hard to get to
> with tracing tools. Like functions are partially inlined.
> Here it's not the case.
> You want to add a tracepoint right after your own bpf prog
> finished. All these debugging could have been part of your
> skmsg program.

I tend to agree. We've on our side found it extremely useful to have
DEBUG infra in our BPF codes and easy ways to turn it off/on. 
If this DEBUG is in your BPF program and you have the pretty printers
to read it yuo can get lots of specifics about your paticular program
logic that can't be put in the tracepoint.

Thanks,
John

> 
> pw-bot: cr