Re: [RFC] CAN FD support part 2 - ideas and commented source

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 03.05.2012 14:37, Kurt Van Dijck wrote:

> Oliver,
> 
> Your proposal looks more elaborate than my first one :-)
> 
> In this email, I'll restrict myself to discussing ETH_P_CANFD.
> 
> On Thu, May 03, 2012 at 01:14:58PM +0200, Oliver Hartkopp wrote:
>> Hi all,
>>
>> in this mail i would like to document the ideas for CAN FD support pointing
>> directly to the changes inside the source.
>>
> 
> [........]
>>
>> That's it.
> 
> using dev's MTU is a very good idea!


Tnx!

> 
> Restricting the API to use one of 2 MTU's makes life easier.
> 
>>
>> Any objections / better ideas?
> I don't see benefit in introducing ETH_P_CANFD.
> Using the skb's payload length differentiates enough between CAN_MTU & CANFD_MTU.
> Why not using that info. IMHO, ETH_P_CANFD duplicates the info.


This has a big advantage when thinking about wireshark & friends.

You can look into the eth protocol and know the skb data layout.
The length information is some kind of implicit knowledge.

> 
> This part proves my point:
> @@ -69,13 +73,23 @@ static inline int can_dropped_invalid_skb(struct net_device *dev,
>  {
>  	const struct can_frame *cf = (struct can_frame *)skb->data;
>  
> -	if (unlikely(skb->len != sizeof(*cf) || cf->can_dlc > 8)) {
> This check should have become:
> +	if (unlikely(skb->len != dev->mut) || (cf->can_dlc > mtu_to_max_can_dlc(dev->mtu))
> 
> Instead of all the if then else flows here.
> A lookup table to use by mtu_to_max_can_dlc is quite straight-forward:
> static const int mtu_to_max_can_dlc_table[] = {
> 	[sizeof(struct can_frame) = CAN_MAX_DLC,
> 	[sizeof(struct canfd_frame) = CANFD_MAX_DLC,
> };


Nice idea.

How big is mtu_to_max_can_dlc_table ??
sizeof(struct canfd_frame) * sizeof(int)

Regards,
Oliver