Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220803101438.24327-1-pabeni@redhat.com> <CAHk-=widn7iZozvVZ37cDPK26BdOegGAX_JxR+v62sCv-5=eZg@mail.gmail.com>
 <YusOpd6IuLB29LHs@salvia>
In-Reply-To: <YusOpd6IuLB29LHs@salvia>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed, 3 Aug 2022 17:27:07 -0700
Message-ID: <CAHk-=wj59jR+pxYHmtf+OJvThEpYLNYLb9P5YkgCcBWDWzhdPQ@mail.gmail.com>
Subject: Re: [GIT PULL] Networking for 6.0
To:     Pablo Neira Ayuso <pablo@netfilter.org>
Cc:     Paolo Abeni <pabeni@redhat.com>, Vlad Buslov <vladbu@nvidia.com>,
        Oz Shlomo <ozsh@nvidia.com>, kuba@kernel.org,
        davem@davemloft.net, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.

                Linus