* [PATCH v2] can: raw: fix CAN FD frame transmissions over CAN XL devices
@ 2023-01-31 9:18 Oliver Hartkopp
2023-01-31 11:06 ` Marc Kleine-Budde
0 siblings, 1 reply; 3+ messages in thread
From: Oliver Hartkopp @ 2023-01-31 9:18 UTC (permalink / raw)
To: linux-can; +Cc: Oliver Hartkopp
A CAN XL device is always capable to process CAN FD frames. The former
check when sending CAN FD frames relied on the existence of a CAN FD
device and did not check for a CAN XL device that would be correct too.
With this patch the CAN FD feature is enabled automatically when CAN XL
is switched on - and CAN FD cannot be switch off while CAN XL is enabled.
This precondition also leads to a clean up and reduction of checks in the
hot path in raw_rcv() and raw_sendmsg().
Fixes: 626332696d75 ("can: raw: add CAN XL support")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
V2: fixed typo: devive -> device
net/can/raw.c | 45 ++++++++++++++++++++++++++++++---------------
1 file changed, 30 insertions(+), 15 deletions(-)
diff --git a/net/can/raw.c b/net/can/raw.c
index 81071cdb0301..c505d315a7a9 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -130,11 +130,11 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
/* check the received tx sock reference */
if (!ro->recv_own_msgs && oskb->sk == sk)
return;
/* make sure to not pass oversized frames to the socket */
- if ((can_is_canfd_skb(oskb) && !ro->fd_frames && !ro->xl_frames) ||
+ if ((can_is_canfd_skb(oskb) && !ro->fd_frames) ||
(can_is_canxl_skb(oskb) && !ro->xl_frames))
return;
/* eliminate multiple filter matches for the same skb */
if (this_cpu_ptr(ro->uniq)->skb == oskb &&
@@ -668,19 +668,27 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
return -EINVAL;
if (copy_from_sockptr(&ro->fd_frames, optval, optlen))
return -EFAULT;
+ /* Enabling CAN XL includes CAN FD */
+ if (ro->xl_frames && !ro->fd_frames) {
+ ro->fd_frames = ro->xl_frames;
+ return -EINVAL;
+ }
break;
case CAN_RAW_XL_FRAMES:
if (optlen != sizeof(ro->xl_frames))
return -EINVAL;
if (copy_from_sockptr(&ro->xl_frames, optval, optlen))
return -EFAULT;
+ /* Enabling CAN XL includes CAN FD */
+ if (ro->xl_frames)
+ ro->fd_frames = ro->xl_frames;
break;
case CAN_RAW_JOIN_FILTERS:
if (optlen != sizeof(ro->join_filters))
return -EINVAL;
@@ -784,10 +792,29 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
if (copy_to_user(optval, val, len))
return -EFAULT;
return 0;
}
+static bool raw_bad_txframe(struct raw_sock *ro, struct sk_buff *skb, int mtu)
+{
+ /* Classical CAN -> no checks for flags and device capabilities */
+ if (can_is_can_skb(skb))
+ return false;
+
+ /* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
+ if (can_is_canfd_skb(skb) && ro->fd_frames &&
+ (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
+ return false;
+
+ /* CAN XL -> needs to be enabled and a CAN XL device */
+ if (can_is_canxl_skb(skb) && ro->xl_frames &&
+ can_is_canxl_dev_mtu(mtu))
+ return false;
+
+ return true;
+}
+
static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct sock *sk = sock->sk;
struct raw_sock *ro = raw_sk(sk);
struct sockcm_cookie sockc;
@@ -831,24 +858,12 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
err = memcpy_from_msg(skb_put(skb, size), msg, size);
if (err < 0)
goto free_skb;
err = -EINVAL;
- if (ro->xl_frames && can_is_canxl_dev_mtu(dev->mtu)) {
- /* CAN XL, CAN FD and Classical CAN */
- if (!can_is_canxl_skb(skb) && !can_is_canfd_skb(skb) &&
- !can_is_can_skb(skb))
- goto free_skb;
- } else if (ro->fd_frames && dev->mtu == CANFD_MTU) {
- /* CAN FD and Classical CAN */
- if (!can_is_canfd_skb(skb) && !can_is_can_skb(skb))
- goto free_skb;
- } else {
- /* Classical CAN */
- if (!can_is_can_skb(skb))
- goto free_skb;
- }
+ if (raw_bad_txframe(ro, skb, dev->mtu))
+ goto free_skb;
sockcm_init(&sockc, sk);
if (msg->msg_controllen) {
err = sock_cmsg_send(sk, msg, &sockc);
if (unlikely(err))
--
2.39.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] can: raw: fix CAN FD frame transmissions over CAN XL devices
2023-01-31 9:18 [PATCH v2] can: raw: fix CAN FD frame transmissions over CAN XL devices Oliver Hartkopp
@ 2023-01-31 11:06 ` Marc Kleine-Budde
2023-01-31 12:00 ` Oliver Hartkopp
0 siblings, 1 reply; 3+ messages in thread
From: Marc Kleine-Budde @ 2023-01-31 11:06 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 2347 bytes --]
On 31.01.2023 10:18:24, Oliver Hartkopp wrote:
> A CAN XL device is always capable to process CAN FD frames. The former
> check when sending CAN FD frames relied on the existence of a CAN FD
> device and did not check for a CAN XL device that would be correct too.
>
> With this patch the CAN FD feature is enabled automatically when CAN XL
> is switched on - and CAN FD cannot be switch off while CAN XL is enabled.
>
> This precondition also leads to a clean up and reduction of checks in the
> hot path in raw_rcv() and raw_sendmsg().
>
> Fixes: 626332696d75 ("can: raw: add CAN XL support")
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
>
> V2: fixed typo: devive -> device
>
> net/can/raw.c | 45 ++++++++++++++++++++++++++++++---------------
> 1 file changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/net/can/raw.c b/net/can/raw.c
> index 81071cdb0301..c505d315a7a9 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -130,11 +130,11 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
> /* check the received tx sock reference */
> if (!ro->recv_own_msgs && oskb->sk == sk)
> return;
>
> /* make sure to not pass oversized frames to the socket */
> - if ((can_is_canfd_skb(oskb) && !ro->fd_frames && !ro->xl_frames) ||
> + if ((can_is_canfd_skb(oskb) && !ro->fd_frames) ||
> (can_is_canxl_skb(oskb) && !ro->xl_frames))
> return;
>
> /* eliminate multiple filter matches for the same skb */
> if (this_cpu_ptr(ro->uniq)->skb == oskb &&
> @@ -668,19 +668,27 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
> return -EINVAL;
>
> if (copy_from_sockptr(&ro->fd_frames, optval, optlen))
> return -EFAULT;
>
> + /* Enabling CAN XL includes CAN FD */
> + if (ro->xl_frames && !ro->fd_frames) {
> + ro->fd_frames = ro->xl_frames;
> + return -EINVAL;
> + }
This looks a bit weird. Maybe copy to a local variable first, then
check, then assign ->fd_frames or exit with error (instead of rolling
back)?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] can: raw: fix CAN FD frame transmissions over CAN XL devices
2023-01-31 11:06 ` Marc Kleine-Budde
@ 2023-01-31 12:00 ` Oliver Hartkopp
0 siblings, 0 replies; 3+ messages in thread
From: Oliver Hartkopp @ 2023-01-31 12:00 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can
On 1/31/23 12:06, Marc Kleine-Budde wrote:
> On 31.01.2023 10:18:24, Oliver Hartkopp wrote:
>> A CAN XL device is always capable to process CAN FD frames. The former
>> check when sending CAN FD frames relied on the existence of a CAN FD
>> device and did not check for a CAN XL device that would be correct too.
>>
>> With this patch the CAN FD feature is enabled automatically when CAN XL
>> is switched on - and CAN FD cannot be switch off while CAN XL is enabled.
>>
>> This precondition also leads to a clean up and reduction of checks in the
>> hot path in raw_rcv() and raw_sendmsg().
>>
>> Fixes: 626332696d75 ("can: raw: add CAN XL support")
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> ---
>>
>> V2: fixed typo: devive -> device
>>
>> net/can/raw.c | 45 ++++++++++++++++++++++++++++++---------------
>> 1 file changed, 30 insertions(+), 15 deletions(-)
>>
>> diff --git a/net/can/raw.c b/net/can/raw.c
>> index 81071cdb0301..c505d315a7a9 100644
>> --- a/net/can/raw.c
>> +++ b/net/can/raw.c
>> @@ -130,11 +130,11 @@ static void raw_rcv(struct sk_buff *oskb, void *data)
>> /* check the received tx sock reference */
>> if (!ro->recv_own_msgs && oskb->sk == sk)
>> return;
>>
>> /* make sure to not pass oversized frames to the socket */
>> - if ((can_is_canfd_skb(oskb) && !ro->fd_frames && !ro->xl_frames) ||
>> + if ((can_is_canfd_skb(oskb) && !ro->fd_frames) ||
>> (can_is_canxl_skb(oskb) && !ro->xl_frames))
>> return;
>>
>> /* eliminate multiple filter matches for the same skb */
>> if (this_cpu_ptr(ro->uniq)->skb == oskb &&
>> @@ -668,19 +668,27 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
>> return -EINVAL;
>>
>> if (copy_from_sockptr(&ro->fd_frames, optval, optlen))
>> return -EFAULT;
>>
>> + /* Enabling CAN XL includes CAN FD */
>> + if (ro->xl_frames && !ro->fd_frames) {
>> + ro->fd_frames = ro->xl_frames;
>> + return -EINVAL;
>> + }
>
> This looks a bit weird. Maybe copy to a local variable first, then
> check, then assign ->fd_frames or exit with error (instead of rolling
> back)?
>
Changed in V4.
Thanks & BR,
Oliver
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-31 12:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-31 9:18 [PATCH v2] can: raw: fix CAN FD frame transmissions over CAN XL devices Oliver Hartkopp
2023-01-31 11:06 ` Marc Kleine-Budde
2023-01-31 12:00 ` Oliver Hartkopp
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.