Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220902141715.1038615-1-imagedong@tencent.com>
 <CANn89iK7Mm4aPpr1-VM5OgicuHrHjo9nm9P9bYgOKKH9yczFzg@mail.gmail.com> <20220905103808.434f6909@gandalf.local.home>
In-Reply-To: <20220905103808.434f6909@gandalf.local.home>
From:   Eric Dumazet <edumazet@google.com>
Date:   Thu, 27 Oct 2022 08:32:02 -0700
Message-ID: <CANn89i+qp=gmhx_1b+=hEiHA7yNGkfh46YPKhUc9GFbtNYBZrA@mail.gmail.com>
Subject: Re: [PATCH net-next v2] net: skb: export skb drop reaons to user by TRACE_DEFINE_ENUM
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     Menglong Dong <menglong8.dong@gmail.com>,
        Jakub Kicinski <kuba@kernel.org>,
        David Miller <davem@davemloft.net>,
        Paolo Abeni <pabeni@redhat.com>,
        Ingo Molnar <mingo@redhat.com>,
        Menglong Dong <imagedong@tencent.com>,
        David Ahern <dsahern@kernel.org>,
        Hao Peng <flyingpeng@tencent.com>,
        Dongli Zhang <dongli.zhang@oracle.com>, robh@kernel.org,
        Pavel Begunkov <asml.silence@gmail.com>,
        Luiz Augusto von Dentz <luiz.von.dentz@intel.com>,
        Vasily Averin <vasily.averin@linux.dev>,
        LKML <linux-kernel@vger.kernel.org>,
        netdev <netdev@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Sep 5, 2022 at 7:37 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 2 Sep 2022 08:43:07 -0700
> Eric Dumazet <edumazet@google.com> wrote:
>
> > > ---
> > > v2:
> > > - undef FN/FNe after use it (Jakub Kicinski)
> >
> > I would love some feedback from Steven :)
>
> The undef should be fine. I usually do not, but that's more of a preference
> than a rule.
>
> As long as the undef is done after the C portion of where the macro is used:
>
> +#undef FN
> +#define FN(reason)     TRACE_DEFINE_ENUM(SKB_DROP_REASON_##reason);
> +DEFINE_DROP_REASON(FN, FN) <<<--- C portion
>
> +#undef FN
> +#undef FNe
> +#define FN(reason)     { SKB_DROP_REASON_##reason, #reason },
> +#define FNe(reason)    { SKB_DROP_REASON_##reason, #reason }
> +
>  /*
>   * Tracepoint for free an sk_buff:
>   */
> @@ -35,9 +44,13 @@ TRACE_EVENT(kfree_skb,
>
>         TP_printk("skbaddr=%p protocol=%u location=%p reason: %s",
>                   __entry->skbaddr, __entry->protocol, __entry->location,
> -                 drop_reasons[__entry->reason])
> +                 __print_symbolic(__entry->reason,
> +                                  DEFINE_DROP_REASON(FN, FNe)))  <<<--- C portion
>  );
>
> +#undef FN
> +#undef FNe
>
> So for this part: Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>
> -- Steve

This seems broken again (tried on latest net-next tree)

perf script
            sshd 12192 [006]   200.090602: skb:kfree_skb:
skbaddr=0x2fa000400000 protocol=7844 location=0x2fa000001ea4 reason:
         swapper     0 [030]   200.497468: skb:kfree_skb:
skbaddr=0xba00400001 protocol=65535 location=0x9080c62800000000
reason:
 kworker/30:1-ev   308 [030]   200.497476: skb:kfree_skb:
skbaddr=0xba00400001 protocol=65535 location=0x9080c62800000000
reason:
         swapper     0 [009]   200.957881: skb:kfree_skb:
skbaddr=0x2fa400400000 protocol=12195 location=0x2fa400002fa3 reason:
         swapper     0 [026]   201.515769: skb:kfree_skb:
skbaddr=0xb600400001 protocol=65535 location=0x9080c62800000000
reason:
 kworker/26:1-mm   276 [026]   201.515797: skb:kfree_skb:
skbaddr=0xb600400001 protocol=65535 location=0x9080c62800000000
reason:
 kworker/26:1-mm   276 [026]   201.515802: skb:kfree_skb:
skbaddr=0x2f9f00400000 protocol=12189 location=0x2f9f00002f9d reason:
         swapper     0 [030]   201.521484: skb:kfree_skb:
skbaddr=0xba00400001 protocol=65535 location=0x9080c62800000000
reason:
 kworker/30:1-ev   308 [030]   201.521491: skb:kfree_skb:
skbaddr=0x2fa100400000 protocol=12192 location=0x2fa100002fa0 reason: