| 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
| ||
|
Message-ID: <bba88460-4f81-aeb1-db35-9d37c48e120f@hartkopp.net>
Date: Fri, 10 Jun 2022 23:38:09 +0200
From: Oliver Hartkopp <socketcan@...tkopp.net>
To: Vincent Mailhol <mailhol.vincent@...adoo.fr>,
Marc Kleine-Budde <mkl@...gutronix.de>
Cc: linux-can@...r.kernel.org, linux-kernel@...r.kernel.org,
Max Staudt <max@...as.org>, netdev@...r.kernel.org,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH v6 0/7] can: refactoring of can-dev module and of Kbuild
On 10.06.22 16:30, Vincent Mailhol wrote:
> Aside of calc_bittiming.o which can be configured with
> CAN_CALC_BITTIMING, all objects from drivers/net/can/dev/ get linked
> unconditionally to can-dev.o even if not needed by the user.
>
> This series first goal it to split the can-dev modules so that the
> only the needed features get built in during compilation.
> Additionally, the CAN Device Drivers menu is moved from the
> "Networking support" category to the "Device Drivers" category (where
> all drivers are supposed to be).
>
>
> * menu before this series *
>
> CAN bus subsystem support
> symbol: CONFIG_CAN
> |
> +-> CAN Device Drivers
> (no symbol)
> |
> +-> software/virtual CAN device drivers
> | (at time of writing: slcan, vcan, vxcan)
> |
> +-> Platform CAN drivers with Netlink support
> symbol: CONFIG_CAN_DEV
> |
> +-> CAN bit-timing calculation (optional for hardware drivers)
> | symbol: CONFIG_CAN_CALC_BITTIMING
> |
> +-> All other CAN devices drivers
>
> * menu after this series *
>
> Network device support
> symbol: CONFIG_NETDEVICES
> |
> +-> CAN Device Drivers
> symbol: CONFIG_CAN_DEV
> |
> +-> software/virtual CAN device drivers
> | (at time of writing: slcan, vcan, vxcan)
> |
> +-> CAN device drivers with Netlink support
> symbol: CONFIG_CAN_NETLINK (matches previous CONFIG_CAN_DEV)
> |
> +-> CAN bit-timing calculation (optional for all drivers)
> | symbol: CONFIG_CAN_CALC_BITTIMING
> |
> +-> All other CAN devices drivers
> (some may select CONFIG_CAN_RX_OFFLOAD)
> |
> +-> CAN rx offload (automatically selected by some drivers)
> (hidden symbol: CONFIG_CAN_RX_OFFLOAD)
>
> Patches 1 to 5 of this series do above modification.
>
> The last two patches add a check toward CAN_CTRLMODE_LISTENONLY in
> can_dropped_invalid_skb() to discard tx skb (such skb can potentially
> reach the driver if injected via the packet socket). In more details,
> patch 6 moves can_dropped_invalid_skb() from skb.h to skb.o and patch
> 7 is the actual change.
>
> Those last two patches are actually connected to the first five ones:
> because slcan and v(x)can requires can_dropped_invalid_skb(), it was
> necessary to add those three devices to the scope of can-dev before
> moving the function to skb.o.
>
> This design results from the lengthy discussion in [1].
>
> [1] https://lore.kernel.org/linux-can/20220514141650.1109542-1-mailhol.vincent@wanadoo.fr/
>
>
> ** Changelog **
>
> v5 -> v6:
>
> * fix typo in patch #1's title: Kbuild -> Kconfig.
>
> * make CONFIG_RX_CAN an hidden config symbol and modify the diagram
> in the cover letter accordingly.
>
> @Oliver, with CONFIG_CAN_RX_OFFLOAD now being an hidden config,
> that option fully depends on the drivers. So contrary to your
> suggestion, I put CONFIG_CAN_RX_OFFLOAD below the device drivers
> in the diagram.
Yes, fine for me.
I did some Kconfig configuration tests and built the drivers to see
vcan.ko depending on can-dev.ko at module loading the first time ;-)
Nice work.
So for these bits I can provide a
Tested-by: Oliver Hartkopp <socketcan@...tkopp.net>
Thanks,
Oliver
>
> * fix typo in cover letter: CONFIG_CAN_BITTIMING -> CONFIG_CAN_CALC_BITTIMING.
>
> v4 -> v5:
>
> * m_can is also requires RX offload. Add the "select CAN_RX_OFFLOAD"
> to its Makefile.
>
> * Reorder the lines of drivers/net/can/dev/Makefile.
>
> * Remove duplicated rx-offload.o target in drivers/net/can/dev/Makefile
>
> * Remove the Nota Bene in the cover letter.
>
>
> v3 -> v4:
>
> * Five additional patches added to split can-dev module and refactor
> Kbuild. c.f. below (lengthy) thread:
> https://lore.kernel.org/linux-can/20220514141650.1109542-1-mailhol.vincent@wanadoo.fr/
>
>
> v2 -> v3:
>
> * Apply can_dropped_invalid_skb() to slcan.
>
> * Make vcan, vxcan and slcan dependent of CONFIG_CAN_DEV by
> modifying Kbuild.
>
> * fix small typos.
>
> v1 -> v2:
>
> * move can_dropped_invalid_skb() to skb.c instead of dev.h
>
> * also move can_skb_headroom_valid() to skb.c
>
> Vincent Mailhol (7):
> can: Kconfig: rename config symbol CAN_DEV into CAN_NETLINK
> can: Kconfig: turn menu "CAN Device Drivers" into a menuconfig using
> CAN_DEV
> can: bittiming: move bittiming calculation functions to
> calc_bittiming.c
> can: Kconfig: add CONFIG_CAN_RX_OFFLOAD
> net: Kconfig: move the CAN device menu to the "Device Drivers" section
> can: skb: move can_dropped_invalid_skb() and can_skb_headroom_valid()
> to skb.c
> can: skb: drop tx skb if in listen only mode
>
> drivers/net/Kconfig | 2 +
> drivers/net/can/Kconfig | 55 +++++--
> drivers/net/can/dev/Makefile | 17 ++-
> drivers/net/can/dev/bittiming.c | 197 -------------------------
> drivers/net/can/dev/calc_bittiming.c | 202 ++++++++++++++++++++++++++
> drivers/net/can/dev/dev.c | 9 +-
> drivers/net/can/dev/skb.c | 72 +++++++++
> drivers/net/can/m_can/Kconfig | 1 +
> drivers/net/can/spi/mcp251xfd/Kconfig | 1 +
> include/linux/can/skb.h | 59 +-------
> net/can/Kconfig | 5 +-
> 11 files changed, 338 insertions(+), 282 deletions(-)
> create mode 100644 drivers/net/can/dev/calc_bittiming.c
>
Powered by blists - more mailing lists