* Extending USB_CONNECTINFO ioctl
@ 2019-06-04 0:24 Dmitry Torokhov
2019-06-04 5:38 ` Greg Kroah-Hartman
2019-06-04 14:17 ` Alan Stern
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2019-06-04 0:24 UTC (permalink / raw
To: Alan Stern, Greg Kroah-Hartman; +Cc: linux-usb
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 */
__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 */
};
/*
* 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)
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Extending USB_CONNECTINFO ioctl
2019-06-04 0:24 Extending USB_CONNECTINFO ioctl Dmitry Torokhov
@ 2019-06-04 5:38 ` Greg Kroah-Hartman
2019-06-04 14:17 ` Alan Stern
1 sibling, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-04 5:38 UTC (permalink / raw
To: Dmitry Torokhov; +Cc: Alan Stern, linux-usb
On Mon, Jun 03, 2019 at 05:24:10PM -0700, 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 */
> __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 */
> };
>
> /*
> * 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 reasonable, as long as you get the 'variable data' portion
correct :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Extending USB_CONNECTINFO ioctl
2019-06-04 0:24 Extending USB_CONNECTINFO ioctl Dmitry Torokhov
2019-06-04 5:38 ` Greg Kroah-Hartman
@ 2019-06-04 14:17 ` Alan Stern
2019-06-04 16:14 ` Dmitry Torokhov
1 sibling, 1 reply; 6+ messages in thread
From: Alan Stern @ 2019-06-04 14:17 UTC (permalink / raw
To: Dmitry Torokhov; +Cc: Greg Kroah-Hartman, linux-usb
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).
> __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.
> /*
> * 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?
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).
Alan Stern
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Extending USB_CONNECTINFO ioctl
2019-06-04 14:17 ` Alan Stern
@ 2019-06-04 16:14 ` Dmitry Torokhov
2019-06-04 16:48 ` Greg Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2019-06-04 16:14 UTC (permalink / raw
To: Alan Stern; +Cc: Greg Kroah-Hartman, linux-usb, libusb-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Extending USB_CONNECTINFO ioctl
2019-06-04 16:14 ` Dmitry Torokhov
@ 2019-06-04 16:48 ` Greg Kroah-Hartman
2019-06-10 22:31 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-04 16:48 UTC (permalink / raw
To: Dmitry Torokhov; +Cc: Alan Stern, linux-usb, libusb-devel
On Tue, Jun 04, 2019 at 09:14:51AM -0700, Dmitry Torokhov wrote:
> 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.
I don't know what USB 4 has in it (and if I did, I couldn't mention it),
but someone who can see those specs might want to take a look at them to
just say "yeah, 31 looks like a good number" or "nope, make it a bit
bigger please!"
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Extending USB_CONNECTINFO ioctl
2019-06-04 16:48 ` Greg Kroah-Hartman
@ 2019-06-10 22:31 ` Dmitry Torokhov
0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2019-06-10 22:31 UTC (permalink / raw
To: Greg Kroah-Hartman; +Cc: Alan Stern, linux-usb, libusb-devel
On Tue, Jun 04, 2019 at 06:48:53PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 04, 2019 at 09:14:51AM -0700, Dmitry Torokhov wrote:
> > 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.
>
> I don't know what USB 4 has in it (and if I did, I couldn't mention it),
> but someone who can see those specs might want to take a look at them to
> just say "yeah, 31 looks like a good number" or "nope, make it a bit
> bigger please!"
OK, I believe 7 tiers should be enough, so I'll cut it down to 7 from
31.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-10 22:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-04 0:24 Extending USB_CONNECTINFO ioctl Dmitry Torokhov
2019-06-04 5:38 ` Greg Kroah-Hartman
2019-06-04 14:17 ` Alan Stern
2019-06-04 16:14 ` Dmitry Torokhov
2019-06-04 16:48 ` Greg Kroah-Hartman
2019-06-10 22:31 ` Dmitry Torokhov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.