Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 2/5] net: Make rtnetlink infrastructure network namespace aware
From: Patrick McHardy @ 2007-09-30 15:39 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: David Miller, netdev, Linux Containers
In-Reply-To: <m1myv56vb3.fsf@ebiederm.dsl.xmission.com>

Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
> 
> 
>>Maybe I can save you some time: we used to do down_trylock()
>>for the rtnl mutex, so senders would simply return if someone
>>else was already processing the queue *or* the rtnl was locked
>>for some other reason. In the first case the process already
>>processing the queue would also process the new messages, but
>>if it the rtnl was locked for some other reason (for example
>>during module registration) the message would sit in the
>>queue until the next rtnetlink sendmsg call, which is why
>>rtnl_unlock does queue processing. Commit 6756ae4b changed
>>the down_trylock to mutex_lock, so senders will now simply wait
>>until the mutex is released and then call netlink_run_queue
>>themselves. This means its not needed anymore.
> 
> 
> Sounds reasonable.
> 
> I started looking through the code paths and I currently cannot
> see anything that would leave a message on a kernel rtnl socket.
> 
> However I did a quick test adding a WARN_ON if there were any messages
> found in the queue during rtnl_unlock and I found this code path
> getting invoked from linkwatch_event.  So there is clearly something I
> don't understand, and it sounds at odds just a bit from your
> description.


That sounds like a bug. Did you place the WARN_ON before or after
the mutex_unlock()?

^ permalink raw reply

* Re: [PATCH] rtnl: Simplify ASSERT_RTNL
From: Patrick McHardy @ 2007-09-30 15:47 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Eric W. Biederman, davem, netdev, oliver, linux-usb-devel
In-Reply-To: <20070930002421.GA7502@gondor.apana.org.au>

Herbert Xu wrote:
> On Sat, Sep 29, 2007 at 05:32:41PM +0200, Patrick McHardy wrote:
> 
>>For unicast addresses its not strictly necessary since they may
>>only be changed under the RTNL anyway. The reason why it takes
>>the tx_lock is for consistency with multicast address handling,
>>which can't rely on the RTNL since IPv6 changes them from
>>BH context. The idea was that the ->set_rx_mode function should
>>handle both secondary unicast and multicast addresses for
>>simplicity.
> 
> 
> In any case, coming back to the original question, the RTNL
> assertion is simply wrong in this case because if we're being
> called from IPv6 then the RTNL won't even be held.
> 
> So I think we need to
> 
> 1) Move the assert into dev_set_promiscuity.
> 2) Take the TX lock in dev_set_promiscuity.


In the IPv6 case we're only changing the multicast list,
so we're never calling into __dev_set_promiscuity.

I actually even added a comment about this :)

        /* Unicast addresses changes may only happen under the rtnl,
         * therefore calling __dev_set_promiscuity here is safe.
         */

I would prefer to keep the ASSERT_RTNL in __dev_set_promiscuity
since it also covers the __dev_set_rx_mode path. How about
adding an ASSERT_RTNL_ATOMIC without the might_sleep or simply
open coding it?

^ permalink raw reply

* ehea work queues
From: Anton Blanchard @ 2007-09-30 16:20 UTC (permalink / raw)
  To: themann, netdev


Hi,

I booted 2.6.23-rc8 and noticed that ehea loves its workqueues:

# ps aux|grep ehea
root      3266  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_driver_wq/]
root      3268  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_driver_wq/]
root      3269  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_driver_wq/]
root      3270  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_driver_wq/]
root      3271  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_driver_wq/]
root      3272  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_driver_wq/]
root      3273  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_driver_wq/]
root      3274  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_driver_wq/]
root      3275  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_wq/0]
root      3276  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_wq/1]
root      3278  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_wq/2]
root      3279  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_wq/3]
root      3280  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_wq/4]
root      3281  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_wq/5]
root      3282  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_wq/6]
root      3283  0.0  0.0    0    0 ?    S<   11:02   0:00 [ehea_wq/7]

(notice also that the ehea_driver_wq/XXX exceeds TASK_COMM_LEN). 

Since they are both infrequent events and not performance critical
(memory hotplug and driver reset), can we just use schedule_work?

Anton

^ permalink raw reply

* [PATCH][E1000E] some cleanups
From: jamal @ 2007-09-30 17:41 UTC (permalink / raw)
  To: Kok, Auke; +Cc: netdev

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

Auke,

heres part of something i promised. 
I couldnt do any packet testing on because 82571EB is disabled in the
driver. I uncommented the code out in the table, but the best i could
get was the module loading, some probing and some sysfs renaming
failures (probably a debianism); the machine access is intermittent, so
thats as far as i could go. In any case, you probably have a good reason
for disabling that chip. So, heres the patch, the burden of testing now
falls on you ;->
Once you have 82571EB on and kicking, my next steps are to kill LLTX
then add batching on top.
BTW, since this driver is just for PCIE, would you take a similar patch
for non-PCIE e1000?

comment:
There used to be an "mmiowb()" call right after the dma wake which is
gone now; is this unneeded with pcie? I have restored it, look for the
"XXX".

cheers,
jamal

[-- Attachment #2: e1000e-p1 --]
[-- Type: text/plain, Size: 10445 bytes --]

[E1000E] some cleanups
This patch makes the xmit path code a lot more readable and reusable.
preps for removing LLTX.

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

---
commit ad63c288ce980907f68d94d5faac08625c0b1782
tree 296a6da371b98c6488c544a8910941ea6d8c18a8
parent 7f5d0afdff875b2c4957031f8934741aefe257cc
author Jamal Hadi Salim <hadi@cyberus.ca> Sun, 30 Sep 2007 13:26:00 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Sun, 30 Sep 2007 13:26:00 -0400

 drivers/net/e1000e/netdev.c |  187 +++++++++++++++++++++++++++----------------
 1 files changed, 119 insertions(+), 68 deletions(-)

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 4a21d7d..5043504 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -3076,6 +3076,18 @@ link_up:
 #define E1000_TX_FLAGS_VLAN_MASK	0xffff0000
 #define E1000_TX_FLAGS_VLAN_SHIFT	16
 
+struct e1000_tx_cbdata {
+	int count;
+	unsigned int max_per_txd;
+	unsigned int nr_frags;
+	unsigned int mss;
+	unsigned int tx_flags;
+};
+
+#define E1000_SKB_CB(__skb) ((struct e1000_tx_cbdata *)&((__skb)->cb[8]))
+#define NETDEV_TX_DROPPED -5
+#define TXD_USE_COUNT(S, X) (((S) >> (X)) + 1)
+
 static int e1000_tso(struct e1000_adapter *adapter,
 		     struct sk_buff *skb)
 {
@@ -3194,8 +3206,7 @@ static bool e1000_tx_csum(struct e1000_adapter *adapter, struct sk_buff *skb)
 
 static int e1000_tx_map(struct e1000_adapter *adapter,
 			struct sk_buff *skb, unsigned int first,
-			unsigned int max_per_txd, unsigned int nr_frags,
-			unsigned int mss)
+			struct e1000_tx_cbdata *cb)
 {
 	struct e1000_ring *tx_ring = adapter->tx_ring;
 	struct e1000_buffer *buffer_info;
@@ -3207,11 +3218,11 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 
 	while (len) {
 		buffer_info = &tx_ring->buffer_info[i];
-		size = min(len, max_per_txd);
+		size = min(len, cb->max_per_txd);
 
 		/* Workaround for premature desc write-backs
 		 * in TSO mode.  Append 4-byte sentinel desc */
-		if (mss && !nr_frags && size == len && size > 8)
+		if (cb->mss && !cb->nr_frags && size == len && size > 8)
 			size -= 4;
 
 		buffer_info->length = size;
@@ -3237,7 +3248,7 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 			i = 0;
 	}
 
-	for (f = 0; f < nr_frags; f++) {
+	for (f = 0; f < cb->nr_frags; f++) {
 		struct skb_frag_struct *frag;
 
 		frag = &skb_shinfo(skb)->frags[f];
@@ -3246,10 +3257,11 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 
 		while (len) {
 			buffer_info = &tx_ring->buffer_info[i];
-			size = min(len, max_per_txd);
+			size = min(len, cb->max_per_txd);
 			/* Workaround for premature desc write-backs
 			 * in TSO mode.  Append 4-byte sentinel desc */
-			if (mss && f == (nr_frags-1) && size == len && size > 8)
+			if (cb->mss && f == (cb->nr_frags-1) &&
+			    size == len && size > 8)
 				size -= 4;
 
 			buffer_info->length = size;
@@ -3334,18 +3346,7 @@ static void e1000_tx_queue(struct e1000_adapter *adapter,
 	}
 
 	tx_desc->lower.data |= cpu_to_le32(adapter->txd_cmd);
-
-	/* Force memory writes to complete before letting h/w
-	 * know there are new descriptors to fetch.  (Only
-	 * applicable for weak-ordered memory model archs,
-	 * such as IA-64). */
-	wmb();
-
 	tx_ring->next_to_use = i;
-	writel(i, adapter->hw.hw_addr + tx_ring->tail);
-	/* we need this if more than one processor can write to our tail
-	 * at a time, it synchronizes IO on IA64/Altix systems */
-	mmiowb();
 }
 
 #define MINIMUM_DHCP_PACKET_SIZE 282
@@ -3417,45 +3418,54 @@ static int e1000_maybe_stop_tx(struct net_device *netdev, int size)
 	return __e1000_maybe_stop_tx(netdev, size);
 }
 
-#define TXD_USE_COUNT(S, X) (((S) >> (X)) + 1 )
-static int e1000_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
+static void
+e1000_complete_tx(struct net_device *netdev)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_ring *tx_ring = adapter->tx_ring;
-	unsigned int first;
-	unsigned int max_per_txd = E1000_MAX_PER_TXD;
+	/* Force memory writes to complete before letting h/w
+	 * know there are new descriptors to fetch.  (Only
+	 * applicable for weak-ordered memory model archs,
+	 * such as IA-64). */
+	wmb();
+	writel(tx_ring->next_to_use, adapter->hw.hw_addr + tx_ring->tail);
+	/* XXX: we need this if more than one processor can write to
+	 * our tail at a time, it syncronizes IO on IA64/Altix systems */
+	mmiowb();
+	netdev->trans_start = jiffies;
+}
+
+static int e1000_prep_frame(struct sk_buff *skb, struct net_device *netdev)
+{
+	struct e1000_adapter *adapter = netdev_priv(netdev);
 	unsigned int max_txd_pwr = E1000_MAX_TXD_PWR;
-	unsigned int tx_flags = 0;
 	unsigned int len = skb->len;
-	unsigned long irq_flags;
-	unsigned int nr_frags = 0;
-	unsigned int mss = 0;
-	int count = 0;
-	int tso;
+	struct e1000_tx_cbdata *cb = E1000_SKB_CB(skb);
 	unsigned int f;
-	len -= skb->data_len;
 
-	if (test_bit(__E1000_DOWN, &adapter->state)) {
-		dev_kfree_skb_any(skb);
-		return NETDEV_TX_OK;
-	}
+	cb->nr_frags = 0;
+	cb->mss = 0;
+	cb->count = 0;
+	cb->max_per_txd = E1000_MAX_PER_TXD;
+
+	len -= skb->data_len;
 
 	if (skb->len <= 0) {
 		dev_kfree_skb_any(skb);
-		return NETDEV_TX_OK;
+		return NETDEV_TX_DROPPED;
 	}
 
-	mss = skb_shinfo(skb)->gso_size;
+	cb->mss = skb_shinfo(skb)->gso_size;
 	/* The controller does a simple calculation to
 	 * make sure there is enough room in the FIFO before
 	 * initiating the DMA for each buffer.  The calc is:
 	 * 4 = ceil(buffer len/mss).  To make sure we don't
 	 * overrun the FIFO, adjust the max buffer len if mss
 	 * drops. */
-	if (mss) {
+	if (cb->mss) {
 		u8 hdr_len;
-		max_per_txd = min(mss << 2, max_per_txd);
-		max_txd_pwr = fls(max_per_txd) - 1;
+		cb->max_per_txd = min(cb->mss << 2, cb->max_per_txd);
+		max_txd_pwr = fls(cb->max_per_txd) - 1;
 
 		/* TSO Workaround for 82571/2/3 Controllers -- if skb->data
 		* points to just header, pull a few bytes of payload from
@@ -3469,80 +3479,121 @@ static int e1000_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 				ndev_err(netdev,
 					 "__pskb_pull_tail failed.\n");
 				dev_kfree_skb_any(skb);
-				return NETDEV_TX_OK;
+				return NETDEV_TX_DROPPED;
 			}
 			len = skb->len - skb->data_len;
 		}
 	}
 
 	/* reserve a descriptor for the offload context */
-	if ((mss) || (skb->ip_summed == CHECKSUM_PARTIAL))
-		count++;
-	count++;
+	if ((cb->mss) || (skb->ip_summed == CHECKSUM_PARTIAL))
+		cb->count++;
+	cb->count++;
 
-	count += TXD_USE_COUNT(len, max_txd_pwr);
+	cb->count += TXD_USE_COUNT(len, max_txd_pwr);
 
-	nr_frags = skb_shinfo(skb)->nr_frags;
-	for (f = 0; f < nr_frags; f++)
-		count += TXD_USE_COUNT(skb_shinfo(skb)->frags[f].size,
+	cb->nr_frags = skb_shinfo(skb)->nr_frags;
+	for (f = 0; f < cb->nr_frags; f++)
+		cb->count += TXD_USE_COUNT(skb_shinfo(skb)->frags[f].size,
 				       max_txd_pwr);
 
 	if (adapter->hw.mac.tx_pkt_filtering)
 		e1000_transfer_dhcp_info(adapter, skb);
 
-	if (!spin_trylock_irqsave(&adapter->tx_queue_lock, irq_flags))
-		/* Collision - tell upper layer to requeue */
-		return NETDEV_TX_LOCKED;
+	if (adapter->vlgrp && vlan_tx_tag_present(skb)) {
+		cb->tx_flags |= E1000_TX_FLAGS_VLAN;
+		cb->tx_flags |= (vlan_tx_tag_get(skb) <<
+				E1000_TX_FLAGS_VLAN_SHIFT);
+	}
+
+	return NETDEV_TX_OK;
+}
+
+static int e1000_queue_frame(struct sk_buff *skb, struct net_device *netdev)
+{
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct e1000_ring *tx_ring = adapter->tx_ring;
+	unsigned int first;
+	int tso;
+	struct e1000_tx_cbdata *cb = E1000_SKB_CB(skb);
+
 
 	/* need: count + 2 desc gap to keep tail from touching
 	 * head, otherwise try next time */
-	if (e1000_maybe_stop_tx(netdev, count + 2)) {
-		spin_unlock_irqrestore(&adapter->tx_queue_lock, irq_flags);
+	if (e1000_maybe_stop_tx(netdev, cb->count + 2))
 		return NETDEV_TX_BUSY;
-	}
-
-	if (adapter->vlgrp && vlan_tx_tag_present(skb)) {
-		tx_flags |= E1000_TX_FLAGS_VLAN;
-		tx_flags |= (vlan_tx_tag_get(skb) << E1000_TX_FLAGS_VLAN_SHIFT);
-	}
 
 	first = tx_ring->next_to_use;
 
 	tso = e1000_tso(adapter, skb);
 	if (tso < 0) {
 		dev_kfree_skb_any(skb);
-		spin_unlock_irqrestore(&adapter->tx_queue_lock, irq_flags);
-		return NETDEV_TX_OK;
+		return NETDEV_TX_DROPPED;
 	}
 
 	if (tso)
-		tx_flags |= E1000_TX_FLAGS_TSO;
+		cb->tx_flags |= E1000_TX_FLAGS_TSO;
 	else if (e1000_tx_csum(adapter, skb))
-		tx_flags |= E1000_TX_FLAGS_CSUM;
+		cb->tx_flags |= E1000_TX_FLAGS_CSUM;
 
 	/* Old method was to assume IPv4 packet by default if TSO was enabled.
 	 * 82571 hardware supports TSO capabilities for IPv6 as well...
 	 * no longer assume, we must. */
 	if (skb->protocol == htons(ETH_P_IP))
-		tx_flags |= E1000_TX_FLAGS_IPV4;
+		cb->tx_flags |= E1000_TX_FLAGS_IPV4;
 
-	count = e1000_tx_map(adapter, skb, first, max_per_txd, nr_frags, mss);
-	if (count < 0) {
+	cb->count = e1000_tx_map(adapter, skb, first, cb);
+	if (cb->count < 0) {
 		/* handle pci_map_single() error in e1000_tx_map */
 		dev_kfree_skb_any(skb);
-		spin_unlock_irqrestore(&adapter->tx_queue_lock, irq_flags);
+		return NETDEV_TX_DROPPED;
+	}
+
+	e1000_tx_queue(adapter, cb->tx_flags, cb->count);
+
+	return NETDEV_TX_OK;
+}
+
+static int e1000_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
+{
+	int ret = NETDEV_TX_OK;
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+	unsigned long irq_flags;
+	struct e1000_tx_cbdata *cb = E1000_SKB_CB(skb);
+
+	ret = e1000_prep_frame(skb, netdev);
+	if (unlikely(ret != NETDEV_TX_OK))
+		return NETDEV_TX_OK;
+
+	if (test_bit(__E1000_DOWN, &adapter->state)) {
+		dev_kfree_skb_any(skb);
 		return NETDEV_TX_OK;
 	}
 
-	e1000_tx_queue(adapter, tx_flags, count);
+	if (!spin_trylock_irqsave(&adapter->tx_queue_lock, irq_flags))
+		/* Collision - tell upper layer to requeue */
+		return NETDEV_TX_LOCKED;
 
-	netdev->trans_start = jiffies;
+	/* need: count + 2 desc gap to keep tail from touching
+	 * head, otherwise try next time */
+	if (e1000_maybe_stop_tx(netdev, cb->count + 2)) {
+		spin_unlock_irqrestore(&adapter->tx_queue_lock, irq_flags);
+		return NETDEV_TX_BUSY;
+	}
+
+	ret = e1000_queue_frame(skb, netdev);
+
+	if (ret == NETDEV_TX_OK)
+		e1000_complete_tx(netdev);
+
+	if (ret == NETDEV_TX_DROPPED)
+		ret = NETDEV_TX_OK;
 
 	/* Make sure there is space in the ring for the next send. */
 	e1000_maybe_stop_tx(netdev, MAX_SKB_FRAGS + 2);
 
 	spin_unlock_irqrestore(&adapter->tx_queue_lock, irq_flags);
-	return NETDEV_TX_OK;
+	return ret;
 }
 
 /**

^ permalink raw reply related

* [PATCH][TG3]Some cleanups
From: jamal @ 2007-09-30 18:11 UTC (permalink / raw)
  To: Michael Chan, Matt Carlson; +Cc: netdev

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


Here are some non-batching related changes that i have in my batching
tree. Like the e1000e, they make the xmit code more readable.
I wouldnt mind if you take them over.

cheers,
jamal


[-- Attachment #2: tg3-p1 --]
[-- Type: text/plain, Size: 13376 bytes --]

[TG3] Some cleanups
These cleanups make the xmit path code better functionally organized.
Matt Carlson contributed the moving of the VLAN formatting into
XXXX_prep_frame() portion.

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

---
commit 260dbcc4b0195897c539c5ff79d95afdddeb3378
tree b2047b0e474abb9f05dd40c22af7f0a86369957d
parent ad63c288ce980907f68d94d5faac08625c0b1782
author Jamal Hadi Salim <hadi@cyberus.ca> Sun, 30 Sep 2007 14:01:46 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Sun, 30 Sep 2007 14:01:46 -0400

 drivers/net/tg3.c |  278 ++++++++++++++++++++++++++++++++---------------------
 1 files changed, 169 insertions(+), 109 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index d4ac6e9..5a864bd 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -3910,47 +3910,69 @@ static void tg3_set_txd(struct tg3 *tp, int entry,
 	txd->vlan_tag = vlan_tag << TXD_VLAN_TAG_SHIFT;
 }
 
-/* hard_start_xmit for devices that don't have any bugs and
- * support TG3_FLG2_HW_TSO_2 only.
- */
-static int tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
+struct tg3_tx_cbdata {
+	u32 base_flags;
+	unsigned int mss;
+};
+#define TG3_SKB_CB(__skb)       ((struct tg3_tx_cbdata *)&((__skb)->cb[0]))
+#define NETDEV_TX_DROPPED       -5
+
+static int tg3_prep_bug_frame(struct sk_buff *skb, struct net_device *dev)
 {
+	struct tg3_tx_cbdata *cb = TG3_SKB_CB(skb);
+#if TG3_VLAN_TAG_USED
 	struct tg3 *tp = netdev_priv(dev);
-	dma_addr_t mapping;
-	u32 len, entry, base_flags, mss;
+	u32 vlantag = 0;
 
-	len = skb_headlen(skb);
+	if (tp->vlgrp != NULL && vlan_tx_tag_present(skb))
+		vlantag = (TXD_FLAG_VLAN | (vlan_tx_tag_get(skb) << 16));
 
-	/* We are running in BH disabled context with netif_tx_lock
-	 * and TX reclaim runs via tp->napi.poll inside of a software
-	 * interrupt.  Furthermore, IRQ processing runs lockless so we have
-	 * no IRQ context deadlocks to worry about either.  Rejoice!
-	 */
-	if (unlikely(tg3_tx_avail(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
-		if (!netif_queue_stopped(dev)) {
-			netif_stop_queue(dev);
+	cb->base_flags = vlantag;
+#endif
 
-			/* This is a hard error, log it. */
-			printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
-			       "queue awake!\n", dev->name);
+	cb->mss = skb_shinfo(skb)->gso_size;
+	if (cb->mss != 0) {
+		if (skb_header_cloned(skb) &&
+		    pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
+			dev_kfree_skb(skb);
+			return NETDEV_TX_DROPPED;
 		}
-		return NETDEV_TX_BUSY;
+
+		cb->base_flags |= (TXD_FLAG_CPU_PRE_DMA |
+			       TXD_FLAG_CPU_POST_DMA);
 	}
 
-	entry = tp->tx_prod;
-	base_flags = 0;
-	mss = 0;
-	if ((mss = skb_shinfo(skb)->gso_size) != 0) {
+	if (skb->ip_summed == CHECKSUM_PARTIAL)
+		cb->base_flags |= TXD_FLAG_TCPUDP_CSUM;
+
+	return NETDEV_TX_OK;
+}
+
+static int tg3_prep_frame(struct sk_buff *skb, struct net_device *dev)
+{
+	struct tg3_tx_cbdata *cb = TG3_SKB_CB(skb);
+#if TG3_VLAN_TAG_USED
+	struct tg3 *tp = netdev_priv(dev);
+	u32 vlantag = 0;
+
+	if (tp->vlgrp != NULL && vlan_tx_tag_present(skb))
+		vlantag = (TXD_FLAG_VLAN | (vlan_tx_tag_get(skb) << 16));
+
+	cb->base_flags = vlantag;
+#endif
+
+	cb->mss = skb_shinfo(skb)->gso_size;
+	if (cb->mss != 0) {
 		int tcp_opt_len, ip_tcp_len;
 
 		if (skb_header_cloned(skb) &&
 		    pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
 			dev_kfree_skb(skb);
-			goto out_unlock;
+			return NETDEV_TX_DROPPED;
 		}
 
 		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
-			mss |= (skb_headlen(skb) - ETH_HLEN) << 9;
+			cb->mss |= (skb_headlen(skb) - ETH_HLEN) << 9;
 		else {
 			struct iphdr *iph = ip_hdr(skb);
 
@@ -3958,32 +3980,58 @@ static int tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			ip_tcp_len = ip_hdrlen(skb) + sizeof(struct tcphdr);
 
 			iph->check = 0;
-			iph->tot_len = htons(mss + ip_tcp_len + tcp_opt_len);
-			mss |= (ip_tcp_len + tcp_opt_len) << 9;
+			iph->tot_len = htons(cb->mss + ip_tcp_len
+					     + tcp_opt_len);
+			cb->mss |= (ip_tcp_len + tcp_opt_len) << 9;
 		}
 
-		base_flags |= (TXD_FLAG_CPU_PRE_DMA |
+		cb->base_flags |= (TXD_FLAG_CPU_PRE_DMA |
 			       TXD_FLAG_CPU_POST_DMA);
 
 		tcp_hdr(skb)->check = 0;
 
 	}
 	else if (skb->ip_summed == CHECKSUM_PARTIAL)
-		base_flags |= TXD_FLAG_TCPUDP_CSUM;
-#if TG3_VLAN_TAG_USED
-	if (tp->vlgrp != NULL && vlan_tx_tag_present(skb))
-		base_flags |= (TXD_FLAG_VLAN |
-			       (vlan_tx_tag_get(skb) << 16));
-#endif
+		cb->base_flags |= TXD_FLAG_TCPUDP_CSUM;
+
+	return NETDEV_TX_OK;
+}
+
+void tg3_kick_DMA(struct net_device *dev)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	u32 entry = tp->tx_prod;
+
+	/* Packets are ready, update Tx producer idx local and on card. */
+	tw32_tx_mbox((MAILBOX_SNDHOST_PROD_IDX_0 + TG3_64BIT_REG_LOW), entry);
+
+	if (unlikely(tg3_tx_avail(tp) <= (MAX_SKB_FRAGS + 1))) {
+		netif_stop_queue(dev);
+		if (tg3_tx_avail(tp) >= TG3_TX_WAKEUP_THRESH(tp))
+			netif_wake_queue(dev);
+	}
+
+	mmiowb();
+	dev->trans_start = jiffies;
+}
 
+static int tg3_enqueue(struct sk_buff *skb, struct net_device *dev)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	dma_addr_t mapping;
+	u32 len, entry;
+	struct tg3_tx_cbdata *cb = TG3_SKB_CB(skb);
+
+	entry = tp->tx_prod;
+	len = skb_headlen(skb);
 	/* Queue skb data, a.k.a. the main skb fragment. */
 	mapping = pci_map_single(tp->pdev, skb->data, len, PCI_DMA_TODEVICE);
 
 	tp->tx_buffers[entry].skb = skb;
 	pci_unmap_addr_set(&tp->tx_buffers[entry], mapping, mapping);
 
-	tg3_set_txd(tp, entry, mapping, len, base_flags,
-		    (skb_shinfo(skb)->nr_frags == 0) | (mss << 1));
+	tg3_set_txd(tp, entry, mapping, len, cb->base_flags,
+		    (skb_shinfo(skb)->nr_frags == 0) | (cb->mss << 1));
 
 	entry = NEXT_TX(entry);
 
@@ -4005,28 +4053,51 @@ static int tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			pci_unmap_addr_set(&tp->tx_buffers[entry], mapping, mapping);
 
 			tg3_set_txd(tp, entry, mapping, len,
-				    base_flags, (i == last) | (mss << 1));
+				    cb->base_flags,
+				    (i == last) | (cb->mss << 1));
 
 			entry = NEXT_TX(entry);
 		}
 	}
 
-	/* Packets are ready, update Tx producer idx local and on card. */
-	tw32_tx_mbox((MAILBOX_SNDHOST_PROD_IDX_0 + TG3_64BIT_REG_LOW), entry);
-
 	tp->tx_prod = entry;
-	if (unlikely(tg3_tx_avail(tp) <= (MAX_SKB_FRAGS + 1))) {
-		netif_stop_queue(dev);
-		if (tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp))
-			netif_wake_queue(tp->dev);
-	}
+	return NETDEV_TX_OK;
+}
+
+/* hard_start_xmit for devices that don't have any bugs and
+ * support TG3_FLG2_HW_TSO_2 only.
+ */
+static int tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	int ret = tg3_prep_frame(skb, dev);
+	/* XXX: original code did mmiowb(); on failure,
+	* I dont think thats necessary
+	*/
+	if (unlikely(ret != NETDEV_TX_OK))
+	       return NETDEV_TX_OK;
 
-out_unlock:
-    	mmiowb();
+	/* We are running in BH disabled context with netif_tx_lock
+	 * and TX reclaim runs via tp->poll inside of a software
+	 * interrupt.  Furthermore, IRQ processing runs lockless so we have
+	 * no IRQ context deadlocks to worry about either.  Rejoice!
+	 */
+	if (unlikely(tg3_tx_avail(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
+		if (!netif_queue_stopped(dev)) {
+			netif_stop_queue(dev);
 
-	dev->trans_start = jiffies;
+			/* This is a hard error, log it. */
+			printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
+			       "queue awake!\n", dev->name);
+		}
+		return NETDEV_TX_BUSY;
+	}
 
-	return NETDEV_TX_OK;
+	ret = tg3_enqueue(skb, dev);
+	if (ret == NETDEV_TX_OK)
+		tg3_kick_DMA(dev);
+
+	return ret;
 }
 
 static int tg3_start_xmit_dma_bug(struct sk_buff *, struct net_device *);
@@ -4067,46 +4138,19 @@ tg3_tso_bug_end:
 /* hard_start_xmit for devices that have the 4G bug and/or 40-bit bug and
  * support TG3_FLG2_HW_TSO_1 or firmware TSO only.
  */
-static int tg3_start_xmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
+static int tg3_enqueue_buggy(struct sk_buff *skb, struct net_device *dev)
 {
 	struct tg3 *tp = netdev_priv(dev);
 	dma_addr_t mapping;
-	u32 len, entry, base_flags, mss;
+	u32 len, entry;
 	int would_hit_hwbug;
+	struct tg3_tx_cbdata *cb = TG3_SKB_CB(skb);
 
-	len = skb_headlen(skb);
 
-	/* We are running in BH disabled context with netif_tx_lock
-	 * and TX reclaim runs via tp->napi.poll inside of a software
-	 * interrupt.  Furthermore, IRQ processing runs lockless so we have
-	 * no IRQ context deadlocks to worry about either.  Rejoice!
-	 */
-	if (unlikely(tg3_tx_avail(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
-		if (!netif_queue_stopped(dev)) {
-			netif_stop_queue(dev);
-
-			/* This is a hard error, log it. */
-			printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
-			       "queue awake!\n", dev->name);
-		}
-		return NETDEV_TX_BUSY;
-	}
-
-	entry = tp->tx_prod;
-	base_flags = 0;
-	if (skb->ip_summed == CHECKSUM_PARTIAL)
-		base_flags |= TXD_FLAG_TCPUDP_CSUM;
-	mss = 0;
-	if ((mss = skb_shinfo(skb)->gso_size) != 0) {
+	if (cb->mss != 0) {
 		struct iphdr *iph;
 		int tcp_opt_len, ip_tcp_len, hdr_len;
 
-		if (skb_header_cloned(skb) &&
-		    pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
-			dev_kfree_skb(skb);
-			goto out_unlock;
-		}
-
 		tcp_opt_len = tcp_optlen(skb);
 		ip_tcp_len = ip_hdrlen(skb) + sizeof(struct tcphdr);
 
@@ -4115,15 +4159,13 @@ static int tg3_start_xmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
 			     (tp->tg3_flags2 & TG3_FLG2_TSO_BUG))
 			return (tg3_tso_bug(tp, skb));
 
-		base_flags |= (TXD_FLAG_CPU_PRE_DMA |
-			       TXD_FLAG_CPU_POST_DMA);
 
 		iph = ip_hdr(skb);
 		iph->check = 0;
-		iph->tot_len = htons(mss + hdr_len);
+		iph->tot_len = htons(cb->mss + hdr_len);
 		if (tp->tg3_flags2 & TG3_FLG2_HW_TSO) {
 			tcp_hdr(skb)->check = 0;
-			base_flags &= ~TXD_FLAG_TCPUDP_CSUM;
+			cb->base_flags &= ~TXD_FLAG_TCPUDP_CSUM;
 		} else
 			tcp_hdr(skb)->check = ~csum_tcpudp_magic(iph->saddr,
 								 iph->daddr, 0,
@@ -4136,22 +4178,19 @@ static int tg3_start_xmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
 				int tsflags;
 
 				tsflags = (iph->ihl - 5) + (tcp_opt_len >> 2);
-				mss |= (tsflags << 11);
+				cb->mss |= (tsflags << 11);
 			}
 		} else {
 			if (tcp_opt_len || iph->ihl > 5) {
 				int tsflags;
 
 				tsflags = (iph->ihl - 5) + (tcp_opt_len >> 2);
-				base_flags |= tsflags << 12;
+				cb->base_flags |= tsflags << 12;
 			}
 		}
 	}
-#if TG3_VLAN_TAG_USED
-	if (tp->vlgrp != NULL && vlan_tx_tag_present(skb))
-		base_flags |= (TXD_FLAG_VLAN |
-			       (vlan_tx_tag_get(skb) << 16));
-#endif
+	len = skb_headlen(skb);
+	entry = tp->tx_prod;
 
 	/* Queue skb data, a.k.a. the main skb fragment. */
 	mapping = pci_map_single(tp->pdev, skb->data, len, PCI_DMA_TODEVICE);
@@ -4164,8 +4203,8 @@ static int tg3_start_xmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
 	if (tg3_4g_overflow_test(mapping, len))
 		would_hit_hwbug = 1;
 
-	tg3_set_txd(tp, entry, mapping, len, base_flags,
-		    (skb_shinfo(skb)->nr_frags == 0) | (mss << 1));
+	tg3_set_txd(tp, entry, mapping, len, cb->base_flags,
+		    (skb_shinfo(skb)->nr_frags == 0) | (cb->mss << 1));
 
 	entry = NEXT_TX(entry);
 
@@ -4194,10 +4233,11 @@ static int tg3_start_xmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
 
 			if (tp->tg3_flags2 & TG3_FLG2_HW_TSO)
 				tg3_set_txd(tp, entry, mapping, len,
-					    base_flags, (i == last)|(mss << 1));
+					    cb->base_flags,
+					    (i == last)|(cb->mss << 1));
 			else
 				tg3_set_txd(tp, entry, mapping, len,
-					    base_flags, (i == last));
+					    cb->base_flags, (i == last));
 
 			entry = NEXT_TX(entry);
 		}
@@ -4214,28 +4254,48 @@ static int tg3_start_xmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
 		 * failure, silently drop this packet.
 		 */
 		if (tigon3_dma_hwbug_workaround(tp, skb, last_plus_one,
-						&start, base_flags, mss))
-			goto out_unlock;
+						&start, cb->base_flags,
+						cb->mss)) {
+			mmiowb();
+			return NETDEV_TX_OK;
+		}
 
 		entry = start;
 	}
 
-	/* Packets are ready, update Tx producer idx local and on card. */
-	tw32_tx_mbox((MAILBOX_SNDHOST_PROD_IDX_0 + TG3_64BIT_REG_LOW), entry);
-
 	tp->tx_prod = entry;
-	if (unlikely(tg3_tx_avail(tp) <= (MAX_SKB_FRAGS + 1))) {
-		netif_stop_queue(dev);
-		if (tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp))
-			netif_wake_queue(tp->dev);
-	}
+	return NETDEV_TX_OK;
+}
+
+static int tg3_start_xmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	int ret = tg3_prep_bug_frame(skb, dev);
 
-out_unlock:
-    	mmiowb();
+	if (unlikely(ret != NETDEV_TX_OK))
+	       return NETDEV_TX_OK;
 
-	dev->trans_start = jiffies;
+	/* We are running in BH disabled context with netif_tx_lock
+	 * and TX reclaim runs via tp->poll inside of a software
+	 * interrupt.  Furthermore, IRQ processing runs lockless so we have
+	 * no IRQ context deadlocks to worry about either.  Rejoice!
+	 */
+	if (unlikely(tg3_tx_avail(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
+		if (!netif_queue_stopped(dev)) {
+			netif_stop_queue(dev);
 
-	return NETDEV_TX_OK;
+			/* This is a hard error, log it. */
+			printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
+			       "queue awake!\n", dev->name);
+		}
+		return NETDEV_TX_BUSY;
+	}
+
+	ret = tg3_enqueue_buggy(skb, dev);
+	if (ret == NETDEV_TX_OK)
+		tg3_kick_DMA(dev);
+
+	return ret;
 }
 
 static inline void tg3_set_mtu(struct net_device *dev, struct tg3 *tp,

^ permalink raw reply related

* Re: [PATCH][TG3]Some cleanups
From: jamal @ 2007-09-30 18:12 UTC (permalink / raw)
  To: Michael Chan; +Cc: Matt Carlson, netdev
In-Reply-To: <1191175890.6165.17.camel@localhost>

On Sun, 2007-30-09 at 14:11 -0400, jamal wrote:
> Here are some non-batching related changes that i have in my batching
> tree. Like the e1000e, they make the xmit code more readable.
> I wouldnt mind if you take them over.

Should have mentioned: Against Dave's tree from a few hours back.

cheers,
jamal


^ permalink raw reply

* Re: [PATCH][E1000E] some cleanups
From: Kok, Auke @ 2007-09-30 18:16 UTC (permalink / raw)
  To: hadi; +Cc: Kok, Auke, netdev
In-Reply-To: <1191174077.6165.11.camel@localhost>

jamal wrote:
> Auke,
> 
> heres part of something i promised. 
> I couldnt do any packet testing on because 82571EB is disabled in the
> driver. I uncommented the code out in the table, but the best i could
> get was the module loading, some probing and some sysfs renaming
> failures (probably a debianism); the machine access is intermittent, so
> thats as far as i could go. In any case, you probably have a good reason
> for disabling that chip. So, heres the patch, the burden of testing now
> falls on you ;->

no, all the hardware that is commented should work just fine. I tested this driver
on 82571, 82573 and ich8/ich9 - extensively.

the reason that we disable them is that we're going to migrate devices over in
batches. At introduction we'll support ich9, afterwards we'll drop in the IDs of
the other groups of silicon.

> Once you have 82571EB on and kicking, my next steps are to kill LLTX
> then add batching on top.
> BTW, since this driver is just for PCIE, would you take a similar patch
> for non-PCIE e1000?

if it's a fix, yes.

> comment:
> There used to be an "mmiowb()" call right after the dma wake which is
> gone now; is this unneeded with pcie? I have restored it, look for the
> "XXX".


thanks, I'll go and look at this in depth in the coming weeks.

Auke

^ permalink raw reply

* [ofa-general] [PATCHES] TX batching
From: jamal @ 2007-09-30 18:50 UTC (permalink / raw)
  To: David Miller
  Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
	Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
	general, mchan, tgraf, jeff, sri, shemminger, kaber
In-Reply-To: <1190569987.4256.52.camel@localhost>


Latest net-2.6.24 breaks the patches i posted last week; so this is an
update to resolve that. If you are receiving these emails and are
finding them overloading, please give me a shout and i will remove your
name.

Please provide feedback on the code and/or architecture.
Last time i posted them i received none. They are now updated to 
work with the latest net-2.6.24 from a few hours ago.

Patch 1: Introduces batching interface
Patch 2: Core uses batching interface
Patch 3: get rid of dev->gso_skb

I have decided i will kill ->hard_batch_xmit() and not support any
more LLTX drivers. This is the last of patches that will have
->hard_batch_xmit() as i am supporting an e1000 that is LLTX.

Dave please let me know if this meets your desires to allow devices
which are SG and able to compute CSUM benefit just in case i
misunderstood. 
Herbert, if you can look at at least patch 3 i will appreaciate it
(since it kills dev->gso_skb that you introduced).

More patches to follow later if i get some feedback - i didnt want to 
overload people by dumping too many patches. Most of these patches 
mentioned below are ready to go; some need some re-testing and others 
need a little porting from an earlier kernel: 
- tg3 driver (tested and works well, but dont want to send 
- tun driver
- pktgen
- netiron driver
- e1000 driver (LLTX)
- e1000e driver (non-LLTX)
- ethtool interface
- There is at least one other driver promised to me

Theres also a driver-howto i wrote that was posted on netdev last week
as well as one that describes the architectural decisions made.

Each of these patches has been performance tested (last with DaveM's
tree from last weekend) and the results are in the logs on a per-patch 
basis.  My system under test hardware is a 2xdual core opteron with a 
couple of tg3s. I have not re-run the tests with this morning's tree
but i suspect not much difference.
My test tool generates udp traffic of different sizes for upto 60 
seconds per run or a total of 30M packets. I have 4 threads each 
running on a specific CPU which keep all the CPUs as busy as they can 
sending packets targetted at a directly connected box's udp discard
port.
All 4 CPUs target a single tg3 to send. The receiving box has a tc rule 
which counts and drops all incoming udp packets to discard port - this
allows me to make sure that the receiver is not the bottleneck in the
testing. Packet sizes sent are {64B, 128B, 256B, 512B, 1024B}. Each
packet size run is repeated 10 times to ensure that there are no
transients. The average of all 10 runs is then computed and collected.

cheers,
jamal

^ permalink raw reply

* [ofa-general] [PATCH 1/4] [NET_BATCH] Introduce batching interface
From: jamal @ 2007-09-30 18:51 UTC (permalink / raw)
  To: David Miller
  Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
	Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
	general, mchan, tgraf, jeff, sri, shemminger, kaber
In-Reply-To: <1190570317.4256.59.camel@localhost>

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


This patch introduces the netdevice interface for batching.

cheers,
jamal



[-- Attachment #2: sep30-p10f3 --]
[-- Type: text/plain, Size: 8794 bytes --]

[NET_BATCH] Introduce batching interface

This patch introduces the netdevice interface for batching.

A typical driver dev->hard_start_xmit() has 4 parts:
a) packet formating (example vlan, mss, descriptor counting etc)
b) chip specific formatting
c) enqueueing the packet on a DMA ring
d) IO operations to complete packet transmit, tell DMA engine to chew on,
tx completion interupts etc

[For code cleanliness/readability sake, regardless of this work,
one should break the dev->hard_start_xmit() into those 4 functions
anyways].

With the api introduced in this patch, a driver which has all
4 parts and needing to support batching is advised to split its
dev->hard_start_xmit() in the following manner:
1)use its dev->hard_prep_xmit() method to achieve #a
2)use its dev->hard_end_xmit() method to achieve #d
3)#b and #c can stay in ->hard_start_xmit() (or whichever way you want
to do this)
Note: There are drivers which may need not support any of the two
methods (example the tun driver i patched) so the two methods are
optional.

The core will first do the packet formatting by invoking your supplied
dev->hard_prep_xmit() method. It will then pass you the packet via
your dev->hard_start_xmit() method and lastly will invoke your
dev->hard_end_xmit() when it completes passing you all the packets
queued for you. dev->hard_prep_xmit() is invoked without holding any
tx lock but the rest are under TX_LOCK().

LLTX present a challenge in that we have to introduce a deviation
from the norm and introduce the ->hard_batch_xmit() method. An LLTX
driver presents us with ->hard_batch_xmit() to which we pass it a list
of packets in a dev->blist skb queue. It is then the responsibility
of the ->hard_batch_xmit() to exercise steps #b and #c for all packets
and #d when the batching is complete. Step #a is already done for you
by the time you get the packets in dev->blist.
And last xmit_win variable is introduced to ensure that when we pass
the driver a list of packets it will swallow all of them - which is
useful because we dont requeue to the qdisc (and avoids burning
unnecessary cpu cycles or introducing any strange re-ordering). The driver
tells us when it invokes netif_wake_queue how much space it has for
descriptors by setting this variable.

Some decisions i had to make:
- every driver will have a xmit_win variable and the core will set it
to 1 which means the behavior of non-batching drivers stays the same.
- the batch list, blist, is no longer a pointer; wastes a little extra
memmory i plan to recoup by killing gso_skb in later patches.

Theres a lot of history and reasoning of why batching in a document
i am writting which i may submit as a patch.
Thomas Graf (who doesnt know this probably) gave me the impetus to
start looking at this back in 2004 when he invited me to the linux
conference he was organizing. Parts of what i presented in SUCON in
2004 talk about batching. Herbert Xu forced me to take a second look around
2.6.18 - refer to my netconf 2006 presentation. Krishna Kumar provided
me with more motivation in May 2007 when he posted on netdev and engaged
me.
Sridhar Samudrala, Krishna Kumar, Matt Carlson, Michael Chan,
Jeremy Ethridge, Evgeniy Polyakov, Sivakumar Subramani, and
David Miller, have contributed in one or more of {bug fixes, enhancements,
testing, lively discussion}. The Broadcom and netiron folks have been
outstanding in their help.

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

---
commit 624a0bfeb971c9aa58496c7372df01f0ed750def
tree c1c0ee53453392866a5241631a7502ce6569b2cc
parent 260dbcc4b0195897c539c5ff79d95afdddeb3378
author Jamal Hadi Salim <hadi@cyberus.ca> Sun, 30 Sep 2007 14:23:31 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Sun, 30 Sep 2007 14:23:31 -0400

 include/linux/netdevice.h |   17 +++++++
 net/core/dev.c            |  106 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+), 0 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 91cd3f3..df1fb61 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -467,6 +467,7 @@ struct net_device
 #define NETIF_F_NETNS_LOCAL	8192	/* Does not change network namespaces */
 #define NETIF_F_MULTI_QUEUE	16384	/* Has multiple TX/RX queues */
 #define NETIF_F_LRO		32768	/* large receive offload */
+#define NETIF_F_BTX		65536	/* Capable of batch tx */
 
 	/* Segmentation offload features */
 #define NETIF_F_GSO_SHIFT	16
@@ -595,6 +596,15 @@ struct net_device
 	void			*priv;	/* pointer to private data	*/
 	int			(*hard_start_xmit) (struct sk_buff *skb,
 						    struct net_device *dev);
+	/* hard_batch_xmit is needed for LLTX, kill it when those
+	 * disappear or better kill it now and dont support LLTX
+	*/
+	int			(*hard_batch_xmit) (struct net_device *dev);
+	int			(*hard_prep_xmit) (struct sk_buff *skb,
+						   struct net_device *dev);
+	void			(*hard_end_xmit) (struct net_device *dev);
+	int			xmit_win;
+
 	/* These may be needed for future network-power-down code. */
 	unsigned long		trans_start;	/* Time (in jiffies) of last Tx	*/
 
@@ -609,6 +619,7 @@ struct net_device
 
 	/* delayed register/unregister */
 	struct list_head	todo_list;
+	struct sk_buff_head     blist;
 	/* device index hash chain */
 	struct hlist_node	index_hlist;
 
@@ -1044,6 +1055,12 @@ extern int		dev_set_mac_address(struct net_device *,
 					    struct sockaddr *);
 extern int		dev_hard_start_xmit(struct sk_buff *skb,
 					    struct net_device *dev);
+extern int		dev_batch_xmit(struct net_device *dev);
+extern int		prepare_gso_skb(struct sk_buff *skb,
+					struct net_device *dev,
+					struct sk_buff_head *skbs);
+extern int		xmit_prepare_skb(struct sk_buff *skb,
+					 struct net_device *dev);
 
 extern int		netdev_budget;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 833f060..f82aff7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1517,6 +1517,110 @@ static int dev_gso_segment(struct sk_buff *skb)
 	return 0;
 }
 
+int prepare_gso_skb(struct sk_buff *skb, struct net_device *dev,
+		    struct sk_buff_head *skbs)
+{
+	int tdq = 0;
+	do {
+		struct sk_buff *nskb = skb->next;
+
+		skb->next = nskb->next;
+		nskb->next = NULL;
+
+		if (dev->hard_prep_xmit) {
+			/* note: skb->cb is set in hard_prep_xmit(),
+			 * it should not be trampled somewhere
+			 * between here and the driver picking it
+			 * The VLAN code wrongly assumes it owns it
+			 * so the driver needs to be careful; for
+			 * good handling look at tg3 driver ..
+			*/
+			int ret = dev->hard_prep_xmit(nskb, dev);
+			if (ret != NETDEV_TX_OK)
+				continue;
+		}
+		/* Driver likes this packet .. */
+		tdq++;
+		__skb_queue_tail(skbs, nskb);
+	} while (skb->next);
+	skb->destructor = DEV_GSO_CB(skb)->destructor;
+	kfree_skb(skb);
+
+	return tdq;
+}
+
+int xmit_prepare_skb(struct sk_buff *skb, struct net_device *dev)
+{
+	struct sk_buff_head *skbs = &dev->blist;
+
+	if (netif_needs_gso(dev, skb)) {
+		if (unlikely(dev_gso_segment(skb))) {
+			kfree_skb(skb);
+			return 0;
+		}
+		if (skb->next)
+			return prepare_gso_skb(skb, dev, skbs);
+	}
+
+	if (dev->hard_prep_xmit) {
+		int ret = dev->hard_prep_xmit(skb, dev);
+		if (ret != NETDEV_TX_OK)
+			return 0;
+	}
+	__skb_queue_tail(skbs, skb);
+	return 1;
+}
+
+int dev_batch_xmit(struct net_device *dev)
+{
+	struct sk_buff_head *skbs = &dev->blist;
+	int rc = NETDEV_TX_OK;
+	struct sk_buff *skb;
+	int orig_w = dev->xmit_win;
+	int orig_pkts = skb_queue_len(skbs);
+
+	if (dev->hard_batch_xmit) { /* only for LLTX devices */
+		rc = dev->hard_batch_xmit(dev);
+	} else {
+		while ((skb = __skb_dequeue(skbs)) != NULL) {
+			if (!list_empty(&ptype_all))
+				dev_queue_xmit_nit(skb, dev);
+			rc = dev->hard_start_xmit(skb, dev);
+			if (unlikely(rc))
+				break;
+			/*
+			 * XXX: multiqueue may need closer srutiny..
+			*/
+			if (unlikely(netif_queue_stopped(dev) ||
+			     netif_subqueue_stopped(dev, skb->queue_mapping))) {
+				rc = NETDEV_TX_BUSY;
+				break;
+			}
+		}
+	}
+
+	/* driver is likely buggy and lied to us on how much
+	 * space it had. Damn you driver ..
+	*/
+	if (unlikely(skb_queue_len(skbs))) {
+		printk(KERN_WARNING "Likely bug %s %s (%d) "
+				"left %d/%d window now %d, orig %d\n",
+			dev->name, rc?"busy":"locked",
+			netif_queue_stopped(dev),
+			skb_queue_len(skbs),
+			orig_pkts,
+			dev->xmit_win,
+			orig_w);
+			rc = NETDEV_TX_BUSY;
+	}
+
+	if (orig_pkts > skb_queue_len(skbs))
+		if (dev->hard_end_xmit)
+			dev->hard_end_xmit(dev);
+
+	return rc;
+}
+
 int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	if (likely(!skb->next)) {
@@ -3551,6 +3655,8 @@ int register_netdevice(struct net_device *dev)
 		}
 	}
 
+	dev->xmit_win = 1;
+	skb_queue_head_init(&dev->blist);
 	ret = netdev_register_kobject(dev);
 	if (ret)
 		goto err_uninit;

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related

* [ofa-general] [PATCH 2/3][NET_BATCH] net core use batching
From: jamal @ 2007-09-30 18:52 UTC (permalink / raw)
  To: David Miller
  Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
	Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
	general, mchan, tgraf, jeff, sri, shemminger, kaber
In-Reply-To: <1190570409.4256.62.camel@localhost>

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


This patch adds the usage of batching within the core.

cheers,
jamal




[-- Attachment #2: sep30-p2of3 --]
[-- Type: text/plain, Size: 6919 bytes --]

[NET_BATCH] net core use batching

This patch adds the usage of batching within the core.
The same test methodology used in introducing txlock is used, with
the following results on different kernels:

        +------------+--------------+-------------+------------+--------+
        |       64B  |  128B        | 256B        | 512B       |1024B   |
        +------------+--------------+-------------+------------+--------+
Original| 467482     | 463061       | 388267      | 216308     | 114704 |
        |            |              |             |            |        |
txlock  | 468922     | 464060       | 388298      | 216316     | 114709 |
        |            |              |             |            |        |
tg3nobtx| 468012     | 464079       | 388293      | 216314     | 114704 |
        |            |              |             |            |        |
tg3btxdr| 480794     | 475102       | 388298      | 216316     | 114705 |
        |            |              |             |            |        |
tg3btxco| 481059     | 475423       | 388285      | 216308     | 114706 |
        +------------+--------------+-------------+------------+--------+

The first two colums "Original" and "txlock" were introduced in an earlier
patch and demonstrate a slight increase in performance with txlock.
"tg3nobtx" shows the tg3 driver with no changes to support batching.
The purpose of this test is to demonstrate the effect of introducing
the core changes to a driver that doesnt support them.
Although this patch brings down perfomance slightly compared to txlock
for such netdevices, it is still better compared to just the original
kernel.
"tg3btxdr" demonstrates the effect of using ->hard_batch_xmit() with tg3
driver. "tg3btxco" demonstrates the effect of letting the core do all the
work. As can be seen the last two are not very different in performance.
The difference is ->hard_batch_xmit() introduces a new method which
is intrusive.

I have #if-0ed some of the old functions so the patch is more readable.

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

---
commit 9b4a8fb190278d388c0a622fb5529d184ac8c7dc
tree 053e8dda02b5d26fe7cc778823306a8a526df513
parent 624a0bfeb971c9aa58496c7372df01f0ed750def
author Jamal Hadi Salim <hadi@cyberus.ca> Sun, 30 Sep 2007 14:38:11 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Sun, 30 Sep 2007 14:38:11 -0400

 net/sched/sch_generic.c |  127 +++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 115 insertions(+), 12 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 95ae119..86a3f9d 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(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,27 @@ 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)
+
+static inline int qdisc_restart(struct net_device *dev,
+				struct sk_buff_head *tpktlist)
 {
 	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, tpktlist);
 
+	if (!ret)
+		return 0;
 
-	/* And release queue */
+	/* We got em packets */
 	spin_unlock(&dev->queue_lock);
 
+	/* prepare to embark */
+	xmit_prepare_pkts(dev, tpktlist);
+
+	/* 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 +255,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 +265,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;
 	}
 
@@ -177,8 +274,11 @@ static inline int qdisc_restart(struct net_device *dev)
 
 void __qdisc_run(struct net_device *dev)
 {
+	struct sk_buff_head tpktlist;
+	skb_queue_head_init(&tpktlist);
+
 	do {
-		if (!qdisc_restart(dev))
+		if (!qdisc_restart(dev, &tpktlist))
 			break;
 	} while (!netif_queue_stopped(dev));
 
@@ -564,6 +664,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);

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related

* [ofa-general] [PATCH 3/3][NET_SCHED] kill dev->gso_skb
From: jamal @ 2007-09-30 18:53 UTC (permalink / raw)
  To: David Miller
  Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
	Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
	general, mchan, tgraf, jeff, sri, shemminger, kaber
In-Reply-To: <1190570521.4256.65.camel@localhost>

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


This patch removes dev->gso_skb as it is no longer necessary with
batching code.

cheers,
jamal



[-- Attachment #2: sep30-p3of3 --]
[-- Type: text/plain, Size: 2277 bytes --]

[NET_SCHED] kill dev->gso_skb
The batching code does what gso used to batch at the drivers.
There is no more need for gso_skb. If for whatever reason the
requeueing is a bad idea we are going to leave packets in dev->blist
(and still not need dev->gso_skb)

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

---
commit c2916c550d228472ddcdd676c2689fa6c8ecfcc0
tree 5beaf197fd08a038d83501f405017f48712d0318
parent 9b4a8fb190278d388c0a622fb5529d184ac8c7dc
author Jamal Hadi Salim <hadi@cyberus.ca> Sun, 30 Sep 2007 14:38:58 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Sun, 30 Sep 2007 14:38:58 -0400

 include/linux/netdevice.h |    3 ---
 net/sched/sch_generic.c   |   12 ------------
 2 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index df1fb61..cea400a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -577,9 +577,6 @@ struct net_device
 	struct list_head	qdisc_list;
 	unsigned long		tx_queue_len;	/* Max frames per queue allowed */
 
-	/* Partially transmitted GSO packet. */
-	struct sk_buff		*gso_skb;
-
 	/* ingress path synchronizer */
 	spinlock_t		ingress_lock;
 	struct Qdisc		*qdisc_ingress;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 86a3f9d..b4e1607 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -172,13 +172,6 @@ static int xmit_get_pkts(struct net_device *dev,
 	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)
@@ -654,7 +647,6 @@ void dev_activate(struct net_device *dev)
 void dev_deactivate(struct net_device *dev)
 {
 	struct Qdisc *qdisc;
-	struct sk_buff *skb;
 
 	spin_lock_bh(&dev->queue_lock);
 	qdisc = dev->qdisc;
@@ -662,15 +654,11 @@ void dev_deactivate(struct net_device *dev)
 
 	qdisc_reset(qdisc);
 
-	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);
-
 	dev_watchdog_down(dev);
 
 	/* Wait for outstanding dev_queue_xmit calls. */

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related

* [ofa-general] Re: [PATCH 1/3] [NET_BATCH] Introduce batching interface
From: jamal @ 2007-09-30 18:54 UTC (permalink / raw)
  To: David Miller
  Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
	Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
	general, mchan, tgraf, jeff, sri, shemminger, kaber
In-Reply-To: <1191178284.6165.27.camel@localhost>

Fixed subject - should be 1/3 not 1/4

On Sun, 2007-30-09 at 14:51 -0400, jamal wrote:
> This patch introduces the netdevice interface for batching.
> 
> cheers,
> jamal
> 
> 

^ permalink raw reply

* Re: [PATCH][E1000E] some cleanups
From: jamal @ 2007-09-30 19:01 UTC (permalink / raw)
  To: Kok, Auke; +Cc: netdev
In-Reply-To: <46FFE809.8020504@intel.com>

On Sun, 2007-30-09 at 11:16 -0700, Kok, Auke wrote:

> no, all the hardware that is commented should work just fine. I tested this driver
> on 82571, 82573 and ich8/ich9 - extensively.

Something else is wrong then. Can you just uncomment the 82571EB bits in
Dave's net-2.6.24 and just send a ping? If it works, let me know what
you did so i can test next time i get a chance.

> the reason that we disable them is that we're going to migrate devices over in
> batches. At introduction we'll support ich9, afterwards we'll drop in the IDs of
> the other groups of silicon.

Turn them on if you want people to start using that driver.

> > Once you have 82571EB on and kicking, my next steps are to kill LLTX
> > then add batching on top.
> > BTW, since this driver is just for PCIE, would you take a similar patch
> > for non-PCIE e1000?
> 
> if it's a fix, yes.

It just makes it easier to kill LLTX. If you consider killing LLTX
risky, then i will focus on e1000e.

cheers,
jamal


^ permalink raw reply

* [ofa-general] Re: [PATCHES] TX batching
From: jamal @ 2007-09-30 19:19 UTC (permalink / raw)
  To: David Miller
  Cc: johnpol, peter.p.waskiewicz.jr, kumarkr, herbert, gaagaan,
	Robert.Olsson, netdev, rdreier, mcarlson, randy.dunlap, jagana,
	general, mchan, tgraf, jeff, sri, shemminger, kaber
In-Reply-To: <1191178205.6165.25.camel@localhost>

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

And heres a patch that provides a sample of the usage for batching with
tg3. 
Requires patch "[TG3]Some cleanups" i posted earlier. 

cheers,
jamal


[-- Attachment #2: tg3.potoc --]
[-- Type: text/x-patch, Size: 5252 bytes --]

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 5a864bd..9aafb78 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -3103,6 +3103,13 @@ static inline u32 tg3_tx_avail(struct tg3 *tp)
 		((tp->tx_prod - tp->tx_cons) & (TG3_TX_RING_SIZE - 1)));
 }
 
+static inline void tg3_set_win(struct tg3 *tp)
+{
+	tp->dev->xmit_win = tg3_tx_avail(tp) - (MAX_SKB_FRAGS + 1);
+	if (tp->dev->xmit_win < 1)
+		tp->dev->xmit_win = 1;
+}
+
 /* Tigon3 never reports partial packet sends.  So we do not
  * need special logic to handle SKBs that have not had all
  * of their frags sent yet, like SunGEM does.
@@ -3165,8 +3172,10 @@ static void tg3_tx(struct tg3 *tp)
 		     (tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp)))) {
 		netif_tx_lock(tp->dev);
 		if (netif_queue_stopped(tp->dev) &&
-		    (tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp)))
+		    (tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp))) {
+			tg3_set_win(tp);
 			netif_wake_queue(tp->dev);
+		}
 		netif_tx_unlock(tp->dev);
 	}
 }
@@ -4007,8 +4016,13 @@ void tg3_kick_DMA(struct net_device *dev)
 
 	if (unlikely(tg3_tx_avail(tp) <= (MAX_SKB_FRAGS + 1))) {
 		netif_stop_queue(dev);
-		if (tg3_tx_avail(tp) >= TG3_TX_WAKEUP_THRESH(tp))
+		dev->xmit_win = 1;
+		if (tg3_tx_avail(tp) >= TG3_TX_WAKEUP_THRESH(tp)) {
+			tg3_set_win(tp);
 			netif_wake_queue(dev);
+		}
+	} else {
+		tg3_set_win(tp);
 	}
 
 	mmiowb();
@@ -4085,6 +4099,7 @@ static int tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (unlikely(tg3_tx_avail(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
 		if (!netif_queue_stopped(dev)) {
 			netif_stop_queue(dev);
+			tp->dev->xmit_win = 1;
 
 			/* This is a hard error, log it. */
 			printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
@@ -4100,6 +4115,25 @@ static int tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return ret;
 }
 
+static int tg3_start_bxmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct tg3 *tp = netdev_priv(dev);
+
+	if (unlikely(tg3_tx_avail(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
+		if (!netif_queue_stopped(dev)) {
+			netif_stop_queue(dev);
+			dev->xmit_win = 1;
+
+			/* This is a hard error, log it. */
+			printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
+			       "queue awake!\n", dev->name);
+		}
+		return NETDEV_TX_BUSY;
+	}
+
+	return tg3_enqueue(skb, dev);
+}
+
 static int tg3_start_xmit_dma_bug(struct sk_buff *, struct net_device *);
 
 /* Use GSO to workaround a rare TSO bug that may be triggered when the
@@ -4112,9 +4146,11 @@ static int tg3_tso_bug(struct tg3 *tp, struct sk_buff *skb)
 	/* Estimate the number of fragments in the worst case */
 	if (unlikely(tg3_tx_avail(tp) <= (skb_shinfo(skb)->gso_segs * 3))) {
 		netif_stop_queue(tp->dev);
+		tp->dev->xmit_win = 1;
 		if (tg3_tx_avail(tp) <= (skb_shinfo(skb)->gso_segs * 3))
 			return NETDEV_TX_BUSY;
 
+		tg3_set_win(tp);
 		netif_wake_queue(tp->dev);
 	}
 
@@ -4267,6 +4303,25 @@ static int tg3_enqueue_buggy(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
+static int tg3_start_bxmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
+{
+	struct tg3 *tp = netdev_priv(dev);
+
+	if (unlikely(tg3_tx_avail(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
+		if (!netif_queue_stopped(dev)) {
+			netif_stop_queue(dev);
+			dev->xmit_win = 1;
+
+			/* This is a hard error, log it. */
+			printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
+			       "queue awake!\n", dev->name);
+		}
+		return NETDEV_TX_BUSY;
+	}
+
+	return  tg3_enqueue_buggy(skb, dev);
+}
+
 static int tg3_start_xmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
 {
 	struct tg3 *tp = netdev_priv(dev);
@@ -4283,6 +4338,7 @@ static int tg3_start_xmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
 	if (unlikely(tg3_tx_avail(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
 		if (!netif_queue_stopped(dev)) {
 			netif_stop_queue(dev);
+			dev->xmit_win = 1;
 
 			/* This is a hard error, log it. */
 			printk(KERN_ERR PFX "%s: BUG! Tx Ring full when "
@@ -11099,15 +11155,19 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
 	else
 		tp->tg3_flags &= ~TG3_FLAG_POLL_SERDES;
 
+	tp->dev->hard_end_xmit = tg3_kick_DMA;
 	/* All chips before 5787 can get confused if TX buffers
 	 * straddle the 4GB address boundary in some cases.
 	 */
 	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5755 ||
 	    GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5787 ||
-	    GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5906)
-		tp->dev->hard_start_xmit = tg3_start_xmit;
-	else
-		tp->dev->hard_start_xmit = tg3_start_xmit_dma_bug;
+	    GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5906) {
+		tp->dev->hard_start_xmit = tg3_start_bxmit;
+		tp->dev->hard_prep_xmit = tg3_prep_frame;
+	} else {
+		tp->dev->hard_start_xmit = tg3_start_bxmit_dma_bug;
+		tp->dev->hard_prep_xmit = tg3_prep_bug_frame;
+	}
 
 	tp->rx_offset = 2;
 	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701 &&
@@ -11955,6 +12015,8 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 	dev->watchdog_timeo = TG3_TX_TIMEOUT;
 	dev->change_mtu = tg3_change_mtu;
 	dev->irq = pdev->irq;
+	dev->features |= NETIF_F_BTX;
+	dev->xmit_win = tp->tx_pending >> 2;
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	dev->poll_controller = tg3_poll_controller;
 #endif

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related

* Re: [PATCH][E1000E] some cleanups
From: Jeff Garzik @ 2007-09-30 19:23 UTC (permalink / raw)
  To: hadi; +Cc: Kok, Auke, netdev
In-Reply-To: <1191178881.6165.40.camel@localhost>

jamal wrote:
> On Sun, 2007-30-09 at 11:16 -0700, Kok, Auke wrote:
> 
>> no, all the hardware that is commented should work just fine. I tested this driver
>> on 82571, 82573 and ich8/ich9 - extensively.
> 
> Something else is wrong then. Can you just uncomment the 82571EB bits in
> Dave's net-2.6.24 and just send a ping? If it works, let me know what
> you did so i can test next time i get a chance.
> 
>> the reason that we disable them is that we're going to migrate devices over in
>> batches. At introduction we'll support ich9, afterwards we'll drop in the IDs of
>> the other groups of silicon.
> 
> Turn them on if you want people to start using that driver.

Gotta wait a bit, otherwise we have confusion and a bit of breakage from 
two drivers with the same PCI IDs.

	Jeff



^ permalink raw reply

* Re: [PATCH][E1000E] some cleanups
From: jamal @ 2007-09-30 19:31 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Kok, Auke, netdev
In-Reply-To: <46FFF7AD.7090309@garzik.org>

On Sun, 2007-30-09 at 15:23 -0400, Jeff Garzik wrote:

> Gotta wait a bit, otherwise we have confusion and a bit of breakage from 
> two drivers with the same PCI IDs.

ah, ok ;-> 
When i was testing i compiled out e1000. I am willing to totaly migrate
to e1000e, since major machine i have access to has PCIE. Auke, just let
me know what you need to do other than uncommenting the table entries
and i will leave you alone ;->

cheers,
jamal



^ permalink raw reply

* [PATCH] IrDA: Oops fix for ksdazzle
From: Samuel Ortiz @ 2007-09-30 23:21 UTC (permalink / raw)
  To: David S. Miller; +Cc: irda-users, netdev, Alex Villacís Lasso

Hi Dave,

This is the last remaining patch for IrDA, against net-2.6.24.

It fixes a kernel oops triggered by the ksdazzle SIR driver.
We need more space for input frames, and 2048 should be plenty of it.

Signed-off-by: Alex Villacís Lasso <a_villacis@palosanto.com>
Signed-off-by: Samuel Ortiz <samuel@sortiz.org>

---
 drivers/net/irda/ksdazzle-sir.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Index: net-2.6.24-quilt/drivers/net/irda/ksdazzle-sir.c
===================================================================
--- net-2.6.24-quilt.orig/drivers/net/irda/ksdazzle-sir.c	2007-10-01 01:53:56.000000000 +0300
+++ net-2.6.24-quilt/drivers/net/irda/ksdazzle-sir.c	2007-10-01 01:53:58.000000000 +0300
@@ -1,7 +1,7 @@
 /*****************************************************************************
 *
 * Filename:      ksdazzle.c
-* Version:       0.1.1
+* Version:       0.1.2
 * Description:   Irda KingSun Dazzle USB Dongle
 * Status:        Experimental
 * Author:        Alex Villacís Lasso <a_villacis@palosanto.com>
@@ -113,6 +113,7 @@
 #define KINGSUN_REQ_SEND 0x09
 
 #define KINGSUN_SND_FIFO_SIZE    2048	/* Max packet we can send */
+#define KINGSUN_RCV_MAX 2048	/* Max transfer we can receive */
 
 struct ksdazzle_speedparams {
 	__le32 baudrate;	/* baud rate, little endian */
@@ -150,7 +151,7 @@
 	__u8 tx_payload[8];
 
 	struct urb *rx_urb;
-	__u8 rx_payload[8];
+	__u8 *rx_buf;
 	iobuff_t rx_unwrap_buff;
 
 	struct usb_ctrlrequest *speed_setuprequest;
@@ -440,7 +441,8 @@
 	/* Start reception. */
 	usb_fill_int_urb(kingsun->rx_urb, kingsun->usbdev,
 			 usb_rcvintpipe(kingsun->usbdev, kingsun->ep_in),
-			 kingsun->rx_payload, 8, ksdazzle_rcv_irq, kingsun, 1);
+			 kingsun->rx_buf, KINGSUN_RCV_MAX, ksdazzle_rcv_irq,
+			 kingsun, 1);
 	kingsun->rx_urb->status = 0;
 	err = usb_submit_urb(kingsun->rx_urb, GFP_KERNEL);
 	if (err) {
@@ -641,6 +643,7 @@
 	kingsun->tx_buf_clear_sent = 0;
 
 	kingsun->rx_urb = NULL;
+	kingsun->rx_buf = NULL;
 	kingsun->rx_unwrap_buff.in_frame = FALSE;
 	kingsun->rx_unwrap_buff.state = OUTSIDE_FRAME;
 	kingsun->rx_unwrap_buff.skb = NULL;
@@ -651,6 +654,11 @@
 	kingsun->speed_urb = NULL;
 	kingsun->speedparams.baudrate = 0;
 
+	/* Allocate input buffer */
+	kingsun->rx_buf = kmalloc(KINGSUN_RCV_MAX, GFP_KERNEL);
+	if (!kingsun->rx_buf)
+		goto free_mem;
+
 	/* Allocate output buffer */
 	kingsun->tx_buf_clear = kmalloc(KINGSUN_SND_FIFO_SIZE, GFP_KERNEL);
 	if (!kingsun->tx_buf_clear)
@@ -714,6 +722,7 @@
       free_mem:
 	kfree(kingsun->speed_setuprequest);
 	kfree(kingsun->tx_buf_clear);
+	kfree(kingsun->rx_buf);
 	free_netdev(net);
       err_out1:
 	return ret;
@@ -746,6 +755,7 @@
 
 	kfree(kingsun->speed_setuprequest);
 	kfree(kingsun->tx_buf_clear);
+	kfree(kingsun->rx_buf);
 	free_netdev(kingsun->netdev);
 
 	usb_set_intfdata(intf, NULL);


^ permalink raw reply

* Re: [PATCH 2/3 Rev4] Initilize and populate age field
From: David Miller @ 2007-10-01  0:41 UTC (permalink / raw)
  To: varunc; +Cc: netdev, kaber, socketcan, shemminger, krkumar2, tgraf, varuncha
In-Reply-To: <20070920205751.dfaa34d8.varunc@linux.vnet.ibm.com>

From: Varun Chandramohan <varunc@linux.vnet.ibm.com>
Date: Thu, 20 Sep 2007 20:57:51 +0530

> @@ -420,6 +421,7 @@ static int fn_hash_insert(struct fib_tab
>  	else
>  		fa = fib_find_alias(&f->fn_alias, tos, fi->fib_priority);
>  
> +	do_gettimeofday(&tv);
>  	/* Now fa, if non-NULL, points to the first fib alias
>  	 * with the same keys [prefix,tos,priority], if such key already
>  	 * exists or to the node before which we will insert new one.

gettimeofday() is expensive, we don't even use it to timestamp every
incoming packet and we therefore should not do it every route cache
entry we create in order to handle DoS situations efficiently

I honestly don't like these patches.  I literally cringe every time
you post a new revision.  It's either going to add new costs to route
management or be so inaccurate as to be useless.

I question it's usefulness even if implemented efficiently and
accurately.  I really don't see people lining up asking for a route
aging metric.

We could report zero and be compliant with the RFC, we don't age our
route entries like the model of the SNMP vars seems to suggest, so
it's honestly accurate.  And this would mean no kernel changes, as the
userland program could just report zero in the absense of a kernel
provided value.

^ permalink raw reply

* Re: SFQ qdisc crashes with limit of 2 packets
From: David Miller @ 2007-10-01  0:51 UTC (permalink / raw)
  To: kuznet; +Cc: kaber, cebbert, netdev
In-Reply-To: <20070921155522.GA6755@ms2.inr.ac.ru>

From: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Date: Fri, 21 Sep 2007 19:55:22 +0400

> Hello!
> 
> Remove artificial limitation for sfq queue limit.
> 
> This is followup to Patrick's patch. A little optimization to enqueue
> routine allows to remove artificial limitation on queue length.
> 
> Plus, testing showed that hash function used by SFQ is too bad or even worse.
> It does not even sweep the whole range of hash values.
> Switched to Jenkins' hash.
> 
> 
> Signed-off-by: Alexey Kuznetsov <kaber@ms2.inr.ac.ru>

Applied, thanks Alexey.

^ permalink raw reply

* Re: [PATCH 1/2] bnx2: factor out gzip unpacker
From: David Miller @ 2007-10-01  0:57 UTC (permalink / raw)
  To: mchan; +Cc: vda.linux, jeff, linux-kernel, netdev
In-Reply-To: <1190429237.4957.6.camel@dell>

From: "Michael Chan" <mchan@broadcom.com>
Date: Fri, 21 Sep 2007 19:47:17 -0700

> On Fri, 2007-09-21 at 10:49 -0700, David Miller wrote:
> > From: Denys Vlasenko <vda.linux@googlemail.com>
> > Date: Fri, 21 Sep 2007 18:03:55 +0100
> > 
> > > Do patches look ok to you?
> > 
> > I'm travelling so I haven't looked closely yet :-)
> > 
> > Michael can take a look and I'll try to do so as well
> > tonight.
> > 
> 
> I've already reviewed the earlier versions of the patch and have made
> some suggestions.  This latest one looks ok to me and tested ok.
> 
> I'll follow up later with another patch to remove all the zeros in other
> firmware sections, and to remove the gzip headers completely.
> 
> Acked-by: Michael Chan <mchan@broadcom.com>

I've added these patches to net-2.6.24, thanks.

^ permalink raw reply

* Re: [PATCH][E1000E] some cleanups
From: Kok, Auke @ 2007-10-01  1:59 UTC (permalink / raw)
  To: hadi; +Cc: Jeff Garzik, Kok, Auke, netdev
In-Reply-To: <1191180669.6165.50.camel@localhost>

jamal wrote:
> On Sun, 2007-30-09 at 15:23 -0400, Jeff Garzik wrote:
> 
>> Gotta wait a bit, otherwise we have confusion and a bit of breakage from 
>> two drivers with the same PCI IDs.
> 
> ah, ok ;-> 
> When i was testing i compiled out e1000. I am willing to totaly migrate
> to e1000e, since major machine i have access to has PCIE. Auke, just let
> me know what you need to do other than uncommenting the table entries
> and i will leave you alone ;->

the IDs are the only thing needed to enable all pci-e e1000 hardware.

by all means we need to have guys like you and Jeff test the commented IDs! I've
been doing this myself and the e1000e driver goes to our labs for a period of
testing from next week. Unfortunately they don't know how to break it that good as
some of you guys ;)

I'll personally try to get an 82571EB tested on monday.

Auke

^ permalink raw reply

* Re: [IPV6] Fix ICMPv6 redirect handling with target multicast address
From: Brian Haley @ 2007-10-01  3:27 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki / ????; +Cc: dlstevens, davem, netdev
In-Reply-To: <20070929.100448.41933886.yoshfuji@linux-ipv6.org>

Hi Yoshifuji,

YOSHIFUJI Hideaki / ???? wrote:
> Dave, Brian,
> 
> Let me double check this patch.
> 
> Regards,
> 
> --yoshfuji
> 
> In article <OF5FC97D70.5FD0A80A-ON88257365.00025E58-88257365.00048D1E@us.ibm.com> (at Fri, 28 Sep 2007 17:50:38 -0700), David Stevens <dlstevens@us.ibm.com> says:
> 
>> Brian,
>>         A multicast address should never be the target of a neighbor
>> discovery request; the sender should use the mapping function for all
>> multicasts. So, I'm not sure that your example can ever happen, and it
>> certainly is ok to send ICMPv6 errors to multicast addresses in general.
>> But I don't see that it hurts anything. either (since it should never 
>> happen :-)),

TAHI generates a lot of packets that shouldn't happen :) see below for 
the problem.  The patch in ndisc_send_redirect() is probably 
unnecessary, but since the code was identical I figured it wouldn't hurt.

>> so I don't particularly object, either.
>>         I think it'd also be better if you add the check to be:
>>
>>         if (ipv6_addr_type(target) & 
>> (IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST))

I did have it:

if (ipv6_addr_type(target) &
     (IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST) ==
     (IPV6_ADDR_LINKLOCAL|IPV6_ADDR_UNICAST))

but changed it since I had proposed ipv6_addr_linklocal() in a previous 
patch because there are other possible users of that.

This is the actual packet trace that was sent to me, edited to remove 
non-relevant data, but I've actually been unable to reproduce Packet 12 
below (note the Destination MAC).  My knee-jerk reaction was the 
proposed patch.  It could just be timing-related, I'll do more testing 
Monday.  Strangely, the TAHI test passes anyway, but it shouldn't.

Packet 10 : Icmpv6 : Redirect
Ethernet
Destination MAC.........................00:1A:4B:EB:FC:E6
Source MAC..............................00:00:10:10:10:60
Ipv6
Version.................................6
Next Header.............................58
Hop Limit...............................255
Source IP.....................FE80:0000:0000:0000:0200:10FF:FE10:1060
Destination IP................3000:0000:0000:0000:021A:4BFF:FEEB:FCE6
Icmpv6
Type....................................0x89
Code....................................0x00
Reserved................................0x00000000
Target Address................FF02:0000:0000:0000:0000:0000:0000:0001
Destination Address...........3001:0000:0000:0000:0200:10FF:FE10:1180
Options
LLAddressOption
   Option Type.............................2
   Option Length...........................1
   Link-Layer Address......................0x000010101061

Packet 11 : Icmpv6 : Echo Request
Ethernet
Destination MAC.........................00:1A:4B:EB:FC:E6
Source MAC..............................00:00:10:10:10:60
Ipv6
Version.................................6
Next Header.............................58
Hop Limit...............................255
Source IP.....................3001:0000:0000:0000:0200:10FF:FE10:1180
Destination IP................3000:0000:0000:0000:021A:4BFF:FEEB:FCE6
Icmpv6
Type....................................0x80
Code....................................0x00
Identifier..............................0x0000
Sequence Number.........................0x0000
Data....................................0x00

Packet 12 : Icmpv6 : Echo Reply
Ethernet
Destination MAC.........................33:33:00:00:00:01
Source MAC..............................00:1A:4B:EB:FC:E6
Ipv6
Version.................................6
Next Header.............................58
Hop Limit...............................64
Source IP.....................3000:0000:0000:0000:021A:4BFF:FEEB:FCE6
Destination IP................3001:0000:0000:0000:0200:10FF:FE10:1180
Icmpv6
Type....................................0x81
Code....................................0x00
Identifier..............................0x0000
Sequence Number.........................0x0000
Data....................................0x00

This should have been sent to 00:00:10:10:10:60

-Brian

^ permalink raw reply

* Removing DAD in IPv6
From: Xia Yang @ 2007-10-01  3:53 UTC (permalink / raw)
  To: netdev

Hi all,

I am new to this list, so please forgive me if I say anything
inappropriate.

I am working on a mobility testbed based on IPv6. In order to improve
the performance, I would like to remove the DAD process during the
auto-configuration process of an IPv6 address for my testbed. 

During my experiment, I observed that when an address is configured
based on a router advertisement, the outgoing packets can be sent out
immediately. But the packets destined to local node can only be received
after some time (about 1 sec). I found the lost packets are discarded in
the kernel and never make it to the transport layer. I suspect it is
caused by the DAD process.

I would like to ask for help on how to remove or disable the DAD process
properly, as long as the node can send, receive and forward packets
immediately after a new IPv6 address is generated. Any pointer is
appreciated. Thanks a lot in advance!

Best Regards,

Xia Yang


^ permalink raw reply

* Re: [PATCH 2/3][NET_BATCH] net core use batching
From: Bill Fink @ 2007-10-01  4:11 UTC (permalink / raw)
  To: hadi
  Cc: David Miller, 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
In-Reply-To: <1191178346.6165.29.camel@localhost>

On Sun, 30 Sep 2007, jamal wrote:

> This patch adds the usage of batching within the core.
> 
> cheers,
> jamal



> [sep30-p2of3  text/plain (6.8KB)]
> [NET_BATCH] net core use batching
> 
> This patch adds the usage of batching within the core.
> The same test methodology used in introducing txlock is used, with
> the following results on different kernels:
> 
>         +------------+--------------+-------------+------------+--------+
>         |       64B  |  128B        | 256B        | 512B       |1024B   |
>         +------------+--------------+-------------+------------+--------+
> Original| 467482     | 463061       | 388267      | 216308     | 114704 |
>         |            |              |             |            |        |
> txlock  | 468922     | 464060       | 388298      | 216316     | 114709 |
>         |            |              |             |            |        |
> tg3nobtx| 468012     | 464079       | 388293      | 216314     | 114704 |
>         |            |              |             |            |        |
> tg3btxdr| 480794     | 475102       | 388298      | 216316     | 114705 |
>         |            |              |             |            |        |
> tg3btxco| 481059     | 475423       | 388285      | 216308     | 114706 |
>         +------------+--------------+-------------+------------+--------+
> 
> The first two colums "Original" and "txlock" were introduced in an earlier
> patch and demonstrate a slight increase in performance with txlock.
> "tg3nobtx" shows the tg3 driver with no changes to support batching.
> The purpose of this test is to demonstrate the effect of introducing
> the core changes to a driver that doesnt support them.
> Although this patch brings down perfomance slightly compared to txlock
> for such netdevices, it is still better compared to just the original
> kernel.
> "tg3btxdr" demonstrates the effect of using ->hard_batch_xmit() with tg3
> driver. "tg3btxco" demonstrates the effect of letting the core do all the
> work. As can be seen the last two are not very different in performance.
> The difference is ->hard_batch_xmit() introduces a new method which
> is intrusive.

Have you done performance comparisons for the case of using 9000-byte
jumbo frames?

						-Bill

^ permalink raw reply

* Re: 2.6.21 -> 2.6.22 & 2.6.23-rc8 performance regression
From: Eric Dumazet @ 2007-10-01  5:59 UTC (permalink / raw)
  To: Denys; +Cc: linux-kernel, David S. Miller, Linux Netdev List
In-Reply-To: <20070930223503.M8966@nuclearcat.com>

Denys a écrit :
> Hi 
> 
> I got
> 
> pi linux-git # git bisect bad
> Bisecting: 0 revisions left to test after this
> [f85958151900f9d30fa5ff941b0ce71eaa45a7de] [NET]: random functions can use 
> nsec resolution instead of usec
> 
> I will make sure and will try to reverse this patch on 2.6.22
> 
> But it seems "that's it".

Well... thats interesting...

No problem here on bigger servers, so I CC David Miller and netdev on this one.

AFAIK do_gettimeofday() and ktime_get_real() should use the same underlying 
hardware functions on PC and no performance problem should happen here.

(relevant part of this patch :

@ -1521,7 +1515,6 @@ __u32 secure_ip_id(__be32 daddr)
  __u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
                                  __be16 sport, __be16 dport)
  {
-       struct timeval tv;
         __u32 seq;
         __u32 hash[4];
         struct keydata *keyptr = get_keyptr();
@@ -1543,12 +1536,11 @@ __u32 secure_tcp_sequence_number(__be32 saddr, __be32 
daddr,
          *      As close as possible to RFC 793, which
          *      suggests using a 250 kHz clock.
          *      Further reading shows this assumes 2 Mb/s networks.
-        *      For 10 Mb/s Ethernet, a 1 MHz clock is appropriate.
+        *      For 10 Gb/s Ethernet, a 1 GHz clock is appropriate.
          *      That's funny, Linux has one built in!  Use it!
          *      (Networks are faster now - should this be increased?)
          */
-       do_gettimeofday(&tv);
-       seq += tv.tv_usec + tv.tv_sec * 1000000;
+       seq += ktime_get_real().tv64;


Thank you for doing this research.

> 
> 
> On Sun, 30 Sep 2007 14:25:37 +1000, Nick Piggin wrote
>> Hi Denys, thanks for reporting (btw. please reply-to-all when 
>> replying on lkml).
>>
>> You say that SLAB is better than SLUB on an otherwise identical 
>> kernel, but I didn't see if you quantified the actual numbers? It 
>> sounds like there is still a regression with SLAB?
>>
>> On Monday 01 October 2007 03:48, Eric Dumazet wrote:
>>> Denys a  :
>>>> I've moved recently one of my proxies(squid and some compressing
>>>> application) from 2.6.21 to 2.6.22, and notice huge performance drop. I
>>>> think this is important, cause it can cause serious regression on some
>>>> other workloads like busy web-servers and etc.
>>>>
>>>> After some analysis of different options i can bring more exact numbers:
>>>>
>>>> 2.6.21 able to process 500-550 requests/second and 15-20 Mbit/s of
>>>> traffic, and working great without any slowdown or instability.
>>>>
>>>> 2.6.22 able to process only 250-300 requests and 8-10 Mbit/s of traffic,
>>>> ssh and console is "freezing" (there is delay even for typing
>>>> characters).
>>>>
>>>> Both proxies is on identical hardware(Sun Fire X4100),
>>>> configuration(small system, LFS-like, on USB flash), different only
>>>> kernel.
>>>>
>>>> I tried to disable/enable various options and optimisations - it doesn't
>>>> change anything, till i reach SLUB/SLAB option.
>>>>
>>>> I've loaded proxy configuration to gentoo PC with 2.6.22 (then upgraded
>>>> it to 2.6.23-rc8), and having same effect.
>>>> Additionally, when load reaching maximum i can notice whole system
>>>> slowdown, for example ssh and scp takes much more time to run, even i do
>>>> nice -n -5 for them.
>>>>
>>>> But even choosing 2.6.23-rc8+SLAB i noticed same "freezing" of ssh (and
>>>> sure it slowdown other kind of network performance), but much less
>>>> comparing with SLUB. On top i am seeing ksoftirqd taking almost 100%
>>>> (sometimes ksoftirqd/0, sometimes ksoftirqd/1).
>>>>
>>>> I tried also different tricks with scheduler (/proc/sys/kernel/sched*),
>>>> but it's also didn't help.
>>>>
>>>> When it freezes it looks like:
>>>>   PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  COMMAND
>>>>     7 root      15  -5     0    0    0 R   64  0.0   2:47.48 ksoftirqd/1
>>>>  5819 root      20   0  134m 130m  596 R   57  3.3   4:36.78 globax
>>>>  5911 squid     20   0 1138m 1.1g 2124 R   26 28.9   2:24.87 squid
>>>>    10 root      15  -5     0    0    0 S    1  0.0   0:01.86 events/1
>>>>  6130 root      20   0  3960 2416 1592 S    0  0.1   0:08.02 oprofiled
>>>>
>>>>
>>>> Oprofile results:
>>>>
>>>>
>>>> Thats oprofile with 2.6.23-rc8 - SLUB
>>>>
>>>> 73918    21.5521  check_bytes
>>>> 38361    11.1848  acpi_pm_read
>>>> 14077     4.1044  init_object
>>>> 13632     3.9747  ip_send_reply
>>>> 8486      2.4742  __slab_alloc
>>>> 7199      2.0990  nf_iterate
>>>> 6718      1.9588  page_address
>>>> 6716      1.9582  tcp_v4_rcv
>>>> 6425      1.8733  __slab_free
>>>> 5604      1.6339  on_freelist
>>>>
>>>>
>>>> Thats oprofile with 2.6.23-rc8 - SLAB
>>>>
>>>> CPU: AMD64 processors, speed 2592.64 MHz (estimated)
>>>> Counted CPU_CLK_UNHALTED events (Cycles outside of halt state) with a
>>>> unit mask of 0x00 (No unit mask) count 100000
>>>> samples  %        symbol name
>>>> 138991   14.0627  acpi_pm_read
>>>> 52401     5.3018  tcp_v4_rcv
>>>> 48466     4.9037  nf_iterate
>>>> 38043     3.8491  __slab_alloc
>>>> 34155     3.4557  ip_send_reply
>>>> 20963     2.1210  ip_rcv
>>>> 19475     1.9704  csum_partial
>>>> 19084     1.9309  kfree
>>>> 17434     1.7639  ip_output
>>>> 17278     1.7481  netif_receive_skb
>>>> 15248     1.5428  nf_hook_slow
>>>>
>>>> My .config is at http://www.nuclearcat.com/.config (there is SPARSEMEM
>>>> enabled, it doesn't make any noticeable difference)
>>>>
>>>> Please CC me on reply, i am not in list.
>>> Could you try with SLUB but disabling CONFIG_SLUB_DEBUG ?
> 
> 
> --
> Denys Fedoryshchenko
> Technical Manager
> Virtual ISP S.A.L.
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox