Re: [PATCH 08/17] can: add ISO 15765-2:2016 transport protocol

On Sat, 10 Oct 2020 16:29:06 +0200 Oliver Hartkopp wrote:

quoted

quoted

diff --git a/net/can/Kconfig b/net/can/Kconfig
index 25436a715db3..021fe03a8ed6 100644
--- a/net/can/Kconfig
+++ b/net/can/Kconfig

@@ -55,6 +55,19 @@ config CAN_GW
  
  source "net/can/j1939/Kconfig"
  
+config CAN_ISOTP
+	tristate "ISO 15765-2:2016 CAN transport protocol"
+	default y  

default should not be y unless there is a very good reason.
I don't see such reason here. This is new functionality, users
can enable it if they need it.

Yes. I agree. But there is a good reason for it.
The ISO 15765-2 protocol is used for vehicle diagnosis and is a *very* 
common CAN bus use case.

More common than j1939? (Google uses words like 'widely used' and
'common' :)) To give you some perspective we don't enable Ethernet 
vlan support by default, vlans are pretty common, too.

The config item only shows up when CONFIG_CAN is selected and then ISO 
15765-2 should be enabled too. I have implemented and maintained the 
out-of-tree driver for ~12 years now and the people have real problems 
using e.g. Ubuntu with signed kernel modules when they need this protocol.

Therefore the option should default to 'y' to make sure the common 
distros (that enable CONFIG_CAN) enable ISO-TP too.

I understand the motivation, but Linus had pushed back on defaulting to
'y' many times over the years, please read this:

https://lkml.org/lkml/2012/1/6/354

This really must not pop up on his screen as default 'y' when he does
an oldconfig after pulling the networking tree..