Re: [PATCH net-next v2 1/9] net: skb: introduce __DEFINE_SKB_DROP_REASON() to simply the code

[Menglong Dong]

On Wed, May 18, 2022 at 9:15 AM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
>
> On Tue, 17 May 2022 16:10:00 +0800 menglong8.dong@xxxxxxxxx wrote:
> > From: Menglong Dong <imagedong@xxxxxxxxxxx>
> >
> > 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.
>

Yeah, I found this problem too. My autocomplete in vscode never helps
me anymore after I use this macro.

> 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?
>

Oh, I forgot about auto-generation, it seems it's a better choice.
I'll try to use auto-generation.

> 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.

Ok, I'll make the enum in a standalone header in the separated
series.

Thans!
Menglong Dong