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
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.
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;
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;
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;
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() */
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);
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
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; };
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
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
b.a.t.m.a.n@lists.open-mesh.org
-
Marek Lindner -
Simon Wunderlich