staging: usbip: bugfix for isochronous packets and optimization

For isochronous packets the actual_length is the sum of the actual
length of each of the packets, however between the packets might be
padding, so it is not sufficient to just send the first actual_length
bytes of the buffer. To fix this and simultanesouly optimize the
bandwidth the content of the isochronous packets are send without the
padding, the padding is restored on the receiving end.

Signed-off-by: Arjan Mels <arjan.mels@gmx.net>
Cc: Takahiro Hirofuchi <hirofuchi@users.sourceforge.net>
Cc: Max Vozeler <max@vozeler.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/staging/usbip/stub_tx.c b/drivers/staging/usbip/stub_tx.c
index 5523f25998e6fcbae42de8490a7cee68f47506ca..64a52b26dcf6d7fe9d185372393a11f8af0e0149 100644 (file)
--- a/drivers/staging/usbip/stub_tx.c
+++ b/drivers/staging/usbip/stub_tx.c
@@ -170,7 +170,6 @@ static int stub_send_ret_submit(struct stub_device *sdev)
        struct stub_priv *priv, *tmp;
 
        struct msghdr msg;
-       struct kvec iov[3];
        size_t txsize;
 
        size_t total_size = 0;
@@ -180,28 +179,73 @@ static int stub_send_ret_submit(struct stub_device *sdev)
                struct urb *urb = priv->urb;
                struct usbip_header pdu_header;
                void *iso_buffer = NULL;
+               struct kvec *iov = NULL;
+               int iovnum = 0;
 
                txsize = 0;
                memset(&pdu_header, 0, sizeof(pdu_header));
                memset(&msg, 0, sizeof(msg));
-               memset(&iov, 0, sizeof(iov));
 
-               usbip_dbg_stub_tx("setup txdata urb %p\n", urb);
+               if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS)
+                       iovnum = 2 + urb->number_of_packets;
+               else
+                       iovnum = 2;
+
+               iov = kzalloc(iovnum * sizeof(struct kvec), GFP_KERNEL);
 
+               if (!iov) {
+                       usbip_event_add(&sdev->ud, SDEV_EVENT_ERROR_MALLOC);
+                       return -1;
+               }
+
+               iovnum = 0;
 
                /* 1. setup usbip_header */
                setup_ret_submit_pdu(&pdu_header, urb);
+               usbip_dbg_stub_tx("setup txdata seqnum: %d urb: %p\n",
+                                               pdu_header.base.seqnum, urb);
+               /*usbip_dump_header(pdu_header);*/
                usbip_header_correct_endian(&pdu_header, 1);
 
-               iov[0].iov_base = &pdu_header;
-               iov[0].iov_len  = sizeof(pdu_header);
+               iov[iovnum].iov_base = &pdu_header;
+               iov[iovnum].iov_len  = sizeof(pdu_header);
+               iovnum++;
                txsize += sizeof(pdu_header);
 
                /* 2. setup transfer buffer */
-               if (usb_pipein(urb->pipe) && urb->actual_length > 0) {
-                       iov[1].iov_base = urb->transfer_buffer;
-                       iov[1].iov_len  = urb->actual_length;
+               if (usb_pipein(urb->pipe) &&
+                               usb_pipetype(urb->pipe) != PIPE_ISOCHRONOUS &&
+                                       urb->actual_length > 0) {
+                       iov[iovnum].iov_base = urb->transfer_buffer;
+                       iov[iovnum].iov_len  = urb->actual_length;
+                       iovnum++;
                        txsize += urb->actual_length;
+               } else if (usb_pipein(urb->pipe) &&
+                               usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS) {
+                       /*
+                        * For isochronous packets: actual length is the sum of
+                        * the actual length of the individual, packets, but as
+                        * the packet offsets are not changed there will be
+                        * padding between the packets. To optimally use the
+                        * bandwidth the padding is not transmitted.
+                        */
+
+                       int i;
+                       for (i = 0; i < urb->number_of_packets; i++) {
+                               iov[iovnum].iov_base = urb->transfer_buffer + urb->iso_frame_desc[i].offset;
+                               iov[iovnum].iov_len = urb->iso_frame_desc[i].actual_length;
+                               iovnum++;
+                               txsize += urb->iso_frame_desc[i].actual_length;
+                       }
+
+                       if (txsize != sizeof(pdu_header) + urb->actual_length) {
+                               dev_err(&sdev->interface->dev,
+                                       "actual length of urb (%d) does not match iso packet sizes (%d)\n",
+                                       urb->actual_length, txsize-sizeof(pdu_header));
+                               kfree(iov);
+                               usbip_event_add(&sdev->ud, SDEV_EVENT_ERROR_TCP);
+                          return -1;
+                       }
                }
 
                /* 3. setup iso_packet_descriptor */
@@ -212,32 +256,34 @@ static int stub_send_ret_submit(struct stub_device *sdev)
                        if (!iso_buffer) {
                                usbip_event_add(&sdev->ud,
                                                SDEV_EVENT_ERROR_MALLOC);
+                               kfree(iov);
                                return -1;
                        }
 
-                       iov[2].iov_base = iso_buffer;
-                       iov[2].iov_len  = len;
+                       iov[iovnum].iov_base = iso_buffer;
+                       iov[iovnum].iov_len  = len;
                        txsize += len;
+                       iovnum++;
                }
 
-               ret = kernel_sendmsg(sdev->ud.tcp_socket, &msg, iov,
-                                    3, txsize);
+               ret = kernel_sendmsg(sdev->ud.tcp_socket, &msg,
+                                               iov,  iovnum, txsize);
                if (ret != txsize) {
                        dev_err(&sdev->interface->dev,
                                "sendmsg failed!, retval %d for %zd\n",
                                ret, txsize);
+                       kfree(iov);
                        kfree(iso_buffer);
                        usbip_event_add(&sdev->ud, SDEV_EVENT_ERROR_TCP);
                        return -1;
                }
 
+               kfree(iov);
                kfree(iso_buffer);
-               usbip_dbg_stub_tx("send txdata\n");
 
                total_size += txsize;
        }
 
-
        spin_lock_irqsave(&sdev->priv_lock, flags);
 
        list_for_each_entry_safe(priv, tmp, &sdev->priv_free, list) {

diff --git a/drivers/staging/usbip/usbip_common.c b/drivers/staging/usbip/usbip_common.c
index 85622153ea7ca96708501c4c28118011ab72acb9..7b1fe45bf7ddf71c733c9853ed30c5fe26c5d98a 100644 (file)
--- a/drivers/staging/usbip/usbip_common.c
+++ b/drivers/staging/usbip/usbip_common.c
@@ -730,6 +730,7 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
        int size = np * sizeof(*iso);
        int i;
        int ret;
+       int total_length = 0;
 
        if (!usb_pipeisoc(urb->pipe))
                return 0;
@@ -759,19 +760,75 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
                return -EPIPE;
        }
 
+
        for (i = 0; i < np; i++) {
                iso = buff + (i * sizeof(*iso));
 
                usbip_iso_pakcet_correct_endian(iso, 0);
                usbip_pack_iso(iso, &urb->iso_frame_desc[i], 0);
+               total_length += urb->iso_frame_desc[i].actual_length;
        }
 
        kfree(buff);
 
+       if (total_length != urb->actual_length) {
+               dev_err(&urb->dev->dev,
+                 "total length of iso packets (%d) not equal to actual length of buffer (%d)\n",
+                 total_length, urb->actual_length);
+
+               if (ud->side == USBIP_STUB)
+                       usbip_event_add(ud, SDEV_EVENT_ERROR_TCP);
+               else
+                       usbip_event_add(ud, VDEV_EVENT_ERROR_TCP);
+
+               return -EPIPE;
+       }
+
        return ret;
 }
 EXPORT_SYMBOL_GPL(usbip_recv_iso);
 
+/*
+ * This functions restores the padding which was removed for optimizing
+ * the bandwidth during transfer over tcp/ip
+ *
+ * buffer and iso packets need to be stored and be in propeper endian in urb
+ * before calling this function
+ */
+int usbip_pad_iso(struct usbip_device *ud, struct urb *urb)
+{
+       int np = urb->number_of_packets;
+       int i;
+       int ret;
+       int actualoffset = urb->actual_length;
+
+       if (!usb_pipeisoc(urb->pipe))
+               return 0;
+
+       /* if no packets or length of data is 0, then nothing to unpack */
+       if (np == 0 || urb->actual_length == 0)
+               return 0;
+
+       /*
+        * if actual_length is transfer_buffer_length then no padding is
+        * present.
+       */
+       if (urb->actual_length == urb->transfer_buffer_length)
+               return 0;
+
+       /*
+        * loop over all packets from last to first (to prevent overwritting
+        * memory when padding) and move them into the proper place
+        */
+       for (i = np-1; i > 0; i--) {
+               actualoffset -= urb->iso_frame_desc[i].actual_length;
+               memmove(urb->transfer_buffer + urb->iso_frame_desc[i].offset,
+                                 urb->transfer_buffer + actualoffset,
+                                 urb->iso_frame_desc[i].actual_length);
+       }
+       return ret;
+}
+EXPORT_SYMBOL_GPL(usbip_pad_iso);
 
 /* some members of urb must be substituted before. */
 int usbip_recv_xbuff(struct usbip_device *ud, struct urb *urb)

diff --git a/drivers/staging/usbip/usbip_common.h b/drivers/staging/usbip/usbip_common.h
index 9f809c315d92fc15f62ac94e2b63f07c74147bfa..c767f52be5fb1499d341a574cfc33de052ae34bd 100644 (file)
--- a/drivers/staging/usbip/usbip_common.h
+++ b/drivers/staging/usbip/usbip_common.h
@@ -379,6 +379,8 @@ void usbip_header_correct_endian(struct usbip_header *pdu, int send);
 int usbip_recv_xbuff(struct usbip_device *ud, struct urb *urb);
 /* some members of urb must be substituted before. */
 int usbip_recv_iso(struct usbip_device *ud, struct urb *urb);
+/* some members of urb must be substituted before. */
+int usbip_pad_iso(struct usbip_device *ud, struct urb *urb);
 void *usbip_alloc_iso_desc_pdu(struct urb *urb, ssize_t *bufflen);
 
 

diff --git a/drivers/staging/usbip/vhci_rx.c b/drivers/staging/usbip/vhci_rx.c
index 09bf2355934bb6834e67641ab679faec43e88818..2ffc96a4c0d4e3c11a6e0836d51f9b41af350770 100644 (file)
--- a/drivers/staging/usbip/vhci_rx.c
+++ b/drivers/staging/usbip/vhci_rx.c
@@ -100,6 +100,9 @@ static void vhci_recv_ret_submit(struct vhci_device *vdev,
        if (usbip_recv_iso(ud, urb) < 0)
                return;
 
+       /* restore the padding in iso packets */
+       if (usbip_pad_iso(ud, urb) < 0)
+               return;
 
        if (usbip_dbg_flag_vhci_rx)
                usbip_dump_urb(urb);