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

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

[Jiayuan Chen]

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}.

It is crucial for debugging sockmap programs, especially in production
environments.

Additionally, the new header file has to be added to bpf_trace.h to
automatically generate tracepoints.

Test results:
$ echo "1" > /sys/kernel/tracing/events/sockmap/enable

msg/skb:
'''
sockmap_redirect: sk=000000000ec02a93, netns=4026531840, inode=318, \
family=2, protocol=6, prog_id=59, len=8192, type=msg, action=REDIRECT, \
redirect_type=ingress

sockmap_redirect: sk=00000000d5d9c931, netns=4026531840, inode=64731, \
family=2, protocol=6, prog_id=91, len=8221, type=skb, action=REDIRECT, \
redirect_type=egress

sockmap_redirect: sk=00000000106fc281, netns=4026531840, inode=64729, \
family=2, protocol=6, prog_id=94, len=8192, type=msg, action=PASS, \
redirect_type=none
'''

strparser:
'''
sockmap_strparser: sk=00000000f15fc1c8, netns=4026531840, inode=52396, \
family=2, protocol=6, prog_id=143, in_len=1000, full_len=10
'''

Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
Suggested-by: Jakub Sitnicki <jakub@cloudflare.com>
Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>

---
v3 -> v4: Resending this patch as v3 was incorrectly closed by Patchwork.
          Additionally, carrying the Reviewed-by tag.
v3: https://lore.kernel.org/all/20250414161153.14990-1-jiayuan.chen@linux.dev

v1 -> v2: Print more valuable information as suggested by the maintainer.
---
 MAINTAINERS                    |   1 +
 include/linux/bpf_trace.h      |   1 +
 include/trace/events/sockmap.h | 158 +++++++++++++++++++++++++++++++++
 net/core/skmsg.c               |   6 ++
 4 files changed, 166 insertions(+)
 create mode 100644 include/trace/events/sockmap.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 3cbf9ac0d83f..76d742b7fbd6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4429,6 +4429,7 @@ L:	netdev@vger.kernel.org
 L:	bpf@vger.kernel.org
 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..d559be0a79c5 100644
--- a/include/linux/bpf_trace.h
+++ b/include/linux/bpf_trace.h
@@ -3,5 +3,6 @@
 #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..79784e8d5866
--- /dev/null
+++ b/include/trace/events/sockmap.h
@@ -0,0 +1,158 @@
+/* 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/tracepoint.h>
+#include <linux/bpf.h>
+#include <linux/skmsg.h>
+
+#ifndef __TRACE_SOCKMAP_HELPER_ONCE_ONLY
+#define __TRACE_SOCKMAP_HELPER_ONCE_ONLY
+
+enum sockmap_direct_type {
+	SOCKMAP_REDIR_NONE	= 0,
+	SOCKMAP_REDIR_INGRESS,
+	SOCKMAP_REDIR_EGRESS,
+};
+
+enum sockmap_data_type {
+	SOCKMAP_MSG		= 0,
+	SOCKMAP_SKB,
+};
+
+#endif /* end __TRACE_SOCKMAP_HELPER_ONCE_ONLY */
+
+TRACE_DEFINE_ENUM(SOCKMAP_MSG);
+TRACE_DEFINE_ENUM(SOCKMAP_SKB);
+TRACE_DEFINE_ENUM(SOCKMAP_REDIR_NONE);
+TRACE_DEFINE_ENUM(SOCKMAP_REDIR_INGRESS);
+TRACE_DEFINE_ENUM(SOCKMAP_REDIR_EGRESS);
+
+TRACE_DEFINE_ENUM(__SK_DROP);
+TRACE_DEFINE_ENUM(__SK_PASS);
+TRACE_DEFINE_ENUM(__SK_REDIRECT);
+TRACE_DEFINE_ENUM(__SK_NONE);
+
+#define show_redirect_type(x)					\
+	__print_symbolic(x,					\
+		{ SOCKMAP_REDIR_NONE,		"none" },	\
+		{ SOCKMAP_REDIR_INGRESS,	"ingress" },	\
+		{ SOCKMAP_REDIR_EGRESS,		"egress" })
+
+#define show_act(x)						\
+	__print_symbolic(x,					\
+		{ __SK_DROP,			"DROP" },	\
+		{ __SK_PASS,			"PASS" },	\
+		{ __SK_REDIRECT,		"REDIRECT" },	\
+		{ __SK_NONE,			"NONE" })
+
+#define show_data_type(x)					\
+	__print_symbolic(x,					\
+		{ SOCKMAP_MSG,			"msg" },	\
+		{ SOCKMAP_SKB,			"skb" })
+
+#define trace_sockmap_skmsg_redirect(sk, prog, msg, act)	\
+	trace_sockmap_redirect((sk), SOCKMAP_MSG, (prog),	\
+			       (msg)->sg.size, (act),		\
+			       sk_msg_to_ingress(msg))
+
+#define trace_sockmap_skb_redirect(sk, prog, skb, act)		\
+	trace_sockmap_redirect((sk), SOCKMAP_SKB, (prog),	\
+			       (skb)->len, (act),		\
+			       skb_bpf_ingress(skb))
+
+#define trace_sockmap_skb_strp_parse(sk, prog, skb, ret)	\
+	trace_sockmap_strparser((sk), (prog), (skb)->len, (ret))
+
+TRACE_EVENT(sockmap_redirect,
+
+	TP_PROTO(const struct sock *sk, enum sockmap_data_type type,
+		 const struct bpf_prog *prog, int len, int act,
+		 bool ingress),
+
+	TP_ARGS(sk, type, prog, len, act, ingress),
+
+	TP_STRUCT__entry(
+		__field(const void *, sk)
+		__field(unsigned long, ino)
+		__field(unsigned int, netns_ino)
+		__field(__u16, family)
+		__field(__u16, protocol)
+		__field(int, prog_id)
+		__field(int, len)
+		__field(int, act)
+		__field(enum sockmap_data_type, type)
+		__field(enum sockmap_direct_type, redir)
+	),
+
+	TP_fast_assign(
+		/* 'redir' is undefined if action is not REDIRECT */
+		enum sockmap_direct_type redir = SOCKMAP_REDIR_NONE;
+
+		if (act == __SK_REDIRECT) {
+			if (ingress)
+				redir = SOCKMAP_REDIR_INGRESS;
+			else
+				redir = SOCKMAP_REDIR_EGRESS;
+		}
+		__entry->sk		= sk;
+		__entry->ino		= sock_i_ino((struct sock *)sk);
+		__entry->netns_ino	= sock_net(sk)->ns.inum;
+		__entry->type		= type;
+		__entry->family		= sk->sk_family;
+		__entry->protocol	= sk->sk_protocol;
+		__entry->prog_id	= prog->aux->id;
+		__entry->len		= len;
+		__entry->act		= act;
+		__entry->redir		= redir;
+	),
+
+	TP_printk("sk=%p, netns=%u, inode=%lu, family=%u, protocol=%u,"
+		  " prog_id=%d, len=%d, type=%s, action=%s, redirect_type=%s",
+		  __entry->sk, __entry->netns_ino, __entry->ino,
+		  __entry->family, __entry->protocol, __entry->prog_id,
+		  __entry->len, show_data_type(__entry->type),
+		  show_act(__entry->act), show_redirect_type(__entry->redir))
+);
+
+TRACE_EVENT(sockmap_strparser,
+
+	TP_PROTO(const struct sock *sk, const struct bpf_prog *prog,
+		 int in_len, int full_len),
+
+	TP_ARGS(sk, prog, in_len, full_len),
+
+	TP_STRUCT__entry(
+		__field(const void *, sk)
+		__field(unsigned long, ino)
+		__field(unsigned int, netns_ino)
+		__field(__u16, family)
+		__field(__u16, protocol)
+		__field(int, prog_id)
+		__field(int, in_len)
+		__field(int, full_len)
+	),
+
+	TP_fast_assign(
+		__entry->sk		= sk;
+		__entry->ino		= sock_i_ino((struct sock *)sk);
+		__entry->netns_ino	= sock_net(sk)->ns.inum;
+		__entry->family		= sk->sk_family;
+		__entry->protocol	= sk->sk_protocol;
+		__entry->prog_id	= prog->aux->id;
+		__entry->in_len		= in_len;
+		__entry->full_len	= full_len;
+	),
+
+	TP_printk("sk=%p, netns=%u, inode=%lu, family=%u, protocol=%u,"
+		  " prog_id=%d, in_len=%d, full_len=%d",
+		  __entry->sk, __entry->netns_ino, __entry->ino,
+		  __entry->family, __entry->protocol, __entry->prog_id,
+		  __entry->in_len, __entry->full_len)
+);
+#endif /* _TRACE_SOCKMAP_H */
+
+#include <trace/define_trace.h>
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 276934673066..517596efafa8 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -9,6 +9,7 @@
 #include <net/tcp.h>
 #include <net/tls.h>
 #include <trace/events/sock.h>
+#include <trace/events/sockmap.h>
 
 static bool sk_msg_try_coalesce_ok(struct sk_msg *msg, int elem_first_coalesce)
 {
@@ -910,6 +911,7 @@ int sk_psock_msg_verdict(struct sock *sk, struct sk_psock *psock,
 		sock_hold(psock->sk_redir);
 	}
 out:
+	trace_sockmap_skmsg_redirect(sk, prog, msg, ret);
 	rcu_read_unlock();
 	return ret;
 }
@@ -981,6 +983,7 @@ int sk_psock_tls_strp_read(struct sk_psock *psock, struct sk_buff *skb)
 		ret = bpf_prog_run_pin_on_cpu(prog, skb);
 		ret = sk_psock_map_verd(ret, skb_bpf_redirect_fetch(skb));
 		skb->sk = NULL;
+		trace_sockmap_skb_redirect(psock->sk, prog, skb, ret);
 	}
 	sk_psock_tls_verdict_apply(skb, psock, ret);
 	rcu_read_unlock();
@@ -1090,6 +1093,7 @@ static void sk_psock_strp_read(struct strparser *strp, struct sk_buff *skb)
 		skb_bpf_set_strparser(skb);
 		ret = sk_psock_map_verd(ret, skb_bpf_redirect_fetch(skb));
 		skb->sk = NULL;
+		trace_sockmap_skb_redirect(sk, prog, skb, ret);
 	}
 	sk_psock_verdict_apply(psock, skb, ret);
 out:
@@ -1113,6 +1117,7 @@ static int sk_psock_strp_parse(struct strparser *strp, struct sk_buff *skb)
 		skb->sk = psock->sk;
 		ret = bpf_prog_run_pin_on_cpu(prog, skb);
 		skb->sk = NULL;
+		trace_sockmap_skb_strp_parse(psock->sk, prog, skb, ret);
 	}
 	rcu_read_unlock();
 	return ret;
@@ -1217,6 +1222,7 @@ static int sk_psock_verdict_recv(struct sock *sk, struct sk_buff *skb)
 		skb_bpf_redirect_clear(skb);
 		ret = bpf_prog_run_pin_on_cpu(prog, skb);
 		ret = sk_psock_map_verd(ret, skb_bpf_redirect_fetch(skb));
+		trace_sockmap_skb_redirect(psock->sk, prog, skb, ret);
 	}
 	ret = sk_psock_verdict_apply(psock, skb, ret);
 	if (ret < 0)
-- 
2.47.1

[RESEND PATCH bpf-next v4 2/2] bpf: Move the BPF net tracepoint definitions to net directory

[Jiayuan Chen]

This commit relocates the BPF tracepoint definitions for XDP and sockmap
from the kernel directory to net/bpf.

This ensures that these tracepoints are controlled by the CONFIG_NET,
avoiding unnecessary function definitions when the CONFIG_NET is disabled.
Additionally, it prevents build failures caused by the use of net module
functions when CONFIG_NET is not enabled.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 kernel/bpf/core.c       | 7 -------
 net/bpf/Makefile        | 1 +
 net/bpf/bpf_net_trace.c | 8 ++++++++
 3 files changed, 9 insertions(+), 7 deletions(-)
 create mode 100644 net/bpf/bpf_net_trace.c

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index a3e571688421..18b6e157362b 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -3185,10 +3185,3 @@ late_initcall(bpf_global_ma_init);
 
 DEFINE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
 EXPORT_SYMBOL(bpf_stats_enabled_key);
-
-/* All definitions of tracepoints related to BPF. */
-#define CREATE_TRACE_POINTS
-#include <linux/bpf_trace.h>
-
-EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_exception);
-EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_bulk_tx);
diff --git a/net/bpf/Makefile b/net/bpf/Makefile
index 1ebe270bde23..e95453053159 100644
--- a/net/bpf/Makefile
+++ b/net/bpf/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_BPF_SYSCALL)	:= test_run.o
+obj-$(CONFIG_BPF_SYSCALL)	+= bpf_net_trace.o
 ifeq ($(CONFIG_BPF_JIT),y)
 obj-$(CONFIG_BPF_SYSCALL)	+= bpf_dummy_struct_ops.o
 endif
diff --git a/net/bpf/bpf_net_trace.c b/net/bpf/bpf_net_trace.c
new file mode 100644
index 000000000000..e7c0537dbffd
--- /dev/null
+++ b/net/bpf/bpf_net_trace.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/* All definitions of net tracepoints related to BPF. */
+#define CREATE_TRACE_POINTS
+#include <linux/bpf_trace.h>
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_exception);
+EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_bulk_tx);
-- 
2.47.1

Re: [RESEND PATCH bpf-next v4 2/2] bpf: Move the BPF net tracepoint definitions to net directory

[kernel test robot]

Hi Jiayuan,

kernel test robot noticed the following build errors:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Jiayuan-Chen/bpf-Move-the-BPF-net-tracepoint-definitions-to-net-directory/20250506-150437
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20250506025131.136929-2-jiayuan.chen%40linux.dev
patch subject: [RESEND PATCH bpf-next v4 2/2] bpf: Move the BPF net tracepoint definitions to net directory
config: arc-defconfig (https://download.01.org/0day-ci/archive/20250508/202505081003.T2Tv4puD-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250508/202505081003.T2Tv4puD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505081003.T2Tv4puD-lkp@intel.com/

All errors (new ones prefixed by >>):

   arc-linux-ld: net/core/dev.o: in function `netif_receive_generic_xdp':
>> dev.c:(.text+0xb3b4): undefined reference to `__tracepoint_xdp_exception'
>> arc-linux-ld: dev.c:(.text+0xb3b4): undefined reference to `__tracepoint_xdp_exception'
>> arc-linux-ld: dev.c:(.text+0xb3e8): undefined reference to `__traceiter_xdp_exception'
>> arc-linux-ld: dev.c:(.text+0xb3e8): undefined reference to `__traceiter_xdp_exception'
   arc-linux-ld: net/core/dev.o: in function `generic_xdp_tx':
   dev.c:(.text+0xb488): undefined reference to `__tracepoint_xdp_exception'
   arc-linux-ld: dev.c:(.text+0xb488): undefined reference to `__tracepoint_xdp_exception'
   arc-linux-ld: dev.c:(.text+0xb4c4): undefined reference to `__tracepoint_xdp_exception'
   arc-linux-ld: dev.c:(.text+0xb4c4): undefined reference to `__tracepoint_xdp_exception'
   arc-linux-ld: dev.c:(.text+0xb594): undefined reference to `__traceiter_xdp_exception'
   arc-linux-ld: dev.c:(.text+0xb594): undefined reference to `__traceiter_xdp_exception'
   arc-linux-ld: net/core/filter.o: in function `__do_trace_xdp_redirect_err':
>> filter.c:(.text+0x6f88): undefined reference to `__traceiter_xdp_redirect_err'
>> arc-linux-ld: filter.c:(.text+0x6f88): undefined reference to `__traceiter_xdp_redirect_err'
   arc-linux-ld: net/core/filter.o: in function `xdp_do_redirect_frame':
>> filter.c:(.text+0xc45e): undefined reference to `__tracepoint_xdp_redirect_err'
>> arc-linux-ld: filter.c:(.text+0xc45e): undefined reference to `__tracepoint_xdp_redirect_err'
>> arc-linux-ld: filter.c:(.text+0xc490): undefined reference to `__tracepoint_xdp_redirect'
>> arc-linux-ld: filter.c:(.text+0xc490): undefined reference to `__tracepoint_xdp_redirect'
>> arc-linux-ld: filter.c:(.text+0xc500): undefined reference to `__traceiter_xdp_redirect'
>> arc-linux-ld: filter.c:(.text+0xc500): undefined reference to `__traceiter_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xc55a): undefined reference to `__traceiter_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xc55a): undefined reference to `__traceiter_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xc594): undefined reference to `__tracepoint_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xc594): undefined reference to `__tracepoint_xdp_redirect_err'
   arc-linux-ld: net/core/filter.o: in function `xdp_do_redirect':
   filter.c:(.text+0xc682): undefined reference to `__tracepoint_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xc682): undefined reference to `__tracepoint_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xc6ba): undefined reference to `__tracepoint_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xc6ba): undefined reference to `__tracepoint_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xc734): undefined reference to `__traceiter_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xc734): undefined reference to `__traceiter_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xc7d6): undefined reference to `__traceiter_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xc7d6): undefined reference to `__traceiter_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xc810): undefined reference to `__tracepoint_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xc810): undefined reference to `__tracepoint_xdp_redirect_err'
   arc-linux-ld: net/core/filter.o: in function `xdp_do_generic_redirect':
   filter.c:(.text+0xd326): undefined reference to `__tracepoint_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xd326): undefined reference to `__tracepoint_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xd3a4): undefined reference to `__tracepoint_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xd3a4): undefined reference to `__tracepoint_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xd3ea): undefined reference to `__tracepoint_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xd3ea): undefined reference to `__tracepoint_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xd454): undefined reference to `__traceiter_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xd454): undefined reference to `__traceiter_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xd4b2): undefined reference to `__traceiter_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xd4b2): undefined reference to `__traceiter_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xd4ec): undefined reference to `__tracepoint_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xd4ec): undefined reference to `__tracepoint_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xd532): undefined reference to `__traceiter_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xd532): undefined reference to `__traceiter_xdp_redirect_err'
   arc-linux-ld: filter.c:(.text+0xd594): undefined reference to `__traceiter_xdp_redirect'
   arc-linux-ld: filter.c:(.text+0xd594): undefined reference to `__traceiter_xdp_redirect'
   arc-linux-ld: net/core/xdp.o: in function `xdp_rxq_info_reg_mem_model':
>> xdp.c:(.text+0x58e): undefined reference to `__tracepoint_mem_connect'
>> arc-linux-ld: xdp.c:(.text+0x58e): undefined reference to `__tracepoint_mem_connect'
>> arc-linux-ld: xdp.c:(.text+0x5d2): undefined reference to `__traceiter_mem_connect'
>> arc-linux-ld: xdp.c:(.text+0x5d2): undefined reference to `__traceiter_mem_connect'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Jiayuan Chen]

I am resending this patch as I have discovered that the previous v3 patch which has already
been reviewed was incorrectly marked as 'v2 superseded':
https://github.com/kernel-patches/bpf/pull/8763
(It seems that Patchwork closed it incorrectly?)

The original v3 thread can be found here:
https://lore.kernel.org/all/20250414161153.14990-1-jiayuan.chen@linux.dev

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

[Martin KaFai Lau]

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 ?

> 
> It is crucial for debugging sockmap programs, especially in production
> environments.
> 
> Additionally, the new header file has to be added to bpf_trace.h to
> automatically generate tracepoints.
> 
> Test results:
> $ echo "1" > /sys/kernel/tracing/events/sockmap/enable
> 
> msg/skb:
> '''
> sockmap_redirect: sk=000000000ec02a93, netns=4026531840, inode=318, \
> family=2, protocol=6, prog_id=59, len=8192, type=msg, action=REDIRECT, \
> redirect_type=ingress
> 
> sockmap_redirect: sk=00000000d5d9c931, netns=4026531840, inode=64731, \
> family=2, protocol=6, prog_id=91, len=8221, type=skb, action=REDIRECT, \
> redirect_type=egress
> 
> sockmap_redirect: sk=00000000106fc281, netns=4026531840, inode=64729, \
> family=2, protocol=6, prog_id=94, len=8192, type=msg, action=PASS, \
> redirect_type=none
> '''
> 
> strparser:
> '''
> sockmap_strparser: sk=00000000f15fc1c8, netns=4026531840, inode=52396, \
> family=2, protocol=6, prog_id=143, in_len=1000, full_len=10
> '''
> 
> Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
> Suggested-by: Jakub Sitnicki <jakub@cloudflare.com>
> Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> 
> ---
> v3 -> v4: Resending this patch as v3 was incorrectly closed by Patchwork.
>            Additionally, carrying the Reviewed-by tag.

John and JakubS, please take a look.

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

[Jiayuan Chen]

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.

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.

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.

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

[Alexei Starovoitov]

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.
>
> 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.
>
> 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.

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.

pw-bot: cr

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

[John Fastabend]

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