linux_usbfs: Improve isochronous transfer submission and error reporting
authorChris Dickens <christopher.a.dickens@gmail.com>
Wed, 27 Dec 2017 03:15:42 +0000 (19:15 -0800)
committerChris Dickens <christopher.a.dickens@gmail.com>
Wed, 27 Dec 2017 03:15:42 +0000 (19:15 -0800)
The Linux kernel has changed the maximum allowed packet length per
isochronous packet numerous times, which can create difficulties in
trying to report appropriate errors back to the user when submitting too
large of a packet on older kernels.

In an attempt to improve this situation, this commit adds logic that
will use different per-packet limits based on the detected kernel
version. Additionally, the logic has been improved to split URBs based
on the number of isochronous packets per URB, which is currently (and
has been forever) limited to 128.

Finally, the error reporting during URB submission has been improved to
catch and report errors relating to the transfer length being too large.

Closes #118

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
libusb/os/linux_usbfs.c
libusb/os/linux_usbfs.h
libusb/version_nano.h

index f88d7ed..96fe07c 100644 (file)
@@ -81,6 +81,15 @@ static const char *usbfs_path = NULL;
 /* use usbdev*.* device names in /dev instead of the usbfs bus directories */
 static int usbdev_names = 0;
 
+/* Linux has changed the maximum length of an individual isochronous packet
+ * over time.  Initially this limit was 1,023 bytes, but Linux 2.6.18
+ * (commit 3612242e527eb47ee4756b5350f8bdf791aa5ede) increased this value to
+ * 8,192 bytes to support higher bandwidth devices.  Linux 3.10
+ * (commit e2e2f0ea1c935edcf53feb4c4c8fdb4f86d57dd9) further increased this
+ * value to 49,152 bytes to support super speed devices.
+ */
+static unsigned int max_iso_packet_len = 0;
+
 /* Linux 2.6.23 adds support for O_CLOEXEC when opening files, which marks the
  * close-on-exec flag in the underlying file descriptor. */
 static int supports_flag_cloexec = -1;
@@ -140,6 +149,12 @@ static int detach_kernel_driver_and_claim(struct libusb_device_handle *, int);
 static int linux_default_scan_devices (struct libusb_context *ctx);
 #endif
 
+struct kernel_version {
+       int major;
+       int minor;
+       int sublevel;
+};
+
 struct linux_device_priv {
        char *sysfs_dir;
        unsigned char *descriptors;
@@ -356,39 +371,59 @@ static clockid_t find_monotonic_clock(void)
        return CLOCK_REALTIME;
 }
 
-static int kernel_version_ge(int major, int minor, int sublevel)
+static int get_kernel_version(struct libusb_context *ctx,
+       struct kernel_version *ver)
 {
        struct utsname uts;
-       int atoms, kmajor, kminor, ksublevel;
+       int atoms;
 
-       if (uname(&uts) < 0)
+       if (uname(&uts) < 0) {
+               usbi_err(ctx, "uname failed, errno %d", errno);
                return -1;
-       atoms = sscanf(uts.release, "%d.%d.%d", &kmajor, &kminor, &ksublevel);
-       if (atoms < 1)
+       }
+
+       atoms = sscanf(uts.release, "%d.%d.%d", &ver->major, &ver->minor, &ver->sublevel);
+       if (atoms < 1) {
+               usbi_err(ctx, "failed to parse uname release '%s'", uts.release);
                return -1;
+       }
 
-       if (kmajor > major)
+       if (atoms < 2)
+               ver->minor = -1;
+       if (atoms < 3)
+               ver->sublevel = -1;
+
+       usbi_dbg("reported kernel version is %s", uts.release);
+
+       return 0;
+}
+
+static int kernel_version_ge(const struct kernel_version *ver,
+       int major, int minor, int sublevel)
+{
+       if (ver->major > major)
                return 1;
-       if (kmajor < major)
+       else if (ver->major < major)
                return 0;
 
        /* kmajor == major */
-       if (atoms < 2)
+       if (ver->minor == -1 && ver->sublevel == -1)
                return 0 == minor && 0 == sublevel;
-       if (kminor > minor)
+       else if (ver->minor > minor)
                return 1;
-       if (kminor < minor)
+       else if (ver->minor < minor)
                return 0;
 
        /* kminor == minor */
-       if (atoms < 3)
+       if (ver->sublevel == -1)
                return 0 == sublevel;
 
-       return ksublevel >= sublevel;
+       return ver->sublevel >= sublevel;
 }
 
 static int op_init(struct libusb_context *ctx)
 {
+       struct kernel_version kversion;
        struct stat statbuf;
        int r;
 
@@ -401,22 +436,17 @@ static int op_init(struct libusb_context *ctx)
        if (monotonic_clkid == -1)
                monotonic_clkid = find_monotonic_clock();
 
+       if (get_kernel_version(ctx, &kversion) < 0)
+               return LIBUSB_ERROR_OTHER;
+
        if (supports_flag_cloexec == -1) {
                /* O_CLOEXEC flag available from Linux 2.6.23 */
-               supports_flag_cloexec = kernel_version_ge(2,6,23);
-               if (supports_flag_cloexec == -1) {
-                       usbi_err(ctx, "error checking for O_CLOEXEC support");
-                       return LIBUSB_ERROR_OTHER;
-               }
+               supports_flag_cloexec = kernel_version_ge(&kversion,2,6,23);
        }
 
        if (supports_flag_bulk_continuation == -1) {
                /* bulk continuation URB flag available from Linux 2.6.32 */
-               supports_flag_bulk_continuation = kernel_version_ge(2,6,32);
-               if (supports_flag_bulk_continuation == -1) {
-                       usbi_err(ctx, "error checking for bulk continuation support");
-                       return LIBUSB_ERROR_OTHER;
-               }
+               supports_flag_bulk_continuation = kernel_version_ge(&kversion,2,6,32);
        }
 
        if (supports_flag_bulk_continuation)
@@ -424,32 +454,31 @@ static int op_init(struct libusb_context *ctx)
 
        if (-1 == supports_flag_zero_packet) {
                /* zero length packet URB flag fixed since Linux 2.6.31 */
-               supports_flag_zero_packet = kernel_version_ge(2,6,31);
-               if (-1 == supports_flag_zero_packet) {
-                       usbi_err(ctx, "error checking for zero length packet support");
-                       return LIBUSB_ERROR_OTHER;
-               }
+               supports_flag_zero_packet = kernel_version_ge(&kversion,2,6,31);
        }
 
        if (supports_flag_zero_packet)
                usbi_dbg("zero length packet flag supported");
 
+       if (!max_iso_packet_len) {
+               if (kernel_version_ge(&kversion,3,10,0))
+                       max_iso_packet_len = 49152;
+               else if (kernel_version_ge(&kversion,2,6,18))
+                       max_iso_packet_len = 8192;
+               else
+                       max_iso_packet_len = 1023;
+       }
+
+       usbi_dbg("max iso packet length is (likely) %u bytes", max_iso_packet_len);
+
        if (-1 == sysfs_has_descriptors) {
                /* sysfs descriptors has all descriptors since Linux 2.6.26 */
-               sysfs_has_descriptors = kernel_version_ge(2,6,26);
-               if (-1 == sysfs_has_descriptors) {
-                       usbi_err(ctx, "error checking for sysfs descriptors");
-                       return LIBUSB_ERROR_OTHER;
-               }
+               sysfs_has_descriptors = kernel_version_ge(&kversion,2,6,26);
        }
 
        if (-1 == sysfs_can_relate_devices) {
                /* sysfs has busnum since Linux 2.6.22 */
-               sysfs_can_relate_devices = kernel_version_ge(2,6,22);
-               if (-1 == sysfs_can_relate_devices) {
-                       usbi_err(ctx, "error checking for sysfs busnum");
-                       return LIBUSB_ERROR_OTHER;
-               }
+               sysfs_can_relate_devices = kernel_version_ge(&kversion,2,6,22);
        }
 
        if (sysfs_can_relate_devices || sysfs_has_descriptors) {
@@ -2002,38 +2031,43 @@ static int submit_iso_transfer(struct usbi_transfer *itransfer)
        struct linux_device_handle_priv *dpriv =
                _device_handle_priv(transfer->dev_handle);
        struct usbfs_urb **urbs;
-       size_t alloc_size;
        int num_packets = transfer->num_iso_packets;
-       int i;
-       int this_urb_len = 0;
-       int num_urbs = 1;
-       int packet_offset = 0;
+       int num_packets_remaining;
+       int i, j;
+       int num_urbs;
        unsigned int packet_len;
+       unsigned int total_len = 0;
        unsigned char *urb_buffer = transfer->buffer;
 
+       if (num_packets < 1)
+               return LIBUSB_ERROR_INVALID_PARAM;
+
        /* usbfs places arbitrary limits on iso URBs. this limit has changed
-        * at least three times, and it's difficult to accurately detect which
-        * limit this running kernel might impose. so we attempt to submit
-        * whatever the user has provided. if the kernel rejects the request
-        * due to its size, we return an error indicating such to the user.
+        * at least three times, but we attempt to detect this limit during
+        * init and check it here. if the kernel rejects the request due to
+        * its size, we return an error indicating such to the user.
         */
-
-       /* calculate how many URBs we need */
        for (i = 0; i < num_packets; i++) {
-               unsigned int space_remaining = MAX_ISO_BUFFER_LENGTH - this_urb_len;
                packet_len = transfer->iso_packet_desc[i].length;
 
-               if (packet_len > space_remaining) {
-                       num_urbs++;
-                       this_urb_len = packet_len;
-                       /* check that we can actually support this packet length */
-                       if (this_urb_len > MAX_ISO_BUFFER_LENGTH)
-                               return LIBUSB_ERROR_INVALID_PARAM;
-               } else {
-                       this_urb_len += packet_len;
+               if (packet_len > max_iso_packet_len) {
+                       usbi_warn(TRANSFER_CTX(transfer),
+                               "iso packet length of %u bytes exceeds maximum of %u bytes",
+                               packet_len, max_iso_packet_len);
+                       return LIBUSB_ERROR_INVALID_PARAM;
                }
+
+               total_len += packet_len;
        }
-       usbi_dbg("need %d %dk URBs for transfer", num_urbs, MAX_ISO_BUFFER_LENGTH / 1024);
+
+       if (transfer->length < (int)total_len)
+               return LIBUSB_ERROR_INVALID_PARAM;
+
+       /* usbfs limits the number of iso packets per URB */
+       num_urbs = (num_packets + (MAX_ISO_PACKETS_PER_URB - 1)) / MAX_ISO_PACKETS_PER_URB;
+
+       usbi_dbg("need %d urbs for new transfer with length %d", num_urbs,
+               transfer->length);
 
        urbs = calloc(num_urbs, sizeof(*urbs));
        if (!urbs)
@@ -2046,31 +2080,15 @@ static int submit_iso_transfer(struct usbi_transfer *itransfer)
        tpriv->iso_packet_offset = 0;
 
        /* allocate + initialize each URB with the correct number of packets */
-       for (i = 0; i < num_urbs; i++) {
+       num_packets_remaining = num_packets;
+       for (i = 0, j = 0; i < num_urbs; i++) {
+               int num_packets_in_urb = MIN(num_packets_remaining, MAX_ISO_PACKETS_PER_URB);
                struct usbfs_urb *urb;
-               unsigned int space_remaining_in_urb = MAX_ISO_BUFFER_LENGTH;
-               int urb_packet_offset = 0;
-               unsigned char *urb_buffer_orig = urb_buffer;
-               int j;
+               size_t alloc_size;
                int k;
 
-               /* swallow up all the packets we can fit into this URB */
-               while (packet_offset < transfer->num_iso_packets) {
-                       packet_len = transfer->iso_packet_desc[packet_offset].length;
-                       if (packet_len <= space_remaining_in_urb) {
-                               /* throw it in */
-                               urb_packet_offset++;
-                               packet_offset++;
-                               space_remaining_in_urb -= packet_len;
-                               urb_buffer += packet_len;
-                       } else {
-                               /* it can't fit, save it for the next URB */
-                               break;
-                       }
-               }
-
                alloc_size = sizeof(*urb)
-                       + (urb_packet_offset * sizeof(struct usbfs_iso_packet_desc));
+                       + (num_packets_in_urb * sizeof(struct usbfs_iso_packet_desc));
                urb = calloc(1, alloc_size);
                if (!urb) {
                        free_iso_urbs(tpriv);
@@ -2079,11 +2097,10 @@ static int submit_iso_transfer(struct usbi_transfer *itransfer)
                urbs[i] = urb;
 
                /* populate packet lengths */
-               for (j = 0, k = packet_offset - urb_packet_offset;
-                               k < packet_offset; k++, j++) {
-                       packet_len = transfer->iso_packet_desc[k].length;
+               for (k = 0; k < num_packets_in_urb; j++, k++) {
+                       packet_len = transfer->iso_packet_desc[j].length;
                        urb->buffer_length += packet_len;
-                       urb->iso_frame_desc[j].length = packet_len;
+                       urb->iso_frame_desc[k].length = packet_len;
                }
 
                urb->usercontext = itransfer;
@@ -2091,8 +2108,11 @@ static int submit_iso_transfer(struct usbi_transfer *itransfer)
                /* FIXME: interface for non-ASAP data? */
                urb->flags = USBFS_URB_ISO_ASAP;
                urb->endpoint = transfer->endpoint;
-               urb->number_of_packets = urb_packet_offset;
-               urb->buffer = urb_buffer_orig;
+               urb->number_of_packets = num_packets_in_urb;
+               urb->buffer = urb_buffer;
+
+               urb_buffer += urb->buffer_length;
+               num_packets_remaining -= num_packets_in_urb;
        }
 
        /* submit URBs */
@@ -2105,6 +2125,10 @@ static int submit_iso_transfer(struct usbi_transfer *itransfer)
                                usbi_warn(TRANSFER_CTX(transfer),
                                        "submiturb failed, transfer too large");
                                r = LIBUSB_ERROR_INVALID_PARAM;
+                       } else if (errno == EMSGSIZE) {
+                               usbi_warn(TRANSFER_CTX(transfer),
+                                       "submiturb failed, iso packet length too large");
+                               r = LIBUSB_ERROR_INVALID_PARAM;
                        } else {
                                usbi_err(TRANSFER_CTX(transfer),
                                        "submiturb failed error %d errno=%d", r, errno);
index 8bd3ebc..2449632 100644 (file)
@@ -81,10 +81,11 @@ struct usbfs_iso_packet_desc {
        unsigned int status;
 };
 
-#define MAX_ISO_BUFFER_LENGTH          49152 * 128
 #define MAX_BULK_BUFFER_LENGTH         16384
 #define MAX_CTRL_BUFFER_LENGTH         4096
 
+#define MAX_ISO_PACKETS_PER_URB                128
+
 struct usbfs_urb {
        unsigned char type;
        unsigned char endpoint;
index ae0b23d..bc3ae8f 100644 (file)
@@ -1 +1 @@
-#define LIBUSB_NANO 11229
+#define LIBUSB_NANO 11230