linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] docs: usbip: Fix major fields and descriptions in protocol
@ 2021-03-15  1:57 Hongren Zheng (Zenithal)
  2021-03-15  6:18 ` [PATCH v2] " Hongren Zheng (Zenithal)
  0 siblings, 1 reply; 14+ messages in thread
From: Hongren Zheng (Zenithal) @ 2021-03-15  1:57 UTC (permalink / raw)
  To: Valentina Manea, Shuah Khan, Greg Kroah-Hartman, Jonathan Corbet,
	linux-usb, linux-doc, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 29609 bytes --]

The old document for usbip protocol is misleading and hard to read:
  * Some fields in header are incorrect
  * Explanation of some fields are unclear or even wrong
  * Padding of header (namely all headers have the same length) is
    not explicitly point out, which is crucial for stream protocol like
    TCP

These fixes are made through reading usbip kernel drivers and userland
codes. Also I have implemented one usbip server.

Major changes:
  * Document the correct field as described in the codebase.
  * Document the padding in usbip headers. This is crucial for TCP
    stream hence these padding should be explicitly point out.
    In code these padding are implemented by a union of all headers.
  * Fix two FIXME related to usbip unlink and Document the behavior
    of unlink in different situation.
  * Clarify some field with more accurate explanation, like those
    fields associated with URB. Some constraints are extracted from
    code.
  * Delete specific transfer_flag doc in usbip as it should be
    documented by the URB part.
  * Add data captured from wire as example

Signed-off-by: Hongren Zheng (Zenithal) <i@zenithal.me>
---
 Documentation/usb/usbip_protocol.rst | 284 +++++++++++++++------------
 1 file changed, 156 insertions(+), 128 deletions(-)

diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
index 988c832166cd..4c5bcec4a2e4 100644
--- a/Documentation/usb/usbip_protocol.rst
+++ b/Documentation/usb/usbip_protocol.rst
@@ -5,8 +5,14 @@ USB/IP protocol
 PRELIMINARY DRAFT, MAY CONTAIN MISTAKES!
 28 Jun 2011
 
+Update: Fix major fields in protocol
+14 Mar 2021
+
+Architecture
+============
+
 The USB/IP protocol follows a server/client architecture. The server exports the
-USB devices and the clients imports them. The device driver for the exported
+USB devices and the client imports them. The device driver for the exported
 USB device runs on the client machine.
 
 The client may ask for the list of the exported USB devices. To get the list the
@@ -37,6 +43,9 @@ to transfer the URB traffic between the client and the server. The client may
 send two types of packets: the USBIP_CMD_SUBMIT to submit an URB, and
 USBIP_CMD_UNLINK to unlink a previously submitted URB. The answers of the
 server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
+Note that after successful USBIP_RET_UNLINK, the unlinked URB request would not
+have a corresponding USBIP_RET_UNLINK (this is explained in
+drivers/usb/usbip/stub_rx.c).
 
 ::
 
@@ -85,16 +94,42 @@ server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
           |                        .                        |
           |                        :                        |
           |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = p)         |
+          | ----------------------------------------------> |
+          |                                                 |
           |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = p+1, unlink_seqnum = p)       |
           | ----------------------------------------------> |
           |                                                 |
           |               USBIP_RET_UNLINK                  |
+          |        (seqnum = p+1, status = -ECONNRESET)     |
+          | <---------------------------------------------- |
+          |                                                 |
+          |         Note: No USBIP_RET_SUBMIT(seqnum = p)   |
+          | <--X---X---X---X---X---X---X---X---X---X---X--- |
+          |                        .                        |
+          |                        :                        |
+          |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = q)         |
+          | ----------------------------------------------> |
+          |                                                 |
+          |            USBIP_RET_SUBMIT(seqnum = q)         |
+          | <---------------------------------------------- |
+          |                                                 |
+          |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = q+1, unlink_seqnum = q)       |
+          | ----------------------------------------------> |
+          |                                                 |
+          |               USBIP_RET_UNLINK                  |
+          |           (seqnum = q+1, status = 0)            |
           | <---------------------------------------------- |
           |                                                 |
 
 The fields are in network (big endian) byte order meaning that the most significant
 byte (MSB) is stored at the lowest address.
 
+Message Format
+==============
 
 OP_REQ_DEVLIST:
 	Retrieve the list of exported USB devices.
@@ -102,7 +137,7 @@ OP_REQ_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8005     | Command code: Retrieve the list of exported USB   |
 |           |        |            | devices.                                          |
@@ -116,7 +151,7 @@ OP_REP_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0.|
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0005     | Reply code: The list of exported USB devices.     |
 +-----------+--------+------------+---------------------------------------------------+
@@ -187,7 +222,7 @@ OP_REQ_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8003     | Command code: import a remote USB device.         |
 +-----------+--------+------------+---------------------------------------------------+
@@ -206,7 +241,7 @@ OP_REP_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0003     | Reply code: Reply to import.                      |
 +-----------+--------+------------+---------------------------------------------------+
@@ -254,65 +289,75 @@ OP_REP_IMPORT:
 | 0x13E     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 
-USBIP_CMD_SUBMIT:
-	Submit an URB
+The following four commands have a common basic header called
+'usbip_header_basic', and their headers, called 'usbip_header' (before URB
+payload), have the same length, therefore paddings are needed.
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000001 | command: Submit an URB                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the sequence number of the URB to submit  |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number, possible values are: 0...15  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | transfer_flags: possible values depend on the     |
-|           |        |            | URB transfer type, see below                      |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x18      | 4      |            | transfer_buffer_length                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: specify the selected frame to        |
-|           |        |            | transmit an ISO frame, ignored if URB_ISO_ASAP    |
-|           |        |            | is specified at transfer_flags                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x24      | 4      |            | interval: maximum time for the request on the     |
-|           |        |            | server-side host controller                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      |        |            | URB data. For ISO transfers the padding between   |
-|           |        |            | each ISO packets is not transmitted.              |
-+-----------+--------+------------+---------------------------------------------------+
+usbip_header_basic:
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 4      | command                                           |
++-----------+--------+---------------------------------------------------+
+| 4         | 4      | seqnum: sequential number that identifies requests|
+|           |        | and corresponding responses;                      |
+|           |        | incremented per connection                        |
++-----------+--------+---------------------------------------------------+
+| 8         | 4      | devid: specifies a remote USB device uniquely     |
+|           |        | instead of busnum and devnum;                     |
+|           |        | for client (request), this value is               |
+|           |        | ((busnum << 16) | devnum);                        |
+|           |        | for server (response), this shall be set to 0     |
++-----------+--------+---------------------------------------------------+
+| 0xC       | 4      | direction:                                        |
+|           |        |                                                   |
+|           |        |    - 0: USBIP_DIR_OUT                             |
+|           |        |    - 1: USBIP_DIR_IN                              |
+|           |        |                                                   |
+|           |        | only used by client, for server this shall be 0   |
++-----------+--------+---------------------------------------------------+
+| 0x10      | 4      | ep: endpoint number                               |
+|           |        | only used by client, for server this shall be 0   |
+|           |        | for UNLINK, this shall be 0                       |
++-----------+--------+---------------------------------------------------+
 
- +-------------------------+------------+---------+-----------+----------+-------------+
- | Allowed transfer_flags  | value      | control | interrupt | bulk     | isochronous |
- +=========================+============+=========+===========+==========+=============+
- | URB_SHORT_NOT_OK        | 0x00000001 | only in | only in   | only in  | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ISO_ASAP            | 0x00000002 | no      | no        | no       | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ZERO_PACKET         | 0x00000040 | no      | no        | only out | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_INTERRUPT        | 0x00000080 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_FREE_BUFFER         | 0x00000100 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_DIR_MASK            | 0x00000200 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
+USBIP_CMD_SUBMIT:
+	Submit an URB
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000001 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | transfer_flags: possible values depend on the     |
+|           |        | URB transfer_flags,                               |
+|           |        | but with URB_NO_TRANSFER_DMA_MAP masked           |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 4      | transfer_buffer_length:                           |
+|           |        | use URB transfer_buffer_length                    |
++-----------+--------+---------------------------------------------------+
+| 0x1C      | 4      | start_frame: use URB start_frame;                 |
+|           |        | initial frame for ISO transfer                    |
+|           |        | shall be set to 0 if not ISO transfer             |
++-----------+--------+---------------------------------------------------+
+| 0x20      | 4      | number_of_packets: number of ISO packets          |
+|           |        | shall be set to 0xffffffff if not ISO transfer    |
++-----------+--------+---------------------------------------------------+
+| 0x24      | 4      | interval: maximum time for the request on the     |
+|           |        | server-side host controller                       |
++-----------+--------+---------------------------------------------------+
+| 0x28      | 8      | setup: data bytes for USB setup, filled with      |
+|           |        | zeros if not used                                 |
++-----------+--------+---------------------------------------------------+
+| 0x30      | n      | tranfer_buffer.                                   |
+|           |        | If direction is USBIP_DIR_OUT then n equals       |
+|           |        | transfer_buffer_length; otherwise n equals 0      |
+|           |        | For ISO transfers the padding between each ISO    |
+|           |        | between each ISO packets is not transmitted.      |
++-----------+--------+---------------------------------------------------+
+| 0x30+n    | m      | iso_packet_descriptor                             |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_SUBMIT:
 	Reply for submitting an URB
@@ -320,92 +365,75 @@ USBIP_RET_SUBMIT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 4      | 0x00000003 | command                                           |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
+| 0         | 20     |            | usbip_header_basic, 'command' shall be 0x00000003 |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x14      | 4      |            | status: zero for successful URB transaction,      |
 |           |        |            | otherwise some kind of error happened.            |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x18      | 4      | n          | actual_length: number of URB data bytes           |
+|           |        |            | use URB actual_length                             |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: for an ISO frame the actually        |
-|           |        |            | selected frame for transmit.                      |
+| 0x1C      | 4      |            | start_frame: use URB start_frame;                 |
+|           |        |            | initial frame for ISO transfer                    |
+|           |        |            | shall be set to 0 if not ISO transfer             |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets                                 |
+| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
+|           |        |            | shall be set to 0xffffffff if not ISO transfer    |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x24      | 4      |            | error_count                                       |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
+| 0x28      | 8      |            | padding, shall be set to 0                        |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
+| 0x30      | n      |            | tranfer_buffer.                                   |
+|           |        |            | If direction is USBIP_DIR_OUT then n equals       |
+|           |        |            | transfer_buffer_length; otherwise n equals 0      |
+|           |        |            | For ISO transfers the padding between each ISO    |
 |           |        |            | between each ISO packets is not transmitted.      |
 +-----------+--------+------------+---------------------------------------------------+
+| 0x30+n    | m      |            | iso_packet_descriptor                             |
++-----------+--------+------------+---------------------------------------------------+
 
 USBIP_CMD_UNLINK:
 	Unlink an URB
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000002 | command: URB unlink command                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number to unlink:            |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |    is this so?                                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number: zero                         |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | seqnum: the URB sequence number given previously  |
-|           |        |            | at USBIP_CMD_SUBMIT.seqnum field                  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000002 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | unlink_seqnum, of the SUBMIT request to unlink    |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_UNLINK:
 	Reply for URB unlink
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000004 | command: reply for the URB unlink command         |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the unlinked URB sequence number          |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | status: This is the value contained in the        |
-|           |        |            | urb->status in the URB completition handler.      |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |      a better explanation needed.                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000004 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | status: This is similar to that status of         |
+|           |        | USBIP_RET_SUBMIT (share the same memory offset)   |
+|           |        | When UNLINK is successful, status is -ECONNRESET; |
+|           |        | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT   |
+|           |        | status is 0                                       |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
+
+EXAMPLE
+=======
+
+  The following data is captured from wire with Human Interface Devices (HID)
+  payload
+
+::
+
+  CmdIntrIN:  00000001 00000d05 0001000f 00000001 00000001 00000200 00000040 ffffffff 00000000 00000004 00000000 00000000
+  CmdIntrOUT: 00000001 00000d06 0001000f 00000000 00000001 00000000 00000040 ffffffff 00000000 00000004 00000000 00000000
+              ffffffff860008a784ce5ae212376300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+  RetIntrOut: 00000003 00000d06 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+  RetIntrIn:  00000003 00000d05 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+              ffffffff860011a784ce5ae2123763612891b1020100000400000000000000000000000000000000000000000000000000000000000000000000000000000000
-- 
2.30.1


-- 
GPG Fingerprint: 1127F188280AE3123619332987E17EEF9B18B6C9

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* [PATCH v2] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-15  1:57 [PATCH] docs: usbip: Fix major fields and descriptions in protocol Hongren Zheng (Zenithal)
@ 2021-03-15  6:18 ` Hongren Zheng (Zenithal)
  2021-03-15  7:36   ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Hongren Zheng (Zenithal) @ 2021-03-15  6:18 UTC (permalink / raw)
  To: Valentina Manea, Shuah Khan, Greg Kroah-Hartman, Jonathan Corbet,
	linux-doc
  Cc: Alexandre Demers, linux-usb, linux-kernel, usbip-devel

The old document for usbip protocol is misleading and hard to read:
  * Some fields in header are incorrect
  * Explanation of some fields are unclear or even wrong
  * Padding of header (namely all headers have the same length) is
    not explicitly point out, which is crucial for stream protocol like
    TCP

These fixes are made through reading usbip kernel drivers and userland
codes. Also I have implemented one usbip server.

Major changes:
  * Document the correct field as described in the codebase.
  * Document the padding in usbip headers. This is crucial for TCP
    stream hence these padding should be explicitly point out.
    In code these padding are implemented by a union of all headers.
  * Fix two FIXME related to usbip unlink and Document the behavior
    of unlink in different situation.
  * Clarify some field with more accurate explanation, like those
    fields associated with URB. Some constraints are extracted from
    code.
  * Delete specific transfer_flag doc in usbip as it should be
    documented by the URB part.
  * Add data captured from wire as example

Also some changes suggested by a previous patch in
https://lore.kernel.org/linux-usb/20180128071514.9107-1-alexandre.f.demers@gmail.com/
is adopted in this patch.

Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
Signed-off-by: Hongren Zheng (Zenithal) <i@zenithal.me>
---
 Documentation/usb/usbip_protocol.rst | 290 +++++++++++++++------------
 1 file changed, 159 insertions(+), 131 deletions(-)

diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
index 988c832166cd..a15d9c1254e2 100644
--- a/Documentation/usb/usbip_protocol.rst
+++ b/Documentation/usb/usbip_protocol.rst
@@ -5,8 +5,14 @@ USB/IP protocol
 PRELIMINARY DRAFT, MAY CONTAIN MISTAKES!
 28 Jun 2011
 
+Update: Fix major fields in protocol
+14 Mar 2021
+
+Architecture
+============
+
 The USB/IP protocol follows a server/client architecture. The server exports the
-USB devices and the clients imports them. The device driver for the exported
+USB devices and the client imports them. The device driver for the exported
 USB device runs on the client machine.
 
 The client may ask for the list of the exported USB devices. To get the list the
@@ -37,6 +43,9 @@ to transfer the URB traffic between the client and the server. The client may
 send two types of packets: the USBIP_CMD_SUBMIT to submit an URB, and
 USBIP_CMD_UNLINK to unlink a previously submitted URB. The answers of the
 server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
+Note that after successful USBIP_RET_UNLINK, the unlinked URB request would not
+have a corresponding USBIP_RET_UNLINK (this is explained in
+drivers/usb/usbip/stub_rx.c).
 
 ::
 
@@ -85,16 +94,42 @@ server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
           |                        .                        |
           |                        :                        |
           |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = p)         |
+          | ----------------------------------------------> |
+          |                                                 |
           |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = p+1, unlink_seqnum = p)       |
           | ----------------------------------------------> |
           |                                                 |
           |               USBIP_RET_UNLINK                  |
+          |        (seqnum = p+1, status = -ECONNRESET)     |
+          | <---------------------------------------------- |
+          |                                                 |
+          |         Note: No USBIP_RET_SUBMIT(seqnum = p)   |
+          | <--X---X---X---X---X---X---X---X---X---X---X--- |
+          |                        .                        |
+          |                        :                        |
+          |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = q)         |
+          | ----------------------------------------------> |
+          |                                                 |
+          |            USBIP_RET_SUBMIT(seqnum = q)         |
+          | <---------------------------------------------- |
+          |                                                 |
+          |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = q+1, unlink_seqnum = q)       |
+          | ----------------------------------------------> |
+          |                                                 |
+          |               USBIP_RET_UNLINK                  |
+          |           (seqnum = q+1, status = 0)            |
           | <---------------------------------------------- |
           |                                                 |
 
 The fields are in network (big endian) byte order meaning that the most significant
 byte (MSB) is stored at the lowest address.
 
+Message Format
+==============
 
 OP_REQ_DEVLIST:
 	Retrieve the list of exported USB devices.
@@ -102,7 +137,7 @@ OP_REQ_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8005     | Command code: Retrieve the list of exported USB   |
 |           |        |            | devices.                                          |
@@ -116,7 +151,7 @@ OP_REP_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0.|
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0005     | Reply code: The list of exported USB devices.     |
 +-----------+--------+------------+---------------------------------------------------+
@@ -165,8 +200,8 @@ OP_REP_DEVLIST:
 | 0x143     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x144     |        | m_0        | From now on each interface is described, all      |
-|           |        |            | together bNumInterfaces times, with the           |
-|           |        |            | the following 4 fields:                           |
+|           |        |            | together bNumInterfaces times, with the following |
+|           |        |            | 4 fields:                                         |
 +-----------+--------+------------+---------------------------------------------------+
 |           | 1      |            | bInterfaceClass                                   |
 +-----------+--------+------------+---------------------------------------------------+
@@ -177,7 +212,7 @@ OP_REP_DEVLIST:
 | 0x147     | 1      |            | padding byte for alignment, shall be set to zero  |
 +-----------+--------+------------+---------------------------------------------------+
 | 0xC +     |        |            | The second exported USB device starts at i=1      |
-| i*0x138 + |        |            | with the busid field.                             |
+| i*0x138 + |        |            | with the path field.                              |
 | m_(i-1)*4 |        |            |                                                   |
 +-----------+--------+------------+---------------------------------------------------+
 
@@ -187,7 +222,7 @@ OP_REQ_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8003     | Command code: import a remote USB device.         |
 +-----------+--------+------------+---------------------------------------------------+
@@ -206,7 +241,7 @@ OP_REP_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0003     | Reply code: Reply to import.                      |
 +-----------+--------+------------+---------------------------------------------------+
@@ -254,65 +289,75 @@ OP_REP_IMPORT:
 | 0x13E     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 
-USBIP_CMD_SUBMIT:
-	Submit an URB
+The following four commands have a common basic header called
+'usbip_header_basic', and their headers, called 'usbip_header' (before URB
+payload), have the same length, therefore paddings are needed.
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000001 | command: Submit an URB                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the sequence number of the URB to submit  |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number, possible values are: 0...15  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | transfer_flags: possible values depend on the     |
-|           |        |            | URB transfer type, see below                      |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x18      | 4      |            | transfer_buffer_length                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: specify the selected frame to        |
-|           |        |            | transmit an ISO frame, ignored if URB_ISO_ASAP    |
-|           |        |            | is specified at transfer_flags                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x24      | 4      |            | interval: maximum time for the request on the     |
-|           |        |            | server-side host controller                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      |        |            | URB data. For ISO transfers the padding between   |
-|           |        |            | each ISO packets is not transmitted.              |
-+-----------+--------+------------+---------------------------------------------------+
+usbip_header_basic:
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 4      | command                                           |
++-----------+--------+---------------------------------------------------+
+| 4         | 4      | seqnum: sequential number that identifies requests|
+|           |        | and corresponding responses;                      |
+|           |        | incremented per connection                        |
++-----------+--------+---------------------------------------------------+
+| 8         | 4      | devid: specifies a remote USB device uniquely     |
+|           |        | instead of busnum and devnum;                     |
+|           |        | for client (request), this value is               |
+|           |        | ((busnum << 16) | devnum);                        |
+|           |        | for server (response), this shall be set to 0     |
++-----------+--------+---------------------------------------------------+
+| 0xC       | 4      | direction:                                        |
+|           |        |                                                   |
+|           |        |    - 0: USBIP_DIR_OUT                             |
+|           |        |    - 1: USBIP_DIR_IN                              |
+|           |        |                                                   |
+|           |        | only used by client, for server this shall be 0   |
++-----------+--------+---------------------------------------------------+
+| 0x10      | 4      | ep: endpoint number                               |
+|           |        | only used by client, for server this shall be 0   |
+|           |        | for UNLINK, this shall be 0                       |
++-----------+--------+---------------------------------------------------+
 
- +-------------------------+------------+---------+-----------+----------+-------------+
- | Allowed transfer_flags  | value      | control | interrupt | bulk     | isochronous |
- +=========================+============+=========+===========+==========+=============+
- | URB_SHORT_NOT_OK        | 0x00000001 | only in | only in   | only in  | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ISO_ASAP            | 0x00000002 | no      | no        | no       | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ZERO_PACKET         | 0x00000040 | no      | no        | only out | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_INTERRUPT        | 0x00000080 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_FREE_BUFFER         | 0x00000100 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_DIR_MASK            | 0x00000200 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
+USBIP_CMD_SUBMIT:
+	Submit an URB
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000001 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | transfer_flags: possible values depend on the     |
+|           |        | URB transfer_flags,                               |
+|           |        | but with URB_NO_TRANSFER_DMA_MAP masked           |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 4      | transfer_buffer_length:                           |
+|           |        | use URB transfer_buffer_length                    |
++-----------+--------+---------------------------------------------------+
+| 0x1C      | 4      | start_frame: use URB start_frame;                 |
+|           |        | initial frame for ISO transfer                    |
+|           |        | shall be set to 0 if not ISO transfer             |
++-----------+--------+---------------------------------------------------+
+| 0x20      | 4      | number_of_packets: number of ISO packets          |
+|           |        | shall be set to 0xffffffff if not ISO transfer    |
++-----------+--------+---------------------------------------------------+
+| 0x24      | 4      | interval: maximum time for the request on the     |
+|           |        | server-side host controller                       |
++-----------+--------+---------------------------------------------------+
+| 0x28      | 8      | setup: data bytes for USB setup, filled with      |
+|           |        | zeros if not used                                 |
++-----------+--------+---------------------------------------------------+
+| 0x30      | n      | tranfer_buffer.                                   |
+|           |        | If direction is USBIP_DIR_OUT then n equals       |
+|           |        | transfer_buffer_length; otherwise n equals 0      |
+|           |        | For ISO transfers the padding between each ISO    |
+|           |        | between each ISO packets is not transmitted.      |
++-----------+--------+---------------------------------------------------+
+| 0x30+n    | m      | iso_packet_descriptor                             |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_SUBMIT:
 	Reply for submitting an URB
@@ -320,92 +365,75 @@ USBIP_RET_SUBMIT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 4      | 0x00000003 | command                                           |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
+| 0         | 20     |            | usbip_header_basic, 'command' shall be 0x00000003 |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x14      | 4      |            | status: zero for successful URB transaction,      |
 |           |        |            | otherwise some kind of error happened.            |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x18      | 4      | n          | actual_length: number of URB data bytes           |
+|           |        |            | use URB actual_length                             |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: for an ISO frame the actually        |
-|           |        |            | selected frame for transmit.                      |
+| 0x1C      | 4      |            | start_frame: use URB start_frame;                 |
+|           |        |            | initial frame for ISO transfer                    |
+|           |        |            | shall be set to 0 if not ISO transfer             |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets                                 |
+| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
+|           |        |            | shall be set to 0xffffffff if not ISO transfer    |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x24      | 4      |            | error_count                                       |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
+| 0x28      | 8      |            | padding, shall be set to 0                        |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
+| 0x30      | n      |            | tranfer_buffer.                                   |
+|           |        |            | If direction is USBIP_DIR_OUT then n equals       |
+|           |        |            | transfer_buffer_length; otherwise n equals 0      |
+|           |        |            | For ISO transfers the padding between each ISO    |
 |           |        |            | between each ISO packets is not transmitted.      |
 +-----------+--------+------------+---------------------------------------------------+
+| 0x30+n    | m      |            | iso_packet_descriptor                             |
++-----------+--------+------------+---------------------------------------------------+
 
 USBIP_CMD_UNLINK:
 	Unlink an URB
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000002 | command: URB unlink command                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number to unlink:            |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |    is this so?                                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number: zero                         |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | seqnum: the URB sequence number given previously  |
-|           |        |            | at USBIP_CMD_SUBMIT.seqnum field                  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000002 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | unlink_seqnum, of the SUBMIT request to unlink    |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_UNLINK:
 	Reply for URB unlink
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000004 | command: reply for the URB unlink command         |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the unlinked URB sequence number          |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | status: This is the value contained in the        |
-|           |        |            | urb->status in the URB completition handler.      |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |      a better explanation needed.                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000004 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | status: This is similar to that status of         |
+|           |        | USBIP_RET_SUBMIT (share the same memory offset)   |
+|           |        | When UNLINK is successful, status is -ECONNRESET; |
+|           |        | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT   |
+|           |        | status is 0                                       |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
+
+EXAMPLE
+=======
+
+  The following data is captured from wire with Human Interface Devices (HID)
+  payload
+
+::
+
+  CmdIntrIN:  00000001 00000d05 0001000f 00000001 00000001 00000200 00000040 ffffffff 00000000 00000004 00000000 00000000
+  CmdIntrOUT: 00000001 00000d06 0001000f 00000000 00000001 00000000 00000040 ffffffff 00000000 00000004 00000000 00000000
+              ffffffff860008a784ce5ae212376300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+  RetIntrOut: 00000003 00000d06 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+  RetIntrIn:  00000003 00000d05 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+              ffffffff860011a784ce5ae2123763612891b1020100000400000000000000000000000000000000000000000000000000000000000000000000000000000000
-- 
2.30.1


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

* Re: [PATCH v2] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-15  6:18 ` [PATCH v2] " Hongren Zheng (Zenithal)
@ 2021-03-15  7:36   ` Greg Kroah-Hartman
  2021-03-15  8:40     ` [PATCH v3] " Hongren Zheng (Zenithal)
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-03-15  7:36 UTC (permalink / raw)
  To: Hongren Zheng (Zenithal)
  Cc: Valentina Manea, Shuah Khan, Jonathan Corbet, linux-doc,
	Alexandre Demers, linux-usb, linux-kernel, usbip-devel

On Mon, Mar 15, 2021 at 02:18:49PM +0800, Hongren Zheng (Zenithal) wrote:
> The old document for usbip protocol is misleading and hard to read:
>   * Some fields in header are incorrect
>   * Explanation of some fields are unclear or even wrong
>   * Padding of header (namely all headers have the same length) is
>     not explicitly point out, which is crucial for stream protocol like
>     TCP
> 
> These fixes are made through reading usbip kernel drivers and userland
> codes. Also I have implemented one usbip server.
> 
> Major changes:
>   * Document the correct field as described in the codebase.
>   * Document the padding in usbip headers. This is crucial for TCP
>     stream hence these padding should be explicitly point out.
>     In code these padding are implemented by a union of all headers.
>   * Fix two FIXME related to usbip unlink and Document the behavior
>     of unlink in different situation.
>   * Clarify some field with more accurate explanation, like those
>     fields associated with URB. Some constraints are extracted from
>     code.
>   * Delete specific transfer_flag doc in usbip as it should be
>     documented by the URB part.
>   * Add data captured from wire as example
> 
> Also some changes suggested by a previous patch in
> https://lore.kernel.org/linux-usb/20180128071514.9107-1-alexandre.f.demers@gmail.com/
> is adopted in this patch.
> 
> Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> Signed-off-by: Hongren Zheng (Zenithal) <i@zenithal.me>
> ---
>  Documentation/usb/usbip_protocol.rst | 290 +++++++++++++++------------
>  1 file changed, 159 insertions(+), 131 deletions(-)

What changed from v1?  Always list that here below the --- line.

> 
> diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
> index 988c832166cd..a15d9c1254e2 100644
> --- a/Documentation/usb/usbip_protocol.rst
> +++ b/Documentation/usb/usbip_protocol.rst
> @@ -5,8 +5,14 @@ USB/IP protocol
>  PRELIMINARY DRAFT, MAY CONTAIN MISTAKES!
>  28 Jun 2011
>  
> +Update: Fix major fields in protocol
> +14 Mar 2021

This does not belong here, the git changelog shows this information.

The original date above can be removed as well if you want.  And if the
mistakes are all fixed now, that line can be dropped too :)

thanks,

greg k-h

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

* [PATCH v3] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-15  7:36   ` Greg Kroah-Hartman
@ 2021-03-15  8:40     ` Hongren Zheng (Zenithal)
  2021-03-15 18:25       ` Randy Dunlap
  2021-03-15 21:15       ` [PATCH v4] " Hongren Zheng (Zenithal)
  0 siblings, 2 replies; 14+ messages in thread
From: Hongren Zheng (Zenithal) @ 2021-03-15  8:40 UTC (permalink / raw)
  To: Valentina Manea, Shuah Khan, Greg Kroah-Hartman,
	Márton Németh, Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, linux-kernel, usbip-devel

The old document for usbip protocol is misleading and hard to read:
  * Some fields in header are incorrect
  * Explanation of some fields are unclear or even wrong
  * Padding of header (namely all headers have the same length) is
    not explicitly point out, which is crucial for stream protocol like
    TCP

These fixes are made through reading usbip kernel drivers and userland
codes. Also I have implemented one usbip server.

Major changes:
  * Document the correct field as described in the codebase.
  * Document the padding in usbip headers. This is crucial for TCP
    stream hence these padding should be explicitly point out.
    In code these padding are implemented by a union of all headers.
  * Fix two FIXME related to usbip unlink and Document the behavior
    of unlink in different situation.
  * Clarify some field with more accurate explanation, like those
    fields associated with URB. Some constraints are extracted from
    code.
  * Delete specific transfer_flag doc in usbip as it should be
    documented by the URB part.
  * Add data captured from wire as example

PATCH v2:
Some changes suggested by a previous patch in
https://lore.kernel.org/linux-usb
/20180128071514.9107-1-alexandre.f.demers@gmail.com/
is adopted in this patch.
  * Fix Typo: duplicated 'the' in 'the following 4 field'
  * Fix incorrect field: in OP_REQ_DEVLIST, the second dev starts with
    field 'path', not 'busid'

PATCH v3:
Suggested by
https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
  * Remove date in doc as these are tracked in git history
  * Remove 'mistake alert' as all data fields are documented properly
    now. However, docs on possible values for some field shall be added
    in the future

Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
Signed-off-by: Hongren Zheng (Zenithal) <i@zenithal.me>
---
 Documentation/usb/usbip_protocol.rst | 288 ++++++++++++++-------------
 1 file changed, 155 insertions(+), 133 deletions(-)

diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
index 988c832166cd..ed423cdda236 100644
--- a/Documentation/usb/usbip_protocol.rst
+++ b/Documentation/usb/usbip_protocol.rst
@@ -2,11 +2,11 @@
 USB/IP protocol
 ===============
 
-PRELIMINARY DRAFT, MAY CONTAIN MISTAKES!
-28 Jun 2011
+Architecture
+============
 
 The USB/IP protocol follows a server/client architecture. The server exports the
-USB devices and the clients imports them. The device driver for the exported
+USB devices and the client imports them. The device driver for the exported
 USB device runs on the client machine.
 
 The client may ask for the list of the exported USB devices. To get the list the
@@ -37,6 +37,9 @@ to transfer the URB traffic between the client and the server. The client may
 send two types of packets: the USBIP_CMD_SUBMIT to submit an URB, and
 USBIP_CMD_UNLINK to unlink a previously submitted URB. The answers of the
 server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
+Note that after successful USBIP_RET_UNLINK, the unlinked URB request would not
+have a corresponding USBIP_RET_UNLINK (this is explained in
+drivers/usb/usbip/stub_rx.c).
 
 ::
 
@@ -85,16 +88,42 @@ server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
           |                        .                        |
           |                        :                        |
           |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = p)         |
+          | ----------------------------------------------> |
+          |                                                 |
           |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = p+1, unlink_seqnum = p)       |
           | ----------------------------------------------> |
           |                                                 |
           |               USBIP_RET_UNLINK                  |
+          |        (seqnum = p+1, status = -ECONNRESET)     |
+          | <---------------------------------------------- |
+          |                                                 |
+          |         Note: No USBIP_RET_SUBMIT(seqnum = p)   |
+          | <--X---X---X---X---X---X---X---X---X---X---X--- |
+          |                        .                        |
+          |                        :                        |
+          |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = q)         |
+          | ----------------------------------------------> |
+          |                                                 |
+          |            USBIP_RET_SUBMIT(seqnum = q)         |
+          | <---------------------------------------------- |
+          |                                                 |
+          |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = q+1, unlink_seqnum = q)       |
+          | ----------------------------------------------> |
+          |                                                 |
+          |               USBIP_RET_UNLINK                  |
+          |           (seqnum = q+1, status = 0)            |
           | <---------------------------------------------- |
           |                                                 |
 
 The fields are in network (big endian) byte order meaning that the most significant
 byte (MSB) is stored at the lowest address.
 
+Message Format
+==============
 
 OP_REQ_DEVLIST:
 	Retrieve the list of exported USB devices.
@@ -102,7 +131,7 @@ OP_REQ_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8005     | Command code: Retrieve the list of exported USB   |
 |           |        |            | devices.                                          |
@@ -116,7 +145,7 @@ OP_REP_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0.|
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0005     | Reply code: The list of exported USB devices.     |
 +-----------+--------+------------+---------------------------------------------------+
@@ -165,8 +194,8 @@ OP_REP_DEVLIST:
 | 0x143     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x144     |        | m_0        | From now on each interface is described, all      |
-|           |        |            | together bNumInterfaces times, with the           |
-|           |        |            | the following 4 fields:                           |
+|           |        |            | together bNumInterfaces times, with the following |
+|           |        |            | 4 fields:                                         |
 +-----------+--------+------------+---------------------------------------------------+
 |           | 1      |            | bInterfaceClass                                   |
 +-----------+--------+------------+---------------------------------------------------+
@@ -177,7 +206,7 @@ OP_REP_DEVLIST:
 | 0x147     | 1      |            | padding byte for alignment, shall be set to zero  |
 +-----------+--------+------------+---------------------------------------------------+
 | 0xC +     |        |            | The second exported USB device starts at i=1      |
-| i*0x138 + |        |            | with the busid field.                             |
+| i*0x138 + |        |            | with the path field.                              |
 | m_(i-1)*4 |        |            |                                                   |
 +-----------+--------+------------+---------------------------------------------------+
 
@@ -187,7 +216,7 @@ OP_REQ_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8003     | Command code: import a remote USB device.         |
 +-----------+--------+------------+---------------------------------------------------+
@@ -206,7 +235,7 @@ OP_REP_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0003     | Reply code: Reply to import.                      |
 +-----------+--------+------------+---------------------------------------------------+
@@ -254,65 +283,75 @@ OP_REP_IMPORT:
 | 0x13E     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 
-USBIP_CMD_SUBMIT:
-	Submit an URB
+The following four commands have a common basic header called
+'usbip_header_basic', and their headers, called 'usbip_header' (before URB
+payload), have the same length, therefore paddings are needed.
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000001 | command: Submit an URB                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the sequence number of the URB to submit  |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number, possible values are: 0...15  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | transfer_flags: possible values depend on the     |
-|           |        |            | URB transfer type, see below                      |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x18      | 4      |            | transfer_buffer_length                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: specify the selected frame to        |
-|           |        |            | transmit an ISO frame, ignored if URB_ISO_ASAP    |
-|           |        |            | is specified at transfer_flags                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x24      | 4      |            | interval: maximum time for the request on the     |
-|           |        |            | server-side host controller                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      |        |            | URB data. For ISO transfers the padding between   |
-|           |        |            | each ISO packets is not transmitted.              |
-+-----------+--------+------------+---------------------------------------------------+
+usbip_header_basic:
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 4      | command                                           |
++-----------+--------+---------------------------------------------------+
+| 4         | 4      | seqnum: sequential number that identifies requests|
+|           |        | and corresponding responses;                      |
+|           |        | incremented per connection                        |
++-----------+--------+---------------------------------------------------+
+| 8         | 4      | devid: specifies a remote USB device uniquely     |
+|           |        | instead of busnum and devnum;                     |
+|           |        | for client (request), this value is               |
+|           |        | ((busnum << 16) | devnum);                        |
+|           |        | for server (response), this shall be set to 0     |
++-----------+--------+---------------------------------------------------+
+| 0xC       | 4      | direction:                                        |
+|           |        |                                                   |
+|           |        |    - 0: USBIP_DIR_OUT                             |
+|           |        |    - 1: USBIP_DIR_IN                              |
+|           |        |                                                   |
+|           |        | only used by client, for server this shall be 0   |
++-----------+--------+---------------------------------------------------+
+| 0x10      | 4      | ep: endpoint number                               |
+|           |        | only used by client, for server this shall be 0   |
+|           |        | for UNLINK, this shall be 0                       |
++-----------+--------+---------------------------------------------------+
 
- +-------------------------+------------+---------+-----------+----------+-------------+
- | Allowed transfer_flags  | value      | control | interrupt | bulk     | isochronous |
- +=========================+============+=========+===========+==========+=============+
- | URB_SHORT_NOT_OK        | 0x00000001 | only in | only in   | only in  | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ISO_ASAP            | 0x00000002 | no      | no        | no       | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ZERO_PACKET         | 0x00000040 | no      | no        | only out | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_INTERRUPT        | 0x00000080 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_FREE_BUFFER         | 0x00000100 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_DIR_MASK            | 0x00000200 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
+USBIP_CMD_SUBMIT:
+	Submit an URB
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000001 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | transfer_flags: possible values depend on the     |
+|           |        | URB transfer_flags,                               |
+|           |        | but with URB_NO_TRANSFER_DMA_MAP masked           |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 4      | transfer_buffer_length:                           |
+|           |        | use URB transfer_buffer_length                    |
++-----------+--------+---------------------------------------------------+
+| 0x1C      | 4      | start_frame: use URB start_frame;                 |
+|           |        | initial frame for ISO transfer                    |
+|           |        | shall be set to 0 if not ISO transfer             |
++-----------+--------+---------------------------------------------------+
+| 0x20      | 4      | number_of_packets: number of ISO packets          |
+|           |        | shall be set to 0xffffffff if not ISO transfer    |
++-----------+--------+---------------------------------------------------+
+| 0x24      | 4      | interval: maximum time for the request on the     |
+|           |        | server-side host controller                       |
++-----------+--------+---------------------------------------------------+
+| 0x28      | 8      | setup: data bytes for USB setup, filled with      |
+|           |        | zeros if not used                                 |
++-----------+--------+---------------------------------------------------+
+| 0x30      | n      | tranfer_buffer.                                   |
+|           |        | If direction is USBIP_DIR_OUT then n equals       |
+|           |        | transfer_buffer_length; otherwise n equals 0      |
+|           |        | For ISO transfers the padding between each ISO    |
+|           |        | between each ISO packets is not transmitted.      |
++-----------+--------+---------------------------------------------------+
+| 0x30+n    | m      | iso_packet_descriptor                             |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_SUBMIT:
 	Reply for submitting an URB
@@ -320,92 +359,75 @@ USBIP_RET_SUBMIT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 4      | 0x00000003 | command                                           |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
+| 0         | 20     |            | usbip_header_basic, 'command' shall be 0x00000003 |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x14      | 4      |            | status: zero for successful URB transaction,      |
 |           |        |            | otherwise some kind of error happened.            |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x18      | 4      | n          | actual_length: number of URB data bytes           |
+|           |        |            | use URB actual_length                             |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: for an ISO frame the actually        |
-|           |        |            | selected frame for transmit.                      |
+| 0x1C      | 4      |            | start_frame: use URB start_frame;                 |
+|           |        |            | initial frame for ISO transfer                    |
+|           |        |            | shall be set to 0 if not ISO transfer             |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets                                 |
+| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
+|           |        |            | shall be set to 0xffffffff if not ISO transfer    |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x24      | 4      |            | error_count                                       |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
+| 0x28      | 8      |            | padding, shall be set to 0                        |
 +-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
+| 0x30      | n      |            | tranfer_buffer.                                   |
+|           |        |            | If direction is USBIP_DIR_OUT then n equals       |
+|           |        |            | transfer_buffer_length; otherwise n equals 0      |
+|           |        |            | For ISO transfers the padding between each ISO    |
 |           |        |            | between each ISO packets is not transmitted.      |
 +-----------+--------+------------+---------------------------------------------------+
+| 0x30+n    | m      |            | iso_packet_descriptor                             |
++-----------+--------+------------+---------------------------------------------------+
 
 USBIP_CMD_UNLINK:
 	Unlink an URB
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000002 | command: URB unlink command                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number to unlink:            |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |    is this so?                                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number: zero                         |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | seqnum: the URB sequence number given previously  |
-|           |        |            | at USBIP_CMD_SUBMIT.seqnum field                  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000002 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | unlink_seqnum, of the SUBMIT request to unlink    |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_UNLINK:
 	Reply for URB unlink
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000004 | command: reply for the URB unlink command         |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the unlinked URB sequence number          |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | status: This is the value contained in the        |
-|           |        |            | urb->status in the URB completition handler.      |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |      a better explanation needed.                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000004 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | status: This is similar to that status of         |
+|           |        | USBIP_RET_SUBMIT (share the same memory offset)   |
+|           |        | When UNLINK is successful, status is -ECONNRESET; |
+|           |        | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT   |
+|           |        | status is 0                                       |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
+
+EXAMPLE
+=======
+
+  The following data is captured from wire with Human Interface Devices (HID)
+  payload
+
+::
+
+  CmdIntrIN:  00000001 00000d05 0001000f 00000001 00000001 00000200 00000040 ffffffff 00000000 00000004 00000000 00000000
+  CmdIntrOUT: 00000001 00000d06 0001000f 00000000 00000001 00000000 00000040 ffffffff 00000000 00000004 00000000 00000000
+              ffffffff860008a784ce5ae212376300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+  RetIntrOut: 00000003 00000d06 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+  RetIntrIn:  00000003 00000d05 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+              ffffffff860011a784ce5ae2123763612891b1020100000400000000000000000000000000000000000000000000000000000000000000000000000000000000
-- 
2.30.1


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

* Re: [PATCH v3] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-15  8:40     ` [PATCH v3] " Hongren Zheng (Zenithal)
@ 2021-03-15 18:25       ` Randy Dunlap
  2021-03-15 21:15       ` [PATCH v4] " Hongren Zheng (Zenithal)
  1 sibling, 0 replies; 14+ messages in thread
From: Randy Dunlap @ 2021-03-15 18:25 UTC (permalink / raw)
  To: Hongren Zheng (Zenithal),
	Valentina Manea, Shuah Khan, Greg Kroah-Hartman,
	Márton Németh, Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, linux-kernel, usbip-devel

Hi,
Some comments inline.


On 3/15/21 1:40 AM, Hongren Zheng (Zenithal) wrote:
> The old document for usbip protocol is misleading and hard to read:
>   * Some fields in header are incorrect
>   * Explanation of some fields are unclear or even wrong
>   * Padding of header (namely all headers have the same length) is
>     not explicitly point out, which is crucial for stream protocol like
>     TCP
> 
> These fixes are made through reading usbip kernel drivers and userland
> codes. Also I have implemented one usbip server.
> 
> Major changes:
> 
> PATCH v2:
> 
> PATCH v3:
> 
> Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> Signed-off-by: Hongren Zheng (Zenithal) <i@zenithal.me>
> ---
>  Documentation/usb/usbip_protocol.rst | 288 ++++++++++++++-------------
>  1 file changed, 155 insertions(+), 133 deletions(-)
> 
> diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
> index 988c832166cd..ed423cdda236 100644
> --- a/Documentation/usb/usbip_protocol.rst
> +++ b/Documentation/usb/usbip_protocol.rst

> @@ -254,65 +283,75 @@ OP_REP_IMPORT:
>  | 0x13E     | 1      |            | bNumInterfaces                                    |
>  +-----------+--------+------------+---------------------------------------------------+
>  
> -USBIP_CMD_SUBMIT:
> -	Submit an URB
> +The following four commands have a common basic header called
> +'usbip_header_basic', and their headers, called 'usbip_header' (before URB
> +payload), have the same length, therefore paddings are needed.
>  
> -+-----------+--------+------------+---------------------------------------------------+
> -| Offset    | Length | Value      | Description                                       |
> -+===========+========+============+===================================================+
> -| 0         | 4      | 0x00000001 | command: Submit an URB                            |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4         | 4      |            | seqnum: the sequence number of the URB to submit  |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8         | 4      |            | devid                                             |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC       | 4      |            | direction:                                        |
> -|           |        |            |                                                   |
> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> -|           |        |            |    - 1: USBIP_DIR_IN                              |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10      | 4      |            | ep: endpoint number, possible values are: 0...15  |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x14      | 4      |            | transfer_flags: possible values depend on the     |
> -|           |        |            | URB transfer type, see below                      |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x18      | 4      |            | transfer_buffer_length                            |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x1C      | 4      |            | start_frame: specify the selected frame to        |
> -|           |        |            | transmit an ISO frame, ignored if URB_ISO_ASAP    |
> -|           |        |            | is specified at transfer_flags                    |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x24      | 4      |            | interval: maximum time for the request on the     |
> -|           |        |            | server-side host controller                       |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
> -|           |        |            | zeros if not used                                 |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x30      |        |            | URB data. For ISO transfers the padding between   |
> -|           |        |            | each ISO packets is not transmitted.              |
> -+-----------+--------+------------+---------------------------------------------------+
> +usbip_header_basic:
>  
> ++-----------+--------+---------------------------------------------------+
> +| Offset    | Length | Description                                       |
> ++===========+========+===================================================+
> +| 0         | 4      | command                                           |
> ++-----------+--------+---------------------------------------------------+
> +| 4         | 4      | seqnum: sequential number that identifies requests|
> +|           |        | and corresponding responses;                      |
> +|           |        | incremented per connection                        |
> ++-----------+--------+---------------------------------------------------+
> +| 8         | 4      | devid: specifies a remote USB device uniquely     |
> +|           |        | instead of busnum and devnum;                     |
> +|           |        | for client (request), this value is               |
> +|           |        | ((busnum << 16) | devnum);                        |
> +|           |        | for server (response), this shall be set to 0     |
> ++-----------+--------+---------------------------------------------------+
> +| 0xC       | 4      | direction:                                        |
> +|           |        |                                                   |
> +|           |        |    - 0: USBIP_DIR_OUT                             |
> +|           |        |    - 1: USBIP_DIR_IN                              |
> +|           |        |                                                   |
> +|           |        | only used by client, for server this shall be 0   |
> ++-----------+--------+---------------------------------------------------+
> +| 0x10      | 4      | ep: endpoint number                               |
> +|           |        | only used by client, for server this shall be 0   |

This could use a ';' after the '0' above for better readability.

> +|           |        | for UNLINK, this shall be 0                       |
> ++-----------+--------+---------------------------------------------------+
>  
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | Allowed transfer_flags  | value      | control | interrupt | bulk     | isochronous |
> - +=========================+============+=========+===========+==========+=============+
> - | URB_SHORT_NOT_OK        | 0x00000001 | only in | only in   | only in  | no          |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_ISO_ASAP            | 0x00000002 | no      | no        | no       | yes         |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes     | yes       | yes      | yes         |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_ZERO_PACKET         | 0x00000040 | no      | no        | only out | no          |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_NO_INTERRUPT        | 0x00000080 | yes     | yes       | yes      | yes         |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_FREE_BUFFER         | 0x00000100 | yes     | yes       | yes      | yes         |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_DIR_MASK            | 0x00000200 | yes     | yes       | yes      | yes         |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> +USBIP_CMD_SUBMIT:
> +	Submit an URB
>  
> ++-----------+--------+---------------------------------------------------+
> +| Offset    | Length | Description                                       |
> ++===========+========+===================================================+
> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000001 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x14      | 4      | transfer_flags: possible values depend on the     |
> +|           |        | URB transfer_flags,                               |
> +|           |        | but with URB_NO_TRANSFER_DMA_MAP masked           |
> ++-----------+--------+---------------------------------------------------+
> +| 0x18      | 4      | transfer_buffer_length:                           |
> +|           |        | use URB transfer_buffer_length                    |
> ++-----------+--------+---------------------------------------------------+
> +| 0x1C      | 4      | start_frame: use URB start_frame;                 |
> +|           |        | initial frame for ISO transfer                    |

                                                transfer;

> +|           |        | shall be set to 0 if not ISO transfer             |
> ++-----------+--------+---------------------------------------------------+
> +| 0x20      | 4      | number_of_packets: number of ISO packets          |

                                                           packets;

> +|           |        | shall be set to 0xffffffff if not ISO transfer    |
> ++-----------+--------+---------------------------------------------------+
> +| 0x24      | 4      | interval: maximum time for the request on the     |
> +|           |        | server-side host controller                       |
> ++-----------+--------+---------------------------------------------------+
> +| 0x28      | 8      | setup: data bytes for USB setup, filled with      |
> +|           |        | zeros if not used                                 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x30      | n      | tranfer_buffer.                                   |
> +|           |        | If direction is USBIP_DIR_OUT then n equals       |
> +|           |        | transfer_buffer_length; otherwise n equals 0      |

                                                              equals 0.

> +|           |        | For ISO transfers the padding between each ISO    |
> +|           |        | between each ISO packets is not transmitted.      |
> ++-----------+--------+---------------------------------------------------+
> +| 0x30+n    | m      | iso_packet_descriptor                             |
> ++-----------+--------+---------------------------------------------------+
>  
>  USBIP_RET_SUBMIT:
>  	Reply for submitting an URB
> @@ -320,92 +359,75 @@ USBIP_RET_SUBMIT:
>  +-----------+--------+------------+---------------------------------------------------+
>  | Offset    | Length | Value      | Description                                       |
>  +===========+========+============+===================================================+
> -| 0         | 4      | 0x00000003 | command                                           |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4         | 4      |            | seqnum: URB sequence number                       |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8         | 4      |            | devid                                             |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC       | 4      |            | direction:                                        |
> -|           |        |            |                                                   |
> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> -|           |        |            |    - 1: USBIP_DIR_IN                              |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10      | 4      |            | ep: endpoint number                               |
> +| 0         | 20     |            | usbip_header_basic, 'command' shall be 0x00000003 |
>  +-----------+--------+------------+---------------------------------------------------+
>  | 0x14      | 4      |            | status: zero for successful URB transaction,      |
>  |           |        |            | otherwise some kind of error happened.            |
>  +-----------+--------+------------+---------------------------------------------------+
>  | 0x18      | 4      | n          | actual_length: number of URB data bytes           |

                                                                         bytes;

> +|           |        |            | use URB actual_length                             |
>  +-----------+--------+------------+---------------------------------------------------+
> -| 0x1C      | 4      |            | start_frame: for an ISO frame the actually        |
> -|           |        |            | selected frame for transmit.                      |
> +| 0x1C      | 4      |            | start_frame: use URB start_frame;                 |
> +|           |        |            | initial frame for ISO transfer                    |

                                                             transfer;

> +|           |        |            | shall be set to 0 if not ISO transfer             |
>  +-----------+--------+------------+---------------------------------------------------+
> -| 0x20      | 4      |            | number_of_packets                                 |
> +| 0x20      | 4      |            | number_of_packets: number of ISO packets          |

                                                                        packets;

> +|           |        |            | shall be set to 0xffffffff if not ISO transfer    |
>  +-----------+--------+------------+---------------------------------------------------+
>  | 0x24      | 4      |            | error_count                                       |
>  +-----------+--------+------------+---------------------------------------------------+
> -| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
> -|           |        |            | zeros if not used                                 |

                                                    used;

> +| 0x28      | 8      |            | padding, shall be set to 0                        |
>  +-----------+--------+------------+---------------------------------------------------+
> -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
> +| 0x30      | n      |            | tranfer_buffer.                                   |

                                       transfer_buffer.

> +|           |        |            | If direction is USBIP_DIR_OUT then n equals       |
> +|           |        |            | transfer_buffer_length; otherwise n equals 0      |

                                                                           equals 0.

> +|           |        |            | For ISO transfers the padding between each ISO    |
>  |           |        |            | between each ISO packets is not transmitted.      |
>  +-----------+--------+------------+---------------------------------------------------+
> +| 0x30+n    | m      |            | iso_packet_descriptor                             |
> ++-----------+--------+------------+---------------------------------------------------+
>  
>  USBIP_CMD_UNLINK:
>  	Unlink an URB
>  
> -+-----------+--------+------------+---------------------------------------------------+
> -| Offset    | Length | Value      | Description                                       |
> -+===========+========+============+===================================================+
> -| 0         | 4      | 0x00000002 | command: URB unlink command                       |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4         | 4      |            | seqnum: URB sequence number to unlink:            |
> -|           |        |            |                                                   |
> -|           |        |            | FIXME:                                            |
> -|           |        |            |    is this so?                                    |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8         | 4      |            | devid                                             |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC       | 4      |            | direction:                                        |
> -|           |        |            |                                                   |
> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> -|           |        |            |    - 1: USBIP_DIR_IN                              |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10      | 4      |            | ep: endpoint number: zero                         |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x14      | 4      |            | seqnum: the URB sequence number given previously  |
> -|           |        |            | at USBIP_CMD_SUBMIT.seqnum field                  |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
> -|           |        |            | between each ISO packets is not transmitted.      |
> -+-----------+--------+------------+---------------------------------------------------+
> ++-----------+--------+---------------------------------------------------+
> +| Offset    | Length | Description                                       |
> ++===========+========+===================================================+
> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000002 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x14      | 4      | unlink_seqnum, of the SUBMIT request to unlink    |
> ++-----------+--------+---------------------------------------------------+
> +| 0x18      | 24     | padding, shall be set to 0                        |
> ++-----------+--------+---------------------------------------------------+
>  
>  USBIP_RET_UNLINK:
>  	Reply for URB unlink
>  
> -+-----------+--------+------------+---------------------------------------------------+
> -| Offset    | Length | Value      | Description                                       |
> -+===========+========+============+===================================================+
> -| 0         | 4      | 0x00000004 | command: reply for the URB unlink command         |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4         | 4      |            | seqnum: the unlinked URB sequence number          |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8         | 4      |            | devid                                             |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC       | 4      |            | direction:                                        |
> -|           |        |            |                                                   |
> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> -|           |        |            |    - 1: USBIP_DIR_IN                              |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10      | 4      |            | ep: endpoint number                               |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x14      | 4      |            | status: This is the value contained in the        |
> -|           |        |            | urb->status in the URB completition handler.      |

                                                              completion

> -|           |        |            |                                                   |
> -|           |        |            | FIXME:                                            |
> -|           |        |            |      a better explanation needed.                 |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
> -|           |        |            | between each ISO packets is not transmitted.      |
> -+-----------+--------+------------+---------------------------------------------------+
> ++-----------+--------+---------------------------------------------------+
> +| Offset    | Length | Description                                       |
> ++===========+========+===================================================+
> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000004 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x14      | 4      | status: This is similar to that status of         |

                                                  to the status of

> +|           |        | USBIP_RET_SUBMIT (share the same memory offset)   |

                                                                  offset).

> +|           |        | When UNLINK is successful, status is -ECONNRESET; |
> +|           |        | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT   |
> +|           |        | status is 0                                       |> ++-----------+--------+---------------------------------------------------+
> +| 0x18      | 24     | padding, shall be set to 0                        |
> ++-----------+--------+---------------------------------------------------+
> +
> +EXAMPLE
> +=======



HTH.
-- 
~Randy


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

* [PATCH v4] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-15  8:40     ` [PATCH v3] " Hongren Zheng (Zenithal)
  2021-03-15 18:25       ` Randy Dunlap
@ 2021-03-15 21:15       ` Hongren Zheng (Zenithal)
  2021-03-16  1:54         ` Randy Dunlap
  2021-03-16  2:25         ` [PATCH v5] " Hongren Zheng (Zenithal)
  1 sibling, 2 replies; 14+ messages in thread
From: Hongren Zheng (Zenithal) @ 2021-03-15 21:15 UTC (permalink / raw)
  To: Valentina Manea, Shuah Khan, Greg Kroah-Hartman,
	Márton Németh, Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, linux-kernel, usbip-devel, Randy Dunlap

The old document for usbip protocol is misleading and hard to read:
  * Some fields in header are incorrect
  * Explanation of some fields are unclear or even wrong
  * Padding of header (namely all headers have the same length) is
    not explicitly pointed out, which is crucial for stream protocol
    like TCP

Major changes:
  * Document the correct field as described in the codebase.
  * Document the padding in usbip headers. This is crucial for TCP
    stream hence these padding should be explicitly point out.
    In code these padding are implemented by a union of all headers.
  * Fix two FIXME related to usbip unlink and Document the behavior
    of unlink in different situation.
  * Clarify some field with more accurate explanation, like those
    fields associated with URB. Some constraints are extracted from
    code.
  * Delete specific transfer_flag doc in usbip as it should be
    documented by the URB part.
  * Add data captured from wire as example

Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
Co-developed-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Hongren Zheng <i@zenithal.me>
---
 Documentation/usb/usbip_protocol.rst | 320 ++++++++++++++-------------
 1 file changed, 171 insertions(+), 149 deletions(-)

PATCH v2:
Some changes suggested by a previous patch in
https://lore.kernel.org/linux-usb
/20180128071514.9107-1-alexandre.f.demers@gmail.com/
is adopted in this patch.
  * Fix Typo: duplicated 'the' in 'the following 4 field'
  * Fix incorrect field: in OP_REQ_DEVLIST, the second dev starts with
    field 'path', not 'busid'

PATCH v3:
Suggested by
https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
  * Remove date and changelog in doc as these are tracked in git history
  * Remove 'mistake alert' as all data fields are documented properly
    now. However, docs on possible values for some field shall be added
    in the future

PATCH v4:
Suggested by https://lore.kernel.org/linux-doc
/40351ed6-2907-3966-e69a-a564173b3682@infradead.org/
  * Add punctuations for readability
  * Move patch changelog after the marker line
  * Remove nickname in signed-off-by line

diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
index 988c832166cd..54c5677adf4e 100644
--- a/Documentation/usb/usbip_protocol.rst
+++ b/Documentation/usb/usbip_protocol.rst
@@ -2,11 +2,11 @@
 USB/IP protocol
 ===============
 
-PRELIMINARY DRAFT, MAY CONTAIN MISTAKES!
-28 Jun 2011
+Architecture
+============
 
 The USB/IP protocol follows a server/client architecture. The server exports the
-USB devices and the clients imports them. The device driver for the exported
+USB devices and the client imports them. The device driver for the exported
 USB device runs on the client machine.
 
 The client may ask for the list of the exported USB devices. To get the list the
@@ -37,6 +37,9 @@ to transfer the URB traffic between the client and the server. The client may
 send two types of packets: the USBIP_CMD_SUBMIT to submit an URB, and
 USBIP_CMD_UNLINK to unlink a previously submitted URB. The answers of the
 server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
+Note that after successful USBIP_RET_UNLINK, the unlinked URB request would not
+have a corresponding USBIP_RET_UNLINK (this is explained in
+drivers/usb/usbip/stub_rx.c).
 
 ::
 
@@ -85,16 +88,42 @@ server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
           |                        .                        |
           |                        :                        |
           |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = p)         |
+          | ----------------------------------------------> |
+          |                                                 |
+          |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = p+1, unlink_seqnum = p)       |
+          | ----------------------------------------------> |
+          |                                                 |
+          |               USBIP_RET_UNLINK                  |
+          |        (seqnum = p+1, status = -ECONNRESET)     |
+          | <---------------------------------------------- |
+          |                                                 |
+          |         Note: No USBIP_RET_SUBMIT(seqnum = p)   |
+          | <--X---X---X---X---X---X---X---X---X---X---X--- |
+          |                        .                        |
+          |                        :                        |
+          |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = q)         |
+          | ----------------------------------------------> |
+          |                                                 |
+          |            USBIP_RET_SUBMIT(seqnum = q)         |
+          | <---------------------------------------------- |
+          |                                                 |
           |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = q+1, unlink_seqnum = q)       |
           | ----------------------------------------------> |
           |                                                 |
           |               USBIP_RET_UNLINK                  |
+          |           (seqnum = q+1, status = 0)            |
           | <---------------------------------------------- |
           |                                                 |
 
 The fields are in network (big endian) byte order meaning that the most significant
 byte (MSB) is stored at the lowest address.
 
+Message Format
+==============
 
 OP_REQ_DEVLIST:
 	Retrieve the list of exported USB devices.
@@ -102,7 +131,7 @@ OP_REQ_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8005     | Command code: Retrieve the list of exported USB   |
 |           |        |            | devices.                                          |
@@ -116,7 +145,7 @@ OP_REP_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0.|
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0005     | Reply code: The list of exported USB devices.     |
 +-----------+--------+------------+---------------------------------------------------+
@@ -165,8 +194,8 @@ OP_REP_DEVLIST:
 | 0x143     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x144     |        | m_0        | From now on each interface is described, all      |
-|           |        |            | together bNumInterfaces times, with the           |
-|           |        |            | the following 4 fields:                           |
+|           |        |            | together bNumInterfaces times, with the following |
+|           |        |            | 4 fields:                                         |
 +-----------+--------+------------+---------------------------------------------------+
 |           | 1      |            | bInterfaceClass                                   |
 +-----------+--------+------------+---------------------------------------------------+
@@ -177,7 +206,7 @@ OP_REP_DEVLIST:
 | 0x147     | 1      |            | padding byte for alignment, shall be set to zero  |
 +-----------+--------+------------+---------------------------------------------------+
 | 0xC +     |        |            | The second exported USB device starts at i=1      |
-| i*0x138 + |        |            | with the busid field.                             |
+| i*0x138 + |        |            | with the path field.                              |
 | m_(i-1)*4 |        |            |                                                   |
 +-----------+--------+------------+---------------------------------------------------+
 
@@ -187,7 +216,7 @@ OP_REQ_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8003     | Command code: import a remote USB device.         |
 +-----------+--------+------------+---------------------------------------------------+
@@ -206,7 +235,7 @@ OP_REP_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0003     | Reply code: Reply to import.                      |
 +-----------+--------+------------+---------------------------------------------------+
@@ -254,158 +283,151 @@ OP_REP_IMPORT:
 | 0x13E     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 
-USBIP_CMD_SUBMIT:
-	Submit an URB
+The following four commands have a common basic header called
+'usbip_header_basic', and their headers, called 'usbip_header' (before URB
+payload), have the same length, therefore paddings are needed.
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000001 | command: Submit an URB                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the sequence number of the URB to submit  |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number, possible values are: 0...15  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | transfer_flags: possible values depend on the     |
-|           |        |            | URB transfer type, see below                      |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x18      | 4      |            | transfer_buffer_length                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: specify the selected frame to        |
-|           |        |            | transmit an ISO frame, ignored if URB_ISO_ASAP    |
-|           |        |            | is specified at transfer_flags                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x24      | 4      |            | interval: maximum time for the request on the     |
-|           |        |            | server-side host controller                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      |        |            | URB data. For ISO transfers the padding between   |
-|           |        |            | each ISO packets is not transmitted.              |
-+-----------+--------+------------+---------------------------------------------------+
+usbip_header_basic:
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 4      | command                                           |
++-----------+--------+---------------------------------------------------+
+| 4         | 4      | seqnum: sequential number that identifies requests|
+|           |        | and corresponding responses;                      |
+|           |        | incremented per connection                        |
++-----------+--------+---------------------------------------------------+
+| 8         | 4      | devid: specifies a remote USB device uniquely     |
+|           |        | instead of busnum and devnum;                     |
+|           |        | for client (request), this value is               |
+|           |        | ((busnum << 16) | devnum);                        |
+|           |        | for server (response), this shall be set to 0     |
++-----------+--------+---------------------------------------------------+
+| 0xC       | 4      | direction:                                        |
+|           |        |                                                   |
+|           |        |    - 0: USBIP_DIR_OUT                             |
+|           |        |    - 1: USBIP_DIR_IN                              |
+|           |        |                                                   |
+|           |        | only used by client, for server this shall be 0   |
++-----------+--------+---------------------------------------------------+
+| 0x10      | 4      | ep: endpoint number                               |
+|           |        | only used by client, for server this shall be 0;  |
+|           |        | for UNLINK, this shall be 0                       |
++-----------+--------+---------------------------------------------------+
 
- +-------------------------+------------+---------+-----------+----------+-------------+
- | Allowed transfer_flags  | value      | control | interrupt | bulk     | isochronous |
- +=========================+============+=========+===========+==========+=============+
- | URB_SHORT_NOT_OK        | 0x00000001 | only in | only in   | only in  | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ISO_ASAP            | 0x00000002 | no      | no        | no       | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ZERO_PACKET         | 0x00000040 | no      | no        | only out | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_INTERRUPT        | 0x00000080 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_FREE_BUFFER         | 0x00000100 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_DIR_MASK            | 0x00000200 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
+USBIP_CMD_SUBMIT:
+	Submit an URB
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000001 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | transfer_flags: possible values depend on the     |
+|           |        | URB transfer_flags,                               |
+|           |        | but with URB_NO_TRANSFER_DMA_MAP masked           |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 4      | transfer_buffer_length:                           |
+|           |        | use URB transfer_buffer_length                    |
++-----------+--------+---------------------------------------------------+
+| 0x1C      | 4      | start_frame: use URB start_frame;                 |
+|           |        | initial frame for ISO transfer;                   |
+|           |        | shall be set to 0 if not ISO transfer             |
++-----------+--------+---------------------------------------------------+
+| 0x20      | 4      | number_of_packets: number of ISO packets;         |
+|           |        | shall be set to 0xffffffff if not ISO transfer    |
++-----------+--------+---------------------------------------------------+
+| 0x24      | 4      | interval: maximum time for the request on the     |
+|           |        | server-side host controller                       |
++-----------+--------+---------------------------------------------------+
+| 0x28      | 8      | setup: data bytes for USB setup, filled with      |
+|           |        | zeros if not used.                                |
++-----------+--------+---------------------------------------------------+
+| 0x30      | n      | transfer_buffer.                                  |
+|           |        | If direction is USBIP_DIR_OUT then n equals       |
+|           |        | transfer_buffer_length; otherwise n equals 0.     |
+|           |        | For ISO transfers the padding between each ISO    |
+|           |        | packets is not transmitted.                       |
++-----------+--------+---------------------------------------------------+
+| 0x30+n    | m      | iso_packet_descriptor                             |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_SUBMIT:
 	Reply for submitting an URB
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000003 | command                                           |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | status: zero for successful URB transaction,      |
-|           |        |            | otherwise some kind of error happened.            |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x18      | 4      | n          | actual_length: number of URB data bytes           |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: for an ISO frame the actually        |
-|           |        |            | selected frame for transmit.                      |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x24      | 4      |            | error_count                                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000003 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | status: zero for successful URB transaction,      |
+|           |        | otherwise some kind of error happened.            |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 4      | actual_length: number of URB data bytes;          |
+|           |        | use URB actual_length                             |
++-----------+--------+---------------------------------------------------+
+| 0x1C      | 4      | start_frame: use URB start_frame;                 |
+|           |        | initial frame for ISO transfer;                   |
+|           |        | shall be set to 0 if not ISO transfer             |
++-----------+--------+---------------------------------------------------+
+| 0x20      | 4      | number_of_packets: number of ISO packets;         |
+|           |        | shall be set to 0xffffffff if not ISO transfer    |
++-----------+--------+---------------------------------------------------+
+| 0x24      | 4      | error_count                                       |
++-----------+--------+---------------------------------------------------+
+| 0x28      | 8      | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
+| 0x30      | n      | transfer_buffer.                                  |
+|           |        | If direction is USBIP_DIR_IN then n equals        |
+|           |        | actual_length; otherwise n equals 0.              |
+|           |        | For ISO transfers the padding between each ISO    |
+|           |        | packets is not transmitted.                       |
++-----------+--------+---------------------------------------------------+
+| 0x30+n    | m      | iso_packet_descriptor                             |
++-----------+--------+---------------------------------------------------+
 
 USBIP_CMD_UNLINK:
 	Unlink an URB
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000002 | command: URB unlink command                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number to unlink:            |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |    is this so?                                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number: zero                         |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | seqnum: the URB sequence number given previously  |
-|           |        |            | at USBIP_CMD_SUBMIT.seqnum field                  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000002 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | unlink_seqnum, of the SUBMIT request to unlink    |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_UNLINK:
 	Reply for URB unlink
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000004 | command: reply for the URB unlink command         |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the unlinked URB sequence number          |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | status: This is the value contained in the        |
-|           |        |            | urb->status in the URB completition handler.      |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |      a better explanation needed.                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000004 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | status: This is similar to the status of          |
+|           |        | USBIP_RET_SUBMIT (share the same memory offset).  |
+|           |        | When UNLINK is successful, status is -ECONNRESET; |
+|           |        | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT   |
+|           |        | status is 0                                       |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
+
+EXAMPLE
+=======
+
+  The following data is captured from wire with Human Interface Devices (HID)
+  payload
+
+::
+
+  CmdIntrIN:  00000001 00000d05 0001000f 00000001 00000001 00000200 00000040 ffffffff 00000000 00000004 00000000 00000000
+  CmdIntrOUT: 00000001 00000d06 0001000f 00000000 00000001 00000000 00000040 ffffffff 00000000 00000004 00000000 00000000
+              ffffffff860008a784ce5ae212376300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+  RetIntrOut: 00000003 00000d06 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+  RetIntrIn:  00000003 00000d05 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+              ffffffff860011a784ce5ae2123763612891b1020100000400000000000000000000000000000000000000000000000000000000000000000000000000000000
-- 
2.30.1


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

* Re: [PATCH v4] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-15 21:15       ` [PATCH v4] " Hongren Zheng (Zenithal)
@ 2021-03-16  1:54         ` Randy Dunlap
  2021-03-16  2:25         ` [PATCH v5] " Hongren Zheng (Zenithal)
  1 sibling, 0 replies; 14+ messages in thread
From: Randy Dunlap @ 2021-03-16  1:54 UTC (permalink / raw)
  To: Hongren Zheng (Zenithal),
	Valentina Manea, Shuah Khan, Greg Kroah-Hartman,
	Márton Németh, Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, linux-kernel, usbip-devel

On 3/15/21 2:15 PM, Hongren Zheng (Zenithal) wrote:
> 
> Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> Co-developed-by: Randy Dunlap <rdunlap@infradead.org>

No, I'm not in the Co-developed-by loop here.

And then you can add:
Reviewed-by: Randy Dunlap <rdunlap@infradead.org>


> Signed-off-by: Hongren Zheng <i@zenithal.me>
> ---
>  Documentation/usb/usbip_protocol.rst | 320 ++++++++++++++-------------
>  1 file changed, 171 insertions(+), 149 deletions(-)
> 
> 
> PATCH v4:
> Suggested by https://lore.kernel.org/linux-doc
> /40351ed6-2907-3966-e69a-a564173b3682@infradead.org/
>   * Add punctuations for readability
>   * Move patch changelog after the marker line
>   * Remove nickname in signed-off-by line
> 
> diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
> index 988c832166cd..54c5677adf4e 100644
> --- a/Documentation/usb/usbip_protocol.rst
> +++ b/Documentation/usb/usbip_protocol.rst



-- 
~Randy


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

* [PATCH v5] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-15 21:15       ` [PATCH v4] " Hongren Zheng (Zenithal)
  2021-03-16  1:54         ` Randy Dunlap
@ 2021-03-16  2:25         ` Hongren Zheng (Zenithal)
  2021-03-16 15:57           ` Shuah Khan
                             ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Hongren Zheng (Zenithal) @ 2021-03-16  2:25 UTC (permalink / raw)
  To: Valentina Manea, Shuah Khan, Greg Kroah-Hartman,
	Márton Németh, Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, usbip-devel, Randy Dunlap

The old document for usbip protocol is misleading and hard to read:
  * Some fields in header are incorrect
  * Explanation of some fields are unclear or even wrong
  * Padding of header (namely all headers have the same length) is
    not explicitly pointed out, which is crucial for stream protocol
    like TCP

Major changes:
  * Document the correct field as described in the codebase.
  * Document the padding in usbip headers. This is crucial for TCP
    stream hence these padding should be explicitly point out.
    In code these padding are implemented by a union of all headers.
  * Fix two FIXME related to usbip unlink and Document the behavior
    of unlink in different situation.
  * Clarify some field with more accurate explanation, like those
    fields associated with URB. Some constraints are extracted from
    code.
  * Delete specific transfer_flag doc in usbip as it should be
    documented by the URB part.
  * Add data captured from wire as example

Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Hongren Zheng <i@zenithal.me>
---
 Documentation/usb/usbip_protocol.rst | 320 ++++++++++++++-------------
 1 file changed, 171 insertions(+), 149 deletions(-)

PATCH v2:
Some changes suggested by a previous patch in
https://lore.kernel.org/linux-usb
/20180128071514.9107-1-alexandre.f.demers@gmail.com/
is adopted in this patch.
  * Fix Typo: duplicated 'the' in 'the following 4 field'
  * Fix incorrect field: in OP_REQ_DEVLIST, the second dev starts with
    field 'path', not 'busid'

PATCH v3:
Suggested by
https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
  * Remove date and changelog in doc as these are tracked in git history
  * Remove 'mistake alert' as all data fields are documented properly
    now. However, docs on possible values for some field shall be added
    in the future

PATCH v4:
Suggested by https://lore.kernel.org/linux-doc
/40351ed6-2907-3966-e69a-a564173b3682@infradead.org/
  * Add punctuations for readability
  * Move patch changelog after the marker line
  * Remove nickname in signed-off-by line

PATCH v5:
  * Instead of co-developed-by, use reviewed-by
    for Randy Dunlap

diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
index 988c832166cd..54c5677adf4e 100644
--- a/Documentation/usb/usbip_protocol.rst
+++ b/Documentation/usb/usbip_protocol.rst
@@ -2,11 +2,11 @@
 USB/IP protocol
 ===============
 
-PRELIMINARY DRAFT, MAY CONTAIN MISTAKES!
-28 Jun 2011
+Architecture
+============
 
 The USB/IP protocol follows a server/client architecture. The server exports the
-USB devices and the clients imports them. The device driver for the exported
+USB devices and the client imports them. The device driver for the exported
 USB device runs on the client machine.
 
 The client may ask for the list of the exported USB devices. To get the list the
@@ -37,6 +37,9 @@ to transfer the URB traffic between the client and the server. The client may
 send two types of packets: the USBIP_CMD_SUBMIT to submit an URB, and
 USBIP_CMD_UNLINK to unlink a previously submitted URB. The answers of the
 server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
+Note that after successful USBIP_RET_UNLINK, the unlinked URB request would not
+have a corresponding USBIP_RET_UNLINK (this is explained in
+drivers/usb/usbip/stub_rx.c).
 
 ::
 
@@ -85,16 +88,42 @@ server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
           |                        .                        |
           |                        :                        |
           |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = p)         |
+          | ----------------------------------------------> |
+          |                                                 |
+          |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = p+1, unlink_seqnum = p)       |
+          | ----------------------------------------------> |
+          |                                                 |
+          |               USBIP_RET_UNLINK                  |
+          |        (seqnum = p+1, status = -ECONNRESET)     |
+          | <---------------------------------------------- |
+          |                                                 |
+          |         Note: No USBIP_RET_SUBMIT(seqnum = p)   |
+          | <--X---X---X---X---X---X---X---X---X---X---X--- |
+          |                        .                        |
+          |                        :                        |
+          |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = q)         |
+          | ----------------------------------------------> |
+          |                                                 |
+          |            USBIP_RET_SUBMIT(seqnum = q)         |
+          | <---------------------------------------------- |
+          |                                                 |
           |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = q+1, unlink_seqnum = q)       |
           | ----------------------------------------------> |
           |                                                 |
           |               USBIP_RET_UNLINK                  |
+          |           (seqnum = q+1, status = 0)            |
           | <---------------------------------------------- |
           |                                                 |
 
 The fields are in network (big endian) byte order meaning that the most significant
 byte (MSB) is stored at the lowest address.
 
+Message Format
+==============
 
 OP_REQ_DEVLIST:
 	Retrieve the list of exported USB devices.
@@ -102,7 +131,7 @@ OP_REQ_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8005     | Command code: Retrieve the list of exported USB   |
 |           |        |            | devices.                                          |
@@ -116,7 +145,7 @@ OP_REP_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0.|
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0005     | Reply code: The list of exported USB devices.     |
 +-----------+--------+------------+---------------------------------------------------+
@@ -165,8 +194,8 @@ OP_REP_DEVLIST:
 | 0x143     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x144     |        | m_0        | From now on each interface is described, all      |
-|           |        |            | together bNumInterfaces times, with the           |
-|           |        |            | the following 4 fields:                           |
+|           |        |            | together bNumInterfaces times, with the following |
+|           |        |            | 4 fields:                                         |
 +-----------+--------+------------+---------------------------------------------------+
 |           | 1      |            | bInterfaceClass                                   |
 +-----------+--------+------------+---------------------------------------------------+
@@ -177,7 +206,7 @@ OP_REP_DEVLIST:
 | 0x147     | 1      |            | padding byte for alignment, shall be set to zero  |
 +-----------+--------+------------+---------------------------------------------------+
 | 0xC +     |        |            | The second exported USB device starts at i=1      |
-| i*0x138 + |        |            | with the busid field.                             |
+| i*0x138 + |        |            | with the path field.                              |
 | m_(i-1)*4 |        |            |                                                   |
 +-----------+--------+------------+---------------------------------------------------+
 
@@ -187,7 +216,7 @@ OP_REQ_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8003     | Command code: import a remote USB device.         |
 +-----------+--------+------------+---------------------------------------------------+
@@ -206,7 +235,7 @@ OP_REP_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0003     | Reply code: Reply to import.                      |
 +-----------+--------+------------+---------------------------------------------------+
@@ -254,158 +283,151 @@ OP_REP_IMPORT:
 | 0x13E     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 
-USBIP_CMD_SUBMIT:
-	Submit an URB
+The following four commands have a common basic header called
+'usbip_header_basic', and their headers, called 'usbip_header' (before URB
+payload), have the same length, therefore paddings are needed.
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000001 | command: Submit an URB                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the sequence number of the URB to submit  |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number, possible values are: 0...15  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | transfer_flags: possible values depend on the     |
-|           |        |            | URB transfer type, see below                      |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x18      | 4      |            | transfer_buffer_length                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: specify the selected frame to        |
-|           |        |            | transmit an ISO frame, ignored if URB_ISO_ASAP    |
-|           |        |            | is specified at transfer_flags                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x24      | 4      |            | interval: maximum time for the request on the     |
-|           |        |            | server-side host controller                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      |        |            | URB data. For ISO transfers the padding between   |
-|           |        |            | each ISO packets is not transmitted.              |
-+-----------+--------+------------+---------------------------------------------------+
+usbip_header_basic:
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 4      | command                                           |
++-----------+--------+---------------------------------------------------+
+| 4         | 4      | seqnum: sequential number that identifies requests|
+|           |        | and corresponding responses;                      |
+|           |        | incremented per connection                        |
++-----------+--------+---------------------------------------------------+
+| 8         | 4      | devid: specifies a remote USB device uniquely     |
+|           |        | instead of busnum and devnum;                     |
+|           |        | for client (request), this value is               |
+|           |        | ((busnum << 16) | devnum);                        |
+|           |        | for server (response), this shall be set to 0     |
++-----------+--------+---------------------------------------------------+
+| 0xC       | 4      | direction:                                        |
+|           |        |                                                   |
+|           |        |    - 0: USBIP_DIR_OUT                             |
+|           |        |    - 1: USBIP_DIR_IN                              |
+|           |        |                                                   |
+|           |        | only used by client, for server this shall be 0   |
++-----------+--------+---------------------------------------------------+
+| 0x10      | 4      | ep: endpoint number                               |
+|           |        | only used by client, for server this shall be 0;  |
+|           |        | for UNLINK, this shall be 0                       |
++-----------+--------+---------------------------------------------------+
 
- +-------------------------+------------+---------+-----------+----------+-------------+
- | Allowed transfer_flags  | value      | control | interrupt | bulk     | isochronous |
- +=========================+============+=========+===========+==========+=============+
- | URB_SHORT_NOT_OK        | 0x00000001 | only in | only in   | only in  | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ISO_ASAP            | 0x00000002 | no      | no        | no       | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ZERO_PACKET         | 0x00000040 | no      | no        | only out | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_INTERRUPT        | 0x00000080 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_FREE_BUFFER         | 0x00000100 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_DIR_MASK            | 0x00000200 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
+USBIP_CMD_SUBMIT:
+	Submit an URB
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000001 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | transfer_flags: possible values depend on the     |
+|           |        | URB transfer_flags,                               |
+|           |        | but with URB_NO_TRANSFER_DMA_MAP masked           |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 4      | transfer_buffer_length:                           |
+|           |        | use URB transfer_buffer_length                    |
++-----------+--------+---------------------------------------------------+
+| 0x1C      | 4      | start_frame: use URB start_frame;                 |
+|           |        | initial frame for ISO transfer;                   |
+|           |        | shall be set to 0 if not ISO transfer             |
++-----------+--------+---------------------------------------------------+
+| 0x20      | 4      | number_of_packets: number of ISO packets;         |
+|           |        | shall be set to 0xffffffff if not ISO transfer    |
++-----------+--------+---------------------------------------------------+
+| 0x24      | 4      | interval: maximum time for the request on the     |
+|           |        | server-side host controller                       |
++-----------+--------+---------------------------------------------------+
+| 0x28      | 8      | setup: data bytes for USB setup, filled with      |
+|           |        | zeros if not used.                                |
++-----------+--------+---------------------------------------------------+
+| 0x30      | n      | transfer_buffer.                                  |
+|           |        | If direction is USBIP_DIR_OUT then n equals       |
+|           |        | transfer_buffer_length; otherwise n equals 0.     |
+|           |        | For ISO transfers the padding between each ISO    |
+|           |        | packets is not transmitted.                       |
++-----------+--------+---------------------------------------------------+
+| 0x30+n    | m      | iso_packet_descriptor                             |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_SUBMIT:
 	Reply for submitting an URB
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000003 | command                                           |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | status: zero for successful URB transaction,      |
-|           |        |            | otherwise some kind of error happened.            |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x18      | 4      | n          | actual_length: number of URB data bytes           |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: for an ISO frame the actually        |
-|           |        |            | selected frame for transmit.                      |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x24      | 4      |            | error_count                                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000003 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | status: zero for successful URB transaction,      |
+|           |        | otherwise some kind of error happened.            |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 4      | actual_length: number of URB data bytes;          |
+|           |        | use URB actual_length                             |
++-----------+--------+---------------------------------------------------+
+| 0x1C      | 4      | start_frame: use URB start_frame;                 |
+|           |        | initial frame for ISO transfer;                   |
+|           |        | shall be set to 0 if not ISO transfer             |
++-----------+--------+---------------------------------------------------+
+| 0x20      | 4      | number_of_packets: number of ISO packets;         |
+|           |        | shall be set to 0xffffffff if not ISO transfer    |
++-----------+--------+---------------------------------------------------+
+| 0x24      | 4      | error_count                                       |
++-----------+--------+---------------------------------------------------+
+| 0x28      | 8      | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
+| 0x30      | n      | transfer_buffer.                                  |
+|           |        | If direction is USBIP_DIR_IN then n equals        |
+|           |        | actual_length; otherwise n equals 0.              |
+|           |        | For ISO transfers the padding between each ISO    |
+|           |        | packets is not transmitted.                       |
++-----------+--------+---------------------------------------------------+
+| 0x30+n    | m      | iso_packet_descriptor                             |
++-----------+--------+---------------------------------------------------+
 
 USBIP_CMD_UNLINK:
 	Unlink an URB
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000002 | command: URB unlink command                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number to unlink:            |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |    is this so?                                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number: zero                         |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | seqnum: the URB sequence number given previously  |
-|           |        |            | at USBIP_CMD_SUBMIT.seqnum field                  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000002 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | unlink_seqnum, of the SUBMIT request to unlink    |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_UNLINK:
 	Reply for URB unlink
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000004 | command: reply for the URB unlink command         |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the unlinked URB sequence number          |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | status: This is the value contained in the        |
-|           |        |            | urb->status in the URB completition handler.      |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |      a better explanation needed.                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000004 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | status: This is similar to the status of          |
+|           |        | USBIP_RET_SUBMIT (share the same memory offset).  |
+|           |        | When UNLINK is successful, status is -ECONNRESET; |
+|           |        | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT   |
+|           |        | status is 0                                       |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
+
+EXAMPLE
+=======
+
+  The following data is captured from wire with Human Interface Devices (HID)
+  payload
+
+::
+
+  CmdIntrIN:  00000001 00000d05 0001000f 00000001 00000001 00000200 00000040 ffffffff 00000000 00000004 00000000 00000000
+  CmdIntrOUT: 00000001 00000d06 0001000f 00000000 00000001 00000000 00000040 ffffffff 00000000 00000004 00000000 00000000
+              ffffffff860008a784ce5ae212376300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+  RetIntrOut: 00000003 00000d06 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+  RetIntrIn:  00000003 00000d05 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+              ffffffff860011a784ce5ae2123763612891b1020100000400000000000000000000000000000000000000000000000000000000000000000000000000000000
-- 
2.30.1


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

* Re: [PATCH v5] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-16  2:25         ` [PATCH v5] " Hongren Zheng (Zenithal)
@ 2021-03-16 15:57           ` Shuah Khan
  2021-03-29 20:06           ` Shuah Khan
  2021-03-30 17:00           ` [PATCH v6] " Hongren Zheng (Zenithal)
  2 siblings, 0 replies; 14+ messages in thread
From: Shuah Khan @ 2021-03-16 15:57 UTC (permalink / raw)
  To: Hongren Zheng (Zenithal),
	Valentina Manea, Shuah Khan, Greg Kroah-Hartman,
	Márton Németh, Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, usbip-devel, Randy Dunlap, Shuah Khan

On 3/15/21 8:25 PM, Hongren Zheng (Zenithal) wrote:
> The old document for usbip protocol is misleading and hard to read:
>    * Some fields in header are incorrect
>    * Explanation of some fields are unclear or even wrong
>    * Padding of header (namely all headers have the same length) is
>      not explicitly pointed out, which is crucial for stream protocol
>      like TCP
> 
> Major changes:
>    * Document the correct field as described in the codebase.
>    * Document the padding in usbip headers. This is crucial for TCP
>      stream hence these padding should be explicitly point out.
>      In code these padding are implemented by a union of all headers.
>    * Fix two FIXME related to usbip unlink and Document the behavior
>      of unlink in different situation.
>    * Clarify some field with more accurate explanation, like those
>      fields associated with URB. Some constraints are extracted from
>      code.
>    * Delete specific transfer_flag doc in usbip as it should be
>      documented by the URB part.
>    * Add data captured from wire as example
> 
> Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Hongren Zheng <i@zenithal.me>
> ---
>   Documentation/usb/usbip_protocol.rst | 320 ++++++++++++++-------------
>   1 file changed, 171 insertions(+), 149 deletions(-)
> 
> PATCH v2:
> Some changes suggested by a previous patch in
> https://lore.kernel.org/linux-usb
> /20180128071514.9107-1-alexandre.f.demers@gmail.com/
> is adopted in this patch.
>    * Fix Typo: duplicated 'the' in 'the following 4 field'
>    * Fix incorrect field: in OP_REQ_DEVLIST, the second dev starts with
>      field 'path', not 'busid'
> 
> PATCH v3:
> Suggested by
> https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
>    * Remove date and changelog in doc as these are tracked in git history
>    * Remove 'mistake alert' as all data fields are documented properly
>      now. However, docs on possible values for some field shall be added
>      in the future
> 
> PATCH v4:
> Suggested by https://lore.kernel.org/linux-doc
> /40351ed6-2907-3966-e69a-a564173b3682@infradead.org/
>    * Add punctuations for readability
>    * Move patch changelog after the marker line
>    * Remove nickname in signed-off-by line
> 
> PATCH v5:
>    * Instead of co-developed-by, use reviewed-by
>      for Randy Dunlap
> 

Hi Hongren Zheng,

Thanks for the patch. The document updates are very much needed.
I will review and give you comments.

Please wait for a couple of days before sending another version.
Makes it easier for you an your reviewers.

I started reviewing patch v3 and I will switch to v5 now
and send you comments.

thanks,
-- Shuah

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

* Re: [PATCH v5] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-16  2:25         ` [PATCH v5] " Hongren Zheng (Zenithal)
  2021-03-16 15:57           ` Shuah Khan
@ 2021-03-29 20:06           ` Shuah Khan
  2021-03-30 12:35             ` Hongren Zheng (Zenithal)
  2021-03-30 17:00           ` [PATCH v6] " Hongren Zheng (Zenithal)
  2 siblings, 1 reply; 14+ messages in thread
From: Shuah Khan @ 2021-03-29 20:06 UTC (permalink / raw)
  To: Hongren Zheng (Zenithal),
	Valentina Manea, Shuah Khan, Greg Kroah-Hartman,
	Márton Németh, Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, usbip-devel, Randy Dunlap, Shuah Khan

On 3/15/21 8:25 PM, Hongren Zheng (Zenithal) wrote:
> The old document for usbip protocol is misleading and hard to read:
>    * Some fields in header are incorrect
>    * Explanation of some fields are unclear or even wrong
>    * Padding of header (namely all headers have the same length) is
>      not explicitly pointed out, which is crucial for stream protocol
>      like TCP
> 
> Major changes:
>    * Document the correct field as described in the codebase.
>    * Document the padding in usbip headers. This is crucial for TCP
>      stream hence these padding should be explicitly point out.
>      In code these padding are implemented by a union of all headers.
>    * Fix two FIXME related to usbip unlink and Document the behavior
>      of unlink in different situation.
>    * Clarify some field with more accurate explanation, like those
>      fields associated with URB. Some constraints are extracted from
>      code.
>    * Delete specific transfer_flag doc in usbip as it should be
>      documented by the URB part.

Why are we deleting this. What do you mean documented by URB part?

>    * Add data captured from wire as example
> 
> Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Hongren Zheng <i@zenithal.me>
> ---
>   Documentation/usb/usbip_protocol.rst | 320 ++++++++++++++-------------
>   1 file changed, 171 insertions(+), 149 deletions(-)
> 
> PATCH v2:
> Some changes suggested by a previous patch in
> https://lore.kernel.org/linux-usb
> /20180128071514.9107-1-alexandre.f.demers@gmail.com/
> is adopted in this patch.
>    * Fix Typo: duplicated 'the' in 'the following 4 field'
>    * Fix incorrect field: in OP_REQ_DEVLIST, the second dev starts with
>      field 'path', not 'busid'
> 
> PATCH v3:
> Suggested by
> https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
>    * Remove date and changelog in doc as these are tracked in git history
>    * Remove 'mistake alert' as all data fields are documented properly
>      now. However, docs on possible values for some field shall be added
>      in the future
> 
> PATCH v4:
> Suggested by https://lore.kernel.org/linux-doc
> /40351ed6-2907-3966-e69a-a564173b3682@infradead.org/
>    * Add punctuations for readability
>    * Move patch changelog after the marker line
>    * Remove nickname in signed-off-by line
> 
> PATCH v5:
>    * Instead of co-developed-by, use reviewed-by
>      for Randy Dunlap
> 
> diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
> index 988c832166cd..54c5677adf4e 100644
> --- a/Documentation/usb/usbip_protocol.rst
> +++ b/Documentation/usb/usbip_protocol.rst
> @@ -2,11 +2,11 @@
>   USB/IP protocol
>   ===============
>    > -PRELIMINARY DRAFT, MAY CONTAIN MISTAKES> -28 Jun 2011
> +Architecture

Let's add doc version preserving the history.
Add Version 1 and date perhaps.

> +============
>   
>   The USB/IP protocol follows a server/client architecture. The server exports the
> -USB devices and the clients imports them. The device driver for the exported
> +USB devices and the client imports them. The device driver for the exported

clients import them.

>   USB device runs on the client machine.
>   
>   The client may ask for the list of the exported USB devices. To get the list the
> @@ -37,6 +37,9 @@ to transfer the URB traffic between the client and the server. The client may
>   send two types of packets: the USBIP_CMD_SUBMIT to submit an URB, and
>   USBIP_CMD_UNLINK to unlink a previously submitted URB. The answers of the
>   server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
> +Note that after successful USBIP_RET_UNLINK, the unlinked URB request would not
> +have a corresponding USBIP_RET_UNLINK (this is explained in
> +drivers/usb/usbip/stub_rx.c).

This is not clear to me. Doesn't look correct. Where do you see this
in drivers/usb/usbip/stub_rx.c?

>   
>   ::
>   
> @@ -85,16 +88,42 @@ server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
>             |                        .                        |
>             |                        :                        |
>             |                                                 |
> +          |            USBIP_CMD_SUBMIT(seqnum = p)         |
> +          | ----------------------------------------------> |
> +          |                                                 |
> +          |               USBIP_CMD_UNLINK                  |
> +          |         (seqnum = p+1, unlink_seqnum = p)       |
> +          | ----------------------------------------------> |
> +          |                                                 |
> +          |               USBIP_RET_UNLINK                  |
> +          |        (seqnum = p+1, status = -ECONNRESET)     |
> +          | <---------------------------------------------- |
> +          |                                                 |
> +          |         Note: No USBIP_RET_SUBMIT(seqnum = p)   |
> +          | <--X---X---X---X---X---X---X---X---X---X---X--- |
> +          |                        .                        |
> +          |                        :                        |
> +          |                                                 |
> +          |            USBIP_CMD_SUBMIT(seqnum = q)         |
> +          | ----------------------------------------------> |
> +          |                                                 |
> +          |            USBIP_RET_SUBMIT(seqnum = q)         |
> +          | <---------------------------------------------- |
> +          |                                                 |
>             |               USBIP_CMD_UNLINK                  |
> +          |         (seqnum = q+1, unlink_seqnum = q)       |
>             | ----------------------------------------------> |
>             |                                                 |
>             |               USBIP_RET_UNLINK                  |
> +          |           (seqnum = q+1, status = 0)            |
>             | <---------------------------------------------- |
>             |                                                 |
>   

I would do this differently. Let's add this as an expanded
USBIP_CMD_UNLINK sequence with a separate heading below the
current flow

>   The fields are in network (big endian) byte order meaning that the most significant
>   byte (MSB) is stored at the lowest address.
>   
> +Message Format
> +==============
>   
>   OP_REQ_DEVLIST:
>   	Retrieve the list of exported USB devices.
> @@ -102,7 +131,7 @@ OP_REQ_DEVLIST:
>   +-----------+--------+------------+---------------------------------------------------+
>   | Offset    | Length | Value      | Description                                       |
>   +===========+========+============+===================================================+
> -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
> +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
>   +-----------+--------+------------+---------------------------------------------------+

Let's make this vx.x.x and specify current version number at the top.
Saves us doc updates as we change version number - one location change
as opposed to the entire document
>   | 2         | 2      | 0x8005     | Command code: Retrieve the list of exported USB   |
>   |           |        |            | devices.                                          |
> @@ -116,7 +145,7 @@ OP_REP_DEVLIST:
>   +-----------+--------+------------+---------------------------------------------------+
>   | Offset    | Length | Value      | Description                                       |
>   +===========+========+============+===================================================+
> -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0.|
> +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
>   +-----------+--------+------------+---------------------------------------------------+
>   | 2         | 2      | 0x0005     | Reply code: The list of exported USB devices.     |
>   +-----------+--------+------------+---------------------------------------------------+
> @@ -165,8 +194,8 @@ OP_REP_DEVLIST:
>   | 0x143     | 1      |            | bNumInterfaces                                    |
>   +-----------+--------+------------+---------------------------------------------------+
>   | 0x144     |        | m_0        | From now on each interface is described, all      |
> -|           |        |            | together bNumInterfaces times, with the           |
> -|           |        |            | the following 4 fields:                           |
> +|           |        |            | together bNumInterfaces times, with the following |
> +|           |        |            | 4 fields:                                         |
>   +-----------+--------+------------+---------------------------------------------------+
>   |           | 1      |            | bInterfaceClass                                   |
>   +-----------+--------+------------+---------------------------------------------------+
> @@ -177,7 +206,7 @@ OP_REP_DEVLIST:
>   | 0x147     | 1      |            | padding byte for alignment, shall be set to zero  |
>   +-----------+--------+------------+---------------------------------------------------+
>   | 0xC +     |        |            | The second exported USB device starts at i=1      |
> -| i*0x138 + |        |            | with the busid field.                             |
> +| i*0x138 + |        |            | with the path field.                              |
>   | m_(i-1)*4 |        |            |                                                   |
>   +-----------+--------+------------+---------------------------------------------------+
>   
> @@ -187,7 +216,7 @@ OP_REQ_IMPORT:
>   +-----------+--------+------------+---------------------------------------------------+
>   | Offset    | Length | Value      | Description                                       |
>   +===========+========+============+===================================================+
> -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
> +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
>   +-----------+--------+------------+---------------------------------------------------+
>   | 2         | 2      | 0x8003     | Command code: import a remote USB device.         |
>   +-----------+--------+------------+---------------------------------------------------+
> @@ -206,7 +235,7 @@ OP_REP_IMPORT:
>   +-----------+--------+------------+---------------------------------------------------+
>   | Offset    | Length | Value      | Description                                       |
>   +===========+========+============+===================================================+
> -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
> +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
>   +-----------+--------+------------+---------------------------------------------------+
>   | 2         | 2      | 0x0003     | Reply code: Reply to import.                      |
>   +-----------+--------+------------+---------------------------------------------------+
> @@ -254,158 +283,151 @@ OP_REP_IMPORT:
>   | 0x13E     | 1      |            | bNumInterfaces                                    |
>   +-----------+--------+------------+---------------------------------------------------+
>   
> -USBIP_CMD_SUBMIT:
> -	Submit an URB
> +The following four commands have a common basic header called
> +'usbip_header_basic', and their headers, called 'usbip_header' (before URB
> +payload), have the same length, therefore paddings are needed.
>   
> -+-----------+--------+------------+---------------------------------------------------+
> -| Offset    | Length | Value      | Description                                       |
> -+===========+========+============+===================================================+
> -| 0         | 4      | 0x00000001 | command: Submit an URB                            |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4         | 4      |            | seqnum: the sequence number of the URB to submit  |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8         | 4      |            | devid                                             |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC       | 4      |            | direction:                                        |
> -|           |        |            |                                                   |
> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> -|           |        |            |    - 1: USBIP_DIR_IN                              |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10      | 4      |            | ep: endpoint number, possible values are: 0...15  |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x14      | 4      |            | transfer_flags: possible values depend on the     |
> -|           |        |            | URB transfer type, see below                      |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x18      | 4      |            | transfer_buffer_length                            |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x1C      | 4      |            | start_frame: specify the selected frame to        |
> -|           |        |            | transmit an ISO frame, ignored if URB_ISO_ASAP    |
> -|           |        |            | is specified at transfer_flags                    |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x24      | 4      |            | interval: maximum time for the request on the     |
> -|           |        |            | server-side host controller                       |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
> -|           |        |            | zeros if not used                                 |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x30      |        |            | URB data. For ISO transfers the padding between   |
> -|           |        |            | each ISO packets is not transmitted.              |
> -+-----------+--------+------------+---------------------------------------------------+
> +usbip_header_basic:
>   
> ++-----------+--------+---------------------------------------------------+
> +| Offset    | Length | Description                                       |
> ++===========+========+===================================================+
> +| 0         | 4      | command                                           |
> ++-----------+--------+---------------------------------------------------+
> +| 4         | 4      | seqnum: sequential number that identifies requests|
> +|           |        | and corresponding responses;                      |
> +|           |        | incremented per connection                        |
> ++-----------+--------+---------------------------------------------------+
> +| 8         | 4      | devid: specifies a remote USB device uniquely     |
> +|           |        | instead of busnum and devnum;                     |
> +|           |        | for client (request), this value is               |
> +|           |        | ((busnum << 16) | devnum);                        |
> +|           |        | for server (response), this shall be set to 0     |
> ++-----------+--------+---------------------------------------------------+
> +| 0xC       | 4      | direction:                                        |
> +|           |        |                                                   |
> +|           |        |    - 0: USBIP_DIR_OUT                             |
> +|           |        |    - 1: USBIP_DIR_IN                              |
> +|           |        |                                                   |
> +|           |        | only used by client, for server this shall be 0   |
> ++-----------+--------+---------------------------------------------------+
> +| 0x10      | 4      | ep: endpoint number                               |
> +|           |        | only used by client, for server this shall be 0;  |
> +|           |        | for UNLINK, this shall be 0                       |
> ++-----------+--------+---------------------------------------------------+
>   
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | Allowed transfer_flags  | value      | control | interrupt | bulk     | isochronous |
> - +=========================+============+=========+===========+==========+=============+
> - | URB_SHORT_NOT_OK        | 0x00000001 | only in | only in   | only in  | no          |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_ISO_ASAP            | 0x00000002 | no      | no        | no       | yes         |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes     | yes       | yes      | yes         |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_ZERO_PACKET         | 0x00000040 | no      | no        | only out | no          |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_NO_INTERRUPT        | 0x00000080 | yes     | yes       | yes      | yes         |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_FREE_BUFFER         | 0x00000100 | yes     | yes       | yes      | yes         |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> - | URB_DIR_MASK            | 0x00000200 | yes     | yes       | yes      | yes         |
> - +-------------------------+------------+---------+-----------+----------+-------------+
> +USBIP_CMD_SUBMIT:
> +	Submit an URB
>   
> ++-----------+--------+---------------------------------------------------+
> +| Offset    | Length | Description                                       |
> ++===========+========+===================================================+
> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000001 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x14      | 4      | transfer_flags: possible values depend on the     |
> +|           |        | URB transfer_flags,                               |
> +|           |        | but with URB_NO_TRANSFER_DMA_MAP masked           |
> ++-----------+--------+---------------------------------------------------+
> +| 0x18      | 4      | transfer_buffer_length:                           |
> +|           |        | use URB transfer_buffer_length                    |
> ++-----------+--------+---------------------------------------------------+
> +| 0x1C      | 4      | start_frame: use URB start_frame;                 |
> +|           |        | initial frame for ISO transfer;                   |
> +|           |        | shall be set to 0 if not ISO transfer             |
> ++-----------+--------+---------------------------------------------------+
> +| 0x20      | 4      | number_of_packets: number of ISO packets;         |
> +|           |        | shall be set to 0xffffffff if not ISO transfer    |
> ++-----------+--------+---------------------------------------------------+
> +| 0x24      | 4      | interval: maximum time for the request on the     |
> +|           |        | server-side host controller                       |
> ++-----------+--------+---------------------------------------------------+
> +| 0x28      | 8      | setup: data bytes for USB setup, filled with      |
> +|           |        | zeros if not used.                                |
> ++-----------+--------+---------------------------------------------------+
> +| 0x30      | n      | transfer_buffer.                                  |
> +|           |        | If direction is USBIP_DIR_OUT then n equals       |
> +|           |        | transfer_buffer_length; otherwise n equals 0.     |
> +|           |        | For ISO transfers the padding between each ISO    |
> +|           |        | packets is not transmitted.                       |
> ++-----------+--------+---------------------------------------------------+
> +| 0x30+n    | m      | iso_packet_descriptor                             |
> ++-----------+--------+---------------------------------------------------+
>   
>   USBIP_RET_SUBMIT:
>   	Reply for submitting an URB
>   
> -+-----------+--------+------------+---------------------------------------------------+
> -| Offset    | Length | Value      | Description                                       |
> -+===========+========+============+===================================================+
> -| 0         | 4      | 0x00000003 | command                                           |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4         | 4      |            | seqnum: URB sequence number                       |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8         | 4      |            | devid                                             |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC       | 4      |            | direction:                                        |
> -|           |        |            |                                                   |
> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> -|           |        |            |    - 1: USBIP_DIR_IN                              |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10      | 4      |            | ep: endpoint number                               |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x14      | 4      |            | status: zero for successful URB transaction,      |
> -|           |        |            | otherwise some kind of error happened.            |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x18      | 4      | n          | actual_length: number of URB data bytes           |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x1C      | 4      |            | start_frame: for an ISO frame the actually        |
> -|           |        |            | selected frame for transmit.                      |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x20      | 4      |            | number_of_packets                                 |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x24      | 4      |            | error_count                                       |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
> -|           |        |            | zeros if not used                                 |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
> -|           |        |            | between each ISO packets is not transmitted.      |
> -+-----------+--------+------------+---------------------------------------------------+
> ++-----------+--------+---------------------------------------------------+
> +| Offset    | Length | Description                                       |
> ++===========+========+===================================================+
> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000003 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x14      | 4      | status: zero for successful URB transaction,      |
> +|           |        | otherwise some kind of error happened.            |
> ++-----------+--------+---------------------------------------------------+
> +| 0x18      | 4      | actual_length: number of URB data bytes;          |
> +|           |        | use URB actual_length                             |
> ++-----------+--------+---------------------------------------------------+
> +| 0x1C      | 4      | start_frame: use URB start_frame;                 |
> +|           |        | initial frame for ISO transfer;                   |
> +|           |        | shall be set to 0 if not ISO transfer             |
> ++-----------+--------+---------------------------------------------------+
> +| 0x20      | 4      | number_of_packets: number of ISO packets;         |
> +|           |        | shall be set to 0xffffffff if not ISO transfer    |
> ++-----------+--------+---------------------------------------------------+
> +| 0x24      | 4      | error_count                                       |
> ++-----------+--------+---------------------------------------------------+
> +| 0x28      | 8      | padding, shall be set to 0                        |
> ++-----------+--------+---------------------------------------------------+
> +| 0x30      | n      | transfer_buffer.                                  |
> +|           |        | If direction is USBIP_DIR_IN then n equals        |
> +|           |        | actual_length; otherwise n equals 0.              |
> +|           |        | For ISO transfers the padding between each ISO    |
> +|           |        | packets is not transmitted.                       |
> ++-----------+--------+---------------------------------------------------+
> +| 0x30+n    | m      | iso_packet_descriptor                             |
> ++-----------+--------+---------------------------------------------------+
>   
>   USBIP_CMD_UNLINK:
>   	Unlink an URB
>   
> -+-----------+--------+------------+---------------------------------------------------+
> -| Offset    | Length | Value      | Description                                       |
> -+===========+========+============+===================================================+
> -| 0         | 4      | 0x00000002 | command: URB unlink command                       |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4         | 4      |            | seqnum: URB sequence number to unlink:            |
> -|           |        |            |                                                   |
> -|           |        |            | FIXME:                                            |
> -|           |        |            |    is this so?                                    |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8         | 4      |            | devid                                             |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC       | 4      |            | direction:                                        |
> -|           |        |            |                                                   |
> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> -|           |        |            |    - 1: USBIP_DIR_IN                              |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10      | 4      |            | ep: endpoint number: zero                         |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x14      | 4      |            | seqnum: the URB sequence number given previously  |
> -|           |        |            | at USBIP_CMD_SUBMIT.seqnum field                  |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
> -|           |        |            | between each ISO packets is not transmitted.      |
> -+-----------+--------+------------+---------------------------------------------------+
> ++-----------+--------+---------------------------------------------------+
> +| Offset    | Length | Description                                       |
> ++===========+========+===================================================+
> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000002 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x14      | 4      | unlink_seqnum, of the SUBMIT request to unlink    |
> ++-----------+--------+---------------------------------------------------+
> +| 0x18      | 24     | padding, shall be set to 0                        |
> ++-----------+--------+---------------------------------------------------+
>   
>   USBIP_RET_UNLINK:
>   	Reply for URB unlink
>   
> -+-----------+--------+------------+---------------------------------------------------+
> -| Offset    | Length | Value      | Description                                       |
> -+===========+========+============+===================================================+
> -| 0         | 4      | 0x00000004 | command: reply for the URB unlink command         |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 4         | 4      |            | seqnum: the unlinked URB sequence number          |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 8         | 4      |            | devid                                             |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0xC       | 4      |            | direction:                                        |
> -|           |        |            |                                                   |
> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> -|           |        |            |    - 1: USBIP_DIR_IN                              |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x10      | 4      |            | ep: endpoint number                               |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x14      | 4      |            | status: This is the value contained in the        |
> -|           |        |            | urb->status in the URB completition handler.      |
> -|           |        |            |                                                   |
> -|           |        |            | FIXME:                                            |
> -|           |        |            |      a better explanation needed.                 |
> -+-----------+--------+------------+---------------------------------------------------+
> -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
> -|           |        |            | between each ISO packets is not transmitted.      |
> -+-----------+--------+------------+---------------------------------------------------+
> ++-----------+--------+---------------------------------------------------+
> +| Offset    | Length | Description                                       |
> ++===========+========+===================================================+
> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000004 |
> ++-----------+--------+---------------------------------------------------+
> +| 0x14      | 4      | status: This is similar to the status of          |
> +|           |        | USBIP_RET_SUBMIT (share the same memory offset).  |
> +|           |        | When UNLINK is successful, status is -ECONNRESET; |
> +|           |        | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT   |
> +|           |        | status is 0                                       |
> ++-----------+--------+---------------------------------------------------+
> +| 0x18      | 24     | padding, shall be set to 0                        |
> ++-----------+--------+---------------------------------------------------+
> +
> +EXAMPLE
> +=======
> +
> +  The following data is captured from wire with Human Interface Devices (HID)
> +  payload
> +
> +::
> +
> +  CmdIntrIN:  00000001 00000d05 0001000f 00000001 00000001 00000200 00000040 ffffffff 00000000 00000004 00000000 00000000
> +  CmdIntrOUT: 00000001 00000d06 0001000f 00000000 00000001 00000000 00000040 ffffffff 00000000 00000004 00000000 00000000
> +              ffffffff860008a784ce5ae212376300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> +  RetIntrOut: 00000003 00000d06 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
> +  RetIntrIn:  00000003 00000d05 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
> +              ffffffff860011a784ce5ae2123763612891b1020100000400000000000000000000000000000000000000000000000000000000000000000000000000000000
> 


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

* Re: Re: [PATCH v5] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-29 20:06           ` Shuah Khan
@ 2021-03-30 12:35             ` Hongren Zheng (Zenithal)
  2021-03-30 14:57               ` Shuah Khan
  0 siblings, 1 reply; 14+ messages in thread
From: Hongren Zheng (Zenithal) @ 2021-03-30 12:35 UTC (permalink / raw)
  To: Shuah Khan, Valentina Manea, Greg Kroah-Hartman,
	Márton Németh, Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, usbip-devel, Randy Dunlap

On Mon, Mar 29, 2021 at 02:06:56PM -0600, Shuah Khan wrote:
> On 3/15/21 8:25 PM, Hongren Zheng (Zenithal) wrote:
> > The old document for usbip protocol is misleading and hard to read:
> >    * Some fields in header are incorrect
> >    * Explanation of some fields are unclear or even wrong
> >    * Padding of header (namely all headers have the same length) is
> >      not explicitly pointed out, which is crucial for stream protocol
> >      like TCP
> > 
> > Major changes:
> >    * Document the correct field as described in the codebase.
> >    * Document the padding in usbip headers. This is crucial for TCP
> >      stream hence these padding should be explicitly point out.
> >      In code these padding are implemented by a union of all headers.
> >    * Fix two FIXME related to usbip unlink and Document the behavior
> >      of unlink in different situation.
> >    * Clarify some field with more accurate explanation, like those
> >      fields associated with URB. Some constraints are extracted from
> >      code.
> >    * Delete specific transfer_flag doc in usbip as it should be
> >      documented by the URB part.
> 
> Why are we deleting this. What do you mean documented by URB part?

transfer_flag are actually defined by the URB struct,
refer to https://www.kernel.org/doc/html/latest/driver-api/usb/URB.html
and usbip headers just encapsulate this flag.
You may refer to usbip_pack_cmd_submit in
/drivers/usb/usbip/usbip_common.c and notice that 
the flag are just copied from one URB with a small tweak.
(Note that this tweak is also documented below).

Hence the possible transfer flags for each transfer type
should be explained by URB document, not USB/IP document.
I think documenting them here is duplicated, hence I deleted
them.

> 
> >    * Add data captured from wire as example
> > 
> > Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> > Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
> > Signed-off-by: Hongren Zheng <i@zenithal.me>
> > ---
> >   Documentation/usb/usbip_protocol.rst | 320 ++++++++++++++-------------
> >   1 file changed, 171 insertions(+), 149 deletions(-)
> > 
> > PATCH v2:
> > Some changes suggested by a previous patch in
> > https://lore.kernel.org/linux-usb
> > /20180128071514.9107-1-alexandre.f.demers@gmail.com/
> > is adopted in this patch.
> >    * Fix Typo: duplicated 'the' in 'the following 4 field'
> >    * Fix incorrect field: in OP_REQ_DEVLIST, the second dev starts with
> >      field 'path', not 'busid'
> > 
> > PATCH v3:
> > Suggested by
> > https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
> >    * Remove date and changelog in doc as these are tracked in git history
> >    * Remove 'mistake alert' as all data fields are documented properly
> >      now. However, docs on possible values for some field shall be added
> >      in the future
> > 
> > PATCH v4:
> > Suggested by https://lore.kernel.org/linux-doc
> > /40351ed6-2907-3966-e69a-a564173b3682@infradead.org/
> >    * Add punctuations for readability
> >    * Move patch changelog after the marker line
> >    * Remove nickname in signed-off-by line
> > 
> > PATCH v5:
> >    * Instead of co-developed-by, use reviewed-by
> >      for Randy Dunlap
> > 
> > diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
> > index 988c832166cd..54c5677adf4e 100644
> > --- a/Documentation/usb/usbip_protocol.rst
> > +++ b/Documentation/usb/usbip_protocol.rst
> > @@ -2,11 +2,11 @@
> >   USB/IP protocol
> >   ===============
> >    > -PRELIMINARY DRAFT, MAY CONTAIN MISTAKES> -28 Jun 2011
> > +Architecture
> 
> Let's add doc version preserving the history.
> Add Version 1 and date perhaps.

GKH suggested tracking version by git changlog, so I deleted
these fields. Refer to
https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/

> 
> > +============
> >   The USB/IP protocol follows a server/client architecture. The server exports the
> > -USB devices and the clients imports them. The device driver for the exported
> > +USB devices and the client imports them. The device driver for the exported
> 
> clients import them.

Will be adopted in the next version of the PATCH

> 
> >   USB device runs on the client machine.
> >   The client may ask for the list of the exported USB devices. To get the list the
> > @@ -37,6 +37,9 @@ to transfer the URB traffic between the client and the server. The client may
> >   send two types of packets: the USBIP_CMD_SUBMIT to submit an URB, and
> >   USBIP_CMD_UNLINK to unlink a previously submitted URB. The answers of the
> >   server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
> > +Note that after successful USBIP_RET_UNLINK, the unlinked URB request would not
> > +have a corresponding USBIP_RET_UNLINK (this is explained in
> > +drivers/usb/usbip/stub_rx.c).
> 
> This is not clear to me. Doesn't look correct. Where do you see this
> in drivers/usb/usbip/stub_rx.c?

Sorry that this is a typo. I meant "the unlinked URB request would not
have a corresponding USBIP_RET_SUBMIT".
This is shown in the following flow by the "<-X--X--" line.
This will be fixed in the next version of this PATCH.

The explanation is in the comments of function stub_recv_cmd_unlink.
"The unlinking flag means that we are now **not** going to return the normal
result pdu of a submission request, **but** going to return a result pdu of
the unlink request"
This behavior is implemented by **replacing** the normal result pdu
with a unlink result pdu. You may walk through this function to 
understand the behavior.
So should I add this function name in the document for clarity?

> 
> >   ::
> > @@ -85,16 +88,42 @@ server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
> >             |                        .                        |
> >             |                        :                        |
> >             |                                                 |
> > +          |            USBIP_CMD_SUBMIT(seqnum = p)         |
> > +          | ----------------------------------------------> |
> > +          |                                                 |
> > +          |               USBIP_CMD_UNLINK                  |
> > +          |         (seqnum = p+1, unlink_seqnum = p)       |
> > +          | ----------------------------------------------> |
> > +          |                                                 |
> > +          |               USBIP_RET_UNLINK                  |
> > +          |        (seqnum = p+1, status = -ECONNRESET)     |
> > +          | <---------------------------------------------- |
> > +          |                                                 |
> > +          |         Note: No USBIP_RET_SUBMIT(seqnum = p)   |
> > +          | <--X---X---X---X---X---X---X---X---X---X---X--- |
> > +          |                        .                        |
> > +          |                        :                        |
> > +          |                                                 |
> > +          |            USBIP_CMD_SUBMIT(seqnum = q)         |
> > +          | ----------------------------------------------> |
> > +          |                                                 |
> > +          |            USBIP_RET_SUBMIT(seqnum = q)         |
> > +          | <---------------------------------------------- |
> > +          |                                                 |
> >             |               USBIP_CMD_UNLINK                  |
> > +          |         (seqnum = q+1, unlink_seqnum = q)       |
> >             | ----------------------------------------------> |
> >             |                                                 |
> >             |               USBIP_RET_UNLINK                  |
> > +          |           (seqnum = q+1, status = 0)            |
> >             | <---------------------------------------------- |
> >             |                                                 |
> 
> I would do this differently. Let's add this as an expanded
> USBIP_CMD_UNLINK sequence with a separate heading below the
> current flow

Currently we have two flows, one for device list and another for
device import and URB transfer.
I think that UNLINK is a common command within the latter flow,
namely SUBMIT and UNLINK are interweaved in the whole flow,
and separating UNLINK from SUBMIT would cause logical separation
as if they follows different flow.

I think what I have documented is the full use case for UNLINK,
so I wonder what do you mean by "expanded sequence".

> 
> >   The fields are in network (big endian) byte order meaning that the most significant
> >   byte (MSB) is stored at the lowest address.
> > +Message Format
> > +==============
> >   OP_REQ_DEVLIST:
> >   	Retrieve the list of exported USB devices.
> > @@ -102,7 +131,7 @@ OP_REQ_DEVLIST:
> >   +-----------+--------+------------+---------------------------------------------------+
> >   | Offset    | Length | Value      | Description                                       |
> >   +===========+========+============+===================================================+
> > -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
> > +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
> >   +-----------+--------+------------+---------------------------------------------------+
> 
> Let's make this vx.x.x and specify current version number at the top.
> Saves us doc updates as we change version number - one location change
> as opposed to the entire document

Will be adopted in the next version of the PATCH.

A sidenote though, the version number has not changed since the 
release (out of staging) of usbip.

> >   | 2         | 2      | 0x8005     | Command code: Retrieve the list of exported USB   |
> >   |           |        |            | devices.                                          |
> > @@ -116,7 +145,7 @@ OP_REP_DEVLIST:
> >   +-----------+--------+------------+---------------------------------------------------+
> >   | Offset    | Length | Value      | Description                                       |
> >   +===========+========+============+===================================================+
> > -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0.|
> > +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
> >   +-----------+--------+------------+---------------------------------------------------+
> >   | 2         | 2      | 0x0005     | Reply code: The list of exported USB devices.     |
> >   +-----------+--------+------------+---------------------------------------------------+
> > @@ -165,8 +194,8 @@ OP_REP_DEVLIST:
> >   | 0x143     | 1      |            | bNumInterfaces                                    |
> >   +-----------+--------+------------+---------------------------------------------------+
> >   | 0x144     |        | m_0        | From now on each interface is described, all      |
> > -|           |        |            | together bNumInterfaces times, with the           |
> > -|           |        |            | the following 4 fields:                           |
> > +|           |        |            | together bNumInterfaces times, with the following |
> > +|           |        |            | 4 fields:                                         |
> >   +-----------+--------+------------+---------------------------------------------------+
> >   |           | 1      |            | bInterfaceClass                                   |
> >   +-----------+--------+------------+---------------------------------------------------+
> > @@ -177,7 +206,7 @@ OP_REP_DEVLIST:
> >   | 0x147     | 1      |            | padding byte for alignment, shall be set to zero  |
> >   +-----------+--------+------------+---------------------------------------------------+
> >   | 0xC +     |        |            | The second exported USB device starts at i=1      |
> > -| i*0x138 + |        |            | with the busid field.                             |
> > +| i*0x138 + |        |            | with the path field.                              |
> >   | m_(i-1)*4 |        |            |                                                   |
> >   +-----------+--------+------------+---------------------------------------------------+
> > @@ -187,7 +216,7 @@ OP_REQ_IMPORT:
> >   +-----------+--------+------------+---------------------------------------------------+
> >   | Offset    | Length | Value      | Description                                       |
> >   +===========+========+============+===================================================+
> > -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
> > +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
> >   +-----------+--------+------------+---------------------------------------------------+
> >   | 2         | 2      | 0x8003     | Command code: import a remote USB device.         |
> >   +-----------+--------+------------+---------------------------------------------------+
> > @@ -206,7 +235,7 @@ OP_REP_IMPORT:
> >   +-----------+--------+------------+---------------------------------------------------+
> >   | Offset    | Length | Value      | Description                                       |
> >   +===========+========+============+===================================================+
> > -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
> > +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
> >   +-----------+--------+------------+---------------------------------------------------+
> >   | 2         | 2      | 0x0003     | Reply code: Reply to import.                      |
> >   +-----------+--------+------------+---------------------------------------------------+
> > @@ -254,158 +283,151 @@ OP_REP_IMPORT:
> >   | 0x13E     | 1      |            | bNumInterfaces                                    |
> >   +-----------+--------+------------+---------------------------------------------------+
> > -USBIP_CMD_SUBMIT:
> > -	Submit an URB
> > +The following four commands have a common basic header called
> > +'usbip_header_basic', and their headers, called 'usbip_header' (before URB
> > +payload), have the same length, therefore paddings are needed.
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| Offset    | Length | Value      | Description                                       |
> > -+===========+========+============+===================================================+
> > -| 0         | 4      | 0x00000001 | command: Submit an URB                            |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 4         | 4      |            | seqnum: the sequence number of the URB to submit  |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 8         | 4      |            | devid                                             |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0xC       | 4      |            | direction:                                        |
> > -|           |        |            |                                                   |
> > -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> > -|           |        |            |    - 1: USBIP_DIR_IN                              |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x10      | 4      |            | ep: endpoint number, possible values are: 0...15  |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x14      | 4      |            | transfer_flags: possible values depend on the     |
> > -|           |        |            | URB transfer type, see below                      |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x18      | 4      |            | transfer_buffer_length                            |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x1C      | 4      |            | start_frame: specify the selected frame to        |
> > -|           |        |            | transmit an ISO frame, ignored if URB_ISO_ASAP    |
> > -|           |        |            | is specified at transfer_flags                    |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x24      | 4      |            | interval: maximum time for the request on the     |
> > -|           |        |            | server-side host controller                       |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
> > -|           |        |            | zeros if not used                                 |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x30      |        |            | URB data. For ISO transfers the padding between   |
> > -|           |        |            | each ISO packets is not transmitted.              |
> > -+-----------+--------+------------+---------------------------------------------------+
> > +usbip_header_basic:
> > ++-----------+--------+---------------------------------------------------+
> > +| Offset    | Length | Description                                       |
> > ++===========+========+===================================================+
> > +| 0         | 4      | command                                           |
> > ++-----------+--------+---------------------------------------------------+
> > +| 4         | 4      | seqnum: sequential number that identifies requests|
> > +|           |        | and corresponding responses;                      |
> > +|           |        | incremented per connection                        |
> > ++-----------+--------+---------------------------------------------------+
> > +| 8         | 4      | devid: specifies a remote USB device uniquely     |
> > +|           |        | instead of busnum and devnum;                     |
> > +|           |        | for client (request), this value is               |
> > +|           |        | ((busnum << 16) | devnum);                        |
> > +|           |        | for server (response), this shall be set to 0     |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0xC       | 4      | direction:                                        |
> > +|           |        |                                                   |
> > +|           |        |    - 0: USBIP_DIR_OUT                             |
> > +|           |        |    - 1: USBIP_DIR_IN                              |
> > +|           |        |                                                   |
> > +|           |        | only used by client, for server this shall be 0   |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x10      | 4      | ep: endpoint number                               |
> > +|           |        | only used by client, for server this shall be 0;  |
> > +|           |        | for UNLINK, this shall be 0                       |
> > ++-----------+--------+---------------------------------------------------+
> > - +-------------------------+------------+---------+-----------+----------+-------------+
> > - | Allowed transfer_flags  | value      | control | interrupt | bulk     | isochronous |
> > - +=========================+============+=========+===========+==========+=============+
> > - | URB_SHORT_NOT_OK        | 0x00000001 | only in | only in   | only in  | no          |
> > - +-------------------------+------------+---------+-----------+----------+-------------+
> > - | URB_ISO_ASAP            | 0x00000002 | no      | no        | no       | yes         |
> > - +-------------------------+------------+---------+-----------+----------+-------------+
> > - | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes     | yes       | yes      | yes         |
> > - +-------------------------+------------+---------+-----------+----------+-------------+
> > - | URB_ZERO_PACKET         | 0x00000040 | no      | no        | only out | no          |
> > - +-------------------------+------------+---------+-----------+----------+-------------+
> > - | URB_NO_INTERRUPT        | 0x00000080 | yes     | yes       | yes      | yes         |
> > - +-------------------------+------------+---------+-----------+----------+-------------+
> > - | URB_FREE_BUFFER         | 0x00000100 | yes     | yes       | yes      | yes         |
> > - +-------------------------+------------+---------+-----------+----------+-------------+
> > - | URB_DIR_MASK            | 0x00000200 | yes     | yes       | yes      | yes         |
> > - +-------------------------+------------+---------+-----------+----------+-------------+
> > +USBIP_CMD_SUBMIT:
> > +	Submit an URB
> > ++-----------+--------+---------------------------------------------------+
> > +| Offset    | Length | Description                                       |
> > ++===========+========+===================================================+
> > +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000001 |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x14      | 4      | transfer_flags: possible values depend on the     |
> > +|           |        | URB transfer_flags,                               |
> > +|           |        | but with URB_NO_TRANSFER_DMA_MAP masked           |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x18      | 4      | transfer_buffer_length:                           |
> > +|           |        | use URB transfer_buffer_length                    |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x1C      | 4      | start_frame: use URB start_frame;                 |
> > +|           |        | initial frame for ISO transfer;                   |
> > +|           |        | shall be set to 0 if not ISO transfer             |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x20      | 4      | number_of_packets: number of ISO packets;         |
> > +|           |        | shall be set to 0xffffffff if not ISO transfer    |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x24      | 4      | interval: maximum time for the request on the     |
> > +|           |        | server-side host controller                       |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x28      | 8      | setup: data bytes for USB setup, filled with      |
> > +|           |        | zeros if not used.                                |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x30      | n      | transfer_buffer.                                  |
> > +|           |        | If direction is USBIP_DIR_OUT then n equals       |
> > +|           |        | transfer_buffer_length; otherwise n equals 0.     |
> > +|           |        | For ISO transfers the padding between each ISO    |
> > +|           |        | packets is not transmitted.                       |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x30+n    | m      | iso_packet_descriptor                             |
> > ++-----------+--------+---------------------------------------------------+
> >   USBIP_RET_SUBMIT:
> >   	Reply for submitting an URB
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| Offset    | Length | Value      | Description                                       |
> > -+===========+========+============+===================================================+
> > -| 0         | 4      | 0x00000003 | command                                           |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 4         | 4      |            | seqnum: URB sequence number                       |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 8         | 4      |            | devid                                             |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0xC       | 4      |            | direction:                                        |
> > -|           |        |            |                                                   |
> > -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> > -|           |        |            |    - 1: USBIP_DIR_IN                              |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x10      | 4      |            | ep: endpoint number                               |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x14      | 4      |            | status: zero for successful URB transaction,      |
> > -|           |        |            | otherwise some kind of error happened.            |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x18      | 4      | n          | actual_length: number of URB data bytes           |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x1C      | 4      |            | start_frame: for an ISO frame the actually        |
> > -|           |        |            | selected frame for transmit.                      |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x20      | 4      |            | number_of_packets                                 |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x24      | 4      |            | error_count                                       |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
> > -|           |        |            | zeros if not used                                 |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
> > -|           |        |            | between each ISO packets is not transmitted.      |
> > -+-----------+--------+------------+---------------------------------------------------+
> > ++-----------+--------+---------------------------------------------------+
> > +| Offset    | Length | Description                                       |
> > ++===========+========+===================================================+
> > +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000003 |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x14      | 4      | status: zero for successful URB transaction,      |
> > +|           |        | otherwise some kind of error happened.            |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x18      | 4      | actual_length: number of URB data bytes;          |
> > +|           |        | use URB actual_length                             |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x1C      | 4      | start_frame: use URB start_frame;                 |
> > +|           |        | initial frame for ISO transfer;                   |
> > +|           |        | shall be set to 0 if not ISO transfer             |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x20      | 4      | number_of_packets: number of ISO packets;         |
> > +|           |        | shall be set to 0xffffffff if not ISO transfer    |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x24      | 4      | error_count                                       |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x28      | 8      | padding, shall be set to 0                        |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x30      | n      | transfer_buffer.                                  |
> > +|           |        | If direction is USBIP_DIR_IN then n equals        |
> > +|           |        | actual_length; otherwise n equals 0.              |
> > +|           |        | For ISO transfers the padding between each ISO    |
> > +|           |        | packets is not transmitted.                       |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x30+n    | m      | iso_packet_descriptor                             |
> > ++-----------+--------+---------------------------------------------------+
> >   USBIP_CMD_UNLINK:
> >   	Unlink an URB
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| Offset    | Length | Value      | Description                                       |
> > -+===========+========+============+===================================================+
> > -| 0         | 4      | 0x00000002 | command: URB unlink command                       |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 4         | 4      |            | seqnum: URB sequence number to unlink:            |
> > -|           |        |            |                                                   |
> > -|           |        |            | FIXME:                                            |
> > -|           |        |            |    is this so?                                    |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 8         | 4      |            | devid                                             |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0xC       | 4      |            | direction:                                        |
> > -|           |        |            |                                                   |
> > -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> > -|           |        |            |    - 1: USBIP_DIR_IN                              |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x10      | 4      |            | ep: endpoint number: zero                         |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x14      | 4      |            | seqnum: the URB sequence number given previously  |
> > -|           |        |            | at USBIP_CMD_SUBMIT.seqnum field                  |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
> > -|           |        |            | between each ISO packets is not transmitted.      |
> > -+-----------+--------+------------+---------------------------------------------------+
> > ++-----------+--------+---------------------------------------------------+
> > +| Offset    | Length | Description                                       |
> > ++===========+========+===================================================+
> > +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000002 |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x14      | 4      | unlink_seqnum, of the SUBMIT request to unlink    |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x18      | 24     | padding, shall be set to 0                        |
> > ++-----------+--------+---------------------------------------------------+
> >   USBIP_RET_UNLINK:
> >   	Reply for URB unlink
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| Offset    | Length | Value      | Description                                       |
> > -+===========+========+============+===================================================+
> > -| 0         | 4      | 0x00000004 | command: reply for the URB unlink command         |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 4         | 4      |            | seqnum: the unlinked URB sequence number          |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 8         | 4      |            | devid                                             |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0xC       | 4      |            | direction:                                        |
> > -|           |        |            |                                                   |
> > -|           |        |            |    - 0: USBIP_DIR_OUT                             |
> > -|           |        |            |    - 1: USBIP_DIR_IN                              |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x10      | 4      |            | ep: endpoint number                               |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x14      | 4      |            | status: This is the value contained in the        |
> > -|           |        |            | urb->status in the URB completition handler.      |
> > -|           |        |            |                                                   |
> > -|           |        |            | FIXME:                                            |
> > -|           |        |            |      a better explanation needed.                 |
> > -+-----------+--------+------------+---------------------------------------------------+
> > -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
> > -|           |        |            | between each ISO packets is not transmitted.      |
> > -+-----------+--------+------------+---------------------------------------------------+
> > ++-----------+--------+---------------------------------------------------+
> > +| Offset    | Length | Description                                       |
> > ++===========+========+===================================================+
> > +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000004 |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x14      | 4      | status: This is similar to the status of          |
> > +|           |        | USBIP_RET_SUBMIT (share the same memory offset).  |
> > +|           |        | When UNLINK is successful, status is -ECONNRESET; |
> > +|           |        | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT   |
> > +|           |        | status is 0                                       |
> > ++-----------+--------+---------------------------------------------------+
> > +| 0x18      | 24     | padding, shall be set to 0                        |
> > ++-----------+--------+---------------------------------------------------+
> > +
> > +EXAMPLE
> > +=======
> > +
> > +  The following data is captured from wire with Human Interface Devices (HID)
> > +  payload
> > +
> > +::
> > +
> > +  CmdIntrIN:  00000001 00000d05 0001000f 00000001 00000001 00000200 00000040 ffffffff 00000000 00000004 00000000 00000000
> > +  CmdIntrOUT: 00000001 00000d06 0001000f 00000000 00000001 00000000 00000040 ffffffff 00000000 00000004 00000000 00000000
> > +              ffffffff860008a784ce5ae212376300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
> > +  RetIntrOut: 00000003 00000d06 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
> > +  RetIntrIn:  00000003 00000d05 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
> > +              ffffffff860011a784ce5ae2123763612891b1020100000400000000000000000000000000000000000000000000000000000000000000000000000000000000
> > 
> 

I will submit the next version of the PATCH after all the conversations above
resolved.

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

* Re: [PATCH v5] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-30 12:35             ` Hongren Zheng (Zenithal)
@ 2021-03-30 14:57               ` Shuah Khan
  0 siblings, 0 replies; 14+ messages in thread
From: Shuah Khan @ 2021-03-30 14:57 UTC (permalink / raw)
  To: Hongren Zheng (Zenithal),
	Valentina Manea, Greg Kroah-Hartman, Márton Németh,
	Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, usbip-devel, Randy Dunlap, Shuah Khan

On 3/30/21 6:35 AM, Hongren Zheng (Zenithal) wrote:
> On Mon, Mar 29, 2021 at 02:06:56PM -0600, Shuah Khan wrote:
>> On 3/15/21 8:25 PM, Hongren Zheng (Zenithal) wrote:
>>> The old document for usbip protocol is misleading and hard to read:
>>>     * Some fields in header are incorrect
>>>     * Explanation of some fields are unclear or even wrong
>>>     * Padding of header (namely all headers have the same length) is
>>>       not explicitly pointed out, which is crucial for stream protocol
>>>       like TCP
>>>
>>> Major changes:
>>>     * Document the correct field as described in the codebase.
>>>     * Document the padding in usbip headers. This is crucial for TCP
>>>       stream hence these padding should be explicitly point out.
>>>       In code these padding are implemented by a union of all headers.
>>>     * Fix two FIXME related to usbip unlink and Document the behavior
>>>       of unlink in different situation.
>>>     * Clarify some field with more accurate explanation, like those
>>>       fields associated with URB. Some constraints are extracted from
>>>       code.
>>>     * Delete specific transfer_flag doc in usbip as it should be
>>>       documented by the URB part.
>>
>> Why are we deleting this. What do you mean documented by URB part?
> 
> transfer_flag are actually defined by the URB struct,
> refer to https://www.kernel.org/doc/html/latest/driver-api/usb/URB.html
> and usbip headers just encapsulate this flag.
> You may refer to usbip_pack_cmd_submit in
> /drivers/usb/usbip/usbip_common.c and notice that
> the flag are just copied from one URB with a small tweak.
> (Note that this tweak is also documented below).
> 

It makes sense to not duplicate the information. Please add a reference
to the URB doc here. Does the tweak require context? It would make sense
to add the reference to URB doc and then call out the tweak.

> Hence the possible transfer flags for each transfer type
> should be explained by URB document, not USB/IP document.
> I think documenting them here is duplicated, hence I deleted
> them.
> 
>>
>>>     * Add data captured from wire as example
>>>
>>> Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
>>> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
>>> Signed-off-by: Hongren Zheng <i@zenithal.me>
>>> ---
>>>    Documentation/usb/usbip_protocol.rst | 320 ++++++++++++++-------------
>>>    1 file changed, 171 insertions(+), 149 deletions(-)
>>>
>>> PATCH v2:
>>> Some changes suggested by a previous patch in
>>> https://lore.kernel.org/linux-usb
>>> /20180128071514.9107-1-alexandre.f.demers@gmail.com/
>>> is adopted in this patch.
>>>     * Fix Typo: duplicated 'the' in 'the following 4 field'
>>>     * Fix incorrect field: in OP_REQ_DEVLIST, the second dev starts with
>>>       field 'path', not 'busid'
>>>
>>> PATCH v3:
>>> Suggested by
>>> https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
>>>     * Remove date and changelog in doc as these are tracked in git history
>>>     * Remove 'mistake alert' as all data fields are documented properly
>>>       now. However, docs on possible values for some field shall be added
>>>       in the future
>>>
>>> PATCH v4:
>>> Suggested by https://lore.kernel.org/linux-doc
>>> /40351ed6-2907-3966-e69a-a564173b3682@infradead.org/
>>>     * Add punctuations for readability
>>>     * Move patch changelog after the marker line
>>>     * Remove nickname in signed-off-by line
>>>
>>> PATCH v5:
>>>     * Instead of co-developed-by, use reviewed-by
>>>       for Randy Dunlap
>>>
>>> diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
>>> index 988c832166cd..54c5677adf4e 100644
>>> --- a/Documentation/usb/usbip_protocol.rst
>>> +++ b/Documentation/usb/usbip_protocol.rst
>>> @@ -2,11 +2,11 @@
>>>    USB/IP protocol
>>>    ===============
>>>     > -PRELIMINARY DRAFT, MAY CONTAIN MISTAKES> -28 Jun 2011
>>> +Architecture
>>
>> Let's add doc version preserving the history.
>> Add Version 1 and date perhaps.
> 
> GKH suggested tracking version by git changlog, so I deleted
> these fields. Refer to
> https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
> 

Ah okay. Please add the version change in the change log.

>>
>>> +============
>>>    The USB/IP protocol follows a server/client architecture. The server exports the
>>> -USB devices and the clients imports them. The device driver for the exported
>>> +USB devices and the client imports them. The device driver for the exported
>>
>> clients import them.
> 
> Will be adopted in the next version of the PATCH
> 

While you are updating the doc, please change all instances of:

"connection towards the server"
to
"connection to the server"

>>
>>>    USB device runs on the client machine.
>>>    The client may ask for the list of the exported USB devices. To get the list the
>>> @@ -37,6 +37,9 @@ to transfer the URB traffic between the client and the server. The client may
>>>    send two types of packets: the USBIP_CMD_SUBMIT to submit an URB, and
>>>    USBIP_CMD_UNLINK to unlink a previously submitted URB. The answers of the
>>>    server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
>>> +Note that after successful USBIP_RET_UNLINK, the unlinked URB request would not
>>> +have a corresponding USBIP_RET_UNLINK (this is explained in
>>> +drivers/usb/usbip/stub_rx.c).
>>
>> This is not clear to me. Doesn't look correct. Where do you see this
>> in drivers/usb/usbip/stub_rx.c?
> 
> Sorry that this is a typo. I meant "the unlinked URB request would not
> have a corresponding USBIP_RET_SUBMIT".
> This is shown in the following flow by the "<-X--X--" line.
> This will be fixed in the next version of this PATCH.
> 

Okay that makes sense.

> The explanation is in the comments of function stub_recv_cmd_unlink.
> "The unlinking flag means that we are now **not** going to return the normal
> result pdu of a submission request, **but** going to return a result pdu of
> the unlink request"
> This behavior is implemented by **replacing** the normal result pdu
> with a unlink result pdu. You may walk through this function to
> understand the behavior.
> So should I add this function name in the document for clarity?
> 

Yes. Just confirming this is the one you are referring to. Yes. Please
call out the function if you are referencing it here.

>>
>>>    ::
>>> @@ -85,16 +88,42 @@ server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
>>>              |                        .                        |
>>>              |                        :                        |
>>>              |                                                 |
>>> +          |            USBIP_CMD_SUBMIT(seqnum = p)         |
>>> +          | ----------------------------------------------> |
>>> +          |                                                 |
>>> +          |               USBIP_CMD_UNLINK                  |
>>> +          |         (seqnum = p+1, unlink_seqnum = p)       |
>>> +          | ----------------------------------------------> |
>>> +          |                                                 |
>>> +          |               USBIP_RET_UNLINK                  |
>>> +          |        (seqnum = p+1, status = -ECONNRESET)     |
>>> +          | <---------------------------------------------- |
>>> +          |                                                 |
>>> +          |         Note: No USBIP_RET_SUBMIT(seqnum = p)   |
>>> +          | <--X---X---X---X---X---X---X---X---X---X---X--- |
>>> +          |                        .                        |
>>> +          |                        :                        |
>>> +          |                                                 |
>>> +          |            USBIP_CMD_SUBMIT(seqnum = q)         |
>>> +          | ----------------------------------------------> |
>>> +          |                                                 |
>>> +          |            USBIP_RET_SUBMIT(seqnum = q)         |
>>> +          | <---------------------------------------------- |
>>> +          |                                                 |
>>>              |               USBIP_CMD_UNLINK                  |
>>> +          |         (seqnum = q+1, unlink_seqnum = q)       |
>>>              | ----------------------------------------------> |
>>>              |                                                 |
>>>              |               USBIP_RET_UNLINK                  |
>>> +          |           (seqnum = q+1, status = 0)            |
>>>              | <---------------------------------------------- |
>>>              |                                                 |
>>
>> I would do this differently. Let's add this as an expanded
>> USBIP_CMD_UNLINK sequence with a separate heading below the
>> current flow
> 
> Currently we have two flows, one for device list and another for
> device import and URB transfer.
> I think that UNLINK is a common command within the latter flow,
> namely SUBMIT and UNLINK are interweaved in the whole flow,
> and separating UNLINK from SUBMIT would cause logical separation
> as if they follows different flow.
> 
> I think what I have documented is the full use case for UNLINK,
> so I wonder what do you mean by "expanded sequence".
> 

Let's expand this flow out separately. I think it will be easier
to follow.

>>
>>>    The fields are in network (big endian) byte order meaning that the most significant
>>>    byte (MSB) is stored at the lowest address.
>>> +Message Format
>>> +==============
>>>    OP_REQ_DEVLIST:
>>>    	Retrieve the list of exported USB devices.
>>> @@ -102,7 +131,7 @@ OP_REQ_DEVLIST:
>>>    +-----------+--------+------------+---------------------------------------------------+
>>>    | Offset    | Length | Value      | Description                                       |
>>>    +===========+========+============+===================================================+
>>> -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
>>> +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>
>> Let's make this vx.x.x and specify current version number at the top.
>> Saves us doc updates as we change version number - one location change
>> as opposed to the entire document
> 
> Will be adopted in the next version of the PATCH.
> 
> A sidenote though, the version number has not changed since the
> release (out of staging) of usbip.
> 
Yes. Agreed. It may never change. However, it will be easier to not have
to update the document in multiple places. Also reference where the
version is - config.h has this defined.

>>>    | 2         | 2      | 0x8005     | Command code: Retrieve the list of exported USB   |
>>>    |           |        |            | devices.                                          |
>>> @@ -116,7 +145,7 @@ OP_REP_DEVLIST:
>>>    +-----------+--------+------------+---------------------------------------------------+
>>>    | Offset    | Length | Value      | Description                                       |
>>>    +===========+========+============+===================================================+
>>> -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0.|
>>> +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>>    | 2         | 2      | 0x0005     | Reply code: The list of exported USB devices.     |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>> @@ -165,8 +194,8 @@ OP_REP_DEVLIST:
>>>    | 0x143     | 1      |            | bNumInterfaces                                    |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>>    | 0x144     |        | m_0        | From now on each interface is described, all      |
>>> -|           |        |            | together bNumInterfaces times, with the           |
>>> -|           |        |            | the following 4 fields:                           |
>>> +|           |        |            | together bNumInterfaces times, with the following |
>>> +|           |        |            | 4 fields:                                         |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>>    |           | 1      |            | bInterfaceClass                                   |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>> @@ -177,7 +206,7 @@ OP_REP_DEVLIST:
>>>    | 0x147     | 1      |            | padding byte for alignment, shall be set to zero  |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>>    | 0xC +     |        |            | The second exported USB device starts at i=1      |
>>> -| i*0x138 + |        |            | with the busid field.                             |
>>> +| i*0x138 + |        |            | with the path field.                              |
>>>    | m_(i-1)*4 |        |            |                                                   |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>> @@ -187,7 +216,7 @@ OP_REQ_IMPORT:
>>>    +-----------+--------+------------+---------------------------------------------------+
>>>    | Offset    | Length | Value      | Description                                       |
>>>    +===========+========+============+===================================================+
>>> -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
>>> +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>>    | 2         | 2      | 0x8003     | Command code: import a remote USB device.         |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>> @@ -206,7 +235,7 @@ OP_REP_IMPORT:
>>>    +-----------+--------+------------+---------------------------------------------------+
>>>    | Offset    | Length | Value      | Description                                       |
>>>    +===========+========+============+===================================================+
>>> -| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
>>> +| 0         | 2      | 0x0111     | Binary-coded decimal USBIP version number: v1.1.1 |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>>    | 2         | 2      | 0x0003     | Reply code: Reply to import.                      |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>> @@ -254,158 +283,151 @@ OP_REP_IMPORT:
>>>    | 0x13E     | 1      |            | bNumInterfaces                                    |
>>>    +-----------+--------+------------+---------------------------------------------------+
>>> -USBIP_CMD_SUBMIT:
>>> -	Submit an URB
>>> +The following four commands have a common basic header called
>>> +'usbip_header_basic', and their headers, called 'usbip_header' (before URB
>>> +payload), have the same length, therefore paddings are needed.
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| Offset    | Length | Value      | Description                                       |
>>> -+===========+========+============+===================================================+
>>> -| 0         | 4      | 0x00000001 | command: Submit an URB                            |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 4         | 4      |            | seqnum: the sequence number of the URB to submit  |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 8         | 4      |            | devid                                             |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0xC       | 4      |            | direction:                                        |
>>> -|           |        |            |                                                   |
>>> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
>>> -|           |        |            |    - 1: USBIP_DIR_IN                              |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x10      | 4      |            | ep: endpoint number, possible values are: 0...15  |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x14      | 4      |            | transfer_flags: possible values depend on the     |
>>> -|           |        |            | URB transfer type, see below                      |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x18      | 4      |            | transfer_buffer_length                            |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x1C      | 4      |            | start_frame: specify the selected frame to        |
>>> -|           |        |            | transmit an ISO frame, ignored if URB_ISO_ASAP    |
>>> -|           |        |            | is specified at transfer_flags                    |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x24      | 4      |            | interval: maximum time for the request on the     |
>>> -|           |        |            | server-side host controller                       |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
>>> -|           |        |            | zeros if not used                                 |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x30      |        |            | URB data. For ISO transfers the padding between   |
>>> -|           |        |            | each ISO packets is not transmitted.              |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> +usbip_header_basic:
>>> ++-----------+--------+---------------------------------------------------+
>>> +| Offset    | Length | Description                                       |
>>> ++===========+========+===================================================+
>>> +| 0         | 4      | command                                           |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 4         | 4      | seqnum: sequential number that identifies requests|
>>> +|           |        | and corresponding responses;                      |
>>> +|           |        | incremented per connection                        |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 8         | 4      | devid: specifies a remote USB device uniquely     |
>>> +|           |        | instead of busnum and devnum;                     |
>>> +|           |        | for client (request), this value is               |
>>> +|           |        | ((busnum << 16) | devnum);                        |
>>> +|           |        | for server (response), this shall be set to 0     |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0xC       | 4      | direction:                                        |
>>> +|           |        |                                                   |
>>> +|           |        |    - 0: USBIP_DIR_OUT                             |
>>> +|           |        |    - 1: USBIP_DIR_IN                              |
>>> +|           |        |                                                   |
>>> +|           |        | only used by client, for server this shall be 0   |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x10      | 4      | ep: endpoint number                               |
>>> +|           |        | only used by client, for server this shall be 0;  |
>>> +|           |        | for UNLINK, this shall be 0                       |
>>> ++-----------+--------+---------------------------------------------------+
>>> - +-------------------------+------------+---------+-----------+----------+-------------+
>>> - | Allowed transfer_flags  | value      | control | interrupt | bulk     | isochronous |
>>> - +=========================+============+=========+===========+==========+=============+
>>> - | URB_SHORT_NOT_OK        | 0x00000001 | only in | only in   | only in  | no          |
>>> - +-------------------------+------------+---------+-----------+----------+-------------+
>>> - | URB_ISO_ASAP            | 0x00000002 | no      | no        | no       | yes         |
>>> - +-------------------------+------------+---------+-----------+----------+-------------+
>>> - | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes     | yes       | yes      | yes         |
>>> - +-------------------------+------------+---------+-----------+----------+-------------+
>>> - | URB_ZERO_PACKET         | 0x00000040 | no      | no        | only out | no          |
>>> - +-------------------------+------------+---------+-----------+----------+-------------+
>>> - | URB_NO_INTERRUPT        | 0x00000080 | yes     | yes       | yes      | yes         |
>>> - +-------------------------+------------+---------+-----------+----------+-------------+
>>> - | URB_FREE_BUFFER         | 0x00000100 | yes     | yes       | yes      | yes         |
>>> - +-------------------------+------------+---------+-----------+----------+-------------+
>>> - | URB_DIR_MASK            | 0x00000200 | yes     | yes       | yes      | yes         |
>>> - +-------------------------+------------+---------+-----------+----------+-------------+
>>> +USBIP_CMD_SUBMIT:
>>> +	Submit an URB
>>> ++-----------+--------+---------------------------------------------------+
>>> +| Offset    | Length | Description                                       |
>>> ++===========+========+===================================================+
>>> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000001 |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x14      | 4      | transfer_flags: possible values depend on the     |
>>> +|           |        | URB transfer_flags,                               |
>>> +|           |        | but with URB_NO_TRANSFER_DMA_MAP masked           |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x18      | 4      | transfer_buffer_length:                           |
>>> +|           |        | use URB transfer_buffer_length                    |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x1C      | 4      | start_frame: use URB start_frame;                 |
>>> +|           |        | initial frame for ISO transfer;                   |
>>> +|           |        | shall be set to 0 if not ISO transfer             |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x20      | 4      | number_of_packets: number of ISO packets;         |
>>> +|           |        | shall be set to 0xffffffff if not ISO transfer    |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x24      | 4      | interval: maximum time for the request on the     |
>>> +|           |        | server-side host controller                       |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x28      | 8      | setup: data bytes for USB setup, filled with      |
>>> +|           |        | zeros if not used.                                |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x30      | n      | transfer_buffer.                                  |
>>> +|           |        | If direction is USBIP_DIR_OUT then n equals       |
>>> +|           |        | transfer_buffer_length; otherwise n equals 0.     |
>>> +|           |        | For ISO transfers the padding between each ISO    |
>>> +|           |        | packets is not transmitted.                       |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x30+n    | m      | iso_packet_descriptor                             |
>>> ++-----------+--------+---------------------------------------------------+
>>>    USBIP_RET_SUBMIT:
>>>    	Reply for submitting an URB
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| Offset    | Length | Value      | Description                                       |
>>> -+===========+========+============+===================================================+
>>> -| 0         | 4      | 0x00000003 | command                                           |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 4         | 4      |            | seqnum: URB sequence number                       |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 8         | 4      |            | devid                                             |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0xC       | 4      |            | direction:                                        |
>>> -|           |        |            |                                                   |
>>> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
>>> -|           |        |            |    - 1: USBIP_DIR_IN                              |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x10      | 4      |            | ep: endpoint number                               |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x14      | 4      |            | status: zero for successful URB transaction,      |
>>> -|           |        |            | otherwise some kind of error happened.            |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x18      | 4      | n          | actual_length: number of URB data bytes           |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x1C      | 4      |            | start_frame: for an ISO frame the actually        |
>>> -|           |        |            | selected frame for transmit.                      |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x20      | 4      |            | number_of_packets                                 |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x24      | 4      |            | error_count                                       |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
>>> -|           |        |            | zeros if not used                                 |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
>>> -|           |        |            | between each ISO packets is not transmitted.      |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> ++-----------+--------+---------------------------------------------------+
>>> +| Offset    | Length | Description                                       |
>>> ++===========+========+===================================================+
>>> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000003 |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x14      | 4      | status: zero for successful URB transaction,      |
>>> +|           |        | otherwise some kind of error happened.            |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x18      | 4      | actual_length: number of URB data bytes;          |
>>> +|           |        | use URB actual_length                             |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x1C      | 4      | start_frame: use URB start_frame;                 |
>>> +|           |        | initial frame for ISO transfer;                   |
>>> +|           |        | shall be set to 0 if not ISO transfer             |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x20      | 4      | number_of_packets: number of ISO packets;         |
>>> +|           |        | shall be set to 0xffffffff if not ISO transfer    |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x24      | 4      | error_count                                       |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x28      | 8      | padding, shall be set to 0                        |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x30      | n      | transfer_buffer.                                  |
>>> +|           |        | If direction is USBIP_DIR_IN then n equals        |
>>> +|           |        | actual_length; otherwise n equals 0.              |
>>> +|           |        | For ISO transfers the padding between each ISO    |
>>> +|           |        | packets is not transmitted.                       |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x30+n    | m      | iso_packet_descriptor                             |
>>> ++-----------+--------+---------------------------------------------------+
>>>    USBIP_CMD_UNLINK:
>>>    	Unlink an URB
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| Offset    | Length | Value      | Description                                       |
>>> -+===========+========+============+===================================================+
>>> -| 0         | 4      | 0x00000002 | command: URB unlink command                       |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 4         | 4      |            | seqnum: URB sequence number to unlink:            |
>>> -|           |        |            |                                                   |
>>> -|           |        |            | FIXME:                                            |
>>> -|           |        |            |    is this so?                                    |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 8         | 4      |            | devid                                             |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0xC       | 4      |            | direction:                                        |
>>> -|           |        |            |                                                   |
>>> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
>>> -|           |        |            |    - 1: USBIP_DIR_IN                              |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x10      | 4      |            | ep: endpoint number: zero                         |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x14      | 4      |            | seqnum: the URB sequence number given previously  |
>>> -|           |        |            | at USBIP_CMD_SUBMIT.seqnum field                  |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
>>> -|           |        |            | between each ISO packets is not transmitted.      |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> ++-----------+--------+---------------------------------------------------+
>>> +| Offset    | Length | Description                                       |
>>> ++===========+========+===================================================+
>>> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000002 |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x14      | 4      | unlink_seqnum, of the SUBMIT request to unlink    |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x18      | 24     | padding, shall be set to 0                        |
>>> ++-----------+--------+---------------------------------------------------+
>>>    USBIP_RET_UNLINK:
>>>    	Reply for URB unlink
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| Offset    | Length | Value      | Description                                       |
>>> -+===========+========+============+===================================================+
>>> -| 0         | 4      | 0x00000004 | command: reply for the URB unlink command         |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 4         | 4      |            | seqnum: the unlinked URB sequence number          |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 8         | 4      |            | devid                                             |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0xC       | 4      |            | direction:                                        |
>>> -|           |        |            |                                                   |
>>> -|           |        |            |    - 0: USBIP_DIR_OUT                             |
>>> -|           |        |            |    - 1: USBIP_DIR_IN                              |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x10      | 4      |            | ep: endpoint number                               |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x14      | 4      |            | status: This is the value contained in the        |
>>> -|           |        |            | urb->status in the URB completition handler.      |
>>> -|           |        |            |                                                   |
>>> -|           |        |            | FIXME:                                            |
>>> -|           |        |            |      a better explanation needed.                 |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> -| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
>>> -|           |        |            | between each ISO packets is not transmitted.      |
>>> -+-----------+--------+------------+---------------------------------------------------+
>>> ++-----------+--------+---------------------------------------------------+
>>> +| Offset    | Length | Description                                       |
>>> ++===========+========+===================================================+
>>> +| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000004 |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x14      | 4      | status: This is similar to the status of          |
>>> +|           |        | USBIP_RET_SUBMIT (share the same memory offset).  |
>>> +|           |        | When UNLINK is successful, status is -ECONNRESET; |
>>> +|           |        | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT   |
>>> +|           |        | status is 0                                       |
>>> ++-----------+--------+---------------------------------------------------+
>>> +| 0x18      | 24     | padding, shall be set to 0                        |
>>> ++-----------+--------+---------------------------------------------------+
>>> +
>>> +EXAMPLE
>>> +=======
>>> +
>>> +  The following data is captured from wire with Human Interface Devices (HID)
>>> +  payload
>>> +
>>> +::
>>> +
>>> +  CmdIntrIN:  00000001 00000d05 0001000f 00000001 00000001 00000200 00000040 ffffffff 00000000 00000004 00000000 00000000
>>> +  CmdIntrOUT: 00000001 00000d06 0001000f 00000000 00000001 00000000 00000040 ffffffff 00000000 00000004 00000000 00000000
>>> +              ffffffff860008a784ce5ae212376300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
>>> +  RetIntrOut: 00000003 00000d06 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
>>> +  RetIntrIn:  00000003 00000d05 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
>>> +              ffffffff860011a784ce5ae2123763612891b1020100000400000000000000000000000000000000000000000000000000000000000000000000000000000000
>>>
>>
> 
> I will submit the next version of the PATCH after all the conversations above
> resolved.
> 

Sounds good. Thanks for doing this work.

thanks,
-- Shuah

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

* [PATCH v6] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-16  2:25         ` [PATCH v5] " Hongren Zheng (Zenithal)
  2021-03-16 15:57           ` Shuah Khan
  2021-03-29 20:06           ` Shuah Khan
@ 2021-03-30 17:00           ` Hongren Zheng (Zenithal)
  2021-04-08 15:18             ` Shuah Khan
  2 siblings, 1 reply; 14+ messages in thread
From: Hongren Zheng (Zenithal) @ 2021-03-30 17:00 UTC (permalink / raw)
  To: Valentina Manea, Shuah Khan, Greg Kroah-Hartman,
	Márton Németh, Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, usbip-devel, Randy Dunlap

The old document for usbip protocol is misleading and hard to read:
  * Some fields in header are incorrect
  * Explanation of some fields are unclear or even wrong
  * Padding of header (namely all headers have the same length) is
    not explicitly pointed out, which is crucial for stream protocol
    like TCP

Major changes:
  * Document the correct field as described in the codebase.
  * Document the padding in usbip headers. This is crucial for TCP
    stream hence these padding should be explicitly point out.
    In code these padding are implemented by a union of all headers.
  * Fix two FIXME related to usbip unlink and Document the behavior
    of unlink in different situation.
  * Clarify some field with more accurate explanation, like those
    fields associated with URB. Some constraints are extracted from
    code.
  * Delete specific transfer_flag doc in usbip as it should be
    documented by the URB doc in Documentation/driver-api/usb/URB.rst
  * Add data captured from wire as example

Version change:
    From "PRELIMINARY DRAFT, MAY CONTAIN MISTAKES, 28 Jun 2011"
    To "Version 1, 31 Mar 2021"

Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Hongren Zheng <i@zenithal.me>
---
 Documentation/usb/usbip_protocol.rst | 344 +++++++++++++++------------
 1 file changed, 193 insertions(+), 151 deletions(-)

PATCH v2:
Some changes suggested by a previous patch in
https://lore.kernel.org/linux-usb
/20180128071514.9107-1-alexandre.f.demers@gmail.com/
is adopted in this patch.
  * Fix Typo: duplicated 'the' in 'the following 4 field'
  * Fix incorrect field: in OP_REQ_DEVLIST, the second dev starts with
    field 'path', not 'busid'

PATCH v3:
Suggested by
https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
  * Remove date and changelog in doc as these are tracked in git history
  * Remove 'mistake alert' as all data fields are documented properly
    now. However, docs on possible values for some field shall be added
    in the future

PATCH v4:
Suggested by https://lore.kernel.org/linux-doc
/40351ed6-2907-3966-e69a-a564173b3682@infradead.org/
  * Add punctuations for readability
  * Move patch changelog after the marker line
  * Remove nickname in signed-off-by line

PATCH v5:
  * Instead of co-developed-by, use reviewed-by
    for Randy Dunlap

PATCH v6:
  * Add reference to URB doc in changelog
  * Add version change in changelog
  * Add reference to URB doc for transfer_flag and other fields in
    USBIP_CMD_SUBMIT
  * Add reference to specific functions for UNLINK behavior
    and transfer_flag
  * Fix typo: "clients import", "connection to the server" and 
    "USBIP_RET_UNLINK" to "USBIP_RET_SUBMIT" in UNLINK behavior part
  * Separate the flow of UNLINK for clarity
  * Extract the version number in message headers to one separate
    section for ease of maintainence

diff --git a/Documentation/usb/usbip_protocol.rst b/Documentation/usb/usbip_protocol.rst
index 988c832166cd..0b8541fda4d8 100644
--- a/Documentation/usb/usbip_protocol.rst
+++ b/Documentation/usb/usbip_protocol.rst
@@ -2,15 +2,15 @@
 USB/IP protocol
 ===============
 
-PRELIMINARY DRAFT, MAY CONTAIN MISTAKES!
-28 Jun 2011
+Architecture
+============
 
 The USB/IP protocol follows a server/client architecture. The server exports the
-USB devices and the clients imports them. The device driver for the exported
+USB devices and the clients import them. The device driver for the exported
 USB device runs on the client machine.
 
 The client may ask for the list of the exported USB devices. To get the list the
-client opens a TCP/IP connection towards the server, and sends an OP_REQ_DEVLIST
+client opens a TCP/IP connection to the server, and sends an OP_REQ_DEVLIST
 packet on top of the TCP/IP connection (so the actual OP_REQ_DEVLIST may be sent
 in one or more pieces at the low level transport layer). The server sends back
 the OP_REP_DEVLIST packet which lists the exported USB devices. Finally the
@@ -30,7 +30,7 @@ TCP/IP connection is closed.
           |                                                 |
 
 Once the client knows the list of exported USB devices it may decide to use one
-of them. First the client opens a TCP/IP connection towards the server and
+of them. First the client opens a TCP/IP connection to the server and
 sends an OP_REQ_IMPORT packet. The server replies with OP_REP_IMPORT. If the
 import was successful the TCP/IP connection remains open and will be used
 to transfer the URB traffic between the client and the server. The client may
@@ -84,17 +84,61 @@ server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
           | <---------------------------------------------- |
           |                        .                        |
           |                        :                        |
+
+For UNLINK, note that after a successful USBIP_RET_UNLINK, the unlinked URB
+submission would not have a corresponding USBIP_RET_SUBMIT (this is explained in
+function stub_recv_cmd_unlink of drivers/usb/usbip/stub_rx.c).
+
+::
+
+ virtual host controller                                 usb host
+      "client"                                           "server"
+  (imports USB devices)                             (exports USB devices)
+          |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = p)         |
+          | ----------------------------------------------> |
           |                                                 |
           |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = p+1, unlink_seqnum = p)       |
           | ----------------------------------------------> |
           |                                                 |
           |               USBIP_RET_UNLINK                  |
+          |        (seqnum = p+1, status = -ECONNRESET)     |
+          | <---------------------------------------------- |
+          |                                                 |
+          |         Note: No USBIP_RET_SUBMIT(seqnum = p)   |
+          | <--X---X---X---X---X---X---X---X---X---X---X--- |
+          |                        .                        |
+          |                        :                        |
+          |                                                 |
+          |            USBIP_CMD_SUBMIT(seqnum = q)         |
+          | ----------------------------------------------> |
+          |                                                 |
+          |            USBIP_RET_SUBMIT(seqnum = q)         |
+          | <---------------------------------------------- |
+          |                                                 |
+          |               USBIP_CMD_UNLINK                  |
+          |         (seqnum = q+1, unlink_seqnum = q)       |
+          | ----------------------------------------------> |
+          |                                                 |
+          |               USBIP_RET_UNLINK                  |
+          |           (seqnum = q+1, status = 0)            |
           | <---------------------------------------------- |
           |                                                 |
 
 The fields are in network (big endian) byte order meaning that the most significant
 byte (MSB) is stored at the lowest address.
 
+Protocol Version
+================
+
+The documented USBIP version is v1.1.1. The binary representation of this
+version in message headers is 0x0111.
+
+This is defined in tools/usb/usbip/configure.ac
+
+Message Format
+==============
 
 OP_REQ_DEVLIST:
 	Retrieve the list of exported USB devices.
@@ -102,7 +146,7 @@ OP_REQ_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      |            | USBIP version                                     |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8005     | Command code: Retrieve the list of exported USB   |
 |           |        |            | devices.                                          |
@@ -116,7 +160,7 @@ OP_REP_DEVLIST:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0.|
+| 0         | 2      |            | USBIP version                                     |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0005     | Reply code: The list of exported USB devices.     |
 +-----------+--------+------------+---------------------------------------------------+
@@ -165,8 +209,8 @@ OP_REP_DEVLIST:
 | 0x143     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 | 0x144     |        | m_0        | From now on each interface is described, all      |
-|           |        |            | together bNumInterfaces times, with the           |
-|           |        |            | the following 4 fields:                           |
+|           |        |            | together bNumInterfaces times, with the following |
+|           |        |            | 4 fields:                                         |
 +-----------+--------+------------+---------------------------------------------------+
 |           | 1      |            | bInterfaceClass                                   |
 +-----------+--------+------------+---------------------------------------------------+
@@ -177,7 +221,7 @@ OP_REP_DEVLIST:
 | 0x147     | 1      |            | padding byte for alignment, shall be set to zero  |
 +-----------+--------+------------+---------------------------------------------------+
 | 0xC +     |        |            | The second exported USB device starts at i=1      |
-| i*0x138 + |        |            | with the busid field.                             |
+| i*0x138 + |        |            | with the path field.                              |
 | m_(i-1)*4 |        |            |                                                   |
 +-----------+--------+------------+---------------------------------------------------+
 
@@ -187,7 +231,7 @@ OP_REQ_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      |            | USBIP version                                     |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x8003     | Command code: import a remote USB device.         |
 +-----------+--------+------------+---------------------------------------------------+
@@ -206,7 +250,7 @@ OP_REP_IMPORT:
 +-----------+--------+------------+---------------------------------------------------+
 | Offset    | Length | Value      | Description                                       |
 +===========+========+============+===================================================+
-| 0         | 2      | 0x0100     | Binary-coded decimal USBIP version number: v1.0.0 |
+| 0         | 2      |            | USBIP version                                     |
 +-----------+--------+------------+---------------------------------------------------+
 | 2         | 2      | 0x0003     | Reply code: Reply to import.                      |
 +-----------+--------+------------+---------------------------------------------------+
@@ -254,158 +298,156 @@ OP_REP_IMPORT:
 | 0x13E     | 1      |            | bNumInterfaces                                    |
 +-----------+--------+------------+---------------------------------------------------+
 
-USBIP_CMD_SUBMIT:
-	Submit an URB
+The following four commands have a common basic header called
+'usbip_header_basic', and their headers, called 'usbip_header' (before
+transfer_buffer payload), have the same length, therefore paddings are needed.
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000001 | command: Submit an URB                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the sequence number of the URB to submit  |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number, possible values are: 0...15  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | transfer_flags: possible values depend on the     |
-|           |        |            | URB transfer type, see below                      |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x18      | 4      |            | transfer_buffer_length                            |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: specify the selected frame to        |
-|           |        |            | transmit an ISO frame, ignored if URB_ISO_ASAP    |
-|           |        |            | is specified at transfer_flags                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets: number of ISO packets          |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x24      | 4      |            | interval: maximum time for the request on the     |
-|           |        |            | server-side host controller                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      |        |            | URB data. For ISO transfers the padding between   |
-|           |        |            | each ISO packets is not transmitted.              |
-+-----------+--------+------------+---------------------------------------------------+
+usbip_header_basic:
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 4      | command                                           |
++-----------+--------+---------------------------------------------------+
+| 4         | 4      | seqnum: sequential number that identifies requests|
+|           |        | and corresponding responses;                      |
+|           |        | incremented per connection                        |
++-----------+--------+---------------------------------------------------+
+| 8         | 4      | devid: specifies a remote USB device uniquely     |
+|           |        | instead of busnum and devnum;                     |
+|           |        | for client (request), this value is               |
+|           |        | ((busnum << 16) | devnum);                        |
+|           |        | for server (response), this shall be set to 0     |
++-----------+--------+---------------------------------------------------+
+| 0xC       | 4      | direction:                                        |
+|           |        |                                                   |
+|           |        |    - 0: USBIP_DIR_OUT                             |
+|           |        |    - 1: USBIP_DIR_IN                              |
+|           |        |                                                   |
+|           |        | only used by client, for server this shall be 0   |
++-----------+--------+---------------------------------------------------+
+| 0x10      | 4      | ep: endpoint number                               |
+|           |        | only used by client, for server this shall be 0;  |
+|           |        | for UNLINK, this shall be 0                       |
++-----------+--------+---------------------------------------------------+
 
- +-------------------------+------------+---------+-----------+----------+-------------+
- | Allowed transfer_flags  | value      | control | interrupt | bulk     | isochronous |
- +=========================+============+=========+===========+==========+=============+
- | URB_SHORT_NOT_OK        | 0x00000001 | only in | only in   | only in  | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ISO_ASAP            | 0x00000002 | no      | no        | no       | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_TRANSFER_DMA_MAP | 0x00000004 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_ZERO_PACKET         | 0x00000040 | no      | no        | only out | no          |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_NO_INTERRUPT        | 0x00000080 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_FREE_BUFFER         | 0x00000100 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
- | URB_DIR_MASK            | 0x00000200 | yes     | yes       | yes      | yes         |
- +-------------------------+------------+---------+-----------+----------+-------------+
+USBIP_CMD_SUBMIT:
+	Submit an URB
 
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000001 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | transfer_flags: possible values depend on the     |
+|           |        | URB transfer_flags (refer to URB doc in           |
+|           |        | Documentation/driver-api/usb/URB.rst)             |
+|           |        | but with URB_NO_TRANSFER_DMA_MAP masked. Refer to |
+|           |        | function usbip_pack_cmd_submit and function       |
+|           |        | tweak_transfer_flags in drivers/usb/usbip/        |
+|           |        | usbip_common.c. The following fields may also ref |
+|           |        | to function usbip_pack_cmd_submit and URB doc     |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 4      | transfer_buffer_length:                           |
+|           |        | use URB transfer_buffer_length                    |
++-----------+--------+---------------------------------------------------+
+| 0x1C      | 4      | start_frame: use URB start_frame;                 |
+|           |        | initial frame for ISO transfer;                   |
+|           |        | shall be set to 0 if not ISO transfer             |
++-----------+--------+---------------------------------------------------+
+| 0x20      | 4      | number_of_packets: number of ISO packets;         |
+|           |        | shall be set to 0xffffffff if not ISO transfer    |
++-----------+--------+---------------------------------------------------+
+| 0x24      | 4      | interval: maximum time for the request on the     |
+|           |        | server-side host controller                       |
++-----------+--------+---------------------------------------------------+
+| 0x28      | 8      | setup: data bytes for USB setup, filled with      |
+|           |        | zeros if not used.                                |
++-----------+--------+---------------------------------------------------+
+| 0x30      | n      | transfer_buffer.                                  |
+|           |        | If direction is USBIP_DIR_OUT then n equals       |
+|           |        | transfer_buffer_length; otherwise n equals 0.     |
+|           |        | For ISO transfers the padding between each ISO    |
+|           |        | packets is not transmitted.                       |
++-----------+--------+---------------------------------------------------+
+| 0x30+n    | m      | iso_packet_descriptor                             |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_SUBMIT:
 	Reply for submitting an URB
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000003 | command                                           |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | status: zero for successful URB transaction,      |
-|           |        |            | otherwise some kind of error happened.            |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x18      | 4      | n          | actual_length: number of URB data bytes           |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x1C      | 4      |            | start_frame: for an ISO frame the actually        |
-|           |        |            | selected frame for transmit.                      |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x20      | 4      |            | number_of_packets                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x24      | 4      |            | error_count                                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x28      | 8      |            | setup: data bytes for USB setup, filled with      |
-|           |        |            | zeros if not used                                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000003 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | status: zero for successful URB transaction,      |
+|           |        | otherwise some kind of error happened.            |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 4      | actual_length: number of URB data bytes;          |
+|           |        | use URB actual_length                             |
++-----------+--------+---------------------------------------------------+
+| 0x1C      | 4      | start_frame: use URB start_frame;                 |
+|           |        | initial frame for ISO transfer;                   |
+|           |        | shall be set to 0 if not ISO transfer             |
++-----------+--------+---------------------------------------------------+
+| 0x20      | 4      | number_of_packets: number of ISO packets;         |
+|           |        | shall be set to 0xffffffff if not ISO transfer    |
++-----------+--------+---------------------------------------------------+
+| 0x24      | 4      | error_count                                       |
++-----------+--------+---------------------------------------------------+
+| 0x28      | 8      | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
+| 0x30      | n      | transfer_buffer.                                  |
+|           |        | If direction is USBIP_DIR_IN then n equals        |
+|           |        | actual_length; otherwise n equals 0.              |
+|           |        | For ISO transfers the padding between each ISO    |
+|           |        | packets is not transmitted.                       |
++-----------+--------+---------------------------------------------------+
+| 0x30+n    | m      | iso_packet_descriptor                             |
++-----------+--------+---------------------------------------------------+
 
 USBIP_CMD_UNLINK:
 	Unlink an URB
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000002 | command: URB unlink command                       |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: URB sequence number to unlink:            |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |    is this so?                                    |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number: zero                         |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | seqnum: the URB sequence number given previously  |
-|           |        |            | at USBIP_CMD_SUBMIT.seqnum field                  |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000002 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | unlink_seqnum, of the SUBMIT request to unlink    |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
 
 USBIP_RET_UNLINK:
 	Reply for URB unlink
 
-+-----------+--------+------------+---------------------------------------------------+
-| Offset    | Length | Value      | Description                                       |
-+===========+========+============+===================================================+
-| 0         | 4      | 0x00000004 | command: reply for the URB unlink command         |
-+-----------+--------+------------+---------------------------------------------------+
-| 4         | 4      |            | seqnum: the unlinked URB sequence number          |
-+-----------+--------+------------+---------------------------------------------------+
-| 8         | 4      |            | devid                                             |
-+-----------+--------+------------+---------------------------------------------------+
-| 0xC       | 4      |            | direction:                                        |
-|           |        |            |                                                   |
-|           |        |            |    - 0: USBIP_DIR_OUT                             |
-|           |        |            |    - 1: USBIP_DIR_IN                              |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x10      | 4      |            | ep: endpoint number                               |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x14      | 4      |            | status: This is the value contained in the        |
-|           |        |            | urb->status in the URB completition handler.      |
-|           |        |            |                                                   |
-|           |        |            | FIXME:                                            |
-|           |        |            |      a better explanation needed.                 |
-+-----------+--------+------------+---------------------------------------------------+
-| 0x30      | n      |            | URB data bytes. For ISO transfers the padding     |
-|           |        |            | between each ISO packets is not transmitted.      |
-+-----------+--------+------------+---------------------------------------------------+
++-----------+--------+---------------------------------------------------+
+| Offset    | Length | Description                                       |
++===========+========+===================================================+
+| 0         | 20     | usbip_header_basic, 'command' shall be 0x00000004 |
++-----------+--------+---------------------------------------------------+
+| 0x14      | 4      | status: This is similar to the status of          |
+|           |        | USBIP_RET_SUBMIT (share the same memory offset).  |
+|           |        | When UNLINK is successful, status is -ECONNRESET; |
+|           |        | when USBIP_CMD_UNLINK is after USBIP_RET_SUBMIT   |
+|           |        | status is 0                                       |
++-----------+--------+---------------------------------------------------+
+| 0x18      | 24     | padding, shall be set to 0                        |
++-----------+--------+---------------------------------------------------+
+
+EXAMPLE
+=======
+
+  The following data is captured from wire with Human Interface Devices (HID)
+  payload
+
+::
+
+  CmdIntrIN:  00000001 00000d05 0001000f 00000001 00000001 00000200 00000040 ffffffff 00000000 00000004 00000000 00000000
+  CmdIntrOUT: 00000001 00000d06 0001000f 00000000 00000001 00000000 00000040 ffffffff 00000000 00000004 00000000 00000000
+              ffffffff860008a784ce5ae212376300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+  RetIntrOut: 00000003 00000d06 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+  RetIntrIn:  00000003 00000d05 00000000 00000000 00000000 00000000 00000040 ffffffff 00000000 00000000 00000000 00000000
+              ffffffff860011a784ce5ae2123763612891b1020100000400000000000000000000000000000000000000000000000000000000000000000000000000000000
-- 
2.31.0


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

* Re: [PATCH v6] docs: usbip: Fix major fields and descriptions in protocol
  2021-03-30 17:00           ` [PATCH v6] " Hongren Zheng (Zenithal)
@ 2021-04-08 15:18             ` Shuah Khan
  0 siblings, 0 replies; 14+ messages in thread
From: Shuah Khan @ 2021-04-08 15:18 UTC (permalink / raw)
  To: Hongren Zheng (Zenithal),
	Valentina Manea, Shuah Khan, Greg Kroah-Hartman,
	Márton Németh, Jonathan Corbet, linux-doc
  Cc: Alexandre Demers, linux-usb, usbip-devel, Randy Dunlap, Shuah Khan

On 3/30/21 11:00 AM, Hongren Zheng (Zenithal) wrote:
> The old document for usbip protocol is misleading and hard to read:
>    * Some fields in header are incorrect
>    * Explanation of some fields are unclear or even wrong
>    * Padding of header (namely all headers have the same length) is
>      not explicitly pointed out, which is crucial for stream protocol
>      like TCP
> 
> Major changes:
>    * Document the correct field as described in the codebase.
>    * Document the padding in usbip headers. This is crucial for TCP
>      stream hence these padding should be explicitly point out.
>      In code these padding are implemented by a union of all headers.
>    * Fix two FIXME related to usbip unlink and Document the behavior
>      of unlink in different situation.
>    * Clarify some field with more accurate explanation, like those
>      fields associated with URB. Some constraints are extracted from
>      code.
>    * Delete specific transfer_flag doc in usbip as it should be
>      documented by the URB doc in Documentation/driver-api/usb/URB.rst
>    * Add data captured from wire as example
> 
> Version change:
>      From "PRELIMINARY DRAFT, MAY CONTAIN MISTAKES, 28 Jun 2011"
>      To "Version 1, 31 Mar 2021"
> 
> Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Hongren Zheng <i@zenithal.me>
> ---
>   Documentation/usb/usbip_protocol.rst | 344 +++++++++++++++------------
>   1 file changed, 193 insertions(+), 151 deletions(-)
> 
> PATCH v2:
> Some changes suggested by a previous patch in
> https://lore.kernel.org/linux-usb
> /20180128071514.9107-1-alexandre.f.demers@gmail.com/
> is adopted in this patch.
>    * Fix Typo: duplicated 'the' in 'the following 4 field'
>    * Fix incorrect field: in OP_REQ_DEVLIST, the second dev starts with
>      field 'path', not 'busid'
> 
> PATCH v3:
> Suggested by
> https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
>    * Remove date and changelog in doc as these are tracked in git history
>    * Remove 'mistake alert' as all data fields are documented properly
>      now. However, docs on possible values for some field shall be added
>      in the future
> 
> PATCH v4:
> Suggested by https://lore.kernel.org/linux-doc
> /40351ed6-2907-3966-e69a-a564173b3682@infradead.org/
>    * Add punctuations for readability
>    * Move patch changelog after the marker line
>    * Remove nickname in signed-off-by line
> 
> PATCH v5:
>    * Instead of co-developed-by, use reviewed-by
>      for Randy Dunlap
> 
> PATCH v6:
>    * Add reference to URB doc in changelog
>    * Add version change in changelog
>    * Add reference to URB doc for transfer_flag and other fields in
>      USBIP_CMD_SUBMIT
>    * Add reference to specific functions for UNLINK behavior
>      and transfer_flag
>    * Fix typo: "clients import", "connection to the server" and
>      "USBIP_RET_UNLINK" to "USBIP_RET_SUBMIT" in UNLINK behavior part
>    * Separate the flow of UNLINK for clarity
>    * Extract the version number in message headers to one separate
>      section for ease of maintainence
> 

Thank you for updating the document.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

Greg, please pick this up.

thanks,
-- Shuah

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

end of thread, other threads:[~2021-04-08 15:18 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15  1:57 [PATCH] docs: usbip: Fix major fields and descriptions in protocol Hongren Zheng (Zenithal)
2021-03-15  6:18 ` [PATCH v2] " Hongren Zheng (Zenithal)
2021-03-15  7:36   ` Greg Kroah-Hartman
2021-03-15  8:40     ` [PATCH v3] " Hongren Zheng (Zenithal)
2021-03-15 18:25       ` Randy Dunlap
2021-03-15 21:15       ` [PATCH v4] " Hongren Zheng (Zenithal)
2021-03-16  1:54         ` Randy Dunlap
2021-03-16  2:25         ` [PATCH v5] " Hongren Zheng (Zenithal)
2021-03-16 15:57           ` Shuah Khan
2021-03-29 20:06           ` Shuah Khan
2021-03-30 12:35             ` Hongren Zheng (Zenithal)
2021-03-30 14:57               ` Shuah Khan
2021-03-30 17:00           ` [PATCH v6] " Hongren Zheng (Zenithal)
2021-04-08 15:18             ` Shuah Khan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).