Re: possible NETGRAPH/NG_ETHER bug
Yevmenkin, Maksim N, CSCIO writes: > again, here is one of the millions of possible patches that works for me :) > > *** ng_ether.c.old Tue Jul 18 21:17:54 2000 > --- ng_ether.c Tue Jul 18 21:48:46 2000 > *** > *** 293,298 > --- 293,299 > bzero(priv, sizeof(*priv)); > FREE(priv, M_NETGRAPH); > node->private = NULL; > + ng_unname(node);/* remove node name */ > ng_unref(node); /* free node itself */ > } I think that is the right patch. Thanks! -Archie ___ Archie Cobbs * Whistle Communications, Inc. * http://www.whistle.com To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
RE: possible NETGRAPH/NG_ETHER bug
[...] > From: Archie Cobbs [mailto:[EMAIL PROTECTED]] > Julian Elischer writes: > > > i was working on integration of Ethernet TAP driver and NETGRAPH > > > and found strange thing. the problem is that NG_ETHER nodes do not > > > detach correctly when interface is gone. i was taking a very quick > > > look at it, and, it seems to me that we are missing one reference > > > to a node. i think it is ng_name_node/ng_unname pair. > > > > This is quite possible because until recently interfaces could never > > be removed. Therefore the act of removing a node was really > > just a case of RESETTING the node. It was not removed. > > Here's some more info that may be helpful. [...] the problem still exists :( i tried to collect some information that, i think, could be helpful. first, my system fly# uname -a FreeBSD fly.private.org 5.0-CURRENT FreeBSD 5.0-CURRENT #2: Tue Jul 18 20:21:57 EST 2000 [EMAIL PROTECTED]:/usr/src/sys/compile/FLY i386 now modules and interfaces fly# kldstat Id Refs AddressSize Name 13 0xc010 1cd99c kernel 21 0xc0974000 4000 logo_saver.ko fly# ifconfig -a lo0: flags=8049 mtu 16384 inet 127.0.0.1 netmask 0xff00 now i will load if_tap module and create virtual interface fly# kldload -v ./if_tap.ko Loaded ./if_tap.ko, id=3 fly# cat /dev/tap0 ^C fly# ifconfig -a lo0: flags=8049 mtu 16384 inet 127.0.0.1 netmask 0xff00 tap0: flags=8802 mtu 1500 ether 00:bd:dd:25:00:00 now i will load ng_ether and check NETGRAPH nodes fly# kldload -v ng_ether Loaded ng_ether, id=4 fly# ngctl list There are 2 total nodes: Name: ngctl183Type: socket ID: 0002 Num hooks: 0 Name: tap0Type: ether ID: 0001 Num hooks: 0 fly# ngctl types There are 2 total types: Type name Number of living nodes - -- socket 1 ether 1 so far so good :) now i will unload if_tap module fly# kldunload if_tap fly# ifconfig -a lo0: flags=8049 mtu 16384 inet 127.0.0.1 netmask 0xff00 fly# cat /dev/tap0 cat: /dev/tap0: Device not configured ok, both device and interface are gone, what about NETGRAPH nodes fly# ngctl list There are 1 total nodes: Name: ngctl210Type: socket ID: 0004 Num hooks: 0 fly# ngctl types There are 2 total types: Type name Number of living nodes - -- socket 1 ether 1 fly# kldunload ng_ether kldunload: can't unload file: Device busy ooops :( there is still 1 ``ether'' node :( i did put some debug printf in ng_base and ng_ether. here is an output ng_ether_detach: start node->refs = 2 --- ng_ether_detach() ng_unref: node->refs = 3 --- ng_unfer(). ng_rmnode() it will add one extra reference ng_ether_detach: before final ng_unref() node->refs = 2 --- ng_ether_detach() just before last ng_unref() ng_unref: node->refs = 2 --- ng_unref() so i think that shows that last ng_unref() was called with node->refs equal to 2, and, i think, that is not correct :( again, here is one of the millions of possible patches that works for me :) *** ng_ether.c.old Tue Jul 18 21:17:54 2000 --- ng_ether.c Tue Jul 18 21:48:46 2000 *** *** 293,298 --- 293,299 bzero(priv, sizeof(*priv)); FREE(priv, M_NETGRAPH); node->private = NULL; + ng_unname(node);/* remove node name */ ng_unref(node); /* free node itself */ } if_tap module sources can be found at http://home.earthlink.net/~evmax/tap-fbsd5b1.tar.gz sorry for long letter :) hope that helps :) thanks, emax To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: possible NETGRAPH/NG_ETHER bug
Julian Elischer writes: > > i was working on integration of Ethernet TAP driver and NETGRAPH > > and found strange thing. the problem is that NG_ETHER nodes do not > > detach correctly when interface is gone. i was taking a very quick > > look at it, and, it seems to me that we are missing one reference > > to a node. i think it is ng_name_node/ng_unname pair. > > This is quite possible because until recently interfaces could never > be removed. Therefore the act of removing a node was really > just a case of RESETTING the node. It was not removed. Here's some more info that may be helpful. First of all, until yesterday, if you detach an ethernet interface that was using netgraph you'd get a kernel panic (or somesuch) -- it was simply broken. This change will be MFC'd soon but it hasn't yet so we're talking -current only at this point. Now, it all should work as designed... where "as designed" means: 1. Ethernet nodes appear for each Ethernet interface at the first moment when the following conditions *both* become true: (a) ng_ether.ko KLD is loaded (or kernel has options NETGRAPH_ETHER) (b) The interface is attached (e.g., at boot time, or when the PCCARD or USB device is connected). 2. Ethernet nodes disappear when/if the interface is detached (e.g., you pop out your Ethernet PCCARD). 3. Telling an Ethernet node to shutdown (e.g., "ngctl kill fxp0:") simply *resets* the node, i.e., breaks all connections to other nodes. The node does NOT go away until #2 happens. 4. You cannot kldunload ng_ether.ko until all Ethernet nodes are detached (for obvious reasons, considering #1 and #2). If you are seeing other behavior that this using -current sources, please let me know, as there is a BUG. OTOH, if you think the behavior "as designed" is incorrect, let's discuss. -Archie ___ Archie Cobbs * Whistle Communications, Inc. * http://www.whistle.com To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
RE: possible NETGRAPH/NG_ETHER bug
> I object to these patches. > the idea is good but these patches are misguided.. ok :) i did not say that is an ultimate solution :) i did not even say that they are good :) the only idea behind these patches is to show that there is a _possible_ node reference problem :) that's it :) [...] > Ok, so the idea is that the actual underlying interface > is going away, and that you want the node to go away too. > The correct way is to signal to ng_ether_rmnode() that > it SHOULD remove the node. may be it will be good to have destructor for node as well as shutdown? since we know that node is doomed ng_rmnode will call destructor. shutdown will do just preparation, i.e. cut links etc. in this case it will be possibe to shutdown node without deletion. and if node should gone destructor will call shoudown and then remove node. > At present this code assumes that the ethernet interface > is permenent, and that the ng_ether node should thus also > be persistant. What you need is a way for it to distinguish > between the case where it should not remove the node, and the case > where the interface is doomed, and it should remove the node. > A flag somewhere would suffice. yes, it will work. i can flag node as doomed before calling ng_ether_detach. but anyway i need to remove extra reference in ng_name_node/ng_unname, otherwise ng_unfer won't destroy node :( there will be a lot of them. ng_ctl shows them perfectly. [...] > Archie's changes (when he applies them) > will give a clearer picture at to how this should be done. > I suggest that you hold off until his patches are added because > it will have an effect. It should be done any day now. i will :) thanks, emax To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: possible NETGRAPH/NG_ETHER bug
I object to these patches.
the idea is good but these patches are misguided..
Yevmenkin, Maksim N, CSCIO wrote:
>
> Hello All,
>
> i was working on integration of Ethernet TAP driver and NETGRAPH
> and found strange thing. the problem is that NG_ETHER nodes do not
> detach correctly when interface is gone. i was taking a very quick
> look at it, and, it seems to me that we are missing one reference
> to a node. i think it is ng_name_node/ng_unname pair.
This is quite possible because until recently interfaces could never
be removed. Therefore the act of removing a node was really
just a case of RESETTING the node. It was not removed.
>
> quick patch (works for NG_ETHER, but i did not have time to look
> at all NG_ modules) available at
>
> http://home.earthlink.net/~evmax/ng_ether-diffs.tar.gz
>
> this problem could, possibly, affect other modules.
>
> after patch i was able to:
> - load Ethernet TAP driver
> - create several virtual Ethernet interfaces
> - load NG_ETHER module (and check nodes via ngctl)
> - unload Ethernet TAP driver (all virtual Ethernet interfaces are gone)
> - check that all NG_ETHER nodes are gone (via ngctl)
> - unload NG_ETHER module
it's rathe rude to do this in such an un-nested manner, but
yes it would be good for this to work.
my comments on you patches follow:
ng_base.c Wed Jul 12 22:54:09 2000
***
*** 632,638
if (node->name) {
FREE(node->name, M_NETGRAPH);
node->name = NULL;
! ng_unref(node);
}
}
--- 632,638
if (node->name) {
FREE(node->name, M_NETGRAPH);
node->name = NULL;
! node->refs--;
}
}
YUK!
never never never decrement refs without checking
that is has not gone to 0. If it has gone to 0 the object
MUST be freed.!!! That is what ng_unref is for.
If you do not want it to be freed than you should add a reference
which represents the reference that you have to it. That way you can
guarantee that it will never go to 0. NEVER EVER EVER subvert a
reference count. (you go blind if you do that)
*** ng_ether.c.old Wed Jul 12 22:53:38 2000
--- ng_ether.c Wed Jul 12 22:55:41 2000
***
*** 287,297
--- 287,299
if (node == NULL) /* no node (why not?), ignore */
return;
/* break all links to other nodes */
+ node->flags |= NG_INVALID;
IFP2NG(ifp) = NULL; /* detach node from interface */
priv = node->private; /* free node private info */
bzero(priv, sizeof(*priv));
FREE(priv, M_NETGRAPH);
node->private = NULL;
+ ng_unname(node);/* unname node */
ng_unref(node); /* free node itself */
}
Ok, so the idea is that the actual underlying interface
is going away, and that you want the node to go away too.
The correct way is to signal to ng_ether_rmnode() that
it SHOULD remove the node.
At present this code assumes that the ethernet interface
is permenent, and that the ng_ether node should thus also
be persistant. What you need is a way for it to distinguish
between the case where it should not remove the node, and the case
where the interface is doomed, and it should remove the node.
A flag somewhere would suffice.
It seems to me from a quick look at the code that
is in the -current tree, that there is some confusion
about device node removal as applied to ethernet interfaces.
Archie's changes (when he applies them)
will give a clearer picture at to how this should be done.
I suggest that you hold off until his patches are added because
it will have an effect. It should be done any day now.
The way you propose to do it will not meet my design criterea
for netgraph.
--
__--_|\ Julian Elischer
/ \ [EMAIL PROTECTED]
( OZ) World tour 2000
;_.---._/ presently in: Budapest
v
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
