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

Hello~

On Fri, Nov 12, 2021 at 10:31 PM Steven Rostedt [off-list ref] wrote:

On Fri, 12 Nov 2021 14:42:23 +0800
Menglong Dong [off-list ref] wrote:

quoted

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/ (local),
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