| Message ID | 20220610143009.323579-1-mailhol.vincent@wanadoo.fr (mailing list archive) |
|---|---|
| Headers | show |
| Series | can: refactoring of can-dev module and of Kbuild | expand |
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@hartkopp.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 >
Looks good to me. Thanks Vincent for this clean-up, and thanks to
everyone involved for the discussion, reviews, and testing!
Acked-by: Max Staudt <max@enpas.org>
On 10.06.2022 23:30:02, 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. > > * 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 Applied to can-next/master....as a merge with the above message! Congrats on this series and the first ever merge to the linux-can branch! regards, Marc
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. * 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