* [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
@ 2025-09-07 8:05 Vincent Mailhol
2025-09-07 19:03 ` Oliver Hartkopp
0 siblings, 1 reply; 9+ messages in thread
From: Vincent Mailhol @ 2025-09-07 8:05 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: linux-can, Vincent Mailhol
Currently, the CAN FD skb validation logic is based on the MTU: the
interface is deemed FD capable if and only if its MTU is greater or
equal to CANFD_MTU.
This logic is showing its limits since the introduction of CAN XL.
Indeed, an interface which is configured to support CAN XL but not CAN
FD would have its MTU set to CANXL_MIN_MTU which is greater than
CANFD_MTU, thus fulfilling the CAN FD check condition. That is to say,
any CAN XL interface currently appears as CAN FD capable with no way
of configuring or probing this at the skb level.
Because of the limitation, the only non-UAPI-breaking workaround is to
perform the check at the device level using the CAN_CTRLMODE_FD flag.
Unfortunately, the virtual interfaces (vcan, vxcan) are left behind
with this approach.
Add a check on the CAN_CTRLMODE_FD flag in can_dev_dropped_skb() and
drop FD frames whenever the feature is turned off.
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
While testing, I encountered a potential issue when:
- CAN FD is off
- CAN XL is on
My understanding is that this is allowed and, even more, it is
sometimes mandatory when, for example, disabling error signalling or
when using the transceiver mode switch (TMS) or both.
It seems to me that the current implementation is incapable of
handling this case at the skb level for the reasons listed in the
patch body.
I have two ideas to fix this issue:
1. above patch which assumes that we are OK not being able to set
CAN XL on and FD off on virtual interfaces
2. add some information to the skb itself. I have not looked at this
in detail. One potential idea would be to use:
sk_buff->protocol
as a bitfield. Whenever the MTU is greater than CANFD_MTU, below
logic would apply:
- if sk_buff->protocol & CAN_SKB_CC is true, classical CAN is
supported
- if sk_buff->protocol & CAN_SKB_FD is true, CAN FD is
supported
- if sk_buff->protocol & CAN_SKB_XL is true, CAN XL is
supported
with any of the combinations above accepted.
Of course, because this comes as an afterthought, it is an UAPI
breaking change for any existing CAN XL code. Also,
sk_buff->protocol was not designed for this, making this solution
an ugly hack.
Thanks for your comments.
---
include/linux/can/dev.h | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 9a92cbe5b2cb7..5053d5bc20c99 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -164,12 +164,18 @@ static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *s
if (priv->ctrlmode & CAN_CTRLMODE_LISTENONLY) {
netdev_info_once(dev,
"interface in listen only mode, dropping skb\n");
- kfree_skb(skb);
- dev->stats.tx_dropped++;
- return true;
+ goto invalid_skb;
}
+ if (!(priv->ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb))
+ goto invalid_skb;
+
return can_dropped_invalid_skb(dev, skb);
+
+invalid_skb:
+ kfree_skb(skb);
+ dev->stats.tx_dropped++;
+ return true;
}
void can_setup(struct net_device *dev);
--
2.49.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
2025-09-07 8:05 [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Vincent Mailhol
@ 2025-09-07 19:03 ` Oliver Hartkopp
2025-09-08 4:07 ` Vincent Mailhol
0 siblings, 1 reply; 9+ messages in thread
From: Oliver Hartkopp @ 2025-09-07 19:03 UTC (permalink / raw)
To: Vincent Mailhol, Marc Kleine-Budde; +Cc: linux-can
Hi Vincent,
can_dev_dropped_skb() is not what you are looking for.
Whether a CAN frame fits to the CAN device is checked in
raw_check_txframe() here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/can/raw.c#n884
Or do I miss anything?
Best regards,
Oliver
On 07.09.25 10:05, Vincent Mailhol wrote:
> Currently, the CAN FD skb validation logic is based on the MTU: the
> interface is deemed FD capable if and only if its MTU is greater or
> equal to CANFD_MTU.
>
> This logic is showing its limits since the introduction of CAN XL.
> Indeed, an interface which is configured to support CAN XL but not CAN
> FD would have its MTU set to CANXL_MIN_MTU which is greater than
> CANFD_MTU, thus fulfilling the CAN FD check condition. That is to say,
> any CAN XL interface currently appears as CAN FD capable with no way
> of configuring or probing this at the skb level.
>
> Because of the limitation, the only non-UAPI-breaking workaround is to
> perform the check at the device level using the CAN_CTRLMODE_FD flag.
> Unfortunately, the virtual interfaces (vcan, vxcan) are left behind
> with this approach.
>
> Add a check on the CAN_CTRLMODE_FD flag in can_dev_dropped_skb() and
> drop FD frames whenever the feature is turned off.
>
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> ---
> While testing, I encountered a potential issue when:
>
> - CAN FD is off
> - CAN XL is on
>
> My understanding is that this is allowed and, even more, it is
> sometimes mandatory when, for example, disabling error signalling or
> when using the transceiver mode switch (TMS) or both.
>
> It seems to me that the current implementation is incapable of
> handling this case at the skb level for the reasons listed in the
> patch body.
>
> I have two ideas to fix this issue:
>
> 1. above patch which assumes that we are OK not being able to set
> CAN XL on and FD off on virtual interfaces
>
> 2. add some information to the skb itself. I have not looked at this
> in detail. One potential idea would be to use:
>
> sk_buff->protocol
>
> as a bitfield. Whenever the MTU is greater than CANFD_MTU, below
> logic would apply:
>
> - if sk_buff->protocol & CAN_SKB_CC is true, classical CAN is
> supported
>
> - if sk_buff->protocol & CAN_SKB_FD is true, CAN FD is
> supported
>
> - if sk_buff->protocol & CAN_SKB_XL is true, CAN XL is
> supported
>
> with any of the combinations above accepted.
>
> Of course, because this comes as an afterthought, it is an UAPI
> breaking change for any existing CAN XL code. Also,
> sk_buff->protocol was not designed for this, making this solution
> an ugly hack.
>
> Thanks for your comments.
> ---
> include/linux/can/dev.h | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> index 9a92cbe5b2cb7..5053d5bc20c99 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -164,12 +164,18 @@ static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *s
> if (priv->ctrlmode & CAN_CTRLMODE_LISTENONLY) {
> netdev_info_once(dev,
> "interface in listen only mode, dropping skb\n");
> - kfree_skb(skb);
> - dev->stats.tx_dropped++;
> - return true;
> + goto invalid_skb;
> }
>
> + if (!(priv->ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb))
> + goto invalid_skb;
> +
> return can_dropped_invalid_skb(dev, skb);
> +
> +invalid_skb:
> + kfree_skb(skb);
> + dev->stats.tx_dropped++;
> + return true;
> }
>
> void can_setup(struct net_device *dev);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
2025-09-07 19:03 ` Oliver Hartkopp
@ 2025-09-08 4:07 ` Vincent Mailhol
2025-09-08 9:00 ` Oliver Hartkopp
0 siblings, 1 reply; 9+ messages in thread
From: Vincent Mailhol @ 2025-09-08 4:07 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: linux-can
On 08/09/2025 at 04:03, Oliver Hartkopp wrote:
> Hi Vincent,
>
> can_dev_dropped_skb() is not what you are looking for.
>
> Whether a CAN frame fits to the CAN device is checked in raw_check_txframe() here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/can/
> raw.c#n884
>
> Or do I miss anything?
My point is not if it fits or not. Of course, if CAN XL is activated, CAN FD
frames would fit.
My point is that activating CAN XL should not imply that CAN FD is also
activated. Having CAN FD off and CAN XL on is a valid configuration.
Let me take an example, imagine that I configure my device with CAN FD off and
CAN XL on, for example:
ip link set can0 up type can bitrate 500000 \
fd off \
xl on xbitrate 10000000 tms on
Where is the check that, with this configuration, the device is not CAN FD
capable and that the FD frames should be dropped? When I try this under my dummy
driver, the FD frames pass the raw_check_txframe() check, reach the driver's
xmit function and pass the can_dev_dropped_skb() checks. And that is the problem.
So, yes, the frame "fits" in above configuration and no buffer overflows nor any
other security problems occurred. But CAN FD is off so the frame should have
been discarded at some point.
The same issue goes for probing. How do you detect if an interface is CAN FD
capable? By checking that its MTU is at least CANFD_MTU? For example like this
in cangen?
https://github.com/linux-can/can-utils/blob/master/cangen.c#L802-L805
If I do this check, it would wrongly detect that my interface is CAN FD capable
when in fact, it is turned off in the configuration. So, under my previous
example, cangen is also fooled into believing that it can send CAN FD frames
when in reality the option is turned off.
So, these are my point:
- how do you configure a vcan so that CAN FD is off and CAN XL is on?
- when CAN FD is off and CAN XL is on, how do you drop CAN FD frames in the
kernel TX path ?
- in the userland, how do you probe that an interface is CAN FD capable or
not?
Does this clarify the problem?
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
2025-09-08 4:07 ` Vincent Mailhol
@ 2025-09-08 9:00 ` Oliver Hartkopp
2025-09-08 13:30 ` Vincent Mailhol
0 siblings, 1 reply; 9+ messages in thread
From: Oliver Hartkopp @ 2025-09-08 9:00 UTC (permalink / raw)
To: Vincent Mailhol, Marc Kleine-Budde; +Cc: linux-can
On 08.09.25 06:07, Vincent Mailhol wrote:
> On 08/09/2025 at 04:03, Oliver Hartkopp wrote:
>> Hi Vincent,
>>
>> can_dev_dropped_skb() is not what you are looking for.
>>
>> Whether a CAN frame fits to the CAN device is checked in raw_check_txframe() here:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/can/
>> raw.c#n884
>>
>> Or do I miss anything?
>
> My point is not if it fits or not. Of course, if CAN XL is activated, CAN FD
> frames would fit.
>
> My point is that activating CAN XL should not imply that CAN FD is also
> activated. Having CAN FD off and CAN XL on is a valid configuration.
>
> Let me take an example, imagine that I configure my device with CAN FD off and
> CAN XL on, for example:
>
> ip link set can0 up type can bitrate 500000 \
> fd off \
> xl on xbitrate 10000000 tms on
>
Ah, ok.
> Where is the check that, with this configuration, the device is not CAN FD
> capable and that the FD frames should be dropped? When I try this under my dummy
> driver, the FD frames pass the raw_check_txframe() check, reach the driver's
> xmit function and pass the can_dev_dropped_skb() checks. And that is the problem.
>
> So, yes, the frame "fits" in above configuration and no buffer overflows nor any
> other security problems occurred. But CAN FD is off so the frame should have
> been discarded at some point.
Yes. I think your original patch with
> + if (!(priv->ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb))
> + goto invalid_skb;
should do that job. Btw. I would also add a netdev_info_once() here too,
so that we can give a heads up to the user.
> The same issue goes for probing. How do you detect if an interface is CAN FD
> capable? By checking that its MTU is at least CANFD_MTU? For example like this
> in cangen?
>
> https://github.com/linux-can/can-utils/blob/master/cangen.c#L802-L805
>
> If I do this check, it would wrongly detect that my interface is CAN FD capable
> when in fact, it is turned off in the configuration. So, under my previous
> example, cangen is also fooled into believing that it can send CAN FD frames
> when in reality the option is turned off.
>
> So, these are my point:
>
> - how do you configure a vcan so that CAN FD is off and CAN XL is on?
This is not possible due to the missing flags (netlink) infrastructure
known from the real hardware drivers. And IMHO this is also not needed
to be implemented for a virtual CAN interface which does not have those
restrictions.
> - when CAN FD is off and CAN XL is on, how do you drop CAN FD frames in the
> kernel TX path ?
As you proposed in your extension for can_dev_dropped_skb() - with some
more comments and a netdev_info_once(), of course ;-)
We might also try to access the CAN flags from the device in the network
layer but this will become very ugly for a comparably unimportant
use-case IMO. And it won't help for PF_PACKET users creating their own
SKB content either.
> - in the userland, how do you probe that an interface is CAN FD capable or
> not?
Test ifr.ifr_mtu for
== CAN_MTU -> CC IF -> CAN CC
== CANFD_MTU -> FD IF -> CAN CC & CAN FD
>= CANXL_MIN_MTU -> XL IF -> CAN CC & CAN FD & CAN XL
There is no way to have CAN CC with CAN XL without CAN FD right now as
we can not detect this without accessing the netlink configuration.
Best regards,
Oliver
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
2025-09-08 9:00 ` Oliver Hartkopp
@ 2025-09-08 13:30 ` Vincent Mailhol
2025-09-08 14:59 ` Oliver Hartkopp
0 siblings, 1 reply; 9+ messages in thread
From: Vincent Mailhol @ 2025-09-08 13:30 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: linux-can
On 08/09/2025 at 18:00, Oliver Hartkopp wrote:
> On 08.09.25 06:07, Vincent Mailhol wrote:
>> On 08/09/2025 at 04:03, Oliver Hartkopp wrote:
>>> Hi Vincent,
>>>
>>> can_dev_dropped_skb() is not what you are looking for.
>>>
>>> Whether a CAN frame fits to the CAN device is checked in raw_check_txframe()
>>> here:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/can/
>>> raw.c#n884
>>>
>>> Or do I miss anything?
>>
>> My point is not if it fits or not. Of course, if CAN XL is activated, CAN FD
>> frames would fit.
>>
>> My point is that activating CAN XL should not imply that CAN FD is also
>> activated. Having CAN FD off and CAN XL on is a valid configuration.
>>
>> Let me take an example, imagine that I configure my device with CAN FD off and
>> CAN XL on, for example:
>>
>> ip link set can0 up type can bitrate 500000 \
>> fd off \
>> xl on xbitrate 10000000 tms on
>>
>
> Ah, ok.
>
>> Where is the check that, with this configuration, the device is not CAN FD
>> capable and that the FD frames should be dropped? When I try this under my dummy
>> driver, the FD frames pass the raw_check_txframe() check, reach the driver's
>> xmit function and pass the can_dev_dropped_skb() checks. And that is the problem.
>>
>> So, yes, the frame "fits" in above configuration and no buffer overflows nor any
>> other security problems occurred. But CAN FD is off so the frame should have
>> been discarded at some point.
>
> Yes. I think your original patch with
>
>> + if (!(priv->ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb))
>> + goto invalid_skb;
>
> should do that job. Btw. I would also add a netdev_info_once() here too, so that
> we can give a heads up to the user.
Ack. This patch will now go to my CAN XL WIP with the netdev_info_once() added
to it ;)
>> The same issue goes for probing. How do you detect if an interface is CAN FD
>> capable? By checking that its MTU is at least CANFD_MTU? For example like this
>> in cangen?
>>
>> https://github.com/linux-can/can-utils/blob/master/cangen.c#L802-L805
>>
>> If I do this check, it would wrongly detect that my interface is CAN FD capable
>> when in fact, it is turned off in the configuration. So, under my previous
>> example, cangen is also fooled into believing that it can send CAN FD frames
>> when in reality the option is turned off.
>>
>> So, these are my point:
>>
>> - how do you configure a vcan so that CAN FD is off and CAN XL is on?
>
> This is not possible due to the missing flags (netlink) infrastructure known
> from the real hardware drivers. And IMHO this is also not needed to be
> implemented for a virtual CAN interface which does not have those restrictions.
I would say that it is not strictly needed but would have been a nice to have.
If you want to proxy between a virtual interface and a real hardware, I can see
how this can become annoying.
But I concede that there is not much that we can do and that it is probably
better to just say that having a virtual interface with CAN FD off and CAN XL on
is just impossible.
>> - when CAN FD is off and CAN XL is on, how do you drop CAN FD frames in the
>> kernel TX path ?
>
> As you proposed in your extension for can_dev_dropped_skb() - with some more
> comments and a netdev_info_once(), of course ;-)
>
> We might also try to access the CAN flags from the device in the network layer
> but this will become very ugly for a comparably unimportant use-case IMO. And it
> won't help for PF_PACKET users creating their own SKB content either.
>
>> - in the userland, how do you probe that an interface is CAN FD capable or
>> not?
>
> Test ifr.ifr_mtu for
>
> == CAN_MTU -> CC IF -> CAN CC
> == CANFD_MTU -> FD IF -> CAN CC & CAN FD
>>= CANXL_MIN_MTU -> XL IF -> CAN CC & CAN FD & CAN XL
>
> There is no way to have CAN CC with CAN XL without CAN FD right now as we can
> not detect this without accessing the netlink configuration.
This brings us to the next point I wanted to discuss. As you say, the only
solution is to access the ctlrmode flags which, at the moment, are only exposed
through the netlink interface.
But using the netlink interface directly in your program is a bit troublesome,
to say the least, because of all the boilerplate code needed as illustrated in
the libsocketcan:
https://github.com/linux-can/libsocketcan/blob/master/src/libsocketcan.c
So, my other idea would be to add a new socket option that would act as a
shortcut to priv->ctrlmode.
getsockopt(s, SOL_CAN_RAW, CAN_RAW_CTRLMODE, &ctrlmode, sizeof(ctrlmode));
The interface would return an error if the interface is a virtual interface.
Otherwise, it would return the flags, allowing for an easier way to probe. As a
bonus, all the flags become easily accessible.
It means that we would have two different ways to do the same thing (netlink and
getsockopt) but I do not see this as an issue.
What do you think?
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
2025-09-08 13:30 ` Vincent Mailhol
@ 2025-09-08 14:59 ` Oliver Hartkopp
2025-09-08 16:31 ` Vincent Mailhol
0 siblings, 1 reply; 9+ messages in thread
From: Oliver Hartkopp @ 2025-09-08 14:59 UTC (permalink / raw)
To: Vincent Mailhol, Marc Kleine-Budde; +Cc: linux-can
On 08.09.25 15:30, Vincent Mailhol wrote:
> On 08/09/2025 at 18:00, Oliver Hartkopp wrote:
>> On 08.09.25 06:07, Vincent Mailhol wrote:
>>> On 08/09/2025 at 04:03, Oliver Hartkopp wrote:
>>>> Hi Vincent,
>>>>
>>>> can_dev_dropped_skb() is not what you are looking for.
>>>>
>>>> Whether a CAN frame fits to the CAN device is checked in raw_check_txframe()
>>>> here:
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/can/
>>>> raw.c#n884
>>>>
>>>> Or do I miss anything?
>>>
>>> My point is not if it fits or not. Of course, if CAN XL is activated, CAN FD
>>> frames would fit.
>>>
>>> My point is that activating CAN XL should not imply that CAN FD is also
>>> activated. Having CAN FD off and CAN XL on is a valid configuration.
>>>
>>> Let me take an example, imagine that I configure my device with CAN FD off and
>>> CAN XL on, for example:
>>>
>>> ip link set can0 up type can bitrate 500000 \
>>> fd off \
>>> xl on xbitrate 10000000 tms on
>>>
>>
>> Ah, ok.
>>
>>> Where is the check that, with this configuration, the device is not CAN FD
>>> capable and that the FD frames should be dropped? When I try this under my dummy
>>> driver, the FD frames pass the raw_check_txframe() check, reach the driver's
>>> xmit function and pass the can_dev_dropped_skb() checks. And that is the problem.
>>>
>>> So, yes, the frame "fits" in above configuration and no buffer overflows nor any
>>> other security problems occurred. But CAN FD is off so the frame should have
>>> been discarded at some point.
>>
>> Yes. I think your original patch with
>>
>>> + if (!(priv->ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb))
>>> + goto invalid_skb;
>>
>> should do that job. Btw. I would also add a netdev_info_once() here too, so that
>> we can give a heads up to the user.
>
> Ack. This patch will now go to my CAN XL WIP with the netdev_info_once() added
> to it ;)
>
>>> The same issue goes for probing. How do you detect if an interface is CAN FD
>>> capable? By checking that its MTU is at least CANFD_MTU? For example like this
>>> in cangen?
>>>
>>> https://github.com/linux-can/can-utils/blob/master/cangen.c#L802-L805
>>>
>>> If I do this check, it would wrongly detect that my interface is CAN FD capable
>>> when in fact, it is turned off in the configuration. So, under my previous
>>> example, cangen is also fooled into believing that it can send CAN FD frames
>>> when in reality the option is turned off.
>>>
>>> So, these are my point:
>>>
>>> - how do you configure a vcan so that CAN FD is off and CAN XL is on?
>>
>> This is not possible due to the missing flags (netlink) infrastructure known
>> from the real hardware drivers. And IMHO this is also not needed to be
>> implemented for a virtual CAN interface which does not have those restrictions.
>
> I would say that it is not strictly needed but would have been a nice to have.
> If you want to proxy between a virtual interface and a real hardware, I can see
> how this can become annoying.
>
> But I concede that there is not much that we can do and that it is probably
> better to just say that having a virtual interface with CAN FD off and CAN XL on
> is just impossible.
>
>>> - when CAN FD is off and CAN XL is on, how do you drop CAN FD frames in the
>>> kernel TX path ?
>>
>> As you proposed in your extension for can_dev_dropped_skb() - with some more
>> comments and a netdev_info_once(), of course ;-)
>>
>> We might also try to access the CAN flags from the device in the network layer
>> but this will become very ugly for a comparably unimportant use-case IMO. And it
>> won't help for PF_PACKET users creating their own SKB content either.
>>
>>> - in the userland, how do you probe that an interface is CAN FD capable or
>>> not?
>>
>> Test ifr.ifr_mtu for
>>
>> == CAN_MTU -> CC IF -> CAN CC
>> == CANFD_MTU -> FD IF -> CAN CC & CAN FD
>>> = CANXL_MIN_MTU -> XL IF -> CAN CC & CAN FD & CAN XL
>>
>> There is no way to have CAN CC with CAN XL without CAN FD right now as we can
>> not detect this without accessing the netlink configuration.
>
> This brings us to the next point I wanted to discuss. As you say, the only
> solution is to access the ctlrmode flags which, at the moment, are only exposed
> through the netlink interface.
>
> But using the netlink interface directly in your program is a bit troublesome,
> to say the least, because of all the boilerplate code needed as illustrated in
> the libsocketcan:
>
> https://github.com/linux-can/libsocketcan/blob/master/src/libsocketcan.c
>
> So, my other idea would be to add a new socket option that would act as a
> shortcut to priv->ctrlmode.
>
> getsockopt(s, SOL_CAN_RAW, CAN_RAW_CTRLMODE, &ctrlmode, sizeof(ctrlmode));
>
> The interface would return an error if the interface is a virtual interface.
> Otherwise, it would return the flags, allowing for an easier way to probe. As a
> bonus, all the flags become easily accessible.
>
> It means that we would have two different ways to do the same thing (netlink and
> getsockopt) but I do not see this as an issue.
>
> What do you think?
You are right with having two APIs for the same thing ...
If we would have such a getsockopt() I would suggest to provide the
ctrlmode and the ctrlmode_supported to the user space.
And the bits are only valid when the interface is up. So additional to
e.g. -EOPNOTSUPP for vcan's we should also be able to return -ENETDOWN.
My biggest concern is whether such a getsockopt() is really needed.
Today you can enable CANFD and CANXL with setsockopt() and when you send
frames that can not be sent to the interface you get an error.
The only thing that is "not that nice" is the CANXL-only (without FD)
possibility.
Best regards,
Oliver
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
2025-09-08 14:59 ` Oliver Hartkopp
@ 2025-09-08 16:31 ` Vincent Mailhol
2025-09-08 16:55 ` Oliver Hartkopp
0 siblings, 1 reply; 9+ messages in thread
From: Vincent Mailhol @ 2025-09-08 16:31 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: linux-can
On 08/09/2025 at 23:59, Oliver Hartkopp wrote:
> On 08.09.25 15:30, Vincent Mailhol wrote:
>> On 08/09/2025 at 18:00, Oliver Hartkopp wrote:
(...)
>>> There is no way to have CAN CC with CAN XL without CAN FD right now as we can
>>> not detect this without accessing the netlink configuration.
>>
>> This brings us to the next point I wanted to discuss. As you say, the only
>> solution is to access the ctlrmode flags which, at the moment, are only exposed
>> through the netlink interface.
>>
>> But using the netlink interface directly in your program is a bit troublesome,
>> to say the least, because of all the boilerplate code needed as illustrated in
>> the libsocketcan:
>>
>> https://github.com/linux-can/libsocketcan/blob/master/src/libsocketcan.c
>>
>> So, my other idea would be to add a new socket option that would act as a
>> shortcut to priv->ctrlmode.
>>
>> getsockopt(s, SOL_CAN_RAW, CAN_RAW_CTRLMODE, &ctrlmode, sizeof(ctrlmode));
>>
>> The interface would return an error if the interface is a virtual interface.
>> Otherwise, it would return the flags, allowing for an easier way to probe. As a
>> bonus, all the flags become easily accessible.
>>
>> It means that we would have two different ways to do the same thing (netlink and
>> getsockopt) but I do not see this as an issue.
>>
>> What do you think?
>
> You are right with having two APIs for the same thing ...
>
> If we would have such a getsockopt() I would suggest to provide the ctrlmode and
> the ctrlmode_supported to the user space.
Yes, and probably also the CAN_RAW_CTRLMODE_STATIC. I do not often use that one,
but for completeness, better to expose all three.
> And the bits are only valid when the interface is up. So additional to e.g. -
> EOPNOTSUPP for vcan's we should also be able to return -ENETDOWN.
Ack for the vcan.
For the real hardware, I think that the CAN_RAW_CTRLMODE_SUPPORTED and
CAN_RAW_CTRLMODE_STATIC should be always available. Only the CAN_RAW_CTRLMODE
would return -ENETDOWN if the interface is down.
> My biggest concern is whether such a getsockopt() is really needed.
>
> Today you can enable CANFD and CANXL with setsockopt() and when you send frames
> that can not be sent to the interface you get an error.
>
> The only thing that is "not that nice" is the CANXL-only (without FD) possibility.
The fact that vcan can not be configured with FD off and XL on is what I would
call a "not that nice".
But the impossibility to reliably probe the FD state is worse than "not that
nice". When I see cangen sending CAN FD frames when it shouldn't, I call this a
bug. The impossibility to probe is a no-go to me.
An alternative would be to revive the libsocketcan to make it easier to use the
netlink interface but this is lot of effort and it means adding one userland
dependency. The getsockopt() seems a better alternative.
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
2025-09-08 16:31 ` Vincent Mailhol
@ 2025-09-08 16:55 ` Oliver Hartkopp
2025-09-08 17:42 ` Vincent Mailhol
0 siblings, 1 reply; 9+ messages in thread
From: Oliver Hartkopp @ 2025-09-08 16:55 UTC (permalink / raw)
To: Vincent Mailhol, Marc Kleine-Budde; +Cc: linux-can
On 08.09.25 18:31, Vincent Mailhol wrote:
> On 08/09/2025 at 23:59, Oliver Hartkopp wrote:
>> On 08.09.25 15:30, Vincent Mailhol wrote:
>>> On 08/09/2025 at 18:00, Oliver Hartkopp wrote:
>
> (...)
>
>>>> There is no way to have CAN CC with CAN XL without CAN FD right now as we can
>>>> not detect this without accessing the netlink configuration.
>>>
>>> This brings us to the next point I wanted to discuss. As you say, the only
>>> solution is to access the ctlrmode flags which, at the moment, are only exposed
>>> through the netlink interface.
>>>
>>> But using the netlink interface directly in your program is a bit troublesome,
>>> to say the least, because of all the boilerplate code needed as illustrated in
>>> the libsocketcan:
>>>
>>> https://github.com/linux-can/libsocketcan/blob/master/src/libsocketcan.c
>>>
>>> So, my other idea would be to add a new socket option that would act as a
>>> shortcut to priv->ctrlmode.
>>>
>>> getsockopt(s, SOL_CAN_RAW, CAN_RAW_CTRLMODE, &ctrlmode, sizeof(ctrlmode));
>>>
>>> The interface would return an error if the interface is a virtual interface.
>>> Otherwise, it would return the flags, allowing for an easier way to probe. As a
>>> bonus, all the flags become easily accessible.
>>>
>>> It means that we would have two different ways to do the same thing (netlink and
>>> getsockopt) but I do not see this as an issue.
>>>
>>> What do you think?
>>
>> You are right with having two APIs for the same thing ...
>>
>> If we would have such a getsockopt() I would suggest to provide the ctrlmode and
>> the ctrlmode_supported to the user space.
>
> Yes, and probably also the CAN_RAW_CTRLMODE_STATIC. I do not often use that one,
> but for completeness, better to expose all three.
>
>> And the bits are only valid when the interface is up. So additional to e.g. -
>> EOPNOTSUPP for vcan's we should also be able to return -ENETDOWN.
>
> Ack for the vcan.
>
> For the real hardware, I think that the CAN_RAW_CTRLMODE_SUPPORTED and
> CAN_RAW_CTRLMODE_STATIC should be always available. Only the CAN_RAW_CTRLMODE
> would return -ENETDOWN if the interface is down.
>
>> My biggest concern is whether such a getsockopt() is really needed.
>>
>> Today you can enable CANFD and CANXL with setsockopt() and when you send frames
>> that can not be sent to the interface you get an error.
>>
>> The only thing that is "not that nice" is the CANXL-only (without FD) possibility.
>
> The fact that vcan can not be configured with FD off and XL on is what I would
> call a "not that nice".
>
> But the impossibility to reliably probe the FD state is worse than "not that
> nice". When I see cangen sending CAN FD frames when it shouldn't, I call this a
> bug. The impossibility to probe is a no-go to me.
>
> An alternative would be to revive the libsocketcan to make it easier to use the
> netlink interface but this is lot of effort and it means adding one userland
> dependency. The getsockopt() seems a better alternative.
I think the best way is to properly split up the FD/XL capabilities for
CAN XL interfaces. Right now CAN FD is implicitly enabled on CAN XL
interfaces when CAN XL is enabled.
I would tend to change the code in raw.c in a way that you have to
switch on FD/XL explicitly. And then the problem would directly show up
in write() and FD frames would not be silently dropped in
can_dev_dropped_skb().
Best regards,
Oliver
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
2025-09-08 16:55 ` Oliver Hartkopp
@ 2025-09-08 17:42 ` Vincent Mailhol
0 siblings, 0 replies; 9+ messages in thread
From: Vincent Mailhol @ 2025-09-08 17:42 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: linux-can
On 09/09/2025 at 01:55, Oliver Hartkopp wrote:
(...)
> I think the best way is to properly split up the FD/XL capabilities for CAN XL
> interfaces. Right now CAN FD is implicitly enabled on CAN XL interfaces when CAN
> XL is enabled.
>
> I would tend to change the code in raw.c in a way that you have to switch on FD/
> XL explicitly. And then the problem would directly show up in write() and FD
> frames would not be silently dropped in can_dev_dropped_skb().
If you can tweak raw.c to achieve this, it can be a better solution. I am just
not sure how you are going to do that without breaking the UAPI. Looking forward
for your patch!
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-09-08 17:42 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-07 8:05 [RFC PATCH] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Vincent Mailhol
2025-09-07 19:03 ` Oliver Hartkopp
2025-09-08 4:07 ` Vincent Mailhol
2025-09-08 9:00 ` Oliver Hartkopp
2025-09-08 13:30 ` Vincent Mailhol
2025-09-08 14:59 ` Oliver Hartkopp
2025-09-08 16:31 ` Vincent Mailhol
2025-09-08 16:55 ` Oliver Hartkopp
2025-09-08 17:42 ` Vincent Mailhol
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox