Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220530081201.10151-1-imagedong@tencent.com> <20220530081201.10151-3-imagedong@tencent.com>
 <20220530131311.40914ab7@kernel.org>
In-Reply-To: <20220530131311.40914ab7@kernel.org>
From:   Menglong Dong <menglong8.dong@gmail.com>
Date:   Wed, 1 Jun 2022 11:27:41 +0800
Message-ID: <CADxym3bQ96s_tsQeE_1_TFNafTvzQfRr9WLB40urZCgn4a2C0A@mail.gmail.com>
Subject: Re: [PATCH net-next v2 2/3] net: skb: use auto-generation to convert
 skb drop reason to string
To:     Jakub Kicinski <kuba@kernel.org>
Cc:     Menglong Dong <imagedong@tencent.com>,
        LKML <linux-kernel@vger.kernel.org>,
        netdev <netdev@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, May 31, 2022 at 4:13 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 30 May 2022 16:12:00 +0800 menglong8.dong@gmail.com wrote:
> > From: Menglong Dong <imagedong@tencent.com>
> >
> > It is annoying to add new skb drop reasons to 'enum skb_drop_reason'
> > and TRACE_SKB_DROP_REASON in trace/event/skb.h, and it's easy to forget
> > to add the new reasons we added to TRACE_SKB_DROP_REASON.
> >
> > TRACE_SKB_DROP_REASON is used to convert drop reason of type number
> > to string. For now, the string we passed to user space is exactly the
> > same as the name in 'enum skb_drop_reason' with a 'SKB_DROP_REASON_'
> > prefix. Therefore, we can use 'auto-generation' to generate these
> > drop reasons to string at build time.
> >
> > The new header 'dropreason_str.h'
>
> Not any more.
>
> > will be generated, and the
> > __DEFINE_SKB_DROP_REASON() in it can do the converting job. Meanwhile,
> > we use a global array to store these string, which can be used both
> > in drop_monitor and 'kfree_skb' tracepoint.
>
> > diff --git a/include/net/dropreason.h b/include/net/dropreason.h
> > index ecd18b7b1364..460de425297c 100644
> > --- a/include/net/dropreason.h
> > +++ b/include/net/dropreason.h
> > @@ -3,6 +3,8 @@
> >  #ifndef _LINUX_DROPREASON_H
> >  #define _LINUX_DROPREASON_H
> >
> > +#include <linux/kernel.h>
>
> Why?

Oh, you noticed it. To simplify the code in dropreason_str.c, as
EXPORT_SYMBOL() is used. Okay, I'll move it to the generation
part.

>
> > +dropreason_str.c
> > \ No newline at end of file
>
> Heed the warning.
>
> > diff --git a/net/core/Makefile b/net/core/Makefile
> > index a8e4f737692b..3c7f99ff6d89 100644
> > --- a/net/core/Makefile
> > +++ b/net/core/Makefile
> > @@ -4,7 +4,8 @@
> >  #
> >
> >  obj-y := sock.o request_sock.o skbuff.o datagram.o stream.o scm.o \
> > -      gen_stats.o gen_estimator.o net_namespace.o secure_seq.o flow_dissector.o
> > +      gen_stats.o gen_estimator.o net_namespace.o secure_seq.o \
> > +      flow_dissector.o dropreason_str.o
> >
> >  obj-$(CONFIG_SYSCTL) += sysctl_net_core.o
> >
> > @@ -39,3 +40,23 @@ obj-$(CONFIG_NET_SOCK_MSG) += skmsg.o
> >  obj-$(CONFIG_BPF_SYSCALL) += sock_map.o
> >  obj-$(CONFIG_BPF_SYSCALL) += bpf_sk_storage.o
> >  obj-$(CONFIG_OF)     += of_net.o
> > +
> > +clean-files := dropreason_str.c
> > +
> > +quiet_cmd_dropreason_str = GEN     $@
> > +cmd_dropreason_str = awk -F ',' 'BEGIN{ print "\#include <net/dropreason.h>\n"; \
> > +     print "const char * const drop_reasons[] = {" }\
> > +     /^enum skb_drop/ { dr=1; }\
> > +     /\}\;/ { dr=0; }\
> > +     /^\tSKB_DROP_REASON_/ {\
> > +             if (dr) {\
> > +                     sub(/\tSKB_DROP_REASON_/, "", $$1);\
> > +                     printf "\t[SKB_DROP_REASON_%s] = \"%s\",\n", $$1, $$1;\
> > +             }\
> > +     } \
> > +     END{ print "};\nEXPORT_SYMBOL(drop_reasons);" }' $< > $@
> > +
> > +$(obj)/dropreason_str.c: $(srctree)/include/net/dropreason.h
> > +     $(call cmd,dropreason_str)
>
> I'm getting this:
>
>   awk: cmd. line:1: warning: regexp escape sequence `\;' is not a known regexp operator
>

Enn...I think this warning comes from the "/\}\;/ { dr=0; }\" part.
Let me do more investigation.

Thanks!
Menglong Dong