netdev - Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <43768ff7-71f8-a6c3-18f8-28609e49eedd@hartkopp.net>
Date:   Wed, 18 May 2022 15:10:44 +0200
From:   Oliver Hartkopp <socketcan@...tkopp.net>
To:     Vincent MAILHOL <mailhol.vincent@...adoo.fr>
Cc:     Max Staudt <max@...as.org>, Marc Kleine-Budde <mkl@...gutronix.de>,
        linux-can@...r.kernel.org, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and
 can_skb_headroom_valid to skb.c

On 18.05.22 14:03, Vincent MAILHOL wrote:
> I didn't think this would trigger such a passionate discussion!

:-D

Maybe your change was the drop that let the bucket run over ;-)

>> 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.
> 
> Are there really some people running custom builds of the Linux kernel
> in a cloud environment? The benefit of saving a few kilobytes by not
> having to carry the entire CAN hardware infrastructure is blown away
> by the cost of having to maintain a custom build.

When looking to the current Kconfig and Makefile content in 
drivers/net/can(/dev) there is also some CONFIG_CAN_LEDS which "depends 
on BROKEN" and builds a leds.o from a non existing leds.c ?!?

Oh leds.c is in drivers/net/can/leds.c but not in 
drivers/net/can/dev/leds.c where it could build ... ?

So what I would suggest is that we always build a can-dev.ko when a CAN 
driver is needed.

Then we have different options that may be built-in:

1. netlink hw config interface
2. bitrate calculation
3. rx-offload
4. leds

E.g. having the netlink interface without bitrate calculation does not 
make sense to me too.

> I perfectly follow the idea to split rx-offload. Integrators building
> some custom firmware for an embedded device might want to strip out
> any unneeded piece. But I am not convinced by this same argument when
> applied to v(x)can.

It does. I've seen CAN setups (really more than one or two!) in VMs and 
container environments that are fed by Ethernet tunnels - sometimes also 
in embedded devices.

> A two level split (with or without rx-offload) is what makes the most
> sense to me.
> 
> Regardless, having the three level split is not harmful. And because
> there seems to be a consensus on that, I am fine to continue in this
> direction.

Thanks!

Should we remove the extra option for the bitrate calculation then?

And what about the LEDS support depending on BROKEN?
That was introduced by commit 30f3b42147ba6f ("can: mark led trigger as 
broken") from Uwe as it seems there were some changes in 2018.

Best regards,
Oliver