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:

quoted

On 03/21/2012 12:05 PM, Kurt Van Dijck wrote:

quoted

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.

quoted

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.

quoted

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   |