Re: [B.A.T.M.A.N.] [PATCH v2 2/2] batman-adv: Simple (re)broadcast avoidance

[Sven Eckelmann <sven@narfation.org>]

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

On Donnerstag, 4. August 2016 07:56:44 CEST Sven Eckelmann wrote:
> On Donnerstag, 4. August 2016 01:43:29 CEST Linus Lüssing wrote:
> > On Wed, Aug 03, 2016 at 11:25:53PM +0200, Linus Lüssing wrote:
> > > On Wed, Aug 03, 2016 at 09:59:27PM +0200, Sven Eckelmann wrote:
> > > > Isn't this causing the reference counting cycle (aka really, really,
> > > > really bad):
> > > 
> > > Hm, I see what you are getting at. Could indeed be a nasty bug...
> > > 
> > > But, actually, just tested in VMs, seems like I'm getting a call to
> > > batadv_hardif_neigh_release() just fine. Which means it counted to
> > > zero successfully o_O?
> > > 
> > > I don't understand why it seems to work right now :D. Will dig deeper.
> > 
> > Ok, I think it just works because hardif_neigh_create()
> > intializes the nodes refcount to one (in contrast to other allocating
> > functions which initialize to 2). In the end, once
> > neigh_node_create() finishes, it stays at one.
> > 
> > So the regular purging routines will break the cycle in the end
> > when they reduce the refcount of the hardif_neigh_node to zero.
> 
> Ok, looks like the neigh_list edge in my graph is incorrectly there.
[...]

Just checked the code and my example graph seems to be correct in regards of 
this edge. There is an reference for the list in the code:

    static struct batadv_neigh_node *
    batadv_neigh_node_create(struct batadv_orig_node *orig_node,
    			 struct batadv_hard_iface *hard_iface,
    			 const u8 *neigh_addr)
    {
    [...]
    	/* extra reference for return */
    	kref_init(&neigh_node->refcount);
    
    	kref_get(&neigh_node->refcount);
    	hlist_add_head_rcu(&neigh_node->list, &orig_node->neigh_list);
    
    [...]
    	return neigh_node;
    }

The function which doesn't get a reference for its list is
batadv_hardif_neigh_create. It doesn't store a reference in
batadv_hardif_neigh_create for the list because its lifetime doesn't depend on
the list & just uses it as management helper and deletes the list entry in its
_release function). But this list cannot be found in the example graph.

Kind regards,
	Sven

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