Re: [PATCH v1] USBNET: fix handling padding packet

2013-09-28 Thread David Miller
From: Ming Lei ming@canonical.com
Date: Mon, 23 Sep 2013 20:59:35 +0800

 Commit 638c5115a7949(USBNET: support DMA SG) introduces DMA SG
 if the usb host controller is capable of building packet from
 discontinuous buffers, but missed handling padding packet when
 building DMA SG.
 
 This patch attachs the pre-allocated padding packet at the
 end of the sg list, so padding packet can be sent to device
 if drivers require that.
 
 Reported-by: David Laight david.lai...@aculab.com
 Acked-by: Oliver Neukum oli...@neukum.org
 Signed-off-by: Ming Lei ming@canonical.com

Applied, thanks.

I still think the suggestion to disable scatter gather for
devices with the padding issue was the most sane approach
to solve this.

I guess people like supporting complicated crap and excess
code for things that pretty much do not exist, or at best
are not prominent enough to cater for at all.

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v1] USBNET: fix handling padding packet

2013-09-23 Thread Ming Lei
Commit 638c5115a7949(USBNET: support DMA SG) introduces DMA SG
if the usb host controller is capable of building packet from
discontinuous buffers, but missed handling padding packet when
building DMA SG.

This patch attachs the pre-allocated padding packet at the
end of the sg list, so padding packet can be sent to device
if drivers require that.

Reported-by: David Laight david.lai...@aculab.com
Acked-by: Oliver Neukum oli...@neukum.org
Signed-off-by: Ming Lei ming@canonical.com
---
v1:
- fix bug in case of dev-can_dma_sg and !urb-num_sgs
---
 drivers/net/usb/usbnet.c   |   27 +--
 include/linux/usb/usbnet.h |1 +
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 7b331e6..bf94e10 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1241,7 +1241,9 @@ static int build_dma_sg(const struct sk_buff *skb, struct 
urb *urb)
if (num_sgs == 1)
return 0;
 
-   urb-sg = kmalloc(num_sgs * sizeof(struct scatterlist), GFP_ATOMIC);
+   /* reserve one for zero packet */
+   urb-sg = kmalloc((num_sgs + 1) * sizeof(struct scatterlist),
+ GFP_ATOMIC);
if (!urb-sg)
return -ENOMEM;
 
@@ -1305,7 +1307,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (build_dma_sg(skb, urb)  0)
goto drop;
}
-   entry-length = length = urb-transfer_buffer_length;
+   length = urb-transfer_buffer_length;
 
/* don't assume the hardware handles USB_ZERO_PACKET
 * NOTE:  strictly conforming cdc-ether devices should expect
@@ -1317,15 +1319,18 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (length % dev-maxpacket == 0) {
if (!(info-flags  FLAG_SEND_ZLP)) {
if (!(info-flags  FLAG_MULTI_PACKET)) {
-   urb-transfer_buffer_length++;
-   if (skb_tailroom(skb)) {
+   length++;
+   if (skb_tailroom(skb)  !urb-num_sgs) {
skb-data[skb-len] = 0;
__skb_put(skb, 1);
-   }
+   } else if (urb-num_sgs)
+   sg_set_buf(urb-sg[urb-num_sgs++],
+   dev-padding_pkt, 1);
}
} else
urb-transfer_flags |= URB_ZERO_PACKET;
}
+   entry-length = urb-transfer_buffer_length = length;
 
spin_lock_irqsave(dev-txq.lock, flags);
retval = usb_autopm_get_interface_async(dev-intf);
@@ -1509,6 +1514,7 @@ void usbnet_disconnect (struct usb_interface *intf)
 
usb_kill_urb(dev-interrupt);
usb_free_urb(dev-interrupt);
+   kfree(dev-padding_pkt);
 
free_netdev(net);
 }
@@ -1679,9 +1685,16 @@ usbnet_probe (struct usb_interface *udev, const struct 
usb_device_id *prod)
/* initialize max rx_qlen and tx_qlen */
usbnet_update_max_qlen(dev);
 
+   if (dev-can_dma_sg  !(info-flags  FLAG_SEND_ZLP) 
+   !(info-flags  FLAG_MULTI_PACKET)) {
+   dev-padding_pkt = kzalloc(1, GFP_KERNEL);
+   if (!dev-padding_pkt)
+   goto out4;
+   }
+
status = register_netdev (net);
if (status)
-   goto out4;
+   goto out5;
netif_info(dev, probe, dev-net,
   register '%s' at usb-%s-%s, %s, %pM\n,
   udev-dev.driver-name,
@@ -1699,6 +1712,8 @@ usbnet_probe (struct usb_interface *udev, const struct 
usb_device_id *prod)
 
return 0;
 
+out5:
+   kfree(dev-padding_pkt);
 out4:
usb_free_urb(dev-interrupt);
 out3:
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 9cb2fe8..e303eef 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -42,6 +42,7 @@ struct usbnet {
struct usb_host_endpoint *status;
unsignedmaxpacket;
struct timer_list   delay;
+   const char  *padding_pkt;
 
/* protocol/interface state */
struct net_device   *net;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html