From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Stephane Grosjean <s.grosjean@peak-system.com>,
"linux-can@vger.kernel.org" <linux-can@vger.kernel.org>
Subject: [RFC] can fd: backward compatibility for CANFD8
Date: Tue, 21 Jan 2014 11:22:11 +0100 [thread overview]
Message-ID: <52DE4A53.9080503@hartkopp.net> (raw)
In-Reply-To: <52DE32FD.5010300@peak-system.com>
On 21.01.2014 09:42, Stephane Grosjean wrote:
> Le 18/01/2014 18:34, Oliver Hartkopp a écrit :
>> The stuff in linux/net/can/ takes care that only applications that know (and
>> enable) CAN FD get CAN frames with a length > 8 byte.
>>
>> E.g. today a CAN FD frame with only 8 bytes (aka CANFD8) can be passed to
>> applications that are not CAN FD capable. In this case the size (MTU)
>> is reduced from CANFD_MTU to CAN_MTU, see:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/net/can/raw.c?id=e2d265d3b587f5f6f8febc0222aace93302ff0be
>>
>>
>> and
>>
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/can/raw.c#n753
>>
>>
>> This leads IMO to two questions:
>>
>> 1. Is it a good idea to provide CANFD8 frames to legacy applications?
>
> Well, at first, I didn't very agree with this idea ... But I'm not able to
> find an example against, so ...
> My first option was to define a new socket option, or using the "dev->mtu"
> field value:
>
> If the application sets this option to "on" (or the MTU to 72) so it tells the
> CAN core that it is able to receive ANY CANFD frames.
Please distinguish application and configuration.
At configuration time CAN_CTRLMODE_FD can be set by 'root' which leads to
CAN_MTU or CANFD_MTU in dev->mtu when the interface is brought 'up'.
The application (e.g. at the CAN_RAW socket) can enable the socket option
CAN_RAW_FD_FRAMES, /* allow CAN FD frames (default:off) */
which means that it is able to cope with CAN frames *and* CAN FD frames.
A CAN controller which is CAN FD enabled is always able to receive 'legacy'
CAN frames too.
> If the application doesn't set one or the other option, as all existing
> "legacy" CAN applications does not, well, the CAN core won't give it ANY
> incoming CANFD frame, that is, any CAN frame with "skb->len != CAN_MTU" if
> "dev->mtu != 72".
When CAN_RAW_FD_FRAMES sockopt is not set, the application does not see CAN FD
frames (CAN_MTU == 72).
But currently it might see CANFD8 frames, where the size which is read from
the socket is reduced from CANFD_MTU to CAN_MTU.
> Something like
>
> static int canfd_rcv(struct sk_buff *skb, struct net_device *dev,
> struct packet_type *pt, struct net_device *orig_dev)
> {
> struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
>
> if (unlikely(!net_eq(dev_net(dev), &init_net)))
> goto drop;
>
> + if (dev->mtu != CANFD_MTU)
> + goto drop;
> +
IMO that's not relevant fo application, when we test for skb->len != CANFD_MTU
some line later.
> if (WARN_ONCE(dev->type != ARPHRD_CAN ||
> skb->len != CANFD_MTU ||
> cfd->len > CANFD_MAX_DLEN,
> "PF_CAN: dropped non conform CAN FD skbuf: "
> "dev type %d, len %d, datalen %d\n",
> dev->type, skb->len, cfd->len))
> goto drop;
>
> can_receive(skb, dev);
> return NET_RX_SUCCESS;
>
> drop:
> kfree_skb(skb);
> return NET_RX_DROP;
> }
>
> Why trying to be backward compatible at any price?
Today you can enable CAN FD on a per socket basis, depending on your ability
to cope with CAN FD.
>
>> 2. If yes, should the CAN FD controller driver be able to make legacy frames
>> CANDF8 frames to support the higher data bitrate for legacy applications?
>
> Same as above, I have no real arguments against this.
>
I'm just not sure, if someone will ever need this ...
Regards,
Oliver
prev parent reply other threads:[~2014-01-21 10:22 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-15 17:54 [PATCH RFC] can fd: Add separate bittiming infrastructure Oliver Hartkopp
2014-01-16 16:30 ` Marc Kleine-Budde
2014-01-17 7:44 ` Stephane Grosjean
2014-01-18 17:34 ` Oliver Hartkopp
2014-01-21 8:42 ` Stephane Grosjean
2014-01-21 9:56 ` Oliver Hartkopp
2014-01-21 10:22 ` Oliver Hartkopp [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52DE4A53.9080503@hartkopp.net \
--to=socketcan@hartkopp.net \
--cc=linux-can@vger.kernel.org \
--cc=s.grosjean@peak-system.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.