[B.A.T.M.A.N.] [PATCHv2 1/4] batman-adv: add list of unique single hop neighbors per hard-interface

Sven Eckelmann sven at narfation.org
Tue Aug 4 00:17:51 CEST 2015


Hi,

On Sunday 26 July 2015 16:27:31 Marek Lindner wrote:
> +static struct batadv_hardif_neigh_node *
> +batadv_hardif_neigh_get_or_create(struct batadv_hard_iface *hard_iface,
> +				  const u8 *neigh_addr)
> +{
> +	struct batadv_hardif_neigh_node *hardif_neigh = NULL;
> +
> +	hardif_neigh = batadv_hardif_neigh_get(hard_iface, neigh_addr);
> +	if (hardif_neigh)
> +		return hardif_neigh;
> +
> +	if (!atomic_inc_not_zero(&hard_iface->refcount))
> +		return NULL;
> +
> +	hardif_neigh = kzalloc(sizeof(*hardif_neigh), GFP_ATOMIC);
> +	if (!hardif_neigh) {
> +		batadv_hardif_free_ref(hard_iface);
> +		return NULL;
> +	}
> +
> +	INIT_HLIST_NODE(&hardif_neigh->list);
> +	ether_addr_copy(hardif_neigh->addr, neigh_addr);
> +	hardif_neigh->if_incoming = hard_iface;
> +	hardif_neigh->last_seen = jiffies;
> +
> +	atomic_set(&hardif_neigh->refcount, 1);
> +
> +	spin_lock_bh(&hard_iface->neigh_list_lock);
> +	hlist_add_head(&hardif_neigh->list, &hard_iface->neigh_list);
> +	spin_unlock_bh(&hard_iface->neigh_list_lock);
> +
> +	return hardif_neigh;
> +}

This can be the cause of inconsistencies [1] when two different contexts call
this function at the same time when no member with this mac is stored in the
list. Two nodes may then be added to the list. This should not a deal breaker
because the second node will time out.

Still, we may use a better check:

create()
{
    hardif_neigh = kmalloc();
    if (!hardif_neigh)
        return NULL;

    spin_lock_bh(&hard_iface->neigh_list_lock);
    hardif_neigh = batadv_hardif_neigh_get(hard_iface, neigh_addr);
    if (hardif_neigh)
        goto out;

    hardif_neigh = kmalloc();
    if (!hardif_neigh)
       goto out;

    hardif_neigh->init = something;
    list_add_rcu(....);

out:
    spin_unlock_bh(&hard_iface->neigh_list_lock);
    return hardif_neigh;
}

get_or_create()
{
    hardif_neigh = batadv_hardif_neigh_get(hard_iface, neigh_addr);
    if (hardif_neigh)
       return hardif_neigh;

    hardif_neigh = create();
    return hardif_neigh;
}

Kind regards,
	Sven

[1] https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2015-June/013345.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.open-mesh.org/pipermail/b.a.t.m.a.n/attachments/20150804/c3354458/attachment.sig>


More information about the B.A.T.M.A.N mailing list