netdev - Re: [PATCH v5 4/7] can: Kconfig: add CONFIG_CAN_RX

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]

Date:   Wed, 8 Jun 2022 01:43:54 +0200
From:   Max Staudt <max@...as.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Vincent MAILHOL <mailhol.vincent@...adoo.fr>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        linux-can@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Oliver Hartkopp <socketcan@...tkopp.net>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v5 4/7] can: Kconfig: add CONFIG_CAN_RX_OFFLOAD

On Tue, 7 Jun 2022 15:06:14 -0700
Jakub Kicinski <kuba@...nel.org> wrote:

> On Tue, 7 Jun 2022 18:22:16 +0200 Max Staudt wrote:
> > > Honestly, I am totally happy to have the "default y" tag, the "if
> > > unsure, say Y" comment and the "select CAN_RX_OFFLOAD" all
> > > together.
> > > 
> > > Unless I am violating some kind of best practices, I prefer to
> > > keep it as-is. Hope this makes sense.    
> 
> AFAIU Linus likes for everything that results in code being added to
> the kernel to default to n. If the drivers hard-select that Kconfig
> why bother user with the question at all? My understanding is that
> Linus also likes to keep Kconfig as simple as possible.
> 
> > I wholeheartedly agree with Vincent's decision.
> > 
> > One example case would be users of my can327 driver, as long as it
> > is not upstream yet. They need to have RX_OFFLOAD built into their
> > distribution's can_dev.ko, otherwise they will have no choice but to
> > build their own kernel.  
> 
> Upstream mentioning out-of-tree modules may have the opposite effect 
> to what you intend :( Forgive my ignorance, what's the reason to keep
> the driver out of tree?

None, it's being upstreamed. But even with the best of intentions, it
has been in this process for a long time, and it's still going on!

For some reason, upstream tends to forget about everything that is not
upstream *yet*. I've also convinced Greg K-H to include the
N_DEVELOPMENT ldisc number for this very reason: To allow new drivers
(ldiscs in this case) to be developed comfortably out-of-tree before
they are upstreamed (and then assigned their own ldisc number).

It seems strange to me to magically build some extra features into
can_dev.ko, depending on whether some other .ko files are built in that
very same moment, or not. By "magically", I mean an invisible Kconfig
option. This is why I think Vincent's approach is best here, by making
the drivers a clearly visible subset of the RX_OFFLOAD option in
Kconfig, and RX_OFFLOAD user-selectable.

How about making RX_OFFLOAD a separate .ko file, so we don't have
various possible versions of can_dev.ko?

@Vincent, I think you suggested that some time ago, IIRC?

(I know, I was against a ton of little modules, but I'm changing my
ways here now since it seems to help...)

Max