On Sat, 2007-08-25 at 02:53 +0200, Hannes Gredler wrote:
> On Fri, Aug 24, 2007 at 09:48:34PM +0200, Sven-Ola Tuecke wrote:
> | Hannes,
> | 
> | great work - thanks a lot:) I've done a little patching on top of your
> | stuff - you may see them as a kind of comment from my side:
> 
> glad you like it ...

Of course!

[...]
> | 102-olsrd-rt-refactoring-fixes.patch
> | ?? Because you changed a lot of basics: It's time to handle a general
> | ?? flaw in the routing system. Plase take a look at chk_if_changed(). This
> | ?? will free() any "struct interface" pointer without warning at any time.
> | ?? This is why it's possile to SEGV olsrd with a simple "ifdown xxx". 
> | ?? The patch replaces the (maybe) invalid pointer with an index reference
> | ?? "iif_index". You can always ask the OS for a name. Please note, that I do
> | ?? not have a working BSD toolchain, so I've placed an #error in the IPv6
> | ?? BSD-part where the author/porter has started to hack something funny.
> 
> what is typically done to address 'transient' datastructure and the problem of
> dead references is to refcount those datastructures.
> 
> any datastructure (mostly nexthops, neighbors) which reference the interface
> should bump the refcount of the interface. same story for unreferencing.
> the function that decreases the refcount from 1 to 0 frees the memory.
> 
> so if a interface goes down you just mark it as DOWN using some flag
> but not free it. if you remove it from an interface tree/list
> you also decrement the refcount but still not free it as long as the refcount > 0.

Why do we free the struct interface for a downed interface at all (and
not just mark it as "down". And mark it "up" if it is up again)?
If an interface is deleted (say a bridge interface with "br ctl delbr"),
it makes sense to remove it's struct interface. But shouldn't we then
also remove all these references to it (read: walk through all relevant
sets/trees/lists and remove the elements which point to that struct
interface)?
With all due respect to saved space but we probably won't have that many
interfaces and with all the hassle with ref-counting or what else the
code for it is much smaller then the few malloc()ed interfaces.

> | 103-olsrd-rt-exportroute-cleanup.patch
> | ?? This is the reworked hooking for quagga.
> 
> whas there something wrong for the quagga plugin ? - i have taken
> care of all the plugins to work with teh new RT RIB API - including the 
> quagga plugin.

As far as I understand it, it had nothing to do with your rt-factoring
(just with the plugins code/implementation as such).
 
[...]
> | P.S.: I'm not against vim/emacs control info in sources files. But because
> | diff and patch do not handle them well, it will be more convenient if all
> | source files have that comments at the end (just in case someone wants to
> | apply diffs at EOF)
> 
> makes sense - shoulnd't be too difficult to mass-cat the comment
> to every file in the tree.

Well, while I know that one could put such things at the top, I always
put it at the end - since the one-liner version is at least for
emacs'ens really cryptic.

	Bernd
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services



