All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
To: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: linux-can <linux-can@vger.kernel.org>, kayoub5@live.com
Subject: Re: More flags for logging
Date: Tue, 4 May 2021 00:02:49 +0900	[thread overview]
Message-ID: <CAMZ6RqLKp981zBus9K1RiLL14zC0B2BGp0U+xuQGvj1fjDMF1g@mail.gmail.com> (raw)
In-Reply-To: <20210503100810.cacbmdfmpjipgoka@pengutronix.de>

On Mon. 3 Mai 2021 at 19:08, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 03.05.2021 12:02:46, Marc Kleine-Budde wrote:
> > The SocketCAN API is a great initiative for standardizing the CAN
> > interface API. This request tries to extend this initiative for more use
> > cases.
> >
> > Context:
> >
> > The SocketCAN was adopted by libpcap and tcpdump as the standard format
> > for logging CAN Frames in PCAP and PCAP-NG. See:
> >
> > https://www.tcpdump.org/linktypes/LINKTYPE_CAN_SOCKETCAN.html
> > https://github.com/wireshark/wireshark/blob/master/epan/dissectors/packet-socketcan.c
> > https://www.wireshark.org/docs/dfref/c/can.html
> >
> > Problem:
> > Applications that perform logging, usually need more details that normal
> > applications, for the sake of debugging later on. Flags needs to be
> > reserved/allocated in the SocketCAN API, so that logging applications
> > can make use of them in the PCAP format. The flags does not need
> > necessary need to be implemented by SocketCAN, they just need to be
> > marked as reserved for such use case.
> >
> > Needed Flags:
> > FDF Flag
> > - Since CAN Frames and CAN-FD frames can co-exist in the same bus,
> >   logging application needs to know if a normal CAN Frame was
> >   transmitted on a CAN-FD bus, namely was the FDF bit set or not.
>
> I think someone asked for that some time ago. But that was never
> mainlined. I'll look for that old mail.

The current API compares the size of the received structure with
the MTU to determine whether the message is classical CAN or CAN
FD.  If the struct can_frame and canfd_frame are stored as
is (including the padding at the end of the data array), then the
information is maintained.

Personally, I would have liked to also have a FDF flag in Socket
CAN userland API because I think it would be more intuitive than
the MTU solution. So I am open to this option and would also be
happy to see the flag implemented in Socket CAN (not only
reserved).

But, I also understand that it will create redundancy.

> > ACK bit in data frame
> > - Some logging hardware can act as a "spy", meaning that it records CAN
> >   Frames, but does not set the ACK bit
> > - A way to know for a given frame (FD or not), was the ACK bit set or
> >   not.
> > - Current API allow detecting missing ACK, but it does not report what
> >   Frame had a missing ACK.
>
> This means the driver has to set a flag if it's configured in
> listen-only mode. That should be possible.

For my understanding, when a controller does not see the ACK bit,
it stops the transmission and sends an error flag. For this
reason, the frame is not received and thus simply does not appear
in the log.

So this issue is not only about adding an ACK flag but is about
implementing a solution to report on which frame a given error
flag occured (not specific to ACK errors).

And to clarify the "spy mode", if all the receivers on the bus
are spies, then the ACK bit is never set and thus the
transmission should always result in an ACK error flag. If at
least one receiver is normal (not spy) it should send a dominant
ACK bit and thus all nodes (including the spies) see the flag as
dominant. In other terms, a received CAN frame has the ACK flag
set.

> I think we can make use of flags in the struct canfd_frame for this:
>
> | struct canfd_frame {
> |       canid_t can_id;  /* 32 bit CAN_ID + EFF/RTR/ERR flags */
> |       __u8    len;     /* frame payload length in byte */
> |       __u8    flags;   /* additional flags for CAN FD */
> |       __u8    __res0;  /* reserved / padding */
> |       __u8    __res1;  /* reserved / padding */
> |       __u8    data[CANFD_MAX_DLEN] __attribute__((aligned(8)));
> | };
>
> The struct can_frame doesn't have the flags member yet, but we can add
> it there.
>
> regards,
> 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 |

  reply	other threads:[~2021-05-03 15:03 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-03 10:02 More flags for logging Marc Kleine-Budde
2021-05-03 10:08 ` Marc Kleine-Budde
2021-05-03 15:02   ` Vincent MAILHOL [this message]
2021-05-03 15:43     ` Marc Kleine-Budde
     [not found]     ` <DBBPR03MB70828377F51A1747B4E9E6529D5B9@DBBPR03MB7082.eurprd03.prod.outlook.com>
2021-05-03 15:47       ` Marc Kleine-Budde
     [not found]         ` <DBBPR03MB7082F029173018680E5D869C9D5B9@DBBPR03MB7082.eurprd03.prod.outlook.com>
2021-05-03 21:46           ` Vincent MAILHOL
2021-05-04  5:40             ` Patrick Menschel
2021-05-04  7:48             ` mcp251xfd receiving non ACKed frames (was: Re: More flags for logging) Marc Kleine-Budde
2021-05-04 12:22               ` Vincent MAILHOL
2021-05-04 12:53                 ` Thomas.Kopp
2021-05-04 14:26                   ` Vincent MAILHOL
2021-05-04 17:44                 ` Vincent MAILHOL
2021-05-04 10:10         ` More flags for logging Kurt Van Dijck
2021-05-04 10:07     ` Kurt Van Dijck
2021-05-04  8:49   ` Oliver Hartkopp
     [not found]     ` <DBBPR03MB7082C7E8FD22D0CA8C2DA99D9D5A9@DBBPR03MB7082.eurprd03.prod.outlook.com>
     [not found]       ` <AM8PR08MB5698886555F8531B6CF65982B75A9@AM8PR08MB5698.eurprd08.prod.outlook.com>
2021-05-04  9:49         ` Ayoub Kaanich
2021-05-04 10:13           ` Oliver Hartkopp
2021-05-04 13:58             ` Ayoub Kaanich
2021-05-04 14:38               ` Oliver Hartkopp
     [not found]     ` <DBBPR03MB70824BB82F7BB335928BE36A9D2F9@DBBPR03MB7082.eurprd03.prod.outlook.com>
2021-05-17  7:29       ` Oliver Hartkopp

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=CAMZ6RqLKp981zBus9K1RiLL14zC0B2BGp0U+xuQGvj1fjDMF1g@mail.gmail.com \
    --to=mailhol.vincent@wanadoo.fr \
    --cc=kayoub5@live.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.