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;
Date:   Tue, 17 May 2022 06:12:21 +0200
From:   Max Staudt <max@enpas.org>
To:     Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
Cc:     Oliver Hartkopp <socketcan@hartkopp.net>,
        Marc Kleine-Budde <mkl@pengutronix.de>,
        linux-can@vger.kernel.org, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org
Subject: Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and
 can_skb_headroom_valid to skb.c
Message-ID: <20220517061221.012e36d1.max@enpas.org>
In-Reply-To: <CAMZ6RqKZMHXB7rQ70GrXcVE7x7kytAAGfE+MOpSgWgWgp0gD2g@mail.gmail.com>
References: <20220513142355.250389-1-mailhol.vincent@wanadoo.fr>
        <20220514141650.1109542-1-mailhol.vincent@wanadoo.fr>
        <20220514141650.1109542-4-mailhol.vincent@wanadoo.fr>
        <7b1644ad-c117-881e-a64f-35b8d8b40ef7@hartkopp.net>
        <CAMZ6RqKZMHXB7rQ70GrXcVE7x7kytAAGfE+MOpSgWgWgp0gD2g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 17 May 2022 10:50:16 +0900
Vincent MAILHOL <mailhol.vincent@wanadoo.fr> wrote:

> My concern is: what would be the merrit? If we do not split, the users
> of slcan and v(x)can would have to load the can-dev module which will
> be slightly bloated for their use, but is this really an issue? I do
> not see how this can become a performance bottleneck, so what is the
> problem?
> I could also argue that most of the devices do not depend on
> rx-offload.o. So should we also split this one out of can-dev on the
> same basis and add another module dependency?
> The benefit (not having to load a bloated module for three drivers)
> does not outweigh the added complexity: all hardware modules will have
> one additional modprobe dependency on the tiny can-skb module.
> 
> But as said above, I am not fully opposed to the split, I am just
> strongly divided. If we go for the split, creating a can-skb module is
> the natural and only option I see.
> If the above argument does not convince you, I will send a v3 with
> that split.

I originally replied to PATCHv2 in agreement with what Vincent said in
the first part - I agree that simply moving this code into can-dev and
making v(x)can/slcan depend on it is the straightforward thing to do.

Having yet another kernel module for a tiny bit of code adds more
unnecessary complexity IMHO. And from a user perspective, it seems
fairly natural to have can-dev loaded any time some can0, slcan0,
vcan0, etc. pops up.

Finally, embedded applications that are truly short on memory are
likely using a "real" CAN adapter, and hence already have can-dev
loaded anyway.

I think it's okay to just move it to can-dev and call it a day :)


Max