All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Linus Torvalds <torvalds@linux-foundation.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
Subject: Re: [GIT PULL] Networking for 6.0
Date: Thu, 4 Aug 2022 02:11:17 +0200	[thread overview]
Message-ID: <YusOpd6IuLB29LHs@salvia> (raw)
In-Reply-To: <CAHk-=widn7iZozvVZ37cDPK26BdOegGAX_JxR+v62sCv-5=eZg@mail.gmail.com>

On Wed, Aug 03, 2022 at 04:52:32PM -0700, Linus Torvalds wrote:
> On Wed, Aug 3, 2022 at 3:15 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git tags/net-next-6.0
> 
> Hmm. Another thing I note about this.
> 
> It adds a new NF_FLOW_TABLE_PROCFS option, and that one has two problems:
> 
>  - it is 'default y'. Why?
>
>  - it has 'depends on PROC_FS' etc, but guess what it does *not*
> depend on? NF_FLOW_TABLE itself.

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.

> So not only does this new code try to enable itself by default, which
> is a no-no. We do "default y" if it's an old feature that got split
> out as a config option, or if it's something that everybody *really*
> should have, but I don't see that being the case here.
> 
> But it also asks the user that question even when the user doesn't
> even have NF_FLOW_TABLE at all. Which seems entirely crazy.
> 
> Am I missing something? Because it looks *completely* broken.
> 
> I've said this before, and I'll say this again: our kernel config is
> hard on users as-is, and we really shouldn't make it worse by making
> it ask invalid questions or have invalid defaults.

Completely agree. Patch:

https://patchwork.ozlabs.org/project/netfilter-devel/patch/20220804000843.86722-1-pablo@netfilter.org/

Thanks for reviewing.



  reply	other threads:[~2022-08-04  0:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-03 10:14 [GIT PULL] Networking for 6.0 Paolo Abeni
2022-08-03 23:35 ` Linus Torvalds
2022-08-04  9:43   ` Drewek, Wojciech
2022-08-03 23:50 ` pr-tracker-bot
2022-08-03 23:52 ` Linus Torvalds
2022-08-04  0:11   ` Pablo Neira Ayuso [this message]
2022-08-04  0:27     ` Linus Torvalds
2022-08-04  0:39       ` Pablo Neira Ayuso
2022-08-04  4:17 ` Linus Torvalds
2022-08-04  9:13   ` Kalle Valo
2022-08-05 14:22     ` Kalle Valo
2022-08-05 16:34       ` Linus Torvalds
2022-08-08  8:14         ` Kalle Valo
2022-08-08 18:33           ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YusOpd6IuLB29LHs@salvia \
    --to=pablo@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=ozsh@nvidia.com \
    --cc=pabeni@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vladbu@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.