Re: Extending USB_CONNECTINFO ioctl

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Alan,

On Tue, Jun 04, 2019 at 10:17:25AM -0400, Alan Stern wrote:
> On Mon, 3 Jun 2019, Dmitry Torokhov wrote:
> 
> > Hi Alan, Greg,
> > 
> > When running software in a jailed environment where sysfs or udev is not
> > readily available and one can only have an FD to usbdevfs device passed
> > into the jail, there is a desire to allow libusb working. Alan recently
> > added USBDEVFS_GET_SPEED, but we are still missing bus number and list
> > of port numbers on the way to the root to be able to better identify the
> > device in question.
> > 
> > What do you think about adding a new ioctl:
> > 
> > struct usbdevfs_connectinfo_ex {
> > 	__u32 size;		/* size of the structure from the kernel POV */
> 
> This should be an input/output parameter.  That is, the original value
> says how big the userspace structure is, and the value stored by the
> kernel says how much space was actually filled in.  Or alternatively, 
> have two size fields (one for input and one for output).

The "incoming" size is encoded in the ioctl number (the 'len' argument
in USBDEVFS_CONNINFO_EX()), there is no reason to add it separately (or
make read/write).

> 
> > 	__u32 busnum;
> > 	__u32 devnum;
> > 	__u32 speed;		/* USB_SPEED_* form ch9.h */
> > 	u8 num_ports;		/* Number of entries in port_numbers array */
> > 	u8 port_numbers[31];	/* Current limit in USB3.0 spec is 7 */
> > };
> 
> Yeah, 31 is overkill.  Even the USB-2.0 spec limits the topology to 7
> tiers (including the root hub), meaning that only 6 port numbers are
> needed.

If you insist I can bring it down to 7, but this structure is short
lived, on stack, and there should not be many of them in the kernel, so
if we can avoid extending a particular field in the future I think it is
worth it.
.

> 
> > /*
> >  * Returns struct usbdevfs_connectinfo_ex; length is variable to allow
> >  * extending size of the data returned.
> >  */
> > #define USBDEVFS_CONNINFO_EX(len)  _IOC(_IOC_READ, 'U', 32, len)
> 
> Sounds okay to me.  Have you asked the libusb Linux-port maintainers
> what they think and if they have any suggestions for additional fields?

Why don't we just get them into this conversation? Hopefully
libusb-devel that I just CCed here is not subscription-only.

> 
> Also, if you implement this, remember to add a USBDEVFS_CAP_CONNINFO_EX
> capability flag for the new ioctl (see the USBDEVFS_GET_CAPABILITIES 
> handler).

OK, will do.

Thanks.

-- 
Dmitry