[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220517181457.04c37147@kernel.org>
Date: Tue, 17 May 2022 18:14:57 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: menglong8.dong@...il.com
Cc: edumazet@...gle.com, rostedt@...dmis.org, mingo@...hat.com,
davem@...emloft.net, yoshfuji@...ux-ipv6.org, dsahern@...nel.org,
pabeni@...hat.com, imagedong@...cent.com, kafai@...com,
talalahmad@...gle.com, keescook@...omium.org,
dongli.zhang@...cle.com, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, Jiang Biao <benbjiang@...cent.com>,
Hao Peng <flyingpeng@...cent.com>
Subject: Re: [PATCH net-next v2 1/9] net: skb: introduce
__DEFINE_SKB_DROP_REASON() to simply the code
On Tue, 17 May 2022 16:10:00 +0800 menglong8.dong@...il.com wrote:
> From: Menglong Dong <imagedong@...cent.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. So why not make them togather by define a macro?
>
> Therefore, introduce __DEFINE_SKB_DROP_REASON() and use it for 'enum
> skb_drop_reason' definition and string converting.
>
> Now, what should we with the document for the reasons? How about follow
> __BPF_FUNC_MAPPER() and make these document togather?
Hi, I know BPF does this but I really find the definition-by-macro
counter productive :(
kdoc will no longer work right because the parser will not see
the real values. cscope and other code indexers will struggle
to find definitions.
Did you investigate using auto-generation? Kernel already generates
a handful of headers. Maybe with a little script we could convert
the enum into the string thing at build time?
Also let's use this opportunity to move the enum to a standalone
header, it's getting huge.
Probably worth keeping this rework separate from the TCP patches.
Up to you which one you'd like to get done first.
Powered by blists - more mailing lists