netdev - 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] [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.