Re: [PATCH v3 5/5] can: allow to change the device mtu for CAN FD capable devices

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 15.02.2014 17:59, Marc Kleine-Budde wrote:

> please add a cover letter (--compose option to git send-email) when
> sending a new version of the series, briefly listing your changes (one
> line per change).

Will try :-)


> Please add a patch description to patch 4, as in the kernel it's
> considered bad style to have patches without a description.

Will do.


> On 02/15/2014 05:32 PM, Oliver Hartkopp wrote:
>> The configuration for CAN FD depends on CAN_CTRLMODE_FD enabled in the driver
>> specific ctrlmode_supported capabilities.
>>
>> The configuration can be done either with the 'fd { on | off }' option in the
>> 'ip' tool from iproute2 or by setting the CAN netdevice MTU to CAN_MTU (16) or
>> to CANFD_MTU (72).
> 
> Please don't add two possibilities to activate canfd on a CAN network
> card. There should only be one.

Yes it /should/ be like this.
I thought about it some time but I didn't find a better solution.

The problem is that dev->mtu is the switch for the skb->len whether CAN frames
or CAN FD frames are generated or received.

The MTU can be modified by 'ip' (independently from CAN specific settings).

OTOH the only way the CAN devices provide their capabilities is the
ctrlmode_supported bitfield - and it does not make sense to provide
CAN_CTRLMODE_FD in ctrlmode_supported and do not provide the configuration for it.

If you would skip the CAN_CTRLMODE_FD bit in ctrlmode_supported there's no way
to identify a CAN FD capable device. You could only try to set the MTU to
CANFD_MTU and see, if it fails. Not a good solution either.


>> +EXPORT_SYMBOL_GPL(can_change_mtu);
> 
> Who will be the user of this function? The individual network drivers?
> 

Yes. It manages the MTU and CAN_CTRLMODE_FD consistency and only allows the
CANFD_MTU when the driver supports it.

Any other things to look at - or is the rest ok so far?

Regards,
Oliver