CAN FD support

I generated this diff by 'git diff fb7944b' on the repository

https://gitorious.org/~hartkopp/linux-can/hartkopps-linux-can-next

Regards,
Oliver

 drivers/net/can/dev.c    |   35 ++++++++++++++
 drivers/net/can/vcan.c   |   27 ++++++++---
 include/linux/can.h      |   51 ++++++++++++++++++++-
 include/linux/can/core.h |    4 -
 include/linux/can/dev.h  |   33 ++++++++++----
 include/linux/can/raw.h  |    3 -
 include/linux/if_ether.h |    3 -
 net/can/af_can.c         |  110 +++++++++++++++++++++++++++++++++++------------
 net/can/raw.c            |   41 ++++++++++++++++-
 9 files changed, 257 insertions(+), 50 deletions(-)

---

include/linux/can/raw.h +2 -1

index 781f3a3..5448c0f 100644
@@ -23,7 +23,8 @@ enum {
 	CAN_RAW_FILTER = 1,	/* set 0 .. n can_filter(s)          */
 	CAN_RAW_ERR_FILTER,	/* set filter for error frames       */
 	CAN_RAW_LOOPBACK,	/* local loopback (default:on)       */
-	CAN_RAW_RECV_OWN_MSGS	/* receive my own msgs (default:off) */
+	CAN_RAW_RECV_OWN_MSGS,	/* receive my own msgs (default:off) */
+	CAN_RAW_FD_FRAMES,	/* use struct canfd_frame (default:off) */
 };
 
 #endif

net/can/raw.c +1 -0

index cde1b4a..2473e6e 100644
@@ -82,6 +82,7 @@ struct raw_sock {
 	struct notifier_block notifier;
 	int loopback;
 	int recv_own_msgs;
+	int fd_frames;
 	int count;                 /* number of active filters */
 	struct can_filter dfilter; /* default/single filter */
 	struct can_filter *filter; /* pointer to filter(s) */

net/can/raw.c +24 -0

+	if (ro->fd_frames) {
+		if (unlikely(oskb->len != CANFD_MTU))
+			return;
+	} else {
+		if (unlikely(oskb->len != CAN_MTU))
+			return;
+	}
+
 	/* clone the given skb to be able to enqueue it into the rcv queue */
 	skb = skb_clone(oskb, GFP_ATOMIC);
 	if (!skb)

@@ -291,6 +301,7 @@ static int raw_init(struct sock *sk)
 	/* set default loopback behaviour */
 	ro->loopback         = 1;
 	ro->recv_own_msgs    = 0;
+	ro->fd_frames        = 0;
 
 	/* set notifier */
 	ro->notifier.notifier_call = raw_notifier;

@@ -569,6 +580,15 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
 
 		break;
 
+	case CAN_RAW_FD_FRAMES:
+		if (optlen != sizeof(ro->fd_frames))
+			return -EINVAL;
+
+		if (copy_from_user(&ro->fd_frames, optval, optlen))
+			return -EFAULT;
+
+		break;
+
 	default:
 		return -ENOPROTOOPT;
 	}

@@ -627,6 +647,12 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
 		val = &ro->recv_own_msgs;
 		break;
 
+	case CAN_RAW_FD_FRAMES:
+		if (len > sizeof(int))
+			len = sizeof(int);
+		val = &ro->fd_frames;
+		break;
+
 	default:
 		return -ENOPROTOOPT;
 	}

net/can/raw.c +13 -3

-	if (size != sizeof(struct can_frame))
-		return -EINVAL;
+	if (ro->fd_frames) {
+		if (unlikely(size != CANFD_MTU))
+			return -EINVAL;
+	} else {
+		if (unlikely(size != CAN_MTU))
+			return -EINVAL;
+	}
 
 	dev = dev_get_by_index(&init_net, ifindex);
 	if (!dev)
 		return -ENXIO;
 
+	/* make sure the created CAN frame can pass the CAN netdevice */
+	if (unlikely(size > dev->mtu)) {
+		err = -EMSGSIZE;
+		goto put_dev;
+	}
+
 	skb = sock_alloc_send_skb(sk, size, msg->msg_flags & MSG_DONTWAIT,
 				  &err);
 	if (!skb)
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-can" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

I like the inversion of HDR & EDL bits. It make CAN & CANFD even more compatible.

I still don't see the contribution of ETH_P_CANFD.

Another issue I'm still struggling with: can_raw is differentiating between CAN & CANFD with CAN_RAW_FD_FRAMES sockopt.

IMO, at most 1 property/method shall exist to differentiate CAN from CANFD. (Preferrably, no method should have been necessary).

I see that 2 struct are in place now, with sizes CAN_MTU & CANFD_MTU. This alone is IMO enough to differentiate.

Next to this property, an _extra_ sockopt is introduced. This sockopt actually implements a filter, at can_raw level, *and* is a second way of telling you want to use CANFD.

I understood that mixing CAN & CANFD on 1 bus is possible on CANFD chip level, and we do want to support those features by using 2 structs
that in fact determine the CAN type. If an application want to mix CAN & CANFD, it is currently not possible without opening another socket, thereby loosing the proper sequence of events.

IMHO, the CAN_RAW_FD_FRAMES is not justifyable towards userspace applications.

I put my view in the code down below, besides the elimination of the 'fd_frames' member in struct raw_sock, and the corresponding CAN_RAW_FD_FRAMES sockopt.

IMO, the old behaviour (that is written in stone) is still preserved for 99.999%, _and_ no difficult sockopts are needed.
Difficult means in this case: it became not straightforward to predict which frames got received.

Hello Kurt,

thanks for your feedback.

Indeed i'm not totally happy with the current hard switch too! Let's see if we can develop something less intrusive but still clean in design.

The question is how to cope with this scenario:

A legacy application wants to communicate via can0 not knowing anything about CAN FD. This legacy app can only read and write struct can_frames and does not know about different data rates in the data field.

1. For this scenario we need to define (globally!?) if legacy apps generated (short) CAN frames use the slow or high data rate in the data field when transmitted via that specific CAN FD interface.

2. in any case legacy applications must not get CAN frames with a DLC > 8

3. legacy applications assume struct can_frames on read and write. There should no MSG_TRUNC been set in msg flags when receiving a can_frame.

OTOH we will have new applications that are aware of CAN FD. These new applications may also be able to deal with two different sizes of structs for read and write operations (can_frame and canfd_frame) - if we specify it so.

Therefore we would need at least some flag if the application is CAN FD aware or not. Btw. i'll think about a better simultaneous access of legacy and new apps to the CAN interfaces.

Some more comments inline:

If you want to exchange canfd_frames you can add a new sockopt or you introduce a new protocol (e.g. CAN_RAWFD). The latter has no additional property/method - besides the different protocol number ... %-)

Would you prefer that solution?

Quoted 5 lines

I understood that mixing CAN & CANFD on 1 bus is possible on CANFD chip level, and we do want to support those features by using 2 structs
that in fact determine the CAN type. If an application want to mix CAN & CANFD, it is currently not possible without opening another socket, thereby loosing the proper sequence of events.

Beware of providing a needless flexibility here.
Once the CAN driver is switched to the CAN FD mode you can send and receive all types of frames. You can send normal CAN frames inside struct canfd_frame too! So when you open a socket - and you are a CAN FD aware app - you switch to CAN_RAW_FD_FRAMES and you are free to send/recv whatever you like.

NB: You can only switch the CAN interface MTU when the interface is down.

Quoted 5 lines

IMHO, the CAN_RAW_FD_FRAMES is not justifyable towards userspace applications.

Quoted 7 lines

I put my view in the code down below, besides the elimination of the 'fd_frames' member in struct raw_sock, and the corresponding CAN_RAW_FD_FRAMES sockopt.

IMO, the old behaviour (that is written in stone) is still preserved for 99.999%, _and_ no difficult sockopts are needed.
Difficult means in this case: it became not straightforward to predict which frames got received.

Indeed :-)

And that's not acceptable for legacy apps.

Quoted 7 lines · Oliver Hartkopp

A legacy application wants to communicate via can0 not knowing anything about CAN FD. This legacy app can only read and write struct can_frames and does not know about different data rates in the data field.

1. For this scenario we need to define (globally!?) if legacy apps generated (short) CAN frames use the slow or high data rate in the data field when transmitted via that specific CAN FD interface.

IMHO, slow datarate matches the most closely to regular CAN2.0B.

Quoted 2 lines

2. in any case legacy applications must not get CAN frames with a DLC > 8

Or cut the DLC to 8, and drop the remainder of the message? This changes the actual can(fd)_frame. That is acceptable since you're running legacy apps on CANFD bus.

An alternative could be to drop CANFD frames in can_recv() itself. This may sound inefficient, but remind you're running legacy apps on CANFD busses _WITH_ CANFD frames on it.

I'd rather choose a suboptimal compatibility mode than adding code to explicitely tweak the subsystem.

Quoted 3 lines

3. legacy applications assume struct can_frames on read and write. There should no MSG_TRUNC been set in msg flags when receiving a can_frame.

see 2. MSG_TRUNC would be set only with sizes != CAN_MTU & != CANFD_MTU.

This level of compatibility involves modifying the actual received CAN frame when CANFD frames are received on a socket using CAN_MTU. I accept this compatibility for sake of preserving the API that DLC <= 8 for such frames.

Quoted 5 lines

OTOH we will have new applications that are aware of CAN FD. These new applications may also be able to deal with two different sizes of structs for read and write operations (can_frame and canfd_frame) - if we specify it so.

they (ideally) always do:

	ret = recv(cansock, &canfd_frame, CANFD_MTU);

and recv() returns CANFD_MTU _or_ CAN_MTU.
That sounds acceptible.

Quoted 5 lines

Beware of providing a needless flexibility here.
Once the CAN driver is switched to the CAN FD mode you can send and receive all types of frames. You can send normal CAN frames inside struct canfd_frame too! So when you open a socket - and you are a CAN FD aware app - you switch to CAN_RAW_FD_FRAMES and you are free to send/recv whatever you like.

Now this is interesting. I really tought that a struct can_frame and a struct canfd_frame with similar content still produce a (slight) different bitstream on the wire, i.e. a struct can_frame would produce a CAN2.0b bitstream that is different than the CANFD bitstream.

I did think that this difference in bitstream/protocol would map on a different struct.
Where am I wrong here?

Even if I'm not convinced, I now better understand your exclusive-or ... This definitely needs clarification.

Even that I avoided this 0.001% difference above in (2.), I'll still make a nasty comparison here.

If an ABI was 100% the same, I argue that if a legacy app does

	int value = 1;
	setsockopt(cansock, 5, &value, sizeof(value));

Then you also are changing the behaviour of the legacy application now.

No problem, let's create CANFD_RAW. But even that is not 100% compatible.

Ok, this is unlikely to be a problem, but 'unlikely' > 0%. You see my point? A legacy app doing

	recv(cansock, &can_frame, <CANFD_MTU hardcoded>);

may break legacy apps, but this is considered unlikely, and such crap should not prevent development of good stuff.

Just think about it.

Quoted 9 lines · Kurt Van Dijck

Quoted 7 lines · Oliver Hartkopp

A legacy application wants to communicate via can0 not knowing anything about CAN FD. This legacy app can only read and write struct can_frames and does not know about different data rates in the data field.

1. For this scenario we need to define (globally!?) if legacy apps generated (short) CAN frames use the slow or high data rate in the data field when transmitted via that specific CAN FD interface.

Quoted 1 line

IMHO, slow datarate matches the most closely to regular CAN2.0B.

You can not rely on this.

The most probable migration scenario is to increase the payload data rate without changing any application.

Quoted 7 lines

Quoted 3 lines

2. in any case legacy applications must not get CAN frames with a DLC > 8

Or cut the DLC to 8, and drop the remainder of the message? This changes the actual can(fd)_frame. That is acceptable since you're running legacy apps on CANFD bus.

Quoted 3 lines

An alternative could be to drop CANFD frames in can_recv() itself. This may sound inefficient, but remind you're running legacy apps on CANFD busses _WITH_ CANFD frames on it.

I have a patch for that later in this mail.

Quoted 4 lines

I'd rather choose a suboptimal compatibility mode than adding code to explicitely tweak the subsystem.

CAN_RAW_FD_FRAMES *is* this compatibility switch :-)

Quoted 4 lines

Quoted 4 lines

3. legacy applications assume struct can_frames on read and write. There should no MSG_TRUNC been set in msg flags when receiving a can_frame.

Quoted 5 lines

see 2. MSG_TRUNC would be set only with sizes != CAN_MTU & != CANFD_MTU.

This level of compatibility involves modifying the actual received CAN frame when CANFD frames are received on a socket using CAN_MTU. I accept this compatibility for sake of preserving the API that DLC <= 8 for such frames.

Yes. I did exactly this in my patch this morning.

If you get a CANFD frame to be passed to a legacy socket

1. check id DLC <= 8 (in raw_rcv())
2. cut the length from CANFD_MTU to CAN_MTU (in raw_recvmsg())

Quoted 2 lines

Quoted 2 lines

Yes. Especially we need to specify how the CAN controller should send can_frames passed to its CAN driver. Should it been send with or without EDL bit, and should it been send with high data rate or not.

This is what i wanted to suggest with the global CANFD driver settings.

Quoted 7 lines

Ok, this is unlikely to be a problem, but 'unlikely' > 0%. You see my point? A legacy app doing

	recv(cansock, &can_frame, <CANFD_MTU hardcoded>);

may break legacy apps, but this is considered unlikely, and such crap should not prevent development of good stuff.

Quoted 1 line

Just think about it.

Done :-)

Legacy crap that was technically possible to do just has to be supported.

Here's my patch:

Changes to the previous version:

CANFD aware apps set CAN_RAW_FD_FRAMES to enable longer MTUs (not new).

1. CANFD aware apps may get CAN_MTU and CANFD_MTU frames on rx
2. CANFD aware apps may send CAN_MTU and CANFD_MTU frames on tx
3. legacy apps can receive CAN_MTU and CANFD_MTU frames if the DLC <= 8 (the possible CANFD_MTU is cut down to CAN_MTU to preserve the ABI)

Effects to the CAN netdev driver:

Legacy operation (CAN_MTU)
- send and receive struct can_frames

CANFD operation (CANFD_MTU and CAN_MTU)
- you can send CANFD_MTU and CAN_MTU frames
(a default setting (EDL, HDR) for CAN_MTU frames needs to be specified)
- the driver generates always canfd_frames on reception

Quoted 5 lines · Oliver Hartkopp

Quoted 5 lines

Quoted 1 line

2. in any case legacy applications must not get CAN frames with a DLC > 8

Or cut the DLC to 8, and drop the remainder of the message? This changes the actual can(fd)_frame. That is acceptable since you're running legacy apps on CANFD bus.

Quoted 6 lines

Quoted 3 lines

An alternative could be to drop CANFD frames in can_recv() itself. This may sound inefficient, but remind you're running legacy apps on CANFD busses _WITH_ CANFD frames on it.

I have a patch for that later in this mail.

waaw. You're running _with_ HDR bit on :-) ?

Quoted 5 lines

Yes. Especially we need to specify how the CAN controller should send can_frames passed to its CAN driver. Should it been send with or without EDL bit, and should it been send with high data rate or not.

This is what i wanted to suggest with the global CANFD driver settings.

netlink-wise, I would have considered this a device setting.

Quoted 17 lines

Here's my patch:

Changes to the previous version:

CANFD aware apps set CAN_RAW_FD_FRAMES to enable longer MTUs (not new).

1. CANFD aware apps may get CAN_MTU and CANFD_MTU frames on rx
2. CANFD aware apps may send CAN_MTU and CANFD_MTU frames on tx
3. legacy apps can receive CAN_MTU and CANFD_MTU frames if the DLC <= 8 (the possible CANFD_MTU is cut down to CAN_MTU to preserve the ABI)

Effects to the CAN netdev driver:

Legacy operation (CAN_MTU)
- send and receive struct can_frames

Quoted 4 lines

CANFD operation (CANFD_MTU and CAN_MTU)
- you can send CANFD_MTU and CAN_MTU frames
(a default setting (EDL, HDR) for CAN_MTU frames needs to be specified)
- the driver generates always canfd_frames on reception

Interesting.
How does a driver makes the difference between received CAN2.0B frames & CANFD frames?

I think it makes sense, equally to the send() path, to allow the driver to generate can_frame _or_ canfd_frame on reception, depending on the bitstream (if possible).

A CANFD application will then be able to distinguish between them based on the return value of recv_msg (CAN_MTU or CANFD_MTU, even with payload <= 8).

Did I mention I very much like this additional patch? Well done.

Quoted 7 lines · Kurt Van Dijck

Quoted 5 lines · Oliver Hartkopp

Quoted 5 lines

Quoted 1 line

2. in any case legacy applications must not get CAN frames with a DLC > 8

Or cut the DLC to 8, and drop the remainder of the message? This changes the actual can(fd)_frame. That is acceptable since you're running legacy apps on CANFD bus.

Quoted 3 lines

Quoted 2 lines

I have a patch for that later in this mail.

waaw. You're running _with_ HDR bit on :-) ?

Quoted 10 lines

Quoted 10 lines

1. CANFD aware apps may get CAN_MTU and CANFD_MTU frames on rx
2. CANFD aware apps may send CAN_MTU and CANFD_MTU frames on tx
3. legacy apps can receive CAN_MTU and CANFD_MTU frames if the DLC <= 8 (the possible CANFD_MTU is cut down to CAN_MTU to preserve the ABI)

Effects to the CAN netdev driver:

Legacy operation (CAN_MTU)
- send and receive struct can_frames

Quoted 7 lines

Quoted 4 lines

CANFD operation (CANFD_MTU and CAN_MTU)
- you can send CANFD_MTU and CAN_MTU frames
(a default setting (EDL, HDR) for CAN_MTU frames needs to be specified)
- the driver generates always canfd_frames on reception

Interesting.
How does a driver makes the difference between received CAN2.0B frames & CANFD frames?

As you always generate canfd_frames in this operating mode you can express this information within canfd_frame.flags.

Quoted 3 lines

I think it makes sense, equally to the send() path, to allow the driver to generate can_frame _or_ canfd_frame on reception, depending on the bitstream (if possible).

As written above this can be done with a singe canfd_frame.

A legacy app would get the can_frame as-is when DLC <= 8
A CANFD frame can evaluate the bits if they are at least interesting for the app. For a detailed 'candump' output this could be interesting ;-)

Quoted 1 line

Quoted 2 lines

A CANFD application will then be able to distinguish between them based on the return value of recv_msg (CAN_MTU or CANFD_MTU, even with payload <= 8).

There's no reason to provide a CAN_MTU frame to a CANFD application once the driver is switched to CANFD-mode.

The CANFD app only gets the old-style CAN_MTU frames when the CAN interface is not in CANFD mode.

Quoted 1 line

Quoted 2 lines

Did I mention I very much like this additional patch? Well done.

Thanks to you! I was not really happy with the hard exclusive-or switch and your remarks made me thinking about the simultaneous access with new and legacy apps again.

I think cutting the canfd_frames down to can_frames when
the content is generally usable for the legacy app makes the migration pretty handy :-)

Will send a v4 patch for the ongoing discussion.

Regards,
Oliver

Quoted 4 lines · Oliver Hartkopp

Quoted 3 lines

Quoted 2 lines

I have a patch for that later in this mail.

waaw. You're running _with_ HDR bit on :-) ?

I meant you have a "High data rate" yourself. It was meant as some form of humor to indicate my supprise of you generating a new patch very quick ...

Quoted 2 lines

There's no reason to provide a CAN_MTU frame to a CANFD application once the driver is switched to CANFD-mode.

I think there is a reason. If a CAN2.0B frame was on the wire, generating a can_frame rather than a canfd_frame better reflects what was on the wire.

I still think a can_frame & canfd_frame with same data differ on the wire.

Quoted 3 lines

Thanks to you! I was not really happy with the hard exclusive-or switch and your remarks made me thinking about the simultaneous access with new and legacy apps again.

You're not tired of me then :-)

Quoted 3 lines

I think cutting the canfd_frames down to can_frames when
the content is generally usable for the legacy app makes the migration pretty handy :-)

My first suggestion was to also cut longer CANFD frames down to 8 bytes (thereby loosing data!).
Remember it's still a compatibility mode. I think that would be acceptable.

Quoted 6 lines

Quoted 3 lines

There's no reason to provide a CAN_MTU frame to a CANFD application once the driver is switched to CANFD-mode.

I think there is a reason. If a CAN2.0B frame was on the wire, generating a can_frame rather than a canfd_frame better reflects what was on the wire.

Quoted 1 line

I still think a can_frame & canfd_frame with same data differ on the wire.

Yes. But you can express this difference with the CANFD_NOEDL bit in canfd_frame.flags:

CANFD_NOEDL == 1 => standard CAN 2.0B frame CANFD_NOEDL == 0 => CAN FD frame (whatever DLC 0-F is used)

Quoted 9 lines

Quoted 6 lines

I think cutting the canfd_frames down to can_frames when
the content is generally usable for the legacy app makes the migration pretty handy :-)

My first suggestion was to also cut longer CANFD frames down to 8 bytes (thereby loosing data!).
Remember it's still a compatibility mode. I think that would be acceptable.

At the point where CAN FD frames with a DLC > 8 become relevant for applications a new CANFD aware application *should* come into service.

Silently cutting payload data is a bad idea.