[PATCH] 8139cp dev->tx_timeout

[PATCH] 8139cp dev->tx_timeout

[Mika.Lansirinne]

Hello All,

We have been experimenting a couple of interface hangs with the 8139cp
driver. It appears that the tx buffer stops transmitting and never starts
up again in some yet unknown conditions. To be able to circumvent this we
implemented the missing dev->tx_timeout function which should reset the
interface and allow it work again.

The problem reoccurs quite seldom and we have yet to be able to confirm
that the attached patch helps the situation. We though that we should
submit the patch anyway for reviewing.

The patch is made against 2.6.21.5 kernel.

(please cc me in your answer)


diff -ru linux-2.6.21.5/drivers/net/8139cp.c linux-2.6.21.5_8139cp-tx_timeout/drivers/net/8139cp.c
--- linux-2.6.21.5/drivers/net/8139cp.c   2007-06-11 21:37:06.000000000 +0300
+++ linux-2.6.21.5_8139cp-tx_timeout/drivers/net/8139cp.c   2007-06-26 15:48:23.000000000 +0300
@@ -26,7 +26,6 @@

      TODO:
      * Test Tx checksumming thoroughly
-     * Implement dev->tx_timeout

      Low priority TODO:
      * Complete reset on PciErr
@@ -1233,6 +1232,30 @@
      return 0;
 }

+static void cp_tx_timeout(struct net_device *dev)
+{
+     struct cp_private *cp = netdev_priv(dev);
+     int rc;
+     unsigned long flags;
+
+        printk (KERN_WARNING "%s: Transmit timeout, status %2x %4x %4x %4x\n",
+                dev->name, cpr8(Cmd), cpr16(CpCmd),
+                cpr16(IntrStatus), cpr16(IntrMask));
+
+     spin_lock_irqsave(&cp->lock, flags);
+
+     cp_stop_hw(cp);
+     cp_clean_rings(cp);
+     rc = cp_init_rings(cp);
+     cp_start_hw(cp);
+
+     netif_wake_queue(dev);
+
+     spin_unlock_irqrestore(&cp->lock, flags);
+
+     return;
+}
+
 #ifdef BROKEN
 static int cp_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -1938,10 +1961,8 @@
      dev->change_mtu = cp_change_mtu;
 #endif
      dev->ethtool_ops = &cp_ethtool_ops;
-#if 0
      dev->tx_timeout = cp_tx_timeout;
      dev->watchdog_timeo = TX_TIMEOUT;
-#endif

 #if CP_VLAN_TAG_USED
      dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;



Best regards,

Mr. Mika Länsirinne
Sofware Specialist
Stonesoft Corporation
Itälahdenkatu 22 A, FIN-00210 Helsinki, Finland
Mobile. +358-40-749 2068
Mika.Lansirinne@stonesoft.com
http://www.stonesoft.com

Re: [PATCH] 8139cp dev->tx_timeout

[Jeff Garzik]

Mika.Lansirinne@stonesoft.com wrote:
> Hello All,
> 
> We have been experimenting a couple of interface hangs with the 8139cp
> driver. It appears that the tx buffer stops transmitting and never starts
> up again in some yet unknown conditions. To be able to circumvent this we
> implemented the missing dev->tx_timeout function which should reset the
> interface and allow it work again.
> 
> The problem reoccurs quite seldom and we have yet to be able to confirm
> that the attached patch helps the situation. We though that we should
> submit the patch anyway for reviewing.
> 
> The patch is made against 2.6.21.5 kernel.

Seems OK, but I'm definitely interested in hearing test feedback.

Also, all patches need a Signed-off-by line per 
Documentation/SubmittingPatches or
http://linux.yyz.us/patch-format.html

	Jeff

Re: [PATCH] 8139cp dev->tx_timeout

[Mika.Lansirinne]

Jeff,

Thanks for the correction. I'll resend the patch in the correct form.

I will also send the results of our test as soon as we have some.


      -Mika


> Seems OK, but I'm definitely interested in hearing test feedback.
>
> Also, all patches need a Signed-off-by line per
> Documentation/SubmittingPatches or
> http://linux.yyz.us/patch-format.html
>
>    Jeff
>
>

[PATCH 2.6.21.5] 8139cp: dev->tx_timeout

[Mika.Lansirinne]

This patch implements the missing dev->tx_timeout for 8139cp driver

Signed-off-by: Mika Lansirinne <mika.lansirinne@stonesoft.com>
---

diff -ru linux-2.6.21.5/drivers/net/8139cp.c linux-2.6.21.5_8139cp-tx_timeout/drivers/net/8139cp.c
--- linux-2.6.21.5/drivers/net/8139cp.c   2007-06-11 21:37:06.000000000 +0300
+++ linux-2.6.21.5_8139cp-tx_timeout/drivers/net/8139cp.c   2007-06-26 15:48:23.000000000 +0300
@@ -26,7 +26,6 @@

      TODO:
      * Test Tx checksumming thoroughly
-     * Implement dev->tx_timeout

      Low priority TODO:
      * Complete reset on PciErr
@@ -1233,6 +1232,30 @@
      return 0;
 }

+static void cp_tx_timeout(struct net_device *dev)
+{
+     struct cp_private *cp = netdev_priv(dev);
+     int rc;
+     unsigned long flags;
+
+        printk (KERN_WARNING "%s: Transmit timeout, status %2x %4x %4x %4x\n",
+                dev->name, cpr8(Cmd), cpr16(CpCmd),
+                cpr16(IntrStatus), cpr16(IntrMask));
+
+     spin_lock_irqsave(&cp->lock, flags);
+
+     cp_stop_hw(cp);
+     cp_clean_rings(cp);
+     rc = cp_init_rings(cp);
+     cp_start_hw(cp);
+
+     netif_wake_queue(dev);
+
+     spin_unlock_irqrestore(&cp->lock, flags);
+
+     return;
+}
+
 #ifdef BROKEN
 static int cp_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -1938,10 +1961,8 @@
      dev->change_mtu = cp_change_mtu;
 #endif
      dev->ethtool_ops = &cp_ethtool_ops;
-#if 0
      dev->tx_timeout = cp_tx_timeout;
      dev->watchdog_timeo = TX_TIMEOUT;
-#endif

 #if CP_VLAN_TAG_USED
      dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;

Re: [PATCH 2.6.21.5] 8139cp: dev->tx_timeout

[Jeff Garzik]

Mika.Lansirinne@stonesoft.com wrote:
> This patch implements the missing dev->tx_timeout for 8139cp driver
> 
> Signed-off-by: Mika Lansirinne <mika.lansirinne@stonesoft.com>

does not seem to apply to current kernel.  please resend against 
netdev-2.6.git#upstream or 2.6.22-rc7.

[PATCH 2.6.22-rc7] 8139cp: dev->tx_timeout

[Mika.Lansirinne]

(Resending the patch against 2.6.22-rc7)

This patch implements the missing dev->tx_timeout for 8139cp driver

Signed-off-by: Mika Lansirinne <mika.lansirinne@stonesoft.com>

---

diff -uprN linux-2.6.22-rc7/drivers/net/8139cp.c linux-2.6.22-rc7_8139cp-tx_timeout/drivers/net/8139cp.c
--- linux-2.6.22-rc7/drivers/net/8139cp.c 2007-07-01 22:54:24.000000000 +0300
+++ linux-2.6.22-rc7_8139cp-tx_timeout/drivers/net/8139cp.c 2007-07-02 16:00:38.000000000 +0300
@@ -26,7 +26,6 @@

      TODO:
      * Test Tx checksumming thoroughly
-     * Implement dev->tx_timeout

      Low priority TODO:
      * Complete reset on PciErr
@@ -1223,6 +1222,30 @@ static int cp_close (struct net_device *
      return 0;
 }

+static void cp_tx_timeout(struct net_device *dev)
+{
+     struct cp_private *cp = netdev_priv(dev);
+     int rc;
+     unsigned long flags;
+
+        printk (KERN_WARNING "%s: Transmit timeout, status %2x %4x %4x %4x\n",
+                dev->name, cpr8(Cmd), cpr16(CpCmd),
+                cpr16(IntrStatus), cpr16(IntrMask));
+
+     spin_lock_irqsave(&cp->lock, flags);
+
+     cp_stop_hw(cp);
+     cp_clean_rings(cp);
+     rc = cp_init_rings(cp);
+     cp_start_hw(cp);
+
+     netif_wake_queue(dev);
+
+     spin_unlock_irqrestore(&cp->lock, flags);
+
+     return;
+}
+
 #ifdef BROKEN
 static int cp_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -1928,10 +1951,8 @@ static int cp_init_one (struct pci_dev *
      dev->change_mtu = cp_change_mtu;
 #endif
      dev->ethtool_ops = &cp_ethtool_ops;
-#if 0
      dev->tx_timeout = cp_tx_timeout;
      dev->watchdog_timeo = TX_TIMEOUT;
-#endif

 #if CP_VLAN_TAG_USED
      dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;

Re: [PATCH 2.6.22-rc7] 8139cp: dev->tx_timeout

[Jeff Garzik]

Mika.Lansirinne@stonesoft.com wrote:
> (Resending the patch against 2.6.22-rc7)
> 
> This patch implements the missing dev->tx_timeout for 8139cp driver
> 
> Signed-off-by: Mika Lansirinne <mika.lansirinne@stonesoft.com>

ACK but still fails to apply

Re: [PATCH 2.6.22-rc7] 8139cp: dev->tx_timeout

[Mika.Lansirinne]

Hmm... that's strange. I just tested again and got it applied perfectly to
vanilla 2.6.22-rc7. The git version must be just that much different.


Can you test this one, this time it's made against the netdev-2.6 version.
There indeed is a slight offset in the line numbers.


By the way, we just got the problem recurred again. The timeout reset did
enable the traffic to flow again after a hang, so this patch clearly helps.

The problem "requires" very heavy network load when the tx queue is
constantly quite full.



-Mika



Jeff Garzik <jeff@garzik.org> wrote on 10.07.2007 19:42:21:

> Mika.Lansirinne@stonesoft.com wrote:
> > (Resending the patch against 2.6.22-rc7)
> >
> > This patch implements the missing dev->tx_timeout for 8139cp driver
> >
> > Signed-off-by: Mika Lansirinne <mika.lansirinne@stonesoft.com>
>
> ACK but still fails to apply
>
>

---

This patch implements the missing dev->tx_timeout for 8139cp driver

Signed-off-by: Mika Lansirinne <mika.lansirinne@stonesoft.com>

---

--- netdev-2.6/drivers/net/8139cp.c 2007-07-11 13:19:56.000000000 +0300
+++ netdev-2.6_8139cp-tx_timeout/drivers/net/8139cp.c 2007-07-11 13:26:44.000000000 +0300
@@ -26,7 +26,6 @@

      TODO:
      * Test Tx checksumming thoroughly
-     * Implement dev->tx_timeout

      Low priority TODO:
      * Complete reset on PciErr
@@ -1218,6 +1217,30 @@ static int cp_close (struct net_device *
      return 0;
 }

+static void cp_tx_timeout(struct net_device *dev)
+{
+     struct cp_private *cp = netdev_priv(dev);
+     int rc;
+     unsigned long flags;
+
+        printk (KERN_WARNING "%s: Transmit timeout, status %2x %4x %4x %4x\n",
+                dev->name, cpr8(Cmd), cpr16(CpCmd),
+                cpr16(IntrStatus), cpr16(IntrMask));
+
+     spin_lock_irqsave(&cp->lock, flags);
+
+     cp_stop_hw(cp);
+     cp_clean_rings(cp);
+     rc = cp_init_rings(cp);
+     cp_start_hw(cp);
+
+     netif_wake_queue(dev);
+
+     spin_unlock_irqrestore(&cp->lock, flags);
+
+     return;
+}
+
 #ifdef BROKEN
 static int cp_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -1923,10 +1946,8 @@ static int cp_init_one (struct pci_dev *
      dev->change_mtu = cp_change_mtu;
 #endif
      dev->ethtool_ops = &cp_ethtool_ops;
-#if 0
      dev->tx_timeout = cp_tx_timeout;
      dev->watchdog_timeo = TX_TIMEOUT;
-#endif

 #if CP_VLAN_TAG_USED
      dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;

Re: [PATCH 2.6.22-rc7] 8139cp: dev->tx_timeout

[Francois Romieu]

Mika.Lansirinne@stonesoft.com <Mika.Lansirinne@stonesoft.com> :
[...]
> Can you test this one, this time it's made against the netdev-2.6 version.
> There indeed is a slight offset in the line numbers.

It still fails.

Fixed version follow.

-- 
Ueimor

[PATCH 1/1] 8139cp: implement the missing dev->tx_timeout

[Francois Romieu]

Signed-off-by: Mika Lansirinne <mika.lansirinne@stonesoft.com>
---
 drivers/net/8139cp.c |   27 ++++++++++++++++++++++++---
 1 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index 807e699..e970e64 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -26,7 +26,6 @@
 
 	TODO:
 	* Test Tx checksumming thoroughly
-	* Implement dev->tx_timeout
 
 	Low priority TODO:
 	* Complete reset on PciErr
@@ -1218,6 +1217,30 @@ static int cp_close (struct net_device *dev)
 	return 0;
 }
 
+static void cp_tx_timeout(struct net_device *dev)
+{
+	struct cp_private *cp = netdev_priv(dev);
+	unsigned long flags;
+	int rc;
+
+	printk(KERN_WARNING "%s: Transmit timeout, status %2x %4x %4x %4x\n",
+	       dev->name, cpr8(Cmd), cpr16(CpCmd),
+	       cpr16(IntrStatus), cpr16(IntrMask));
+
+	spin_lock_irqsave(&cp->lock, flags);
+
+	cp_stop_hw(cp);
+	cp_clean_rings(cp);
+	rc = cp_init_rings(cp);
+	cp_start_hw(cp);
+
+	netif_wake_queue(dev);
+
+	spin_unlock_irqrestore(&cp->lock, flags);
+
+	return;
+}
+
 #ifdef BROKEN
 static int cp_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -1920,10 +1943,8 @@ static int cp_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 	dev->change_mtu = cp_change_mtu;
 #endif
 	dev->ethtool_ops = &cp_ethtool_ops;
-#if 0
 	dev->tx_timeout = cp_tx_timeout;
 	dev->watchdog_timeo = TX_TIMEOUT;
-#endif
 
 #if CP_VLAN_TAG_USED
 	dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
-- 
1.5.2.1

Re: [PATCH 1/1] 8139cp: implement the missing dev->tx_timeout

[Jeff Garzik]

Francois Romieu wrote:
> Signed-off-by: Mika Lansirinne <mika.lansirinne@stonesoft.com>
> ---
>  drivers/net/8139cp.c |   27 ++++++++++++++++++++++++---
>  1 files changed, 24 insertions(+), 3 deletions(-)

applied, thanks for helping out Francois

Re: [PATCH 1/1] 8139cp: implement the missing dev->tx_timeout

[Arnaldo Carvalho de Melo]

On 7/13/07, Francois Romieu <romieu@fr.zoreil.com> wrote:
> Signed-off-by: Mika Lansirinne <mika.lansirinne@stonesoft.com>
> ---
>  drivers/net/8139cp.c |   27 ++++++++++++++++++++++++---
>  1 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
> index 807e699..e970e64 100644
> --- a/drivers/net/8139cp.c
> +++ b/drivers/net/8139cp.c
> @@ -26,7 +26,6 @@
>
>         TODO:
>         * Test Tx checksumming thoroughly
> -       * Implement dev->tx_timeout
>
>         Low priority TODO:
>         * Complete reset on PciErr
> @@ -1218,6 +1217,30 @@ static int cp_close (struct net_device *dev)
>         return 0;
>  }
>
> +static void cp_tx_timeout(struct net_device *dev)
> +{
> +       struct cp_private *cp = netdev_priv(dev);
> +       unsigned long flags;
> +       int rc;

What is the point of this variable...

> +
> +       printk(KERN_WARNING "%s: Transmit timeout, status %2x %4x %4x %4x\n",
> +              dev->name, cpr8(Cmd), cpr16(CpCmd),
> +              cpr16(IntrStatus), cpr16(IntrMask));
> +
> +       spin_lock_irqsave(&cp->lock, flags);
> +
> +       cp_stop_hw(cp);
> +       cp_clean_rings(cp);
> +       rc = cp_init_rings(cp);

... if it is set and ...
> +       cp_start_hw(cp);
> +
> +       netif_wake_queue(dev);
> +
> +       spin_unlock_irqrestore(&cp->lock, flags);
> +
> +       return;
> +}

... never used?

- Arnaldo

Re: [PATCH] 8139cp dev->tx_timeout

[Jarek Poplawski]

(second try! sorry)

On 27-06-2007 10:36, Jeff Garzik wrote:
> Mika.Lansirinne@stonesoft.com wrote:
>> Hello All,
>>
>> We have been experimenting a couple of interface hangs with the 8139cp
>> driver. It appears that the tx buffer stops transmitting and never starts
>> up again in some yet unknown conditions. To be able to circumvent this we
>> implemented the missing dev->tx_timeout function which should reset the
>> interface and allow it work again.
>>
>> The problem reoccurs quite seldom and we have yet to be able to confirm
>> that the attached patch helps the situation. We though that we should
>> submit the patch anyway for reviewing.
>>
>> The patch is made against 2.6.21.5 kernel.
> 
> Seems OK, but I'm definitely interested in hearing test feedback.

Hi!

I think maybe there are too many similar problems under 2.6.21
to be individual, and there could be something common to fix?

So, I "link" here a few  probably interested souls to cc:

Subject: Re: 2.6.20->2.6.21 - networking dies after random time
...
On Tue, Jun 26, 2007 at 04:24:07PM +0200, Jean-Baptiste Vignaud wrote:
> Hello, i have a very similar problem with 2.6.21 also;
> 
> 2 3com NICs and they are failling randomly.
> 
> The kernel is a basic fedora 7 kernel (2.6.21-1.3228.fc7)
> I found a bug report and added details here : https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243960
> 
> I'm not subcribed on this list, so please cc me if there is any questions.
> 
> JB
> 
> > On Tue, Jun 26, 2007 at 08:10:17AM +0200, Marcin Ślusarz wrote:
> > ...
> > > I reproduced it on minimal config:
...
> > We know your hardware should be OK - since it was fine with 2.6.20.
...

It looks like there is something common in the air...

Marcin: ne2k_pci with 8390, Jean: 3com, and now I see
similar problem with 8139cp too (plus some ideas):

http://marc.info/?l=linux-netdev&m=118293314109648&w=2

So, you probably should wait a little & look for new patches here.

Cheers,
Jarek P.