From patchwork Sat Dec 4 20:03:46 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sven Eckelmann X-Patchwork-Id: 595 Return-Path: Received: from mail.gmx.net (mailout-de.gmx.net [213.165.64.22]) by open-mesh.org (Postfix) with SMTP id E78E3154638 for ; Sat, 4 Dec 2010 21:03:12 +0100 (CET) Received: (qmail invoked by alias); 04 Dec 2010 20:03:11 -0000 Received: from i59F6C8C4.versanet.de (EHLO sven-desktop.lazhur.ath.cx) [89.246.200.196] by mail.gmx.net (mp046) with SMTP; 04 Dec 2010 21:03:11 +0100 X-Authenticated: #15668376 X-Provags-ID: V01U2FsdGVkX1+edKEDqnueBLFMxgQ8p2WsjYywEMnR49Nr9UaB6V TAOJ9kg2IcUHdF From: Sven Eckelmann To: b.a.t.m.a.n@lists.open-mesh.org Date: Sat, 4 Dec 2010 21:03:46 +0100 Message-Id: <1291493028-29957-7-git-send-email-sven.eckelmann@gmx.de> X-Mailer: git-send-email 1.7.2.3 In-Reply-To: <201012042042.16056.sven.eckelmann@gmx.de> References: <201012042042.16056.sven.eckelmann@gmx.de> X-Y-GMX-Trusted: 0 Cc: Marek Lindner Subject: [B.A.T.M.A.N.] [PATCH 7/9] batman-adv: protect originator nodes with reference counters X-BeenThere: b.a.t.m.a.n@lists.open-mesh.org X-Mailman-Version: 2.1.11 Precedence: list Reply-To: The list for a Better Approach To Mobile Ad-hoc Networking List-Id: The list for a Better Approach To Mobile Ad-hoc Networking List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 04 Dec 2010 20:03:13 -0000 From: Marek Lindner Signed-off-by: Marek Lindner --- Here is a great patch to see the call_rcu and kref_put problem batman-adv/originator.c | 49 +++++++++++++++++++++++++++++++++++++++------- batman-adv/originator.h | 1 + batman-adv/routing.c | 39 ++++++++++++++++++++++++------------ batman-adv/types.h | 2 + 4 files changed, 70 insertions(+), 21 deletions(-) diff --git a/batman-adv/originator.c b/batman-adv/originator.c index a43cda4..fa34ea0 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,14 +133,38 @@ 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); } @@ -156,8 +182,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 +196,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 +227,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 +345,9 @@ 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); + kref_put(&orig_node->refcount, + orig_node_free_ref); call_rcu(&bucket->rcu, bucket_free_rcu); - free_orig_node(orig_node, bat_priv); 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 035f159..a9a30ff 100644 --- a/batman-adv/routing.c +++ b/batman-adv/routing.c @@ -301,6 +301,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) return; } else @@ -409,7 +411,7 @@ static char count_real_packets(struct ethhdr *ethhdr, int set_mark; orig_node = get_orig_node(bat_priv, batman_packet->orig); - if (orig_node == NULL) + if (!orig_node) return 0; seq_diff = batman_packet->seqno - orig_node->last_real_seqno; @@ -417,7 +419,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, @@ -450,7 +452,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 */ @@ -667,7 +674,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; @@ -688,6 +694,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; } @@ -699,7 +706,7 @@ void receive_bat_packet(struct ethhdr *ethhdr, } orig_node = get_orig_node(bat_priv, batman_packet->orig); - if (orig_node == NULL) + if (!orig_node) return; is_duplicate = count_real_packets(ethhdr, batman_packet, if_incoming); @@ -708,13 +715,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 */ @@ -728,7 +735,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 @@ -736,8 +743,8 @@ void receive_bat_packet(struct ethhdr *ethhdr, orig_neigh_node = (is_single_hop_neigh ? orig_node : get_orig_node(bat_priv, ethhdr->h_source)); - if (orig_neigh_node == NULL) - return; + if (!orig_neigh_node) + goto out_neigh; /* drop packet if sender is not a direct neighbor and if we * don't route towards it */ @@ -745,7 +752,7 @@ void receive_bat_packet(struct ethhdr *ethhdr, (orig_neigh_node->router == NULL)) { 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, @@ -773,26 +780,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 1d6365d..f4ce526 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;