[PATCH 0/2] USBNET: max rx/tx qlen change

[PATCH 0/2] USBNET: max rx/tx qlen change

[Ming Lei]

Hi,

There are two patches on computing max rx/tx qlen, and fix one
performance problem on USB3 NIC.

 drivers/net/usb/asix_devices.c |    3 +++
 drivers/net/usb/ax88179_178a.c |    3 +++
 drivers/net/usb/usbnet.c       |   54 ++++++++++++++++++++++++++++++++++------
 include/linux/usb/usbnet.h     |    3 +++
 4 files changed, 55 insertions(+), 8 deletions(-)


Thanks,
--
Ming Lei

[PATCH 1/2] USBNET: centralize computing of max rx/tx qlen

[Ming Lei]

This patch centralizes computing of max rx/tx qlen, because:

- RX_QLEN()/TX_QLEN() is called in hot path
- computing depends on device's usb speed, now we have ls/fs, hs, ss,
so more checks need to be involved
- in fact, max rx/tx qlen should not only depend on device USB
speed, but also depend on ethernet link speed, so we need to
consider that in future.
- if SG support is done, max tx qlen may need change too

Generally, hard_mtu and rx_urb_size are changed in bind(), reset()
and link_reset() callback, and change mtu network operation, this
patches introduces the API of usbnet_update_max_qlen(), and calls
it in above path.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/asix_devices.c |    3 +++
 drivers/net/usb/ax88179_178a.c |    3 +++
 drivers/net/usb/usbnet.c       |   45 +++++++++++++++++++++++++++++++++-------
 include/linux/usb/usbnet.h     |    3 +++
 4 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index ad5d1e4..b96ad4f 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -778,6 +778,9 @@ static int ax88178_change_mtu(struct net_device *net, int new_mtu)
 	dev->hard_mtu = net->mtu + net->hard_header_len;
 	ax88178_set_mfb(dev);
 
+	/* max qlen depend on hard_mtu and rx_urb_size */
+	usbnet_update_max_qlen(dev);
+
 	return 0;
 }
 
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 2bc87e3..5a468f3 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -688,6 +688,9 @@ static int ax88179_change_mtu(struct net_device *net, int new_mtu)
 				  2, 2, &tmp16);
 	}
 
+	/* max qlen depend on hard_mtu and rx_urb_size */
+	usbnet_update_max_qlen(dev);
+
 	return 0;
 }
 
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 06ee82f..d74de07 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -59,15 +59,13 @@
  * For high speed, each frame comfortably fits almost 36 max size
  * Ethernet packets (so queues should be bigger).
  *
- * REVISIT qlens should be members of 'struct usbnet'; the goal is to
- * let the USB host controller be busy for 5msec or more before an irq
- * is required, under load.  Jumbograms change the equation.
+ * The goal is to let the USB host controller be busy for 5msec or
+ * more before an irq is required, under load.  Jumbograms change
+ * the equation.
  */
-#define RX_MAX_QUEUE_MEMORY (60 * 1518)
-#define	RX_QLEN(dev) (((dev)->udev->speed == USB_SPEED_HIGH) ? \
-			(RX_MAX_QUEUE_MEMORY/(dev)->rx_urb_size) : 4)
-#define	TX_QLEN(dev) (((dev)->udev->speed == USB_SPEED_HIGH) ? \
-			(RX_MAX_QUEUE_MEMORY/(dev)->hard_mtu) : 4)
+#define	MAX_QUEUE_MEMORY	(60 * 1518)
+#define	RX_QLEN(dev)		((dev)->rx_qlen)
+#define	TX_QLEN(dev)		((dev)->tx_qlen)
 
 // reawaken network queue this soon after stopping; else watchdog barks
 #define TX_TIMEOUT_JIFFIES	(5*HZ)
@@ -347,6 +345,22 @@ void usbnet_skb_return (struct usbnet *dev, struct sk_buff *skb)
 }
 EXPORT_SYMBOL_GPL(usbnet_skb_return);
 
+/* must be called if hard_mtu or rx_urb_size changed */
+void usbnet_update_max_qlen(struct usbnet *dev)
+{
+	enum usb_device_speed speed = dev->udev->speed;
+
+	switch (speed) {
+	case USB_SPEED_HIGH:
+		dev->rx_qlen = MAX_QUEUE_MEMORY / dev->rx_urb_size;
+		dev->tx_qlen = MAX_QUEUE_MEMORY / dev->hard_mtu;
+		break;
+	default:
+		dev->rx_qlen = dev->tx_qlen = 4;
+	}
+}
+EXPORT_SYMBOL_GPL(usbnet_update_max_qlen);
+
 \f
 /*-------------------------------------------------------------------------
  *
@@ -375,6 +389,9 @@ int usbnet_change_mtu (struct net_device *net, int new_mtu)
 			usbnet_unlink_rx_urbs(dev);
 	}
 
+	/* max qlen depend on hard_mtu and rx_urb_size */
+	usbnet_update_max_qlen(dev);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(usbnet_change_mtu);
@@ -843,6 +860,9 @@ int usbnet_open (struct net_device *net)
 		goto done;
 	}
 
+	/* hard_mtu or rx_urb_size may change in reset() */
+	usbnet_update_max_qlen(dev);
+
 	// insist peer be connected
 	if (info->check_connect && (retval = info->check_connect (dev)) < 0) {
 		netif_dbg(dev, ifup, dev->net, "can't open; %d\n", retval);
@@ -927,6 +947,9 @@ int usbnet_set_settings (struct net_device *net, struct ethtool_cmd *cmd)
 	if (dev->driver_info->link_reset)
 		dev->driver_info->link_reset(dev);
 
+	/* hard_mtu or rx_urb_size may change in link_reset() */
+	usbnet_update_max_qlen(dev);
+
 	return retval;
 
 }
@@ -1020,6 +1043,9 @@ static void __handle_link_change(struct usbnet *dev)
 		tasklet_schedule(&dev->bh);
 	}
 
+	/* hard_mtu or rx_urb_size may change during link change */
+	usbnet_update_max_qlen(dev);
+
 	clear_bit(EVENT_LINK_CHANGE, &dev->flags);
 }
 
@@ -1599,6 +1625,9 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
 	if ((dev->driver_info->flags & FLAG_WWAN) != 0)
 		SET_NETDEV_DEVTYPE(net, &wwan_type);
 
+	/* initialize max rx_qlen and tx_qlen */
+	usbnet_update_max_qlen(dev);
+
 	status = register_netdev (net);
 	if (status)
 		goto out4;
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index f18d641..8fbc008 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -34,6 +34,7 @@ struct usbnet {
 	struct mutex		phy_mutex;
 	unsigned char		suspend_count;
 	unsigned char		pkt_cnt, pkt_err;
+	unsigned short		rx_qlen, tx_qlen;
 
 	/* i/o info: pipes etc */
 	unsigned		in, out;
@@ -253,4 +254,6 @@ extern void usbnet_link_change(struct usbnet *, bool, bool);
 extern int usbnet_status_start(struct usbnet *dev, gfp_t mem_flags);
 extern void usbnet_status_stop(struct usbnet *dev);
 
+extern void usbnet_update_max_qlen(struct usbnet *dev);
+
 #endif /* __LINUX_USB_USBNET_H */
-- 
1.7.9.5

[PATCH 2/2] USBNET: increase max rx/tx qlen for improving USB3 thoughtput

[Ming Lei]

The default RX_QLEN()/TX_QLEN() didn't consider susper speed
USB device, so only max 4 URBs are scheduled at the same time
for tx/rx, then USB3 NIC can't perform very well.

With this patch, both rx and tx thoughput are increased more than
100Mbps when doing iperf test on ax88179_178a USB 3.0 NIC.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/usbnet.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index d74de07..e4811d7 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -355,6 +355,15 @@ void usbnet_update_max_qlen(struct usbnet *dev)
 		dev->rx_qlen = MAX_QUEUE_MEMORY / dev->rx_urb_size;
 		dev->tx_qlen = MAX_QUEUE_MEMORY / dev->hard_mtu;
 		break;
+	case USB_SPEED_SUPER:
+		/*
+		 * Not take default 5ms qlen for super speed HC to
+		 * save memory, and iperf tests show 2.5ms qlen can
+		 * work well
+		 */
+		dev->rx_qlen = 5 * MAX_QUEUE_MEMORY / dev->rx_urb_size;
+		dev->tx_qlen = 5 * MAX_QUEUE_MEMORY / dev->hard_mtu;
+		break;
 	default:
 		dev->rx_qlen = dev->tx_qlen = 4;
 	}
-- 
1.7.9.5

Re: [PATCH 0/2] USBNET: max rx/tx qlen change

[Oliver Neukum]

On Thu, 2013-07-25 at 13:47 +0800, Ming Lei wrote:
> Hi,
> 
> There are two patches on computing max rx/tx qlen, and fix one
> performance problem on USB3 NIC.

I am afraid bisectability on SS is impaired. Otherwise
this looks good.

	Regards
		Oliver


--
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 0/2] USBNET: max rx/tx qlen change

[Ming Lei]

Hi,

On Thu, Jul 25, 2013 at 3:38 PM, Oliver Neukum <oneukum@suse.de> wrote:
> On Thu, 2013-07-25 at 13:47 +0800, Ming Lei wrote:
>> Hi,
>>
>> There are two patches on computing max rx/tx qlen, and fix one
>> performance problem on USB3 NIC.
>
> I am afraid bisectability on SS is impaired. Otherwise
> this looks good.

Bisectability on SS can't be impaired, and it is always the 2nd
patch which fixes the performance issue on SS, and the 1st one
only does code refactoring thing.


Thanks,
--
Ming Lei

Re: [PATCH 0/2] USBNET: max rx/tx qlen change

[Oliver Neukum]

On Thu, 2013-07-25 at 22:19 +0800, Ming Lei wrote:
> Hi,
> 
> On Thu, Jul 25, 2013 at 3:38 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
> > On Thu, 2013-07-25 at 13:47 +0800, Ming Lei wrote:
> >> Hi,
> >>
> >> There are two patches on computing max rx/tx qlen, and fix one
> >> performance problem on USB3 NIC.
> >
> > I am afraid bisectability on SS is impaired. Otherwise
> > this looks good.
> 
> Bisectability on SS can't be impaired, and it is always the 2nd
> patch which fixes the performance issue on SS, and the 1st one
> only does code refactoring thing.

Yes, it looks a bit unusual, bit is OK.

	Sorry
		Oliver


--
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 0/2] USBNET: max rx/tx qlen change

[David Miller]

From: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Date: Thu, 25 Jul 2013 13:47:52 +0800

> There are two patches on computing max rx/tx qlen, and fix one
> performance problem on USB3 NIC.

Applied to net-next, 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