Messages in this thread | | | Date | Thu, 4 Aug 2022 02:39:15 +0200 | From | Pablo Neira Ayuso <> | Subject | Re: [GIT PULL] Networking for 6.0 |
| |
On Wed, Aug 03, 2022 at 05:27:07PM -0700, Linus Torvalds wrote: > On Wed, Aug 3, 2022 at 5:11 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > For these two questions, this new Kconfig toggle was copied from: > > > > config NF_CONNTRACK_PROCFS > > bool "Supply CT list in procfs (OBSOLETE)" > > default y > > depends on PROC_FS > > > > which is under: > > > > if NF_CONNTRACK > > > > but the copy and paste was missing this. > > Note that there's two problems with that > > (1) the NF_CONNTRACK_PROCFS thing is 'default y' because it *USED* to > be unconditional, and was split up as a config option back in 2011. > > See commit 54b07dca6855 ("netfilter: provide config option to disable > ancient procfs parts"). > > IOW, that NF_CONNTRACK_PROCFS exists and defaults to on, not because > people added new code and wanted to make it default, but because the > code used to always be enabled if NF_CONNTRACK was enabled, and people > wanted the option to *disable* it. > > That's when you do 'default y' - you take existing code that didn't > originally have a question at all, and you make it optional. Then you > use 'default y' so that people who used it don't get screwed in the > process. > > (2) it didn't do the proper conditional on the feature it depended on. > > So let's not do copy-and-paste programming. The old Kconfig snippet > had completely different rules, had completely different history, and > completely different default values as a result. > > I realize that it's very easy to think of Kconfig as some > not-very-important detail to just hook things up. But because it's > front-facing to users, I do want people to think about it more than > perhaps people otherwise would.
Agreed, it was a bad a idea to copy and paste it from NF_CONNTRACK_PROCFS, this new toggle has nothing to do with it.
I'll take a closer look at any new Kconfig toggle coming in the future to avoid issues like this.
Thanks for reviewing.
| |