[v2,1/1] Documentation: networking: document ISO 15765-2:2016

Commit Message

Francesco Valla March 29, 2024, 1:34 p.m. UTC

Document basic concepts, APIs and behaviour of the ISO 15675-2:2016
(ISO-TP) CAN stack.

Signed-off-by: Francesco Valla <[e-mail]>
---
 Documentation/networking/index.rst      |   1 +
 Documentation/networking/iso15765-2.rst | 356 ++++++++++++++++++++++++
 MAINTAINERS                             |   1 +
 3 files changed, 358 insertions(+)
 create mode 100644 Documentation/networking/iso15765-2.rst

Comments

Vincent Mailhol April 13, 2024, 1:11 p.m. UTC | #1

Hi Francesco,

Thank you for the ISO-TP documentation.

I left a few comments, but overall, good work. Also, I did not double
check each individual option one by one.

On Sat. 30 Mar 2024 at 00:06, Francesco Valla <[e-mail]> wrote:
> Document basic concepts, APIs and behaviour of the ISO 15675-2:2016
> (ISO-TP) CAN stack.
>
> Signed-off-by: Francesco Valla <[e-mail]>
> ---
>  Documentation/networking/index.rst      |   1 +
>  Documentation/networking/iso15765-2.rst | 356 ++++++++++++++++++++++++
>  MAINTAINERS                             |   1 +
>  3 files changed, 358 insertions(+)
>  create mode 100644 Documentation/networking/iso15765-2.rst
>
> diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
> index 473d72c36d61..bbd9bf537793 100644
> --- a/Documentation/networking/index.rst
> +++ b/Documentation/networking/index.rst
> @@ -19,6 +19,7 @@ Contents:
>     caif/index
>     ethtool-netlink
>     ieee802154
> +   iso15765-2
>     j1939
>     kapi
>     msg_zerocopy
> diff --git a/Documentation/networking/iso15765-2.rst b/Documentation/networking/iso15765-2.rst
> new file mode 100644
> index 000000000000..bbed4d2ef1a8
> --- /dev/null
> +++ b/Documentation/networking/iso15765-2.rst
> @@ -0,0 +1,356 @@
> +.. SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +
> +=========================
> +ISO 15765-2:2016 (ISO-TP)
> +=========================
> +
> +Overview
> +========
> +
> +ISO 15765-2:2016, also known as ISO-TP, is a transport protocol specifically
> +defined for diagnostic communication on CAN. It is widely used in the automotive
> +industry, for example as the transport protocol for UDSonCAN (ISO 14229-3) or
> +emission-related diagnostic services (ISO 15031-5).
> +
> +ISO-TP can be used both on CAN CC (aka Classical CAN, CAN 2.0B) and CAN FD (CAN

CC is already the abbreviation of *C*lassical *C*AN. Saying CAN CC, is
like saying CAN Classical CAN, c.f. the RAS syndrome:

  https://en.wikipedia.org/wiki/RAS_syndrome

Then, considering the CAN2.0B, as far as I know, ISO-TP can also be
used on CAN2.0A (as long as you only use 11 bits CAN ids).

So, I would rather just say:

  ISO-TP can be used both on Classical CAN and CAN FD...

> +with Flexible Datarate) based networks. It is also designed to be compatible
> +with a CAN network using SAE J1939 as data link layer (however, this is not a
> +requirement).
> +
> +Specifications used
> +-------------------
> +
> +* ISO 15765-2:2016 : Road vehicles - Diagnostic communication over Controller

ISO 15765-2:2016 is withdrawn. The latest version is 15765-2:2024.

> +  Area Network (DoCAN). Part 2: Transport protocol and network layer services.
> +
> +Addressing
> +----------
> +
> +In its simplest form, ISO-TP is based on two kinds of addressing modes for the
> +nodes connected to the same network:
> +
> +- physical addressing is implemented by two node-specific addresses (CAN
> +  identifiers) and is used in 1-to-1 communication
> +- functional addressing is implemented by one node-specific address (CAN
> +  identifier) and is used in 1-to-N communication
> +
> +In a so-called "normal" addressing scenario, both these addresses are
> +represented by a 29-bit CAN ID. However, in order to support larger networks,
> +an "extended" addressing scheme can be adopted: in this case, the first byte of
> +the data payload is used as an additional component of the address (both for
> +the physical and functional cases); two different CAN IDs are still required.

There is more than that.

  - The normal addressing can also use the non-extended 11 bits CAN ID.
  - In addition to the normal and extended addressing mode, there
    is a third mode: the mixed addressing.

Ref:

  - ISO 15765:2024 §10.3.1 "Addressing formats"
  - https://www.embedded-communication.com/en/misc/iso-tp-addressing/

> +
> +Transport protocol and associated frame types
> +---------------------------------------------
> +
> +When transmitting data using the ISO-TP protocol, the payload can either fit
> +inside one single CAN message or not, also considering the overhead the protocol
> +is generating and the optional extended addressing. In the first case, the data
> +is transmitted at once using a so-called Single Frame (SF). In the second case,
> +ISO-TP defines a multi-frame protocol, in which the sender provides (through a
> +First Frame - FF) the PDU length which is to be transmitted and also asks for a
> +Flow Control (FC) frame, which provides the maximum supported size of a macro
> +data block (``blocksize``) and the minimum time between the single CAN messages
> +composing such block (``stmin``). Once this information has been received, the
> +sender starts to send frames containing fragments of the data payload (called
> +Consecutive Frames - CF), stopping after every ``blocksize``-sized block to wait
> +confirmation from the receiver (which should then send another Flow Control
> +frame to inform the sender about its availability to receive more data).

Nitpick: I do not see the need for the brackets here:

  confirmation from the receiver which should then send another Flow Control
  frame to inform the sender about its availability to receive more data.

> +How to Use ISO-TP
> +=================
> +
> +As with others CAN protocols, the ISO-TP stack support is built into the
> +Linux network subsystem for the CAN bus, aka. Linux-CAN or SocketCAN, and
> +thus follows the same socket API.
> +
> +Creation and basic usage of an ISO-TP socket
> +--------------------------------------------
> +
> +To use the ISO-TP stack, ``#include <linux/can/isotp.h>`` shall be used. A
> +socket can then be created using the ``PF_CAN`` protocol family, the
> +``SOCK_DGRAM`` type (as the underlying protocol is datagram-based by design)
> +and the ``CAN_ISOTP`` protocol:
> +
> +.. code-block:: C
> +
> +    s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
> +
> +After the socket has been successfully created, ``bind(2)`` shall be called to
> +bind the socket to the desired CAN interface; to do so:
> +
> +* a TX CAN ID shall be specified as part of the sockaddr supplied to the call
> +  itself, and

Why did you put a newline here?

> +* a RX CAN ID shall also specified, unless broadcast flags have been set
> +  through socket option (explained below)

Add a period at the end of your sentence:

  through socket option (explained below).

> +Once bound to an interface, the socket can be read from and written to using
> +the usual ``read(2)`` and ``write(2)`` system calls, as well as ``send(2)``,
> +``sendmsg(2)``, ``recv(2)`` and ``recvmsg(2)``.

If this is a new paragraph, leave an empty newline. If not, do not add
a newline before you reach the character column limit.

> +Unlike the CAN_RAW socket API, only the data payload shall be specified in all
> +these calls, as the CAN header is automatically filled by the ISO-TP stack
> +using information supplied during socket creation. In the same way, the stack

This is making a shortcut. There are the raw CAN payload and the
ISO-TP payload. In this paragraph it is not clear that "data payload"
is referring to the ISO-TP payload.

Also, what is the meaning of "the CAN header". Here, I think you mean
CAN ID plus some of the few first byte of the CAN payload.

I suggest that you use more precise vocabulary from the standard:

  - Address information
  - Protocol Information
  - Data field

Something like:

  only the ISO-TP data field (the actual payload) is sent. The
  address information and the protocol information is
  automatically filled by the ISO-TP stack...

> +will use the transport mechanism when required (i.e., when the size of the data
> +payload exceeds the MTU of the underlying CAN bus).
> +
> +The sockaddr structure used for SocketCAN has extensions for use with ISO-TP,
> +as specified below:
> +
> +.. code-block:: C
> +
> +    struct sockaddr_can {
> +        sa_family_t can_family;
> +        int         can_ifindex;
> +        union {
> +            struct { canid_t rx_id, tx_id; } tp;
> +        ...
> +        } can_addr;
> +    }
> +
> +* ``can_family`` and ``can_ifindex`` serve the same purpose as for other
> +  SocketCAN sockets.
> +
> +* ``can_addr.tp.rx_id`` specifies the receive (RX) CAN ID and will be used as
> +  a RX filter.
> +
> +* ``can_addr.tp.tx_id`` specifies the transmit (TX) CAN ID
> +
> +ISO-TP socket options
> +---------------------
> +
> +When creating an ISO-TP socket, reasonable defaults are set. Some options can
> +be modified with ``setsockopt(2)`` and/or read back with ``getsockopt(2)``.
> +
> +General options
> +~~~~~~~~~~~~~~~
> +
> +General socket options can be passed using the ``CAN_ISOTP_OPTS`` optname:
> +
> +.. code-block:: C
> +
> +    struct can_isotp_options opts;
> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_OPTS, &opts, sizeof(opts))
> +
> +where the ``can_isotp_options`` structure has the following contents:
> +
> +.. code-block:: C
> +
> +    struct can_isotp_options {
> +        u32 flags;
> +        u32 frame_txtime;
> +        u8  ext_address;
> +        u8  txpad_content;
> +        u8  rxpad_content;
> +        u8  rx_ext_address;
> +    };
> +
> +* ``flags``: modifiers to be applied to the default behaviour of the ISO-TP
> +  stack. Following flags are available:
> +
> +  - ``CAN_ISOTP_LISTEN_MODE``: listen only (do not send FC frames); normally
> +    used as a testing feature.
> +  - ``CAN_ISOTP_EXTEND_ADDR``: enable extended addressing, using the byte
> +    specified in ``ext_address`` as additional address byte.
                                    ^^^^^^^^^^^^^
as an additional

> +  - ``CAN_ISOTP_TX_PADDING``: enable padding for tranmsitted frames, using
                                                    ^^^^^^^^^^^
transmitted

> +    ``txpad_content`` as value for the padding bytes.
> +  - ``CAN_ISOTP_RX_PADDING``: enable padding for the received frames, using
> +    ``rxpad_content`` as value for the padding bytes.
> +  - ``CAN_ISOTP_CHK_PAD_LEN``: check for correct padding length on the received
> +    frames.
> +  - ``CAN_ISOTP_CHK_PAD_DATA``: check padding bytes on the received frames
> +    against ``rxpad_content``; if ``CAN_ISOTP_RX_PADDING`` is not specified,
> +    this flag is ignored.
> +  - ``CAN_ISOTP_HALF_DUPLEX``: force ISO-TP socket in half duples mode
                                                              ^^^^^^
duplex

> +    (that is, transport mechanism can only be incoming or outgoing at the same
> +    time, not both).
> +  - ``CAN_ISOTP_FORCE_TXSTMIN``: ignore stmin from received FC; normally
> +    used as a testing feature.
> +  - ``CAN_ISOTP_FORCE_RXSTMIN``: ignore CFs depending on rx stmin; normally
> +    used as a testing feature.
> +  - ``CAN_ISOTP_RX_EXT_ADDR``: use ``rx_ext_address`` instead of ``ext_address``
> +    as extended addressing byte on the reception path.
> +  - ``CAN_ISOTP_WAIT_TX_DONE``: wait until the frame is sent before returning
> +    from ``write(2)`` and ``send(2)`` calls (i.e., blocking write operations).
> +  - ``CAN_ISOTP_SF_BROADCAST``: use 1-to-N functional addressing (cannot be
> +    specified alongside ``CAN_ISOTP_CF_BROADCAST``).
> +  - ``CAN_ISOTP_CF_BROADCAST``: use 1-to-N transmission without flow control
> +    (cannot be specified alongside ``CAN_ISOTP_SF_BROADCAST``).
> +    NOTE: this is not covered by the ISO15765-2:2016 standard.
                                        ^^^^^^^^
Add a space between ISO and the number. Also, update the year:

  ISO 15765-2:2024

> +  - ``CAN_ISOTP_DYN_FC_PARMS``: enable dynamic update of flow control
> +    parameters.
> +

Sometimes you put an empty line between the items, sometimes not. Be consistent.

> +* ``frame_txtime``: frame transmission time (defined as N_As/N_Ar inside the
> +  ISO standard); if ``0``, the default (or the last set value) is used.
> +  To set the transmission time to ``0``, the ``CAN_ISOTP_FRAME_TXTIME_ZERO``
> +  macro (equal to 0xFFFFFFFF) shall be used.
> +
> +* ``ext_address``: extended addressing byte, used if the
> +  ``CAN_ISOTP_EXTEND_ADDR`` flag is specified.
> +
> +* ``txpad_content``: byte used as padding value for transmitted frames

Add a period at the end of the sentence.

> +
> +* ``rxpad_content``: byte used as padding value for received frames

Add a period at the end of the sentence.

> +
> +* ``rx_ext_address``: extended addressing byte for the reception path, used if
> +  the ``CAN_ISOTP_RX_EXT_ADDR`` flag is specified.
> +
> +Flow Control options
> +~~~~~~~~~~~~~~~~~~~~
> +
> +Flow Control (FC) options can be passed using the ``CAN_ISOTP_RECV_FC`` optname
> +to provide the communication parameters for receiving ISO-TP PDUs.
> +
> +.. code-block:: C
> +
> +    struct can_isotp_fc_options fc_opts;
> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_RECV_FC, &fc_opts, sizeof(fc_opts));
> +
> +where the ``can_isotp_fc_options`` structure has the following contents:
> +
> +.. code-block:: C
> +
> +    struct can_isotp_options {
> +        u8 bs;
> +        u8 stmin;
> +        u8 wftmax;
> +    };
> +
> +* ``bs``: blocksize provided in flow control frames.
> +
> +* ``stmin``: minimum separation time provided in flow control frames; can
> +  have the following values (others are reserved):
> +  - 0x00 - 0x7F : 0 - 127 ms
> +  - 0xF1 - 0xF9 : 100 us - 900 us
> +
> +* ``wftmax``: maximum number of wait frames provided in flow control frames.
> +
> +Link Layer options
> +~~~~~~~~~~~~~~~~~~
> +
> +Link Layer (LL) options can be passed using the ``CAN_ISOTP_LL_OPTS`` optname:
> +
> +.. code-block:: C
> +
> +    struct can_isotp_ll_options ll_opts;
> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_LL_OPTS, &ll_opts, sizeof(ll_opts));
> +
> +where the ``can_isotp_ll_options`` structure has the following contents:
> +
> +.. code-block:: C
> +
> +    struct can_isotp_ll_options {
> +        u8 mtu;
> +        u8 tx_dl;
> +        u8 tx_flags;
> +    };
> +
> +* ``mtu``: generated and accepted CAN frame type, can be equal to ``CAN_MTU``
> +  for classical CAN frames or ``CANFD_MTU`` for CAN FD frames.
> +
> +* ``tx_dl``: maximum payload length for transmitted frames, can have one value
> +  among: 8, 12, 16, 20, 24, 32, 48, 64. Values above 8 only apply to CAN FD
> +  traffic (i.e.: ``mtu = CANFD_MTU``).
> +
> +* ``tx_flags``: flags set into ``struct canfd_frame.flags`` at frame creation.
> +  Only applies to CAN FD traffic (i.e.: ``mtu = CANFD_MTU``).
> +
> +Transmission stmin
> +~~~~~~~~~~~~~~~~~~
> +
> +The transmission minimum separaton time (stmin) can be forced using the
                            ^^^^^^^^^
separation

> +``CAN_ISOTP_TX_STMIN`` optname and providing an stmin value in microseconds as
> +a 32bit unsigned integer; this will overwrite the value sent by the receiver in
> +flow control frames:
> +
> +.. code-block:: C
> +
> +    uint32_t stmin;
> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_TX_STMIN, &stmin, sizeof(stmin));
> +
> +Reception stmin
> +~~~~~~~~~~~~~~~
> +
> +The reception minimum separaton time (stmin) can be forced using the
                         ^^^^^^^^^
separation

> +``CAN_ISOTP_RX_STMIN`` optname and providing an stmin value in microseconds as
> +a 32bit unsigned integer; received Consecutive Frames (CF) which timestamps
> +differ less than this value will be ignored:
> +
> +.. code-block:: C
> +
> +    uint32_t stmin;
> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_RX_STMIN, &stmin, sizeof(stmin));
> +
> +Multi-frame transport support
> +-----------------------------
> +
> +The ISO-TP stack contained inside the Linux kernel supports the multi-frame
> +transport mechanism defined by the standard, with the following constraints:
> +
> +* the maximum size of a PDU is defined by a module parameter, with an hard
> +  limit imposed at build time
> +* when a transmission is in progress, subsequent calls to ``write(2)`` will
> +  block, while calls to ``send(2)`` will either block or fail depending on the
> +  presence of the ``MSG_DONTWAIT`` flag
> +* no support is present for sending "wait frames": whether a PDU can be fully
> +  received or not is decided when the First Frame is received

Add a period at the end of all the sentences of this enumeration.

Also, you sometimes use the hyphen '-' for your itemized list,
sometimes you use the asterix '*'. Choose one style and stay
consistent.

> +
> +Errors
> +------
> +
> +Following errors are reported to userspace:
> +
> +RX path errors
> +~~~~~~~~~~~~~~
> +
> +============ ===============================================================
> +-ETIMEDOUT   timeout of data reception
> +-EILSEQ      sequence number mismatch during a multi-frame reception
> +-EBADMSG     data reception with wrong padding
> +============ ===============================================================
> +
> +TX path errors
> +~~~~~~~~~~~~~~
> +
> +========== =================================================================
> +-ECOMM     flow control reception timeout
> +-EMSGSIZE  flow control reception overflow
> +-EBADMSG   flow control reception with wrong layout/padding
> +========== =================================================================
> +
> +Examples
> +========
> +
> +Basic node example
> +------------------
> +
> +Following example implements a node using "normal" physical addressing, with
> +RX ID equal to 0x18DAF142 and a TX ID equal to 0x18DA42F1. All options are left
> +to their default.
> +
> +.. code-block:: C
> +
> +  int s;
> +  struct sockaddr_can addr;

Here, I would suggest the C99 designated field initialization:

  struct sockaddr_can addr = {
          .can_family = AF_CAN;
          .can_ifindex = if_nametoindex("can0");
          .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
          .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
  };

Well, this is just a suggestion, feel free to reject it if you do not like it.

> +  int ret;
> +
> +  s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
> +  if (s < 0)
> +      exit(1);
> +
> +  addr.can_family = AF_CAN;
> +  addr.can_ifindex = if_nametoindex("can0");

if_nametoindex() may fail. Because you are doing error handling in
this example, do it also here:

  if (!addr.can_ifindex)
          err("if_nametoindex()");

> +  addr.tp.tx_id = (0x18DA42F1 | CAN_EFF_FLAG);
> +  addr.tp.rx_id = (0x18DAF142 | CAN_EFF_FLAG);

Nitpick: the bracket are not needed here:

  addr.tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
  addr.tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;

> +
> +  ret = bind(s, (struct sockaddr *)&addr, sizeof(addr));
> +  if (ret < 0)
> +      exit(1);
> +
> +  // Data can now be received using read(s, ...) and sent using write(s, ...)

Kernel style prefers C style comments over C++. I think that should
also apply to the documentation:

  /* Data can now be received using read(s, ...) and sent using write(s, ...) */

> +
> +Additional examples
> +-------------------
> +
> +More complete (and complex) examples can be found inside the ``isotp*`` userland
> +tools, distributed as part of the ``can-utils`` utilities at:
> +https://github.com/linux-can/can-utils
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6a233e1a3cf2..e0190b90d1a8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4695,6 +4695,7 @@ W:        https://github.com/linux-can
>  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git
>  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
>  F:     Documentation/networking/can.rst
> +F:     Documentation/networking/iso15765-2.rst
>  F:     include/linux/can/can-ml.h
>  F:     include/linux/can/core.h
>  F:     include/linux/can/skb.h
> --
> 2.44.0
>
>

Oliver Hartkopp April 13, 2024, 5:28 p.m. UTC | #2

Hi Vincent,

On 13.04.24 15:11, Vincent Mailhol wrote:
> Hi Francesco,
> 
> Thank you for the ISO-TP documentation.
> 
> I left a few comments, but overall, good work. Also, I did not double
> check each individual option one by one.
> 
> On Sat. 30 Mar 2024 at 00:06, Francesco Valla <[e-mail]> wrote:
>> Document basic concepts, APIs and behaviour of the ISO 15675-2:2016
>> (ISO-TP) CAN stack.
>>
>> Signed-off-by: Francesco Valla <[e-mail]>
>> ---
>>   Documentation/networking/index.rst      |   1 +
>>   Documentation/networking/iso15765-2.rst | 356 ++++++++++++++++++++++++
>>   MAINTAINERS                             |   1 +
>>   3 files changed, 358 insertions(+)
>>   create mode 100644 Documentation/networking/iso15765-2.rst
>>
>> diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
>> index 473d72c36d61..bbd9bf537793 100644
>> --- a/Documentation/networking/index.rst
>> +++ b/Documentation/networking/index.rst
>> @@ -19,6 +19,7 @@ Contents:
>>      caif/index
>>      ethtool-netlink
>>      ieee802154
>> +   iso15765-2
>>      j1939
>>      kapi
>>      msg_zerocopy
>> diff --git a/Documentation/networking/iso15765-2.rst b/Documentation/networking/iso15765-2.rst
>> new file mode 100644
>> index 000000000000..bbed4d2ef1a8
>> --- /dev/null
>> +++ b/Documentation/networking/iso15765-2.rst
>> @@ -0,0 +1,356 @@
>> +.. SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
>> +
>> +=========================
>> +ISO 15765-2:2016 (ISO-TP)
>> +=========================
>> +
>> +Overview
>> +========
>> +
>> +ISO 15765-2:2016, also known as ISO-TP, is a transport protocol specifically
>> +defined for diagnostic communication on CAN. It is widely used in the automotive
>> +industry, for example as the transport protocol for UDSonCAN (ISO 14229-3) or
>> +emission-related diagnostic services (ISO 15031-5).
>> +
>> +ISO-TP can be used both on CAN CC (aka Classical CAN, CAN 2.0B) and CAN FD (CAN
> 
> CC is already the abbreviation of *C*lassical *C*AN. Saying CAN CC, is
> like saying CAN Classical CAN, c.f. the RAS syndrome:
> 
>    https://en.wikipedia.org/wiki/RAS_syndrome
> 
> Then, considering the CAN2.0B, as far as I know, ISO-TP can also be
> used on CAN2.0A (as long as you only use 11 bits CAN ids).

The suggestion "be used both on CAN CC (aka Classical CAN, CAN 2.0B) and 
CAN FD" was from my side.

And this follows the new CAN in Automation (can-cia.org) naming scheme.
E.g. https://www.can-cia.org/can-knowledge/can/cybersecurity-for-can/
"“Safety and security” specifies generic security options for CAN CC and 
CAN FD protocols."

So your hint to the RAS syndrome is right but in this case the this is 
intentional to be able to reference CAN CC/FD/XL content.

For that reason I wanted to introduce the new CAN CC naming scheme which 
is pretty handy IMO.

> 
> So, I would rather just say:
> 
>    ISO-TP can be used both on Classical CAN and CAN FD...
> 



>> +with Flexible Datarate) based networks. It is also designed to be compatible
>> +with a CAN network using SAE J1939 as data link layer (however, this is not a
>> +requirement).
>> +
>> +Specifications used
>> +-------------------
>> +
>> +* ISO 15765-2:2016 : Road vehicles - Diagnostic communication over Controller
> 
> ISO 15765-2:2016 is withdrawn. The latest version is 15765-2:2024.
> 
>> +  Area Network (DoCAN). Part 2: Transport protocol and network layer services.
>> +
>> +Addressing
>> +----------
>> +
>> +In its simplest form, ISO-TP is based on two kinds of addressing modes for the
>> +nodes connected to the same network:
>> +
>> +- physical addressing is implemented by two node-specific addresses (CAN
>> +  identifiers) and is used in 1-to-1 communication
>> +- functional addressing is implemented by one node-specific address (CAN
>> +  identifier) and is used in 1-to-N communication
>> +
>> +In a so-called "normal" addressing scenario, both these addresses are
>> +represented by a 29-bit CAN ID. However, in order to support larger networks,
>> +an "extended" addressing scheme can be adopted: in this case, the first byte of
>> +the data payload is used as an additional component of the address (both for
>> +the physical and functional cases); two different CAN IDs are still required.
> 
> There is more than that.
> 
>    - The normal addressing can also use the non-extended 11 bits CAN ID.

Right!

>    - In addition to the normal and extended addressing mode, there
>      is a third mode: the mixed addressing.
> 
> Ref:
> 
>    - ISO 15765:2024 §10.3.1 "Addressing formats"
>    - https://www.embedded-communication.com/en/misc/iso-tp-addressing/
> 
>> +
>> +Transport protocol and associated frame types
>> +---------------------------------------------
>> +
>> +When transmitting data using the ISO-TP protocol, the payload can either fit
>> +inside one single CAN message or not, also considering the overhead the protocol
>> +is generating and the optional extended addressing. In the first case, the data
>> +is transmitted at once using a so-called Single Frame (SF). In the second case,
>> +ISO-TP defines a multi-frame protocol, in which the sender provides (through a
>> +First Frame - FF) the PDU length which is to be transmitted and also asks for a
>> +Flow Control (FC) frame, which provides the maximum supported size of a macro
>> +data block (``blocksize``) and the minimum time between the single CAN messages
>> +composing such block (``stmin``). Once this information has been received, the
>> +sender starts to send frames containing fragments of the data payload (called
>> +Consecutive Frames - CF), stopping after every ``blocksize``-sized block to wait
>> +confirmation from the receiver (which should then send another Flow Control
>> +frame to inform the sender about its availability to receive more data).
> 
> Nitpick: I do not see the need for the brackets here:
> 
>    confirmation from the receiver which should then send another Flow Control
>    frame to inform the sender about its availability to receive more data.
> 
>> +How to Use ISO-TP
>> +=================
>> +
>> +As with others CAN protocols, the ISO-TP stack support is built into the
>> +Linux network subsystem for the CAN bus, aka. Linux-CAN or SocketCAN, and
>> +thus follows the same socket API.
>> +
>> +Creation and basic usage of an ISO-TP socket
>> +--------------------------------------------
>> +
>> +To use the ISO-TP stack, ``#include <linux/can/isotp.h>`` shall be used. A
>> +socket can then be created using the ``PF_CAN`` protocol family, the
>> +``SOCK_DGRAM`` type (as the underlying protocol is datagram-based by design)
>> +and the ``CAN_ISOTP`` protocol:
>> +
>> +.. code-block:: C
>> +
>> +    s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
>> +
>> +After the socket has been successfully created, ``bind(2)`` shall be called to
>> +bind the socket to the desired CAN interface; to do so:
>> +
>> +* a TX CAN ID shall be specified as part of the sockaddr supplied to the call
>> +  itself, and
> 
> Why did you put a newline here?
> 
>> +* a RX CAN ID shall also specified, unless broadcast flags have been set
>> +  through socket option (explained below)
> 
> Add a period at the end of your sentence:
> 
>    through socket option (explained below).
> 
>> +Once bound to an interface, the socket can be read from and written to using
>> +the usual ``read(2)`` and ``write(2)`` system calls, as well as ``send(2)``,
>> +``sendmsg(2)``, ``recv(2)`` and ``recvmsg(2)``.
> 
> If this is a new paragraph, leave an empty newline. If not, do not add
> a newline before you reach the character column limit.
> 
>> +Unlike the CAN_RAW socket API, only the data payload shall be specified in all
>> +these calls, as the CAN header is automatically filled by the ISO-TP stack
>> +using information supplied during socket creation. In the same way, the stack
> 
> This is making a shortcut. There are the raw CAN payload and the
> ISO-TP payload. In this paragraph it is not clear that "data payload"
> is referring to the ISO-TP payload.
> 
> Also, what is the meaning of "the CAN header". Here, I think you mean
> CAN ID plus some of the few first byte of the CAN payload.
> 
> I suggest that you use more precise vocabulary from the standard:
> 
>    - Address information
>    - Protocol Information
>    - Data field
> 
> Something like:
> 
>    only the ISO-TP data field (the actual payload) is sent. The
>    address information and the protocol information is
>    automatically filled by the ISO-TP stack...
> 
>> +will use the transport mechanism when required (i.e., when the size of the data
>> +payload exceeds the MTU of the underlying CAN bus).
>> +
>> +The sockaddr structure used for SocketCAN has extensions for use with ISO-TP,
>> +as specified below:
>> +
>> +.. code-block:: C
>> +
>> +    struct sockaddr_can {
>> +        sa_family_t can_family;
>> +        int         can_ifindex;
>> +        union {
>> +            struct { canid_t rx_id, tx_id; } tp;
>> +        ...
>> +        } can_addr;
>> +    }
>> +
>> +* ``can_family`` and ``can_ifindex`` serve the same purpose as for other
>> +  SocketCAN sockets.
>> +
>> +* ``can_addr.tp.rx_id`` specifies the receive (RX) CAN ID and will be used as
>> +  a RX filter.
>> +
>> +* ``can_addr.tp.tx_id`` specifies the transmit (TX) CAN ID
>> +
>> +ISO-TP socket options
>> +---------------------
>> +
>> +When creating an ISO-TP socket, reasonable defaults are set. Some options can
>> +be modified with ``setsockopt(2)`` and/or read back with ``getsockopt(2)``.
>> +
>> +General options
>> +~~~~~~~~~~~~~~~
>> +
>> +General socket options can be passed using the ``CAN_ISOTP_OPTS`` optname:
>> +
>> +.. code-block:: C
>> +
>> +    struct can_isotp_options opts;
>> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_OPTS, &opts, sizeof(opts))
>> +
>> +where the ``can_isotp_options`` structure has the following contents:
>> +
>> +.. code-block:: C
>> +
>> +    struct can_isotp_options {
>> +        u32 flags;
>> +        u32 frame_txtime;
>> +        u8  ext_address;
>> +        u8  txpad_content;
>> +        u8  rxpad_content;
>> +        u8  rx_ext_address;
>> +    };
>> +
>> +* ``flags``: modifiers to be applied to the default behaviour of the ISO-TP
>> +  stack. Following flags are available:
>> +
>> +  - ``CAN_ISOTP_LISTEN_MODE``: listen only (do not send FC frames); normally
>> +    used as a testing feature.
>> +  - ``CAN_ISOTP_EXTEND_ADDR``: enable extended addressing, using the byte
>> +    specified in ``ext_address`` as additional address byte.
>                                      ^^^^^^^^^^^^^
> as an additional
> 
>> +  - ``CAN_ISOTP_TX_PADDING``: enable padding for tranmsitted frames, using
>                                                      ^^^^^^^^^^^
> transmitted
> 
>> +    ``txpad_content`` as value for the padding bytes.
>> +  - ``CAN_ISOTP_RX_PADDING``: enable padding for the received frames, using
>> +    ``rxpad_content`` as value for the padding bytes.
>> +  - ``CAN_ISOTP_CHK_PAD_LEN``: check for correct padding length on the received
>> +    frames.
>> +  - ``CAN_ISOTP_CHK_PAD_DATA``: check padding bytes on the received frames
>> +    against ``rxpad_content``; if ``CAN_ISOTP_RX_PADDING`` is not specified,
>> +    this flag is ignored.
>> +  - ``CAN_ISOTP_HALF_DUPLEX``: force ISO-TP socket in half duples mode
>                                                                ^^^^^^
> duplex
> 
>> +    (that is, transport mechanism can only be incoming or outgoing at the same
>> +    time, not both).
>> +  - ``CAN_ISOTP_FORCE_TXSTMIN``: ignore stmin from received FC; normally
>> +    used as a testing feature.
>> +  - ``CAN_ISOTP_FORCE_RXSTMIN``: ignore CFs depending on rx stmin; normally
>> +    used as a testing feature.
>> +  - ``CAN_ISOTP_RX_EXT_ADDR``: use ``rx_ext_address`` instead of ``ext_address``
>> +    as extended addressing byte on the reception path.
>> +  - ``CAN_ISOTP_WAIT_TX_DONE``: wait until the frame is sent before returning
>> +    from ``write(2)`` and ``send(2)`` calls (i.e., blocking write operations).
>> +  - ``CAN_ISOTP_SF_BROADCAST``: use 1-to-N functional addressing (cannot be
>> +    specified alongside ``CAN_ISOTP_CF_BROADCAST``).
>> +  - ``CAN_ISOTP_CF_BROADCAST``: use 1-to-N transmission without flow control
>> +    (cannot be specified alongside ``CAN_ISOTP_SF_BROADCAST``).
>> +    NOTE: this is not covered by the ISO15765-2:2016 standard.
>                                          ^^^^^^^^
> Add a space between ISO and the number. Also, update the year:
> 
>    ISO 15765-2:2024
> 

Interesting! Didn't know there's already a new version.

Will check this out whether we really support ISO 15765-2:2024 ...

Do you have the standard at hand right now or should we leave this as 
ISO15765-2:2016 until we know?

>> +  - ``CAN_ISOTP_DYN_FC_PARMS``: enable dynamic update of flow control
>> +    parameters.
>> +
> 
> Sometimes you put an empty line between the items, sometimes not. Be consistent.
> 
>> +* ``frame_txtime``: frame transmission time (defined as N_As/N_Ar inside the
>> +  ISO standard); if ``0``, the default (or the last set value) is used.
>> +  To set the transmission time to ``0``, the ``CAN_ISOTP_FRAME_TXTIME_ZERO``
>> +  macro (equal to 0xFFFFFFFF) shall be used.
>> +
>> +* ``ext_address``: extended addressing byte, used if the
>> +  ``CAN_ISOTP_EXTEND_ADDR`` flag is specified.
>> +
>> +* ``txpad_content``: byte used as padding value for transmitted frames
> 
> Add a period at the end of the sentence.
> 
>> +
>> +* ``rxpad_content``: byte used as padding value for received frames
> 
> Add a period at the end of the sentence.
> 
>> +
>> +* ``rx_ext_address``: extended addressing byte for the reception path, used if
>> +  the ``CAN_ISOTP_RX_EXT_ADDR`` flag is specified.
>> +
>> +Flow Control options
>> +~~~~~~~~~~~~~~~~~~~~
>> +
>> +Flow Control (FC) options can be passed using the ``CAN_ISOTP_RECV_FC`` optname
>> +to provide the communication parameters for receiving ISO-TP PDUs.
>> +
>> +.. code-block:: C
>> +
>> +    struct can_isotp_fc_options fc_opts;
>> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_RECV_FC, &fc_opts, sizeof(fc_opts));
>> +
>> +where the ``can_isotp_fc_options`` structure has the following contents:
>> +
>> +.. code-block:: C
>> +
>> +    struct can_isotp_options {
>> +        u8 bs;
>> +        u8 stmin;
>> +        u8 wftmax;
>> +    };
>> +
>> +* ``bs``: blocksize provided in flow control frames.
>> +
>> +* ``stmin``: minimum separation time provided in flow control frames; can
>> +  have the following values (others are reserved):
>> +  - 0x00 - 0x7F : 0 - 127 ms
>> +  - 0xF1 - 0xF9 : 100 us - 900 us
>> +
>> +* ``wftmax``: maximum number of wait frames provided in flow control frames.
>> +
>> +Link Layer options
>> +~~~~~~~~~~~~~~~~~~
>> +
>> +Link Layer (LL) options can be passed using the ``CAN_ISOTP_LL_OPTS`` optname:
>> +
>> +.. code-block:: C
>> +
>> +    struct can_isotp_ll_options ll_opts;
>> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_LL_OPTS, &ll_opts, sizeof(ll_opts));
>> +
>> +where the ``can_isotp_ll_options`` structure has the following contents:
>> +
>> +.. code-block:: C
>> +
>> +    struct can_isotp_ll_options {
>> +        u8 mtu;
>> +        u8 tx_dl;
>> +        u8 tx_flags;
>> +    };
>> +
>> +* ``mtu``: generated and accepted CAN frame type, can be equal to ``CAN_MTU``
>> +  for classical CAN frames or ``CANFD_MTU`` for CAN FD frames.
>> +
>> +* ``tx_dl``: maximum payload length for transmitted frames, can have one value
>> +  among: 8, 12, 16, 20, 24, 32, 48, 64. Values above 8 only apply to CAN FD
>> +  traffic (i.e.: ``mtu = CANFD_MTU``).
>> +
>> +* ``tx_flags``: flags set into ``struct canfd_frame.flags`` at frame creation.
>> +  Only applies to CAN FD traffic (i.e.: ``mtu = CANFD_MTU``).
>> +
>> +Transmission stmin
>> +~~~~~~~~~~~~~~~~~~
>> +
>> +The transmission minimum separaton time (stmin) can be forced using the
>                              ^^^^^^^^^
> separation
> 
>> +``CAN_ISOTP_TX_STMIN`` optname and providing an stmin value in microseconds as
>> +a 32bit unsigned integer; this will overwrite the value sent by the receiver in
>> +flow control frames:
>> +
>> +.. code-block:: C
>> +
>> +    uint32_t stmin;
>> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_TX_STMIN, &stmin, sizeof(stmin));
>> +
>> +Reception stmin
>> +~~~~~~~~~~~~~~~
>> +
>> +The reception minimum separaton time (stmin) can be forced using the
>                           ^^^^^^^^^
> separation
> 
>> +``CAN_ISOTP_RX_STMIN`` optname and providing an stmin value in microseconds as
>> +a 32bit unsigned integer; received Consecutive Frames (CF) which timestamps
>> +differ less than this value will be ignored:
>> +
>> +.. code-block:: C
>> +
>> +    uint32_t stmin;
>> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_RX_STMIN, &stmin, sizeof(stmin));
>> +
>> +Multi-frame transport support
>> +-----------------------------
>> +
>> +The ISO-TP stack contained inside the Linux kernel supports the multi-frame
>> +transport mechanism defined by the standard, with the following constraints:
>> +
>> +* the maximum size of a PDU is defined by a module parameter, with an hard
>> +  limit imposed at build time
>> +* when a transmission is in progress, subsequent calls to ``write(2)`` will
>> +  block, while calls to ``send(2)`` will either block or fail depending on the
>> +  presence of the ``MSG_DONTWAIT`` flag
>> +* no support is present for sending "wait frames": whether a PDU can be fully
>> +  received or not is decided when the First Frame is received
> 
> Add a period at the end of all the sentences of this enumeration.
> 
> Also, you sometimes use the hyphen '-' for your itemized list,
> sometimes you use the asterix '*'. Choose one style and stay
> consistent.
> 
>> +
>> +Errors
>> +------
>> +
>> +Following errors are reported to userspace:
>> +
>> +RX path errors
>> +~~~~~~~~~~~~~~
>> +
>> +============ ===============================================================
>> +-ETIMEDOUT   timeout of data reception
>> +-EILSEQ      sequence number mismatch during a multi-frame reception
>> +-EBADMSG     data reception with wrong padding
>> +============ ===============================================================
>> +
>> +TX path errors
>> +~~~~~~~~~~~~~~
>> +
>> +========== =================================================================
>> +-ECOMM     flow control reception timeout
>> +-EMSGSIZE  flow control reception overflow
>> +-EBADMSG   flow control reception with wrong layout/padding
>> +========== =================================================================
>> +
>> +Examples
>> +========
>> +
>> +Basic node example
>> +------------------
>> +
>> +Following example implements a node using "normal" physical addressing, with
>> +RX ID equal to 0x18DAF142 and a TX ID equal to 0x18DA42F1. All options are left
>> +to their default.
>> +
>> +.. code-block:: C
>> +
>> +  int s;
>> +  struct sockaddr_can addr;
> 
> Here, I would suggest the C99 designated field initialization:
> 
>    struct sockaddr_can addr = {
>            .can_family = AF_CAN;
>            .can_ifindex = if_nametoindex("can0");
>            .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
>            .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
>    };
> 
> Well, this is just a suggestion, feel free to reject it if you do not like it.

At least I don't like it.

These values are usually interactively given on the command line:

 >            .can_ifindex = if_nametoindex("can0");
 >            .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
 >            .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;

So have it in a static field initialization leads to a wrong path IMO.

> 
>> +  int ret;
>> +
>> +  s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
>> +  if (s < 0)
>> +      exit(1);
>> +
>> +  addr.can_family = AF_CAN;
>> +  addr.can_ifindex = if_nametoindex("can0");
> 
> if_nametoindex() may fail. Because you are doing error handling in
> this example, do it also here:
> 
>    if (!addr.can_ifindex)
>            err("if_nametoindex()");
> 

This is not really needed for an example like this.

When we have a problem here the bind() syscall with fail with -ENODEV

>> +  addr.tp.tx_id = (0x18DA42F1 | CAN_EFF_FLAG);
>> +  addr.tp.rx_id = (0x18DAF142 | CAN_EFF_FLAG);
> 
> Nitpick: the bracket are not needed here:
> 
>    addr.tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
>    addr.tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
> 
>> +
>> +  ret = bind(s, (struct sockaddr *)&addr, sizeof(addr));
>> +  if (ret < 0)
>> +      exit(1);
>> +
>> +  // Data can now be received using read(s, ...) and sent using write(s, ...)
> 
> Kernel style prefers C style comments over C++. I think that should
> also apply to the documentation:
> 
>    /* Data can now be received using read(s, ...) and sent using write(s, ...) */
>

ACK

>> +
>> +Additional examples
>> +-------------------
>> +
>> +More complete (and complex) examples can be found inside the ``isotp*`` userland
>> +tools, distributed as part of the ``can-utils`` utilities at:
>> +https://github.com/linux-can/can-utils
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 6a233e1a3cf2..e0190b90d1a8 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4695,6 +4695,7 @@ W:        https://github.com/linux-can
>>   T:     git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git
>>   T:     git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
>>   F:     Documentation/networking/can.rst
>> +F:     Documentation/networking/iso15765-2.rst
>>   F:     include/linux/can/can-ml.h
>>   F:     include/linux/can/core.h
>>   F:     include/linux/can/skb.h
>> --
>> 2.44.0
>>
>>
> 

Thanks for the review, Vincent!

Best regards,
Oliver

Vincent Mailhol April 14, 2024, 4:03 a.m. UTC | #3

On Sun. 14 Apr. 2024 at 02:29, Oliver Hartkopp <[e-mail]> wrote:
> Hi Vincent,
>
> On 13.04.24 15:11, Vincent Mailhol wrote:
> > Hi Francesco,
> >
> > Thank you for the ISO-TP documentation.
> >
> > I left a few comments, but overall, good work. Also, I did not double
> > check each individual option one by one.
> >
> > On Sat. 30 Mar 2024 at 00:06, Francesco Valla <[e-mail]> wrote:
> >> Document basic concepts, APIs and behaviour of the ISO 15675-2:2016
> >> (ISO-TP) CAN stack.
> >>
> >> Signed-off-by: Francesco Valla <[e-mail]>
> >> ---
> >>   Documentation/networking/index.rst      |   1 +
> >>   Documentation/networking/iso15765-2.rst | 356 ++++++++++++++++++++++++
> >>   MAINTAINERS                             |   1 +
> >>   3 files changed, 358 insertions(+)
> >>   create mode 100644 Documentation/networking/iso15765-2.rst
> >>
> >> diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
> >> index 473d72c36d61..bbd9bf537793 100644
> >> --- a/Documentation/networking/index.rst
> >> +++ b/Documentation/networking/index.rst
> >> @@ -19,6 +19,7 @@ Contents:
> >>      caif/index
> >>      ethtool-netlink
> >>      ieee802154
> >> +   iso15765-2
> >>      j1939
> >>      kapi
> >>      msg_zerocopy
> >> diff --git a/Documentation/networking/iso15765-2.rst b/Documentation/networking/iso15765-2.rst
> >> new file mode 100644
> >> index 000000000000..bbed4d2ef1a8
> >> --- /dev/null
> >> +++ b/Documentation/networking/iso15765-2.rst
> >> @@ -0,0 +1,356 @@
> >> +.. SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> >> +
> >> +=========================
> >> +ISO 15765-2:2016 (ISO-TP)
> >> +=========================
> >> +
> >> +Overview
> >> +========
> >> +
> >> +ISO 15765-2:2016, also known as ISO-TP, is a transport protocol specifically
> >> +defined for diagnostic communication on CAN. It is widely used in the automotive
> >> +industry, for example as the transport protocol for UDSonCAN (ISO 14229-3) or
> >> +emission-related diagnostic services (ISO 15031-5).
> >> +
> >> +ISO-TP can be used both on CAN CC (aka Classical CAN, CAN 2.0B) and CAN FD (CAN
> >
> > CC is already the abbreviation of *C*lassical *C*AN. Saying CAN CC, is
> > like saying CAN Classical CAN, c.f. the RAS syndrome:
> >
> >    https://en.wikipedia.org/wiki/RAS_syndrome
> >
> > Then, considering the CAN2.0B, as far as I know, ISO-TP can also be
> > used on CAN2.0A (as long as you only use 11 bits CAN ids).
>
> The suggestion "be used both on CAN CC (aka Classical CAN, CAN 2.0B) and
> CAN FD" was from my side.
>
> And this follows the new CAN in Automation (can-cia.org) naming scheme.
> E.g. https://www.can-cia.org/can-knowledge/can/cybersecurity-for-can/
> "“Safety and security” specifies generic security options for CAN CC and
> CAN FD protocols."
>
> So your hint to the RAS syndrome is right but in this case the this is
> intentional to be able to reference CAN CC/FD/XL content.
>
> For that reason I wanted to introduce the new CAN CC naming scheme which
> is pretty handy IMO.

I double checked. ISO 11898-1:2015 and ISO 15765-2:2016 never use the
"CC" abbreviation a single time, thus my confusion. *BUT* ISO
15765-2:2024 actually uses that naming, in the exact same way that CAN
in Automation does.

This doesn't remove the fact that I think that this naming convention
is stupid because of the RAS syndrome, but I acknowledge that CAN CC
is now the official denomination and thus, that we should adopt it in
our documentation as well.

> > So, I would rather just say:
> >
> >    ISO-TP can be used both on Classical CAN and CAN FD...
> >

(...)

> >> +    NOTE: this is not covered by the ISO15765-2:2016 standard.
> >                                          ^^^^^^^^
> > Add a space between ISO and the number. Also, update the year:
> >
> >    ISO 15765-2:2024
> >
>
> Interesting! Didn't know there's already a new version.
>
> Will check this out whether we really support ISO 15765-2:2024 ...
>
> Do you have the standard at hand right now or should we leave this as
> ISO15765-2:2016 until we know?

I have access to the newer revisions. But I never really invested time
into reading that standard (neither the 2016 nor the 2024 versions).

Regardless, here is a verbatim extract from the Foreworld section of
ISO 15765-2:2024

  This fourth edition cancels and replaces the third edition (ISO
  15765-2:2016), which has been technically revised.

  The main changes are as follows:

    - restructured the document to achieve compatibility with OSI
      7-layers model;

    - introduced T_Data abstract service primitive interface to
      achieve compatibility with ISO 14229-2;

    - moved all transport layer protocol-related information to Clause 9;

    - clarification and editorial corrections

> >> +  - ``CAN_ISOTP_DYN_FC_PARMS``: enable dynamic update of flow control
> >> +    parameters.

(...)

> >
> > Here, I would suggest the C99 designated field initialization:
> >
> >    struct sockaddr_can addr = {
> >            .can_family = AF_CAN;
> >            .can_ifindex = if_nametoindex("can0");
> >            .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
> >            .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
> >    };

Typo in my previous message: the designated initializers are not
separated by colon ";" but by comma ",". So it should have been:

  struct sockaddr_can addr = {
        .can_family = AF_CAN,
        .can_ifindex = if_nametoindex("can0"),
        .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG,
        .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG,
  };

> > Well, this is just a suggestion, feel free to reject it if you do not like it.
>
> At least I don't like it.
>
> These values are usually interactively given on the command line:
>
>  >            .can_ifindex = if_nametoindex("can0");
>  >            .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
>  >            .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
>
> So have it in a static field initialization leads to a wrong path IMO.

There is no such limitation that C99 designated initializers should
only work with variables which have static storage duration. In my
suggested example, nothing is static.

I see this as the same thing as below example:

  int foo(void);

  int bar()
  {
          int i = foo();
  }

  int baz()
  {
          int i;

          i = foo();
  }

In bar(), the fact that the variable i is initialized at declaration
does not mean that it is static. In both examples, the variable i uses
automatic storage duration.

Here, my preference goes to bar(), but I recognize that baz() is also
perfectly fine. Replace the int type by the struct sockaddr_can type
and the scalar initialization by designated initializers and you
should see the connection.


** Different topic **

While replying on this, I encountered something which made me worry a bit:

The type of sockaddr_can.can_ifindex is a signed int:

  https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/can.h#L243

But if_nametoindex() returns an unsigned int:

   https://man7.org/linux/man-pages/man3/if_nametoindex.3.html

Shouldn't sockaddr_can.can_ifindex also be declared as an unsigned int?

> >
> >> +  int ret;
> >> +
> >> +  s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
> >> +  if (s < 0)
> >> +      exit(1);
> >> +
> >> +  addr.can_family = AF_CAN;
> >> +  addr.can_ifindex = if_nametoindex("can0");
> >
> > if_nametoindex() may fail. Because you are doing error handling in
> > this example, do it also here:
> >
> >    if (!addr.can_ifindex)
> >            err("if_nametoindex()");
> >
>
> This is not really needed for an example like this.
>
> When we have a problem here the bind() syscall with fail with -ENODEV

Ack.

> >> +  addr.tp.tx_id = (0x18DA42F1 | CAN_EFF_FLAG);
> >> +  addr.tp.rx_id = (0x18DAF142 | CAN_EFF_FLAG);
> >
> > Nitpick: the bracket are not needed here:
> >
> >    addr.tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
> >    addr.tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
> >
> >> +
> >> +  ret = bind(s, (struct sockaddr *)&addr, sizeof(addr));
> >> +  if (ret < 0)
> >> +      exit(1);
> >> +
> >> +  // Data can now be received using read(s, ...) and sent using write(s, ...)
> >
> > Kernel style prefers C style comments over C++. I think that should
> > also apply to the documentation:
> >
> >    /* Data can now be received using read(s, ...) and sent using write(s, ...) */
> >
>
> ACK
>
> >> +
> >> +Additional examples
> >> +-------------------
> >> +
> >> +More complete (and complex) examples can be found inside the ``isotp*`` userland
> >> +tools, distributed as part of the ``can-utils`` utilities at:
> >> +https://github.com/linux-can/can-utils
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 6a233e1a3cf2..e0190b90d1a8 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -4695,6 +4695,7 @@ W:        https://github.com/linux-can
> >>   T:     git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git
> >>   T:     git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
> >>   F:     Documentation/networking/can.rst
> >> +F:     Documentation/networking/iso15765-2.rst
> >>   F:     include/linux/can/can-ml.h
> >>   F:     include/linux/can/core.h
> >>   F:     include/linux/can/skb.h
> >> --
> >> 2.44.0
> >>
> >>
> >
>
> Thanks for the review, Vincent!

You are welcome!

Yours sincerely,
Vincent Mailhol

Oliver Hartkopp April 14, 2024, 8:21 p.m. UTC | #4

On 14.04.24 06:03, Vincent Mailhol wrote:

> 
> This doesn't remove the fact that I think that this naming convention
> is stupid because of the RAS syndrome, but I acknowledge that CAN CC
> is now the official denomination and thus, that we should adopt it in
> our documentation as well.
> 

;-)


>>> Add a space between ISO and the number. Also, update the year:
>>>
>>>     ISO 15765-2:2024
>>>
>>
>> Interesting! Didn't know there's already a new version.
>>
>> Will check this out whether we really support ISO 15765-2:2024 ...
>>
>> Do you have the standard at hand right now or should we leave this as
>> ISO15765-2:2016 until we know?
> 
> I have access to the newer revisions. But I never really invested time
> into reading that standard (neither the 2016 nor the 2024 versions).
> 
> Regardless, here is a verbatim extract from the Foreworld section of
> ISO 15765-2:2024
> 
>    This fourth edition cancels and replaces the third edition (ISO
>    15765-2:2016), which has been technically revised.
> 
>    The main changes are as follows:
> 
>      - restructured the document to achieve compatibility with OSI
>        7-layers model;
> 
>      - introduced T_Data abstract service primitive interface to
>        achieve compatibility with ISO 14229-2;
> 
>      - moved all transport layer protocol-related information to Clause 9;
> 
>      - clarification and editorial corrections
> 

Yes, I've checked the release notes on the ISO website too.
This really looks like editorial stuff that has nothing to do with the 
data protocol and its segmentation.

>>>
>>> Here, I would suggest the C99 designated field initialization:
>>>
>>>     struct sockaddr_can addr = {
>>>             .can_family = AF_CAN;
>>>             .can_ifindex = if_nametoindex("can0");
>>>             .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
>>>             .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
>>>     };
> 
> Typo in my previous message: the designated initializers are not
> separated by colon ";" but by comma ",". So it should have been:
> 
>    struct sockaddr_can addr = {
>          .can_family = AF_CAN,
>          .can_ifindex = if_nametoindex("can0"),
>          .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG,
>          .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG,
>    };
> 
>>> Well, this is just a suggestion, feel free to reject it if you do not like it.
>>
>> At least I don't like it.
>>
>> These values are usually interactively given on the command line:
>>
>>   >            .can_ifindex = if_nametoindex("can0");
>>   >            .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
>>   >            .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
>>
>> So have it in a static field initialization leads to a wrong path IMO.
> 
> There is no such limitation that C99 designated initializers should
> only work with variables which have static storage duration. In my
> suggested example, nothing is static.
> 
> I see this as the same thing as below example:
> 
>    int foo(void);
> 
>    int bar()
>    {
>            int i = foo();
>    }
> 
>    int baz()
>    {
>            int i;
> 
>            i = foo();
>    }
> 
> In bar(), the fact that the variable i is initialized at declaration
> does not mean that it is static. In both examples, the variable i uses
> automatic storage duration.
> 
> Here, my preference goes to bar(), but I recognize that baz() is also
> perfectly fine. Replace the int type by the struct sockaddr_can type
> and the scalar initialization by designated initializers and you
> should see the connection.

Oh, sorry. Maybe I expressed myself wrong.

IMHO your way to work with an initializer is correct from the C standpoint.

But I think this is pretty unusual for a code example when an 
application programmer starts to work with ISO-TP.

You usually get most of these values from the command line an fill the 
struct _by hand_ - and not with a static initialization.

That was my suggestion.

> 
> ** Different topic **
> 
> While replying on this, I encountered something which made me worry a bit:
> 
> The type of sockaddr_can.can_ifindex is a signed int:
> 
>    https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/can.h#L243
> 
> But if_nametoindex() returns an unsigned int:
> 
>     https://man7.org/linux/man-pages/man3/if_nametoindex.3.html
> 
> Shouldn't sockaddr_can.can_ifindex also be declared as an unsigned int?
> 

The if_index derives from struct netdevice.if_index

https://elixir.bootlin.com/linux/v6.8.6/source/include/linux/netdevice.h#L2158

which is an int.

I don't think this would have an effect in real world to change 
sockaddr_can.can_ifindex to an unsigned int.

I wonder if it is more critical to existing user space code to change it 
to unsigned int or to leave it as-is ...

Best regards,
Oliver

Bagas Sanjaya April 15, 2024, 3:09 a.m. UTC | #5

On Fri, Mar 29, 2024 at 02:34:41PM +0100, Francesco Valla wrote:
> diff --git a/Documentation/networking/iso15765-2.rst b/Documentation/networking/iso15765-2.rst
> new file mode 100644
> index 000000000000..bbed4d2ef1a8
> --- /dev/null
> +++ b/Documentation/networking/iso15765-2.rst
> @@ -0,0 +1,356 @@
> +.. SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +
> +=========================
> +ISO 15765-2:2016 (ISO-TP)
> +=========================
> +
> +Overview
> +========
> +
> +ISO 15765-2:2016, also known as ISO-TP, is a transport protocol specifically
> +defined for diagnostic communication on CAN. It is widely used in the automotive
> +industry, for example as the transport protocol for UDSonCAN (ISO 14229-3) or
> +emission-related diagnostic services (ISO 15031-5).
> +
> +ISO-TP can be used both on CAN CC (aka Classical CAN, CAN 2.0B) and CAN FD (CAN
> +with Flexible Datarate) based networks. It is also designed to be compatible
> +with a CAN network using SAE J1939 as data link layer (however, this is not a
> +requirement).
> +
> +Specifications used
> +-------------------
> +
> +* ISO 15765-2:2016 : Road vehicles - Diagnostic communication over Controller
> +  Area Network (DoCAN). Part 2: Transport protocol and network layer services.
> +
> +Addressing
> +----------
> +
> +In its simplest form, ISO-TP is based on two kinds of addressing modes for the
> +nodes connected to the same network:
> +
> +- physical addressing is implemented by two node-specific addresses (CAN
> +  identifiers) and is used in 1-to-1 communication
> +- functional addressing is implemented by one node-specific address (CAN
> +  identifier) and is used in 1-to-N communication
> +
> +In a so-called "normal" addressing scenario, both these addresses are
> +represented by a 29-bit CAN ID. However, in order to support larger networks,
> +an "extended" addressing scheme can be adopted: in this case, the first byte of
> +the data payload is used as an additional component of the address (both for
> +the physical and functional cases); two different CAN IDs are still required.
> +
> +Transport protocol and associated frame types
> +---------------------------------------------
> +
> +When transmitting data using the ISO-TP protocol, the payload can either fit
> +inside one single CAN message or not, also considering the overhead the protocol
> +is generating and the optional extended addressing. In the first case, the data
> +is transmitted at once using a so-called Single Frame (SF). In the second case,
> +ISO-TP defines a multi-frame protocol, in which the sender provides (through a
> +First Frame - FF) the PDU length which is to be transmitted and also asks for a
> +Flow Control (FC) frame, which provides the maximum supported size of a macro
> +data block (``blocksize``) and the minimum time between the single CAN messages
> +composing such block (``stmin``). Once this information has been received, the
> +sender starts to send frames containing fragments of the data payload (called
> +Consecutive Frames - CF), stopping after every ``blocksize``-sized block to wait
> +confirmation from the receiver (which should then send another Flow Control
> +frame to inform the sender about its availability to receive more data).
> +
> +How to Use ISO-TP
> +=================
> +
> +As with others CAN protocols, the ISO-TP stack support is built into the
> +Linux network subsystem for the CAN bus, aka. Linux-CAN or SocketCAN, and
> +thus follows the same socket API.
> +
> +Creation and basic usage of an ISO-TP socket
> +--------------------------------------------
> +
> +To use the ISO-TP stack, ``#include <linux/can/isotp.h>`` shall be used. A
> +socket can then be created using the ``PF_CAN`` protocol family, the
> +``SOCK_DGRAM`` type (as the underlying protocol is datagram-based by design)
> +and the ``CAN_ISOTP`` protocol:
> +
> +.. code-block:: C
> +
> +    s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
> +
> +After the socket has been successfully created, ``bind(2)`` shall be called to
> +bind the socket to the desired CAN interface; to do so:
> +
> +* a TX CAN ID shall be specified as part of the sockaddr supplied to the call
> +  itself, and
> +* a RX CAN ID shall also specified, unless broadcast flags have been set
> +  through socket option (explained below)
> +
> +Once bound to an interface, the socket can be read from and written to using
> +the usual ``read(2)`` and ``write(2)`` system calls, as well as ``send(2)``,
> +``sendmsg(2)``, ``recv(2)`` and ``recvmsg(2)``.
> +Unlike the CAN_RAW socket API, only the data payload shall be specified in all
> +these calls, as the CAN header is automatically filled by the ISO-TP stack
> +using information supplied during socket creation. In the same way, the stack
> +will use the transport mechanism when required (i.e., when the size of the data
> +payload exceeds the MTU of the underlying CAN bus).
> +
> +The sockaddr structure used for SocketCAN has extensions for use with ISO-TP,
> +as specified below:
> +
> +.. code-block:: C
> +
> +    struct sockaddr_can {
> +        sa_family_t can_family;
> +        int         can_ifindex;
> +        union {
> +            struct { canid_t rx_id, tx_id; } tp;
> +        ...
> +        } can_addr;
> +    }
> +
> +* ``can_family`` and ``can_ifindex`` serve the same purpose as for other
> +  SocketCAN sockets.
> +
> +* ``can_addr.tp.rx_id`` specifies the receive (RX) CAN ID and will be used as
> +  a RX filter.
> +
> +* ``can_addr.tp.tx_id`` specifies the transmit (TX) CAN ID
> +
> +ISO-TP socket options
> +---------------------
> +
> +When creating an ISO-TP socket, reasonable defaults are set. Some options can
> +be modified with ``setsockopt(2)`` and/or read back with ``getsockopt(2)``.
> +
> +General options
> +~~~~~~~~~~~~~~~
> +
> +General socket options can be passed using the ``CAN_ISOTP_OPTS`` optname:
> +
> +.. code-block:: C
> +
> +    struct can_isotp_options opts;
> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_OPTS, &opts, sizeof(opts))
> +
> +where the ``can_isotp_options`` structure has the following contents:
> +
> +.. code-block:: C
> +
> +    struct can_isotp_options {
> +        u32 flags;
> +        u32 frame_txtime;
> +        u8  ext_address;
> +        u8  txpad_content;
> +        u8  rxpad_content;
> +        u8  rx_ext_address;
> +    };
> +
> +* ``flags``: modifiers to be applied to the default behaviour of the ISO-TP
> +  stack. Following flags are available:
> +
> +  - ``CAN_ISOTP_LISTEN_MODE``: listen only (do not send FC frames); normally
> +    used as a testing feature.
> +  - ``CAN_ISOTP_EXTEND_ADDR``: enable extended addressing, using the byte
> +    specified in ``ext_address`` as additional address byte.
> +  - ``CAN_ISOTP_TX_PADDING``: enable padding for tranmsitted frames, using
> +    ``txpad_content`` as value for the padding bytes.
> +  - ``CAN_ISOTP_RX_PADDING``: enable padding for the received frames, using
> +    ``rxpad_content`` as value for the padding bytes.
> +  - ``CAN_ISOTP_CHK_PAD_LEN``: check for correct padding length on the received
> +    frames.
> +  - ``CAN_ISOTP_CHK_PAD_DATA``: check padding bytes on the received frames
> +    against ``rxpad_content``; if ``CAN_ISOTP_RX_PADDING`` is not specified,
> +    this flag is ignored.
> +  - ``CAN_ISOTP_HALF_DUPLEX``: force ISO-TP socket in half duples mode
> +    (that is, transport mechanism can only be incoming or outgoing at the same
> +    time, not both).
> +  - ``CAN_ISOTP_FORCE_TXSTMIN``: ignore stmin from received FC; normally
> +    used as a testing feature.
> +  - ``CAN_ISOTP_FORCE_RXSTMIN``: ignore CFs depending on rx stmin; normally
> +    used as a testing feature.
> +  - ``CAN_ISOTP_RX_EXT_ADDR``: use ``rx_ext_address`` instead of ``ext_address``
> +    as extended addressing byte on the reception path.
> +  - ``CAN_ISOTP_WAIT_TX_DONE``: wait until the frame is sent before returning
> +    from ``write(2)`` and ``send(2)`` calls (i.e., blocking write operations).
> +  - ``CAN_ISOTP_SF_BROADCAST``: use 1-to-N functional addressing (cannot be
> +    specified alongside ``CAN_ISOTP_CF_BROADCAST``).
> +  - ``CAN_ISOTP_CF_BROADCAST``: use 1-to-N transmission without flow control
> +    (cannot be specified alongside ``CAN_ISOTP_SF_BROADCAST``).
> +    NOTE: this is not covered by the ISO15765-2:2016 standard.
> +  - ``CAN_ISOTP_DYN_FC_PARMS``: enable dynamic update of flow control
> +    parameters.
> +
> +* ``frame_txtime``: frame transmission time (defined as N_As/N_Ar inside the
> +  ISO standard); if ``0``, the default (or the last set value) is used.
> +  To set the transmission time to ``0``, the ``CAN_ISOTP_FRAME_TXTIME_ZERO``
> +  macro (equal to 0xFFFFFFFF) shall be used.
> +
> +* ``ext_address``: extended addressing byte, used if the
> +  ``CAN_ISOTP_EXTEND_ADDR`` flag is specified.
> +
> +* ``txpad_content``: byte used as padding value for transmitted frames
> +
> +* ``rxpad_content``: byte used as padding value for received frames
> +
> +* ``rx_ext_address``: extended addressing byte for the reception path, used if
> +  the ``CAN_ISOTP_RX_EXT_ADDR`` flag is specified.
> +
> +Flow Control options
> +~~~~~~~~~~~~~~~~~~~~
> +
> +Flow Control (FC) options can be passed using the ``CAN_ISOTP_RECV_FC`` optname
> +to provide the communication parameters for receiving ISO-TP PDUs.
> +
> +.. code-block:: C
> +
> +    struct can_isotp_fc_options fc_opts;
> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_RECV_FC, &fc_opts, sizeof(fc_opts));
> +
> +where the ``can_isotp_fc_options`` structure has the following contents:
> +
> +.. code-block:: C
> +
> +    struct can_isotp_options {
> +        u8 bs;
> +        u8 stmin;
> +        u8 wftmax;
> +    };
> +
> +* ``bs``: blocksize provided in flow control frames.
> +
> +* ``stmin``: minimum separation time provided in flow control frames; can
> +  have the following values (others are reserved):
> +  - 0x00 - 0x7F : 0 - 127 ms
> +  - 0xF1 - 0xF9 : 100 us - 900 us
> +
> +* ``wftmax``: maximum number of wait frames provided in flow control frames.
> +
> +Link Layer options
> +~~~~~~~~~~~~~~~~~~
> +
> +Link Layer (LL) options can be passed using the ``CAN_ISOTP_LL_OPTS`` optname:
> +
> +.. code-block:: C
> +
> +    struct can_isotp_ll_options ll_opts;
> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_LL_OPTS, &ll_opts, sizeof(ll_opts));
> +
> +where the ``can_isotp_ll_options`` structure has the following contents:
> +
> +.. code-block:: C
> +
> +    struct can_isotp_ll_options {
> +        u8 mtu;
> +        u8 tx_dl;
> +        u8 tx_flags;
> +    };
> +
> +* ``mtu``: generated and accepted CAN frame type, can be equal to ``CAN_MTU``
> +  for classical CAN frames or ``CANFD_MTU`` for CAN FD frames.
> +
> +* ``tx_dl``: maximum payload length for transmitted frames, can have one value
> +  among: 8, 12, 16, 20, 24, 32, 48, 64. Values above 8 only apply to CAN FD
> +  traffic (i.e.: ``mtu = CANFD_MTU``).
> +
> +* ``tx_flags``: flags set into ``struct canfd_frame.flags`` at frame creation.
> +  Only applies to CAN FD traffic (i.e.: ``mtu = CANFD_MTU``).
> +
> +Transmission stmin
> +~~~~~~~~~~~~~~~~~~
> +
> +The transmission minimum separaton time (stmin) can be forced using the
> +``CAN_ISOTP_TX_STMIN`` optname and providing an stmin value in microseconds as
> +a 32bit unsigned integer; this will overwrite the value sent by the receiver in
> +flow control frames:
> +
> +.. code-block:: C
> +
> +    uint32_t stmin;
> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_TX_STMIN, &stmin, sizeof(stmin));
> +
> +Reception stmin
> +~~~~~~~~~~~~~~~
> +
> +The reception minimum separaton time (stmin) can be forced using the
> +``CAN_ISOTP_RX_STMIN`` optname and providing an stmin value in microseconds as
> +a 32bit unsigned integer; received Consecutive Frames (CF) which timestamps
> +differ less than this value will be ignored:
> +
> +.. code-block:: C
> +
> +    uint32_t stmin;
> +    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_RX_STMIN, &stmin, sizeof(stmin));
> +
> +Multi-frame transport support
> +-----------------------------
> +
> +The ISO-TP stack contained inside the Linux kernel supports the multi-frame
> +transport mechanism defined by the standard, with the following constraints:
> +
> +* the maximum size of a PDU is defined by a module parameter, with an hard
> +  limit imposed at build time
> +* when a transmission is in progress, subsequent calls to ``write(2)`` will
> +  block, while calls to ``send(2)`` will either block or fail depending on the
> +  presence of the ``MSG_DONTWAIT`` flag
> +* no support is present for sending "wait frames": whether a PDU can be fully
> +  received or not is decided when the First Frame is received
> +
> +Errors
> +------
> +
> +Following errors are reported to userspace:
> +
> +RX path errors
> +~~~~~~~~~~~~~~
> +
> +============ ===============================================================
> +-ETIMEDOUT   timeout of data reception
> +-EILSEQ      sequence number mismatch during a multi-frame reception
> +-EBADMSG     data reception with wrong padding
> +============ ===============================================================
> +
> +TX path errors
> +~~~~~~~~~~~~~~
> +
> +========== =================================================================
> +-ECOMM     flow control reception timeout
> +-EMSGSIZE  flow control reception overflow
> +-EBADMSG   flow control reception with wrong layout/padding
> +========== =================================================================
> +
> +Examples
> +========
> +
> +Basic node example
> +------------------
> +
> +Following example implements a node using "normal" physical addressing, with
> +RX ID equal to 0x18DAF142 and a TX ID equal to 0x18DA42F1. All options are left
> +to their default.
> +
> +.. code-block:: C
> +
> +  int s;
> +  struct sockaddr_can addr;
> +  int ret;
> +
> +  s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
> +  if (s < 0)
> +      exit(1);
> +
> +  addr.can_family = AF_CAN;
> +  addr.can_ifindex = if_nametoindex("can0");
> +  addr.tp.tx_id = (0x18DA42F1 | CAN_EFF_FLAG);
> +  addr.tp.rx_id = (0x18DAF142 | CAN_EFF_FLAG);
> +
> +  ret = bind(s, (struct sockaddr *)&addr, sizeof(addr));
> +  if (ret < 0)
> +      exit(1);
> +
> +  // Data can now be received using read(s, ...) and sent using write(s, ...)
> +
> +Additional examples
> +-------------------
> +
> +More complete (and complex) examples can be found inside the ``isotp*`` userland
> +tools, distributed as part of the ``can-utils`` utilities at:
> +https://github.com/linux-can/can-utils

Other than the ongoing review comments, the doc LGTM (no htmldocs warnings).
Thanks!

Reviewed-by: Bagas Sanjaya <[e-mail]>

Vincent Mailhol April 15, 2024, 5:29 a.m. UTC | #6

On Mon. 15 Apr. 2024 at 05:22, Oliver Hartkopp <[e-mail]> wrote:
> On 14.04.24 06:03, Vincent Mailhol wrote:
> >
> > This doesn't remove the fact that I think that this naming convention
> > is stupid because of the RAS syndrome, but I acknowledge that CAN CC
> > is now the official denomination and thus, that we should adopt it in
> > our documentation as well.
> >
>
> ;-)
>
>
> >>> Add a space between ISO and the number. Also, update the year:
> >>>
> >>>     ISO 15765-2:2024
> >>>
> >>
> >> Interesting! Didn't know there's already a new version.
> >>
> >> Will check this out whether we really support ISO 15765-2:2024 ...
> >>
> >> Do you have the standard at hand right now or should we leave this as
> >> ISO15765-2:2016 until we know?
> >
> > I have access to the newer revisions. But I never really invested time
> > into reading that standard (neither the 2016 nor the 2024 versions).
> >
> > Regardless, here is a verbatim extract from the Foreworld section of
> > ISO 15765-2:2024
> >
> >    This fourth edition cancels and replaces the third edition (ISO
> >    15765-2:2016), which has been technically revised.
> >
> >    The main changes are as follows:
> >
> >      - restructured the document to achieve compatibility with OSI
> >        7-layers model;
> >
> >      - introduced T_Data abstract service primitive interface to
> >        achieve compatibility with ISO 14229-2;
> >
> >      - moved all transport layer protocol-related information to Clause 9;
> >
> >      - clarification and editorial corrections
> >
>
> Yes, I've checked the release notes on the ISO website too.
> This really looks like editorial stuff that has nothing to do with the
> data protocol and its segmentation.

This is also my feeling. Short story, I think it is reasonable to
quote ISO 15765-2:2024 instead of ISO 15765-2:2016 in this document.

> >>>
> >>> Here, I would suggest the C99 designated field initialization:
> >>>
> >>>     struct sockaddr_can addr = {
> >>>             .can_family = AF_CAN;
> >>>             .can_ifindex = if_nametoindex("can0");
> >>>             .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
> >>>             .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
> >>>     };
> >
> > Typo in my previous message: the designated initializers are not
> > separated by colon ";" but by comma ",". So it should have been:
> >
> >    struct sockaddr_can addr = {
> >          .can_family = AF_CAN,
> >          .can_ifindex = if_nametoindex("can0"),
> >          .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG,
> >          .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG,
> >    };
> >
> >>> Well, this is just a suggestion, feel free to reject it if you do not like it.
> >>
> >> At least I don't like it.
> >>
> >> These values are usually interactively given on the command line:
> >>
> >>   >            .can_ifindex = if_nametoindex("can0");
> >>   >            .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
> >>   >            .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
> >>
> >> So have it in a static field initialization leads to a wrong path IMO.
> >
> > There is no such limitation that C99 designated initializers should
> > only work with variables which have static storage duration. In my
> > suggested example, nothing is static.
> >
> > I see this as the same thing as below example:
> >
> >    int foo(void);
> >
> >    int bar()
> >    {
> >            int i = foo();
> >    }
> >
> >    int baz()
> >    {
> >            int i;
> >
> >            i = foo();
> >    }
> >
> > In bar(), the fact that the variable i is initialized at declaration
> > does not mean that it is static. In both examples, the variable i uses
> > automatic storage duration.
> >
> > Here, my preference goes to bar(), but I recognize that baz() is also
> > perfectly fine. Replace the int type by the struct sockaddr_can type
> > and the scalar initialization by designated initializers and you
> > should see the connection.
>
> Oh, sorry. Maybe I expressed myself wrong.
>
> IMHO your way to work with an initializer is correct from the C standpoint.
>
> But I think this is pretty unusual for a code example when an
> application programmer starts to work with ISO-TP.
>
> You usually get most of these values from the command line an fill the
> struct _by hand_ - and not with a static initialization.
>
> That was my suggestion.

OK. I get your point of view, which I think is a fair argument. So
let's keep it as is.

Just to conclude the debate, in real life, how to write it would
depend on the situation.

You may for example receive the values as function parameters:

  static int tp_bind(const char* ifname, canid_t rx_id, canid_t tx_id)
  {
          int s;
          struct sockaddr_can addr = {
                  .can_family = AF_CAN,
                  .can_ifindex = if_nametoindex(ifname),
                  .tp.rx_id = rx_id,
                  .tp.tx_id = tx_id,
          };
          int ret;

          s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
          if (s < 0)
                  exit(1);

          ret = bind(s, (struct sockaddr *)&addr, sizeof(addr));
          if (ret < 0)
                  exit(1);

          return s;
  }

Or, if you get the values from the command line, you could have an
hybrid approach:

          int s;
          struct sockaddr_can addr = {
                  .can_family = AF_CAN,
          };
          char ifname[IF_NAMESIZE];
          canid_t rx_id, tx_id;
          int ret;

          /* Parse command line and fill ifname, rx_id and tx_id */
          addr.can_ifindex = if_nametoindex(ifname);
          addr.tp.rx_id = rx_id;
          addr.tp.tx_id = tx_id;

          s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
          if (s < 0)
                  exit(1);

          ret = bind(s, (struct sockaddr *)&addr, sizeof(addr));
          if (ret < 0)
                  exit(1);

In the end, there is not enough context to decide which one is best.
And, I diverged too much. Francesco, keep the original.

> >
> > ** Different topic **
> >
> > While replying on this, I encountered something which made me worry a bit:
> >
> > The type of sockaddr_can.can_ifindex is a signed int:
> >
> >    https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/can.h#L243
> >
> > But if_nametoindex() returns an unsigned int:
> >
> >     https://man7.org/linux/man-pages/man3/if_nametoindex.3.html
> >
> > Shouldn't sockaddr_can.can_ifindex also be declared as an unsigned int?
> >
>
> The if_index derives from struct netdevice.if_index
>
> https://elixir.bootlin.com/linux/v6.8.6/source/include/linux/netdevice.h#L2158
>
> which is an int.

Ack.

> I don't think this would have an effect in real world to change
> sockaddr_can.can_ifindex to an unsigned int.
>
> I wonder if it is more critical to existing user space code to change it
> to unsigned int or to leave it as-is ...

The only impact is the warnings if compiled with the zealous flags
which warn on conversion between signed and unsigned, which would
normally be a sufficient reason to change. But, I understand that this
if_index is also a signed int at other locations in the kernel, so
let's keep it as such in our code.

> Best regards,
> Oliver

Francesco Valla April 16, 2024, 4:27 p.m. UTC | #7

Hi Vincent,

thank you for the review!

I'll omit from this reply the issue about the standard to be referenced
and the CAN-CC naming (discussed in another thread also with Oliver).

About the typos and formatting observations: rst is not my native
language (I'm more on the Markdown side), I'll apply all the corrections
you suggested. Thank you.

Some other considerations follow.

On Sat, Apr 13, 2024 at 10:11:55PM +0900, Vincent Mailhol wrote:
> Hi Francesco,
> 
> Thank you for the ISO-TP documentation.
> 
> I left a few comments, but overall, good work. Also, I did not double
> check each individual option one by one.

(...)

> > +
> > +- physical addressing is implemented by two node-specific addresses (CAN
> > +  identifiers) and is used in 1-to-1 communication
> > +- functional addressing is implemented by one node-specific address (CAN
> > +  identifier) and is used in 1-to-N communication
> > +
> > +In a so-called "normal" addressing scenario, both these addresses are
> > +represented by a 29-bit CAN ID. However, in order to support larger networks,
> > +an "extended" addressing scheme can be adopted: in this case, the first byte of
> > +the data payload is used as an additional component of the address (both for
> > +the physical and functional cases); two different CAN IDs are still required.
> 
> There is more than that.
> 
>   - The normal addressing can also use the non-extended 11 bits CAN ID.
>   - In addition to the normal and extended addressing mode, there
>     is a third mode: the mixed addressing.
> 
> Ref:
> 
>   - ISO 15765:2024 §10.3.1 "Addressing formats"
>   - https://www.embedded-communication.com/en/misc/iso-tp-addressing/
> 

You are right. I'll drop the reference to "29-bit" and add the mixed
addressing (I did not know it, I'll have to investigate a bit - I
personally always used the normal one).

(...)

> 
> > +Unlike the CAN_RAW socket API, only the data payload shall be specified in all
> > +these calls, as the CAN header is automatically filled by the ISO-TP stack
> > +using information supplied during socket creation. In the same way, the stack
> 
> This is making a shortcut. There are the raw CAN payload and the
> ISO-TP payload. In this paragraph it is not clear that "data payload"
> is referring to the ISO-TP payload.
> 
> Also, what is the meaning of "the CAN header". Here, I think you mean
> CAN ID plus some of the few first byte of the CAN payload.
> 
> I suggest that you use more precise vocabulary from the standard:
> 
>   - Address information
>   - Protocol Information
>   - Data field
> 
> Something like:
> 
>   only the ISO-TP data field (the actual payload) is sent. The
>   address information and the protocol information is
>   automatically filled by the ISO-TP stack...
>

Indeed it is a shortcut. Your suggestion to adhere more to the standard
is welcome, I'd rephrase as:

  Unlike the CAN_RAW socket API, only the ISO-TP data field (the actual payload)
  is sent and received by the userspace application using these calls. The address
  information and the protocol information are automatically filled by the ISO-TP
  stack using the configuration supplied during socket creation. In the same way,
  the stack will use the transport mechanism when required (i.e., when the size
  of the data payload exceeds the MTU of the underlying CAN bus).


(...)

> > +Examples
> > +========
> > +
> > +Basic node example
> > +------------------
> > +
> > +Following example implements a node using "normal" physical addressing, with
> > +RX ID equal to 0x18DAF142 and a TX ID equal to 0x18DA42F1. All options are left
> > +to their default.
> > +
> > +.. code-block:: C
> > +
> > +  int s;
> > +  struct sockaddr_can addr;
> 
> Here, I would suggest the C99 designated field initialization:
> 
>   struct sockaddr_can addr = {
>           .can_family = AF_CAN;
>           .can_ifindex = if_nametoindex("can0");
>           .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
>           .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
>   };
> 
> Well, this is just a suggestion, feel free to reject it if you do not like it.
> 

Not a fan of C99 designated field initialization inside functions, TBH.
Moreover, these parameters are typically specified through either the
command line or some configuration file. I'll keep my version.

> > +  int ret;
> > +
> > +  s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
> > +  if (s < 0)
> > +      exit(1);
> > +
> > +  addr.can_family = AF_CAN;
> > +  addr.can_ifindex = if_nametoindex("can0");
> 
> if_nametoindex() may fail. Because you are doing error handling in
> this example, do it also here:
> 
>   if (!addr.can_ifindex)
>           err("if_nametoindex()");
> 
> > +  addr.tp.tx_id = (0x18DA42F1 | CAN_EFF_FLAG);
> > +  addr.tp.rx_id = (0x18DAF142 | CAN_EFF_FLAG);
> 
> Nitpick: the bracket are not needed here:
> 
>   addr.tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
>   addr.tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
>

Ack.

> > +
> > +  ret = bind(s, (struct sockaddr *)&addr, sizeof(addr));
> > +  if (ret < 0)
> > +      exit(1);
> > +
> > +  // Data can now be received using read(s, ...) and sent using write(s, ...)
> 
> Kernel style prefers C style comments over C++. I think that should
> also apply to the documentation:
> 
>   /* Data can now be received using read(s, ...) and sent using write(s, ...) */
> 

Ack.


Again, thank you for the review.

Regards,
Francesco

Francesco Valla April 16, 2024, 4:42 p.m. UTC | #8

On Sun, Apr 14, 2024 at 10:21:33PM +0200, Oliver Hartkopp wrote:
> 
> 
> On 14.04.24 06:03, Vincent Mailhol wrote:
> 
> > 
> > This doesn't remove the fact that I think that this naming convention
> > is stupid because of the RAS syndrome, but I acknowledge that CAN CC
> > is now the official denomination and thus, that we should adopt it in
> > our documentation as well.
> > 
> 
> ;-)
> 
>

I honestly did not knwow the new CAN in Automation naming scheme. Will
keep the CAN-CC here. Thanks!

> > > > Add a space between ISO and the number. Also, update the year:
> > > > 
> > > >     ISO 15765-2:2024
> > > > 
> > > 
> > > Interesting! Didn't know there's already a new version.
> > > 
> > > Will check this out whether we really support ISO 15765-2:2024 ...
> > > 
> > > Do you have the standard at hand right now or should we leave this as
> > > ISO15765-2:2016 until we know?
> > 
> > I have access to the newer revisions. But I never really invested time
> > into reading that standard (neither the 2016 nor the 2024 versions).
> > 
> > Regardless, here is a verbatim extract from the Foreworld section of
> > ISO 15765-2:2024
> > 
> >    This fourth edition cancels and replaces the third edition (ISO
> >    15765-2:2016), which has been technically revised.
> > 
> >    The main changes are as follows:
> > 
> >      - restructured the document to achieve compatibility with OSI
> >        7-layers model;
> > 
> >      - introduced T_Data abstract service primitive interface to
> >        achieve compatibility with ISO 14229-2;
> > 
> >      - moved all transport layer protocol-related information to Clause 9;
> > 
> >      - clarification and editorial corrections
> > 
> 
> Yes, I've checked the release notes on the ISO website too.
> This really looks like editorial stuff that has nothing to do with the data
> protocol and its segmentation.
> 

The :2016 suffix is cited both here and inside the Kconfig. We can:
- keep the :2016 here and then update both the documentation and the
  Kconfig once the standard has been checked
- move to :2024 both here and inside the Kconfig
- drop the :2016 from everywhere (leaving only ISO 15765) and move to
  ISO 15765:2024 only inside the "Specifications used" paragraph

What do you think? Shall the modifications to the Kconfig be done as part of
this series?


Best regards,
Francesco Valla

Francesco Valla April 16, 2024, 4:46 p.m. UTC | #9

On Mon, Apr 15, 2024 at 10:09:35AM +0700, Bagas Sanjaya wrote:

(...)

> 
> Other than the ongoing review comments, the doc LGTM (no htmldocs warnings).
> Thanks!
> 
> Reviewed-by: Bagas Sanjaya <[e-mail]>
> 

Thank you!

Best regards,
Francesco Valla

Oliver Hartkopp April 16, 2024, 5:19 p.m. UTC | #10

Hi Francesco and Vincent,

On 16.04.24 18:42, Francesco Valla wrote:
> On Sun, Apr 14, 2024 at 10:21:33PM +0200, Oliver Hartkopp wrote:
>> On 14.04.24 06:03, Vincent Mailhol wrote:

>>> Regardless, here is a verbatim extract from the Foreworld section of
>>> ISO 15765-2:2024
>>>
>>>     This fourth edition cancels and replaces the third edition (ISO
>>>     15765-2:2016), which has been technically revised.
>>>
>>>     The main changes are as follows:
>>>
>>>       - restructured the document to achieve compatibility with OSI
>>>         7-layers model;
>>>
>>>       - introduced T_Data abstract service primitive interface to
>>>         achieve compatibility with ISO 14229-2;
>>>
>>>       - moved all transport layer protocol-related information to Clause 9;
>>>
>>>       - clarification and editorial corrections
>>>
>>
>> Yes, I've checked the release notes on the ISO website too.
>> This really looks like editorial stuff that has nothing to do with the data
>> protocol and its segmentation.
>>
> 
> The :2016 suffix is cited both here and inside the Kconfig. We can:
> - keep the :2016 here and then update both the documentation and the
>    Kconfig once the standard has been checked
> - move to :2024 both here and inside the Kconfig
> - drop the :2016 from everywhere (leaving only ISO 15765) and move to
>    ISO 15765:2024 only inside the "Specifications used" paragraph
> 
> What do you think? Shall the modifications to the Kconfig be done as part of
> this series?

So here is my completely new view on this version topic ... ;-D

I would vote for ISO 15765-2:2016 in all places.

The ISO 15765-2:2016 is the first ISO 15765-2 standard which supports 
CAN FD and ISO 15765-2:2024 does not bring any functional change neither 
to the standard nor to the implementation in the Linux kernel.

For that reason ISO 15765-2:2016 is still correct and relevant (due to 
the CAN FD support) and does not confuse the users whether the 2024 
version has some completely new feature or is potentially incompatible 
to the 2016 version.

Best regards,
Oliver

Vincent Mailhol April 17, 2024, 3:21 p.m. UTC | #11

On Wed. 17 Apr. 2024 at 02:19, Oliver Hartkopp <[e-mail]> wrote:
> Hi Francesco and Vincent,
>
> On 16.04.24 18:42, Francesco Valla wrote:
> > On Sun, Apr 14, 2024 at 10:21:33PM +0200, Oliver Hartkopp wrote:
> >> On 14.04.24 06:03, Vincent Mailhol wrote:
>
> >>> Regardless, here is a verbatim extract from the Foreworld section of
> >>> ISO 15765-2:2024
> >>>
> >>>     This fourth edition cancels and replaces the third edition (ISO
> >>>     15765-2:2016), which has been technically revised.
> >>>
> >>>     The main changes are as follows:
> >>>
> >>>       - restructured the document to achieve compatibility with OSI
> >>>         7-layers model;
> >>>
> >>>       - introduced T_Data abstract service primitive interface to
> >>>         achieve compatibility with ISO 14229-2;
> >>>
> >>>       - moved all transport layer protocol-related information to Clause 9;
> >>>
> >>>       - clarification and editorial corrections
> >>>
> >>
> >> Yes, I've checked the release notes on the ISO website too.
> >> This really looks like editorial stuff that has nothing to do with the data
> >> protocol and its segmentation.
> >>
> >
> > The :2016 suffix is cited both here and inside the Kconfig. We can:
> > - keep the :2016 here and then update both the documentation and the
> >    Kconfig once the standard has been checked
> > - move to :2024 both here and inside the Kconfig
> > - drop the :2016 from everywhere (leaving only ISO 15765) and move to
> >    ISO 15765:2024 only inside the "Specifications used" paragraph
> >
> > What do you think? Shall the modifications to the Kconfig be done as part of
> > this series?

If we bump the version to :2024, then I suggest to:

  - add a first patch in this series to update Kconfig.
  - add your documentation as a second patch directly with the :2024 version.

Generally speaking, when a feature requires any kind of clean-up, I
think it is better to do that clean-up first, prior to introducing the
feature.

I am also supportive of your idea to drop the year suffix in most
places and only keep it once.

> So here is my completely new view on this version topic ... ;-D
>
> I would vote for ISO 15765-2:2016 in all places.
>
> The ISO 15765-2:2016 is the first ISO 15765-2 standard which supports
> CAN FD and ISO 15765-2:2024 does not bring any functional change neither
> to the standard nor to the implementation in the Linux kernel.
>
> For that reason ISO 15765-2:2016 is still correct and relevant (due to
> the CAN FD support) and does not confuse the users whether the 2024
> version has some completely new feature or is potentially incompatible
> to the 2016 version.

ISO publications are backward compatible (if you dig enough, you may
find exceptions, but it is *extremely* uncommon that a newer revision
would break the specification from a prior publication). Without prior
knowledge, if I see ISO 15765-2:2024, I would know that it is the
latest and that I can thus expect support for both ISO 15765-2:2016
and ISO 15765-2:2024. If I see ISO 15765-2:2016, I may think that the
newer ISO 15765-2:2024 is not supported.

I can also use ISO 11898-1 as an example. Our documentation says that
we support ISO 11898-1:2015. The previous version: ISO 11898-1:2003 is
not mentioned a single time in the full kernel tree. Yet, I do not
think that any one was ever confused that the kernel may not be
compatible with ISO 11898-1:2003.

If you really think that there is a risk of confusion, then maybe just
adding a sentence to say that we support ISO 15765-2:2024 and all
previous versions would be enough?

But overall, I do not see the benefit to keep the older version.

> Best regards,
> Oliver

Vincent Mailhol April 17, 2024, 3:26 p.m. UTC | #12

On Wed. 17 Apr. 2024 at 01:27, Francesco Valla
<[e-mail]> wrote:
> Hi Vincent,
>
> thank you for the review!

You are welcome.

> I'll omit from this reply the issue about the standard to be referenced
> and the CAN-CC naming (discussed in another thread also with Oliver).
>
> About the typos and formatting observations: rst is not my native
> language (I'm more on the Markdown side), I'll apply all the corrections
> you suggested. Thank you.
>
> Some other considerations follow.

Aside from the 2024 year bump on which I replied separately, I
acknowledge all of these. Thanks.

Yours sincerely,
Vincent Mailhol

Oliver Hartkopp April 20, 2024, 7:51 p.m. UTC | #13

On 17.04.24 17:21, Vincent Mailhol wrote:

> If we bump the version to :2024, then I suggest to:
> 
>    - add a first patch in this series to update Kconfig.
>    - add your documentation as a second patch directly with the :2024 version.
> 

Ok.

> I can also use ISO 11898-1 as an example. Our documentation says that
> we support ISO 11898-1:2015. The previous version: ISO 11898-1:2003 is
> not mentioned a single time in the full kernel tree. Yet, I do not
> think that any one was ever confused that the kernel may not be
> compatible with ISO 11898-1:2003.
> 
> If you really think that there is a risk of confusion, then maybe just
> adding a sentence to say that we support ISO 15765-2:2024 and all
> previous versions would be enough?
> 
> But overall, I do not see the benefit to keep the older version.

We currently have different occurrences of the 15765-2 term:

$ git grep "15765-2"
include/uapi/linux/can.h:#define CAN_ISOTP      6 /* ISO 15765-2 
Transport Protocol */
include/uapi/linux/can/isotp.h: * Definitions for isotp CAN sockets (ISO 
15765-2:2016)
net/can/Kconfig:        tristate "ISO 15765-2:2016 CAN transport protocol"
net/can/Kconfig:          ISO 15765-2:2016 for 'classic' CAN and CAN FD 
frame types.
net/can/isotp.c:/* isotp.c - ISO 15765-2 CAN transport protocol for 
protocol family CAN
net/can/isotp.c:MODULE_DESCRIPTION("PF_CAN isotp 15765-2:2016 protocol");
net/can/isotp.c:/* ISO 15765-2:2016 supports more than 4095 byte per ISO 
PDU as the FF_DL can
net/can/isotp.c:/* maximum PDU size before ISO 15765-2:2016 extension 
was 4095 */

I've sent a patch to remove the ISO 15675-2 specification version/date 
where possible:
https://lore.kernel.org/linux-can/[e-mail]/T/#u

This also makes clear where the ISO 15765-2:2016 remains helpful IMHO.

I would be fine to remove the version/date in the documentation from 
Francesco where possible too.

Best regards,
Oliver

Francesco Valla April 21, 2024, 8:41 p.m. UTC | #14

On Sat, Apr 20, 2024 at 09:51:41PM +0200, Oliver Hartkopp wrote:
> On 17.04.24 17:21, Vincent Mailhol wrote:
> 
> > If we bump the version to :2024, then I suggest to:
> > 
> >    - add a first patch in this series to update Kconfig.
> >    - add your documentation as a second patch directly with the :2024 version.
> > 
> 
> Ok.
> 
> > I can also use ISO 11898-1 as an example. Our documentation says that
> > we support ISO 11898-1:2015. The previous version: ISO 11898-1:2003 is
> > not mentioned a single time in the full kernel tree. Yet, I do not
> > think that any one was ever confused that the kernel may not be
> > compatible with ISO 11898-1:2003.
> > 
> > If you really think that there is a risk of confusion, then maybe just
> > adding a sentence to say that we support ISO 15765-2:2024 and all
> > previous versions would be enough?
> > 
> > But overall, I do not see the benefit to keep the older version.
> 
> We currently have different occurrences of the 15765-2 term:
> 
> $ git grep "15765-2"
> include/uapi/linux/can.h:#define CAN_ISOTP      6 /* ISO 15765-2 Transport
> Protocol */
> include/uapi/linux/can/isotp.h: * Definitions for isotp CAN sockets (ISO
> 15765-2:2016)
> net/can/Kconfig:        tristate "ISO 15765-2:2016 CAN transport protocol"
> net/can/Kconfig:          ISO 15765-2:2016 for 'classic' CAN and CAN FD
> frame types.
> net/can/isotp.c:/* isotp.c - ISO 15765-2 CAN transport protocol for protocol
> family CAN
> net/can/isotp.c:MODULE_DESCRIPTION("PF_CAN isotp 15765-2:2016 protocol");
> net/can/isotp.c:/* ISO 15765-2:2016 supports more than 4095 byte per ISO PDU
> as the FF_DL can
> net/can/isotp.c:/* maximum PDU size before ISO 15765-2:2016 extension was
> 4095 */
> 
> I've sent a patch to remove the ISO 15675-2 specification version/date where
> possible:
> https://lore.kernel.org/linux-can/[e-mail]/T/#u
> 
> This also makes clear where the ISO 15765-2:2016 remains helpful IMHO.
> 
> I would be fine to remove the version/date in the documentation from
> Francesco where possible too.
>

Ok, I'll follow this path (first RFC for this patch was without dates).
I'll try to send a revised v3, also with the details on the mixed
addressing, as soon as possible.

Thank you

Regards,
Francesco

Patch

diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index 473d72c36d61..bbd9bf537793 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -19,6 +19,7 @@  Contents:
    caif/index
    ethtool-netlink
    ieee802154
+   iso15765-2
    j1939
    kapi
    msg_zerocopy
diff --git a/Documentation/networking/iso15765-2.rst b/Documentation/networking/iso15765-2.rst
new file mode 100644
index 000000000000..bbed4d2ef1a8
--- /dev/null
+++ b/Documentation/networking/iso15765-2.rst
@@ -0,0 +1,356 @@ 
+.. SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+
+=========================
+ISO 15765-2:2016 (ISO-TP)
+=========================
+
+Overview
+========
+
+ISO 15765-2:2016, also known as ISO-TP, is a transport protocol specifically
+defined for diagnostic communication on CAN. It is widely used in the automotive
+industry, for example as the transport protocol for UDSonCAN (ISO 14229-3) or
+emission-related diagnostic services (ISO 15031-5).
+
+ISO-TP can be used both on CAN CC (aka Classical CAN, CAN 2.0B) and CAN FD (CAN
+with Flexible Datarate) based networks. It is also designed to be compatible
+with a CAN network using SAE J1939 as data link layer (however, this is not a
+requirement).
+
+Specifications used
+-------------------
+
+* ISO 15765-2:2016 : Road vehicles - Diagnostic communication over Controller
+  Area Network (DoCAN). Part 2: Transport protocol and network layer services.
+
+Addressing
+----------
+
+In its simplest form, ISO-TP is based on two kinds of addressing modes for the
+nodes connected to the same network:
+
+- physical addressing is implemented by two node-specific addresses (CAN
+  identifiers) and is used in 1-to-1 communication
+- functional addressing is implemented by one node-specific address (CAN
+  identifier) and is used in 1-to-N communication
+
+In a so-called "normal" addressing scenario, both these addresses are
+represented by a 29-bit CAN ID. However, in order to support larger networks,
+an "extended" addressing scheme can be adopted: in this case, the first byte of
+the data payload is used as an additional component of the address (both for
+the physical and functional cases); two different CAN IDs are still required.
+
+Transport protocol and associated frame types
+---------------------------------------------
+
+When transmitting data using the ISO-TP protocol, the payload can either fit
+inside one single CAN message or not, also considering the overhead the protocol
+is generating and the optional extended addressing. In the first case, the data
+is transmitted at once using a so-called Single Frame (SF). In the second case,
+ISO-TP defines a multi-frame protocol, in which the sender provides (through a
+First Frame - FF) the PDU length which is to be transmitted and also asks for a
+Flow Control (FC) frame, which provides the maximum supported size of a macro
+data block (``blocksize``) and the minimum time between the single CAN messages
+composing such block (``stmin``). Once this information has been received, the
+sender starts to send frames containing fragments of the data payload (called
+Consecutive Frames - CF), stopping after every ``blocksize``-sized block to wait
+confirmation from the receiver (which should then send another Flow Control
+frame to inform the sender about its availability to receive more data).
+
+How to Use ISO-TP
+=================
+
+As with others CAN protocols, the ISO-TP stack support is built into the
+Linux network subsystem for the CAN bus, aka. Linux-CAN or SocketCAN, and
+thus follows the same socket API.
+
+Creation and basic usage of an ISO-TP socket
+--------------------------------------------
+
+To use the ISO-TP stack, ``#include <linux/can/isotp.h>`` shall be used. A
+socket can then be created using the ``PF_CAN`` protocol family, the
+``SOCK_DGRAM`` type (as the underlying protocol is datagram-based by design)
+and the ``CAN_ISOTP`` protocol:
+
+.. code-block:: C
+
+    s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
+
+After the socket has been successfully created, ``bind(2)`` shall be called to
+bind the socket to the desired CAN interface; to do so:
+
+* a TX CAN ID shall be specified as part of the sockaddr supplied to the call
+  itself, and
+* a RX CAN ID shall also specified, unless broadcast flags have been set
+  through socket option (explained below)
+
+Once bound to an interface, the socket can be read from and written to using
+the usual ``read(2)`` and ``write(2)`` system calls, as well as ``send(2)``,
+``sendmsg(2)``, ``recv(2)`` and ``recvmsg(2)``.
+Unlike the CAN_RAW socket API, only the data payload shall be specified in all
+these calls, as the CAN header is automatically filled by the ISO-TP stack
+using information supplied during socket creation. In the same way, the stack
+will use the transport mechanism when required (i.e., when the size of the data
+payload exceeds the MTU of the underlying CAN bus).
+
+The sockaddr structure used for SocketCAN has extensions for use with ISO-TP,
+as specified below:
+
+.. code-block:: C
+
+    struct sockaddr_can {
+        sa_family_t can_family;
+        int         can_ifindex;
+        union {
+            struct { canid_t rx_id, tx_id; } tp;
+        ...
+        } can_addr;
+    }
+
+* ``can_family`` and ``can_ifindex`` serve the same purpose as for other
+  SocketCAN sockets.
+
+* ``can_addr.tp.rx_id`` specifies the receive (RX) CAN ID and will be used as
+  a RX filter.
+
+* ``can_addr.tp.tx_id`` specifies the transmit (TX) CAN ID
+
+ISO-TP socket options
+---------------------
+
+When creating an ISO-TP socket, reasonable defaults are set. Some options can
+be modified with ``setsockopt(2)`` and/or read back with ``getsockopt(2)``.
+
+General options
+~~~~~~~~~~~~~~~
+
+General socket options can be passed using the ``CAN_ISOTP_OPTS`` optname:
+
+.. code-block:: C
+
+    struct can_isotp_options opts;
+    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_OPTS, &opts, sizeof(opts))
+
+where the ``can_isotp_options`` structure has the following contents:
+
+.. code-block:: C
+
+    struct can_isotp_options {
+        u32 flags;
+        u32 frame_txtime;
+        u8  ext_address;
+        u8  txpad_content;
+        u8  rxpad_content;
+        u8  rx_ext_address;
+    };
+
+* ``flags``: modifiers to be applied to the default behaviour of the ISO-TP
+  stack. Following flags are available:
+
+  - ``CAN_ISOTP_LISTEN_MODE``: listen only (do not send FC frames); normally
+    used as a testing feature.
+  - ``CAN_ISOTP_EXTEND_ADDR``: enable extended addressing, using the byte
+    specified in ``ext_address`` as additional address byte.
+  - ``CAN_ISOTP_TX_PADDING``: enable padding for tranmsitted frames, using
+    ``txpad_content`` as value for the padding bytes.
+  - ``CAN_ISOTP_RX_PADDING``: enable padding for the received frames, using
+    ``rxpad_content`` as value for the padding bytes.
+  - ``CAN_ISOTP_CHK_PAD_LEN``: check for correct padding length on the received
+    frames.
+  - ``CAN_ISOTP_CHK_PAD_DATA``: check padding bytes on the received frames
+    against ``rxpad_content``; if ``CAN_ISOTP_RX_PADDING`` is not specified,
+    this flag is ignored.
+  - ``CAN_ISOTP_HALF_DUPLEX``: force ISO-TP socket in half duples mode
+    (that is, transport mechanism can only be incoming or outgoing at the same
+    time, not both).
+  - ``CAN_ISOTP_FORCE_TXSTMIN``: ignore stmin from received FC; normally
+    used as a testing feature.
+  - ``CAN_ISOTP_FORCE_RXSTMIN``: ignore CFs depending on rx stmin; normally
+    used as a testing feature.
+  - ``CAN_ISOTP_RX_EXT_ADDR``: use ``rx_ext_address`` instead of ``ext_address``
+    as extended addressing byte on the reception path.
+  - ``CAN_ISOTP_WAIT_TX_DONE``: wait until the frame is sent before returning
+    from ``write(2)`` and ``send(2)`` calls (i.e., blocking write operations).
+  - ``CAN_ISOTP_SF_BROADCAST``: use 1-to-N functional addressing (cannot be
+    specified alongside ``CAN_ISOTP_CF_BROADCAST``).
+  - ``CAN_ISOTP_CF_BROADCAST``: use 1-to-N transmission without flow control
+    (cannot be specified alongside ``CAN_ISOTP_SF_BROADCAST``).
+    NOTE: this is not covered by the ISO15765-2:2016 standard.
+  - ``CAN_ISOTP_DYN_FC_PARMS``: enable dynamic update of flow control
+    parameters.
+
+* ``frame_txtime``: frame transmission time (defined as N_As/N_Ar inside the
+  ISO standard); if ``0``, the default (or the last set value) is used.
+  To set the transmission time to ``0``, the ``CAN_ISOTP_FRAME_TXTIME_ZERO``
+  macro (equal to 0xFFFFFFFF) shall be used.
+
+* ``ext_address``: extended addressing byte, used if the
+  ``CAN_ISOTP_EXTEND_ADDR`` flag is specified.
+
+* ``txpad_content``: byte used as padding value for transmitted frames
+
+* ``rxpad_content``: byte used as padding value for received frames
+
+* ``rx_ext_address``: extended addressing byte for the reception path, used if
+  the ``CAN_ISOTP_RX_EXT_ADDR`` flag is specified.
+
+Flow Control options
+~~~~~~~~~~~~~~~~~~~~
+
+Flow Control (FC) options can be passed using the ``CAN_ISOTP_RECV_FC`` optname
+to provide the communication parameters for receiving ISO-TP PDUs.
+
+.. code-block:: C
+
+    struct can_isotp_fc_options fc_opts;
+    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_RECV_FC, &fc_opts, sizeof(fc_opts));
+
+where the ``can_isotp_fc_options`` structure has the following contents:
+
+.. code-block:: C
+
+    struct can_isotp_options {
+        u8 bs;
+        u8 stmin;
+        u8 wftmax;
+    };
+
+* ``bs``: blocksize provided in flow control frames.
+
+* ``stmin``: minimum separation time provided in flow control frames; can
+  have the following values (others are reserved):
+  - 0x00 - 0x7F : 0 - 127 ms
+  - 0xF1 - 0xF9 : 100 us - 900 us
+
+* ``wftmax``: maximum number of wait frames provided in flow control frames.
+
+Link Layer options
+~~~~~~~~~~~~~~~~~~
+
+Link Layer (LL) options can be passed using the ``CAN_ISOTP_LL_OPTS`` optname:
+
+.. code-block:: C
+
+    struct can_isotp_ll_options ll_opts;
+    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_LL_OPTS, &ll_opts, sizeof(ll_opts));
+
+where the ``can_isotp_ll_options`` structure has the following contents:
+
+.. code-block:: C
+
+    struct can_isotp_ll_options {
+        u8 mtu;
+        u8 tx_dl;
+        u8 tx_flags;
+    };
+
+* ``mtu``: generated and accepted CAN frame type, can be equal to ``CAN_MTU``
+  for classical CAN frames or ``CANFD_MTU`` for CAN FD frames.
+
+* ``tx_dl``: maximum payload length for transmitted frames, can have one value
+  among: 8, 12, 16, 20, 24, 32, 48, 64. Values above 8 only apply to CAN FD
+  traffic (i.e.: ``mtu = CANFD_MTU``).
+
+* ``tx_flags``: flags set into ``struct canfd_frame.flags`` at frame creation.
+  Only applies to CAN FD traffic (i.e.: ``mtu = CANFD_MTU``).
+
+Transmission stmin
+~~~~~~~~~~~~~~~~~~
+
+The transmission minimum separaton time (stmin) can be forced using the
+``CAN_ISOTP_TX_STMIN`` optname and providing an stmin value in microseconds as
+a 32bit unsigned integer; this will overwrite the value sent by the receiver in
+flow control frames:
+
+.. code-block:: C
+
+    uint32_t stmin;
+    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_TX_STMIN, &stmin, sizeof(stmin));
+
+Reception stmin
+~~~~~~~~~~~~~~~
+
+The reception minimum separaton time (stmin) can be forced using the
+``CAN_ISOTP_RX_STMIN`` optname and providing an stmin value in microseconds as
+a 32bit unsigned integer; received Consecutive Frames (CF) which timestamps
+differ less than this value will be ignored:
+
+.. code-block:: C
+
+    uint32_t stmin;
+    ret = setsockopt(s, SOL_CAN_ISOTP, CAN_ISOTP_RX_STMIN, &stmin, sizeof(stmin));
+
+Multi-frame transport support
+-----------------------------
+
+The ISO-TP stack contained inside the Linux kernel supports the multi-frame
+transport mechanism defined by the standard, with the following constraints:
+
+* the maximum size of a PDU is defined by a module parameter, with an hard
+  limit imposed at build time
+* when a transmission is in progress, subsequent calls to ``write(2)`` will
+  block, while calls to ``send(2)`` will either block or fail depending on the
+  presence of the ``MSG_DONTWAIT`` flag
+* no support is present for sending "wait frames": whether a PDU can be fully
+  received or not is decided when the First Frame is received
+
+Errors
+------
+
+Following errors are reported to userspace:
+
+RX path errors
+~~~~~~~~~~~~~~
+
+============ ===============================================================
+-ETIMEDOUT   timeout of data reception
+-EILSEQ      sequence number mismatch during a multi-frame reception
+-EBADMSG     data reception with wrong padding
+============ ===============================================================
+
+TX path errors
+~~~~~~~~~~~~~~
+
+========== =================================================================
+-ECOMM     flow control reception timeout
+-EMSGSIZE  flow control reception overflow
+-EBADMSG   flow control reception with wrong layout/padding
+========== =================================================================
+
+Examples
+========
+
+Basic node example
+------------------
+
+Following example implements a node using "normal" physical addressing, with
+RX ID equal to 0x18DAF142 and a TX ID equal to 0x18DA42F1. All options are left
+to their default.
+
+.. code-block:: C
+
+  int s;
+  struct sockaddr_can addr;
+  int ret;
+
+  s = socket(PF_CAN, SOCK_DGRAM, CAN_ISOTP);
+  if (s < 0)
+      exit(1);
+
+  addr.can_family = AF_CAN;
+  addr.can_ifindex = if_nametoindex("can0");
+  addr.tp.tx_id = (0x18DA42F1 | CAN_EFF_FLAG);
+  addr.tp.rx_id = (0x18DAF142 | CAN_EFF_FLAG);
+
+  ret = bind(s, (struct sockaddr *)&addr, sizeof(addr));
+  if (ret < 0)
+      exit(1);
+
+  // Data can now be received using read(s, ...) and sent using write(s, ...)
+
+Additional examples
+-------------------
+
+More complete (and complex) examples can be found inside the ``isotp*`` userland
+tools, distributed as part of the ``can-utils`` utilities at:
+https://github.com/linux-can/can-utils
diff --git a/MAINTAINERS b/MAINTAINERS
index 6a233e1a3cf2..e0190b90d1a8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4695,6 +4695,7 @@  W:	https://github.com/linux-can
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
 F:	Documentation/networking/can.rst
+F:	Documentation/networking/iso15765-2.rst
 F:	include/linux/can/can-ml.h
 F:	include/linux/can/core.h
 F:	include/linux/can/skb.h