All of lore.kernel.org
 help / color / mirror / Atom feed
* [Question] Sending CAN error frames
@ 2021-01-31  6:22 Vincent MAILHOL
       [not found] ` <87e3dd54-50ab-1190-efdb-18ddb3b21a02@hartkopp.net>
  0 siblings, 1 reply; 24+ messages in thread
From: Vincent MAILHOL @ 2021-01-31  6:22 UTC (permalink / raw)
  To: linux-can
  Cc: Vincent Mailhol, Marc Kleine-Budde, Oliver Hartkopp,
	Jimmy Assarsson, Christer Beskow

Hello,

The socket CAN API handles the CAN errors (as reported by the
microcontroller) by emitting some RX CAN frames with the
CAN_ERR_FLAG set.

My question concerns the transmission path: I would like to
understand how drivers should handle *TX* CAN frames which have
the CAN_ERR_FLAG set.

The socket API allows sending such frames. For example doing:
    cansend can0 20000123#0011223344556677
will generate such frames and it will reach the xmit() function of
the driver.

However, contrary to the other flags (EFF, RTR, FDF, BRS), the
ERR flag is not present on the data link layer. Instead, the data
link layer is responsible for detecting errors and signaling those
as soon as they occur (thus interrupting the transmission).

While the ISO standard does not explicitly forbid having upper
layers generating such frames, it is not documented. Also, I am
not aware of CAN controllers allowing to generate error frames on
demand.

My initial expectation is that those error frames only make
sense in the RX path and that we should drop such TX frames in,
for example, can_dropped_invalid_skb().

However, after looking at the code of other drivers, it appears
that one (and only one) of them: the Kvaser hydra, does actually
check this CAN_ERR_FLAG flag in the TX path:
https://elixir.bootlin.com/linux/v5.11-rc5/source/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c#L1421

I would be thankful if anyone knowledgeable about the Kvaser hydra
could explain to me how the device handles those error frames.

Also, please comment if you are aware of any use cases for TX
error frames.


Yours sincerely,
Vincent

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
       [not found] ` <87e3dd54-50ab-1190-efdb-18ddb3b21a02@hartkopp.net>
@ 2021-01-31 14:12   ` Vincent MAILHOL
  2021-01-31 20:42   ` Jimmy Assarsson
  1 sibling, 0 replies; 24+ messages in thread
From: Vincent MAILHOL @ 2021-01-31 14:12 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: linux-can, Marc Kleine-Budde, Jimmy Assarsson, Christer Beskow

Hi Oliver,

On Sun. 31 Jan 2021 at 21:59, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Hi Vincent,
>
> On 31.01.21 07:22, Vincent MAILHOL wrote:
> > Hello,
> >
> > The socket CAN API handles the CAN errors (as reported by the
> > microcontroller) by emitting some RX CAN frames with the
> > CAN_ERR_FLAG set.
>
> Yes. This is the only intention.
>
> > My question concerns the transmission path: I would like to
> > understand how drivers should handle *TX* CAN frames which have
> > the CAN_ERR_FLAG set.
> >
> > The socket API allows sending such frames. For example doing:
> >      cansend can0 20000123#0011223344556677
> > will generate such frames and it will reach the xmit() function of
> > the driver.
>
> The reason to pass the frame as-is to the driver layer (including
> CAN_ERR_FLAG) is the possibility to test the correct behavior on the RX
> path, e.g. when you use the vcan driver.

ACK. I was not thinking of the virtual interfaces. This is a valid use case.

> On the sending path the CAN_ERR_FLAG has no functionality so far - at
> least it was not defined by the community.
>
> > However, contrary to the other flags (EFF, RTR, FDF, BRS), the
> > ERR flag is not present on the data link layer. Instead, the data
> > link layer is responsible for detecting errors and signaling those
> > as soon as they occur (thus interrupting the transmission).
> >
> > While the ISO standard does not explicitly forbid having upper
> > layers generating such frames, it is not documented. Also, I am
> > not aware of CAN controllers allowing to generate error frames on
> > demand.
>
> There are specialized CAN testers, e.g. IIRC Vector CANstress that can
> generate error frames on specific conditions (e.g. when detecting a
> specific CAN ID).

Thanks for the references!

> But I heave not seen CAN controllers that provide such functionality.
>
> > My initial expectation is that those error frames only make
> > sense in the RX path and that we should drop such TX frames in,
> > for example, can_dropped_invalid_skb().
>
> No. As written above the bit is defined to be valid in the RX path only
> and it makes sense for testing.

ACK. My follow-up question would be: how should the driver handle
such frames? Drop them or ignore them?
From what you just explained, I now think that ignoring it is the
best solution (e.g. mask the CAN ID with either CAN_EFF_MASK or
CAN_SFF_MASK and send it as a normal frame).
Does that make sense?

> > However, after looking at the code of other drivers, it appears
> > that one (and only one) of them: the Kvaser hydra, does actually
> > check this CAN_ERR_FLAG flag in the TX path:
> > https://elixir.bootlin.com/linux/v5.11-rc5/source/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c#L1421
> >
> > I would be thankful if anyone knowledgeable about the Kvaser hydra
> > could explain to me how the device handles those error frames.
>
> o_O - Yes, would be interested too!!
>
> > Also, please comment if you are aware of any use cases for TX
> > error frames.
>
> Done.

And thanks for that!

> Best regards,
> Oliver

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
       [not found] ` <87e3dd54-50ab-1190-efdb-18ddb3b21a02@hartkopp.net>
  2021-01-31 14:12   ` Vincent MAILHOL
@ 2021-01-31 20:42   ` Jimmy Assarsson
  2021-02-01 14:19     ` Vincent MAILHOL
  1 sibling, 1 reply; 24+ messages in thread
From: Jimmy Assarsson @ 2021-01-31 20:42 UTC (permalink / raw)
  To: Oliver Hartkopp, Vincent MAILHOL, linux-can, Marc Kleine-Budde
  Cc: Jimmy Assarsson, Christer Beskow

On 2021-01-31 13:59, Oliver Hartkopp wrote:
> Hi Vincent,
> 
> On 31.01.21 07:22, Vincent MAILHOL wrote:
>> Hello,
>>
>> The socket CAN API handles the CAN errors (as reported by the
>> microcontroller) by emitting some RX CAN frames with the
>> CAN_ERR_FLAG set.
> 
> Yes. This is the only intention.
> 
>> My question concerns the transmission path: I would like to
>> understand how drivers should handle *TX* CAN frames which have
>> the CAN_ERR_FLAG set.
>>
>> The socket API allows sending such frames. For example doing:
>>      cansend can0 20000123#0011223344556677
>> will generate such frames and it will reach the xmit() function of
>> the driver.
> 
> The reason to pass the frame as-is to the driver layer (including CAN_ERR_FLAG) is the possibility to test the correct behavior on the RX path, e.g. when you use the vcan driver.
> 
> On the sending path the CAN_ERR_FLAG has no functionality so far - at least it was not defined by the community.
> 
>> However, contrary to the other flags (EFF, RTR, FDF, BRS), the
>> ERR flag is not present on the data link layer. Instead, the data
>> link layer is responsible for detecting errors and signaling those
>> as soon as they occur (thus interrupting the transmission).
>>
>> While the ISO standard does not explicitly forbid having upper
>> layers generating such frames, it is not documented. Also, I am
>> not aware of CAN controllers allowing to generate error frames on
>> demand.
> 
> There are specialized CAN testers, e.g. IIRC Vector CANstress that can generate error frames on specific conditions (e.g. when detecting a specific CAN ID).
> 
> But I heave not seen CAN controllers that provide such functionality.
> 
>> My initial expectation is that those error frames only make
>> sense in the RX path and that we should drop such TX frames in,
>> for example, can_dropped_invalid_skb().
> 
> No. As written above the bit is defined to be valid in the RX path only and it makes sense for testing.
> 
>> However, after looking at the code of other drivers, it appears
>> that one (and only one) of them: the Kvaser hydra, does actually
>> check this CAN_ERR_FLAG flag in the TX path:
>> https://elixir.bootlin.com/linux/v5.11-rc5/source/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c#L1421
>>
>> I would be thankful if anyone knowledgeable about the Kvaser hydra
>> could explain to me how the device handles those error frames.
> 
> o_O - Yes, would be interested too!!

Hi Vincent and Oliver,

When the user passes a frame with CAN_ERR_FLAG set, the CAN controller will generate an error frame.
We got customers that use this for testing system robustness and fault reporting/handling.

We also got this implemented in the early version of kvaser_pciefd driver, but dropped it:
https://marc.info/?l=linux-can&m=154324867704480&w=2
Is this something that we should remove from kvasr_usb aswell?

Regards,
jimmy

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-01-31 20:42   ` Jimmy Assarsson
@ 2021-02-01 14:19     ` Vincent MAILHOL
  2021-02-01 15:41       ` Jimmy Assarsson
  0 siblings, 1 reply; 24+ messages in thread
From: Vincent MAILHOL @ 2021-02-01 14:19 UTC (permalink / raw)
  To: Jimmy Assarsson
  Cc: Oliver Hartkopp, linux-can, Marc Kleine-Budde, Jimmy Assarsson,
	Christer Beskow

Hi Jimmy,

On Mon. 1 Feb 2021 at 05:42, Jimmy Assarsson <jimmyassarsson@gmail.com> wrote:
>
> On 2021-01-31 13:59, Oliver Hartkopp wrote:
> > Hi Vincent,
> >
> > On 31.01.21 07:22, Vincent MAILHOL wrote:
> >> Hello,
> >>
> >> The socket CAN API handles the CAN errors (as reported by the
> >> microcontroller) by emitting some RX CAN frames with the
> >> CAN_ERR_FLAG set.
> >
> > Yes. This is the only intention.
> >
> >> My question concerns the transmission path: I would like to
> >> understand how drivers should handle *TX* CAN frames which have
> >> the CAN_ERR_FLAG set.
> >>
> >> The socket API allows sending such frames. For example doing:
> >>      cansend can0 20000123#0011223344556677
> >> will generate such frames and it will reach the xmit() function of
> >> the driver.
> >
> > The reason to pass the frame as-is to the driver layer (including CAN_ERR_FLAG) is the possibility to test the correct behavior on the RX path, e.g. when you use the vcan driver.
> >
> > On the sending path the CAN_ERR_FLAG has no functionality so far - at least it was not defined by the community.
> >
> >> However, contrary to the other flags (EFF, RTR, FDF, BRS), the
> >> ERR flag is not present on the data link layer. Instead, the data
> >> link layer is responsible for detecting errors and signaling those
> >> as soon as they occur (thus interrupting the transmission).
> >>
> >> While the ISO standard does not explicitly forbid having upper
> >> layers generating such frames, it is not documented. Also, I am
> >> not aware of CAN controllers allowing to generate error frames on
> >> demand.
> >
> > There are specialized CAN testers, e.g. IIRC Vector CANstress that can generate error frames on specific conditions (e.g. when detecting a specific CAN ID).
> >
> > But I heave not seen CAN controllers that provide such functionality.
> >
> >> My initial expectation is that those error frames only make
> >> sense in the RX path and that we should drop such TX frames in,
> >> for example, can_dropped_invalid_skb().
> >
> > No. As written above the bit is defined to be valid in the RX path only and it makes sense for testing.
> >
> >> However, after looking at the code of other drivers, it appears
> >> that one (and only one) of them: the Kvaser hydra, does actually
> >> check this CAN_ERR_FLAG flag in the TX path:
> >> https://elixir.bootlin.com/linux/v5.11-rc5/source/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c#L1421
> >>
> >> I would be thankful if anyone knowledgeable about the Kvaser hydra
> >> could explain to me how the device handles those error frames.
> >
> > o_O - Yes, would be interested too!!
>
> Hi Vincent and Oliver,
>
> When the user passes a frame with CAN_ERR_FLAG set, the CAN controller will generate an error frame.
> We got customers that use this for testing system robustness and fault reporting/handling.

Interesting.

There are two forms of error flags:
  - The active error flag: 6 consecutive dominant bits
  - The passive error flag: 6 consecutive recessive bits
Can it generate both or only one of these? Is it generated as
soon as the device receives the command or is it generated at a
predefined timing (e.g. beginning of the frame)?

Which CAN microcontroller is used inside the device?

Also, could you point me to the Kvaser hydra product page? I
couldn't find it with a quick search so I figured out that maybe
the commercial name is different?

> We also got this implemented in the early version of kvaser_pciefd driver, but dropped it:
> https://marc.info/?l=linux-can&m=154324867704480&w=2
> Is this something that we should remove from kvasr_usb aswell?

No. My intent is not to ask you to remove it. It is rather the
opposite: I want to understand more how you are able to achieve
what I thought not to be possible. For what you told me, the
Kvaser hydra is the kind of device which I would like to have in
my testing environment :)

However, it would make sense to normalize this use case in the
Socket CAN interface. For the discussion with Oliver, it is clear
that this use case is currently not expected and undefined, the
reason being that such a CAN controller was not thought to
exist. Now that you have proved us wrong, things are different.


Yours sincerely,
Vincent

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-01 14:19     ` Vincent MAILHOL
@ 2021-02-01 15:41       ` Jimmy Assarsson
  2021-02-02  0:22         ` Vincent MAILHOL
  0 siblings, 1 reply; 24+ messages in thread
From: Jimmy Assarsson @ 2021-02-01 15:41 UTC (permalink / raw)
  To: Vincent MAILHOL
  Cc: Jimmy Assarsson, Oliver Hartkopp, linux-can, Marc Kleine-Budde,
	Christer Beskow

On 2021-02-01 15:19, Vincent MAILHOL wrote:
> Hi Jimmy,
> On Mon. 1 Feb 2021 at 05:42, Jimmy Assarsson <jimmyassarsson@gmail.com> wrote:
>> On 2021-01-31 13:59, Oliver Hartkopp wrote:
>>> Hi Vincent,
>>> On 31.01.21 07:22, Vincent MAILHOL wrote:
>>>> Hello,
>>>>
>>>> The socket CAN API handles the CAN errors (as reported by the
>>>> microcontroller) by emitting some RX CAN frames with the
>>>> CAN_ERR_FLAG set.
>>>
>>> Yes. This is the only intention.
>>>
>>>> My question concerns the transmission path: I would like to
>>>> understand how drivers should handle *TX* CAN frames which have
>>>> the CAN_ERR_FLAG set.
>>>>
>>>> The socket API allows sending such frames. For example doing:
>>>>       cansend can0 20000123#0011223344556677
>>>> will generate such frames and it will reach the xmit() function of
>>>> the driver.
>>>
>>> The reason to pass the frame as-is to the driver layer (including CAN_ERR_FLAG) is the possibility to test the correct behavior on the RX path, e.g. when you use the vcan driver.
>>>
>>> On the sending path the CAN_ERR_FLAG has no functionality so far - at least it was not defined by the community.
>>>
>>>> However, contrary to the other flags (EFF, RTR, FDF, BRS), the
>>>> ERR flag is not present on the data link layer. Instead, the data
>>>> link layer is responsible for detecting errors and signaling those
>>>> as soon as they occur (thus interrupting the transmission).
>>>>
>>>> While the ISO standard does not explicitly forbid having upper
>>>> layers generating such frames, it is not documented. Also, I am
>>>> not aware of CAN controllers allowing to generate error frames on
>>>> demand.
>>>
>>> There are specialized CAN testers, e.g. IIRC Vector CANstress that can generate error frames on specific conditions (e.g. when detecting a specific CAN ID).
>>>
>>> But I heave not seen CAN controllers that provide such functionality.
>>>
>>>> My initial expectation is that those error frames only make
>>>> sense in the RX path and that we should drop such TX frames in,
>>>> for example, can_dropped_invalid_skb().
>>>
>>> No. As written above the bit is defined to be valid in the RX path only and it makes sense for testing.
>>>
>>>> However, after looking at the code of other drivers, it appears
>>>> that one (and only one) of them: the Kvaser hydra, does actually
>>>> check this CAN_ERR_FLAG flag in the TX path:
>>>> https://elixir.bootlin.com/linux/v5.11-rc5/source/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c#L1421
>>>>
>>>> I would be thankful if anyone knowledgeable about the Kvaser hydra
>>>> could explain to me how the device handles those error frames.
>>>
>>> o_O - Yes, would be interested too!!
>>
>> Hi Vincent and Oliver,
>>
>> When the user passes a frame with CAN_ERR_FLAG set, the CAN controller will generate an error frame.
>> We got customers that use this for testing system robustness and fault reporting/handling.
> 
> Interesting.
> 
> There are two forms of error flags:
>    - The active error flag: 6 consecutive dominant bits
>    - The passive error flag: 6 consecutive recessive bits
> Can it generate both or only one of these? Is it generated as
> soon as the device receives the command or is it generated at a
> predefined timing (e.g. beginning of the frame)?

It can only generate the active error flag.
It will generate an error frame on the bus as soon as the bus is idle.

$ sudo ip link set can4 type can bitrate 100000
$ sudo ip link set can5 type can bitrate 100000
$ sudo ip link set can4 up
$ sudo ip link set can5 up
$ ./candump -c -ta -H -d -e -x can4,#FFFFFFFF &
[1] 177884
$ ./cansend can5 20000000#
  (0000000084.786201)  can4  RX - -  20000080   [8]  00 00 00 00 00 00 
00 01   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{1}}
$ ./cansend can5 20000123#0011223344556677
  (0000000086.798898)  can4  RX - -  20000080   [8]  00 00 00 00 00 00 
00 02   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{2}}
$ ./cansend can5 20000000#
  (0000000087.385292)  can4  RX - -  20000080   [8]  00 00 00 00 00 00 
00 03   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{3}}
$ ./cansend can5 20000000#
  (0000000087.911860)  can4  RX - -  20000080   [8]  00 00 00 00 00 00 
00 04   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{4}}
$ ./cansend can5 20000000#
  (0000000088.404890)  can4  RX - -  20000080   [8]  00 00 00 00 00 00 
00 05   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{5}}
$ ./cansend can5 005#FF.12.20
  (0000000093.061655)  can4  RX - -  005   [3]  FF 12 20
$ ./cansend can5 005#FF.12.21
  (0000000094.667557)  can4  RX - -  005   [3]  FF 12 21
$ ./cansend can5 005#FF.12.22
  (0000000095.617019)  can4  RX - -  005   [3]  FF 12 22
$ ./cansend can5 20000000#
  (0000000097.883071)  can4  RX - -  20000080   [8]  00 00 00 00 00 00 
00 03   ERRORFRAME
	bus-error
	error-counter-tx-rx{{0}{3}}


> Which CAN microcontroller is used inside the device?

It is an IP developed by Kvaser called KCAN. I'm not sure, but I think
it has only been used in FPGAs.

> Also, could you point me to the Kvaser hydra product page? I
> couldn't find it with a quick search so I figured out that maybe
> the commercial name is different?

Hydra is the name of the platform for the USB based products.
You find all devices with this capability here:
https://www.kvaser.com/products-services/our-products/#/?descriptors=can_fd,err_frame_gen&pc_int=mini-pci-express,pci-express,usb
As mentioned before, the SocketCAN driver for the PCIe based devices
does not support this.

>> We also got this implemented in the early version of kvaser_pciefd driver, but dropped it:
>> https://marc.info/?l=linux-can&m=154324867704480&w=2
>> Is this something that we should remove from kvasr_usb aswell?
> 
> No. My intent is not to ask you to remove it. It is rather the
> opposite: I want to understand more how you are able to achieve
> what I thought not to be possible. For what you told me, the
> Kvaser hydra is the kind of device which I would like to have in
> my testing environment :)
> 
> However, it would make sense to normalize this use case in the
> Socket CAN interface. For the discussion with Oliver, it is clear
> that this use case is currently not expected and undefined, the
> reason being that such a CAN controller was not thought to
> exist. Now that you have proved us wrong, things are different.

Right, it would be nice to sort this out. I prefer to keep the
functionality, since we got customers using it.

Regards,
jimmy

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-01 15:41       ` Jimmy Assarsson
@ 2021-02-02  0:22         ` Vincent MAILHOL
  2021-02-02  7:35           ` Marc Kleine-Budde
  2021-02-02  9:59           ` Jimmy Assarsson
  0 siblings, 2 replies; 24+ messages in thread
From: Vincent MAILHOL @ 2021-02-02  0:22 UTC (permalink / raw)
  To: Jimmy Assarsson
  Cc: Jimmy Assarsson, Oliver Hartkopp, linux-can, Marc Kleine-Budde

Hi Jimmy,

On Tue. 2 Feb 2021 at 00:41, Jimmy Assarsson <extja@kvaser.com> wrote:
> On 2021-02-01 15:19, Vincent MAILHOL wrote:
> > Hi Jimmy,
> > On Mon. 1 Feb 2021 at 05:42, Jimmy Assarsson <jimmyassarsson@gmail.com> wrote:
> >> On 2021-01-31 13:59, Oliver Hartkopp wrote:
> >>> Hi Vincent,
> >>> On 31.01.21 07:22, Vincent MAILHOL wrote:
> >>>> Hello,
> >>>>
> >>>> The socket CAN API handles the CAN errors (as reported by the
> >>>> microcontroller) by emitting some RX CAN frames with the
> >>>> CAN_ERR_FLAG set.
> >>>
> >>> Yes. This is the only intention.
> >>>
> >>>> My question concerns the transmission path: I would like to
> >>>> understand how drivers should handle *TX* CAN frames which have
> >>>> the CAN_ERR_FLAG set.
> >>>>
> >>>> The socket API allows sending such frames. For example doing:
> >>>>       cansend can0 20000123#0011223344556677
> >>>> will generate such frames and it will reach the xmit() function of
> >>>> the driver.
> >>>
> >>> The reason to pass the frame as-is to the driver layer (including CAN_ERR_FLAG) is the possibility to test the correct behavior on the RX path, e.g. when you use the vcan driver.
> >>>
> >>> On the sending path the CAN_ERR_FLAG has no functionality so far - at least it was not defined by the community.
> >>>
> >>>> However, contrary to the other flags (EFF, RTR, FDF, BRS), the
> >>>> ERR flag is not present on the data link layer. Instead, the data
> >>>> link layer is responsible for detecting errors and signaling those
> >>>> as soon as they occur (thus interrupting the transmission).
> >>>>
> >>>> While the ISO standard does not explicitly forbid having upper
> >>>> layers generating such frames, it is not documented. Also, I am
> >>>> not aware of CAN controllers allowing to generate error frames on
> >>>> demand.
> >>>
> >>> There are specialized CAN testers, e.g. IIRC Vector CANstress that can generate error frames on specific conditions (e.g. when detecting a specific CAN ID).
> >>>
> >>> But I heave not seen CAN controllers that provide such functionality.
> >>>
> >>>> My initial expectation is that those error frames only make
> >>>> sense in the RX path and that we should drop such TX frames in,
> >>>> for example, can_dropped_invalid_skb().
> >>>
> >>> No. As written above the bit is defined to be valid in the RX path only and it makes sense for testing.
> >>>
> >>>> However, after looking at the code of other drivers, it appears
> >>>> that one (and only one) of them: the Kvaser hydra, does actually
> >>>> check this CAN_ERR_FLAG flag in the TX path:
> >>>> https://elixir.bootlin.com/linux/v5.11-rc5/source/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c#L1421
> >>>>
> >>>> I would be thankful if anyone knowledgeable about the Kvaser hydra
> >>>> could explain to me how the device handles those error frames.
> >>>
> >>> o_O - Yes, would be interested too!!
> >>
> >> Hi Vincent and Oliver,
> >>
> >> When the user passes a frame with CAN_ERR_FLAG set, the CAN controller will generate an error frame.
> >> We got customers that use this for testing system robustness and fault reporting/handling.
> >
> > Interesting.
> >
> > There are two forms of error flags:
> >    - The active error flag: 6 consecutive dominant bits
> >    - The passive error flag: 6 consecutive recessive bits
> > Can it generate both or only one of these? Is it generated as
> > soon as the device receives the command or is it generated at a
> > predefined timing (e.g. beginning of the frame)?
>
> It can only generate the active error flag.
> It will generate an error frame on the bus as soon as the bus is idle.
>
> $ sudo ip link set can4 type can bitrate 100000
> $ sudo ip link set can5 type can bitrate 100000
> $ sudo ip link set can4 up
> $ sudo ip link set can5 up
> $ ./candump -c -ta -H -d -e -x can4,#FFFFFFFF &
> [1] 177884
> $ ./cansend can5 20000000#
>   (0000000084.786201)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
> 00 01   ERRORFRAME
>         bus-error
>         error-counter-tx-rx{{0}{1}}
> $ ./cansend can5 20000123#0011223344556677
>   (0000000086.798898)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
> 00 02   ERRORFRAME
>         bus-error
>         error-counter-tx-rx{{0}{2}}
> $ ./cansend can5 20000000#
>   (0000000087.385292)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
> 00 03   ERRORFRAME
>         bus-error
>         error-counter-tx-rx{{0}{3}}
> $ ./cansend can5 20000000#
>   (0000000087.911860)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
> 00 04   ERRORFRAME
>         bus-error
>         error-counter-tx-rx{{0}{4}}
> $ ./cansend can5 20000000#
>   (0000000088.404890)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
> 00 05   ERRORFRAME
>         bus-error
>         error-counter-tx-rx{{0}{5}}
> $ ./cansend can5 005#FF.12.20
>   (0000000093.061655)  can4  RX - -  005   [3]  FF 12 20
> $ ./cansend can5 005#FF.12.21
>   (0000000094.667557)  can4  RX - -  005   [3]  FF 12 21
> $ ./cansend can5 005#FF.12.22
>   (0000000095.617019)  can4  RX - -  005   [3]  FF 12 22
> $ ./cansend can5 20000000#
>   (0000000097.883071)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
> 00 03   ERRORFRAME
>         bus-error
>         error-counter-tx-rx{{0}{3}}

Thanks for the example, things are now clear.

So the error flag always increments the RX counter even if you
are the transmitter.

>
> > Which CAN microcontroller is used inside the device?
>
> It is an IP developed by Kvaser called KCAN. I'm not sure, but I think
> it has only been used in FPGAs.

This makes more sense. As I wrote before, I did not know of any
microcontroller allowing you to generate error flags on
demand. So you build your device on an FPGA to be able to control
the bit level. Nice!

I also once wanted to use an FPGA to play with error flags but it
only remained an idea.

> > Also, could you point me to the Kvaser hydra product page? I
> > couldn't find it with a quick search so I figured out that maybe
> > the commercial name is different?
>
> Hydra is the name of the platform for the USB based products.
> You find all devices with this capability here:
> https://www.kvaser.com/products-services/our-products/#/?descriptors=can_fd,err_frame_gen&pc_int=mini-pci-express,pci-express,usb
> As mentioned before, the SocketCAN driver for the PCIe based devices
> does not support this.

Thanks!

> >> We also got this implemented in the early version of kvaser_pciefd driver, but dropped it:
> >> https://marc.info/?l=linux-can&m=154324867704480&w=2
> >> Is this something that we should remove from kvasr_usb aswell?
> >
> > No. My intent is not to ask you to remove it. It is rather the
> > opposite: I want to understand more how you are able to achieve
> > what I thought not to be possible. For what you told me, the
> > Kvaser hydra is the kind of device which I would like to have in
> > my testing environment :)
> >
> > However, it would make sense to normalize this use case in the
> > Socket CAN interface. For the discussion with Oliver, it is clear
> > that this use case is currently not expected and undefined, the
> > reason being that such a CAN controller was not thought to
> > exist. Now that you have proved us wrong, things are different.
>
> Right, it would be nice to sort this out. I prefer to keep the
> functionality, since we got customers using it.

Basically, I would see this as an expert function: add a
CAN_CTRLMODE_TX_ERR and have the user explicitly enable the
feature through netlink when configuring the interface. The
rationale is to prevent by default an unprivileged application
from messing with the bus.

If CAN_CTRLMODE_TX_ERR is on the device generates an error
flag. Else, the CAN_ERR_FLAG is simply ignored (masked out).
The CAN ID, DLC and payload of the TX error frames are
ignored (i.e. reserved for future).

I do not see the need for more complex logic at the moment
because your device is only capable of generating one type of
error flags: the active error. If one day a device has the
ability to generate both the active and passive error flags, we
should then define how to send those (maybe by putting a flag in
the payload, similar to what is done on the RX path).

What do you think of the above?


Yours sincerely,
Vincent

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02  0:22         ` Vincent MAILHOL
@ 2021-02-02  7:35           ` Marc Kleine-Budde
  2021-02-02  8:23             ` Kurt Van Dijck
  2021-02-02  9:59           ` Jimmy Assarsson
  1 sibling, 1 reply; 24+ messages in thread
From: Marc Kleine-Budde @ 2021-02-02  7:35 UTC (permalink / raw)
  To: Vincent MAILHOL, Jimmy Assarsson
  Cc: Jimmy Assarsson, Oliver Hartkopp, linux-can


[-- Attachment #1.1: Type: text/plain, Size: 1590 bytes --]

On 2/2/21 1:22 AM, Vincent MAILHOL wrote:
[...]

>> Right, it would be nice to sort this out. I prefer to keep the
>> functionality, since we got customers using it.
> 
> Basically, I would see this as an expert function: add a
> CAN_CTRLMODE_TX_ERR and have the user explicitly enable the
> feature through netlink when configuring the interface. The
> rationale is to prevent by default an unprivileged application
> from messing with the bus.

The CAN_CTRLMODE_TX_ERR would be a per device option. Another option might be a
sockopt, where you have to enable the TX_ERR explicitly. I'm not sure, which
option is the best here.

> If CAN_CTRLMODE_TX_ERR is on the device generates an error
> flag. Else, the CAN_ERR_FLAG is simply ignored (masked out).
> The CAN ID, DLC and payload of the TX error frames are
> ignored (i.e. reserved for future).
> 
> I do not see the need for more complex logic at the moment
> because your device is only capable of generating one type of
> error flags: the active error. If one day a device has the
> ability to generate both the active and passive error flags, we
> should then define how to send those (maybe by putting a flag in
> the payload, similar to what is done on the RX path).
> 
> What do you think of the above?

Sounds good.

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: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02  7:35           ` Marc Kleine-Budde
@ 2021-02-02  8:23             ` Kurt Van Dijck
  2021-02-02  8:41               ` Marc Kleine-Budde
  0 siblings, 1 reply; 24+ messages in thread
From: Kurt Van Dijck @ 2021-02-02  8:23 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Vincent MAILHOL, Jimmy Assarsson, Jimmy Assarsson,
	Oliver Hartkopp, linux-can

On Tue, 02 Feb 2021 08:35:14 +0100, Marc Kleine-Budde wrote:
> On 2/2/21 1:22 AM, Vincent MAILHOL wrote:
> [...]
> 
> >> Right, it would be nice to sort this out. I prefer to keep the
> >> functionality, since we got customers using it.
> > 
> > Basically, I would see this as an expert function: add a
> > CAN_CTRLMODE_TX_ERR and have the user explicitly enable the
> > feature through netlink when configuring the interface. The
> > rationale is to prevent by default an unprivileged application
> > from messing with the bus.
> 
> The CAN_CTRLMODE_TX_ERR would be a per device option. Another option might be a
> sockopt, where you have to enable the TX_ERR explicitly. I'm not sure, which
> option is the best here.

a sockopt is only correct if it can detect that the device underneath
has CAN_CTRLMODE_TX_ERR capability.

So I'd think we start with adding the CAN_CTRLMODE_TX_ERR to the driver level.

It would allow to see if a driver will behave properly with CAN_ERR_FLAG
can_frames in the tx path.

> 
> > If CAN_CTRLMODE_TX_ERR is on the device generates an error
> > flag. Else, the CAN_ERR_FLAG is simply ignored (masked out).
> > The CAN ID, DLC and payload of the TX error frames are
> > ignored (i.e. reserved for future).

IMO, can_frames in the tx path with CAN_ERR_FLAG should be dropped
if the driver can't handle them. vcan in this regard is capable of
handling those, as does the kvaser usb.

I think it's wrong that CAN_ERR_FLAG messages would appear as regular
frame on CAN, as happens today if I understood well.

> > 
> > I do not see the need for more complex logic at the moment
> > because your device is only capable of generating one type of
> > error flags: the active error. If one day a device has the
> > ability to generate both the active and passive error flags, we
> > should then define how to send those (maybe by putting a flag in
> > the payload, similar to what is done on the RX path).

ack.

Kurt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02  8:23             ` Kurt Van Dijck
@ 2021-02-02  8:41               ` Marc Kleine-Budde
  2021-02-02  9:00                 ` Oliver Hartkopp
  0 siblings, 1 reply; 24+ messages in thread
From: Marc Kleine-Budde @ 2021-02-02  8:41 UTC (permalink / raw)
  To: Vincent MAILHOL, Jimmy Assarsson, Jimmy Assarsson,
	Oliver Hartkopp, linux-can


[-- Attachment #1.1: Type: text/plain, Size: 2252 bytes --]

On 2/2/21 9:23 AM, Kurt Van Dijck wrote:
>>>> Right, it would be nice to sort this out. I prefer to keep the
>>>> functionality, since we got customers using it.
>>>
>>> Basically, I would see this as an expert function: add a
>>> CAN_CTRLMODE_TX_ERR and have the user explicitly enable the
>>> feature through netlink when configuring the interface. The
>>> rationale is to prevent by default an unprivileged application
>>> from messing with the bus.
>>
>> The CAN_CTRLMODE_TX_ERR would be a per device option. Another option might be a
>> sockopt, where you have to enable the TX_ERR explicitly. I'm not sure, which
>> option is the best here.
> 
> a sockopt is only correct if it can detect that the device underneath
> has CAN_CTRLMODE_TX_ERR capability.

ACK

The user space use case would be:

- fd = socket()
- bind(fd, "can0")
- setsockopt(fd, SOCKOPT_TX_ERR)

The raw_setsockopt() in the kernel can check the CAN devices supported ctrl modes.

> So I'd think we start with adding the CAN_CTRLMODE_TX_ERR to the driver
> level.

ACK

> It would allow to see if a driver will behave properly with CAN_ERR_FLAG 
> can_frames in the tx path.

ACK

>>> If CAN_CTRLMODE_TX_ERR is on the device generates an error
>>> flag. Else, the CAN_ERR_FLAG is simply ignored (masked out).
>>> The CAN ID, DLC and payload of the TX error frames are
>>> ignored (i.e. reserved for future).
> 
> IMO, can_frames in the tx path with CAN_ERR_FLAG should be dropped
> if the driver can't handle them. vcan in this regard is capable of
> handling those, as does the kvaser usb.

Makes sense. The implementation steps could be:
- convert can_dropped_invalid_skb() from static inline to
  regular function
- add check for CAN_ERR_FLAG and enabled CAN_CTRLMODE_TX_ERR
  to can_dropped_invalid_skb()

> I think it's wrong that CAN_ERR_FLAG messages would appear as regular
> frame on CAN, as happens today if I understood well.

ACK

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: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02  8:41               ` Marc Kleine-Budde
@ 2021-02-02  9:00                 ` Oliver Hartkopp
  2021-02-02  9:05                   ` Marc Kleine-Budde
  0 siblings, 1 reply; 24+ messages in thread
From: Oliver Hartkopp @ 2021-02-02  9:00 UTC (permalink / raw)
  To: Marc Kleine-Budde, Vincent MAILHOL, Jimmy Assarsson,
	Jimmy Assarsson, linux-can



On 02.02.21 09:41, Marc Kleine-Budde wrote:
> On 2/2/21 9:23 AM, Kurt Van Dijck wrote:
>>>>> Right, it would be nice to sort this out. I prefer to keep the
>>>>> functionality, since we got customers using it.
>>>>
>>>> Basically, I would see this as an expert function: add a
>>>> CAN_CTRLMODE_TX_ERR and have the user explicitly enable the
>>>> feature through netlink when configuring the interface. The
>>>> rationale is to prevent by default an unprivileged application
>>>> from messing with the bus.
>>>
>>> The CAN_CTRLMODE_TX_ERR would be a per device option. Another option might be a
>>> sockopt, where you have to enable the TX_ERR explicitly. I'm not sure, which
>>> option is the best here.
>>
>> a sockopt is only correct if it can detect that the device underneath
>> has CAN_CTRLMODE_TX_ERR capability.
> 
> ACK
> 
> The user space use case would be:
> 
> - fd = socket()
> - bind(fd, "can0")
> - setsockopt(fd, SOCKOPT_TX_ERR)
> 
> The raw_setsockopt() in the kernel can check the CAN devices supported ctrl modes.
> 
>> So I'd think we start with adding the CAN_CTRLMODE_TX_ERR to the driver
>> level.
> 
> ACK
> 
>> It would allow to see if a driver will behave properly with CAN_ERR_FLAG
>> can_frames in the tx path.
> 
> ACK
> 
>>>> If CAN_CTRLMODE_TX_ERR is on the device generates an error
>>>> flag. Else, the CAN_ERR_FLAG is simply ignored (masked out).
>>>> The CAN ID, DLC and payload of the TX error frames are
>>>> ignored (i.e. reserved for future).
>>
>> IMO, can_frames in the tx path with CAN_ERR_FLAG should be dropped
>> if the driver can't handle them. vcan in this regard is capable of
>> handling those, as does the kvaser usb.
> 
> Makes sense. The implementation steps could be:
> - convert can_dropped_invalid_skb() from static inline to
>    regular function
> - add check for CAN_ERR_FLAG and enabled CAN_CTRLMODE_TX_ERR
>    to can_dropped_invalid_skb()
> 

Which means the vcan has to support CAN_CTRLMODE_TX_ERR too.

>> I think it's wrong that CAN_ERR_FLAG messages would appear as regular
>> frame on CAN, as happens today if I understood well.
> 
> ACK

What happens if you send a valid CAN frame with CAN_ERR_FLAG set?

I did not check it but I assume the frame is sent as normal frame and 
the echo'ed CAN frame would *only* go through the error message filter 
bank in af_can.c.

This is probably not what we want for 'real' CAN devices, so we might 
have to take a look at this too.

Regards,
Oliver

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02  9:00                 ` Oliver Hartkopp
@ 2021-02-02  9:05                   ` Marc Kleine-Budde
  2021-02-02  9:16                     ` Oliver Hartkopp
  0 siblings, 1 reply; 24+ messages in thread
From: Marc Kleine-Budde @ 2021-02-02  9:05 UTC (permalink / raw)
  To: Oliver Hartkopp, Vincent MAILHOL, Jimmy Assarsson,
	Jimmy Assarsson, linux-can


[-- Attachment #1.1: Type: text/plain, Size: 1647 bytes --]

On 2/2/21 10:00 AM, Oliver Hartkopp wrote:
>>> IMO, can_frames in the tx path with CAN_ERR_FLAG should be dropped
>>> if the driver can't handle them. vcan in this regard is capable of
>>> handling those, as does the kvaser usb.
>>
>> Makes sense. The implementation steps could be:
>> - convert can_dropped_invalid_skb() from static inline to
>>    regular function
>> - add check for CAN_ERR_FLAG and enabled CAN_CTRLMODE_TX_ERR
>>    to can_dropped_invalid_skb()
>>
> 
> Which means the vcan has to support CAN_CTRLMODE_TX_ERR too.
> 
>>> I think it's wrong that CAN_ERR_FLAG messages would appear as regular
>>> frame on CAN, as happens today if I understood well.
>>
>> ACK
> 
> What happens if you send a valid CAN frame with CAN_ERR_FLAG set?
> 
> I did not check it but I assume the frame is sent as normal frame and 
> the echo'ed CAN frame would *only* go through the error message filter 
> bank in af_can.c.

If CAN_CTRLMODE_TX_ERR has been added to the kernel and
can_dropped_invalid_skb() is updated, then a CAN frame with CAN_ERR_FLAG set
would be either send as an error frame or dropped by can_dropped_invalid_skb().

So it would be echoed only if the driver supports CAN_CTRLMODE_TX_ERR and it's
enabled.

> This is probably not what we want for 'real' CAN devices, so we might 
> have to take a look at this too.

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: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02  9:05                   ` Marc Kleine-Budde
@ 2021-02-02  9:16                     ` Oliver Hartkopp
  2021-02-02 10:00                       ` Vincent MAILHOL
  0 siblings, 1 reply; 24+ messages in thread
From: Oliver Hartkopp @ 2021-02-02  9:16 UTC (permalink / raw)
  To: Marc Kleine-Budde, Vincent MAILHOL, Jimmy Assarsson,
	Jimmy Assarsson, linux-can



On 02.02.21 10:05, Marc Kleine-Budde wrote:
> On 2/2/21 10:00 AM, Oliver Hartkopp wrote:
>>>> IMO, can_frames in the tx path with CAN_ERR_FLAG should be dropped
>>>> if the driver can't handle them. vcan in this regard is capable of
>>>> handling those, as does the kvaser usb.
>>>
>>> Makes sense. The implementation steps could be:
>>> - convert can_dropped_invalid_skb() from static inline to
>>>     regular function
>>> - add check for CAN_ERR_FLAG and enabled CAN_CTRLMODE_TX_ERR
>>>     to can_dropped_invalid_skb()
>>>
>>
>> Which means the vcan has to support CAN_CTRLMODE_TX_ERR too.
>>
>>>> I think it's wrong that CAN_ERR_FLAG messages would appear as regular
>>>> frame on CAN, as happens today if I understood well.
>>>
>>> ACK
>>
>> What happens if you send a valid CAN frame with CAN_ERR_FLAG set?
>>
>> I did not check it but I assume the frame is sent as normal frame and
>> the echo'ed CAN frame would *only* go through the error message filter
>> bank in af_can.c.
> 
> If CAN_CTRLMODE_TX_ERR has been added to the kernel and
> can_dropped_invalid_skb() is updated, then a CAN frame with CAN_ERR_FLAG set
> would be either send as an error frame or dropped by can_dropped_invalid_skb().
> 
> So it would be echoed only if the driver supports CAN_CTRLMODE_TX_ERR and it's
> enabled.

ACK.

So a second time Vincent found a mismatch nobody cared about so far. 
Congrats, Vincent!
(no irony here)

;-)

Best,
Oliver

>> This is probably not what we want for 'real' CAN devices, so we might
>> have to take a look at this too.
> 
> Marc
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02  0:22         ` Vincent MAILHOL
  2021-02-02  7:35           ` Marc Kleine-Budde
@ 2021-02-02  9:59           ` Jimmy Assarsson
  1 sibling, 0 replies; 24+ messages in thread
From: Jimmy Assarsson @ 2021-02-02  9:59 UTC (permalink / raw)
  To: Vincent MAILHOL
  Cc: Jimmy Assarsson, Oliver Hartkopp, linux-can, Marc Kleine-Budde

On 2021-02-02 01:22, Vincent MAILHOL wrote> On Tue. 2 Feb 2021 at 00:41, 
Jimmy Assarsson <extja@kvaser.com> wrote:
>> On 2021-02-01 15:19, Vincent MAILHOL wrote:
>>> On Mon. 1 Feb 2021 at 05:42, Jimmy Assarsson <jimmyassarsson@gmail.com> wrote:
...
>>> There are two forms of error flags:
>>>     - The active error flag: 6 consecutive dominant bits
>>>     - The passive error flag: 6 consecutive recessive bits
>>> Can it generate both or only one of these? Is it generated as
>>> soon as the device receives the command or is it generated at a
>>> predefined timing (e.g. beginning of the frame)?
>>
>> It can only generate the active error flag.
>> It will generate an error frame on the bus as soon as the bus is idle.
>>
>> $ sudo ip link set can4 type can bitrate 100000
>> $ sudo ip link set can5 type can bitrate 100000
>> $ sudo ip link set can4 up
>> $ sudo ip link set can5 up
>> $ ./candump -c -ta -H -d -e -x can4,#FFFFFFFF &
>> [1] 177884
>> $ ./cansend can5 20000000#
>>    (0000000084.786201)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
>> 00 01   ERRORFRAME
>>          bus-error
>>          error-counter-tx-rx{{0}{1}}
>> $ ./cansend can5 20000123#0011223344556677
>>    (0000000086.798898)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
>> 00 02   ERRORFRAME
>>          bus-error
>>          error-counter-tx-rx{{0}{2}}
>> $ ./cansend can5 20000000#
>>    (0000000087.385292)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
>> 00 03   ERRORFRAME
>>          bus-error
>>          error-counter-tx-rx{{0}{3}}
>> $ ./cansend can5 20000000#
>>    (0000000087.911860)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
>> 00 04   ERRORFRAME
>>          bus-error
>>          error-counter-tx-rx{{0}{4}}
>> $ ./cansend can5 20000000#
>>    (0000000088.404890)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
>> 00 05   ERRORFRAME
>>          bus-error
>>          error-counter-tx-rx{{0}{5}}
>> $ ./cansend can5 005#FF.12.20
>>    (0000000093.061655)  can4  RX - -  005   [3]  FF 12 20
>> $ ./cansend can5 005#FF.12.21
>>    (0000000094.667557)  can4  RX - -  005   [3]  FF 12 21
>> $ ./cansend can5 005#FF.12.22
>>    (0000000095.617019)  can4  RX - -  005   [3]  FF 12 22
>> $ ./cansend can5 20000000#
>>    (0000000097.883071)  can4  RX - -  20000080   [8]  00 00 00 00 00 00
>> 00 03   ERRORFRAME
>>          bus-error
>>          error-counter-tx-rx{{0}{3}}
> 
> Thanks for the example, things are now clear.
> 
> So the error flag always increments the RX counter even if you
> are the transmitter.

To be clear, the printouts (from candump) are from the interface
that is not transmitting the error frame (can4). I transmit error
frames with can5. However, the transmitting interface (can5) will
also detect and report the error frame (and increase the rx error
counter). The only difference in the above example would be that the rx
error counter wont decrement when can5 successfully transmits a CAN
frame.
Sorry for the confusion.

Regards,
jimmy

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02  9:16                     ` Oliver Hartkopp
@ 2021-02-02 10:00                       ` Vincent MAILHOL
  2021-02-02 10:14                         ` Oliver Hartkopp
  2021-02-02 12:14                         ` Jimmy Assarsson
  0 siblings, 2 replies; 24+ messages in thread
From: Vincent MAILHOL @ 2021-02-02 10:00 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Marc Kleine-Budde, Jimmy Assarsson, Jimmy Assarsson, linux-can

On Tue. 2 Feb 2021 à 18:16, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> On 02.02.21 10:05, Marc Kleine-Budde wrote:
> > On 2/2/21 10:00 AM, Oliver Hartkopp wrote:
> >>>> IMO, can_frames in the tx path with CAN_ERR_FLAG should be dropped
> >>>> if the driver can't handle them. vcan in this regard is capable of
> >>>> handling those, as does the kvaser usb.
> >>>
> >>> Makes sense. The implementation steps could be:
> >>> - convert can_dropped_invalid_skb() from static inline to
> >>>     regular function
> >>> - add check for CAN_ERR_FLAG and enabled CAN_CTRLMODE_TX_ERR
> >>>     to can_dropped_invalid_skb()

Back to the idea of my initial e-mail :)

> >> Which means the vcan has to support CAN_CTRLMODE_TX_ERR too.
> >>
> >>>> I think it's wrong that CAN_ERR_FLAG messages would appear as regular
> >>>> frame on CAN, as happens today if I understood well.
> >>>
> >>> ACK

If we could go in the past, those should have appeared on the error socket.

> >> What happens if you send a valid CAN frame with CAN_ERR_FLAG set?
> >>
> >> I did not check it but I assume the frame is sent as normal frame and
> >> the echo'ed CAN frame would *only* go through the error message filter
> >> bank in af_can.c.
> >
> > If CAN_CTRLMODE_TX_ERR has been added to the kernel and
> > can_dropped_invalid_skb() is updated, then a CAN frame with CAN_ERR_FLAG set
> > would be either send as an error frame or dropped by can_dropped_invalid_skb().
> >
> > So it would be echoed only if the driver supports CAN_CTRLMODE_TX_ERR and it's
> > enabled.
>
> ACK.

The echo would be special here.  We have to remember that the
payload of the CAN_ERR_FLAG frames is an arbitrary design. This
payload has no meaning for the data link.

When we echo back the frame, only the DLC, CAN ID and payload of
the TX frame are irrelevant (except for vcan).

My current idea would be to follow what Kvaser did: they send the
frame and the device reports the error flag (c.f. the example
given by Jimmy above). So the echo feature would not be used
for error flags.

@Jimmy, could you confirm my understanding? Do you use the echo
functionality for error flags in the Kvaser driver?

> So a second time Vincent found a mismatch nobody cared about so far.
> Congrats, Vincent!
> (no irony here)
>
> ;-)

Thanks, this is me wearing my hacker's goggles.

> >> This is probably not what we want for 'real' CAN devices, so we might
> >> have to take a look at this too.
> >
> > Marc
> >

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02 10:00                       ` Vincent MAILHOL
@ 2021-02-02 10:14                         ` Oliver Hartkopp
  2021-02-02 11:12                           ` Vincent MAILHOL
  2021-02-02 12:14                         ` Jimmy Assarsson
  1 sibling, 1 reply; 24+ messages in thread
From: Oliver Hartkopp @ 2021-02-02 10:14 UTC (permalink / raw)
  To: Vincent MAILHOL
  Cc: Marc Kleine-Budde, Jimmy Assarsson, Jimmy Assarsson, linux-can



On 02.02.21 11:00, Vincent MAILHOL wrote:

>>> If CAN_CTRLMODE_TX_ERR has been added to the kernel and
>>> can_dropped_invalid_skb() is updated, then a CAN frame with CAN_ERR_FLAG set
>>> would be either send as an error frame or dropped by can_dropped_invalid_skb().
>>>
>>> So it would be echoed only if the driver supports CAN_CTRLMODE_TX_ERR and it's
>>> enabled.
>>
>> ACK.
> 
> The echo would be special here.  We have to remember that the
> payload of the CAN_ERR_FLAG frames is an arbitrary design. This
> payload has no meaning for the data link.
> 
> When we echo back the frame, only the DLC, CAN ID and payload of
> the TX frame are irrelevant (except for vcan).
> 
> My current idea would be to follow what Kvaser did: they send the
> frame and the device reports the error flag (c.f. the example
> given by Jimmy above). So the echo feature would not be used
> for error flags.

In fact I would suggest to think a about a proper API (aka CAN frame 
content definition) for outgoing CAN frames with the CAN_ERR_FLAG set.

The CAN_ERR_FLAG has been renamed in the documentation to indicate 
"error messages" from the CAN controller, as an error frame is something 
completely different.

Now as we are talking about having CAN_ERR_FLAG in the TX path besides 
the vcan testing stuff, we should think about an API for the really 
outgoing frames.

We could not only think about "create an error frame right now" but also 
think about a more intelligent CAN node, which also offers to destroy 
one or more specific CAN ID(s) at a specific bit position after 
detecting that CAN ID.

We could use the CAN_RTR_FLAG and the data[] section of the outgoing 
error CAN frames for such an API.

Regards,
Oliver

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02 10:14                         ` Oliver Hartkopp
@ 2021-02-02 11:12                           ` Vincent MAILHOL
  2021-02-02 19:19                             ` Oliver Hartkopp
  0 siblings, 1 reply; 24+ messages in thread
From: Vincent MAILHOL @ 2021-02-02 11:12 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Marc Kleine-Budde, Jimmy Assarsson, Jimmy Assarsson, linux-can

On Tue. 2 févr. 2021 à 19:14, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> On 02.02.21 11:00, Vincent MAILHOL wrote:
> >>> If CAN_CTRLMODE_TX_ERR has been added to the kernel and
> >>> can_dropped_invalid_skb() is updated, then a CAN frame with CAN_ERR_FLAG set
> >>> would be either send as an error frame or dropped by can_dropped_invalid_skb().
> >>>
> >>> So it would be echoed only if the driver supports CAN_CTRLMODE_TX_ERR and it's
> >>> enabled.
> >>
> >> ACK.
> >
> > The echo would be special here.  We have to remember that the
> > payload of the CAN_ERR_FLAG frames is an arbitrary design. This
> > payload has no meaning for the data link.
> >
> > When we echo back the frame, only the DLC, CAN ID and payload of
> > the TX frame are irrelevant (except for vcan).
> >
> > My current idea would be to follow what Kvaser did: they send the
> > frame and the device reports the error flag (c.f. the example
> > given by Jimmy above). So the echo feature would not be used
> > for error flags.
>
> In fact I would suggest to think a about a proper API (aka CAN frame
> content definition) for outgoing CAN frames with the CAN_ERR_FLAG set.
>
> The CAN_ERR_FLAG has been renamed in the documentation to indicate
> "error messages" from the CAN controller, as an error frame is something
> completely different.
>
> Now as we are talking about having CAN_ERR_FLAG in the TX path besides
> the vcan testing stuff, we should think about an API for the really
> outgoing frames.
>
> We could not only think about "create an error frame right now" but also
> think about a more intelligent CAN node, which also offers to destroy
> one or more specific CAN ID(s) at a specific bit position after
> detecting that CAN ID.

My original idea was to leave it unspecified until a device was
actually capable of doing such a thing. But I am not against
defining the API now :) We might just have to wait a long time
for someone to actually implement it.

> We could use the CAN_RTR_FLAG and the data[] section of the outgoing
> error CAN frames for such an API.

We are not limited to the CAN_RTR_FLAG and the data[]

First, we have to list the use cases.

As I wrote before, there are only two forms for the error flag:
  - The active error flag: 6 consecutive dominant bits
  - The passive error flag: 6 consecutive recessive bits

The device can either inject the flag either during:
  - bus idle
  - while it is transmitting a frame
  - while it is receiving a frame
The error flag can occur at any time.


My first proposal would be:

 1/ One flag to specify the error flag form (active or
    passive). Passive error flags can not be used when receiving
    a frame.

 2/ One flag to specify if we inject during bus idle or not.

 3/ One flag to specify if we inject while transmitting or
    receiving a frame.

 4/ If injecting while receiving, have the ability to specify
    the CAN ID, DLC and flag filters (EFF, SFF, RTR, BRS...) of
    the frame on which we want to inject the error flag. I do not
    see the need for a complex filter. I would propose to either
    have an exact match or no match (e.g. inject on next frame
    whatever this frame is).

 5/ One flag to specify if injection while transmitting is done
    on exact match or on any frames (if done on any frames,
    the filters 4/ are ignored). This flag is ignore for bus idle or
    while receiving.

 6/ If we inject while receiving, do it on the next normal frame
    we transmit (no filter). The userland have to send two frame:
    one with CAN_ERR_FLAG and one normal one on which we
    will inject the error flag in order to use the feature.

 7/ Have an offset: the bit index on which we want to inject the
    error flag. I am not yet sure if we should ignore bitstuffing
    or not. If the bus is idle, wait offset bit times before
    injecting. Else, start counting the bits from the Start of
    Frame.


The flags 1/, 2/, 3/, 5/ and the offset 7/ goes into the data[]
payload, the filters 3/ goes into their respective field (CAN ID,
DLC, FF, SFF, RTR, BRS flags).



Yours sincerely,
Vincent

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02 10:00                       ` Vincent MAILHOL
  2021-02-02 10:14                         ` Oliver Hartkopp
@ 2021-02-02 12:14                         ` Jimmy Assarsson
  1 sibling, 0 replies; 24+ messages in thread
From: Jimmy Assarsson @ 2021-02-02 12:14 UTC (permalink / raw)
  To: Vincent MAILHOL, Oliver Hartkopp
  Cc: Marc Kleine-Budde, Jimmy Assarsson, linux-can

On 2021-02-02 11:00, Vincent MAILHOL wrote:> The echo would be special 
here.  We have to remember that the
> payload of the CAN_ERR_FLAG frames is an arbitrary design. This
> payload has no meaning for the data link.
> 
> When we echo back the frame, only the DLC, CAN ID and payload of
> the TX frame are irrelevant (except for vcan).
> 
> My current idea would be to follow what Kvaser did: they send the
> frame and the device reports the error flag (c.f. the example
> given by Jimmy above). So the echo feature would not be used
> for error flags.
> 
> @Jimmy, could you confirm my understanding? Do you use the echo
> functionality for error flags in the Kvaser driver?

I had to double check how this works.
When we get an skb with a frame where CAN_EFF_FLAG is set,
we will set the corresponding bit for the CAN controller, and
copy the rest of frame content (id, flags and data), and call
can_put_echo_skb(). Same as we do for all other packages.

We will receive a "TX ACK" for this package (from the USB device), and
we call can_get_echo_skb(). However, it looks like the "TX ACK" is
dropped, probably because CAN_EFF_FLAG is set.

We will also receive the generated error frame, and eventually call
netif_rx(skb). It is the same behavior for all the devices connected to
the bus.


The "TX ACK" path in the kvaser_usb driver is wrong, and it happens to
be that the ACK is not passed to the user. We also increment the
tx_packets and tx_bytes (depending on the data in the frame) stats.
I'll fix this once we know what the API should look like.

Regards,
jimmy

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02 11:12                           ` Vincent MAILHOL
@ 2021-02-02 19:19                             ` Oliver Hartkopp
  2021-02-03  6:42                               ` Jimmy Assarsson
  0 siblings, 1 reply; 24+ messages in thread
From: Oliver Hartkopp @ 2021-02-02 19:19 UTC (permalink / raw)
  To: Vincent MAILHOL
  Cc: Marc Kleine-Budde, Jimmy Assarsson, Jimmy Assarsson, linux-can



On 02.02.21 12:12, Vincent MAILHOL wrote:

>> The CAN_ERR_FLAG has been renamed in the documentation to indicate
>> "error messages" from the CAN controller, as an error frame is something
>> completely different.
>>
>> Now as we are talking about having CAN_ERR_FLAG in the TX path besides
>> the vcan testing stuff, we should think about an API for the really
>> outgoing frames.
>>
>> We could not only think about "create an error frame right now" but also
>> think about a more intelligent CAN node, which also offers to destroy
>> one or more specific CAN ID(s) at a specific bit position after
>> detecting that CAN ID.
> 
> My original idea was to leave it unspecified until a device was
> actually capable of doing such a thing. But I am not against
> defining the API now :) We might just have to wait a long time
> for someone to actually implement it.
> 
>> We could use the CAN_RTR_FLAG and the data[] section of the outgoing
>> error CAN frames for such an API.
> 
> We are not limited to the CAN_RTR_FLAG and the data[]
> 
> First, we have to list the use cases.
> 
> As I wrote before, there are only two forms for the error flag:
>    - The active error flag: 6 consecutive dominant bits
>    - The passive error flag: 6 consecutive recessive bits

IMO this passive error flag stuff is pointless.
The passive error does not have any effect on the bus. Nobody sees it. 
It's just a measure to continue counting error counters inside the CAN 
controller. IMO it's a read-only feature about the controller internal 
status.

> The device can either inject the flag either during:
>    - bus idle

Is an error flag defined at bus idle?

Error flags are intended to destroy *other CAN controllers* 
transmissions when detecting protocol violations. There can not be a 
protocol violation at idle time, right?

>    - while it is transmitting a frame

What's the use-case for destroying your own transmission?

>    - while it is receiving a frame

This makes sense, especially when you can destroy specific CAN ID frames 
at a specific bit position.
Or for any CAN ID at a specific bit position.
Or for any CAN ID at an undefined bit position.

> The error flag can occur at any time.

Sure? (see above)

Of course we might also provide some pump gun mode which just sends an 
error flag at some (any) time.

But for what reason?

Therefore I would massively reduce the following list:

> My first proposal would be:
> 
>   1/ One flag to specify the error flag form (active or
>      passive). Passive error flags can not be used when receiving
>      a frame.

NO

> 
>   2/ One flag to specify if we inject during bus idle or not.

NO

> 
>   3/ One flag to specify if we inject while transmitting or
>      receiving a frame.

NO - receiving only

> 
>   4/ If injecting while receiving, have the ability to specify
>      the CAN ID, DLC and flag filters (EFF, SFF, RTR, BRS...) of
>      the frame on which we want to inject the error flag. I do not
>      see the need for a complex filter. I would propose to either
>      have an exact match or no match (e.g. inject on next frame
>      whatever this frame is).

Yes. Additionally to a bit position after detecting the CAN ID, we could 
also provide these symbolic defines like 'BRS' from which position we 
want to start the error flag transmission.

But we should be careful not to make the job too complex for the sending 
CAN controller.

> 
>   5/ One flag to specify if injection while transmitting is done
>      on exact match or on any frames (if done on any frames,
>      the filters 4/ are ignored). This flag is ignore for bus idle or
>      while receiving.

NO - receiving only

> 
>   6/ If we inject while receiving, do it on the next normal frame
>      we transmit (no filter). The userland have to send two frame:
>      one with CAN_ERR_FLAG and one normal one on which we
>      will inject the error flag in order to use the feature.

Maybe the 'pump gun' could always hit the 'next' frame as default.

> 
>   7/ Have an offset: the bit index on which we want to inject the
>      error flag. I am not yet sure if we should ignore bitstuffing
>      or not. If the bus is idle, wait offset bit times before
>      injecting. Else, start counting the bits from the Start of
>      Frame.

See 4/

> 
> The flags 1/, 2/, 3/, 5/ and the offset 7/ goes into the data[]
> payload, the filters 3/ goes into their respective field (CAN ID,
> DLC, FF, SFF, RTR, BRS flags).
> 
> 

Regards,
Oliver

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-02 19:19                             ` Oliver Hartkopp
@ 2021-02-03  6:42                               ` Jimmy Assarsson
  2021-02-03  8:26                                 ` Vincent MAILHOL
  0 siblings, 1 reply; 24+ messages in thread
From: Jimmy Assarsson @ 2021-02-03  6:42 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Marc Kleine-Budde, Jimmy Assarsson, linux-can, Vincent MAILHOL

On 2021-02-02 20:19, Oliver Hartkopp wrote:
> On 02.02.21 12:12, Vincent MAILHOL wrote:
> 
>>> The CAN_ERR_FLAG has been renamed in the documentation to indicate
>>> "error messages" from the CAN controller, as an error frame is something
>>> completely different.
>>>
>>> Now as we are talking about having CAN_ERR_FLAG in the TX path besides
>>> the vcan testing stuff, we should think about an API for the really
>>> outgoing frames.
>>>
>>> We could not only think about "create an error frame right now" but also
>>> think about a more intelligent CAN node, which also offers to destroy
>>> one or more specific CAN ID(s) at a specific bit position after
>>> detecting that CAN ID.
>>
>> My original idea was to leave it unspecified until a device was
>> actually capable of doing such a thing. But I am not against
>> defining the API now :) We might just have to wait a long time
>> for someone to actually implement it.
>>
>>> We could use the CAN_RTR_FLAG and the data[] section of the outgoing
>>> error CAN frames for such an API.
>>
>> We are not limited to the CAN_RTR_FLAG and the data[]
>>
>> First, we have to list the use cases.
>>
>> As I wrote before, there are only two forms for the error flag:
>>    - The active error flag: 6 consecutive dominant bits
>>    - The passive error flag: 6 consecutive recessive bits
> 
> IMO this passive error flag stuff is pointless.
> The passive error does not have any effect on the bus. Nobody sees it. It's just a measure to continue counting error counters inside the CAN controller. IMO it's a read-only feature about the controller internal status.

I agree.

>> The device can either inject the flag either during:
>>    - bus idle
> 
> Is an error flag defined at bus idle?
> 
> Error flags are intended to destroy *other CAN controllers* transmissions when detecting protocol violations. There can not be a protocol violation at idle time, right?

This is what the Kvaser CAN controller does. It will wait for the bus to become idle, before an active error flag is transmitted.

>>    - while it is transmitting a frame
> 
> What's the use-case for destroying your own transmission?
> 
>>    - while it is receiving a frame
> 
> This makes sense, especially when you can destroy specific CAN ID frames at a specific bit position.
> Or for any CAN ID at a specific bit position.
> Or for any CAN ID at an undefined bit position.

I agree.

>> The error flag can occur at any time.
> 
> Sure? (see above)
> 
> Of course we might also provide some pump gun mode which just sends an error flag at some (any) time.

As above.

> But for what reason?

Testing purpose, e.g if you develop software where you want to keep track of bus errors, this makes it possible to test such software in a controlled way.
We also use this ourselves when testing the transitions ERROR_ACTIVE <-> ERROR_WARNING <-> ERROR_PASSIVE, for Rx.

Regards,
jimmy

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-03  6:42                               ` Jimmy Assarsson
@ 2021-02-03  8:26                                 ` Vincent MAILHOL
  2021-02-03 20:06                                   ` Kurt Van Dijck
  0 siblings, 1 reply; 24+ messages in thread
From: Vincent MAILHOL @ 2021-02-03  8:26 UTC (permalink / raw)
  To: Jimmy Assarsson
  Cc: Oliver Hartkopp, Marc Kleine-Budde, Jimmy Assarsson, linux-can

On Wed. 3 Feb 2021 at 15:42, Jimmy Assarsson <jimmyassarsson@gmail.com> wrote:
>
> On 2021-02-02 20:19, Oliver Hartkopp wrote:
> > On 02.02.21 12:12, Vincent MAILHOL wrote:
> >
> >>> The CAN_ERR_FLAG has been renamed in the documentation to indicate
> >>> "error messages" from the CAN controller, as an error frame is something
> >>> completely different.
> >>>
> >>> Now as we are talking about having CAN_ERR_FLAG in the TX path besides
> >>> the vcan testing stuff, we should think about an API for the really
> >>> outgoing frames.
> >>>
> >>> We could not only think about "create an error frame right now" but also
> >>> think about a more intelligent CAN node, which also offers to destroy
> >>> one or more specific CAN ID(s) at a specific bit position after
> >>> detecting that CAN ID.
> >>
> >> My original idea was to leave it unspecified until a device was
> >> actually capable of doing such a thing. But I am not against
> >> defining the API now :) We might just have to wait a long time
> >> for someone to actually implement it.
> >>
> >>> We could use the CAN_RTR_FLAG and the data[] section of the outgoing
> >>> error CAN frames for such an API.
> >>
> >> We are not limited to the CAN_RTR_FLAG and the data[]
> >>
> >> First, we have to list the use cases.
> >>
> >> As I wrote before, there are only two forms for the error flag:
> >>    - The active error flag: 6 consecutive dominant bits
> >>    - The passive error flag: 6 consecutive recessive bits
> >
> > IMO this passive error flag stuff is pointless.
> > The passive error does not have any effect on the bus. Nobody sees it. It's just a measure to continue counting error counters inside the CAN controller. IMO it's a read-only feature about the controller internal status.
>
> I agree.

Passive error flags will have effect if initiated by the
transmitter. It will break the bitsuffing rule and other nodes
will notice.  Passive error flags initiated by the receivers
shall indeed have no effects.

This is admitelly an edge case. It really depends how much we
want to cover with our API.

> >> The device can either inject the flag either during:
> >>    - bus idle
> >
> > Is an error flag defined at bus idle?
> >
> > Error flags are intended to destroy *other CAN controllers* transmissions when detecting protocol violations. There can not be a protocol violation at idle time, right?
>
> This is what the Kvaser CAN controller does. It will wait for the bus to become idle, before an active error flag is transmitted.

To add to Jimmy's comment, the ISO standard does not
differentiate this. In section 10.4.4.2 "Error flag", it states
that "An error-active node detecting an error condition shall
signal this by sending an active error flag."

If you try to shortcut the CAN High and CAN low wires or connect
those to the ground while the bus is idle, you would probably get
some errors.

> >>    - while it is transmitting a frame
> >
> > What's the use-case for destroying your own transmission?

For active error flags: I admit that there is no benefit. You
could emulate this easily if you have two nodes under your
control: you send the frame on the first node and inject the error
on the second one.

The only use case is for the passive error flag just because the
transmission is the only way to do it. So it just brings us back
to the above discussion: how much do we want to cover.

> >>    - while it is receiving a frame
> >
> > This makes sense, especially when you can destroy specific CAN ID frames at a specific bit position.
> > Or for any CAN ID at a specific bit position.
> > Or for any CAN ID at an undefined bit position.
>
> I agree.
>
> >> The error flag can occur at any time.
> >
> > Sure? (see above)
> >
> > Of course we might also provide some pump gun mode which just sends an error flag at some (any) time.
>
> As above.
>
> > But for what reason?
>
> Testing purpose, e.g if you develop software where you want to keep track of bus errors, this makes it possible to test such software in a controlled way.
> We also use this ourselves when testing the transitions ERROR_ACTIVE <-> ERROR_WARNING <-> ERROR_PASSIVE, for Rx.

I think that there are two axes in this discussion: the attacker
point of view and the functional testing point of view.

From the attacker point of view, you are mostly interested in
destroying the transmitter frames.

For the functional testing, it is about covering the all the
aspects of the standard: make sure that all the TX and RX counters
are correctly incremented, test the transitions between the
different states and that for all offsets. And to confirm all
aspects, you might want to inject both the active and the passive
error flags and do it at all possible positions.

That said, my vision on functional testing is an uneducated
guess. I never worked on that and my personal focus is more the
attacker point of view.


Yours sincerely,
Vincent

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-03  8:26                                 ` Vincent MAILHOL
@ 2021-02-03 20:06                                   ` Kurt Van Dijck
  2021-02-04  4:13                                     ` Vincent MAILHOL
  0 siblings, 1 reply; 24+ messages in thread
From: Kurt Van Dijck @ 2021-02-03 20:06 UTC (permalink / raw)
  To: Vincent MAILHOL
  Cc: Jimmy Assarsson, Oliver Hartkopp, Marc Kleine-Budde,
	Jimmy Assarsson, linux-can

On Wed, 03 Feb 2021 17:26:11 +0900, Vincent MAILHOL wrote:
> On Wed. 3 Feb 2021 at 15:42, Jimmy Assarsson <jimmyassarsson@gmail.com> wrote:
> > >
> > > Of course we might also provide some pump gun mode which just sends an error flag at some (any) time.
> >
> > As above.
> >
> > > But for what reason?
> >
> > Testing purpose, e.g if you develop software where you want to keep track of bus errors, this makes it possible to test such software in a controlled way.
> > We also use this ourselves when testing the transitions ERROR_ACTIVE <-> ERROR_WARNING <-> ERROR_PASSIVE, for Rx.
> 
> I think that there are two axes in this discussion: the attacker
> point of view and the functional testing point of view.
> 
> From the attacker point of view, you are mostly interested in
> destroying the transmitter frames.
> 
> For the functional testing, it is about covering the all the
> aspects of the standard: make sure that all the TX and RX counters
> are correctly incremented, test the transitions between the
> different states and that for all offsets. And to confirm all
> aspects, you might want to inject both the active and the passive
> error flags and do it at all possible positions.
> 
> That said, my vision on functional testing is an uneducated
> guess. I never worked on that and my personal focus is more the
> attacker point of view.

Looking back to it, my first interest would be to fire N error frames,
so to control other nodes' rx error counters.
Controlling your own tx error counter makes less sense, I assume that if
your chip is capable of triggering error frames on demand, then I also
assume that the tx error counter detection is done right.

destroying specific CAN frames sounds much like functional testing,
and can be done much simpler by modifying the node that sends it and add
some very ad-hoc test code to not send specific can frames at all.

The attacker point of view indeed could require a more elaborate API,
but I still doubt we can deliver what is required for attacking.

Kurt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-03 20:06                                   ` Kurt Van Dijck
@ 2021-02-04  4:13                                     ` Vincent MAILHOL
  2021-02-04  7:38                                       ` Kurt Van Dijck
  0 siblings, 1 reply; 24+ messages in thread
From: Vincent MAILHOL @ 2021-02-04  4:13 UTC (permalink / raw)
  To: Vincent MAILHOL, Jimmy Assarsson, Oliver Hartkopp,
	Marc Kleine-Budde, Jimmy Assarsson, linux-can

Hi Kurt,

On Tues 4 Feb. 2021 at 05:06, Kurt Van Dijck
<dev.kurt@vandijck-laurijssen.be> wrote:
> On Wed, 03 Feb 2021 17:26:11 +0900, Vincent MAILHOL wrote:
> > On Wed. 3 Feb 2021 at 15:42, Jimmy Assarsson <jimmyassarsson@gmail.com> wrote:
> > > >
> > > > Of course we might also provide some pump gun mode which just sends an error flag at some (any) time.
> > >
> > > As above.
> > >
> > > > But for what reason?
> > >
> > > Testing purpose, e.g if you develop software where you want to keep track of bus errors, this makes it possible to test such software in a controlled way.
> > > We also use this ourselves when testing the transitions ERROR_ACTIVE <-> ERROR_WARNING <-> ERROR_PASSIVE, for Rx.
> >
> > I think that there are two axes in this discussion: the attacker
> > point of view and the functional testing point of view.
> >
> > From the attacker point of view, you are mostly interested in
> > destroying the transmitter frames.
> >
> > For the functional testing, it is about covering the all the
> > aspects of the standard: make sure that all the TX and RX counters
> > are correctly incremented, test the transitions between the
> > different states and that for all offsets. And to confirm all
> > aspects, you might want to inject both the active and the passive
> > error flags and do it at all possible positions.
> >
> > That said, my vision on functional testing is an uneducated
> > guess. I never worked on that and my personal focus is more the
> > attacker point of view.
>
> Looking back to it, my first interest would be to fire N error frames,
> so to control other nodes' rx error counters.

It is slightly more complex.

Let's consider three nodes all on the same bus.

A: Test node, sends error flags
B: Normal node, send normal frames
C: Normal node, only receiving

   ___            ___            ___
  | _ |          | _ |          | _ |
  ||A||          ||B||          ||C||
  |___|          |___|          |___|
    |              |              |
    | Sends        | Sends        | Only
    | error           | normal       | receives
    | flags        | frames       |
    |              |              |
  --------------------------------------- CAN bus

A waits for B to start sending its frame and trigger the error
flag. This error flag will eventually overwrite one of B's recessive
bit into a dominant one and thus B has his TX error count increased.t

C who is a spectator will just have its RX error count increased.

The error count of node A is kind of undefined because the standard
does not expect you to generate your own errors. By default, I think
node A should *not* increase its counter if the error flag is generated
by itself on purpose so that it is free to mess on the bus as much as it
wants without going bus-off.

So sending error frames can increment either the TX or RX error
counters depending if the node is in transmitting or receiving at the
moment the error flag occurs.

> Controlling your own tx error counter makes less sense, I assume that if
> your chip is capable of triggering error frames on demand, then I also
> assume that the tx error counter detection is done right.

Any use cases toward your own chip (e.g. increasing your own TX
counter) is also not my interest. My idea of injecting the error flags
on your own frame is just to cover the error passive flag use cases in
order to see if *other* frames correctly handle the error passive
flag.

Passive error flags are indeed an edge case and if there is a
consensus that this feature is not needed (which seems to be the
case), I have no objection to not having it.

> destroying specific CAN frames sounds much like functional testing,
> and can be done much simpler by modifying the node that sends it and add
> some very ad-hoc test code to not send specific can frames at all.
>
> The attacker point of view indeed could require a more elaborate API,
> but I still doubt we can deliver what is required for attacking.

This is interesting because we have an opposite view of the attacker
and functional testing approaches.

For the attacker, I am thinking of:
https://youtu.be/oajtDFw_t3Q
In a nutshell, it is an elaborate technique in which you first DoS the
target node by increasing its TX counter until it gets in bus-off
state. Once done, the attacker can send messages in place of
the genuine node. This way, contrary to an simple injection attack
on which the bus contains both the genuine and the attacker frames,
here, only the attacker speaks on the bus.
This attack does not really care when the error flag occurs as long as
the error counter increases.

My vision of the functional testing is more: does the controller
react correctly in all situations? You could imagine an implementation
issue that would cause the error count to not correctly behave only on
a specific field. How would you test for such implementation issues
other than injecting the error flag at each offset of the frame?


Yours sincerely,
Vincent

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-04  4:13                                     ` Vincent MAILHOL
@ 2021-02-04  7:38                                       ` Kurt Van Dijck
  2021-02-04  9:42                                         ` Vincent MAILHOL
  0 siblings, 1 reply; 24+ messages in thread
From: Kurt Van Dijck @ 2021-02-04  7:38 UTC (permalink / raw)
  To: Vincent MAILHOL
  Cc: Jimmy Assarsson, Oliver Hartkopp, Marc Kleine-Budde,
	Jimmy Assarsson, linux-can

On Thu, 04 Feb 2021 13:13:18 +0900, Vincent MAILHOL wrote:
> Hi Kurt,
> 
> On Tues 4 Feb. 2021 at 05:06, Kurt Van Dijck
> <dev.kurt@vandijck-laurijssen.be> wrote:
> > On Wed, 03 Feb 2021 17:26:11 +0900, Vincent MAILHOL wrote:
> > > On Wed. 3 Feb 2021 at 15:42, Jimmy Assarsson <jimmyassarsson@gmail.com> wrote:
> > > > >
> > > > > Of course we might also provide some pump gun mode which just sends an error flag at some (any) time.
> > > >
> > > > As above.
> > > >
> > > > > But for what reason?
> > > >
> > > > Testing purpose, e.g if you develop software where you want to keep track of bus errors, this makes it possible to test such software in a controlled way.
> > > > We also use this ourselves when testing the transitions ERROR_ACTIVE <-> ERROR_WARNING <-> ERROR_PASSIVE, for Rx.
> > >
> > > I think that there are two axes in this discussion: the attacker
> > > point of view and the functional testing point of view.
> > >
> > > From the attacker point of view, you are mostly interested in
> > > destroying the transmitter frames.
> > >
> > > For the functional testing, it is about covering the all the
> > > aspects of the standard: make sure that all the TX and RX counters
> > > are correctly incremented, test the transitions between the
> > > different states and that for all offsets. And to confirm all
> > > aspects, you might want to inject both the active and the passive
> > > error flags and do it at all possible positions.
> > >
> > > That said, my vision on functional testing is an uneducated
> > > guess. I never worked on that and my personal focus is more the
> > > attacker point of view.
> >
> > Looking back to it, my first interest would be to fire N error frames,
> > so to control other nodes' rx error counters.
> 
> It is slightly more complex.
> 
> Let's consider three nodes all on the same bus.
> 
> A: Test node, sends error flags
> B: Normal node, send normal frames
> C: Normal node, only receiving
> 
>    ___            ___            ___
>   | _ |          | _ |          | _ |
>   ||A||          ||B||          ||C||
>   |___|          |___|          |___|
>     |              |              |
>     | Sends        | Sends        | Only
>     | error           | normal       | receives
>     | flags        | frames       |
>     |              |              |
>   --------------------------------------- CAN bus
> 
> A waits for B to start sending its frame and trigger the error
> flag. This error flag will eventually overwrite one of B's recessive
> bit into a dominant one and thus B has his TX error count increased.t
> 
> C who is a spectator will just have its RX error count increased.

Right, I understand. I was too quick with my conclusion.
Thanks for explaining.

[...]

> > The attacker point of view indeed could require a more elaborate API,
> > but I still doubt we can deliver what is required for attacking.
> 
> This is interesting because we have an opposite view of the attacker
> and functional testing approaches.
> 
> For the attacker, I am thinking of:
> https://youtu.be/oajtDFw_t3Q
> In a nutshell, it is an elaborate technique in which you first DoS the
> target node by increasing its TX counter until it gets in bus-off
> state. Once done, the attacker can send messages in place of
> the genuine node. This way, contrary to an simple injection attack
> on which the bus contains both the genuine and the attacker frames,
> here, only the attacker speaks on the bus.
> This attack does not really care when the error flag occurs as long as
> the error counter increases.

Yep, I see.

I tend to program the nodes to recover from bus-off in 10msec
magnitude. The scenario you describe, if I understand well, is hard to
implement you need to repeat it every 10msec.

Am I mistaken?
Or is 10msec order rather stupid to recover and does everyone apply much
longer delays before recovery?

> 
> My vision of the functional testing is more: does the controller
> react correctly in all situations? You could imagine an implementation
> issue that would cause the error count to not correctly behave only on
> a specific field. How would you test for such implementation issues
> other than injecting the error flag at each offset of the frame?

ok

> 
> 
> Yours sincerely,
> Vincent

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Question] Sending CAN error frames
  2021-02-04  7:38                                       ` Kurt Van Dijck
@ 2021-02-04  9:42                                         ` Vincent MAILHOL
  0 siblings, 0 replies; 24+ messages in thread
From: Vincent MAILHOL @ 2021-02-04  9:42 UTC (permalink / raw)
  To: Vincent MAILHOL, Jimmy Assarsson, Oliver Hartkopp,
	Marc Kleine-Budde, Jimmy Assarsson, linux-can

On Thu. 4 Feb 2021 at 16:38, Kurt Van Dijck
<dev.kurt@vandijck-laurijssen.be> wrote:
>
> On Thu, 04 Feb 2021 13:13:18 +0900, Vincent MAILHOL wrote:
> > Hi Kurt,
> >
> > On Tues 4 Feb. 2021 at 05:06, Kurt Van Dijck
> > <dev.kurt@vandijck-laurijssen.be> wrote:
> > > On Wed, 03 Feb 2021 17:26:11 +0900, Vincent MAILHOL wrote:
> > > > On Wed. 3 Feb 2021 at 15:42, Jimmy Assarsson <jimmyassarsson@gmail.com> wrote:
> > > > > >
> > > > > > Of course we might also provide some pump gun mode which just sends an error flag at some (any) time.
> > > > >
> > > > > As above.
> > > > >
> > > > > > But for what reason?
> > > > >
> > > > > Testing purpose, e.g if you develop software where you want to keep track of bus errors, this makes it possible to test such software in a controlled way.
> > > > > We also use this ourselves when testing the transitions ERROR_ACTIVE <-> ERROR_WARNING <-> ERROR_PASSIVE, for Rx.
> > > >
> > > > I think that there are two axes in this discussion: the attacker
> > > > point of view and the functional testing point of view.
> > > >
> > > > From the attacker point of view, you are mostly interested in
> > > > destroying the transmitter frames.
> > > >
> > > > For the functional testing, it is about covering the all the
> > > > aspects of the standard: make sure that all the TX and RX counters
> > > > are correctly incremented, test the transitions between the
> > > > different states and that for all offsets. And to confirm all
> > > > aspects, you might want to inject both the active and the passive
> > > > error flags and do it at all possible positions.
> > > >
> > > > That said, my vision on functional testing is an uneducated
> > > > guess. I never worked on that and my personal focus is more the
> > > > attacker point of view.
> > >
> > > Looking back to it, my first interest would be to fire N error frames,
> > > so to control other nodes' rx error counters.
> >
> > It is slightly more complex.
> >
> > Let's consider three nodes all on the same bus.
> >
> > A: Test node, sends error flags
> > B: Normal node, send normal frames
> > C: Normal node, only receiving
> >
> >    ___            ___            ___
> >   | _ |          | _ |          | _ |
> >   ||A||          ||B||          ||C||
> >   |___|          |___|          |___|
> >     |              |              |
> >     | Sends        | Sends        | Only
> >     | error           | normal       | receives
> >     | flags        | frames       |
> >     |              |              |
> >   --------------------------------------- CAN bus
> >
> > A waits for B to start sending its frame and trigger the error
> > flag. This error flag will eventually overwrite one of B's recessive
> > bit into a dominant one and thus B has his TX error count increased.t
> >
> > C who is a spectator will just have its RX error count increased.
>
> Right, I understand. I was too quick with my conclusion.
> Thanks for explaining.
>
> [...]
>
> > > The attacker point of view indeed could require a more elaborate API,
> > > but I still doubt we can deliver what is required for attacking.
> >
> > This is interesting because we have an opposite view of the attacker
> > and functional testing approaches.
> >
> > For the attacker, I am thinking of:
> > https://youtu.be/oajtDFw_t3Q
> > In a nutshell, it is an elaborate technique in which you first DoS the
> > target node by increasing its TX counter until it gets in bus-off
> > state. Once done, the attacker can send messages in place of
> > the genuine node. This way, contrary to an simple injection attack
> > on which the bus contains both the genuine and the attacker frames,
> > here, only the attacker speaks on the bus.
> > This attack does not really care when the error flag occurs as long as
> > the error counter increases.
>
> Yep, I see.
>
> I tend to program the nodes to recover from bus-off in 10msec
> magnitude. The scenario you describe, if I understand well, is hard to
> implement you need to repeat it every 10msec.
>
> Am I mistaken?
> Or is 10msec order rather stupid to recover and does everyone apply much
> longer delays before recovery?

I can not comment on what is the best bus-off reset value. I never had
the opportunity to build my own FPGA and to try those kinds of
attacks. I know more about the theory than the practice.

Also, I do not know what kind of bus-off reset values are used by
OEMs in their production vehicles.

That said, if you are building a safety critical network, my
recommendation is to enforce message authentication. This is
typically achieved with Autosar Secure Onboard
communication (SecOC).
That way, an attacker might still DoS your ECU but he or she will not
be able to inject messages.

For me, the bus-off reset is a safety topic. Using it for security
design is not something I recommend.

> > My vision of the functional testing is more: does the controller
> > react correctly in all situations? You could imagine an implementation
> > issue that would cause the error count to not correctly behave only on
> > a specific field. How would you test for such implementation issues
> > other than injecting the error flag at each offset of the frame?
>
> ok

Let me try to recenter the discussion.

In the light of everyone comments, I now propose the
CAN_CTRLMODE_TX_ERR API to be as below:

  - allow to inject error active flags only (by consensus, error
    passive flags are not needed).

  - allow to inject error flags during either bus idle state or while
    other nodes are transmitting (but not be able to inject error
    flags while it is itself transmitting).

  - allow to target a specific CAN ID. The filtering option does not
    need to be complex. I propose to just match the CAN ID (including
    EFF flag).

  - allow to inject the flag at one specific offset in the frame.

  - when generating error flags, the device does not increase its
    tx/rx error count.

Thanks for your comments!


Yours sincerely,
Vincent

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2021-02-04  9:43 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-31  6:22 [Question] Sending CAN error frames Vincent MAILHOL
     [not found] ` <87e3dd54-50ab-1190-efdb-18ddb3b21a02@hartkopp.net>
2021-01-31 14:12   ` Vincent MAILHOL
2021-01-31 20:42   ` Jimmy Assarsson
2021-02-01 14:19     ` Vincent MAILHOL
2021-02-01 15:41       ` Jimmy Assarsson
2021-02-02  0:22         ` Vincent MAILHOL
2021-02-02  7:35           ` Marc Kleine-Budde
2021-02-02  8:23             ` Kurt Van Dijck
2021-02-02  8:41               ` Marc Kleine-Budde
2021-02-02  9:00                 ` Oliver Hartkopp
2021-02-02  9:05                   ` Marc Kleine-Budde
2021-02-02  9:16                     ` Oliver Hartkopp
2021-02-02 10:00                       ` Vincent MAILHOL
2021-02-02 10:14                         ` Oliver Hartkopp
2021-02-02 11:12                           ` Vincent MAILHOL
2021-02-02 19:19                             ` Oliver Hartkopp
2021-02-03  6:42                               ` Jimmy Assarsson
2021-02-03  8:26                                 ` Vincent MAILHOL
2021-02-03 20:06                                   ` Kurt Van Dijck
2021-02-04  4:13                                     ` Vincent MAILHOL
2021-02-04  7:38                                       ` Kurt Van Dijck
2021-02-04  9:42                                         ` Vincent MAILHOL
2021-02-02 12:14                         ` Jimmy Assarsson
2021-02-02  9:59           ` Jimmy Assarsson

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.