linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
To: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>,
	Jimmy Assarsson <extja@kvaser.com>,
	Jimmy Assarsson <jimmyassarsson@gmail.com>,
	linux-can <linux-can@vger.kernel.org>
Subject: Re: [Question] Sending CAN error frames
Date: Tue, 2 Feb 2021 19:00:30 +0900	[thread overview]
Message-ID: <CAMZ6RqKx5NCFKiahb8AbUx=LC5xS6oYCdVZk8WGSAzZeAVs9Qg@mail.gmail.com> (raw)
In-Reply-To: <abc8923c-8cf6-c0d5-ec67-73afe183b9b3@hartkopp.net>

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
> >

  reply	other threads:[~2021-02-02 10:01 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMZ6RqKx5NCFKiahb8AbUx=LC5xS6oYCdVZk8WGSAzZeAVs9Qg@mail.gmail.com' \
    --to=mailhol.vincent@wanadoo.fr \
    --cc=extja@kvaser.com \
    --cc=jimmyassarsson@gmail.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=socketcan@hartkopp.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).