[B.A.T.M.A.N.] performance patches (v3)

[B.A.T.M.A.N.] performance patches (v3)

[Marek Lindner]

Hi,

the performance patches progress step by step. A few patches already went into 
the repository the rest is waiting for more review. Thanks to Sven's feedback 
quite some bugs were squashed.

Changes since v2:
* code rebased on top of the trunk
* some more neigh_node protected by reference counters
* call_rcu and kref_put race condition fixed

Known issues:
* bonding / alternating candidates are not secured by refcounting [see  
find_router()] - ideas welcome


Regards,
Marek

[B.A.T.M.A.N.] [PATCH 1/5] batman-adv: protect neigh_nodes used outside of rcu_locks with refcounting

[Marek Lindner]

Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 batman-adv/routing.c |   40 +++++++++++++++++++++++++++++++---------
 1 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/batman-adv/routing.c b/batman-adv/routing.c
index abce1ae..7e9499f 100644
--- a/batman-adv/routing.c
+++ b/batman-adv/routing.c
@@ -152,6 +152,7 @@ static int is_bidirectional_neigh(struct orig_node *orig_node,
 	struct neigh_node *neigh_node = NULL, *tmp_neigh_node = NULL;
 	struct hlist_node *node;
 	unsigned char total_count;
+	int ret = 0;
 
 	if (orig_node == orig_neigh_node) {
 		rcu_read_lock();
@@ -163,7 +164,6 @@ static int is_bidirectional_neigh(struct orig_node *orig_node,
 			    (tmp_neigh_node->if_incoming == if_incoming))
 				neigh_node = tmp_neigh_node;
 		}
-		rcu_read_unlock();
 
 		if (!neigh_node)
 			neigh_node = create_neighbor(orig_node,
@@ -172,7 +172,10 @@ static int is_bidirectional_neigh(struct orig_node *orig_node,
 						     if_incoming);
 		/* create_neighbor failed, return 0 */
 		if (!neigh_node)
-			return 0;
+			goto unlock;
+
+		kref_get(&neigh_node->refcount);
+		rcu_read_unlock();
 
 		neigh_node->last_valid = jiffies;
 	} else {
@@ -186,7 +189,6 @@ static int is_bidirectional_neigh(struct orig_node *orig_node,
 			    (tmp_neigh_node->if_incoming == if_incoming))
 				neigh_node = tmp_neigh_node;
 		}
-		rcu_read_unlock();
 
 		if (!neigh_node)
 			neigh_node = create_neighbor(orig_neigh_node,
@@ -195,7 +197,10 @@ static int is_bidirectional_neigh(struct orig_node *orig_node,
 						     if_incoming);
 		/* create_neighbor failed, return 0 */
 		if (!neigh_node)
-			return 0;
+			goto unlock;
+
+		kref_get(&neigh_node->refcount);
+		rcu_read_unlock();
 	}
 
 	orig_node->last_valid = jiffies;
@@ -251,9 +256,16 @@ static int is_bidirectional_neigh(struct orig_node *orig_node,
 	/* if link has the minimum required transmission quality
 	 * consider it bidirectional */
 	if (batman_packet->tq >= TQ_TOTAL_BIDRECT_LIMIT)
-		return 1;
+		ret = 1;
 
-	return 0;
+	goto out;
+
+unlock:
+	rcu_read_unlock();
+out:
+	if (neigh_node)
+		kref_put(&neigh_node->refcount, neigh_node_free_ref);
+	return ret;
 }
 
 static void update_orig(struct bat_priv *bat_priv,
@@ -288,23 +300,25 @@ static void update_orig(struct bat_priv *bat_priv,
 		tmp_neigh_node->tq_avg =
 			ring_buffer_avg(tmp_neigh_node->tq_recv);
 	}
-	rcu_read_unlock();
 
 	if (!neigh_node) {
 		struct orig_node *orig_tmp;
 
 		orig_tmp = get_orig_node(bat_priv, ethhdr->h_source);
 		if (!orig_tmp)
-			return;
+			goto unlock;
 
 		neigh_node = create_neighbor(orig_node, orig_tmp,
 					     ethhdr->h_source, if_incoming);
 		if (!neigh_node)
-			return;
+			goto unlock;
 	} else
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"Updating existing last-hop neighbor of originator\n");
 
+	kref_get(&neigh_node->refcount);
+	rcu_read_unlock();
+
 	orig_node->flags = batman_packet->flags;
 	neigh_node->last_valid = jiffies;
 
@@ -358,6 +372,14 @@ update_gw:
 	    (atomic_read(&bat_priv->gw_mode) == GW_MODE_CLIENT) &&
 	    (atomic_read(&bat_priv->gw_sel_class) > 2))
 		gw_check_election(bat_priv, orig_node);
+
+	goto out;
+
+unlock:
+	rcu_read_unlock();
+out:
+	if (neigh_node)
+		kref_put(&neigh_node->refcount, neigh_node_free_ref);
 }
 
 /* checks whether the host restarted and is in the protection time.
-- 
1.7.2.3

[B.A.T.M.A.N.] [PATCH 2/5] batman-adv: protect each hash row with rcu locks

[Marek Lindner]

Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 batman-adv/hash.c              |   34 ++++++++++++++-----
 batman-adv/hash.h              |   73 +++++++++++++++++++++++++++-------------
 batman-adv/icmp_socket.c       |    2 +
 batman-adv/originator.c        |   27 +++++++++++----
 batman-adv/routing.c           |   16 ++++++++-
 batman-adv/translation-table.c |   14 ++++++++
 batman-adv/unicast.c           |    7 +++-
 batman-adv/vis.c               |   18 ++++++++--
 8 files changed, 145 insertions(+), 46 deletions(-)

diff --git a/batman-adv/hash.c b/batman-adv/hash.c
index 26e623e..aaf3e0c 100644
--- a/batman-adv/hash.c
+++ b/batman-adv/hash.c
@@ -27,13 +27,16 @@ static void hash_init(struct hashtable_t *hash)
 {
 	int i;
 
-	for (i = 0 ; i < hash->size; i++)
+	for (i = 0 ; i < hash->size; i++) {
 		INIT_HLIST_HEAD(&hash->table[i]);
+		spin_lock_init(&hash->list_locks[i]);
+	}
 }
 
 /* free only the hashtable and the hash itself. */
 void hash_destroy(struct hashtable_t *hash)
 {
+	kfree(hash->list_locks);
 	kfree(hash->table);
 	kfree(hash);
 }
@@ -43,20 +46,33 @@ struct hashtable_t *hash_new(int size)
 {
 	struct hashtable_t *hash;
 
-	hash = kmalloc(sizeof(struct hashtable_t) , GFP_ATOMIC);
-
+	hash = kmalloc(sizeof(struct hashtable_t), GFP_ATOMIC);
 	if (!hash)
 		return NULL;
 
-	hash->size = size;
 	hash->table = kmalloc(sizeof(struct element_t *) * size, GFP_ATOMIC);
+	if (!hash->table)
+		goto free_hash;
 
-	if (!hash->table) {
-		kfree(hash);
-		return NULL;
-	}
+	hash->list_locks = kmalloc(sizeof(spinlock_t) * size, GFP_ATOMIC);
+	if (!hash->list_locks)
+		goto free_table;
 
+	hash->size = size;
 	hash_init(hash);
-
 	return hash;
+
+free_table:
+	kfree(hash->table);
+free_hash:
+	kfree(hash);
+	return NULL;
+}
+
+void bucket_free_rcu(struct rcu_head *rcu)
+{
+	struct element_t *bucket;
+
+	bucket = container_of(rcu, struct element_t, rcu);
+	kfree(bucket);
 }
diff --git a/batman-adv/hash.h b/batman-adv/hash.h
index 09216ad..d47735c 100644
--- a/batman-adv/hash.h
+++ b/batman-adv/hash.h
@@ -39,10 +39,12 @@ typedef void (*hashdata_free_cb)(void *, void *);
 struct element_t {
 	void *data;		/* pointer to the data */
 	struct hlist_node hlist;	/* bucket list pointer */
+	struct rcu_head rcu;
 };
 
 struct hashtable_t {
-	struct hlist_head *table;   /* the hashtable itself, with the buckets */
+	struct hlist_head *table;   /* the hashtable itself with the buckets */
+	spinlock_t *list_locks;     /* spinlock for each hash list entry */
 	int size;		    /* size of hashtable */
 };
 
@@ -57,6 +59,8 @@ void *hash_remove_element(struct hashtable_t *hash, struct element_t *elem);
 /* free only the hashtable and the hash itself. */
 void hash_destroy(struct hashtable_t *hash);
 
+void bucket_free_rcu(struct rcu_head *rcu);
+
 /* remove the hash structure. if hashdata_free_cb != NULL, this function will be
  * called to remove the elements inside of the hash.  if you don't remove the
  * elements, memory might be leaked. */
@@ -66,19 +70,22 @@ static inline void hash_delete(struct hashtable_t *hash,
 	struct hlist_head *head;
 	struct hlist_node *walk, *safe;
 	struct element_t *bucket;
+	spinlock_t *list_lock;
 	int i;
 
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
+		list_lock = &hash->list_locks[i];
 
-		hlist_for_each_safe(walk, safe, head) {
-			bucket = hlist_entry(walk, struct element_t, hlist);
+		spin_lock_bh(list_lock);
+		hlist_for_each_entry_safe(bucket, walk, safe, head, hlist) {
 			if (free_cb)
 				free_cb(bucket->data, arg);
 
-			hlist_del(walk);
-			kfree(bucket);
+			hlist_del_rcu(walk);
+			call_rcu(&bucket->rcu, bucket_free_rcu);
 		}
+		spin_unlock_bh(list_lock);
 	}
 
 	hash_destroy(hash);
@@ -93,29 +100,39 @@ static inline int hash_add(struct hashtable_t *hash,
 	struct hlist_head *head;
 	struct hlist_node *walk, *safe;
 	struct element_t *bucket;
+	spinlock_t *list_lock;
 
 	if (!hash)
-		return -1;
+		goto err;
 
 	index = choose(data, hash->size);
 	head = &hash->table[index];
+	list_lock = &hash->list_locks[index];
 
-	hlist_for_each_safe(walk, safe, head) {
-		bucket = hlist_entry(walk, struct element_t, hlist);
+	rcu_read_lock();
+	hlist_for_each_entry_safe(bucket, walk, safe, head, hlist) {
 		if (compare(bucket->data, data))
-			return -1;
+			goto err_unlock;
 	}
+	rcu_read_unlock();
 
 	/* no duplicate found in list, add new element */
 	bucket = kmalloc(sizeof(struct element_t), GFP_ATOMIC);
-
 	if (!bucket)
-		return -1;
+		goto err;
 
 	bucket->data = data;
-	hlist_add_head(&bucket->hlist, head);
+
+	spin_lock_bh(list_lock);
+	hlist_add_head_rcu(&bucket->hlist, head);
+	spin_unlock_bh(list_lock);
 
 	return 0;
+
+err_unlock:
+	rcu_read_unlock();
+err:
+	return -1;
 }
 
 /* removes data from hash, if found. returns pointer do data on success, so you
@@ -130,25 +147,31 @@ static inline void *hash_remove(struct hashtable_t *hash,
 	struct hlist_node *walk;
 	struct element_t *bucket;
 	struct hlist_head *head;
-	void *data_save;
+	void *data_save = NULL;
 
 	index = choose(data, hash->size);
 	head = &hash->table[index];
 
+	spin_lock_bh(&hash->list_locks[index]);
 	hlist_for_each_entry(bucket, walk, head, hlist) {
 		if (compare(bucket->data, data)) {
 			data_save = bucket->data;
-			hlist_del(walk);
-			kfree(bucket);
-			return data_save;
+			hlist_del_rcu(walk);
+			call_rcu(&bucket->rcu, bucket_free_rcu);
+			break;
 		}
 	}
+	spin_unlock_bh(&hash->list_locks[index]);
 
-	return NULL;
+	return data_save;
 }
 
-/* finds data, based on the key in keydata. returns the found data on success,
- * or NULL on error */
+/**
+ * finds data, based on the key in keydata. returns the found data on success,
+ * or NULL on error
+ *
+ * caller must lock with rcu_read_lock() / rcu_read_unlock()
+ **/
 static inline void *hash_find(struct hashtable_t *hash,
 			      hashdata_compare_cb compare,
 			      hashdata_choose_cb choose, void *keydata)
@@ -157,6 +180,7 @@ static inline void *hash_find(struct hashtable_t *hash,
 	struct hlist_head *head;
 	struct hlist_node *walk;
 	struct element_t *bucket;
+	void *bucket_data = NULL;
 
 	if (!hash)
 		return NULL;
@@ -164,13 +188,14 @@ static inline void *hash_find(struct hashtable_t *hash,
 	index = choose(keydata , hash->size);
 	head = &hash->table[index];
 
-	hlist_for_each(walk, head) {
-		bucket = hlist_entry(walk, struct element_t, hlist);
-		if (compare(bucket->data, keydata))
-			return bucket->data;
+	hlist_for_each_entry(bucket, walk, head, hlist) {
+		if (compare(bucket->data, keydata)) {
+			bucket_data = bucket->data;
+			break;
+		}
 	}
 
-	return NULL;
+	return bucket_data;
 }
 
 #endif /* _NET_BATMAN_ADV_HASH_H_ */
diff --git a/batman-adv/icmp_socket.c b/batman-adv/icmp_socket.c
index ecf6d7f..a9a10ed 100644
--- a/batman-adv/icmp_socket.c
+++ b/batman-adv/icmp_socket.c
@@ -221,9 +221,11 @@ static ssize_t bat_socket_write(struct file *file, const char __user *buff,
 		goto dst_unreach;
 
 	spin_lock_bh(&bat_priv->orig_hash_lock);
+	rcu_read_lock();
 	orig_node = ((struct orig_node *)hash_find(bat_priv->orig_hash,
 						   compare_orig, choose_orig,
 						   icmp_packet->dst));
+	rcu_read_unlock();
 
 	if (!orig_node)
 		goto unlock;
diff --git a/batman-adv/originator.c b/batman-adv/originator.c
index 6816054..961c60c 100644
--- a/batman-adv/originator.c
+++ b/batman-adv/originator.c
@@ -150,9 +150,11 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)
 	int size;
 	int hash_added;
 
+	rcu_read_lock();
 	orig_node = ((struct orig_node *)hash_find(bat_priv->orig_hash,
 						   compare_orig, choose_orig,
 						   addr));
+	rcu_read_unlock();
 
 	if (orig_node)
 		return orig_node;
@@ -289,6 +291,7 @@ static void _purge_orig(struct bat_priv *bat_priv)
 	struct hlist_node *walk, *safe;
 	struct hlist_head *head;
 	struct element_t *bucket;
+	spinlock_t *list_lock;
 	struct orig_node *orig_node;
 	int i;
 
@@ -300,22 +303,26 @@ static void _purge_orig(struct bat_priv *bat_priv)
 	/* for all origins... */
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
+		list_lock = &hash->list_locks[i];
 
+		spin_lock_bh(list_lock);
 		hlist_for_each_entry_safe(bucket, walk, safe, head, hlist) {
 			orig_node = bucket->data;
 
 			if (purge_orig_node(bat_priv, orig_node)) {
 				if (orig_node->gw_flags)
 					gw_node_delete(bat_priv, orig_node);
-				hlist_del(walk);
-				kfree(bucket);
+				hlist_del_rcu(walk);
+				call_rcu(&bucket->rcu, bucket_free_rcu);
 				free_orig_node(orig_node, bat_priv);
+				continue;
 			}
 
 			if (time_after(jiffies, orig_node->last_frag_packet +
 						msecs_to_jiffies(FRAG_TIMEOUT)))
 				frag_list_free(&orig_node->frag_list);
 		}
+		spin_unlock_bh(list_lock);
 	}
 
 	spin_unlock_bh(&bat_priv->orig_hash_lock);
@@ -382,7 +389,8 @@ int orig_seq_print_text(struct seq_file *seq, void *offset)
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
-		hlist_for_each_entry(bucket, walk, head, hlist) {
+		rcu_read_lock();
+		hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
 			orig_node = bucket->data;
 
 			if (!orig_node->router)
@@ -403,17 +411,16 @@ int orig_seq_print_text(struct seq_file *seq, void *offset)
 				   neigh_node->addr,
 				   neigh_node->if_incoming->net_dev->name);
 
-			rcu_read_lock();
 			hlist_for_each_entry_rcu(neigh_node, node,
 						 &orig_node->neigh_list, list) {
 				seq_printf(seq, " %pM (%3i)", neigh_node->addr,
 						neigh_node->tq_avg);
 			}
-			rcu_read_unlock();
 
 			seq_printf(seq, "\n");
 			batman_count++;
 		}
+		rcu_read_unlock();
 	}
 
 	spin_unlock_bh(&bat_priv->orig_hash_lock);
@@ -471,18 +478,21 @@ int orig_hash_add_if(struct batman_if *batman_if, int max_if_num)
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
-		hlist_for_each_entry(bucket, walk, head, hlist) {
+		rcu_read_lock();
+		hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
 			orig_node = bucket->data;
 
 			if (orig_node_add_if(orig_node, max_if_num) == -1)
 				goto err;
 		}
+		rcu_read_unlock();
 	}
 
 	spin_unlock_bh(&bat_priv->orig_hash_lock);
 	return 0;
 
 err:
+	rcu_read_unlock();
 	spin_unlock_bh(&bat_priv->orig_hash_lock);
 	return -ENOMEM;
 }
@@ -557,7 +567,8 @@ int orig_hash_del_if(struct batman_if *batman_if, int max_if_num)
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
-		hlist_for_each_entry(bucket, walk, head, hlist) {
+		rcu_read_lock();
+		hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
 			orig_node = bucket->data;
 
 			ret = orig_node_del_if(orig_node, max_if_num,
@@ -566,6 +577,7 @@ int orig_hash_del_if(struct batman_if *batman_if, int max_if_num)
 			if (ret == -1)
 				goto err;
 		}
+		rcu_read_unlock();
 	}
 
 	/* renumber remaining batman interfaces _inside_ of orig_hash_lock */
@@ -590,6 +602,7 @@ int orig_hash_del_if(struct batman_if *batman_if, int max_if_num)
 	return 0;
 
 err:
+	rcu_read_unlock();
 	spin_unlock_bh(&bat_priv->orig_hash_lock);
 	return -ENOMEM;
 }
diff --git a/batman-adv/routing.c b/batman-adv/routing.c
index 7e9499f..b6991c0 100644
--- a/batman-adv/routing.c
+++ b/batman-adv/routing.c
@@ -53,7 +53,8 @@ void slide_own_bcast_window(struct batman_if *batman_if)
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
-		hlist_for_each_entry(bucket, walk, head, hlist) {
+		rcu_read_lock();
+		hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
 			orig_node = bucket->data;
 			word_index = batman_if->if_num * NUM_WORDS;
 			word = &(orig_node->bcast_own[word_index]);
@@ -62,6 +63,7 @@ void slide_own_bcast_window(struct batman_if *batman_if)
 			orig_node->bcast_own_sum[batman_if->if_num] =
 				bit_packet_count(word);
 		}
+		rcu_read_unlock();
 	}
 
 	spin_unlock_bh(&bat_priv->orig_hash_lock);
@@ -879,9 +881,11 @@ static int recv_my_icmp_packet(struct bat_priv *bat_priv,
 	/* answer echo request (ping) */
 	/* get routing information */
 	spin_lock_bh(&bat_priv->orig_hash_lock);
+	rcu_read_lock();
 	orig_node = ((struct orig_node *)hash_find(bat_priv->orig_hash,
 						   compare_orig, choose_orig,
 						   icmp_packet->orig));
+	rcu_read_unlock();
 	ret = NET_RX_DROP;
 
 	if ((orig_node) && (orig_node->router)) {
@@ -940,9 +944,11 @@ static int recv_icmp_ttl_exceeded(struct bat_priv *bat_priv,
 
 	/* get routing information */
 	spin_lock_bh(&bat_priv->orig_hash_lock);
+	rcu_read_lock();
 	orig_node = ((struct orig_node *)
 		     hash_find(bat_priv->orig_hash, compare_orig, choose_orig,
 			       icmp_packet->orig));
+	rcu_read_unlock();
 	ret = NET_RX_DROP;
 
 	if ((orig_node) && (orig_node->router)) {
@@ -1033,9 +1039,11 @@ int recv_icmp_packet(struct sk_buff *skb, struct batman_if *recv_if)
 
 	/* get routing information */
 	spin_lock_bh(&bat_priv->orig_hash_lock);
+	rcu_read_lock();
 	orig_node = ((struct orig_node *)
 		     hash_find(bat_priv->orig_hash, compare_orig, choose_orig,
 			       icmp_packet->dst));
+	rcu_read_unlock();
 
 	if ((orig_node) && (orig_node->router)) {
 
@@ -1105,9 +1113,11 @@ struct neigh_node *find_router(struct bat_priv *bat_priv,
 				router_orig->orig, ETH_ALEN) == 0) {
 		primary_orig_node = router_orig;
 	} else {
+		rcu_read_lock();
 		primary_orig_node = hash_find(bat_priv->orig_hash, compare_orig,
 					       choose_orig,
 					       router_orig->primary_addr);
+		rcu_read_unlock();
 
 		if (!primary_orig_node)
 			return orig_node->router;
@@ -1210,9 +1220,11 @@ int route_unicast_packet(struct sk_buff *skb, struct batman_if *recv_if,
 
 	/* get routing information */
 	spin_lock_bh(&bat_priv->orig_hash_lock);
+	rcu_read_lock();
 	orig_node = ((struct orig_node *)
 		     hash_find(bat_priv->orig_hash, compare_orig, choose_orig,
 			       unicast_packet->dest));
+	rcu_read_unlock();
 
 	router = find_router(bat_priv, orig_node, recv_if);
 
@@ -1356,9 +1368,11 @@ int recv_bcast_packet(struct sk_buff *skb, struct batman_if *recv_if)
 		return NET_RX_DROP;
 
 	spin_lock_bh(&bat_priv->orig_hash_lock);
+	rcu_read_lock();
 	orig_node = ((struct orig_node *)
 		     hash_find(bat_priv->orig_hash, compare_orig, choose_orig,
 			       bcast_packet->orig));
+	rcu_read_unlock();
 
 	if (!orig_node) {
 		spin_unlock_bh(&bat_priv->orig_hash_lock);
diff --git a/batman-adv/translation-table.c b/batman-adv/translation-table.c
index a19e16c..6f6ee58 100644
--- a/batman-adv/translation-table.c
+++ b/batman-adv/translation-table.c
@@ -61,10 +61,12 @@ void hna_local_add(struct net_device *soft_iface, uint8_t *addr)
 	int required_bytes;
 
 	spin_lock_bh(&bat_priv->hna_lhash_lock);
+	rcu_read_lock();
 	hna_local_entry =
 		((struct hna_local_entry *)hash_find(bat_priv->hna_local_hash,
 						     compare_orig, choose_orig,
 						     addr));
+	rcu_read_unlock();
 	spin_unlock_bh(&bat_priv->hna_lhash_lock);
 
 	if (hna_local_entry) {
@@ -117,9 +119,11 @@ void hna_local_add(struct net_device *soft_iface, uint8_t *addr)
 	/* remove address from global hash if present */
 	spin_lock_bh(&bat_priv->hna_ghash_lock);
 
+	rcu_read_lock();
 	hna_global_entry = ((struct hna_global_entry *)
 				hash_find(bat_priv->hna_global_hash,
 					  compare_orig, choose_orig, addr));
+	rcu_read_unlock();
 
 	if (hna_global_entry)
 		_hna_global_del_orig(bat_priv, hna_global_entry,
@@ -253,9 +257,11 @@ void hna_local_remove(struct bat_priv *bat_priv,
 
 	spin_lock_bh(&bat_priv->hna_lhash_lock);
 
+	rcu_read_lock();
 	hna_local_entry = (struct hna_local_entry *)
 		hash_find(bat_priv->hna_local_hash, compare_orig, choose_orig,
 			  addr);
+	rcu_read_unlock();
 
 	if (hna_local_entry)
 		hna_local_del(bat_priv, hna_local_entry, message);
@@ -335,9 +341,11 @@ void hna_global_add_orig(struct bat_priv *bat_priv,
 		spin_lock_bh(&bat_priv->hna_ghash_lock);
 
 		hna_ptr = hna_buff + (hna_buff_count * ETH_ALEN);
+		rcu_read_lock();
 		hna_global_entry = (struct hna_global_entry *)
 			hash_find(bat_priv->hna_global_hash, compare_orig,
 				  choose_orig, hna_ptr);
+		rcu_read_unlock();
 
 		if (!hna_global_entry) {
 			spin_unlock_bh(&bat_priv->hna_ghash_lock);
@@ -369,9 +377,11 @@ void hna_global_add_orig(struct bat_priv *bat_priv,
 		spin_lock_bh(&bat_priv->hna_lhash_lock);
 
 		hna_ptr = hna_buff + (hna_buff_count * ETH_ALEN);
+		rcu_read_lock();
 		hna_local_entry = (struct hna_local_entry *)
 			hash_find(bat_priv->hna_local_hash, compare_orig,
 				  choose_orig, hna_ptr);
+		rcu_read_unlock();
 
 		if (hna_local_entry)
 			hna_local_del(bat_priv, hna_local_entry,
@@ -484,9 +494,11 @@ void hna_global_del_orig(struct bat_priv *bat_priv,
 
 	while ((hna_buff_count + 1) * ETH_ALEN <= orig_node->hna_buff_len) {
 		hna_ptr = orig_node->hna_buff + (hna_buff_count * ETH_ALEN);
+		rcu_read_lock();
 		hna_global_entry = (struct hna_global_entry *)
 			hash_find(bat_priv->hna_global_hash, compare_orig,
 				  choose_orig, hna_ptr);
+		rcu_read_unlock();
 
 		if ((hna_global_entry) &&
 		    (hna_global_entry->orig_node == orig_node))
@@ -522,9 +534,11 @@ struct orig_node *transtable_search(struct bat_priv *bat_priv, uint8_t *addr)
 	struct hna_global_entry *hna_global_entry;
 
 	spin_lock_bh(&bat_priv->hna_ghash_lock);
+	rcu_read_lock();
 	hna_global_entry = (struct hna_global_entry *)
 				hash_find(bat_priv->hna_global_hash,
 					  compare_orig, choose_orig, addr);
+	rcu_read_unlock();
 	spin_unlock_bh(&bat_priv->hna_ghash_lock);
 
 	if (!hna_global_entry)
diff --git a/batman-adv/unicast.c b/batman-adv/unicast.c
index dc2e28b..0b2ea82 100644
--- a/batman-adv/unicast.c
+++ b/batman-adv/unicast.c
@@ -179,9 +179,11 @@ int frag_reassemble_skb(struct sk_buff *skb, struct bat_priv *bat_priv,
 
 	*new_skb = NULL;
 	spin_lock_bh(&bat_priv->orig_hash_lock);
+	rcu_read_lock();
 	orig_node = ((struct orig_node *)
 		    hash_find(bat_priv->orig_hash, compare_orig, choose_orig,
 			      unicast_packet->orig));
+	rcu_read_unlock();
 
 	if (!orig_node) {
 		pr_debug("couldn't find originator in orig_hash\n");
@@ -285,11 +287,14 @@ int unicast_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv)
 	/* get routing information */
 	if (is_multicast_ether_addr(ethhdr->h_dest))
 		orig_node = (struct orig_node *)gw_get_selected(bat_priv);
-	else
+	else {
+		rcu_read_lock();
 		orig_node = ((struct orig_node *)hash_find(bat_priv->orig_hash,
 							   compare_orig,
 							   choose_orig,
 							   ethhdr->h_dest));
+		rcu_read_unlock();
+	}
 
 	/* check for hna host */
 	if (!orig_node)
diff --git a/batman-adv/vis.c b/batman-adv/vis.c
index cd4c423..230e66b 100644
--- a/batman-adv/vis.c
+++ b/batman-adv/vis.c
@@ -380,8 +380,10 @@ static struct vis_info *add_packet(struct bat_priv *bat_priv,
 						     sizeof(struct vis_packet));
 
 	memcpy(search_packet->vis_orig, vis_packet->vis_orig, ETH_ALEN);
+	rcu_read_lock();
 	old_info = hash_find(bat_priv->vis_hash, vis_info_cmp, vis_info_choose,
 			     &search_elem);
+	rcu_read_unlock();
 	kfree_skb(search_elem.skb_packet);
 
 	if (old_info) {
@@ -540,7 +542,8 @@ static int find_best_vis_server(struct bat_priv *bat_priv,
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
-		hlist_for_each_entry(bucket, walk, head, hlist) {
+		rcu_read_lock();
+		hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
 			orig_node = bucket->data;
 			if ((orig_node) && (orig_node->router) &&
 			(orig_node->flags & VIS_SERVER) &&
@@ -550,6 +553,7 @@ static int find_best_vis_server(struct bat_priv *bat_priv,
 				       ETH_ALEN);
 			}
 		}
+		rcu_read_unlock();
 	}
 
 	return best_tq;
@@ -605,7 +609,8 @@ static int generate_vis_packet(struct bat_priv *bat_priv)
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
-		hlist_for_each_entry(bucket, walk, head, hlist) {
+		rcu_read_lock();
+		hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
 			orig_node = bucket->data;
 			neigh_node = orig_node->router;
 
@@ -632,10 +637,12 @@ static int generate_vis_packet(struct bat_priv *bat_priv)
 			packet->entries++;
 
 			if (vis_packet_full(info)) {
+				rcu_read_unlock();
 				spin_unlock_bh(&bat_priv->orig_hash_lock);
 				return 0;
 			}
 		}
+		rcu_read_unlock();
 	}
 
 	spin_unlock_bh(&bat_priv->orig_hash_lock);
@@ -721,7 +728,8 @@ static void broadcast_vis_packet(struct bat_priv *bat_priv,
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
-		hlist_for_each_entry(bucket, walk, head, hlist) {
+		rcu_read_lock();
+		hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
 			orig_node = bucket->data;
 
 			/* if it's a vis server and reachable, send it. */
@@ -746,7 +754,7 @@ static void broadcast_vis_packet(struct bat_priv *bat_priv,
 
 			spin_lock_bh(&bat_priv->orig_hash_lock);
 		}
-
+		rcu_read_unlock();
 	}
 
 	spin_unlock_bh(&bat_priv->orig_hash_lock);
@@ -763,9 +771,11 @@ static void unicast_vis_packet(struct bat_priv *bat_priv,
 
 	spin_lock_bh(&bat_priv->orig_hash_lock);
 	packet = (struct vis_packet *)info->skb_packet->data;
+	rcu_read_lock();
 	orig_node = ((struct orig_node *)hash_find(bat_priv->orig_hash,
 						   compare_orig, choose_orig,
 						   packet->target_orig));
+	rcu_read_unlock();
 
 	if ((!orig_node) || (!orig_node->router))
 		goto out;
-- 
1.7.2.3

[B.A.T.M.A.N.] [PATCH 3/5] batman-adv: protect originator nodes with reference counters

[Marek Lindner]

Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 batman-adv/originator.c |   61 ++++++++++++++++++++++++++++++++++++++++-------
 batman-adv/originator.h |    1 +
 batman-adv/routing.c    |   33 +++++++++++++++++-------
 batman-adv/types.h      |    2 +
 4 files changed, 78 insertions(+), 19 deletions(-)

diff --git a/batman-adv/originator.c b/batman-adv/originator.c
index 961c60c..35d1859 100644
--- a/batman-adv/originator.c
+++ b/batman-adv/originator.c
@@ -103,12 +103,13 @@ struct neigh_node *create_neighbor(struct orig_node *orig_node,
 	return neigh_node;
 }
 
-static void free_orig_node(void *data, void *arg)
+void orig_node_free_ref(struct kref *refcount)
 {
 	struct hlist_node *node, *node_tmp;
 	struct neigh_node *neigh_node;
-	struct orig_node *orig_node = (struct orig_node *)data;
-	struct bat_priv *bat_priv = (struct bat_priv *)arg;
+	struct orig_node *orig_node;
+
+	orig_node = container_of(refcount, struct orig_node, refcount);
 
 	spin_lock_bh(&orig_node->neigh_list_lock);
 
@@ -122,7 +123,8 @@ static void free_orig_node(void *data, void *arg)
 	spin_unlock_bh(&orig_node->neigh_list_lock);
 
 	frag_list_free(&orig_node->frag_list);
-	hna_global_del_orig(bat_priv, orig_node, "originator timed out");
+	hna_global_del_orig(orig_node->bat_priv, orig_node,
+			    "originator timed out");
 
 	kfree(orig_node->bcast_own);
 	kfree(orig_node->bcast_own_sum);
@@ -131,17 +133,53 @@ static void free_orig_node(void *data, void *arg)
 
 void originator_free(struct bat_priv *bat_priv)
 {
-	if (!bat_priv->orig_hash)
+	struct hashtable_t *hash = bat_priv->orig_hash;
+	struct hlist_node *walk, *safe;
+	struct hlist_head *head;
+	struct element_t *bucket;
+	spinlock_t *list_lock;
+	struct orig_node *orig_node;
+	int i;
+
+	if (!hash)
 		return;
 
 	cancel_delayed_work_sync(&bat_priv->orig_work);
 
 	spin_lock_bh(&bat_priv->orig_hash_lock);
-	hash_delete(bat_priv->orig_hash, free_orig_node, bat_priv);
 	bat_priv->orig_hash = NULL;
+
+	for (i = 0; i < hash->size; i++) {
+		head = &hash->table[i];
+		list_lock = &hash->list_locks[i];
+
+		spin_lock_bh(list_lock);
+		hlist_for_each_entry_safe(bucket, walk, safe, head, hlist) {
+			orig_node = bucket->data;
+
+			hlist_del_rcu(walk);
+			call_rcu(&bucket->rcu, bucket_free_rcu);
+			kref_put(&orig_node->refcount, orig_node_free_ref);
+		}
+		spin_unlock_bh(list_lock);
+	}
+
+	hash_destroy(hash);
 	spin_unlock_bh(&bat_priv->orig_hash_lock);
 }
 
+void bucket_free_orig_rcu(struct rcu_head *rcu)
+{
+	struct element_t *bucket;
+	struct orig_node *orig_node;
+
+	bucket = container_of(rcu, struct element_t, rcu);
+	orig_node = bucket->data;
+
+	kref_put(&orig_node->refcount, orig_node_free_ref);
+	kfree(bucket);
+}
+
 /* this function finds or creates an originator entry for the given
  * address if it does not exits */
 struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)
@@ -156,8 +194,10 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)
 						   addr));
 	rcu_read_unlock();
 
-	if (orig_node)
+	if (orig_node) {
+		kref_get(&orig_node->refcount);
 		return orig_node;
+	}
 
 	bat_dbg(DBG_BATMAN, bat_priv,
 		"Creating new originator: %pM\n", addr);
@@ -168,7 +208,9 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)
 
 	INIT_HLIST_HEAD(&orig_node->neigh_list);
 	spin_lock_init(&orig_node->neigh_list_lock);
+	kref_init(&orig_node->refcount);
 
+	orig_node->bat_priv = bat_priv;
 	memcpy(orig_node->orig, addr, ETH_ALEN);
 	orig_node->router = NULL;
 	orig_node->hna_buff = NULL;
@@ -197,6 +239,8 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)
 	if (hash_added < 0)
 		goto free_bcast_own_sum;
 
+	/* extra reference for return */
+	kref_get(&orig_node->refcount);
 	return orig_node;
 free_bcast_own_sum:
 	kfree(orig_node->bcast_own_sum);
@@ -313,8 +357,7 @@ static void _purge_orig(struct bat_priv *bat_priv)
 				if (orig_node->gw_flags)
 					gw_node_delete(bat_priv, orig_node);
 				hlist_del_rcu(walk);
-				call_rcu(&bucket->rcu, bucket_free_rcu);
-				free_orig_node(orig_node, bat_priv);
+				call_rcu(&bucket->rcu, bucket_free_orig_rcu);
 				continue;
 			}
 
diff --git a/batman-adv/originator.h b/batman-adv/originator.h
index f3676fa..f5a6964 100644
--- a/batman-adv/originator.h
+++ b/batman-adv/originator.h
@@ -25,6 +25,7 @@
 int originator_init(struct bat_priv *bat_priv);
 void originator_free(struct bat_priv *bat_priv);
 void purge_orig_ref(struct bat_priv *bat_priv);
+void orig_node_free_ref(struct kref *refcount);
 struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr);
 struct neigh_node *create_neighbor(struct orig_node *orig_node,
 				   struct orig_node *orig_neigh_node,
diff --git a/batman-adv/routing.c b/batman-adv/routing.c
index b6991c0..f2d4ae1 100644
--- a/batman-adv/routing.c
+++ b/batman-adv/routing.c
@@ -312,6 +312,8 @@ static void update_orig(struct bat_priv *bat_priv,
 
 		neigh_node = create_neighbor(orig_node, orig_tmp,
 					     ethhdr->h_source, if_incoming);
+
+		kref_put(&orig_tmp->refcount, orig_node_free_ref);
 		if (!neigh_node)
 			goto unlock;
 	} else
@@ -439,7 +441,7 @@ static char count_real_packets(struct ethhdr *ethhdr,
 	/* signalize caller that the packet is to be dropped. */
 	if (window_protected(bat_priv, seq_diff,
 			     &orig_node->batman_seqno_reset))
-		return -1;
+		goto err;
 
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(tmp_neigh_node, node,
@@ -472,7 +474,12 @@ static char count_real_packets(struct ethhdr *ethhdr,
 		orig_node->last_real_seqno = batman_packet->seqno;
 	}
 
+	kref_put(&orig_node->refcount, orig_node_free_ref);
 	return is_duplicate;
+
+err:
+	kref_put(&orig_node->refcount, orig_node_free_ref);
+	return -1;
 }
 
 /* copy primary address for bonding */
@@ -689,7 +696,6 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 		int offset;
 
 		orig_neigh_node = get_orig_node(bat_priv, ethhdr->h_source);
-
 		if (!orig_neigh_node)
 			return;
 
@@ -710,6 +716,7 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 
 		bat_dbg(DBG_BATMAN, bat_priv, "Drop packet: "
 			"originator packet from myself (via neighbor)\n");
+		kref_put(&orig_neigh_node->refcount, orig_node_free_ref);
 		return;
 	}
 
@@ -730,13 +737,13 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"Drop packet: packet within seqno protection time "
 			"(sender: %pM)\n", ethhdr->h_source);
-		return;
+		goto out;
 	}
 
 	if (batman_packet->tq == 0) {
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"Drop packet: originator packet with tq equal 0\n");
-		return;
+		goto out;
 	}
 
 	/* avoid temporary routing loops */
@@ -750,7 +757,7 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"Drop packet: ignoring all rebroadcast packets that "
 			"may make me loop (sender: %pM)\n", ethhdr->h_source);
-		return;
+		goto out;
 	}
 
 	/* if sender is a direct neighbor the sender mac equals
@@ -759,14 +766,14 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 			   orig_node :
 			   get_orig_node(bat_priv, ethhdr->h_source));
 	if (!orig_neigh_node)
-		return;
+		goto out_neigh;
 
 	/* drop packet if sender is not a direct neighbor and if we
 	 * don't route towards it */
 	if (!is_single_hop_neigh && (!orig_neigh_node->router)) {
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"Drop packet: OGM via unknown neighbor!\n");
-		return;
+		goto out_neigh;
 	}
 
 	is_bidirectional = is_bidirectional_neigh(orig_node, orig_neigh_node,
@@ -794,26 +801,32 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 
 		bat_dbg(DBG_BATMAN, bat_priv, "Forwarding packet: "
 			"rebroadcast neighbor packet with direct link flag\n");
-		return;
+		goto out_neigh;
 	}
 
 	/* multihop originator */
 	if (!is_bidirectional) {
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"Drop packet: not received via bidirectional link\n");
-		return;
+		goto out_neigh;
 	}
 
 	if (is_duplicate) {
 		bat_dbg(DBG_BATMAN, bat_priv,
 			"Drop packet: duplicate packet received\n");
-		return;
+		goto out_neigh;
 	}
 
 	bat_dbg(DBG_BATMAN, bat_priv,
 		"Forwarding packet: rebroadcast originator packet\n");
 	schedule_forward_packet(orig_node, ethhdr, batman_packet,
 				0, hna_buff_len, if_incoming);
+
+out_neigh:
+	if (!is_single_hop_neigh)
+		kref_put(&orig_neigh_node->refcount, orig_node_free_ref);
+out:
+	kref_put(&orig_node->refcount, orig_node_free_ref);
 }
 
 int recv_bat_packet(struct sk_buff *skb, struct batman_if *batman_if)
diff --git a/batman-adv/types.h b/batman-adv/types.h
index 09d18d8..80413d1 100644
--- a/batman-adv/types.h
+++ b/batman-adv/types.h
@@ -86,6 +86,8 @@ struct orig_node {
 	struct hlist_head neigh_list;
 	struct list_head frag_list;
 	spinlock_t neigh_list_lock; /* protects neighbor list */
+	struct kref refcount;
+	struct bat_priv *bat_priv;
 	unsigned long last_frag_packet;
 	struct {
 		uint8_t candidates;
-- 
1.7.2.3

[B.A.T.M.A.N.] [PATCH 4/5] batman-adv: protect ogm counter arrays with spinlock

[Marek Lindner]

Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 batman-adv/originator.c |   11 +++++++++--
 batman-adv/routing.c    |   27 +++++++++++++++++++++++----
 batman-adv/types.h      |    1 +
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/batman-adv/originator.c b/batman-adv/originator.c
index 35d1859..332dce1 100644
--- a/batman-adv/originator.c
+++ b/batman-adv/originator.c
@@ -207,6 +207,7 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)
 		return NULL;
 
 	INIT_HLIST_HEAD(&orig_node->neigh_list);
+	spin_lock_init(&orig_node->ogm_cnt_lock);
 	spin_lock_init(&orig_node->neigh_list_lock);
 	kref_init(&orig_node->refcount);
 
@@ -512,7 +513,7 @@ int orig_hash_add_if(struct batman_if *batman_if, int max_if_num)
 	struct hlist_head *head;
 	struct element_t *bucket;
 	struct orig_node *orig_node;
-	int i;
+	int i, ret;
 
 	/* resize all orig nodes because orig_node->bcast_own(_sum) depend on
 	 * if_num */
@@ -525,7 +526,11 @@ int orig_hash_add_if(struct batman_if *batman_if, int max_if_num)
 		hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
 			orig_node = bucket->data;
 
-			if (orig_node_add_if(orig_node, max_if_num) == -1)
+			spin_lock_bh(&orig_node->ogm_cnt_lock);
+			ret = orig_node_add_if(orig_node, max_if_num);
+			spin_unlock_bh(&orig_node->ogm_cnt_lock);
+
+			if (ret == -1)
 				goto err;
 		}
 		rcu_read_unlock();
@@ -614,8 +619,10 @@ int orig_hash_del_if(struct batman_if *batman_if, int max_if_num)
 		hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
 			orig_node = bucket->data;
 
+			spin_lock_bh(&orig_node->ogm_cnt_lock);
 			ret = orig_node_del_if(orig_node, max_if_num,
 					batman_if->if_num);
+			spin_unlock_bh(&orig_node->ogm_cnt_lock);
 
 			if (ret == -1)
 				goto err;
diff --git a/batman-adv/routing.c b/batman-adv/routing.c
index f2d4ae1..f0e658a 100644
--- a/batman-adv/routing.c
+++ b/batman-adv/routing.c
@@ -56,12 +56,14 @@ void slide_own_bcast_window(struct batman_if *batman_if)
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(bucket, walk, head, hlist) {
 			orig_node = bucket->data;
+			spin_lock_bh(&orig_node->ogm_cnt_lock);
 			word_index = batman_if->if_num * NUM_WORDS;
 			word = &(orig_node->bcast_own[word_index]);
 
 			bit_get_packet(bat_priv, word, 1, 0);
 			orig_node->bcast_own_sum[batman_if->if_num] =
 				bit_packet_count(word);
+			spin_unlock_bh(&orig_node->ogm_cnt_lock);
 		}
 		rcu_read_unlock();
 	}
@@ -279,8 +281,10 @@ static void update_orig(struct bat_priv *bat_priv,
 			char is_duplicate)
 {
 	struct neigh_node *neigh_node = NULL, *tmp_neigh_node = NULL;
+	struct orig_node *orig_node_tmp;
 	struct hlist_node *node;
 	int tmp_hna_buff_len;
+	uint8_t bcast_own_sum_orig, bcast_own_sum_neigh;
 
 	bat_dbg(DBG_BATMAN, bat_priv, "update_originator(): "
 		"Searching and updating originator entry of received packet\n");
@@ -352,10 +356,22 @@ static void update_orig(struct bat_priv *bat_priv,
 	/* if the TQ is the same and the link not more symetric we
 	 * won't consider it either */
 	if ((orig_node->router) &&
-	     ((neigh_node->tq_avg == orig_node->router->tq_avg) &&
-	     (orig_node->router->orig_node->bcast_own_sum[if_incoming->if_num]
-	      >= neigh_node->orig_node->bcast_own_sum[if_incoming->if_num])))
-		goto update_hna;
+	     (neigh_node->tq_avg == orig_node->router->tq_avg)) {
+		orig_node_tmp = orig_node->router->orig_node;
+		spin_lock_bh(&orig_node_tmp->ogm_cnt_lock);
+		bcast_own_sum_orig =
+			orig_node_tmp->bcast_own_sum[if_incoming->if_num];
+		spin_unlock_bh(&orig_node_tmp->ogm_cnt_lock);
+
+		orig_node_tmp = neigh_node->orig_node;
+		spin_lock_bh(&orig_node_tmp->ogm_cnt_lock);
+		bcast_own_sum_neigh =
+			orig_node_tmp->bcast_own_sum[if_incoming->if_num];
+		spin_unlock_bh(&orig_node_tmp->ogm_cnt_lock);
+
+		if (bcast_own_sum_orig >= bcast_own_sum_neigh)
+			goto update_hna;
+	}
 
 	update_routes(bat_priv, orig_node, neigh_node,
 		      hna_buff, tmp_hna_buff_len);
@@ -708,10 +724,13 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 				 batman_packet->orig) &&
 		    (batman_packet->seqno - if_incoming_seqno + 2 == 0)) {
 			offset = if_incoming->if_num * NUM_WORDS;
+
+			spin_lock_bh(&orig_neigh_node->ogm_cnt_lock);
 			word = &(orig_neigh_node->bcast_own[offset]);
 			bit_mark(word, 0);
 			orig_neigh_node->bcast_own_sum[if_incoming->if_num] =
 				bit_packet_count(word);
+			spin_unlock_bh(&orig_neigh_node->ogm_cnt_lock);
 		}
 
 		bat_dbg(DBG_BATMAN, bat_priv, "Drop packet: "
diff --git a/batman-adv/types.h b/batman-adv/types.h
index 80413d1..11f6b65 100644
--- a/batman-adv/types.h
+++ b/batman-adv/types.h
@@ -89,6 +89,7 @@ struct orig_node {
 	struct kref refcount;
 	struct bat_priv *bat_priv;
 	unsigned long last_frag_packet;
+	spinlock_t ogm_cnt_lock; /* protects ogm counter */
 	struct {
 		uint8_t candidates;
 		struct neigh_node *selected;
-- 
1.7.2.3

[B.A.T.M.A.N.] [PATCH 5/5] batman-adv: remove orig_hash spinlock

[Marek Lindner]

Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 batman-adv/icmp_socket.c       |   35 +++---
 batman-adv/main.c              |    1 -
 batman-adv/originator.c        |   21 ---
 batman-adv/routing.c           |  313 +++++++++++++++++++++-------------------
 batman-adv/translation-table.c |    4 +
 batman-adv/types.h             |    1 -
 batman-adv/unicast.c           |   96 ++++++++-----
 batman-adv/vis.c               |   59 ++++----
 8 files changed, 270 insertions(+), 260 deletions(-)

diff --git a/batman-adv/icmp_socket.c b/batman-adv/icmp_socket.c
index a9a10ed..3598f2b 100644
--- a/batman-adv/icmp_socket.c
+++ b/batman-adv/icmp_socket.c
@@ -157,10 +157,9 @@ static ssize_t bat_socket_write(struct file *file, const char __user *buff,
 	struct sk_buff *skb;
 	struct icmp_packet_rr *icmp_packet;
 
-	struct orig_node *orig_node;
-	struct batman_if *batman_if;
+	struct orig_node *orig_node = NULL;
+	struct neigh_node *neigh_node = NULL;
 	size_t packet_len = sizeof(struct icmp_packet);
-	uint8_t dstaddr[ETH_ALEN];
 
 	if (len < sizeof(struct icmp_packet)) {
 		bat_dbg(DBG_BATMAN, bat_priv,
@@ -220,49 +219,51 @@ static ssize_t bat_socket_write(struct file *file, const char __user *buff,
 	if (atomic_read(&bat_priv->mesh_state) != MESH_ACTIVE)
 		goto dst_unreach;
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	rcu_read_lock();
 	orig_node = ((struct orig_node *)hash_find(bat_priv->orig_hash,
 						   compare_orig, choose_orig,
 						   icmp_packet->dst));
-	rcu_read_unlock();
 
 	if (!orig_node)
 		goto unlock;
 
-	if (!orig_node->router)
-		goto unlock;
+	kref_get(&orig_node->refcount);
+	neigh_node = orig_node->router;
 
-	batman_if = orig_node->router->if_incoming;
-	memcpy(dstaddr, orig_node->router->addr, ETH_ALEN);
+	if (!neigh_node)
+		goto unlock;
 
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
+	kref_get(&neigh_node->refcount);
+	rcu_read_unlock();
 
-	if (!batman_if)
+	if (!neigh_node->if_incoming)
 		goto dst_unreach;
 
-	if (batman_if->if_status != IF_ACTIVE)
+	if (neigh_node->if_incoming->if_status != IF_ACTIVE)
 		goto dst_unreach;
 
 	memcpy(icmp_packet->orig,
 	       bat_priv->primary_if->net_dev->dev_addr, ETH_ALEN);
 
 	if (packet_len == sizeof(struct icmp_packet_rr))
-		memcpy(icmp_packet->rr, batman_if->net_dev->dev_addr, ETH_ALEN);
-
-
-	send_skb_packet(skb, batman_if, dstaddr);
+		memcpy(icmp_packet->rr,
+		       neigh_node->if_incoming->net_dev->dev_addr, ETH_ALEN);
 
+	send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
 	goto out;
 
 unlock:
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
+	rcu_read_unlock();
 dst_unreach:
 	icmp_packet->msg_type = DESTINATION_UNREACHABLE;
 	bat_socket_add_packet(socket_client, icmp_packet, packet_len);
 free_skb:
 	kfree_skb(skb);
 out:
+	if (neigh_node)
+		kref_put(&neigh_node->refcount, neigh_node_free_ref);
+	if (orig_node)
+		kref_put(&orig_node->refcount, orig_node_free_ref);
 	return len;
 }
 
diff --git a/batman-adv/main.c b/batman-adv/main.c
index b827f6a..cb3c3a3 100644
--- a/batman-adv/main.c
+++ b/batman-adv/main.c
@@ -80,7 +80,6 @@ int mesh_init(struct net_device *soft_iface)
 {
 	struct bat_priv *bat_priv = netdev_priv(soft_iface);
 
-	spin_lock_init(&bat_priv->orig_hash_lock);
 	spin_lock_init(&bat_priv->forw_bat_list_lock);
 	spin_lock_init(&bat_priv->forw_bcast_list_lock);
 	spin_lock_init(&bat_priv->hna_lhash_lock);
diff --git a/batman-adv/originator.c b/batman-adv/originator.c
index 332dce1..899ab0b 100644
--- a/batman-adv/originator.c
+++ b/batman-adv/originator.c
@@ -44,18 +44,15 @@ int originator_init(struct bat_priv *bat_priv)
 	if (bat_priv->orig_hash)
 		return 1;
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	bat_priv->orig_hash = hash_new(1024);
 
 	if (!bat_priv->orig_hash)
 		goto err;
 
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 	start_purge_timer(bat_priv);
 	return 1;
 
 err:
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 	return 0;
 }
 
@@ -146,7 +143,6 @@ void originator_free(struct bat_priv *bat_priv)
 
 	cancel_delayed_work_sync(&bat_priv->orig_work);
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	bat_priv->orig_hash = NULL;
 
 	for (i = 0; i < hash->size; i++) {
@@ -165,7 +161,6 @@ void originator_free(struct bat_priv *bat_priv)
 	}
 
 	hash_destroy(hash);
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 }
 
 void bucket_free_orig_rcu(struct rcu_head *rcu)
@@ -343,8 +338,6 @@ static void _purge_orig(struct bat_priv *bat_priv)
 	if (!hash)
 		return;
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
-
 	/* for all origins... */
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
@@ -369,8 +362,6 @@ static void _purge_orig(struct bat_priv *bat_priv)
 		spin_unlock_bh(list_lock);
 	}
 
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
-
 	gw_node_purge(bat_priv);
 	gw_election(bat_priv);
 
@@ -428,8 +419,6 @@ int orig_seq_print_text(struct seq_file *seq, void *offset)
 		   "Originator", "last-seen", "#", TQ_MAX_VALUE, "Nexthop",
 		   "outgoingIF", "Potential nexthops");
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
-
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
@@ -467,8 +456,6 @@ int orig_seq_print_text(struct seq_file *seq, void *offset)
 		rcu_read_unlock();
 	}
 
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
-
 	if ((batman_count == 0))
 		seq_printf(seq, "No batman nodes in range ...\n");
 
@@ -517,8 +504,6 @@ int orig_hash_add_if(struct batman_if *batman_if, int max_if_num)
 
 	/* resize all orig nodes because orig_node->bcast_own(_sum) depend on
 	 * if_num */
-	spin_lock_bh(&bat_priv->orig_hash_lock);
-
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
@@ -536,12 +521,10 @@ int orig_hash_add_if(struct batman_if *batman_if, int max_if_num)
 		rcu_read_unlock();
 	}
 
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 	return 0;
 
 err:
 	rcu_read_unlock();
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 	return -ENOMEM;
 }
 
@@ -610,8 +593,6 @@ int orig_hash_del_if(struct batman_if *batman_if, int max_if_num)
 
 	/* resize all orig nodes because orig_node->bcast_own(_sum) depend on
 	 * if_num */
-	spin_lock_bh(&bat_priv->orig_hash_lock);
-
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
@@ -648,11 +629,9 @@ int orig_hash_del_if(struct batman_if *batman_if, int max_if_num)
 	rcu_read_unlock();
 
 	batman_if->if_num = -1;
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 	return 0;
 
 err:
 	rcu_read_unlock();
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 	return -ENOMEM;
 }
diff --git a/batman-adv/routing.c b/batman-adv/routing.c
index f0e658a..557e7d7 100644
--- a/batman-adv/routing.c
+++ b/batman-adv/routing.c
@@ -48,8 +48,6 @@ void slide_own_bcast_window(struct batman_if *batman_if)
 	int i;
 	size_t word_index;
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
-
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 
@@ -67,8 +65,6 @@ void slide_own_bcast_window(struct batman_if *batman_if)
 		}
 		rcu_read_unlock();
 	}
-
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 }
 
 static void update_HNA(struct bat_priv *bat_priv, struct orig_node *orig_node,
@@ -785,7 +781,7 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 			   orig_node :
 			   get_orig_node(bat_priv, ethhdr->h_source));
 	if (!orig_neigh_node)
-		goto out_neigh;
+		goto out;
 
 	/* drop packet if sender is not a direct neighbor and if we
 	 * don't route towards it */
@@ -842,7 +838,7 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 				0, hna_buff_len, if_incoming);
 
 out_neigh:
-	if (!is_single_hop_neigh)
+	if ((orig_neigh_node) && (!is_single_hop_neigh))
 		kref_put(&orig_neigh_node->refcount, orig_node_free_ref);
 out:
 	kref_put(&orig_node->refcount, orig_node_free_ref);
@@ -850,7 +846,6 @@ out:
 
 int recv_bat_packet(struct sk_buff *skb, struct batman_if *batman_if)
 {
-	struct bat_priv *bat_priv = netdev_priv(batman_if->soft_iface);
 	struct ethhdr *ethhdr;
 
 	/* drop packet if it has not necessary minimum size */
@@ -877,12 +872,10 @@ int recv_bat_packet(struct sk_buff *skb, struct batman_if *batman_if)
 
 	ethhdr = (struct ethhdr *)skb_mac_header(skb);
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	receive_aggr_bat_packet(ethhdr,
 				skb->data,
 				skb_headlen(skb),
 				batman_if);
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 
 	kfree_skb(skb);
 	return NET_RX_SUCCESS;
@@ -891,12 +884,11 @@ int recv_bat_packet(struct sk_buff *skb, struct batman_if *batman_if)
 static int recv_my_icmp_packet(struct bat_priv *bat_priv,
 			       struct sk_buff *skb, size_t icmp_len)
 {
-	struct orig_node *orig_node;
+	struct orig_node *orig_node = NULL;
+	struct neigh_node *neigh_node = NULL;
 	struct icmp_packet_rr *icmp_packet;
 	struct ethhdr *ethhdr;
-	struct batman_if *batman_if;
-	int ret;
-	uint8_t dstaddr[ETH_ALEN];
+	int ret = NET_RX_DROP;
 
 	icmp_packet = (struct icmp_packet_rr *)skb->data;
 	ethhdr = (struct ethhdr *)skb_mac_header(skb);
@@ -904,61 +896,65 @@ static int recv_my_icmp_packet(struct bat_priv *bat_priv,
 	/* add data to device queue */
 	if (icmp_packet->msg_type != ECHO_REQUEST) {
 		bat_socket_receive_packet(icmp_packet, icmp_len);
-		return NET_RX_DROP;
+		goto out;
 	}
 
 	if (!bat_priv->primary_if)
-		return NET_RX_DROP;
+		goto out;
 
 	/* answer echo request (ping) */
 	/* get routing information */
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	rcu_read_lock();
 	orig_node = ((struct orig_node *)hash_find(bat_priv->orig_hash,
 						   compare_orig, choose_orig,
 						   icmp_packet->orig));
-	rcu_read_unlock();
-	ret = NET_RX_DROP;
+	if (!orig_node)
+		goto unlock;
 
-	if ((orig_node) && (orig_node->router)) {
+	kref_get(&orig_node->refcount);
+	neigh_node = orig_node->router;
 
-		/* don't lock while sending the packets ... we therefore
-		 * copy the required data before sending */
-		batman_if = orig_node->router->if_incoming;
-		memcpy(dstaddr, orig_node->router->addr, ETH_ALEN);
-		spin_unlock_bh(&bat_priv->orig_hash_lock);
+	if (!neigh_node)
+		goto unlock;
 
-		/* create a copy of the skb, if needed, to modify it. */
-		if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
-			return NET_RX_DROP;
+	kref_get(&neigh_node->refcount);
+	rcu_read_unlock();
 
-		icmp_packet = (struct icmp_packet_rr *)skb->data;
-		ethhdr = (struct ethhdr *)skb_mac_header(skb);
+	/* create a copy of the skb, if needed, to modify it. */
+	if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
+		goto out;
 
-		memcpy(icmp_packet->dst, icmp_packet->orig, ETH_ALEN);
-		memcpy(icmp_packet->orig,
-		       bat_priv->primary_if->net_dev->dev_addr, ETH_ALEN);
-		icmp_packet->msg_type = ECHO_REPLY;
-		icmp_packet->ttl = TTL;
+	icmp_packet = (struct icmp_packet_rr *)skb->data;
+	ethhdr = (struct ethhdr *)skb_mac_header(skb);
 
-		send_skb_packet(skb, batman_if, dstaddr);
-		ret = NET_RX_SUCCESS;
+	memcpy(icmp_packet->dst, icmp_packet->orig, ETH_ALEN);
+	memcpy(icmp_packet->orig,
+		bat_priv->primary_if->net_dev->dev_addr, ETH_ALEN);
+	icmp_packet->msg_type = ECHO_REPLY;
+	icmp_packet->ttl = TTL;
 
-	} else
-		spin_unlock_bh(&bat_priv->orig_hash_lock);
+	send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
+	ret = NET_RX_SUCCESS;
+	goto out;
 
+unlock:
+	rcu_read_unlock();
+out:
+	if (neigh_node)
+		kref_put(&neigh_node->refcount, neigh_node_free_ref);
+	if (orig_node)
+		kref_put(&orig_node->refcount, orig_node_free_ref);
 	return ret;
 }
 
 static int recv_icmp_ttl_exceeded(struct bat_priv *bat_priv,
 				  struct sk_buff *skb, size_t icmp_len)
 {
-	struct orig_node *orig_node;
+	struct orig_node *orig_node = NULL;
+	struct neigh_node *neigh_node = NULL;
 	struct icmp_packet *icmp_packet;
 	struct ethhdr *ethhdr;
-	struct batman_if *batman_if;
-	int ret;
-	uint8_t dstaddr[ETH_ALEN];
+	int ret = NET_RX_DROP;
 
 	icmp_packet = (struct icmp_packet *)skb->data;
 	ethhdr = (struct ethhdr *)skb_mac_header(skb);
@@ -968,48 +964,53 @@ static int recv_icmp_ttl_exceeded(struct bat_priv *bat_priv,
 		pr_debug("Warning - can't forward icmp packet from %pM to "
 			 "%pM: ttl exceeded\n", icmp_packet->orig,
 			 icmp_packet->dst);
-		return NET_RX_DROP;
+		goto out;
 	}
 
 	if (!bat_priv->primary_if)
-		return NET_RX_DROP;
+		goto out;
 
 	/* get routing information */
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	rcu_read_lock();
 	orig_node = ((struct orig_node *)
 		     hash_find(bat_priv->orig_hash, compare_orig, choose_orig,
 			       icmp_packet->orig));
-	rcu_read_unlock();
-	ret = NET_RX_DROP;
+	if (!orig_node)
+		goto unlock;
 
-	if ((orig_node) && (orig_node->router)) {
+	kref_get(&orig_node->refcount);
+	neigh_node = orig_node->router;
 
-		/* don't lock while sending the packets ... we therefore
-		 * copy the required data before sending */
-		batman_if = orig_node->router->if_incoming;
-		memcpy(dstaddr, orig_node->router->addr, ETH_ALEN);
-		spin_unlock_bh(&bat_priv->orig_hash_lock);
+	if (!neigh_node)
+		goto unlock;
 
-		/* create a copy of the skb, if needed, to modify it. */
-		if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
-			return NET_RX_DROP;
+	kref_get(&neigh_node->refcount);
+	rcu_read_unlock();
 
-		icmp_packet = (struct icmp_packet *) skb->data;
-		ethhdr = (struct ethhdr *)skb_mac_header(skb);
+	/* create a copy of the skb, if needed, to modify it. */
+	if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
+		goto out;
 
-		memcpy(icmp_packet->dst, icmp_packet->orig, ETH_ALEN);
-		memcpy(icmp_packet->orig,
-		       bat_priv->primary_if->net_dev->dev_addr, ETH_ALEN);
-		icmp_packet->msg_type = TTL_EXCEEDED;
-		icmp_packet->ttl = TTL;
+	icmp_packet = (struct icmp_packet *)skb->data;
+	ethhdr = (struct ethhdr *)skb_mac_header(skb);
 
-		send_skb_packet(skb, batman_if, dstaddr);
-		ret = NET_RX_SUCCESS;
+	memcpy(icmp_packet->dst, icmp_packet->orig, ETH_ALEN);
+	memcpy(icmp_packet->orig,
+		bat_priv->primary_if->net_dev->dev_addr, ETH_ALEN);
+	icmp_packet->msg_type = TTL_EXCEEDED;
+	icmp_packet->ttl = TTL;
 
-	} else
-		spin_unlock_bh(&bat_priv->orig_hash_lock);
+	send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
+	ret = NET_RX_SUCCESS;
+	goto out;
 
+unlock:
+	rcu_read_unlock();
+out:
+	if (neigh_node)
+		kref_put(&neigh_node->refcount, neigh_node_free_ref);
+	if (orig_node)
+		kref_put(&orig_node->refcount, orig_node_free_ref);
 	return ret;
 }
 
@@ -1019,11 +1020,10 @@ int recv_icmp_packet(struct sk_buff *skb, struct batman_if *recv_if)
 	struct bat_priv *bat_priv = netdev_priv(recv_if->soft_iface);
 	struct icmp_packet_rr *icmp_packet;
 	struct ethhdr *ethhdr;
-	struct orig_node *orig_node;
-	struct batman_if *batman_if;
+	struct orig_node *orig_node = NULL;
+	struct neigh_node *neigh_node = NULL;
 	int hdr_size = sizeof(struct icmp_packet);
-	int ret;
-	uint8_t dstaddr[ETH_ALEN];
+	int ret = NET_RX_DROP;
 
 	/**
 	 * we truncate all incoming icmp packets if they don't match our size
@@ -1033,21 +1033,21 @@ int recv_icmp_packet(struct sk_buff *skb, struct batman_if *recv_if)
 
 	/* drop packet if it has not necessary minimum size */
 	if (unlikely(!pskb_may_pull(skb, hdr_size)))
-		return NET_RX_DROP;
+		goto out;
 
 	ethhdr = (struct ethhdr *)skb_mac_header(skb);
 
 	/* packet with unicast indication but broadcast recipient */
 	if (is_broadcast_ether_addr(ethhdr->h_dest))
-		return NET_RX_DROP;
+		goto out;
 
 	/* packet with broadcast sender address */
 	if (is_broadcast_ether_addr(ethhdr->h_source))
-		return NET_RX_DROP;
+		goto out;
 
 	/* not for me */
 	if (!is_my_mac(ethhdr->h_dest))
-		return NET_RX_DROP;
+		goto out;
 
 	icmp_packet = (struct icmp_packet_rr *)skb->data;
 
@@ -1067,41 +1067,45 @@ int recv_icmp_packet(struct sk_buff *skb, struct batman_if *recv_if)
 	if (icmp_packet->ttl < 2)
 		return recv_icmp_ttl_exceeded(bat_priv, skb, hdr_size);
 
-	ret = NET_RX_DROP;
-
 	/* get routing information */
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	rcu_read_lock();
 	orig_node = ((struct orig_node *)
 		     hash_find(bat_priv->orig_hash, compare_orig, choose_orig,
 			       icmp_packet->dst));
-	rcu_read_unlock();
+	if (!orig_node)
+		goto unlock;
 
-	if ((orig_node) && (orig_node->router)) {
+	kref_get(&orig_node->refcount);
+	neigh_node = orig_node->router;
 
-		/* don't lock while sending the packets ... we therefore
-		 * copy the required data before sending */
-		batman_if = orig_node->router->if_incoming;
-		memcpy(dstaddr, orig_node->router->addr, ETH_ALEN);
-		spin_unlock_bh(&bat_priv->orig_hash_lock);
+	if (!neigh_node)
+		goto unlock;
 
-		/* create a copy of the skb, if needed, to modify it. */
-		if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
-			return NET_RX_DROP;
+	kref_get(&neigh_node->refcount);
+	rcu_read_unlock();
 
-		icmp_packet = (struct icmp_packet_rr *)skb->data;
-		ethhdr = (struct ethhdr *)skb_mac_header(skb);
+	/* create a copy of the skb, if needed, to modify it. */
+	if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
+		goto out;
 
-		/* decrement ttl */
-		icmp_packet->ttl--;
+	icmp_packet = (struct icmp_packet_rr *)skb->data;
+	ethhdr = (struct ethhdr *)skb_mac_header(skb);
 
-		/* route it */
-		send_skb_packet(skb, batman_if, dstaddr);
-		ret = NET_RX_SUCCESS;
+	/* decrement ttl */
+	icmp_packet->ttl--;
 
-	} else
-		spin_unlock_bh(&bat_priv->orig_hash_lock);
+	/* route it */
+	send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
+	ret = NET_RX_SUCCESS;
+	goto out;
 
+unlock:
+	rcu_read_unlock();
+out:
+	if (neigh_node)
+		kref_put(&neigh_node->refcount, neigh_node_free_ref);
+	if (orig_node)
+		kref_put(&orig_node->refcount, orig_node_free_ref);
 	return ret;
 }
 
@@ -1231,13 +1235,11 @@ int route_unicast_packet(struct sk_buff *skb, struct batman_if *recv_if,
 			 int hdr_size)
 {
 	struct bat_priv *bat_priv = netdev_priv(recv_if->soft_iface);
-	struct orig_node *orig_node;
-	struct neigh_node *router;
-	struct batman_if *batman_if;
-	uint8_t dstaddr[ETH_ALEN];
+	struct orig_node *orig_node = NULL;
+	struct neigh_node *neigh_node = NULL;
 	struct unicast_packet *unicast_packet;
 	struct ethhdr *ethhdr = (struct ethhdr *)skb_mac_header(skb);
-	int ret;
+	int ret = NET_RX_DROP;
 	struct sk_buff *new_skb;
 
 	unicast_packet = (struct unicast_packet *)skb->data;
@@ -1247,55 +1249,54 @@ int route_unicast_packet(struct sk_buff *skb, struct batman_if *recv_if,
 		pr_debug("Warning - can't forward unicast packet from %pM to "
 			 "%pM: ttl exceeded\n", ethhdr->h_source,
 			 unicast_packet->dest);
-		return NET_RX_DROP;
+		goto out;
 	}
 
 	/* get routing information */
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	rcu_read_lock();
 	orig_node = ((struct orig_node *)
 		     hash_find(bat_priv->orig_hash, compare_orig, choose_orig,
 			       unicast_packet->dest));
-	rcu_read_unlock();
 
-	router = find_router(bat_priv, orig_node, recv_if);
-
-	if (!router) {
-		spin_unlock_bh(&bat_priv->orig_hash_lock);
-		return NET_RX_DROP;
-	}
+	if (!orig_node)
+		goto unlock;
 
-	/* don't lock while sending the packets ... we therefore
-	 * copy the required data before sending */
+	kref_get(&orig_node->refcount);
+	neigh_node = find_router(bat_priv, orig_node, recv_if);
 
-	batman_if = router->if_incoming;
-	memcpy(dstaddr, router->addr, ETH_ALEN);
+	if (!neigh_node)
+		goto unlock;
 
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
+	kref_get(&neigh_node->refcount);
+	rcu_read_unlock();
 
 	/* create a copy of the skb, if needed, to modify it. */
 	if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
-		return NET_RX_DROP;
+		goto out;
 
 	unicast_packet = (struct unicast_packet *)skb->data;
 
 	if (unicast_packet->packet_type == BAT_UNICAST &&
 	    atomic_read(&bat_priv->fragmentation) &&
-	    skb->len > batman_if->net_dev->mtu)
-		return frag_send_skb(skb, bat_priv, batman_if,
-				     dstaddr);
+	    skb->len > neigh_node->if_incoming->net_dev->mtu) {
+		ret = frag_send_skb(skb, bat_priv,
+				    neigh_node->if_incoming, neigh_node->addr);
+		goto out;
+	}
 
 	if (unicast_packet->packet_type == BAT_UNICAST_FRAG &&
-	    2 * skb->len - hdr_size <= batman_if->net_dev->mtu) {
+	    2 * skb->len - hdr_size <= neigh_node->if_incoming->net_dev->mtu) {
 
 		ret = frag_reassemble_skb(skb, bat_priv, &new_skb);
 
 		if (ret == NET_RX_DROP)
-			return NET_RX_DROP;
+			goto out;
 
 		/* packet was buffered for late merge */
-		if (!new_skb)
-			return NET_RX_SUCCESS;
+		if (!new_skb) {
+			ret = NET_RX_SUCCESS;
+			goto out;
+		}
 
 		skb = new_skb;
 		unicast_packet = (struct unicast_packet *)skb->data;
@@ -1305,9 +1306,18 @@ int route_unicast_packet(struct sk_buff *skb, struct batman_if *recv_if,
 	unicast_packet->ttl--;
 
 	/* route it */
-	send_skb_packet(skb, batman_if, dstaddr);
+	send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
+	ret = NET_RX_SUCCESS;
+	goto out;
 
-	return NET_RX_SUCCESS;
+unlock:
+	rcu_read_unlock();
+out:
+	if (neigh_node)
+		kref_put(&neigh_node->refcount, neigh_node_free_ref);
+	if (orig_node)
+		kref_put(&orig_node->refcount, orig_node_free_ref);
+	return ret;
 }
 
 int recv_unicast_packet(struct sk_buff *skb, struct batman_if *recv_if)
@@ -1366,81 +1376,82 @@ int recv_ucast_frag_packet(struct sk_buff *skb, struct batman_if *recv_if)
 int recv_bcast_packet(struct sk_buff *skb, struct batman_if *recv_if)
 {
 	struct bat_priv *bat_priv = netdev_priv(recv_if->soft_iface);
-	struct orig_node *orig_node;
+	struct orig_node *orig_node = NULL;
 	struct bcast_packet *bcast_packet;
 	struct ethhdr *ethhdr;
 	int hdr_size = sizeof(struct bcast_packet);
+	int ret = NET_RX_DROP;
 	int32_t seq_diff;
 
 	/* drop packet if it has not necessary minimum size */
 	if (unlikely(!pskb_may_pull(skb, hdr_size)))
-		return NET_RX_DROP;
+		goto out;
 
 	ethhdr = (struct ethhdr *)skb_mac_header(skb);
 
 	/* packet with broadcast indication but unicast recipient */
 	if (!is_broadcast_ether_addr(ethhdr->h_dest))
-		return NET_RX_DROP;
+		goto out;
 
 	/* packet with broadcast sender address */
 	if (is_broadcast_ether_addr(ethhdr->h_source))
-		return NET_RX_DROP;
+		goto out;
 
 	/* ignore broadcasts sent by myself */
 	if (is_my_mac(ethhdr->h_source))
-		return NET_RX_DROP;
+		goto out;
 
 	bcast_packet = (struct bcast_packet *)skb->data;
 
 	/* ignore broadcasts originated by myself */
 	if (is_my_mac(bcast_packet->orig))
-		return NET_RX_DROP;
+		goto out;
 
 	if (bcast_packet->ttl < 2)
-		return NET_RX_DROP;
+		goto out;
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	rcu_read_lock();
 	orig_node = ((struct orig_node *)
 		     hash_find(bat_priv->orig_hash, compare_orig, choose_orig,
 			       bcast_packet->orig));
-	rcu_read_unlock();
 
-	if (!orig_node) {
-		spin_unlock_bh(&bat_priv->orig_hash_lock);
-		return NET_RX_DROP;
-	}
+	if (!orig_node)
+		goto unlock;
+
+	kref_get(&orig_node->refcount);
+	rcu_read_unlock();
 
 	/* check whether the packet is a duplicate */
-	if (get_bit_status(orig_node->bcast_bits,
-			   orig_node->last_bcast_seqno,
-			   ntohl(bcast_packet->seqno))) {
-		spin_unlock_bh(&bat_priv->orig_hash_lock);
-		return NET_RX_DROP;
-	}
+	if (get_bit_status(orig_node->bcast_bits, orig_node->last_bcast_seqno,
+			   ntohl(bcast_packet->seqno)))
+		goto out;
 
 	seq_diff = ntohl(bcast_packet->seqno) - orig_node->last_bcast_seqno;
 
 	/* check whether the packet is old and the host just restarted. */
 	if (window_protected(bat_priv, seq_diff,
-			     &orig_node->bcast_seqno_reset)) {
-		spin_unlock_bh(&bat_priv->orig_hash_lock);
-		return NET_RX_DROP;
-	}
+			     &orig_node->bcast_seqno_reset))
+		goto out;
 
 	/* mark broadcast in flood history, update window position
 	 * if required. */
 	if (bit_get_packet(bat_priv, orig_node->bcast_bits, seq_diff, 1))
 		orig_node->last_bcast_seqno = ntohl(bcast_packet->seqno);
 
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 	/* rebroadcast packet */
 	add_bcast_packet_to_list(bat_priv, skb);
 
 	/* broadcast for me */
 	interface_rx(recv_if->soft_iface, skb, recv_if, hdr_size);
+	ret = NET_RX_SUCCESS;
+	goto out;
 
-	return NET_RX_SUCCESS;
+unlock:
+	rcu_read_unlock();
+out:
+	if (orig_node)
+		kref_put(&orig_node->refcount, orig_node_free_ref);
+	return ret;
 }
 
 int recv_vis_packet(struct sk_buff *skb, struct batman_if *recv_if)
diff --git a/batman-adv/translation-table.c b/batman-adv/translation-table.c
index 6f6ee58..f299957 100644
--- a/batman-adv/translation-table.c
+++ b/batman-adv/translation-table.c
@@ -538,6 +538,10 @@ struct orig_node *transtable_search(struct bat_priv *bat_priv, uint8_t *addr)
 	hna_global_entry = (struct hna_global_entry *)
 				hash_find(bat_priv->hna_global_hash,
 					  compare_orig, choose_orig, addr);
+
+	if (hna_global_entry)
+		kref_get(&hna_global_entry->orig_node->refcount);
+
 	rcu_read_unlock();
 	spin_unlock_bh(&bat_priv->hna_ghash_lock);
 
diff --git a/batman-adv/types.h b/batman-adv/types.h
index 11f6b65..52b6b08 100644
--- a/batman-adv/types.h
+++ b/batman-adv/types.h
@@ -157,7 +157,6 @@ struct bat_priv {
 	struct hashtable_t *hna_local_hash;
 	struct hashtable_t *hna_global_hash;
 	struct hashtable_t *vis_hash;
-	spinlock_t orig_hash_lock; /* protects orig_hash */
 	spinlock_t forw_bat_list_lock; /* protects forw_bat_list */
 	spinlock_t forw_bcast_list_lock; /* protects  */
 	spinlock_t hna_lhash_lock; /* protects hna_local_hash */
diff --git a/batman-adv/unicast.c b/batman-adv/unicast.c
index 0b2ea82..67bed2d 100644
--- a/batman-adv/unicast.c
+++ b/batman-adv/unicast.c
@@ -178,17 +178,16 @@ int frag_reassemble_skb(struct sk_buff *skb, struct bat_priv *bat_priv,
 		(struct unicast_frag_packet *)skb->data;
 
 	*new_skb = NULL;
-	spin_lock_bh(&bat_priv->orig_hash_lock);
+
 	rcu_read_lock();
 	orig_node = ((struct orig_node *)
 		    hash_find(bat_priv->orig_hash, compare_orig, choose_orig,
 			      unicast_packet->orig));
-	rcu_read_unlock();
+	if (!orig_node)
+		goto unlock;
 
-	if (!orig_node) {
-		pr_debug("couldn't find originator in orig_hash\n");
-		goto out;
-	}
+	kref_get(&orig_node->refcount);
+	rcu_read_unlock();
 
 	orig_node->last_frag_packet = jiffies;
 
@@ -212,9 +211,14 @@ int frag_reassemble_skb(struct sk_buff *skb, struct bat_priv *bat_priv,
 	/* if not, merge failed */
 	if (*new_skb)
 		ret = NET_RX_SUCCESS;
-out:
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 
+	goto out;
+
+unlock:
+	rcu_read_unlock();
+out:
+	if (orig_node)
+		kref_put(&orig_node->refcount, orig_node_free_ref);
 	return ret;
 }
 
@@ -277,47 +281,53 @@ int unicast_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv)
 	struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
 	struct unicast_packet *unicast_packet;
 	struct orig_node *orig_node;
-	struct batman_if *batman_if;
-	struct neigh_node *router;
+	struct neigh_node *neigh_node;
 	int data_len = skb->len;
-	uint8_t dstaddr[6];
-
-	spin_lock_bh(&bat_priv->orig_hash_lock);
+	int ret = 1;
 
 	/* get routing information */
-	if (is_multicast_ether_addr(ethhdr->h_dest))
+	if (is_multicast_ether_addr(ethhdr->h_dest)) {
 		orig_node = (struct orig_node *)gw_get_selected(bat_priv);
-	else {
+		if (!orig_node)
+			goto trans_search;
+
+		kref_get(&orig_node->refcount);
+		goto find_router;
+	} else {
 		rcu_read_lock();
 		orig_node = ((struct orig_node *)hash_find(bat_priv->orig_hash,
 							   compare_orig,
 							   choose_orig,
 							   ethhdr->h_dest));
+		if (!orig_node) {
+			rcu_read_unlock();
+			goto trans_search;
+		}
+
+		kref_get(&orig_node->refcount);
 		rcu_read_unlock();
+		goto find_router;
 	}
 
-	/* check for hna host */
-	if (!orig_node)
-		orig_node = transtable_search(bat_priv, ethhdr->h_dest);
+trans_search:
+	/* check for hna host - increases orig_node refcount */
+	orig_node = transtable_search(bat_priv, ethhdr->h_dest);
 
-	router = find_router(bat_priv, orig_node, NULL);
+find_router:
+	rcu_read_lock();
+	neigh_node = find_router(bat_priv, orig_node, NULL);
 
-	if (!router)
+	if (!neigh_node)
 		goto unlock;
 
-	/* don't lock while sending the packets ... we therefore
-		* copy the required data before sending */
-
-	batman_if = router->if_incoming;
-	memcpy(dstaddr, router->addr, ETH_ALEN);
-
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
+	kref_get(&neigh_node->refcount);
+	rcu_read_unlock();
 
-	if (batman_if->if_status != IF_ACTIVE)
-		goto dropped;
+	if (neigh_node->if_incoming->if_status != IF_ACTIVE)
+		goto out;
 
 	if (my_skb_head_push(skb, sizeof(struct unicast_packet)) < 0)
-		goto dropped;
+		goto out;
 
 	unicast_packet = (struct unicast_packet *)skb->data;
 
@@ -331,18 +341,26 @@ int unicast_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv)
 
 	if (atomic_read(&bat_priv->fragmentation) &&
 	    data_len + sizeof(struct unicast_packet) >
-	    batman_if->net_dev->mtu) {
+				neigh_node->if_incoming->net_dev->mtu) {
 		/* send frag skb decreases ttl */
 		unicast_packet->ttl++;
-		return frag_send_skb(skb, bat_priv, batman_if,
-				     dstaddr);
+		ret = frag_send_skb(skb, bat_priv,
+				    neigh_node->if_incoming, neigh_node->addr);
+		goto out;
 	}
-	send_skb_packet(skb, batman_if, dstaddr);
-	return 0;
+
+	send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
+	ret = 0;
+	goto out;
 
 unlock:
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
-dropped:
-	kfree_skb(skb);
-	return 1;
+	rcu_read_unlock();
+out:
+	if (neigh_node)
+		kref_put(&neigh_node->refcount, neigh_node_free_ref);
+	if (orig_node)
+		kref_put(&orig_node->refcount, orig_node_free_ref);
+	if (ret == 1)
+		kfree_skb(skb);
+	return ret;
 }
diff --git a/batman-adv/vis.c b/batman-adv/vis.c
index 230e66b..69178d9 100644
--- a/batman-adv/vis.c
+++ b/batman-adv/vis.c
@@ -590,7 +590,6 @@ static int generate_vis_packet(struct bat_priv *bat_priv)
 	info->first_seen = jiffies;
 	packet->vis_type = atomic_read(&bat_priv->vis_mode);
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	memcpy(packet->target_orig, broadcast_addr, ETH_ALEN);
 	packet->ttl = TTL;
 	packet->seqno = htonl(ntohl(packet->seqno) + 1);
@@ -600,10 +599,8 @@ static int generate_vis_packet(struct bat_priv *bat_priv)
 	if (packet->vis_type == VIS_TYPE_CLIENT_UPDATE) {
 		best_tq = find_best_vis_server(bat_priv, info);
 
-		if (best_tq < 0) {
-			spin_unlock_bh(&bat_priv->orig_hash_lock);
+		if (best_tq < 0)
 			return -1;
-		}
 	}
 
 	for (i = 0; i < hash->size; i++) {
@@ -636,17 +633,12 @@ static int generate_vis_packet(struct bat_priv *bat_priv)
 			entry->quality = neigh_node->tq_avg;
 			packet->entries++;
 
-			if (vis_packet_full(info)) {
-				rcu_read_unlock();
-				spin_unlock_bh(&bat_priv->orig_hash_lock);
-				return 0;
-			}
+			if (vis_packet_full(info))
+				goto unlock;
 		}
 		rcu_read_unlock();
 	}
 
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
-
 	hash = bat_priv->hna_local_hash;
 
 	spin_lock_bh(&bat_priv->hna_lhash_lock);
@@ -672,6 +664,10 @@ static int generate_vis_packet(struct bat_priv *bat_priv)
 
 	spin_unlock_bh(&bat_priv->hna_lhash_lock);
 	return 0;
+
+unlock:
+	rcu_read_unlock();
+	return 0;
 }
 
 /* free old vis packets. Must be called with this vis_hash_lock
@@ -721,7 +717,6 @@ static void broadcast_vis_packet(struct bat_priv *bat_priv,
 	int i;
 
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	packet = (struct vis_packet *)info->skb_packet->data;
 
 	/* send to all routers in range. */
@@ -746,54 +741,58 @@ static void broadcast_vis_packet(struct bat_priv *bat_priv,
 			memcpy(packet->target_orig, orig_node->orig, ETH_ALEN);
 			batman_if = orig_node->router->if_incoming;
 			memcpy(dstaddr, orig_node->router->addr, ETH_ALEN);
-			spin_unlock_bh(&bat_priv->orig_hash_lock);
 
 			skb = skb_clone(info->skb_packet, GFP_ATOMIC);
 			if (skb)
 				send_skb_packet(skb, batman_if, dstaddr);
 
-			spin_lock_bh(&bat_priv->orig_hash_lock);
 		}
 		rcu_read_unlock();
 	}
-
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
 }
 
 static void unicast_vis_packet(struct bat_priv *bat_priv,
 			       struct vis_info *info)
 {
 	struct orig_node *orig_node;
+	struct neigh_node *neigh_node = NULL;
 	struct sk_buff *skb;
 	struct vis_packet *packet;
-	struct batman_if *batman_if;
-	uint8_t dstaddr[ETH_ALEN];
 
-	spin_lock_bh(&bat_priv->orig_hash_lock);
 	packet = (struct vis_packet *)info->skb_packet->data;
+
 	rcu_read_lock();
 	orig_node = ((struct orig_node *)hash_find(bat_priv->orig_hash,
 						   compare_orig, choose_orig,
 						   packet->target_orig));
-	rcu_read_unlock();
 
-	if ((!orig_node) || (!orig_node->router))
-		goto out;
+	if (!orig_node)
+		goto unlock;
 
-	/* don't lock while sending the packets ... we therefore
-	 * copy the required data before sending */
-	batman_if = orig_node->router->if_incoming;
-	memcpy(dstaddr, orig_node->router->addr, ETH_ALEN);
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
+	kref_get(&orig_node->refcount);
+	neigh_node = orig_node->router;
+
+	if (!neigh_node)
+		goto unlock;
+
+	kref_get(&neigh_node->refcount);
+	rcu_read_unlock();
 
 	skb = skb_clone(info->skb_packet, GFP_ATOMIC);
 	if (skb)
-		send_skb_packet(skb, batman_if, dstaddr);
+		send_skb_packet(skb, neigh_node->if_incoming,
+				neigh_node->addr);
 
-	return;
+	goto out;
 
+unlock:
+	rcu_read_unlock();
 out:
-	spin_unlock_bh(&bat_priv->orig_hash_lock);
+	if (neigh_node)
+		kref_put(&neigh_node->refcount, neigh_node_free_ref);
+	if (orig_node)
+		kref_put(&orig_node->refcount, orig_node_free_ref);
+	return;
 }
 
 /* only send one vis packet. called from send_vis_packets() */
-- 
1.7.2.3

[B.A.T.M.A.N.] [PATCH] batman-adv: protect bonding with rcu locks

[Simon Wunderlich]

bonding / alternating candidates need to be secured by rcu locks
as well. This patch therefore converts the bonding list
from a plain pointer list to a rcu securable lists and references
the bonding candidates.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
 originator.c |   17 +++++++-
 routing.c    |  140 +++++++++++++++++++++++++++++++++-------------------------
 types.h      |    4 +-
 unicast.c    |    9 +---
 4 files changed, 100 insertions(+), 70 deletions(-)

diff --git a/batman-adv/originator.c b/batman-adv/originator.c
index 899ab0b..3e18488 100644
--- a/batman-adv/originator.c
+++ b/batman-adv/originator.c
@@ -88,6 +88,7 @@ struct neigh_node *create_neighbor(struct orig_node *orig_node,
 		return NULL;
 
 	INIT_HLIST_NODE(&neigh_node->list);
+	INIT_LIST_HEAD(&neigh_node->bonding_list);
 
 	memcpy(neigh_node->addr, neigh, ETH_ALEN);
 	neigh_node->orig_node = orig_neigh_node;
@@ -103,13 +104,20 @@ struct neigh_node *create_neighbor(struct orig_node *orig_node,
 void orig_node_free_ref(struct kref *refcount)
 {
 	struct hlist_node *node, *node_tmp;
-	struct neigh_node *neigh_node;
+	struct neigh_node *neigh_node, *tmp_neigh_node;
 	struct orig_node *orig_node;
 
 	orig_node = container_of(refcount, struct orig_node, refcount);
 
 	spin_lock_bh(&orig_node->neigh_list_lock);
 
+	/* for all bonding members ... */
+	list_for_each_entry_safe(neigh_node, tmp_neigh_node,
+				 &orig_node->bond.selected, bonding_list) {
+		list_del_rcu(&neigh_node->bonding_list);
+		call_rcu(&neigh_node->rcu, neigh_node_free_rcu);
+	}
+
 	/* for all neighbors towards this originator ... */
 	hlist_for_each_entry_safe(neigh_node, node, node_tmp,
 				  &orig_node->neigh_list, list) {
@@ -202,6 +210,7 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)
 		return NULL;
 
 	INIT_HLIST_HEAD(&orig_node->neigh_list);
+	INIT_LIST_HEAD(&orig_node->bond.selected);
 	spin_lock_init(&orig_node->ogm_cnt_lock);
 	spin_lock_init(&orig_node->neigh_list_lock);
 	kref_init(&orig_node->refcount);
@@ -285,6 +294,12 @@ static bool purge_orig_neighbors(struct bat_priv *bat_priv,
 			neigh_purged = true;
 
 			hlist_del_rcu(&neigh_node->list);
+
+			if (!list_empty(&neigh_node->bonding_list)) {
+				orig_node->bond.candidates--;
+				list_del_rcu(&neigh_node->bonding_list);
+				call_rcu(&neigh_node->rcu, neigh_node_free_rcu);
+			}
 			call_rcu(&neigh_node->rcu, neigh_node_free_rcu);
 		} else {
 			if ((!*best_neigh_node) ||
diff --git a/batman-adv/routing.c b/batman-adv/routing.c
index 557e7d7..ad8d237 100644
--- a/batman-adv/routing.c
+++ b/batman-adv/routing.c
@@ -517,7 +517,6 @@ void update_bonding_candidates(struct bat_priv *bat_priv,
 	int best_tq;
 	struct hlist_node *node, *node2;
 	struct neigh_node *tmp_neigh_node, *tmp_neigh_node2;
-	struct neigh_node *first_candidate, *last_candidate;
 
 	/* update the candidates for this originator */
 	if (!orig_node->router) {
@@ -525,6 +524,7 @@ void update_bonding_candidates(struct bat_priv *bat_priv,
 		return;
 	}
 
+	spin_lock_bh(&orig_node->neigh_list_lock);
 	best_tq = orig_node->router->tq_avg;
 
 	/* update bond.candidates */
@@ -535,19 +535,14 @@ void update_bonding_candidates(struct bat_priv *bat_priv,
 	 * as "bonding partner" */
 
 	/* first, zero the list */
-	rcu_read_lock();
-	hlist_for_each_entry_rcu(tmp_neigh_node, node,
-				 &orig_node->neigh_list, list) {
-		tmp_neigh_node->next_bond_candidate = NULL;
+	list_for_each_entry_safe(tmp_neigh_node, tmp_neigh_node2,
+				 &orig_node->bond.selected, bonding_list) {
+		list_del_rcu(&tmp_neigh_node->bonding_list);
+		kref_put(&tmp_neigh_node->refcount, neigh_node_free_ref);
 	}
-	rcu_read_unlock();
 
-	first_candidate = NULL;
-	last_candidate = NULL;
-
-	rcu_read_lock();
 	hlist_for_each_entry_rcu(tmp_neigh_node, node,
-				 &orig_node->neigh_list, list) {
+		&orig_node->neigh_list, list) {
 
 		/* only consider if it has the same primary address ...  */
 		if (memcmp(orig_node->orig,
@@ -572,7 +567,7 @@ void update_bonding_candidates(struct bat_priv *bat_priv,
 
 			/* we only care if the other candidate is even
 			 * considered as candidate. */
-			if (!tmp_neigh_node2->next_bond_candidate)
+			if (!list_empty(&tmp_neigh_node2->bonding_list))
 				continue;
 
 
@@ -589,24 +584,16 @@ void update_bonding_candidates(struct bat_priv *bat_priv,
 		if (interference_candidate)
 			continue;
 
-		if (!first_candidate) {
-			first_candidate = tmp_neigh_node;
-			tmp_neigh_node->next_bond_candidate = first_candidate;
-		} else
-			tmp_neigh_node->next_bond_candidate = last_candidate;
-
-		last_candidate = tmp_neigh_node;
+		list_add_rcu(&tmp_neigh_node->bonding_list,
+				&orig_node->bond.selected);
+		kref_get(&tmp_neigh_node->refcount);
 
 		candidates++;
 	}
-	rcu_read_unlock();
-
-	if (candidates > 0) {
-		first_candidate->next_bond_candidate = last_candidate;
-		orig_node->bond.selected = first_candidate;
-	}
-
 	orig_node->bond.candidates = candidates;
+
+	spin_unlock_bh(&orig_node->neigh_list_lock);
+
 }
 
 void receive_bat_packet(struct ethhdr *ethhdr,
@@ -1110,16 +1097,18 @@ out:
 }
 
 /* find a suitable router for this originator, and use
- * bonding if possible. */
+ * bonding if possible. increases the found neighbors
+ * refcount.*/
 struct neigh_node *find_router(struct bat_priv *bat_priv,
 			       struct orig_node *orig_node,
 			       struct batman_if *recv_if)
 {
 	struct orig_node *primary_orig_node;
 	struct orig_node *router_orig;
-	struct neigh_node *router, *first_candidate, *best_router;
+	struct neigh_node *router, *first_candidate, *tmp_neigh_node;
 	static uint8_t zero_mac[ETH_ALEN] = {0, 0, 0, 0, 0, 0};
 	int bonding_enabled;
+	int best_router_tq;
 
 	if (!orig_node)
 		return NULL;
@@ -1132,15 +1121,23 @@ struct neigh_node *find_router(struct bat_priv *bat_priv,
 
 	bonding_enabled = atomic_read(&bat_priv->bonding);
 
-	if ((!recv_if) && (!bonding_enabled))
-		return orig_node->router;
-
+	rcu_read_lock();
+	/* select default router to output */
+	router = orig_node->router;
 	router_orig = orig_node->router->orig_node;
+	if (!router_orig) {
+		rcu_read_unlock();
+		return NULL;
+	}
+
+
+	if ((!recv_if) && (!bonding_enabled))
+		goto return_router;
 
 	/* if we have something in the primary_addr, we can search
 	 * for a potential bonding candidate. */
 	if (memcmp(router_orig->primary_addr, zero_mac, ETH_ALEN) == 0)
-		return orig_node->router;
+		goto return_router;
 
 	/* find the orig_node which has the primary interface. might
 	 * even be the same as our router_orig in many cases */
@@ -1149,60 +1146,83 @@ struct neigh_node *find_router(struct bat_priv *bat_priv,
 				router_orig->orig, ETH_ALEN) == 0) {
 		primary_orig_node = router_orig;
 	} else {
-		rcu_read_lock();
 		primary_orig_node = hash_find(bat_priv->orig_hash, compare_orig,
 					       choose_orig,
 					       router_orig->primary_addr);
-		rcu_read_unlock();
-
 		if (!primary_orig_node)
-			return orig_node->router;
+			goto return_router;
 	}
-
 	/* with less than 2 candidates, we can't do any
 	 * bonding and prefer the original router. */
 
 	if (primary_orig_node->bond.candidates < 2)
-		return orig_node->router;
+		goto return_router;
 
 
 	/* all nodes between should choose a candidate which
 	 * is is not on the interface where the packet came
 	 * in. */
-	first_candidate = primary_orig_node->bond.selected;
-	router = first_candidate;
+
+	first_candidate = NULL;
+	router = NULL;
 
 	if (bonding_enabled) {
 		/* in the bonding case, send the packets in a round
 		 * robin fashion over the remaining interfaces. */
-		do {
+
+		list_for_each_entry_rcu(tmp_neigh_node,
+			&primary_orig_node->bond.selected, bonding_list) {
+			if (!first_candidate)
+				first_candidate = tmp_neigh_node;
 			/* recv_if == NULL on the first node. */
-			if (router->if_incoming != recv_if)
+			if (tmp_neigh_node->if_incoming != recv_if) {
+				router = tmp_neigh_node;
 				break;
+			}
+		}
 
-			router = router->next_bond_candidate;
-		} while (router != first_candidate);
+		/* use the first candidate if nothing was found. */
+		if (!router)
+			router = first_candidate;
 
-		primary_orig_node->bond.selected = router->next_bond_candidate;
+		/* selected should point to the next element
+		 * after the current router */
+		spin_lock_bh(&primary_orig_node->neigh_list_lock);
+		/* this is a list_move(), which unfortunately
+		 * does not exist as rcu version */
+		list_del_rcu(&primary_orig_node->bond.selected);
+		list_add_rcu(&primary_orig_node->bond.selected,
+				&router->bonding_list);
+		spin_unlock_bh(&primary_orig_node->neigh_list_lock);
 
 	} else {
 		/* if bonding is disabled, use the best of the
 		 * remaining candidates which are not using
 		 * this interface. */
-		best_router = first_candidate;
+		best_router_tq = 0;
+		list_for_each_entry_rcu(tmp_neigh_node,
+			&primary_orig_node->bond.selected, bonding_list) {
+			if (!first_candidate)
+				first_candidate = tmp_neigh_node;
 
-		do {
 			/* recv_if == NULL on the first node. */
-			if ((router->if_incoming != recv_if) &&
-				(router->tq_avg > best_router->tq_avg))
-					best_router = router;
+			if (tmp_neigh_node->if_incoming != recv_if)
+				/* if we don't have a router yet
+				 * or this one is better, choose it. */
+				if ((!router) ||
+				(tmp_neigh_node->tq_avg > router->tq_avg)) {
+					router = tmp_neigh_node;
+					best_router_tq = 0;
+				}
+		}
 
-			router = router->next_bond_candidate;
-		} while (router != first_candidate);
-
-		router = best_router;
+		/* use the first candidate if nothing was found. */
+		if (!router)
+			router = first_candidate;
 	}
-
+return_router:
+	kref_get(&router->refcount);
+	rcu_read_unlock();
 	return router;
 }
 
@@ -1210,7 +1230,7 @@ static int check_unicast_packet(struct sk_buff *skb, int hdr_size)
 {
 	struct ethhdr *ethhdr;
 
-	/* drop packet if it has not necessary minimum size */
+/* drop packet if it has not necessary minimum size */
 	if (unlikely(!pskb_may_pull(skb, hdr_size)))
 		return -1;
 
@@ -1262,13 +1282,13 @@ int route_unicast_packet(struct sk_buff *skb, struct batman_if *recv_if,
 		goto unlock;
 
 	kref_get(&orig_node->refcount);
+	rcu_read_unlock();
+
+	/* find_router() increases neigh_nodes refcount if found. */
 	neigh_node = find_router(bat_priv, orig_node, recv_if);
 
 	if (!neigh_node)
-		goto unlock;
-
-	kref_get(&neigh_node->refcount);
-	rcu_read_unlock();
+		goto out;
 
 	/* create a copy of the skb, if needed, to modify it. */
 	if (skb_cow(skb, sizeof(struct ethhdr)) < 0)
diff --git a/batman-adv/types.h b/batman-adv/types.h
index 52b6b08..8264050 100644
--- a/batman-adv/types.h
+++ b/batman-adv/types.h
@@ -92,7 +92,7 @@ struct orig_node {
 	spinlock_t ogm_cnt_lock; /* protects ogm counter */
 	struct {
 		uint8_t candidates;
-		struct neigh_node *selected;
+		struct list_head selected;
 	} bond;
 };
 
@@ -116,7 +116,7 @@ struct neigh_node {
 	uint8_t tq_index;
 	uint8_t tq_avg;
 	uint8_t last_ttl;
-	struct neigh_node *next_bond_candidate;
+	struct list_head bonding_list;
 	unsigned long last_valid;
 	unsigned long real_bits[NUM_WORDS];
 	struct kref refcount;
diff --git a/batman-adv/unicast.c b/batman-adv/unicast.c
index 67bed2d..cfe08dd 100644
--- a/batman-adv/unicast.c
+++ b/batman-adv/unicast.c
@@ -314,14 +314,11 @@ trans_search:
 	orig_node = transtable_search(bat_priv, ethhdr->h_dest);
 
 find_router:
-	rcu_read_lock();
+	/* find_router() increases neigh_nodes refcount if found. */
 	neigh_node = find_router(bat_priv, orig_node, NULL);
 
 	if (!neigh_node)
-		goto unlock;
-
-	kref_get(&neigh_node->refcount);
-	rcu_read_unlock();
+		goto out;
 
 	if (neigh_node->if_incoming->if_status != IF_ACTIVE)
 		goto out;
@@ -353,8 +350,6 @@ find_router:
 	ret = 0;
 	goto out;
 
-unlock:
-	rcu_read_unlock();
 out:
 	if (neigh_node)
 		kref_put(&neigh_node->refcount, neigh_node_free_ref);
-- 
1.7.2.3

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: protect bonding with rcu locks

[Marek Lindner]

On Thursday 30 December 2010 03:09:17 Simon Wunderlich wrote:
> bonding / alternating candidates need to be secured by rcu locks
> as well. This patch therefore converts the bonding list
> from a plain pointer list to a rcu securable lists and references
> the bonding candidates.

Thanks for your patch! As the bonding / alternating candidate list is the last 
item on the way to an orig_hash_lock free kernel module this patch is a very 
welcome one. However, first tests revealed some memory leaks due to misbehaving 
reference counters. Therefore I took the time to dive deeper into this code 
section and propose an addition to your patch. The changes include:

* The bonding / alternating candidate list is not destroyed & re-created with 
each incoming OGM but candidates are added & deleted in a dynamic fashion.
* The memory leaks have been fixed / a workaround has been found. The "free 
neighbors when an interface is deactivated" patch is also required to get the 
full benefit of these memory leak fixes. 
* I introduced some style changes which are meant to increase readability & 
maintainability. Feedback is welcome.  ;-)

Note: One of the memory leaks turned out to be part of a bigger problem which 
we may or may not have in other code sections as well. purge_orig_neighbors() 
called call_rcu() on the same struct more then once in a short time period, so 
that each consecutive call overwrote the previous one. We have to look into 
this.

While reorganizing the code I stumbled over these lines which are part of the 
interference check:

/* we only care if the other candidate is even considered as candidate. */
if (!list_empty(&tmp_neigh_node2->bonding_list))
         continue;

This seems wrong in this context. If the list is empty then this neighbor is 
not part of the candidate list. Wouldn't you agree ?

Regards,
Marek

[B.A.T.M.A.N.] [PATCH] batman-adv: protect bonding with rcu locks (proposal)

[Marek Lindner]

Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 batman-adv/hard-interface.c |    2 +-
 batman-adv/originator.c     |   26 +++---
 batman-adv/originator.h     |    1 +
 batman-adv/routing.c        |  223 +++++++++++++++++++++----------------------
 batman-adv/routing.h        |    6 +-
 batman-adv/types.h          |    7 +-
 6 files changed, 132 insertions(+), 133 deletions(-)

diff --git a/batman-adv/hard-interface.c b/batman-adv/hard-interface.c
index 4f95777..3ab9a20 100644
--- a/batman-adv/hard-interface.c
+++ b/batman-adv/hard-interface.c
@@ -265,7 +265,7 @@ static void hardif_activate_interface(struct batman_if *batman_if)
 static void hardif_deactivate_interface(struct batman_if *batman_if)
 {
 	if ((batman_if->if_status != IF_ACTIVE) &&
-	   (batman_if->if_status != IF_TO_BE_ACTIVATED))
+	    (batman_if->if_status != IF_TO_BE_ACTIVATED))
 		return;
 
 	batman_if->if_status = IF_INACTIVE;
diff --git a/batman-adv/originator.c b/batman-adv/originator.c
index a3a275a..35ab3ab 100644
--- a/batman-adv/originator.c
+++ b/batman-adv/originator.c
@@ -72,6 +72,14 @@ static void neigh_node_free_rcu(struct rcu_head *rcu)
 	kref_put(&neigh_node->refcount, neigh_node_free_ref);
 }
 
+void neigh_node_free_rcu_bond(struct rcu_head *rcu)
+{
+	struct neigh_node *neigh_node;
+
+	neigh_node = container_of(rcu, struct neigh_node, rcu_bond);
+	kref_put(&neigh_node->refcount, neigh_node_free_ref);
+}
+
 struct neigh_node *create_neighbor(struct orig_node *orig_node,
 				   struct orig_node *orig_neigh_node,
 				   uint8_t *neigh,
@@ -113,9 +121,9 @@ void orig_node_free_ref(struct kref *refcount)
 
 	/* for all bonding members ... */
 	list_for_each_entry_safe(neigh_node, tmp_neigh_node,
-				 &orig_node->bond.selected, bonding_list) {
+				 &orig_node->bond_list, bonding_list) {
 		list_del_rcu(&neigh_node->bonding_list);
-		call_rcu(&neigh_node->rcu, neigh_node_free_rcu);
+		call_rcu(&neigh_node->rcu_bond, neigh_node_free_rcu_bond);
 	}
 
 	/* for all neighbors towards this originator ... */
@@ -210,7 +218,7 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)
 		return NULL;
 
 	INIT_HLIST_HEAD(&orig_node->neigh_list);
-	INIT_LIST_HEAD(&orig_node->bond.selected);
+	INIT_LIST_HEAD(&orig_node->bond_list);
 	spin_lock_init(&orig_node->ogm_cnt_lock);
 	spin_lock_init(&orig_node->neigh_list_lock);
 	kref_init(&orig_node->refcount);
@@ -224,6 +232,8 @@ struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr)
 	orig_node->batman_seqno_reset = jiffies - 1
 					- msecs_to_jiffies(RESET_PROTECTION_MS);
 
+	atomic_set(&orig_node->bond_candidates, 0);
+
 	size = bat_priv->num_ifaces * sizeof(unsigned long) * NUM_WORDS;
 
 	orig_node->bcast_own = kzalloc(size, GFP_ATOMIC);
@@ -299,12 +309,7 @@ static bool purge_orig_neighbors(struct bat_priv *bat_priv,
 			neigh_purged = true;
 
 			hlist_del_rcu(&neigh_node->list);
-
-			if (!list_empty(&neigh_node->bonding_list)) {
-				orig_node->bond.candidates--;
-				list_del_rcu(&neigh_node->bonding_list);
-				call_rcu(&neigh_node->rcu, neigh_node_free_rcu);
-			}
+			bonding_candidate_del(orig_node, neigh_node);
 			call_rcu(&neigh_node->rcu, neigh_node_free_rcu);
 		} else {
 			if ((!*best_neigh_node) ||
@@ -336,9 +341,6 @@ static bool purge_orig_node(struct bat_priv *bat_priv,
 				      best_neigh_node,
 				      orig_node->hna_buff,
 				      orig_node->hna_buff_len);
-			/* update bonding candidates, we could have lost
-			 * some candidates. */
-			update_bonding_candidates(orig_node);
 		}
 	}
 
diff --git a/batman-adv/originator.h b/batman-adv/originator.h
index f5a6964..7739b22 100644
--- a/batman-adv/originator.h
+++ b/batman-adv/originator.h
@@ -26,6 +26,7 @@ int originator_init(struct bat_priv *bat_priv);
 void originator_free(struct bat_priv *bat_priv);
 void purge_orig_ref(struct bat_priv *bat_priv);
 void orig_node_free_ref(struct kref *refcount);
+void neigh_node_free_rcu_bond(struct rcu_head *rcu);
 struct orig_node *get_orig_node(struct bat_priv *bat_priv, uint8_t *addr);
 struct neigh_node *create_neighbor(struct orig_node *orig_node,
 				   struct orig_node *orig_neigh_node,
diff --git a/batman-adv/routing.c b/batman-adv/routing.c
index 3d16261..a90d105 100644
--- a/batman-adv/routing.c
+++ b/batman-adv/routing.c
@@ -268,6 +268,101 @@ out:
 	return ret;
 }
 
+/* caller must hold the neigh_list_lock */
+void bonding_candidate_del(struct orig_node *orig_node,
+			   struct neigh_node *neigh_node)
+{
+	/* this neighbor is not part of our candidate list */
+	if (list_empty(&neigh_node->bonding_list))
+		goto out;
+
+	list_del_rcu(&neigh_node->bonding_list);
+	call_rcu(&neigh_node->rcu_bond, neigh_node_free_rcu_bond);
+	INIT_LIST_HEAD(&neigh_node->bonding_list);
+	atomic_dec(&orig_node->bond_candidates);
+
+out:
+	return;
+}
+
+static void bonding_candidate_add(struct orig_node *orig_node,
+				  struct neigh_node *neigh_node)
+{
+	struct hlist_node *node;
+	struct neigh_node *tmp_neigh_node;
+	uint8_t best_tq, interference_candidate = 0;
+
+	spin_lock_bh(&orig_node->neigh_list_lock);
+
+	/* only consider if it has the same primary address ...  */
+	if (!compare_orig(orig_node->orig,
+			  neigh_node->orig_node->primary_addr))
+		goto candidate_del;
+
+	if (!orig_node->router)
+		goto candidate_del;
+
+	best_tq = orig_node->router->tq_avg;
+
+	/* ... and is good enough to be considered */
+	if (neigh_node->tq_avg < best_tq - BONDING_TQ_THRESHOLD)
+		goto candidate_del;
+
+	/**
+	 * check if we have another candidate with the same mac address or
+	 * interface. If we do, we won't select this candidate because of
+	 * possible interference.
+	 */
+	hlist_for_each_entry_rcu(tmp_neigh_node, node,
+				 &orig_node->neigh_list, list) {
+
+		if (tmp_neigh_node == neigh_node)
+			continue;
+
+		/* we only care if the other candidate is even
+		* considered as candidate. */
+		if (list_empty(&tmp_neigh_node->bonding_list))
+			continue;
+
+		if ((neigh_node->if_incoming == tmp_neigh_node->if_incoming) ||
+		    (compare_orig(neigh_node->addr, tmp_neigh_node->addr))) {
+			interference_candidate = 1;
+			break;
+		}
+	}
+
+	/* don't care further if it is an interference candidate */
+	if (interference_candidate)
+		goto candidate_del;
+
+	/* this neighbor already is part of our candidate list */
+	if (!list_empty(&neigh_node->bonding_list))
+		goto out;
+
+	list_add_rcu(&neigh_node->bonding_list, &orig_node->bond_list);
+	kref_get(&neigh_node->refcount);
+	atomic_inc(&orig_node->bond_candidates);
+	goto out;
+
+candidate_del:
+	bonding_candidate_del(orig_node, neigh_node);
+
+out:
+	spin_unlock_bh(&orig_node->neigh_list_lock);
+	return;
+}
+
+/* copy primary address for bonding */
+static void bonding_save_primary(struct orig_node *orig_node,
+				 struct orig_node *orig_neigh_node,
+				 struct batman_packet *batman_packet)
+{
+	if (!(batman_packet->flags & PRIMARIES_FIRST_HOP))
+		return;
+
+	memcpy(orig_neigh_node->primary_addr, orig_node->orig, ETH_ALEN);
+}
+
 static void update_orig(struct bat_priv *bat_priv,
 			struct orig_node *orig_node,
 			struct ethhdr *ethhdr,
@@ -336,6 +431,8 @@ static void update_orig(struct bat_priv *bat_priv,
 		neigh_node->last_ttl = batman_packet->ttl;
 	}
 
+	bonding_candidate_add(orig_node, neigh_node);
+
 	tmp_hna_buff_len = (hna_buff_len > batman_packet->num_hna * ETH_ALEN ?
 			    batman_packet->num_hna * ETH_ALEN : hna_buff_len);
 
@@ -494,110 +591,10 @@ err:
 	return -1;
 }
 
-/* copy primary address for bonding */
-static void mark_bonding_address(struct orig_node *orig_node,
-				 struct orig_node *orig_neigh_node,
-				 struct batman_packet *batman_packet)
-
-{
-	if (batman_packet->flags & PRIMARIES_FIRST_HOP)
-		memcpy(orig_neigh_node->primary_addr,
-		       orig_node->orig, ETH_ALEN);
-
-	return;
-}
-
-/* mark possible bond.candidates in the neighbor list */
-void update_bonding_candidates(struct orig_node *orig_node)
-{
-	int candidates;
-	int interference_candidate;
-	int best_tq;
-	struct hlist_node *node, *node2;
-	struct neigh_node *tmp_neigh_node, *tmp_neigh_node2;
-
-	/* update the candidates for this originator */
-	if (!orig_node->router) {
-		orig_node->bond.candidates = 0;
-		return;
-	}
-
-	spin_lock_bh(&orig_node->neigh_list_lock);
-	best_tq = orig_node->router->tq_avg;
-
-	/* update bond.candidates */
-
-	candidates = 0;
-
-	/* mark other nodes which also received "PRIMARIES FIRST HOP" packets
-	 * as "bonding partner" */
-
-	/* first, zero the list */
-	list_for_each_entry_safe(tmp_neigh_node, tmp_neigh_node2,
-				 &orig_node->bond.selected, bonding_list) {
-		list_del_rcu(&tmp_neigh_node->bonding_list);
-		kref_put(&tmp_neigh_node->refcount, neigh_node_free_ref);
-	}
-
-	hlist_for_each_entry_rcu(tmp_neigh_node, node,
-		&orig_node->neigh_list, list) {
-
-		/* only consider if it has the same primary address ...  */
-		if (memcmp(orig_node->orig,
-				tmp_neigh_node->orig_node->primary_addr,
-				ETH_ALEN) != 0)
-			continue;
-
-		/* ... and is good enough to be considered */
-		if (tmp_neigh_node->tq_avg < best_tq - BONDING_TQ_THRESHOLD)
-			continue;
-
-		/* check if we have another candidate with the same
-		 * mac address or interface. If we do, we won't
-		 * select this candidate because of possible interference. */
-
-		interference_candidate = 0;
-		hlist_for_each_entry_rcu(tmp_neigh_node2, node2,
-					 &orig_node->neigh_list, list) {
-
-			if (tmp_neigh_node2 == tmp_neigh_node)
-				continue;
-
-			/* we only care if the other candidate is even
-			 * considered as candidate. */
-			if (!list_empty(&tmp_neigh_node2->bonding_list))
-				continue;
-
-
-			if ((tmp_neigh_node->if_incoming ==
-				tmp_neigh_node2->if_incoming)
-				|| (memcmp(tmp_neigh_node->addr,
-				tmp_neigh_node2->addr, ETH_ALEN) == 0)) {
-
-				interference_candidate = 1;
-				break;
-			}
-		}
-		/* don't care further if it is an interference candidate */
-		if (interference_candidate)
-			continue;
-
-		list_add_rcu(&tmp_neigh_node->bonding_list,
-				&orig_node->bond.selected);
-		kref_get(&tmp_neigh_node->refcount);
-
-		candidates++;
-	}
-	orig_node->bond.candidates = candidates;
-
-	spin_unlock_bh(&orig_node->neigh_list_lock);
-
-}
-
 void receive_bat_packet(struct ethhdr *ethhdr,
-				struct batman_packet *batman_packet,
-				unsigned char *hna_buff, int hna_buff_len,
-				struct batman_if *if_incoming)
+			struct batman_packet *batman_packet,
+			unsigned char *hna_buff, int hna_buff_len,
+			struct batman_if *if_incoming)
 {
 	struct bat_priv *bat_priv = netdev_priv(if_incoming->soft_iface);
 	struct batman_if *batman_if;
@@ -779,6 +776,8 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 	is_bidirectional = is_bidirectional_neigh(orig_node, orig_neigh_node,
 						batman_packet, if_incoming);
 
+	bonding_save_primary(orig_node, orig_neigh_node, batman_packet);
+
 	/* update ranking if it is not a duplicate or has the same
 	 * seqno and similar ttl as the non-duplicate */
 	if (is_bidirectional &&
@@ -788,9 +787,6 @@ void receive_bat_packet(struct ethhdr *ethhdr,
 		update_orig(bat_priv, orig_node, ethhdr, batman_packet,
 			    if_incoming, hna_buff, hna_buff_len, is_duplicate);
 
-	mark_bonding_address(orig_node, orig_neigh_node, batman_packet);
-	update_bonding_candidates(orig_node);
-
 	/* is single hop (direct) neighbor */
 	if (is_single_hop_neigh) {
 
@@ -1115,7 +1111,6 @@ struct neigh_node *find_router(struct bat_priv *bat_priv,
 
 	/* without bonding, the first node should
 	 * always choose the default router. */
-
 	bonding_enabled = atomic_read(&bat_priv->bonding);
 
 	rcu_read_lock();
@@ -1149,10 +1144,10 @@ struct neigh_node *find_router(struct bat_priv *bat_priv,
 		if (!primary_orig_node)
 			goto return_router;
 	}
+
 	/* with less than 2 candidates, we can't do any
 	 * bonding and prefer the original router. */
-
-	if (primary_orig_node->bond.candidates < 2)
+	if (atomic_read(&primary_orig_node->bond_candidates) < 2)
 		goto return_router;
 
 
@@ -1168,7 +1163,7 @@ struct neigh_node *find_router(struct bat_priv *bat_priv,
 		 * robin fashion over the remaining interfaces. */
 
 		list_for_each_entry_rcu(tmp_neigh_node,
-			&primary_orig_node->bond.selected, bonding_list) {
+				&primary_orig_node->bond_list, bonding_list) {
 			if (!first_candidate)
 				first_candidate = tmp_neigh_node;
 			/* recv_if == NULL on the first node. */
@@ -1187,8 +1182,8 @@ struct neigh_node *find_router(struct bat_priv *bat_priv,
 		spin_lock_bh(&primary_orig_node->neigh_list_lock);
 		/* this is a list_move(), which unfortunately
 		 * does not exist as rcu version */
-		list_del_rcu(&primary_orig_node->bond.selected);
-		list_add_rcu(&primary_orig_node->bond.selected,
+		list_del_rcu(&primary_orig_node->bond_list);
+		list_add_rcu(&primary_orig_node->bond_list,
 				&router->bonding_list);
 		spin_unlock_bh(&primary_orig_node->neigh_list_lock);
 
@@ -1198,7 +1193,7 @@ struct neigh_node *find_router(struct bat_priv *bat_priv,
 		 * this interface. */
 		best_router_tq = 0;
 		list_for_each_entry_rcu(tmp_neigh_node,
-			&primary_orig_node->bond.selected, bonding_list) {
+			&primary_orig_node->bond_list, bonding_list) {
 			if (!first_candidate)
 				first_candidate = tmp_neigh_node;
 
@@ -1227,7 +1222,7 @@ static int check_unicast_packet(struct sk_buff *skb, int hdr_size)
 {
 	struct ethhdr *ethhdr;
 
-/* drop packet if it has not necessary minimum size */
+	/* drop packet if it has not necessary minimum size */
 	if (unlikely(!pskb_may_pull(skb, hdr_size)))
 		return -1;
 
diff --git a/batman-adv/routing.h b/batman-adv/routing.h
index 725cc38..e02789e 100644
--- a/batman-adv/routing.h
+++ b/batman-adv/routing.h
@@ -41,7 +41,9 @@ int recv_bcast_packet(struct sk_buff *skb, struct batman_if *recv_if);
 int recv_vis_packet(struct sk_buff *skb, struct batman_if *recv_if);
 int recv_bat_packet(struct sk_buff *skb, struct batman_if *recv_if);
 struct neigh_node *find_router(struct bat_priv *bat_priv,
-		struct orig_node *orig_node, struct batman_if *recv_if);
-void update_bonding_candidates(struct orig_node *orig_node);
+			       struct orig_node *orig_node,
+			       struct batman_if *recv_if);
+void bonding_candidate_del(struct orig_node *orig_node,
+			   struct neigh_node *neigh_node);
 
 #endif /* _NET_BATMAN_ADV_ROUTING_H_ */
diff --git a/batman-adv/types.h b/batman-adv/types.h
index f947336..8e97861 100644
--- a/batman-adv/types.h
+++ b/batman-adv/types.h
@@ -90,10 +90,8 @@ struct orig_node {
 	struct bat_priv *bat_priv;
 	unsigned long last_frag_packet;
 	spinlock_t ogm_cnt_lock; /* protects ogm counter */
-	struct {
-		uint8_t candidates;
-		struct list_head selected;
-	} bond;
+	atomic_t bond_candidates;
+	struct list_head bond_list;
 };
 
 struct gw_node {
@@ -121,6 +119,7 @@ struct neigh_node {
 	unsigned long real_bits[NUM_WORDS];
 	struct kref refcount;
 	struct rcu_head rcu;
+	struct rcu_head rcu_bond;
 	struct orig_node *orig_node;
 	struct batman_if *if_incoming;
 };
-- 
1.7.2.3

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: protect bonding with rcu locks

[Simon Wunderlich]

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

Hello Marek,

thanks for the review!

On Sun, Jan 16, 2011 at 12:35:53PM +0100, Marek Lindner wrote:
> On Thursday 30 December 2010 03:09:17 Simon Wunderlich wrote:
> [...]
> The changes include:
> 
> * The bonding / alternating candidate list is not destroyed & re-created with 
> each incoming OGM but candidates are added & deleted in a dynamic fashion.

This is a good idea. As discussed in private, this would add some latency in 
the update process, but this should not harm the bonding.

> * The memory leaks have been fixed / a workaround has been found. The "free 
> neighbors when an interface is deactivated" patch is also required to get the 
> full benefit of these memory leak fixes. 

Good job!

> * I introduced some style changes which are meant to increase readability & 
> maintainability. Feedback is welcome.  ;-)

Fine with me. :)

> While reorganizing the code I stumbled over these lines which are part of the 
> interference check:
> 
> /* we only care if the other candidate is even considered as candidate. */
> if (!list_empty(&tmp_neigh_node2->bonding_list))
>          continue;
> 
> This seems wrong in this context. If the list is empty then this neighbor is 
> not part of the candidate list. Wouldn't you agree ?

Agreed, this is wrong. Your patch fixes that, i see.

I've also checked your patch in my VM with 3 hosts and did some ping tests. 
No regressions found, the bonding and alternating worked fine. I did not look
into memleaks thou, but feel free to add my signoff and/or merge the patches.

Thanks,
	Simon
 

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

Re: [B.A.T.M.A.N.] performance patches (v3)

[Marek Lindner]

On Monday 13 December 2010 01:18:14 Marek Lindner wrote:
> the performance patches progress step by step. A few patches already went
> into  the repository the rest is waiting for more review. Thanks to Sven's
> feedback quite some bugs were squashed.
> 
> Changes since v2:
> * code rebased on top of the trunk
> * some more neigh_node protected by reference counters
> * call_rcu and kref_put race condition fixed
> 
> Known issues:
> * bonding / alternating candidates are not secured by refcounting [see  
> find_router()] - ideas welcome

Thanks to the bonding / alternating patch I was able to commit the remaining 
patches that replace the orig_hash spinlock with more advanced locking 
techniques (revision 1904 - 1909).

Regards,
Marek