Re: [PATCH] Re: rtl8169 problem and 2.4.23

Re: [PATCH] Re: rtl8169 problem and 2.4.23

[Francois Romieu]

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

Jeff,

  can you push the attached patch directly to both 2.6.x and 2.4.x ?
It will exhibit an offset of 3 lines against 2.4.x but it works the same
on 2.4.x and on 2.6.x. The fix already exists in -mm/-netdev serie.

Daniel Egger <degger@fhm.edu> confirmed I did not manage to add a giant
typo in a 4 lines patch. When hit, this bug is more or less a killer.

Daniel, I have no clear idea for the performance issues. Actually I am more
concerned with the stability side of this driver, especially in the new,
shamelessly hacked, branch of the driver. I'll probably regenerate a set
of patches and spam^W reach the testers to have a new data point.

--
Ueimor

[-- Attachment #2: r8169-tx-desc-overflow.patch --]
[-- Type: text/plain, Size: 1210 bytes --]

Assume tp->dirty_tx = NUM_TX_DESC/2, tp->cur_tx = NUM_TX_DESC - 1,
watch "entry" go beyond NUM_TX_DESC. This bug was copied from the 
(2.6.x only) sis190 driver where it is now fixed.
Stats are fixed as an extra bonus.


diff -Nrup drivers/net/r8169.c.orig drivers/net/r8169.c
--- drivers/net/r8169.c.orig	Thu Dec 18 03:58:50 2003
+++ drivers/net/r8169.c	Sat Feb 21 14:11:31 2004
@@ -871,7 +871,6 @@ rtl8169_tx_interrupt(struct net_device *
 		     void *ioaddr)
 {
 	unsigned long dirty_tx, tx_left = 0;
-	int entry = tp->cur_tx % NUM_TX_DESC;
 
 	assert(dev != NULL);
 	assert(tp != NULL);
@@ -881,14 +880,18 @@ rtl8169_tx_interrupt(struct net_device *
 	tx_left = tp->cur_tx - dirty_tx;
 
 	while (tx_left > 0) {
+		int entry = dirty_tx % NUM_TX_DESC;
+
 		if ((tp->TxDescArray[entry].status & OWNbit) == 0) {
-			dev_kfree_skb_irq(tp->
-					  Tx_skbuff[dirty_tx % NUM_TX_DESC]);
-			tp->Tx_skbuff[dirty_tx % NUM_TX_DESC] = NULL;
+			struct sk_buff *skb = tp->Tx_skbuff[entry];
+
+			tp->stats.tx_bytes += skb->len >= ETH_ZLEN ?
+					      skb->len : ETH_ZLEN;
 			tp->stats.tx_packets++;
+			dev_kfree_skb_irq(skb);
+			tp->Tx_skbuff[entry] = NULL;
 			dirty_tx++;
 			tx_left--;
-			entry++;
 		}
 	}
 

Re: [PATCH] Re: rtl8169 problem and 2.4.23

[Daniel Egger]

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

On Jan 25, 2004, at 11:07 pm, Francois Romieu wrote:

> Try the patch above. If it compiles, it should fix the stats and you 
> get a
> bugfix as an extra.

Sorry for the delay. I've not had the chance to recompile the kernel
for my fileserver and restart it, yet. But in the meantime I've received
another card with RTL8169S and tried it in a different machine.

The current driver in 2.4.24 and 2.6.3 will lock up the box in almost
no time when under load (not a real kernel lockup but a soft one because
the machine is running over NFS and the driver seems to lose packets
and then rejects to transfer more). Your patch fixes this problem and 
the
counter issue for both 2.4.24 and 2.6.3, so it probably should go in 
ASAP.

However there's another thing bugging me: abysmal performance. In both
a switched and a direct environment the best I could get using netio
was 10MB/s send and 21MB/s receive, using NFS I get just under 10MB/s,
this is slower than with a 8139 el cheapo 100Mbit card.

The netio results are illogical anyway because the other side is the
fileserver, which has the same card, so if one end receives 21MB/s the
other end has to send equally as fast, no? :) The CPU utilization was
almost zero on server and client (both Athlon XP).

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

[PATCH] Re: rtl8169 problem and 2.4.23

[Francois Romieu]

Daniel Egger <degger@fhm.edu> :
[r8169 stats broken]
> Furthermore the performance is really scary slow: I'm not even getting 
> 100Base-T speeds from an Athlon XP to my G4 PowerBook under MacOS X over
> a PtP connection.

Try the patch above. If it compiles, it should fix the stats and you get a
bugfix as an extra.

Please Cc: netdev@oss.sgi.com on followup.

--- linux-2.4.23.orig/drivers/net/r8169.c	Sun Jan 25 21:00:51 2004
+++ linux-2.4.23/drivers/net/r8169.c	Sun Jan 25 22:58:17 2004
@@ -874,7 +874,6 @@
 		     void *ioaddr)
 {
 	unsigned long dirty_tx, tx_left = 0;
-	int entry = tp->cur_tx % NUM_TX_DESC;
 
 	assert(dev != NULL);
 	assert(tp != NULL);
@@ -884,14 +883,18 @@
 	tx_left = tp->cur_tx - dirty_tx;
 
 	while (tx_left > 0) {
+		int entry = dirty_tx % NUM_TX_DESC;
+
 		if ((tp->TxDescArray[entry].status & OWNbit) == 0) {
-			dev_kfree_skb_irq(tp->
-					  Tx_skbuff[dirty_tx % NUM_TX_DESC]);
-			tp->Tx_skbuff[dirty_tx % NUM_TX_DESC] = NULL;
+			struct sk_buff *skb = tp->Tx_skbuff[entry];
+
+			tp->stats.tx_bytes += skb->len >= ETH_ZLEN ?
+					      skb->len : ETH_ZLEN;
 			tp->stats.tx_packets++;
+			dev_kfree_skb_irq(skb);
+			tp->Tx_skbuff[entry] = NULL;
 			dirty_tx++;
 			tx_left--;
-			entry++;
 		}
 	}