Subject: Re: [PATCH bpf-next v1] bpf, sockmap: Introduce tracing capability for sockmap

From: "Jiayuan Chen" <[email-masked]>
Date: Thu, 10 Apr 2025 14:27:17 +0000

April 10, 2025 at 17:14, "Jakub Sitnicki" <[email-masked]> wrote:

> 
> On Wed, Apr 09, 2025 at 06:29 PM +08, Jiayuan Chen wrote:
> 
> > 
> > Sockmap has the same high-performance forwarding capability as XDP, but
> > 
> >  operates at Layer 7.
> > 
> >  Introduce tracing capability for sockmap, similar to XDP, to trace the
> > 
> >  execution results of BPF programs without modifying the programs
> > 
> >  themselves, similar to the existing trace_xdp_redirect{_map}.
> > 
> >  It is crucial for debugging BPF programs, especially in production
> > 
> >  environments.
> > 
> >  Additionally, a header file was added to bpf_trace.h to automatically
> > 
> >  generate tracepoints.
> > 
> >  Test results:
> > 
> >  $ echo "1" > /sys/kernel/tracing/events/sockmap/enable
> > 
> >  skb:
> > 
> >  sockmap_redirect: sk=00000000d3266a8d, type=skb, family=2, protocol=6, \
> > 
> >  prog_id=73, length=256, action=PASS
> > 
> >  msg:
> > 
> >  sockmap_redirect: sk=00000000528c7614, type=msg, family=2, protocol=6, \
> > 
> >  prog_id=185, length=5, action=REDIRECT
> > 
> >  tls:
> > 
> >  sockmap_redirect: sk=00000000d04d2224, type=skb, family=2, protocol=6, \
> > 
> >  prog_id=143, length=35, action=PASS
> > 
> >  strparser:
> > 
> >  sockmap_skb_strp_parse: sk=00000000ecab0b30, family=2, protocol=6, \
> > 
> >  prog_id=170, size=5
> > 
> >  Signed-off-by: Jiayuan Chen <[email-masked]>
> > 
> >  ---
> > 
> >  MAINTAINERS | 1 +
> > 
> >  include/linux/bpf_trace.h | 2 +-
> > 
> >  include/trace/events/sockmap.h | 89 ++++++++++++++++++++++++++++++++++
> > 
> >  net/core/skmsg.c | 6 +++
> > 
> >  4 files changed, 97 insertions(+), 1 deletion(-)
> > 
> >  create mode 100644 include/trace/events/sockmap.h
> > 
> >  diff --git a/MAINTAINERS b/MAINTAINERS
> > 
> >  index a7a1d121a83e..578e16d86853 100644
> > 
> >  --- a/MAINTAINERS
> > 
> >  +++ b/MAINTAINERS
> > 
> >  @@ -4420,6 +4420,7 @@ L: [email-masked]
> > 
> >  L: [email-masked]
> > 
> >  S: Maintained
> > 
> >  F: include/linux/skmsg.h
> > 
> >  +F: include/trace/events/sockmap.h
> > 
> >  F: net/core/skmsg.c
> > 
> >  F: net/core/sock_map.c
> > 
> >  F: net/ipv4/tcp_bpf.c
> > 
> >  diff --git a/include/linux/bpf_trace.h b/include/linux/bpf_trace.h
> > 
> >  index ddf896abcfb6..896346fb2b46 100644
> > 
> >  --- a/include/linux/bpf_trace.h
> > 
> >  +++ b/include/linux/bpf_trace.h
> > 
> >  @@ -3,5 +3,5 @@
> > 
> >  #define __LINUX_BPF_TRACE_H__
> > 
> >  
> > 
> >  #include <trace/events/xdp.h>
> > 
> >  -
> > 
> >  +#include <trace/events/sockmap.h>
> > 
> >  #endif /* __LINUX_BPF_TRACE_H__ */
> > 
> >  diff --git a/include/trace/events/sockmap.h b/include/trace/events/sockmap.h
> > 
> >  new file mode 100644
> > 
> >  index 000000000000..2a69b011e88f
> > 
> >  --- /dev/null
> > 
> >  +++ b/include/trace/events/sockmap.h
> > 
> >  @@ -0,0 +1,89 @@
> > 
> >  +/* SPDX-License-Identifier: GPL-2.0 */
> > 
> >  +#undef TRACE_SYSTEM
> > 
> >  +#define TRACE_SYSTEM sockmap
> > 
> >  +
> > 
> >  +#if !defined(_TRACE_SOCKMAP_H) || defined(TRACE_HEADER_MULTI_READ)
> > 
> >  +#define _TRACE_SOCKMAP_H
> > 
> >  +
> > 
> >  +#include <linux/filter.h>
> > 
> >  +#include <linux/tracepoint.h>
> > 
> >  +#include <linux/bpf.h>
> > 
> >  +#include <linux/skmsg.h>
> > 
> >  +
> > 
> >  +TRACE_DEFINE_ENUM(__SK_DROP);
> > 
> >  +TRACE_DEFINE_ENUM(__SK_PASS);
> > 
> >  +TRACE_DEFINE_ENUM(__SK_REDIRECT);
> > 
> >  +TRACE_DEFINE_ENUM(__SK_NONE);
> > 
> >  +
> > 
> >  +#define show_act(x) \
> > 
> >  + __print_symbolic(x, \
> > 
> >  + { __SK_DROP, "DROP" }, \
> > 
> >  + { __SK_PASS, "PASS" }, \
> > 
> >  + { __SK_REDIRECT, "REDIRECT" }, \
> > 
> >  + { __SK_NONE, "NONE" })
> > 
> >  +
> > 
> >  +#define trace_sockmap_skmsg_redirect(sk, prog, msg, act) \
> > 
> >  + trace_sockmap_redirect((sk), "msg", (prog), (msg)->sg.size, (act))
> > 
> >  +
> > 
> >  +#define trace_sockmap_skb_redirect(sk, prog, skb, act) \
> > 
> >  + trace_sockmap_redirect((sk), "skb", (prog), (skb)->len, (act))
> > 
> >  +
> > 
> >  +TRACE_EVENT(sockmap_redirect,
> > 
> >  + TP_PROTO(const struct sock *sk, const char *type,
> > 
> >  + const struct bpf_prog *prog, int length, int act),
> > 
> >  + TP_ARGS(sk, type, prog, length, act),
> > 
> >  +
> > 
> >  + TP_STRUCT__entry(
> > 
> >  + __field(const void *, sk)
> > 
> >  + __field(const char *, type)
> > 
> >  + __field(__u16, family)
> > 
> >  + __field(__u16, protocol)
> > 
> >  + __field(int, prog_id)
> > 
> >  + __field(int, length)
> > 
> >  + __field(int, act)
> > 
> >  + ),
> > 
> >  +
> > 
> >  + TP_fast_assign(
> > 
> >  + __entry->sk = sk;
> > 
> >  + __entry->type = type;
> > 
> >  + __entry->family = sk->sk_family;
> > 
> >  + __entry->protocol = sk->sk_protocol;
> > 
> >  + __entry->prog_id = prog->aux->id;
> > 
> >  + __entry->length = length;
> > 
> >  + __entry->act = act;
> > 
> >  + ),
> > 
> >  +
> > 
> >  + TP_printk("sk=%p, type=%s, family=%d, protocol=%d, prog_id=%d, length=%d, action=%s",
> > 
> >  + __entry->sk, __entry->type, __entry->family, __entry->protocol,
> > 
> >  + __entry->prog_id, __entry->length,
> > 
> >  + show_act(__entry->act))
> > 
> 
> sk address is useful if you're going to attach a bpf program to the
> tracepoint. Not so much if you're printing the recorded trace.
> 
> I'd print the netns and the socket inode instead, or in addition to.
> These can be cross-referenced against `lsns` and `ss` output.

Good suggestions. I will print all of this.
sk address helps us track connection more easily.

Thanks~