Re: [PATCH v3 1/2] drivers: usb/core/urb: Add URB_FREE_COHERENT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/23/22 11:30 AM, Hongren Zenithal Zheng wrote:
On Fri, Jun 10, 2022 at 05:33:35PM -0400, Rhett Aultman wrote:

In order to have all the flags in numerical order, URB_DIR_IN is
renumbered from 0x0200 to 0x0400 so that URB_FREE_COHERENT can reuse
value 0x0200.

  #define URB_FREE_BUFFER		0x0100	/* Free transfer buffer with the URB */
+#define URB_FREE_COHERENT	0x0200  /* Free DMA memory of transfer buffer */
/* The following flags are used internally by usbcore and HCDs */
-#define URB_DIR_IN		0x0200	/* Transfer from device to host */
+#define URB_DIR_IN		0x0400	/* Transfer from device to host */
  #define URB_DIR_OUT		0
  #define URB_DIR_MASK		URB_DIR_IN
--
2.30.2


I'm afraid this is a change of uapi as this field is, unfortunately,
exported by usbip to userspace as TCP packets.

This may also cause incompatibility (surprisingly not for this case,
detailed below) between usbip server and client
when one kernel is using the new flags and the other one is not.

If we do change this, we may need to bump usbip protocol version
accordingly.



A copy of Alan Stern's suggestion here for reference
I don't see anything wrong with this, except that it would be nice to keep
the flag values in numerical order.  In other words, set URB_FREE_COHERENT
to 0x0200 and change URB_DIR_IN to 0x0400.

Alan Stern

Thank you Alan for this detailed analysis of uapi impacts and
usbip host side and vhci incompatibilities. Userspace is going
to be affected. In addition to the usbip tool in the kernel repo,
there are other versions floating around that would break if we
were to change the flags.

One way to solve this issue for usbip
is to add some boilerplate transform
from URB_* to USBIP_FLAGS_*
as it is de facto uapi now.

It doesn't sound like a there is a compelling reason other than
"it would be nice to keep the flag values in numerical order".

I would not recommend this option. I am not seeing any value to adding
change URB_* to USBIP_FLAGS_* layer without some serious techinical
concerns.


Another way is to use 0x0400 for FREE_COHERENT.
usbip will not take care of this bit as
it would be masked.


I would go with this option adding a clear comment with link to this
discussion.

Cc Shuah Khan here since she is the maintainer
on usbip.


Thank you adding me to the discussion.

thanks,
-- Shuah



[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux