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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 16 Nov 2021 10:27:41 +0800
From:   Menglong Dong <menglong8.dong@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>, mingo@...hat.com,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        dsahern@...nel.org, Menglong Dong <imagedong@...cent.com>,
        LKML <linux-kernel@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 0/2] net: snmp: tracepoint support for snmp

Hello~

On Fri, Nov 12, 2021 at 10:31 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Fri, 12 Nov 2021 14:42:23 +0800
> Menglong Dong <menglong8.dong@...il.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