From 2f8c4c5494c2589e6cb9a62f399e61a1c4c2378d Mon Sep 17 00:00:00 2001 From: Arjan Mels Date: Tue, 5 Apr 2011 20:24:56 +0200 Subject: staging: usbip: fix shutdown problems. When shuting down the tcp_rx and tcp_tx threads first check if they are not closed already (maybe because an error caused them to return). Signed-off-by: Arjan Mels Cc: Takahiro Hirofuchi Cc: Max Vozeler Cc: Arnd Bergmann Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/staging/usbip/stub_dev.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/staging/usbip') diff --git a/drivers/staging/usbip/stub_dev.c b/drivers/staging/usbip/stub_dev.c index 8214c353d9f5..bce7d039346c 100644 --- a/drivers/staging/usbip/stub_dev.c +++ b/drivers/staging/usbip/stub_dev.c @@ -220,8 +220,10 @@ static void stub_shutdown_connection(struct usbip_device *ud) } /* 1. stop threads */ - kthread_stop(ud->tcp_rx); - kthread_stop(ud->tcp_tx); + if (ud->tcp_rx && !task_is_dead(ud->tcp_rx)) + kthread_stop(ud->tcp_rx); + if (ud->tcp_tx && !task_is_dead(ud->tcp_tx)) + kthread_stop(ud->tcp_tx); /* 2. close the socket */ /* -- cgit v1.2.3 From d2dd0b07c3e725d386d20294ec906f7ddef207fa Mon Sep 17 00:00:00 2001 From: Arjan Mels Date: Tue, 5 Apr 2011 20:26:11 +0200 Subject: staging: usbip: bugfixes related to kthread conversion When doing a usb port reset do a queued reset instead to prevent a deadlock: the reset will cause the driver to unbind, causing the usb_driver_lock_for_reset to stall. Signed-off-by: Arjan Mels Cc: Takahiro Hirofuchi Cc: Max Vozeler Cc: Arnd Bergmann Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/staging/usbip/stub_rx.c | 40 +++++++++++++++------------------------- 1 file changed, 15 insertions(+), 25 deletions(-) (limited to 'drivers/staging/usbip') diff --git a/drivers/staging/usbip/stub_rx.c b/drivers/staging/usbip/stub_rx.c index 6445f12cb4fd..51fbd0986475 100644 --- a/drivers/staging/usbip/stub_rx.c +++ b/drivers/staging/usbip/stub_rx.c @@ -171,33 +171,23 @@ static int tweak_set_configuration_cmd(struct urb *urb) static int tweak_reset_device_cmd(struct urb *urb) { - struct usb_ctrlrequest *req; - __u16 value; - __u16 index; - int ret; - - req = (struct usb_ctrlrequest *) urb->setup_packet; - value = le16_to_cpu(req->wValue); - index = le16_to_cpu(req->wIndex); - - usbip_uinfo("reset_device (port %d) to %s\n", index, - dev_name(&urb->dev->dev)); + struct stub_priv *priv = (struct stub_priv *) urb->context; + struct stub_device *sdev = priv->sdev; - /* all interfaces should be owned by usbip driver, so just reset it. */ - ret = usb_lock_device_for_reset(urb->dev, NULL); - if (ret < 0) { - dev_err(&urb->dev->dev, "lock for reset\n"); - return ret; - } - - /* try to reset the device */ - ret = usb_reset_device(urb->dev); - if (ret < 0) - dev_err(&urb->dev->dev, "device reset\n"); + usbip_uinfo("reset_device %s\n", dev_name(&urb->dev->dev)); - usb_unlock_device(urb->dev); - - return ret; + /* + * usb_lock_device_for_reset caused a deadlock: it causes the driver + * to unbind. In the shutdown the rx thread is signalled to shut down + * but this thread is pending in the usb_lock_device_for_reset. + * + * Instead queue the reset. + * + * Unfortunatly an existing usbip connection will be dropped due to + * driver unbinding. + */ + usb_queue_reset_device(sdev->interface); + return 0; } /* -- cgit v1.2.3 From 1325f85fa49f57df034869de430f7c302ae23109 Mon Sep 17 00:00:00 2001 From: Arjan Mels Date: Tue, 5 Apr 2011 20:26:38 +0200 Subject: staging: usbip: bugfix add number of packets for isochronous frames The number_of_packets was not transmitted for RET_SUBMIT packets. The linux client used the stored number_of_packet from the submitted request. The windows userland client does not do this however and needs to know the number_of_packets to determine the size of the transmission. Signed-off-by: Arjan Mels Cc: Takahiro Hirofuchi Cc: Max Vozeler Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/staging/usbip/usbip_common.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers/staging/usbip') diff --git a/drivers/staging/usbip/usbip_common.c b/drivers/staging/usbip/usbip_common.c index 337abc48f714..85622153ea7c 100644 --- a/drivers/staging/usbip/usbip_common.c +++ b/drivers/staging/usbip/usbip_common.c @@ -333,10 +333,11 @@ void usbip_dump_header(struct usbip_header *pdu) usbip_udbg("CMD_UNLINK: seq %u\n", pdu->u.cmd_unlink.seqnum); break; case USBIP_RET_SUBMIT: - usbip_udbg("RET_SUBMIT: st %d al %u sf %d ec %d\n", + usbip_udbg("RET_SUBMIT: st %d al %u sf %d #p %d ec %d\n", pdu->u.ret_submit.status, pdu->u.ret_submit.actual_length, pdu->u.ret_submit.start_frame, + pdu->u.ret_submit.number_of_packets, pdu->u.ret_submit.error_count); case USBIP_RET_UNLINK: usbip_udbg("RET_UNLINK: status %d\n", pdu->u.ret_unlink.status); @@ -520,6 +521,7 @@ static void usbip_pack_ret_submit(struct usbip_header *pdu, struct urb *urb, rpdu->status = urb->status; rpdu->actual_length = urb->actual_length; rpdu->start_frame = urb->start_frame; + rpdu->number_of_packets = urb->number_of_packets; rpdu->error_count = urb->error_count; } else { /* vhci_rx.c */ @@ -527,6 +529,7 @@ static void usbip_pack_ret_submit(struct usbip_header *pdu, struct urb *urb, urb->status = rpdu->status; urb->actual_length = rpdu->actual_length; urb->start_frame = rpdu->start_frame; + urb->number_of_packets = rpdu->number_of_packets; urb->error_count = rpdu->error_count; } } @@ -595,11 +598,13 @@ static void correct_endian_ret_submit(struct usbip_header_ret_submit *pdu, cpu_to_be32s(&pdu->status); cpu_to_be32s(&pdu->actual_length); cpu_to_be32s(&pdu->start_frame); + cpu_to_be32s(&pdu->number_of_packets); cpu_to_be32s(&pdu->error_count); } else { be32_to_cpus(&pdu->status); be32_to_cpus(&pdu->actual_length); be32_to_cpus(&pdu->start_frame); + cpu_to_be32s(&pdu->number_of_packets); be32_to_cpus(&pdu->error_count); } } -- cgit v1.2.3 From 28276a28d8b3cd19f4449991faad4945fe557656 Mon Sep 17 00:00:00 2001 From: Arjan Mels Date: Tue, 5 Apr 2011 20:26:59 +0200 Subject: 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 Cc: Takahiro Hirofuchi Cc: Max Vozeler Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/staging/usbip/stub_tx.c | 74 +++++++++++++++++++++++++++++------- drivers/staging/usbip/usbip_common.c | 57 +++++++++++++++++++++++++++ drivers/staging/usbip/usbip_common.h | 2 + drivers/staging/usbip/vhci_rx.c | 3 ++ 4 files changed, 122 insertions(+), 14 deletions(-) (limited to 'drivers/staging/usbip') diff --git a/drivers/staging/usbip/stub_tx.c b/drivers/staging/usbip/stub_tx.c index 5523f25998e6..64a52b26dcf6 100644 --- 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 85622153ea7c..7b1fe45bf7dd 100644 --- 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 9f809c315d92..c767f52be5fb 100644 --- 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 09bf2355934b..2ffc96a4c0d4 100644 --- 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); -- cgit v1.2.3