Fw: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11

Fw: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11

[Andrew Morton]

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


That sounds like a bug in appletalk or in the tap driver.  Please describe
a means by which other developers can reproduce that bug (a sequence of
shell commands would be ideal).

Thanks.



Begin forwarded message:

Date: Sun, 10 Apr 2005 11:16:25 +0200
From: Felix Palmen <fmp@palmen.homeip.net>
To: linux-kernel@vger.kernel.org
Subject: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11


Hi lkml-members,

I recently had a problem with appletalk. After starting atalkd on a TAP
interface and stopping it later, unregister_netdevice() just stated
| unregister_netdevice: waiting for tap0 to become free. Usage count = -1

So I assume there is a problem in the appletalk code, but I didn't try
reproducing that on other systems so far.

I changed my kernel to "correct" a negative refcnt to 0 and that kind
of fixes the problem. I'm not sure whether this could break anything,
but certainly waiting for a device to become free is no use when there
is a negative number of users, so I think it would be better to allow
the system to shut down cleanly in this case.

Here's m suggestion:

#v+
--- linux-2.6.11-orig/net/core/dev.c 2005-03-02 08:38:09.000000000 +0100
+++ linux-2.6.11/net/core/dev.c    2005-04-09 16:44:42.000000000 +0200
@@ -2876,7 +2876,7 @@
	unsigned long rebroadcast_time, warning_time;

	rebroadcast_time = warning_time = jiffies;
-	while (atomic_read(&dev->refcnt) != 0) {
+	while (atomic_read(&dev->refcnt) > 0) {
		if (time_after(jiffies, rebroadcast_time + 1 * HZ)) {
			rtnl_shlock();

@@ -2910,6 +2910,13 @@
			warning_time = jiffies;
		}
	}
+	if (atomic_read(&dev->refcnt) != 0) {
+		printk(KERN_ERR "unregister_netdevice: "
+			"%s has negative refcnt (%d). "
+			"This should never happen! Setting refcnt to 0.\n",
+			dev->name, atomic_read(&dev->refcnt));
+		atomic_set(&dev->refcnt, 0);
+	}
 }

 /* The sequence is:
#v-

Greets, Felix

PS: Please note I'm not subscribed to lkml and CC me in replies, thanks.

-- 
 | /"\   ASCII Ribbon   | Felix M. Palmen (Zirias)    http://zirias.ath.cx/ |
 | \ / Campaign Against | fmp@palmen.homeip.net      encrypted mail welcome |
 |  X    HTML In Mail   | PGP key: http://zirias.ath.cx/pub.txt             |
 | / \     And News     | ED9B 62D0 BE39 32F9 2488 5D0C 8177 9D80 5ECF F683 |


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 308 bytes --]

Re: Fw: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11

[Felix Palmen]

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

* Felix M. Palmen <fmp@palmen.homeip.net> [20050410 11:16]:
> So I assume there is a problem in the appletalk code, but I didn't try
> reproducing that on other systems so far.

I've now tested this issue on a vanilla 2.6.11.7 kernel. I only applied
my own patch from the previous post so that I am able to shut down the
computer cleanly. So I did the following:

- boot 2.6.11.7
- create a tap device with 'openvpn --mktun --dev tap0'
- create /etc/netatalk/atalkd.conf with a single line 'tap0'
- launch atalkd, wait.
- stop atalkd.
- destroy tap device with 'openvpn --rmtun --dev tap0'

refcnt was -256, very strange.

Would you consider my patch harmful? I intend using it for now, because
otherwise, my system won't shut down cleanly any more...

Greets, Felix

PS: Please CC replies to me.
PPS: I'm sorry for the multipost (lkml and here), at first I didn't
realize Andrew was directing this thread to linux-netdev

-- 
 | /"\   ASCII Ribbon   | Felix M. Palmen (Zirias)    http://zirias.ath.cx/ |
 | \ / Campaign Against | fmp@palmen.homeip.net      encrypted mail welcome |
 |  X    HTML In Mail   | PGP key: http://zirias.ath.cx/pub.txt             |
 | / \     And News     | ED9B 62D0 BE39 32F9 2488 5D0C 8177 9D80 5ECF F683 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 307 bytes --]

Re: Fw: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11

[Thomas Graf]

* Felix Palmen <20050410212727.GA13829@palmen.homeip.net> 2005-04-10 23:27
> I've now tested this issue on a vanilla 2.6.11.7 kernel. I only applied
> my own patch from the previous post so that I am able to shut down the
> computer cleanly. So I did the following:
> 
> - boot 2.6.11.7
> - create a tap device with 'openvpn --mktun --dev tap0'
> - create /etc/netatalk/atalkd.conf with a single line 'tap0'
> - launch atalkd, wait.
> - stop atalkd.
> - destroy tap device with 'openvpn --rmtun --dev tap0'
> 
> refcnt was -256, very strange.

Is it always 256? Do you have any appletalk routes configured?
If so, is the 256 dependand on the number of routes?

I'm asking because:
ddp.c:          for (nodect = 0; nodect < 256; nodect++) {
ddp.c:          for (nodect = 0; nodect < 256; nodect++) {

However, I have not checked if this could be invovled at all.

> Would you consider my patch harmful? I intend using it for now, because
> otherwise, my system won't shut down cleanly any more...

Your system is not fully reliable with and without the patch so
it doesn't matter at all. ;->

Re: Fw: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11

[Felix Palmen]

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

Hallo Thomas,

* Thomas Graf <tgraf@suug.ch> [20050410 23:50]:
> Is it always 256? Do you have any appletalk routes configured?
> If so, is the 256 dependand on the number of routes?

That could really be the case:

- In my test with vanilla 2.6.11.7, there was no other appletalk
  station on the net -> refcnt was -256
- On my working system, I use a single route and get refcnt = -1
- When I first tried appletalk without configuring anything but the
  interface and PearPC with Mac OS X runnning on the other side, refcnt
  was -2

I'll try to double-check that tomorrow.

Greets, Felix

-- 
 | /"\   ASCII Ribbon   | Felix M. Palmen (Zirias)    http://zirias.ath.cx/ |
 | \ / Campaign Against | fmp@palmen.homeip.net      encrypted mail welcome |
 |  X    HTML In Mail   | PGP key: http://zirias.ath.cx/pub.txt             |
 | / \     And News     | ED9B 62D0 BE39 32F9 2488 5D0C 8177 9D80 5ECF F683 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 307 bytes --]

Re: Fw: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11

[Thomas Graf]

* Felix Palmen <20050411023038.GB13829@palmen.homeip.net> 2005-04-11 04:30
> * Thomas Graf <tgraf@suug.ch> [20050410 23:50]:
> > Is it always 256? Do you have any appletalk routes configured?
> > If so, is the 256 dependand on the number of routes?
> 
> That could really be the case:
> 
> - In my test with vanilla 2.6.11.7, there was no other appletalk
>   station on the net -> refcnt was -256

How long was the interface up before you removed it again? That 256
might be due to probes still floating around expiring very slowly
because there is no other station around.

> - On my working system, I use a single route and get refcnt = -1

This one I can explain quite easly, I did not found a dev_hold()
for the dev_put() that is done upon deletion of the routes when
the interface is removed.

> - When I first tried appletalk without configuring anything but the
>   interface and PearPC with Mac OS X runnning on the other side, refcnt
>   was -2

OK, I think the problem is a few missing dev_hold() when the net_device
handle is assigned to routes, probes, etc.

Re: Fw: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11

[Herbert Xu]

Thomas Graf <tgraf@suug.ch> wrote:
> 
> OK, I think the problem is a few missing dev_hold() when the net_device
> handle is assigned to routes, probes, etc.

Indeed.  Here is a patch that adds the references that are held by
the routes.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
===== net/appletalk/ddp.c 1.58 vs edited =====
--- 1.58/net/appletalk/ddp.c	2005-03-27 09:04:35 +10:00
+++ edited/net/appletalk/ddp.c	2005-04-18 11:29:17 +10:00
@@ -573,6 +573,7 @@
 
 	/* Fill in the routing entry */
 	rt->target  = ta->sat_addr;
+	dev_hold(devhint);
 	rt->dev     = devhint;
 	rt->flags   = r->rt_flags;
 	rt->gateway = ga->sat_addr;

Re: Fw: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11

[Herbert Xu]

On Mon, Apr 18, 2005 at 12:07:49PM +1000, Herbert Xu wrote:
> 
> Indeed.  Here is a patch that adds the references that are held by
> the routes.

BTW, there are other problems in the appletalk stack that should
be fixed too.  For instance, the route entries and skb's there don't
hold a reference to the device that they point to.  So if anyone is
going to spend time on appletalk then they should have a look at them.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Fw: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11

[Arnaldo Carvalho de Melo]

On 4/18/05, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Mon, Apr 18, 2005 at 12:07:49PM +1000, Herbert Xu wrote:
> >
> > Indeed.  Here is a patch that adds the references that are held by
> > the routes.
> 
> BTW, there are other problems in the appletalk stack that should
> be fixed too.  For instance, the route entries and skb's there don't
> hold a reference to the device that they point to.  So if anyone is
> going to spend time on appletalk then they should have a look at them.

Appletalk is known to be in need of a major reworking, I haven't had time
so far to devote to it even being listed as "MAINTAINER" of such stuff,
perhaps I should send a patch dropping maintainership.

After I'm satisfied with DCCP I may well revisit Appletalk to use the skills I
got/new infrastructure I put in place while working on DCCP, time will tell.

- Arnaldo

Re: Fw: unregister_netdevice(): negative refcnt, suggest patch against 2.6.11

[Thomas Graf]

* Herbert Xu <E1DNLfx-0000yd-00@gondolin.me.apana.org.au> 2005-04-18 12:07
> Thomas Graf <tgraf@suug.ch> wrote:
> > 
> > OK, I think the problem is a few missing dev_hold() when the net_device
> > handle is assigned to routes, probes, etc.
> 
> Indeed.  Here is a patch that adds the references that are held by
> the routes.

Thanks for taking over Herbert, I simply didn't have the time lately.
The patch looks perfectly fine to me.