[PATCH net-next 0/2] net: snmp: tracepoint support for snmp

[PATCH net-next 0/2] net: snmp: tracepoint support for snmp

[menglong8.dong]

From: Menglong Dong <imagedong@tencent.com>

snmp is the network package statistics module in kernel, and it is
useful in network issue diagnosis, such as packet drop.

However, it is hard to get the detail information about the packet.
For example, we can know that there is something wrong with the
checksum of udp packet though 'InCsumErrors' of UDP protocol in
/proc/net/snmp, but we can't figure out the ip and port of the packet
that this error is happening on.

Add tracepoint for snmp. Therefor, users can use some tools (such as
eBPF) to get the information of the exceptional packet.

In the first patch, the frame of snmp-tracepoint is created. And in
the second patch, tracepoint for udp-snmp is introduced.


Menglong Dong (2):
  net: snmp: add tracepoint support for snmp
  net: snmp: add snmp tracepoint support for udp

 include/net/udp.h           | 25 ++++++++++++++-----
 include/trace/events/snmp.h | 50 +++++++++++++++++++++++++++++++++++++
 net/core/net-traces.c       |  3 +++
 net/ipv4/udp.c              | 28 +++++++++++++--------
 4 files changed, 89 insertions(+), 17 deletions(-)
 create mode 100644 include/trace/events/snmp.h

-- 
2.27.0

[PATCH net-next 1/2] net: snmp: add tracepoint support for snmp

[menglong8.dong]

From: Menglong Dong <imagedong@tencent.com>

snmp is the network package statistics module in kernel, and it is
useful in network issue diagnosis, such as packet drop.

However, it is hard to get the detail information about the packet.
For example, we can know that there is something wrong with the
checksum of udp packet though 'InCsumErrors' of UDP protocol in
/proc/net/snmp, but we can't figure out the ip and port of the packet
that this error is happening on.

Add tracepoint for snmp. Therefor, users can use some tools (such as
eBPF) to get the information of the exceptional packet.

Signed-off-by: Menglong Dong <imagedong@tencent.com>
---
 include/trace/events/snmp.h | 45 +++++++++++++++++++++++++++++++++++++
 net/core/net-traces.c       |  1 +
 2 files changed, 46 insertions(+)
 create mode 100644 include/trace/events/snmp.h

diff --git a/include/trace/events/snmp.h b/include/trace/events/snmp.h
new file mode 100644
index 000000000000..9dbd630306dd
--- /dev/null
+++ b/include/trace/events/snmp.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM snmp
+
+#if !defined(_TRACE_SNMP_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SNMP_H
+
+#include <linux/tracepoint.h>
+#include <linux/skbuff.h>
+#include <linux/snmp.h>
+
+DECLARE_EVENT_CLASS(snmp_template,
+
+	TP_PROTO(struct sk_buff *skb, int field, int val),
+
+	TP_ARGS(skb, field, val),
+
+	TP_STRUCT__entry(
+		__field(void *, skbaddr)
+		__field(int, field)
+		__field(int, val)
+	),
+
+	TP_fast_assign(
+		__entry->skbaddr = skb;
+		__entry->field = field;
+		__entry->val = val;
+	),
+
+	TP_printk("skbaddr=%p, field=%d, val=%d", __entry->skbaddr,
+		  __entry->field, __entry->val)
+);
+
+#define DEFINE_SNMP_EVENT(proto)				\
+DEFINE_EVENT(snmp_template, snmp_##proto,			\
+	TP_PROTO(struct sk_buff *skb, int field, int val),	\
+	TP_ARGS(skb, field, val)				\
+)
+
+#define TRACE_SNMP(skb, proto, field, val) \
+	trace_snmp_##proto(skb, field, val)
+
+#endif
+
+#include <trace/define_trace.h>
diff --git a/net/core/net-traces.c b/net/core/net-traces.c
index c40cd8dd75c7..15ff40b83ca7 100644
--- a/net/core/net-traces.c
+++ b/net/core/net-traces.c
@@ -35,6 +35,7 @@
 #include <trace/events/tcp.h>
 #include <trace/events/fib.h>
 #include <trace/events/qdisc.h>
+#include <trace/events/snmp.h>
 #if IS_ENABLED(CONFIG_BRIDGE)
 #include <trace/events/bridge.h>
 EXPORT_TRACEPOINT_SYMBOL_GPL(br_fdb_add);
-- 
2.27.0

[PATCH net-next 2/2] net: snmp: add snmp tracepoint support for udp

[menglong8.dong]

From: Menglong Dong <imagedong@tencent.com>

Add snmp tracepoint support for udp. Here is the new tracepoint:
/sys/kernel/debug/tracing/events/snmp/snmp_udp/

Signed-off-by: Menglong Dong <imagedong@tencent.com>
---
 include/net/udp.h           | 25 +++++++++++++++++++------
 include/trace/events/snmp.h |  5 +++++
 net/core/net-traces.c       |  2 ++
 net/ipv4/udp.c              | 28 +++++++++++++++++-----------
 4 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/include/net/udp.h b/include/net/udp.h
index 909ecf447e0f..bf39793f2052 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -28,6 +28,7 @@
 #include <linux/seq_file.h>
 #include <linux/poll.h>
 #include <linux/indirect_call_wrapper.h>
+#include <trace/events/snmp.h>
 
 /**
  *	struct udp_skb_cb  -  UDP(-Lite) private variables
@@ -408,12 +409,24 @@ static inline int copy_linear_skb(struct sk_buff *skb, int len, int off,
 /*
  * 	SNMP statistics for UDP and UDP-Lite
  */
-#define UDP_INC_STATS(net, field, is_udplite)		      do { \
-	if (is_udplite) SNMP_INC_STATS((net)->mib.udplite_statistics, field);       \
-	else		SNMP_INC_STATS((net)->mib.udp_statistics, field);  }  while(0)
-#define __UDP_INC_STATS(net, field, is_udplite) 	      do { \
-	if (is_udplite) __SNMP_INC_STATS((net)->mib.udplite_statistics, field);         \
-	else		__SNMP_INC_STATS((net)->mib.udp_statistics, field);    }  while(0)
+#define UDP_INC_STATS(net, field, is_udplite)			do {	\
+	if (is_udplite) {						\
+		SNMP_INC_STATS((net)->mib.udplite_statistics, field);	\
+		TRACE_SNMP(skb, udp, field, 1);				\
+	} else {							\
+		SNMP_INC_STATS((net)->mib.udp_statistics, field);	\
+		TRACE_SNMP(skb, udplite, field, 1);			\
+	}								\
+} while (0)
+#define __UDP_INC_STATS(net, skb, field, is_udplite)		do {	\
+	if (is_udplite) {						\
+		__SNMP_INC_STATS((net)->mib.udplite_statistics, field);	\
+		TRACE_SNMP(skb, udp, field, 1);				\
+	} else {							\
+		__SNMP_INC_STATS((net)->mib.udp_statistics, field);	\
+		TRACE_SNMP(skb, udplite, field, 1);			\
+	}								\
+}  while (0)
 
 #define __UDP6_INC_STATS(net, field, is_udplite)	    do { \
 	if (is_udplite) __SNMP_INC_STATS((net)->mib.udplite_stats_in6, field);\
diff --git a/include/trace/events/snmp.h b/include/trace/events/snmp.h
index 9dbd630306dd..799a8b66b438 100644
--- a/include/trace/events/snmp.h
+++ b/include/trace/events/snmp.h
@@ -37,6 +37,11 @@ DEFINE_EVENT(snmp_template, snmp_##proto,			\
 	TP_ARGS(skb, field, val)				\
 )
 
+
+
+DEFINE_SNMP_EVENT(udp);
+DEFINE_SNMP_EVENT(udplite);
+
 #define TRACE_SNMP(skb, proto, field, val) \
 	trace_snmp_##proto(skb, field, val)
 
diff --git a/net/core/net-traces.c b/net/core/net-traces.c
index 15ff40b83ca7..c33a86bb2db3 100644
--- a/net/core/net-traces.c
+++ b/net/core/net-traces.c
@@ -62,3 +62,5 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(napi_poll);
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(tcp_send_reset);
 EXPORT_TRACEPOINT_SYMBOL_GPL(tcp_bad_csum);
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(snmp_udp);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 319dd7bbfe33..d5116971892f 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1648,9 +1648,11 @@ static struct sk_buff *__first_packet_length(struct sock *sk,
 
 	while ((skb = skb_peek(rcvq)) != NULL) {
 		if (udp_lib_checksum_complete(skb)) {
-			__UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS,
+			__UDP_INC_STATS(sock_net(sk), skb,
+					UDP_MIB_CSUMERRORS,
 					IS_UDPLITE(sk));
-			__UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS,
+			__UDP_INC_STATS(sock_net(sk), skb,
+					UDP_MIB_INERRORS,
 					IS_UDPLITE(sk));
 			atomic_inc(&sk->sk_drops);
 			__skb_unlink(skb, rcvq);
@@ -2143,7 +2145,7 @@ static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
 
 			ret = encap_rcv(sk, skb);
 			if (ret <= 0) {
-				__UDP_INC_STATS(sock_net(sk),
+				__UDP_INC_STATS(sock_net(sk), skb,
 						UDP_MIB_INDATAGRAMS,
 						is_udplite);
 				return -ret;
@@ -2201,9 +2203,10 @@ static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
 	return __udp_queue_rcv_skb(sk, skb);
 
 csum_error:
-	__UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
+	__UDP_INC_STATS(sock_net(sk), skb, UDP_MIB_CSUMERRORS,
+			is_udplite);
 drop:
-	__UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
+	__UDP_INC_STATS(sock_net(sk), skb, UDP_MIB_INERRORS, is_udplite);
 	atomic_inc(&sk->sk_drops);
 	kfree_skb(skb);
 	return -1;
@@ -2290,9 +2293,9 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 
 		if (unlikely(!nskb)) {
 			atomic_inc(&sk->sk_drops);
-			__UDP_INC_STATS(net, UDP_MIB_RCVBUFERRORS,
+			__UDP_INC_STATS(net, skb, UDP_MIB_RCVBUFERRORS,
 					IS_UDPLITE(sk));
-			__UDP_INC_STATS(net, UDP_MIB_INERRORS,
+			__UDP_INC_STATS(net, skb, UDP_MIB_INERRORS,
 					IS_UDPLITE(sk));
 			continue;
 		}
@@ -2311,7 +2314,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 			consume_skb(skb);
 	} else {
 		kfree_skb(skb);
-		__UDP_INC_STATS(net, UDP_MIB_IGNOREDMULTI,
+		__UDP_INC_STATS(net, skb, UDP_MIB_IGNOREDMULTI,
 				proto == IPPROTO_UDPLITE);
 	}
 	return 0;
@@ -2454,7 +2457,8 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	if (udp_lib_checksum_complete(skb))
 		goto csum_error;
 
-	__UDP_INC_STATS(net, UDP_MIB_NOPORTS, proto == IPPROTO_UDPLITE);
+	__UDP_INC_STATS(net, skb, UDP_MIB_NOPORTS,
+			proto == IPPROTO_UDPLITE);
 	icmp_send(skb, ICMP_DEST_UNREACH, ICMP_PORT_UNREACH, 0);
 
 	/*
@@ -2481,9 +2485,11 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 			    proto == IPPROTO_UDPLITE ? "Lite" : "",
 			    &saddr, ntohs(uh->source), &daddr, ntohs(uh->dest),
 			    ulen);
-	__UDP_INC_STATS(net, UDP_MIB_CSUMERRORS, proto == IPPROTO_UDPLITE);
+	__UDP_INC_STATS(net, skb, UDP_MIB_CSUMERRORS,
+			proto == IPPROTO_UDPLITE);
 drop:
-	__UDP_INC_STATS(net, UDP_MIB_INERRORS, proto == IPPROTO_UDPLITE);
+	__UDP_INC_STATS(net, skb, UDP_MIB_INERRORS,
+			proto == IPPROTO_UDPLITE);
 	kfree_skb(skb);
 	return 0;
 }
-- 
2.27.0

Re: [PATCH net-next 0/2] net: snmp: tracepoint support for snmp

[Jakub Kicinski]

On Thu, 11 Nov 2021 21:35:28 +0800 menglong8.dong@gmail.com wrote:
> From: Menglong Dong <imagedong@tencent.com>
> 
> snmp is the network package statistics module in kernel, and it is
> useful in network issue diagnosis, such as packet drop.
> 
> However, it is hard to get the detail information about the packet.
> For example, we can know that there is something wrong with the
> checksum of udp packet though 'InCsumErrors' of UDP protocol in
> /proc/net/snmp, but we can't figure out the ip and port of the packet
> that this error is happening on.
> 
> Add tracepoint for snmp. Therefor, users can use some tools (such as
> eBPF) to get the information of the exceptional packet.
> 
> In the first patch, the frame of snmp-tracepoint is created. And in
> the second patch, tracepoint for udp-snmp is introduced.

I feel like I have seen this idea before. Is this your first posting?

Would you mind including links to previous discussion if you're aware
of any?

Regardless:


# Form letter - net-next is closed

We have already sent the networking pull request for 5.16
and therefore net-next is closed for new drivers, features,
code refactoring and optimizations. We are currently accepting
bug fixes only.

Please repost when net-next reopens after 5.16-rc1 is cut.

Look out for the announcement on the mailing list or check:
http://vger.kernel.org/~davem/net-next.html

RFC patches sent for review only are obviously welcome at any time.

Re: [PATCH net-next 2/2] net: snmp: add snmp tracepoint support for udp

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 15002 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/menglong8-dong-gmail-com/net-snmp-tracepoint-support-for-snmp/20211111-213642
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 84882cf72cd774cf16fd338bdbf00f69ac9f9194
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/15def40653e2754aa06d5af35d8fccd51ea903d2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review menglong8-dong-gmail-com/net-snmp-tracepoint-support-for-snmp/20211111-213642
        git checkout 15def40653e2754aa06d5af35d8fccd51ea903d2
        # save the attached .config to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/net/ethernet/mellanox/mlx5/core/diag/en_tc_tracepoint.c:5:
   drivers/net/ethernet/mellanox/mlx5/core/diag/en_tc_tracepoint.h:25:43: error: expected ')' before 'const'
      25 | DECLARE_EVENT_CLASS(mlx5e_flower_template,
         |                                           ^
         |                                           )
   drivers/net/ethernet/mellanox/mlx5/core/diag/en_tc_tracepoint.h:48:60: error: expected ')' before 'const'
      48 | DEFINE_EVENT(mlx5e_flower_template, mlx5e_configure_flower,
         |                                                            ^
         |                                                            )
   drivers/net/ethernet/mellanox/mlx5/core/diag/en_tc_tracepoint.h:53:57: error: expected ')' before 'const'
      53 | DEFINE_EVENT(mlx5e_flower_template, mlx5e_delete_flower,
         |                                                         ^
         |                                                         )
   drivers/net/ethernet/mellanox/mlx5/core/diag/en_tc_tracepoint.h:58:32: error: expected ')' before 'const'
      58 | TRACE_EVENT(mlx5e_stats_flower,
         |                                ^
         |                                )
   drivers/net/ethernet/mellanox/mlx5/core/diag/en_tc_tracepoint.h:77:46: error: expected ')' before 'const'
      77 | TRACE_EVENT(mlx5e_tc_update_neigh_used_value,
         |                                              ^
         |                                              )
   In file included from include/trace/define_trace.h:95,
                    from drivers/net/ethernet/mellanox/mlx5/core/diag/en_tc_tracepoint.h:114,
                    from drivers/net/ethernet/mellanox/mlx5/core/diag/en_tc_tracepoint.c:5:
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h:25:43: error: expected ')' before 'const'
      25 | DECLARE_EVENT_CLASS(mlx5e_flower_template,
         |                                           ^
         |                                           )
   In file included from <command-line>:
>> include/linux/static_call_types.h:15:34: error: '__SCT__tp_func_mlx5e_configure_flower' undeclared here (not in a function); did you mean '__SCK__tp_func_mlx5e_configure_flower'?
      15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
         |                                  ^~~~~~~
   include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
      59 | #define ___PASTE(a,b) a##b
         |                       ^
   include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
      18 | #define STATIC_CALL_TRAMP(name)  __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
         |                                  ^~~~~~~
   include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
      18 | #define STATIC_CALL_TRAMP(name)  __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
         |                                          ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/static_call.h:146:39: note: in expansion of macro 'STATIC_CALL_TRAMP'
     146 | #define STATIC_CALL_TRAMP_ADDR(name) &STATIC_CALL_TRAMP(name)
         |                                       ^~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:303:24: note: in expansion of macro 'STATIC_CALL_TRAMP_ADDR'
     303 |   .static_call_tramp = STATIC_CALL_TRAMP_ADDR(tp_func_##_name), \
         |                        ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:328:2: note: in expansion of macro 'DEFINE_TRACE_FN'
     328 |  DEFINE_TRACE_FN(name, NULL, NULL, PARAMS(proto), PARAMS(args));
         |  ^~~~~~~~~~~~~~~
   include/trace/define_trace.h:57:2: note: in expansion of macro 'DEFINE_TRACE'
      57 |  DEFINE_TRACE(name, PARAMS(proto), PARAMS(args))
         |  ^~~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h:48:1: note: in expansion of macro 'DEFINE_EVENT'
      48 | DEFINE_EVENT(mlx5e_flower_template, mlx5e_configure_flower,
         | ^~~~~~~~~~~~
>> include/linux/static_call_types.h:15:34: error: '__SCT__tp_func_mlx5e_delete_flower' undeclared here (not in a function); did you mean '__SCK__tp_func_mlx5e_delete_flower'?
      15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
         |                                  ^~~~~~~
   include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
      59 | #define ___PASTE(a,b) a##b
         |                       ^
   include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
      18 | #define STATIC_CALL_TRAMP(name)  __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
         |                                  ^~~~~~~
   include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
      18 | #define STATIC_CALL_TRAMP(name)  __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
         |                                          ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/static_call.h:146:39: note: in expansion of macro 'STATIC_CALL_TRAMP'
     146 | #define STATIC_CALL_TRAMP_ADDR(name) &STATIC_CALL_TRAMP(name)
         |                                       ^~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:303:24: note: in expansion of macro 'STATIC_CALL_TRAMP_ADDR'
     303 |   .static_call_tramp = STATIC_CALL_TRAMP_ADDR(tp_func_##_name), \
         |                        ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:328:2: note: in expansion of macro 'DEFINE_TRACE_FN'
     328 |  DEFINE_TRACE_FN(name, NULL, NULL, PARAMS(proto), PARAMS(args));
         |  ^~~~~~~~~~~~~~~
   include/trace/define_trace.h:57:2: note: in expansion of macro 'DEFINE_TRACE'
      57 |  DEFINE_TRACE(name, PARAMS(proto), PARAMS(args))
         |  ^~~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h:53:1: note: in expansion of macro 'DEFINE_EVENT'
      53 | DEFINE_EVENT(mlx5e_flower_template, mlx5e_delete_flower,
         | ^~~~~~~~~~~~
>> include/linux/static_call_types.h:15:34: error: '__SCT__tp_func_mlx5e_stats_flower' undeclared here (not in a function); did you mean '__SCK__tp_func_mlx5e_stats_flower'?
      15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
         |                                  ^~~~~~~
   include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
      59 | #define ___PASTE(a,b) a##b
         |                       ^
   include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
      18 | #define STATIC_CALL_TRAMP(name)  __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
         |                                  ^~~~~~~
   include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
      18 | #define STATIC_CALL_TRAMP(name)  __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
         |                                          ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/static_call.h:146:39: note: in expansion of macro 'STATIC_CALL_TRAMP'
     146 | #define STATIC_CALL_TRAMP_ADDR(name) &STATIC_CALL_TRAMP(name)
         |                                       ^~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:303:24: note: in expansion of macro 'STATIC_CALL_TRAMP_ADDR'
     303 |   .static_call_tramp = STATIC_CALL_TRAMP_ADDR(tp_func_##_name), \
         |                        ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:328:2: note: in expansion of macro 'DEFINE_TRACE_FN'
     328 |  DEFINE_TRACE_FN(name, NULL, NULL, PARAMS(proto), PARAMS(args));
         |  ^~~~~~~~~~~~~~~
   include/trace/define_trace.h:28:2: note: in expansion of macro 'DEFINE_TRACE'
      28 |  DEFINE_TRACE(name, PARAMS(proto), PARAMS(args))
         |  ^~~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h:58:1: note: in expansion of macro 'TRACE_EVENT'
      58 | TRACE_EVENT(mlx5e_stats_flower,
         | ^~~~~~~~~~~
>> include/linux/static_call_types.h:15:34: error: '__SCT__tp_func_mlx5e_tc_update_neigh_used_value' undeclared here (not in a function); did you mean '__SCK__tp_func_mlx5e_tc_update_neigh_used_value'?
      15 | #define STATIC_CALL_TRAMP_PREFIX __SCT__
         |                                  ^~~~~~~
   include/linux/compiler_types.h:59:23: note: in definition of macro '___PASTE'
      59 | #define ___PASTE(a,b) a##b
         |                       ^
   include/linux/static_call_types.h:18:34: note: in expansion of macro '__PASTE'
      18 | #define STATIC_CALL_TRAMP(name)  __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
         |                                  ^~~~~~~
   include/linux/static_call_types.h:18:42: note: in expansion of macro 'STATIC_CALL_TRAMP_PREFIX'
      18 | #define STATIC_CALL_TRAMP(name)  __PASTE(STATIC_CALL_TRAMP_PREFIX, name)
         |                                          ^~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/static_call.h:146:39: note: in expansion of macro 'STATIC_CALL_TRAMP'
     146 | #define STATIC_CALL_TRAMP_ADDR(name) &STATIC_CALL_TRAMP(name)
         |                                       ^~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:303:24: note: in expansion of macro 'STATIC_CALL_TRAMP_ADDR'
     303 |   .static_call_tramp = STATIC_CALL_TRAMP_ADDR(tp_func_##_name), \
         |                        ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:328:2: note: in expansion of macro 'DEFINE_TRACE_FN'
     328 |  DEFINE_TRACE_FN(name, NULL, NULL, PARAMS(proto), PARAMS(args));
         |  ^~~~~~~~~~~~~~~
   include/trace/define_trace.h:28:2: note: in expansion of macro 'DEFINE_TRACE'
      28 |  DEFINE_TRACE(name, PARAMS(proto), PARAMS(args))
         |  ^~~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h:77:1: note: in expansion of macro 'TRACE_EVENT'
      77 | TRACE_EVENT(mlx5e_tc_update_neigh_used_value,
         | ^~~~~~~~~~~
   In file included from include/trace/define_trace.h:102,
                    from drivers/net/ethernet/mellanox/mlx5/core/diag/en_tc_tracepoint.h:114,
                    from drivers/net/ethernet/mellanox/mlx5/core/diag/en_tc_tracepoint.c:5:
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h: In function 'ftrace_test_probe_mlx5e_configure_flower':
   include/trace/trace_events.h:757:2: error: implicit declaration of function 'check_trace_callback_type_mlx5e_configure_flower' [-Werror=implicit-function-declaration]
     757 |  check_trace_callback_type_##call(trace_event_raw_event_##template); \
         |  ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h:48:1: note: in expansion of macro 'DEFINE_EVENT'
      48 | DEFINE_EVENT(mlx5e_flower_template, mlx5e_configure_flower,
         | ^~~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h: In function 'ftrace_test_probe_mlx5e_delete_flower':
   include/trace/trace_events.h:757:2: error: implicit declaration of function 'check_trace_callback_type_mlx5e_delete_flower'; did you mean 'check_trace_callback_type_snmp_udplite'? [-Werror=implicit-function-declaration]
     757 |  check_trace_callback_type_##call(trace_event_raw_event_##template); \
         |  ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h:53:1: note: in expansion of macro 'DEFINE_EVENT'
      53 | DEFINE_EVENT(mlx5e_flower_template, mlx5e_delete_flower,
         | ^~~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h: In function 'ftrace_test_probe_mlx5e_stats_flower':
   include/trace/trace_events.h:757:2: error: implicit declaration of function 'check_trace_callback_type_mlx5e_stats_flower'; did you mean 'check_trace_callback_type_snmp_udplite'? [-Werror=implicit-function-declaration]
     757 |  check_trace_callback_type_##call(trace_event_raw_event_##template); \
         |  ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/trace/trace_events.h:81:2: note: in expansion of macro 'DEFINE_EVENT'
      81 |  DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args));
         |  ^~~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h:58:1: note: in expansion of macro 'TRACE_EVENT'
      58 | TRACE_EVENT(mlx5e_stats_flower,
         | ^~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h: In function 'ftrace_test_probe_mlx5e_tc_update_neigh_used_value':
   include/trace/trace_events.h:757:2: error: implicit declaration of function 'check_trace_callback_type_mlx5e_tc_update_neigh_used_value'; did you mean 'trace_raw_output_mlx5e_tc_update_neigh_used_value'? [-Werror=implicit-function-declaration]
     757 |  check_trace_callback_type_##call(trace_event_raw_event_##template); \
         |  ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/trace/trace_events.h:81:2: note: in expansion of macro 'DEFINE_EVENT'
      81 |  DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args));
         |  ^~~~~~~~~~~~
   drivers/net/ethernet/mellanox/mlx5/core/./diag/en_tc_tracepoint.h:77:1: note: in expansion of macro 'TRACE_EVENT'
      77 | TRACE_EVENT(mlx5e_tc_update_neigh_used_value,
         | ^~~~~~~~~~~
   cc1: all warnings being treated as errors


vim +15 include/linux/static_call_types.h

115284d89a436e9 Josh Poimboeuf 2020-08-18  14  
115284d89a436e9 Josh Poimboeuf 2020-08-18 @15  #define STATIC_CALL_TRAMP_PREFIX	__SCT__
115284d89a436e9 Josh Poimboeuf 2020-08-18  16  #define STATIC_CALL_TRAMP_PREFIX_STR	__stringify(STATIC_CALL_TRAMP_PREFIX)
9183c3f9ed710a8 Josh Poimboeuf 2020-08-18  17  #define STATIC_CALL_TRAMP_PREFIX_LEN	(sizeof(STATIC_CALL_TRAMP_PREFIX_STR) - 1)
115284d89a436e9 Josh Poimboeuf 2020-08-18  18  #define STATIC_CALL_TRAMP(name)		__PASTE(STATIC_CALL_TRAMP_PREFIX, name)
115284d89a436e9 Josh Poimboeuf 2020-08-18  19  #define STATIC_CALL_TRAMP_STR(name)	__stringify(STATIC_CALL_TRAMP(name))
115284d89a436e9 Josh Poimboeuf 2020-08-18  20  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66058 bytes --]

Re: [PATCH net-next 0/2] net: snmp: tracepoint support for snmp

[Menglong Dong]

Hello,

On Thu, Nov 11, 2021 at 10:08 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
[...]
>
> I feel like I have seen this idea before. Is this your first posting?
>
> Would you mind including links to previous discussion if you're aware
> of any?

This is the first time that I post this patch. Do you mean that someone
else has done this before? Sorry, I didn't find it~

>
> Regardless:
>
>
> # Form letter - net-next is closed
>
> We have already sent the networking pull request for 5.16
> and therefore net-next is closed for new drivers, features,
> code refactoring and optimizations. We are currently accepting
> bug fixes only.
>
> Please repost when net-next reopens after 5.16-rc1 is cut.
>

Ok, I'll repost it later. By the way, is this idea acceptable? In fact,
I also introduced the snmp-tracepoint for ICMP, TCP and IP.

Thanks!
Menglong Dong

> Look out for the announcement on the mailing list or check:
> http://vger.kernel.org/~davem/net-next.html
>
> RFC patches sent for review only are obviously welcome at any time.

Re: [PATCH net-next 0/2] net: snmp: tracepoint support for snmp

[Jakub Kicinski]

On Fri, 12 Nov 2021 09:40:47 +0800 Menglong Dong wrote:
> > I feel like I have seen this idea before. Is this your first posting?
> >
> > Would you mind including links to previous discussion if you're aware
> > of any?  
> 
> This is the first time that I post this patch. Do you mean that someone
> else has done this before? Sorry, I didn't find it~

I see. Yes, I believe very similar changes were proposed in the past.

I believe that concerns about the performance impact had prevented them
from being merged.

Re: [PATCH net-next 0/2] net: snmp: tracepoint support for snmp

[Menglong Dong]

Hello,

On Fri, Nov 12, 2021 at 9:50 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 12 Nov 2021 09:40:47 +0800 Menglong Dong wrote:
> > > I feel like I have seen this idea before. Is this your first posting?
> > >
> > > Would you mind including links to previous discussion if you're aware
> > > of any?
> >
> > This is the first time that I post this patch. Do you mean that someone
> > else has done this before? Sorry, I didn't find it~
>
> I see. Yes, I believe very similar changes were proposed in the past.
>
> I believe that concerns about the performance impact had prevented them
> from being merged.

I have found a similar post:
https://lore.kernel.org/netdev/20090303165747.GA1480@hmsreliant.think-freely.org/

And this is the tracepoint for kfree_skb().

I also concerns about the performance. However, with the tracepoints disabled,
they don't have any impact; with enabled, their impact is no more than the
tracepoint in kfree_skb() and consume_skb().

What's more, I have also realized another version: create tracepoint for every
statistics type, such as snmp_udp_incsumerrors, snmp_udp_rcvbuferrors, etc.
This can solve performance issue, as users can enable part of them, which
may be triggered not frequently. However, too many tracepoint are created, and
I think it may be not applicable.

Thanks!
Menglong Dong

Re: [PATCH net-next 0/2] net: snmp: tracepoint support for snmp

[Steven Rostedt]

On Fri, 12 Nov 2021 14:42:23 +0800
Menglong Dong <menglong8.dong@gmail.com> wrote:

> What's more, I have also realized another version: create tracepoint for every
> statistics type, such as snmp_udp_incsumerrors, snmp_udp_rcvbuferrors, etc.
> This can solve performance issue, as users can enable part of them, which
> may be triggered not frequently. However, too many tracepoint are created, and
> I think it may be not applicable.

If possible, it would be great to have a single tracepoint to handle all
statistics (not sure what data it will be having). Or at least break it
down to one tracepoint per group of statistics.

There's two approaches that can be taken.

1) Create a DECLARE_EVENT_CLASS() template that the group of tracepoints
use, and then create a DEFINE_EVENT() for each one. This will create a
separate trace event for each stat. Most the footprint of a trace event is
in the CLASS portion, so having a single class helps keep the size overhead
down.

2) Just use a single trace event for all stats in a group, but perhaps have
a type field for each to use. That way it can be easy to filter on a set of
stats to trace.

-- Steve

Re: [PATCH net-next 2/2] net: snmp: add snmp tracepoint support for udp

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 9127 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/menglong8-dong-gmail-com/net-snmp-tracepoint-support-for-snmp/20211111-213642
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 84882cf72cd774cf16fd338bdbf00f69ac9f9194
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/15def40653e2754aa06d5af35d8fccd51ea903d2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review menglong8-dong-gmail-com/net-snmp-tracepoint-support-for-snmp/20211111-213642
        git checkout 15def40653e2754aa06d5af35d8fccd51ea903d2
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash net/ipv6/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/ipv6/udp.c: In function 'udpv6_queue_rcv_one_skb':
>> net/ipv6/udp.c:705:59: error: macro "__UDP_INC_STATS" requires 4 arguments, but only 3 given
     705 |                                                 is_udplite);
         |                                                           ^
   In file included from net/ipv6/udp_impl.h:4,
                    from net/ipv6/udp.c:55:
   include/net/udp.h:421: note: macro "__UDP_INC_STATS" defined here
     421 | #define __UDP_INC_STATS(net, skb, field, is_udplite)            do {    \
         | 
>> net/ipv6/udp.c:703:33: error: '__UDP_INC_STATS' undeclared (first use in this function); did you mean 'UDP_INC_STATS'?
     703 |                                 __UDP_INC_STATS(sock_net(sk),
         |                                 ^~~~~~~~~~~~~~~
         |                                 UDP_INC_STATS
   net/ipv6/udp.c:703:33: note: each undeclared identifier is reported only once for each function it appears in


vim +/__UDP_INC_STATS +705 net/ipv6/udp.c

ba4e58eca8aa94 Gerrit Renker        2006-11-27  669  
cf329aa42b6659 Paolo Abeni          2018-11-07  670  static int udpv6_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
ba4e58eca8aa94 Gerrit Renker        2006-11-27  671  {
ba4e58eca8aa94 Gerrit Renker        2006-11-27  672  	struct udp_sock *up = udp_sk(sk);
b2bf1e2659b1cb Wang Chen            2007-12-03  673  	int is_udplite = IS_UDPLITE(sk);
a18135eb9389c2 David S. Miller      2006-08-15  674  
ba4e58eca8aa94 Gerrit Renker        2006-11-27  675  	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
ba4e58eca8aa94 Gerrit Renker        2006-11-27  676  		goto drop;
^1da177e4c3f41 Linus Torvalds       2005-04-16  677  
88ab31081b8c8d Davidlohr Bueso      2018-05-08  678  	if (static_branch_unlikely(&udpv6_encap_needed_key) && up->encap_type) {
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  679  		int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  680  
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  681  		/*
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  682  		 * This is an encapsulation socket so pass the skb to
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  683  		 * the socket's udp_encap_rcv() hook. Otherwise, just
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  684  		 * fall through and pass this up the UDP socket.
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  685  		 * up->encap_rcv() returns the following value:
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  686  		 * =0 if skb was successfully passed to the encap
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  687  		 *    handler or was discarded by it.
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  688  		 * >0 if skb should be passed on to UDP.
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  689  		 * <0 if skb should be resubmitted as proto -N
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  690  		 */
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  691  
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  692  		/* if we're overly short, let UDP handle it */
6aa7de059173a9 Mark Rutland         2017-10-23  693  		encap_rcv = READ_ONCE(up->encap_rcv);
e5aed006be918a Hannes Frederic Sowa 2016-05-19  694  		if (encap_rcv) {
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  695  			int ret;
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  696  
0a80966b1043c3 Tom Herbert          2014-05-07  697  			/* Verify checksum before giving to encap */
0a80966b1043c3 Tom Herbert          2014-05-07  698  			if (udp_lib_checksum_complete(skb))
0a80966b1043c3 Tom Herbert          2014-05-07  699  				goto csum_error;
0a80966b1043c3 Tom Herbert          2014-05-07  700  
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  701  			ret = encap_rcv(sk, skb);
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  702  			if (ret <= 0) {
02c223470c3cc3 Eric Dumazet         2016-04-27 @703  				__UDP_INC_STATS(sock_net(sk),
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  704  						UDP_MIB_INDATAGRAMS,
d7f3f62167bc22 Benjamin LaHaise     2012-04-27 @705  						is_udplite);
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  706  				return -ret;
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  707  			}
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  708  		}
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  709  
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  710  		/* FALLTHROUGH -- it's a UDP Packet */
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  711  	}
d7f3f62167bc22 Benjamin LaHaise     2012-04-27  712  
ba4e58eca8aa94 Gerrit Renker        2006-11-27  713  	/*
ba4e58eca8aa94 Gerrit Renker        2006-11-27  714  	 * UDP-Lite specific tests, ignored on UDP sockets (see net/ipv4/udp.c).
ba4e58eca8aa94 Gerrit Renker        2006-11-27  715  	 */
b0a422772fec29 Miaohe Lin           2020-07-21  716  	if ((up->pcflag & UDPLITE_RECV_CC)  &&  UDP_SKB_CB(skb)->partial_cov) {
ba4e58eca8aa94 Gerrit Renker        2006-11-27  717  
ba4e58eca8aa94 Gerrit Renker        2006-11-27  718  		if (up->pcrlen == 0) {          /* full coverage was set  */
ba7a46f16dd29f Joe Perches          2014-11-11  719  			net_dbg_ratelimited("UDPLITE6: partial coverage %d while full coverage %d requested\n",
ba4e58eca8aa94 Gerrit Renker        2006-11-27  720  					    UDP_SKB_CB(skb)->cscov, skb->len);
ba4e58eca8aa94 Gerrit Renker        2006-11-27  721  			goto drop;
^1da177e4c3f41 Linus Torvalds       2005-04-16  722  		}
ba4e58eca8aa94 Gerrit Renker        2006-11-27  723  		if (UDP_SKB_CB(skb)->cscov  <  up->pcrlen) {
ba7a46f16dd29f Joe Perches          2014-11-11  724  			net_dbg_ratelimited("UDPLITE6: coverage %d too small, need min %d\n",
ba4e58eca8aa94 Gerrit Renker        2006-11-27  725  					    UDP_SKB_CB(skb)->cscov, up->pcrlen);
ba4e58eca8aa94 Gerrit Renker        2006-11-27  726  			goto drop;
ba4e58eca8aa94 Gerrit Renker        2006-11-27  727  		}
ba4e58eca8aa94 Gerrit Renker        2006-11-27  728  	}
ba4e58eca8aa94 Gerrit Renker        2006-11-27  729  
4b943faedfc29e Paolo Abeni          2017-06-22  730  	prefetch(&sk->sk_rmem_alloc);
ce25d66ad5f8d9 Eric Dumazet         2016-06-02  731  	if (rcu_access_pointer(sk->sk_filter) &&
ce25d66ad5f8d9 Eric Dumazet         2016-06-02  732  	    udp_lib_checksum_complete(skb))
6a5dc9e598fe90 Eric Dumazet         2013-04-29  733  		goto csum_error;
ce25d66ad5f8d9 Eric Dumazet         2016-06-02  734  
ba66bbe5480a01 Daniel Borkmann      2016-07-25  735  	if (sk_filter_trim_cap(sk, skb, sizeof(struct udphdr)))
a612769774a30e Michal Kubeček       2016-07-08  736  		goto drop;
^1da177e4c3f41 Linus Torvalds       2005-04-16  737  
e6afc8ace6dd5c samanthakumar        2016-04-05  738  	udp_csum_pull_header(skb);
cb80ef463d1881 Benjamin LaHaise     2012-04-27  739  
d826eb14ecef35 Eric Dumazet         2011-11-09  740  	skb_dst_drop(skb);
cb75994ec311b2 Wang Chen            2007-12-03  741  
850cbaddb52dfd Paolo Abeni          2016-10-21  742  	return __udpv6_queue_rcv_skb(sk, skb);
3e215c8d1b6b77 James M Leddy        2014-06-25  743  
6a5dc9e598fe90 Eric Dumazet         2013-04-29  744  csum_error:
02c223470c3cc3 Eric Dumazet         2016-04-27  745  	__UDP6_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
ba4e58eca8aa94 Gerrit Renker        2006-11-27  746  drop:
02c223470c3cc3 Eric Dumazet         2016-04-27  747  	__UDP6_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
cb80ef463d1881 Benjamin LaHaise     2012-04-27  748  	atomic_inc(&sk->sk_drops);
ba4e58eca8aa94 Gerrit Renker        2006-11-27  749  	kfree_skb(skb);
ba4e58eca8aa94 Gerrit Renker        2006-11-27  750  	return -1;
^1da177e4c3f41 Linus Torvalds       2005-04-16  751  }
^1da177e4c3f41 Linus Torvalds       2005-04-16  752  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61714 bytes --]

Re: [PATCH net-next 0/2] net: snmp: tracepoint support for snmp

[Menglong Dong]

Hello~

On Fri, Nov 12, 2021 at 10:31 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 12 Nov 2021 14:42:23 +0800
> Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> > What's more, I have also realized another version: create tracepoint for every
> > statistics type, such as snmp_udp_incsumerrors, snmp_udp_rcvbuferrors, etc.
> > This can solve performance issue, as users can enable part of them, which
> > may be triggered not frequently. However, too many tracepoint are created, and
> > I think it may be not applicable.
>
> If possible, it would be great to have a single tracepoint to handle all
> statistics (not sure what data it will be having). Or at least break it
> down to one tracepoint per group of statistics.
>
> There's two approaches that can be taken.
>
> 1) Create a DECLARE_EVENT_CLASS() template that the group of tracepoints
> use, and then create a DEFINE_EVENT() for each one. This will create a
> separate trace event for each stat. Most the footprint of a trace event is
> in the CLASS portion, so having a single class helps keep the size overhead
> down.
>

In fact, I think I'm using the first idea. I defined the DEFINE_SNMP_EVENT() in
https://lore.kernel.org/all/20211111133530.2156478-2-imagedong@tencent.com/,
which is used to create the tracepoint for each group. And I plan to create
tracepoint by protocols. such as: events/snmp/snmp_udp, events/snmp/snmp_tcp,
events/snmp/snmp_icmp, etc. And every trace event have a type field, which is
used to simply filter by statistics type:

+DECLARE_EVENT_CLASS(snmp_template,
+
+       TP_PROTO(struct sk_buff *skb, int field, int val),
+
+       TP_ARGS(skb, field, val),
+
+       TP_STRUCT__entry(
+               __field(void *, skbaddr)
+               __field(int, field)
+               __field(int, val)
+       ),
+
+       TP_fast_assign(
+               __entry->skbaddr = skb;
+               __entry->field = field;
+               __entry->val = val;
+       ),
+
+       TP_printk("skbaddr=%p, field=%d, val=%d", __entry->skbaddr,
+                 __entry->field, __entry->val)
+);
+
+#define DEFINE_SNMP_EVENT(proto)                               \
+DEFINE_EVENT(snmp_template, snmp_##proto,                      \
+       TP_PROTO(struct sk_buff *skb, int field, int val),      \
+       TP_ARGS(skb, field, val)                                \
+)

I think using a single trace event may have performance impact?

I will post the complete patch series after netdev opens.

Thanks!
Menglong Dong

> 2) Just use a single trace event for all stats in a group, but perhaps have
> a type field for each to use. That way it can be easy to filter on a set of
> stats to trace.
>
> -- Steve

Re: [PATCH net-next 1/2] net: snmp: add tracepoint support for snmp

[Steven Rostedt]

On Thu, 11 Nov 2021 21:35:29 +0800
menglong8.dong@gmail.com wrote:

> +#define DEFINE_SNMP_EVENT(proto)				\
> +DEFINE_EVENT(snmp_template, snmp_##proto,			\
> +	TP_PROTO(struct sk_buff *skb, int field, int val),	\
> +	TP_ARGS(skb, field, val)				\
> +)
> +
> +#define TRACE_SNMP(skb, proto, field, val) \
> +	trace_snmp_##proto(skb, field, val)
> +
> +#endif

Why make a separate trace event for each protocol, and not just create an
enum that gets passed to the trace event? Then you could just filter on
what you want.

-- Steve

Re: [PATCH net-next 1/2] net: snmp: add tracepoint support for snmp

[Menglong Dong]

On Wed, Nov 17, 2021 at 5:34 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 11 Nov 2021 21:35:29 +0800
> menglong8.dong@gmail.com wrote:
>
> > +#define DEFINE_SNMP_EVENT(proto)                             \
> > +DEFINE_EVENT(snmp_template, snmp_##proto,                    \
> > +     TP_PROTO(struct sk_buff *skb, int field, int val),      \
> > +     TP_ARGS(skb, field, val)                                \
> > +)
> > +
> > +#define TRACE_SNMP(skb, proto, field, val) \
> > +     trace_snmp_##proto(skb, field, val)
> > +
> > +#endif
>
> Why make a separate trace event for each protocol, and not just create an
> enum that gets passed to the trace event? Then you could just filter on
> what you want.

enn....I'm not sure, just feel comfortable to create a separate trace event for
each protocol. Maybe it is easier to use? However, making them together
seems more fridently to users who want to do statistics for all protocols. I'll
think over it~~~

Thanks!
Menglong Dong

>
> -- Steve