From: Randy Dunlap <rdunlap@infradead.org>
To: "Hongren Zheng (Zenithal)" <i@zenithal.me>,
"Valentina Manea" <valentina.manea.m@gmail.com>,
"Shuah Khan" <shuah@kernel.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Márton Németh" <nm127@freemail.hu>,
"Jonathan Corbet" <corbet@lwn.net>,
linux-doc@vger.kernel.org
Cc: Alexandre Demers <alexandre.f.demers@gmail.com>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
usbip-devel@lists.sourceforge.net
Subject: Re: [PATCH v3] docs: usbip: Fix major fields and descriptions in protocol
Date: Mon, 15 Mar 2021 11:25:03 -0700 [thread overview]
Message-ID: <40351ed6-2907-3966-e69a-a564173b3682@infradead.org> (raw)
In-Reply-To: <YE8dakf3nIn0jJew@Sun>
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
next prev parent reply other threads:[~2021-03-15 18:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=40351ed6-2907-3966-e69a-a564173b3682@infradead.org \
--to=rdunlap@infradead.org \
--cc=alexandre.f.demers@gmail.com \
--cc=corbet@lwn.net \
--cc=gregkh@linuxfoundation.org \
--cc=i@zenithal.me \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=nm127@freemail.hu \
--cc=shuah@kernel.org \
--cc=usbip-devel@lists.sourceforge.net \
--cc=valentina.manea.m@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).