pull request: batman-adv 2011-10-29

pull request: batman-adv 2011-10-29

[Marek Lindner]

Hi David,

we have 3 more bugfixes for you which we would like to see getting
merged into net-next/linux-3.2. They fix refcounting, a crash on
module unload and a protocol handling bug.

Thanks,
Marek

The following changes since commit 9d8523931f7f5eb8900077f0da0fbe6b8ad0010b:

  batman-adv: correctly set the data field in the TT_REPONSE packet (2011-10-18 22:45:10 +0200)

are available in the git repository at:
  git://git.open-mesh.org/linux-merge.git batman-adv/maint

Antonio Quartulli (1):
      batman-adv: unify hash_entry field position in tt_local/global_entry

Simon Wunderlich (2):
      batman-adv: remove references for global tt entries
      batman-adv: add sanity check when removing global tts

 net/batman-adv/translation-table.c |   17 ++++++++++++++++-
 net/batman-adv/types.h             |    4 ++--
 2 files changed, 18 insertions(+), 3 deletions(-)

[PATCH 1/3] batman-adv: remove references for global tt entries

[Marek Lindner]

From: Simon Wunderlich <simon.wunderlich-Y4E02TeZ33kaBlGTGt4zH4SGEyLTKazZ@public.gmane.org>

struct tt_global_entry holds a reference to an orig_node which must be
decremented before deallocating the structure.

Signed-off-by: Simon Wunderlich <siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX@public.gmane.org>
Tested-by: Alexey Fisher <bug-track-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org>
Signed-off-by: Marek Lindner <lindner_marek-LWAfsSFWpa4@public.gmane.org>
---
 net/batman-adv/translation-table.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 873fb3d..abf05cb 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -137,10 +137,22 @@ static void tt_local_entry_free_ref(struct tt_local_entry *tt_local_entry)
 		kfree_rcu(tt_local_entry, rcu);
 }
 
+static void tt_global_entry_free_rcu(struct rcu_head *rcu)
+{
+	struct tt_global_entry *tt_global_entry;
+
+	tt_global_entry = container_of(rcu, struct tt_global_entry, rcu);
+
+	if (tt_global_entry->orig_node)
+		orig_node_free_ref(tt_global_entry->orig_node);
+
+	kfree(tt_global_entry);
+}
+
 static void tt_global_entry_free_ref(struct tt_global_entry *tt_global_entry)
 {
 	if (atomic_dec_and_test(&tt_global_entry->refcount))
-		kfree_rcu(tt_global_entry, rcu);
+		call_rcu(&tt_global_entry->rcu, tt_global_entry_free_rcu);
 }
 
 static void tt_local_event(struct bat_priv *bat_priv, const uint8_t *addr,
-- 
1.7.5.4

[PATCH 3/3] batman-adv: unify hash_entry field position in tt_local/global_entry

[Marek Lindner]

From: Antonio Quartulli <ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>

Function tt_response_fill_table() actually uses a tt_local_entry pointer to
iterate either over the local or the global table entries (it depends on the
what hash table is passed as argument). To iterate over such entries the
hlist_for_each_entry_rcu() macro has to access their "hash_entry" field which
MUST be at the same position in both the tt_global/local_entry structures.

Reported-by: Simon Wunderlich <siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX@public.gmane.org>
Signed-off-by: Antonio Quartulli <ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>
Signed-off-by: Marek Lindner <lindner_marek-LWAfsSFWpa4@public.gmane.org>
---
 net/batman-adv/types.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 1ae3557..ab8d0fe 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -224,22 +224,22 @@ struct socket_packet {
 
 struct tt_local_entry {
 	uint8_t addr[ETH_ALEN];
+	struct hlist_node hash_entry;
 	unsigned long last_seen;
 	uint16_t flags;
 	atomic_t refcount;
 	struct rcu_head rcu;
-	struct hlist_node hash_entry;
 };
 
 struct tt_global_entry {
 	uint8_t addr[ETH_ALEN];
+	struct hlist_node hash_entry; /* entry in the global table */
 	struct orig_node *orig_node;
 	uint8_t ttvn;
 	uint16_t flags; /* only TT_GLOBAL_ROAM is used */
 	unsigned long roam_at; /* time at which TT_GLOBAL_ROAM was set */
 	atomic_t refcount;
 	struct rcu_head rcu;
-	struct hlist_node hash_entry; /* entry in the global table */
 };
 
 struct tt_change_node {
-- 
1.7.5.4

Re: pull request: batman-adv 2011-10-29

[David Miller]

From: Marek Lindner <lindner_marek-LWAfsSFWpa4@public.gmane.org>
Date: Sat, 29 Oct 2011 10:06:43 +0200

>   git://git.open-mesh.org/linux-merge.git batman-adv/maint

Pulled, but long term you should shore up your datastructures to
handle that issue in patch #3.

Make a common header:

	struct tt_entry_common {
		u8 addr[ETH_ALEN];
		struct hlist_node hash_entry;
	};

Then use that at the beginning of both structures:

	struct tt_local_entry {
		struct tt_entry_common common;
		unsigned long last_seen;
		...
	};

	struct tt_global_entry {
		struct tt_entry_comomn common;
		struct orig_node *orig_node;
		...
	};

And &p->common is what gets passed into tt_response_fill_table().

Re: pull request: batman-adv 2011-10-29

[Sven Eckelmann]

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

On Sunday 30 October 2011 03:07:45 David Miller wrote:
[...]
> Make a common header:
> 
> 	struct tt_entry_common {
> 		u8 addr[ETH_ALEN];
> 		struct hlist_node hash_entry;
> 	};
> 
> Then use that at the beginning of both structures:
> 
> 	struct tt_local_entry {
> 		struct tt_entry_common common;
> 		unsigned long last_seen;
> 		...
> 	};
> 
> 	struct tt_global_entry {
> 		struct tt_entry_comomn common;
> 		struct orig_node *orig_node;
> 		...
> 	};
> 
> And &p->common is what gets passed into tt_response_fill_table().

Thanks for the pull. This is exactly the long term solution we want to submit 
later to net-next. But we also wanted to keep the patch as small as possible 
for stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Thanks,
	Sven

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

[PATCH 2/3] batman-adv: add sanity check when removing global tts

[Marek Lindner]

From: Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>

After removing the batman-adv module, the hash may be already gone
when tt_global_del_orig() tries to clean the hash. This patch adds
a sanity check to avoid this.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Tested-by: Alexey Fisher <bug-track@fisher-privat.net>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 net/batman-adv/translation-table.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index abf05cb..c7aafc7 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -722,6 +722,9 @@ void tt_global_del_orig(struct bat_priv *bat_priv,
 	struct hlist_head *head;
 	spinlock_t *list_lock; /* protects write access to the hash lists */
 
+	if (!hash)
+		return;
+
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 		list_lock = &hash->list_locks[i];
-- 
1.7.5.4