netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/10 REV5] Implement skb batching and support in IPoIB/E1000
@ 2007-09-14  9:00 Krishna Kumar
  2007-09-14  9:01 ` [PATCH 1/10 REV5] [Doc] HOWTO Documentation for batching Krishna Kumar
                   ` (11 more replies)
  0 siblings, 12 replies; 121+ messages in thread
From: Krishna Kumar @ 2007-09-14  9:00 UTC (permalink / raw)
  To: johnpol, herbert, hadi, kaber, shemminger, davem
  Cc: jagana, Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
	peter.p.waskiewicz.jr, mcarlson, jeff, mchan, general, kumarkr,
	tgraf, randy.dunlap, Krishna Kumar, sri

This set of patches implements the batching xmit capability, and adds support
for batching in IPoIB and E1000 (E1000 driver changes is ported, thanks to
changes taken from Jamal's code from an old kernel).

List of changes from previous revision:
----------------------------------------
1. [Dave] Enable batching as default (change in register_netdev).
2. [Randy] Update documentation (however ethtool cmd to get/set batching is
	not implemented, hence I am guessing the usage).
3. [KK] When changing tx_batch_skb, qdisc xmits need to be blocked since
	qdisc_restart() drops queue_lock before calling driver xmit, and
	driver could find blist change under it.
4. [KK] sched: requeue could wrongly requeue skb already put in the batching
	list (in case a single skb was sent to the device but not sent as the
	device was full, resulting in the skb getting added to blist). This
	also results in slight optimization of batching behavior where for
	getting skbs #2 onwards don't require to check for gso_skb as that
	is the first skb that is processed.
4. [KK] Change documentation to explain this behavior.
5. [KK] sched: Fix panic when GSO is enabled in driver.
6. [KK] IPoIB: Small optimization in ipoib_ib_handle_tx_wc
7. [KK] netdevice: Needed to change NETIF_F_GSO_SHIFT/NETIF_F_GSO_MASK as
	BATCH_SKBS is now defined as 65536 (earlier it was using 8192 which
	was taken up by NETIF_F_NETNS_LOCAL).


Will submit in the next 1-2 days:
---------------------------------
1. [Auke] Enable batching in e1000e.


Extras that I can do later:
---------------------------
1. [Patrick] Use skb_blist statically in netdevice. This could also be used
	to integrate GSO and batching.
2. [Evgeniy] Useful to splice lists dev_add_skb_to_blist (and this can be
	done for regular xmit's of GSO skbs too for #1 above).

Patches are described as:
		 Mail 0/10:  This mail
		 Mail 1/10:  HOWTO documentation
		 Mail 2/10:  Introduce skb_blist, NETIF_F_BATCH_SKBS, use
		 	     single API for batching/no-batching, etc.
		 Mail 3/10:  Modify qdisc_run() to support batching
		 Mail 4/10:  Add ethtool support to enable/disable batching
		 Mail 5/10:  IPoIB: Header file changes to use batching
		 Mail 6/10:  IPoIB: CM & Multicast changes
		 Mail 7/10:  IPoIB: Verbs changes to use batching
		 Mail 8/10:  IPoIB: Internal post and work completion handler
		 Mail 9/10:  IPoIB: Implement the new batching capability
		 Mail 10/10: E1000: Implement the new batching capability

Issues:
--------
The retransmission problem reported earlier seems to happen when mthca is
used as the underlying device, but when I tested ehca the retransmissions
dropped to normal levels (around 2 times the regular code). The performance
improvement is around 55% for TCP.

Please review and provide feedback; and consider for inclusion.

Thanks,

- KK

----------------------------------------------------
			TCP
			----
Size:32 Procs:1		2728	3544	29.91
Size:128 Procs:1	11803	13679	15.89
Size:512 Procs:1	43279	49665	14.75
Size:4096 Procs:1	147952	101246	-31.56
Size:16384 Procs:1	149852	141897	-5.30

Size:32 Procs:4		10562	11349	7.45
Size:128 Procs:4	41010	40832	-.43
Size:512 Procs:4	75374	130943	73.72
Size:4096 Procs:4	167996	368218	119.18
Size:16384 Procs:4	123176	379524	208.11

Size:32 Procs:8		21125	21990	4.09
Size:128 Procs:8	77419	78605	1.53
Size:512 Procs:8	234678	265047	12.94
Size:4096 Procs:8	218063	367604	68.57
Size:16384 Procs:8	184283	370972	101.30

Average:	1509300 -> 2345115 = 55.38%
----------------------------------------------------

^ permalink raw reply	[flat|nested] 121+ messages in thread
* [PATCH 2/3][NET_BATCH] net core use batching
@ 2007-10-08 18:26 jamal
  2007-10-08 19:46 ` Waskiewicz Jr, Peter P
  0 siblings, 1 reply; 121+ messages in thread
From: jamal @ 2007-10-08 18:26 UTC (permalink / raw)
  To: David Miller
  Cc: krkumar2, johnpol, herbert, kaber, shemminger, jagana,
	Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
	peter.p.waskiewicz.jr, mcarlson, jeff, mchan, general, kumarkr,
	tgraf, randy.dunlap, sri

[-- Attachment #1: Type: text/plain, Size: 71 bytes --]

This patch adds the usage of batching within the core.

cheers,
jamal


[-- Attachment #2: 02-net-core-use-batching.patch --]
[-- Type: text/x-patch, Size: 4790 bytes --]

[NET_BATCH] net core use batching

This patch adds the usage of batching within the core.
Performance results demonstrating improvement are provided separately.

I have #if-0ed some of the old functions so the patch is more readable.
A future patch will remove all if-0ed content.
Patrick McHardy eyeballed a bug that will cause re-ordering in case
of a requeue.

Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>

---
commit 63381156b35719a364d0f81fec487e6263fb5467
tree 615dfb5f8617a8a4edffbb965e81a75c60597319
parent 8b9960cfbb98d48c0ac30b2c00d27c25217adb26
author Jamal Hadi Salim <hadi@cyberus.ca> Mon, 08 Oct 2007 09:10:14 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Mon, 08 Oct 2007 09:10:14 -0400

 net/sched/sch_generic.c |  116 +++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 106 insertions(+), 10 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 95ae119..96bfdcb 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -56,6 +56,7 @@ static inline int qdisc_qlen(struct Qdisc *q)
 	return q->q.qlen;
 }
 
+#if 0
 static inline int dev_requeue_skb(struct sk_buff *skb, struct net_device *dev,
 				  struct Qdisc *q)
 {
@@ -110,6 +111,97 @@ static inline int handle_dev_cpu_collision(struct sk_buff *skb,
 
 	return ret;
 }
+#endif
+
+static inline int handle_dev_cpu_collision(struct net_device *dev)
+{
+	if (unlikely(dev->xmit_lock_owner == smp_processor_id())) {
+		if (net_ratelimit())
+			printk(KERN_WARNING
+				"Dead loop on netdevice %s, fix it urgently!\n",
+				dev->name);
+		return 1;
+	}
+	__get_cpu_var(netdev_rx_stat).cpu_collision++;
+	return 0;
+}
+
+static inline int
+dev_requeue_skbs(struct sk_buff_head *skbs, struct net_device *dev,
+	       struct Qdisc *q)
+{
+
+	struct sk_buff *skb;
+
+	while ((skb = __skb_dequeue_tail(skbs)) != NULL)
+		q->ops->requeue(skb, q);
+
+	netif_schedule(dev);
+	return 0;
+}
+
+static inline int
+xmit_islocked(struct sk_buff_head *skbs, struct net_device *dev,
+	    struct Qdisc *q)
+{
+	int ret = handle_dev_cpu_collision(dev);
+
+	if (ret) {
+		if (!skb_queue_empty(skbs))
+			skb_queue_purge(skbs);
+		return qdisc_qlen(q);
+	}
+
+	return dev_requeue_skbs(skbs, dev, q);
+}
+
+static int xmit_count_skbs(struct sk_buff *skb)
+{
+	int count = 0;
+	for (; skb; skb = skb->next) {
+		count += skb_shinfo(skb)->nr_frags;
+		count += 1;
+	}
+	return count;
+}
+
+static int xmit_get_pkts(struct net_device *dev,
+			   struct Qdisc *q,
+			   struct sk_buff_head *pktlist)
+{
+	struct sk_buff *skb;
+	int count = dev->xmit_win;
+
+	if (count  && dev->gso_skb) {
+		skb = dev->gso_skb;
+		dev->gso_skb = NULL;
+		count -= xmit_count_skbs(skb);
+		__skb_queue_tail(pktlist, skb);
+	}
+
+	while (count > 0) {
+		skb = q->dequeue(q);
+		if (!skb)
+			break;
+
+		count -= xmit_count_skbs(skb);
+		__skb_queue_tail(pktlist, skb);
+	}
+
+	return skb_queue_len(pktlist);
+}
+
+static int xmit_prepare_pkts(struct net_device *dev,
+			     struct sk_buff_head *tlist)
+{
+	struct sk_buff *skb;
+	struct sk_buff_head *flist = &dev->blist;
+
+	while ((skb = __skb_dequeue(tlist)) != NULL)
+		xmit_prepare_skb(skb, dev);
+
+	return skb_queue_len(flist);
+}
 
 /*
  * NOTE: Called under dev->queue_lock with locally disabled BH.
@@ -130,22 +222,23 @@ static inline int handle_dev_cpu_collision(struct sk_buff *skb,
  *				>0 - queue is not empty.
  *
  */
+
 static inline int qdisc_restart(struct net_device *dev)
 {
 	struct Qdisc *q = dev->qdisc;
-	struct sk_buff *skb;
-	int ret;
+	int ret = 0;
 
-	/* Dequeue packet */
-	if (unlikely((skb = dev_dequeue_skb(dev, q)) == NULL))
-		return 0;
+	ret = xmit_get_pkts(dev, q, &dev->blist);
 
+	if (!ret)
+		return 0;
 
-	/* And release queue */
+	/* We got em packets */
 	spin_unlock(&dev->queue_lock);
 
+	/* bye packets ....*/
 	HARD_TX_LOCK(dev, smp_processor_id());
-	ret = dev_hard_start_xmit(skb, dev);
+	ret = dev_batch_xmit(dev);
 	HARD_TX_UNLOCK(dev);
 
 	spin_lock(&dev->queue_lock);
@@ -158,8 +251,8 @@ static inline int qdisc_restart(struct net_device *dev)
 		break;
 
 	case NETDEV_TX_LOCKED:
-		/* Driver try lock failed */
-		ret = handle_dev_cpu_collision(skb, dev, q);
+		/* Driver lock failed */
+		ret = xmit_islocked(&dev->blist, dev, q);
 		break;
 
 	default:
@@ -168,7 +261,7 @@ static inline int qdisc_restart(struct net_device *dev)
 			printk(KERN_WARNING "BUG %s code %d qlen %d\n",
 			       dev->name, ret, q->q.qlen);
 
-		ret = dev_requeue_skb(skb, dev, q);
+		ret = dev_requeue_skbs(&dev->blist, dev, q);
 		break;
 	}
 
@@ -564,6 +657,9 @@ void dev_deactivate(struct net_device *dev)
 
 	skb = dev->gso_skb;
 	dev->gso_skb = NULL;
+	if (!skb_queue_empty(&dev->blist))
+		skb_queue_purge(&dev->blist);
+	dev->xmit_win = 1;
 	spin_unlock_bh(&dev->queue_lock);
 
 	kfree_skb(skb);

^ permalink raw reply related	[flat|nested] 121+ messages in thread

end of thread, other threads:[~2007-11-14  8:28 UTC | newest]

Thread overview: 121+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-14  9:00 [PATCH 0/10 REV5] Implement skb batching and support in IPoIB/E1000 Krishna Kumar
2007-09-14  9:01 ` [PATCH 1/10 REV5] [Doc] HOWTO Documentation for batching Krishna Kumar
2007-09-14 18:37   ` [ofa-general] " Randy Dunlap
2007-09-17  4:10     ` Krishna Kumar2
2007-09-17  4:13       ` [ofa-general] " Jeff Garzik
2007-09-14  9:01 ` [PATCH 2/10 REV5] [core] Add skb_blist & support " Krishna Kumar
2007-09-14 12:46   ` [ofa-general] " Evgeniy Polyakov
2007-09-17  3:51     ` Krishna Kumar2
2007-09-14  9:01 ` [PATCH 3/10 REV5] [sched] Modify qdisc_run to support batching Krishna Kumar
2007-09-14 12:15   ` [ofa-general] " Evgeniy Polyakov
2007-09-17  3:49     ` Krishna Kumar2
2007-09-14  9:02 ` [PATCH 4/10 REV5] [ethtool] Add ethtool support Krishna Kumar
2007-09-14  9:02 ` [PATCH 5/10 REV5] [IPoIB] Header file changes Krishna Kumar
2007-09-14  9:03 ` [PATCH 6/10 REV5] [IPoIB] CM & Multicast changes Krishna Kumar
2007-09-14  9:03 ` [PATCH 7/10 REV5] [IPoIB] Verbs changes Krishna Kumar
2007-09-14  9:03 ` [PATCH 8/10 REV5] [IPoIB] Post and work completion handler changes Krishna Kumar
2007-09-14  9:04 ` [PATCH 9/10 REV5] [IPoIB] Implement batching Krishna Kumar
2007-09-14  9:04 ` [PATCH 10/10 REV5] [E1000] " Krishna Kumar
2007-09-14 12:47   ` [ofa-general] " Evgeniy Polyakov
2007-09-17  3:56     ` Krishna Kumar2
2007-11-13 21:28   ` [ofa-general] " Kok, Auke
2007-11-14  8:30     ` Krishna Kumar2
2007-09-14 12:49 ` [ofa-general] Re: [PATCH 0/10 REV5] Implement skb batching and support in IPoIB/E1000 Evgeniy Polyakov
2007-09-16 23:17 ` David Miller
2007-09-17  0:29   ` jamal
2007-09-17  1:02     ` David Miller
2007-09-17  2:14       ` [ofa-general] " jamal
2007-09-17  2:25         ` David Miller
2007-09-17  3:01           ` jamal
2007-09-17  3:13             ` David Miller
2007-09-17 12:51               ` jamal
2007-09-17 16:37                 ` [ofa-general] " David Miller
2007-09-17  4:46           ` Krishna Kumar2
2007-09-23 17:53     ` [PATCHES] TX batching jamal
2007-09-23 17:56       ` [ofa-general] [PATCH 1/4] [NET_SCHED] explict hold dev tx lock jamal
2007-09-23 17:58         ` [ofa-general] [PATCH 2/4] [NET_BATCH] Introduce batching interface jamal
2007-09-23 18:00           ` [PATCH 3/4][NET_BATCH] net core use batching jamal
2007-09-23 18:02             ` [ofa-general] [PATCH 4/4][NET_SCHED] kill dev->gso_skb jamal
2007-09-30 18:53               ` [ofa-general] [PATCH 3/3][NET_SCHED] " jamal
2007-10-07 18:39               ` [ofa-general] [PATCH 3/3][NET_BATCH] " jamal
2007-09-30 18:52             ` [ofa-general] [PATCH 2/3][NET_BATCH] net core use batching jamal
2007-10-01  4:11               ` Bill Fink
2007-10-01 13:30                 ` jamal
2007-10-02  4:25                   ` [ofa-general] " Bill Fink
2007-10-02 13:20                     ` jamal
2007-10-03  5:29                       ` [ofa-general] " Bill Fink
2007-10-03 13:42                         ` jamal
2007-10-01 10:42               ` [ofa-general] " Patrick McHardy
2007-10-01 13:21                 ` jamal
2007-10-08  5:03                   ` Krishna Kumar2
2007-10-08 13:17                     ` jamal
2007-10-09  3:09                       ` [ofa-general] " Krishna Kumar2
2007-10-09 13:10                         ` jamal
2007-10-07 18:38             ` [ofa-general] " jamal
2007-09-30 18:51           ` [ofa-general] [PATCH 1/4] [NET_BATCH] Introduce batching interface jamal
2007-09-30 18:54             ` [ofa-general] Re: [PATCH 1/3] " jamal
2007-10-07 18:36           ` [ofa-general] " jamal
2007-10-08  9:59             ` Krishna Kumar2
2007-10-08 13:49               ` jamal
2007-09-24 19:12         ` [ofa-general] RE: [PATCH 1/4] [NET_SCHED] explict hold dev tx lock Waskiewicz Jr, Peter P
2007-09-24 22:51           ` jamal
2007-09-24 22:57             ` Waskiewicz Jr, Peter P
2007-09-24 23:38               ` [ofa-general] " jamal
2007-09-24 23:47                 ` Waskiewicz Jr, Peter P
2007-09-25  0:14                   ` [ofa-general] " Stephen Hemminger
2007-09-25  0:31                     ` [ofa-general] " Waskiewicz Jr, Peter P
2007-09-25 13:15                     ` [ofa-general] " jamal
2007-09-25 15:24                       ` Stephen Hemminger
2007-09-25 22:14                         ` jamal
2007-09-25 22:43                           ` jamal
2007-09-25 13:08                   ` [ofa-general] " jamal
2007-10-08  4:51                 ` [ofa-general] " David Miller
2007-10-08 13:34                   ` jamal
2007-10-08 14:22                     ` parallel networking (was Re: [PATCH 1/4] [NET_SCHED] explict hold dev tx lock) Jeff Garzik
2007-10-08 15:18                       ` [ofa-general] " jamal
2007-10-08 21:11                       ` [ofa-general] Re: parallel networking David Miller
2007-10-08 22:30                         ` jamal
2007-10-08 22:33                           ` David Miller
2007-10-08 22:35                             ` [ofa-general] " Waskiewicz Jr, Peter P
2007-10-08 23:42                             ` [ofa-general] " jamal
2007-10-09  1:53                         ` Jeff Garzik
2007-10-09 14:59                           ` Michael Krause
2007-10-08 21:05                     ` [PATCH 1/4] [NET_SCHED] explict hold dev tx lock David Miller
2007-09-23 18:19       ` [PATCHES] TX batching Jeff Garzik
2007-09-23 19:11         ` [ofa-general] " jamal
2007-09-23 19:36           ` Kok, Auke
2007-09-23 21:20             ` jamal
2007-09-24  7:00               ` Kok, Auke
2007-09-24 22:38                 ` jamal
2007-09-24 22:52                   ` [ofa-general] " Kok, Auke
2007-09-24 22:54           ` [DOC] Net batching driver howto jamal
2007-09-25 20:16             ` [ofa-general] " Randy Dunlap
2007-09-25 22:28               ` jamal
2007-09-25  0:15           ` [PATCHES] TX batching Jeff Garzik
2007-09-30 18:50       ` [ofa-general] " jamal
2007-09-30 19:19         ` [ofa-general] " jamal
2007-10-07 18:34       ` [ofa-general] " jamal
2007-10-08 12:51         ` [ofa-general] " Evgeniy Polyakov
2007-10-08 14:05           ` jamal
2007-10-09  8:14             ` Krishna Kumar2
2007-10-09 13:25               ` jamal
2007-09-17  4:08   ` [PATCH 0/10 REV5] Implement skb batching and support in IPoIB/E1000 Krishna Kumar2
  -- strict thread matches above, loose matches on Subject: below --
2007-10-08 18:26 [PATCH 2/3][NET_BATCH] net core use batching jamal
2007-10-08 19:46 ` Waskiewicz Jr, Peter P
2007-10-08 20:48   ` jamal
2007-10-08 22:33     ` [ofa-general] " Waskiewicz Jr, Peter P
2007-10-08 23:40       ` jamal
2007-10-09  1:13         ` Jeff Garzik
2007-10-09  1:41           ` [ofa-general] " David Miller
2007-10-09  2:01             ` Herbert Xu
2007-10-09  2:03               ` Herbert Xu
2007-10-09  2:04                 ` Herbert Xu
2007-10-09  2:15                   ` jamal
2007-10-09  2:16                     ` Herbert Xu
2007-10-09  2:19                       ` [ofa-general] " jamal
2007-10-09  2:20                         ` Herbert Xu
2007-10-09  2:43                 ` [ofa-general] " David Miller
2007-10-09  2:46                   ` Herbert Xu
2007-10-09  2:12             ` [ofa-general] " Jeff Garzik
2007-10-09 18:48               ` [ofa-general] " Waskiewicz Jr, Peter P
2007-10-09 19:04                 ` Jeff Garzik
2007-10-09 19:07                   ` Waskiewicz Jr, Peter P
2007-10-09  2:14             ` [ofa-general] " jamal
2007-10-09  2:16               ` Herbert Xu
2007-10-09  1:31         ` Jeff Garzik
2007-10-09 10:58       ` [ofa-general] " Krishna Kumar2
2007-10-09 11:02         ` David Miller
2007-10-09 11:21           ` [ofa-general] " Krishna Kumar2
2007-10-09 11:24             ` David Miller
2007-10-09 12:44               ` [ofa-general] " Jeff Garzik
2007-10-09 12:55                 ` Herbert Xu
2007-10-09 13:00                   ` Jeff Garzik
2007-10-09 20:14                 ` David Miller
2007-10-09 20:20                   ` [ofa-general] " Jeff Garzik
2007-10-09 21:25                     ` David Miller
2007-10-09 20:22               ` [ofa-general] " Roland Dreier
2007-10-09 20:51                 ` David Miller
2007-10-09 21:40                   ` Roland Dreier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).