Re: [GIT PULL] Networking for 6.0

[Pablo Neira Ayuso]

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