Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3164 bytes --]

On 03/21/2012 01:08 PM, Kurt Van Dijck wrote:
> On Wed, Mar 21, 2012 at 12:43:31PM +0100, Marc Kleine-Budde wrote:
>> On 03/21/2012 12:05 PM, Kurt Van Dijck wrote:
>>> Hi Sebastian,
>>>
>>> On the ICC, Bosch presentend CANFD[1].
>>> I'll share my interpretation:
>>> * CANFD is meant as a successor of CAN.
>>> * it addressess data throughput in 2 ways:
>>>   - higher data bitrate for data using a second
>>>     set of bittimings.
>>>   - longer message payload (up to 64 byte).
>>> * it remains very compatible to CAN, although _NOT_
>>>   bus-compatible (ie. you put it on a seperate bus).
>>
>> This means:
>> You cannot have a Bus using one (or more) of the CAN-FD features with
>> non CAN-FD compatible nodes.
>>
>> From my point of view this can be translated into a new property in
>> "can.ctrlmode_supported". A device adds "CAN_CTRLMODE_CANFD" if it
>> supports CAN FD mode. Then it's a global setting to put the device into
>> CAN FD mode.
> Yep, good idea.
>>
>> In the TX path the stack should refuse the new 64 byte frames at all or
>> frames with dlc > 8 byes if the device is not in CAN FD mode.
> I did not yet do such think since I did not implement CANFD on device level yet.
> I think can_send() is a good place to put such tricks.

Yes, for CAN_RAW, but in the driver we must extend the
"can_dropped_invalid_skb()" function, too: drop frames with dlc >8 if
device is not in CAN FD mode.

Further, there are only certain dlc values allowed for CAN FD. We must
decide if the enforce these values or simply do padding with "0" somewhere.

>> What about
>> the RX path? We can just use the new 64 byte CAN frames and have to
>> tweak the CAN_RAW if the read from the userspace is too small.
> You may have noticed that I did not need to do anything
> in the receive path in order to receive longer frames.
> 
> look at this block in raw_recvmsg:
> 
> 	if (size < skb->len)
> 		msg->msg_flags |= MSG_TRUNC;
> 	else
> 		size = skb->len;
> 
> Even today already, you can recv. with smaller or bigger buffers than
> the actual CAN frame.

...using an unmodified mainline kernel?

> I did try to let the kernel work with 64bytes, and still use
> 8byte frames in userspace. that all works, especially since noone
> seems to test for MSG_TRUNC.

Have you checked _all_ CAN userspace applications? :P
We cannot break the ABI.

> An improvement that just crosses my mind is something like:
> 
> 	int real_len;
> 
> 	real_len = offsetof(struct can_frame, data) +
> 		((struct can_frame)skb->data)->dlc;
> 	if (size < real_len)
> 		msg->msg_flags |= MSG_TRUNC;
> 	else
> 		size = real_len;
> 
> Such would modify the ABI a bit more, but prevents copying unused bytes
> and avoids unnecessary MSG_TRUNC flags.

I suggest to copy the full 8 bytes of data if dlc <= 8, for full ABI
compatibility.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]