[PATCH v4 0/4] USB & USBNET: loose SG check and support usbnet DMA SG

[PATCH v4 0/4] USB & USBNET: loose SG check and support usbnet DMA SG

[Ming Lei]

Hi,

This patchset allows drivers to pass sg buffers which size can't be divided
by max packet size of endpoint if the host controllers(such ax xHCI) support
this kind of sg buffers.

Previously we added check[1] on the situation and don't allow these sg buffers
passed to USB HCD, looks the check is too strict to make use of new feature of
new hardware(xHCI) for some applications(such as network stack) which can't
provide this kind of sg buffers to usbnet driver, so the patch looses the check
in case that the host controller supports it.

Patch 3/4 implements DMA SG on usbnet driver, and patch 4/4 uses it on ax88179_178a
USB3 NIC for supporting TSO, so both CPU utilization and tx throughput can be
improved with TSO and DMA SG in case of the USB NIC is attached to xHCI controller.

This patchset depends on both net-next and usb-next tree, so hope David and Greg
to figure out one elegent way to merge it.

[1], http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=10e232c597ac757e7f8600649f7e872e86de190f

V4:
	- don't set NETIF_F_SG | NETIF_F_TSO in reset() callback
	as pointed out by Eric(only 4/4 changed)

V3:
        - save 3 lines code for usb_device_no_sg_constraint() as suggested by Alan
        - fix urb->sg leak in xmit failure path

V2:
        - add missed kfree(urb->sg) in 3/4
        - rename no_sg_limit as no_sg_constraint as suggested by Alan

V1:
        - introduce and apply usb_device_no_sg_limit() helper as suggested by Greg
        - simplify patch 4/4 against Eric Dumazet's patch(ax88179_178a: avoid copy of tx tcp packets)
        - don't pass usbnet header as sg buffer

 drivers/net/usb/ax88179_178a.c |    8 +++++++
 drivers/net/usb/usbnet.c       |   45 +++++++++++++++++++++++++++++++++++++---
 drivers/usb/core/urb.c         |    3 ++-
 drivers/usb/host/xhci.c        |    4 ++++
 include/linux/usb.h            |    8 ++++++-
 include/linux/usb/usbnet.h     |    1 +
 6 files changed, 64 insertions(+), 5 deletions(-)



Thanks,
--
Ming Lei

[PATCH v4 1/4] USB: introduce usb_device_no_sg_constraint() helper

[Ming Lei]

Some host controllers(such as xHCI) can support building
packet from discontinuous buffers, so introduce one flag
and helper for this kind of host controllers, then the
feature can help some applications(such as usbnet) by
supporting arbitrary length of sg buffers.

Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/usb/core/urb.c |    3 ++-
 include/linux/usb.h    |    8 +++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index e75115a..c77ec78 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -414,7 +414,8 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
 			urb->iso_frame_desc[n].status = -EXDEV;
 			urb->iso_frame_desc[n].actual_length = 0;
 		}
-	} else if (dev->speed != USB_SPEED_WIRELESS && urb->num_sgs) {
+	} else if (urb->num_sgs && !urb->dev->bus->no_sg_constraint &&
+			dev->speed != USB_SPEED_WIRELESS) {
 		struct scatterlist *sg;
 		int i;
 
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 84f14e2..bbd2c8d 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -337,6 +337,7 @@ struct usb_bus {
 					 * the ep queue on a short transfer
 					 * with the URB_SHORT_NOT_OK flag set.
 					 */
+	unsigned no_sg_constraint:1;	/* no sg constraint */
 	unsigned sg_tablesize;		/* 0 or largest number of sg list entries */
 
 	int devnum_next;		/* Next open device number in
@@ -684,6 +685,11 @@ static inline bool usb_device_supports_ltm(struct usb_device *udev)
 	return udev->bos->ss_cap->bmAttributes & USB_LTM_SUPPORT;
 }
 
+static inline bool usb_device_no_sg_constraint(struct usb_device *udev)
+{
+	return udev && udev->bus && udev->bus->no_sg_constraint;
+}
+
 
 /*-------------------------------------------------------------------------*/
 
@@ -1249,7 +1255,7 @@ typedef void (*usb_complete_t)(struct urb *);
  *	transfer_buffer.
  * @sg: scatter gather buffer list, the buffer size of each element in
  * 	the list (except the last) must be divisible by the endpoint's
- * 	max packet size
+ * 	max packet size if no_sg_constraint isn't set in 'struct usb_bus'
  * @num_mapped_sgs: (internal) number of mapped sg entries
  * @num_sgs: number of entries in the sg list
  * @transfer_buffer_length: How big is transfer_buffer.  The transfer may
-- 
1.7.9.5

[PATCH v4 2/4] USB: XHCI: mark no_sg_constraint

[Ming Lei]

This patch marks all xHCI controllers as no_sg_constraint
since xHCI supports building packet from discontinuous buffers.

Cc: Alan Stern <stern@rowland.harvard.edu>
Acked-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/usb/host/xhci.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2c49f00..6e2ac57 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4841,6 +4841,10 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 
 	/* Accept arbitrarily long scatter-gather lists */
 	hcd->self.sg_tablesize = ~0;
+
+	/* support to build packet from discontinuous buffers */
+	hcd->self.no_sg_constraint = 1;
+
 	/* XHCI controllers don't stop the ep queue on short packets :| */
 	hcd->self.no_stop_on_short = 1;
 
-- 
1.7.9.5

[PATCH v4 3/4] USBNET: support DMA SG

[Ming Lei]

This patch introduces support of DMA SG if the USB host controller
which usbnet device is attached to is capable of building packet from
discontinuous buffers.

The patch supports passing the skb fragment buffers to usb stack directly
via urb->sg.

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Ben Hutchings <bhutchings@solarflare.com>
Cc: Grant Grundler <grundler@google.com>
Cc: Freddy Xin <freddy@asix.com.tw>
Cc: Alan Stern <stern@rowland.harvard.edu>
Acked-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/usbnet.c   |   45 +++++++++++++++++++++++++++++++++++++++++---
 include/linux/usb/usbnet.h |    1 +
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index e4811d7..534b60b 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1232,6 +1232,37 @@ EXPORT_SYMBOL_GPL(usbnet_tx_timeout);
 
 /*-------------------------------------------------------------------------*/
 
+static int build_dma_sg(const struct sk_buff *skb, struct urb *urb)
+{
+	unsigned num_sgs, total_len = 0;
+	int i, s = 0;
+
+	num_sgs = skb_shinfo(skb)->nr_frags + 1;
+	if (num_sgs == 1)
+		return 0;
+
+	urb->sg = kmalloc(num_sgs * sizeof(struct scatterlist), GFP_ATOMIC);
+	if (!urb->sg)
+		return -ENOMEM;
+
+	urb->num_sgs = num_sgs;
+	sg_init_table(urb->sg, urb->num_sgs);
+
+	sg_set_buf(&urb->sg[s++], skb->data, skb_headlen(skb));
+	total_len += skb_headlen(skb);
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		struct skb_frag_struct *f = &skb_shinfo(skb)->frags[i];
+
+		total_len += skb_frag_size(f);
+		sg_set_page(&urb->sg[i + s], f->page.p, f->size,
+				f->page_offset);
+	}
+	urb->transfer_buffer_length = total_len;
+
+	return 1;
+}
+
 netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
 				     struct net_device *net)
 {
@@ -1258,7 +1289,6 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
 			goto drop;
 		}
 	}
-	length = skb->len;
 
 	if (!(urb = usb_alloc_urb (0, GFP_ATOMIC))) {
 		netif_dbg(dev, tx_err, dev->net, "no urb\n");
@@ -1268,10 +1298,14 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
 	entry = (struct skb_data *) skb->cb;
 	entry->urb = urb;
 	entry->dev = dev;
-	entry->length = length;
 
 	usb_fill_bulk_urb (urb, dev->udev, dev->out,
 			skb->data, skb->len, tx_complete, skb);
+	if (dev->can_dma_sg) {
+		if (build_dma_sg(skb, urb) < 0)
+			goto drop;
+	}
+	entry->length = length = urb->transfer_buffer_length;
 
 	/* don't assume the hardware handles USB_ZERO_PACKET
 	 * NOTE:  strictly conforming cdc-ether devices should expect
@@ -1340,7 +1374,10 @@ drop:
 not_drop:
 		if (skb)
 			dev_kfree_skb_any (skb);
-		usb_free_urb (urb);
+		if (urb) {
+			kfree(urb->sg);
+			usb_free_urb(urb);
+		}
 	} else
 		netif_dbg(dev, tx_queued, dev->net,
 			  "> tx, len %d, type 0x%x\n", length, skb->protocol);
@@ -1391,6 +1428,7 @@ static void usbnet_bh (unsigned long param)
 			rx_process (dev, skb);
 			continue;
 		case tx_done:
+			kfree(entry->urb->sg);
 		case rx_cleanup:
 			usb_free_urb (entry->urb);
 			dev_kfree_skb (skb);
@@ -1727,6 +1765,7 @@ int usbnet_resume (struct usb_interface *intf)
 			retval = usb_submit_urb(res, GFP_ATOMIC);
 			if (retval < 0) {
 				dev_kfree_skb_any(skb);
+				kfree(res->sg);
 				usb_free_urb(res);
 				usb_autopm_put_interface_async(dev->intf);
 			} else {
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 8fbc008..9cb2fe8 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -35,6 +35,7 @@ struct usbnet {
 	unsigned char		suspend_count;
 	unsigned char		pkt_cnt, pkt_err;
 	unsigned short		rx_qlen, tx_qlen;
+	unsigned		can_dma_sg:1;
 
 	/* i/o info: pipes etc */
 	unsigned		in, out;
-- 
1.7.9.5

[PATCH v4 4/4] USBNET: ax88179_178a: enable tso if usb host supports sg dma

[Ming Lei]

This patch enables 'can_dma_sg' flag for ax88179_178a device
if the attached host controller supports building packet from
discontinuous buffers(DMA SG is possible), so TSO can be enabled
and skb fragment buffers can be passed to usb stack via urb->sg
directly.

With the patch, system CPU utilization decreased ~50% and throughput
increased by ~10% when doing iperf client test on one ARM A15 dual
core board.

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Ben Hutchings <bhutchings@solarflare.com>
Cc: Grant Grundler <grundler@google.com>
Cc: Oliver Neukum <oneukum@suse.de>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Freddy Xin <freddy@asix.com.tw>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/ax88179_178a.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index fb0caa2..3569293 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1031,12 +1031,20 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->mii.phy_id = 0x03;
 	dev->mii.supports_gmii = 1;
 
+	if (usb_device_no_sg_constraint(dev->udev))
+		dev->can_dma_sg = 1;
+
 	dev->net->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 			      NETIF_F_RXCSUM;
 
 	dev->net->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 				 NETIF_F_RXCSUM;
 
+	if (dev->can_dma_sg) {
+		dev->net->features |= NETIF_F_SG | NETIF_F_TSO;
+		dev->net->hw_features |= NETIF_F_SG | NETIF_F_TSO;
+	}
+
 	/* Enable checksum offload */
 	*tmp = AX_RXCOE_IP | AX_RXCOE_TCP | AX_RXCOE_UDP |
 	       AX_RXCOE_TCPV6 | AX_RXCOE_UDPV6;
-- 
1.7.9.5

Re: [PATCH v4 4/4] USBNET: ax88179_178a: enable tso if usb host supports sg dma

[Eric Dumazet]

On Thu, 2013-08-08 at 21:48 +0800, Ming Lei wrote:
> This patch enables 'can_dma_sg' flag for ax88179_178a device
> if the attached host controller supports building packet from
> discontinuous buffers(DMA SG is possible), so TSO can be enabled
> and skb fragment buffers can be passed to usb stack via urb->sg
> directly.
> 
> With the patch, system CPU utilization decreased ~50% and throughput
> increased by ~10% when doing iperf client test on one ARM A15 dual
> core board.
> 
> Cc: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
> Cc: Grant Grundler <grundler-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Cc: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
> Cc: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
> Cc: Freddy Xin <freddy-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
> Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> ---
>  drivers/net/usb/ax88179_178a.c |    8 ++++++++
>  1 file changed, 8 insertions(+)

Acked-by: Eric Dumazet <edumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Thanks for doing this !


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

Re: [PATCH v4 3/4] USBNET: support DMA SG

[Eric Dumazet]

On Thu, 2013-08-08 at 21:48 +0800, Ming Lei wrote:
> This patch introduces support of DMA SG if the USB host controller
> which usbnet device is attached to is capable of building packet from
> discontinuous buffers.


> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> index 8fbc008..9cb2fe8 100644
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -35,6 +35,7 @@ struct usbnet {
>  	unsigned char		suspend_count;
>  	unsigned char		pkt_cnt, pkt_err;
>  	unsigned short		rx_qlen, tx_qlen;
> +	unsigned		can_dma_sg:1;

We try to use "unsigned int" instead of plain "unsigned", but its a
minor point and should not block your patches.

Apart from this :

Reviewed-by: Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>


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

Re: [PATCH v4 2/4] USB: XHCI: mark no_sg_constraint

[Eric Dumazet]

On Thu, 2013-08-08 at 21:48 +0800, Ming Lei wrote:
> This patch marks all xHCI controllers as no_sg_constraint
> since xHCI supports building packet from discontinuous buffers.
> 
> Cc: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
> Acked-by: Sarah Sharp <sarah.a.sharp-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> ---
>  drivers/usb/host/xhci.c |    4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>


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

Re: [PATCH v4 1/4] USB: introduce usb_device_no_sg_constraint() helper

[Eric Dumazet]

On Thu, 2013-08-08 at 21:48 +0800, Ming Lei wrote:
> Some host controllers(such as xHCI) can support building
> packet from discontinuous buffers, so introduce one flag
> and helper for this kind of host controllers, then the
> feature can help some applications(such as usbnet) by
> supporting arbitrary length of sg buffers.
> 
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH v4 0/4] USB & USBNET: loose SG check and support usbnet DMA SG

[David Miller]

From: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Date: Thu,  8 Aug 2013 21:48:21 +0800

> This patchset allows drivers to pass sg buffers which size can't be divided
> by max packet size of endpoint if the host controllers(such ax xHCI) support
> this kind of sg buffers.
> 
> Previously we added check[1] on the situation and don't allow these sg buffers
> passed to USB HCD, looks the check is too strict to make use of new feature of
> new hardware(xHCI) for some applications(such as network stack) which can't
> provide this kind of sg buffers to usbnet driver, so the patch looses the check
> in case that the host controller supports it.
> 
> Patch 3/4 implements DMA SG on usbnet driver, and patch 4/4 uses it on ax88179_178a
> USB3 NIC for supporting TSO, so both CPU utilization and tx throughput can be
> improved with TSO and DMA SG in case of the USB NIC is attached to xHCI controller.
> 
> This patchset depends on both net-next and usb-next tree, so hope David and Greg
> to figure out one elegent way to merge it.

Greg feel free to merge this in via your USB tree with my:

Acked-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

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

Re: [PATCH v4 0/4] USB & USBNET: loose SG check and support usbnet DMA SG

[Greg KH]

On Fri, Aug 09, 2013 at 02:12:17PM -0700, David Miller wrote:
> From: Ming Lei <ming.lei@canonical.com>
> Date: Thu,  8 Aug 2013 21:48:21 +0800
> 
> > This patchset allows drivers to pass sg buffers which size can't be divided
> > by max packet size of endpoint if the host controllers(such ax xHCI) support
> > this kind of sg buffers.
> > 
> > Previously we added check[1] on the situation and don't allow these sg buffers
> > passed to USB HCD, looks the check is too strict to make use of new feature of
> > new hardware(xHCI) for some applications(such as network stack) which can't
> > provide this kind of sg buffers to usbnet driver, so the patch looses the check
> > in case that the host controller supports it.
> > 
> > Patch 3/4 implements DMA SG on usbnet driver, and patch 4/4 uses it on ax88179_178a
> > USB3 NIC for supporting TSO, so both CPU utilization and tx throughput can be
> > improved with TSO and DMA SG in case of the USB NIC is attached to xHCI controller.
> > 
> > This patchset depends on both net-next and usb-next tree, so hope David and Greg
> > to figure out one elegent way to merge it.
> 
> Greg feel free to merge this in via your USB tree with my:
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Thanks, will do.

greg k-h