linux-kernel - Re: [PATCH net-next v2 1/9] net: skb: introduce __DEFINE_SKB_DROP

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] [day] [month] [year] [list]

Date:   Wed, 18 May 2022 10:15:25 +0800
From:   Menglong Dong <menglong8.dong@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Eric Dumazet <edumazet@...gle.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        David Miller <davem@...emloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Menglong Dong <imagedong@...cent.com>,
        Martin Lau <kafai@...com>, Talal Ahmad <talalahmad@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Dongli Zhang <dongli.zhang@...cle.com>,
        LKML <linux-kernel@...r.kernel.org>,
        netdev <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 Wed, May 18, 2022 at 9:15 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> 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.
>

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