Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom_valid to skb.c

From: Oliver Hartkopp
Date: Tue May 17 2022 - 09:35:21 EST




On 5/17/22 14:39, Max Staudt wrote:
On Tue, 17 May 2022 14:21:53 +0200
Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:

On 17.05.2022 14:14:04, Max Staudt wrote:
After looking through drivers/net/can/Kconfig I would probably
phrase it like this:

Select CAN devices (hw/sw) -> we compile a can_dev module. E.g.
to handle the skb stuff for vcan's.

Select hardware CAN devices -> we compile the netlink stuff into
can_dev and offer CAN_CALC_BITTIMING and CAN_LEDS to be compiled
into can_dev too.

In the latter case: The selection of flexcan, ti_hecc and
mcp251xfd automatically selects CAN_RX_OFFLOAD which is then also
compiled into can_dev.

Would that fit in terms of complexity?

IMHO these should always be compiled into can-dev. Out of tree
drivers are fairly common here, and having to determine which kind
of can-dev (stripped or not) the user has on their system is a
nightmare waiting to happen.

I personally don't care about out-of-tree drivers.

I know that this is the official stance in the kernel.

But out-of-tree drivers do happen on a regular basis, even when
developing with the aim of upstreaming. And if a developer builds a
minimal kernel to host a CAN driver, without building in-tree hardware
CAN drivers, then can-dev will be there but behave differently from
can-dev in a full distro. Leading to heisenbugs and wasting time. The
source of heisenbugs really are the suggested *hidden* Kconfigs.


On another note, is the module accounting overhead in the kernel for
two new modules with relatively little code in each, code that almost
always is loaded when CAN is used, really worth it?

Oh, I didn't want to introduce two new kernel modules but to have can_dev in different 'feature levels'.

I would assume a distro kernel to have everything enabled with a full featured can_dev - which is likely the base for out-of-tree drivers too.

But e.g. the people that are running Linux instances in a cloud only using vcan and vxcan would not need to carry the entire infrastructure of CAN hardware support and rx-offload.

Best regards,
Oliver