* [B.A.T.M.A.N.] bug - unregister_netdevice: usage_count = 1
@ 2011-05-11 0:17 Antonio Quartulli
2011-05-11 12:44 ` [B.A.T.M.A.N.] [RFC PATCH] batman-adv: Add missing hardif_free_ref in forw_packet_free Sven Eckelmann
0 siblings, 1 reply; 5+ messages in thread
From: Antonio Quartulli @ 2011-05-11 0:17 UTC (permalink / raw)
To: B.A.T.M.A.N
Hi all,
I found a bug playing with my interfaces while bat0 was active.
The bug is described here: http://www.open-mesh.org/issues/148
The steps to reproduce it are reported too.
In a few words, when trying to unregister a device by unloading its module,
the modprobe hangs and in the dmesg I can see this message:
unregister_netdevice: waiting for wlan0 to become free. Usage count = 1
I tried to do some tests and what I observed is that
hard_iface->refcount doesn't reach 0. This means that
hard_iface_free_rcu() is never invoked and dev_put() too.
I hope the information are enough..
Regards,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
^ permalink raw reply [flat|nested] 5+ messages in thread
* [B.A.T.M.A.N.] [RFC PATCH] batman-adv: Add missing hardif_free_ref in forw_packet_free
2011-05-11 0:17 [B.A.T.M.A.N.] bug - unregister_netdevice: usage_count = 1 Antonio Quartulli
@ 2011-05-11 12:44 ` Sven Eckelmann
2011-05-11 18:59 ` [B.A.T.M.A.N.] [RFC PATCHv2] " Sven Eckelmann
2011-05-13 13:05 ` [B.A.T.M.A.N.] [RFC PATCH] " Antonio Quartulli
0 siblings, 2 replies; 5+ messages in thread
From: Sven Eckelmann @ 2011-05-11 12:44 UTC (permalink / raw)
To: b.a.t.m.a.n
add_bcast_packet_to_list increases the refcount for if_incoming but the
reference count is never decreased. This is an regression introduced by
89079aec4c0c2c60a3a2561afcce62538cebd71d.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
This patch is not ready to be applied - just a first note. And before
someone asks - no, it doesn't fix the problem.
aggregation.c | 14 +++++++++++---
send.c | 2 ++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/aggregation.c b/aggregation.c
index 9b94590..a8c3203 100644
--- a/aggregation.c
+++ b/aggregation.c
@@ -23,6 +23,7 @@
#include "aggregation.h"
#include "send.h"
#include "routing.h"
+#include "hard-interface.h"
/* calculate the size of the tt information for a given packet */
static int tt_len(struct batman_packet *batman_packet)
@@ -105,12 +106,15 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
struct forw_packet *forw_packet_aggr;
unsigned char *skb_buff;
+ if (!atomic_inc_not_zero(&if_incoming->refcount))
+ return;
+
/* own packet should always be scheduled */
if (!own_packet) {
if (!atomic_dec_not_zero(&bat_priv->batman_queue_left)) {
bat_dbg(DBG_BATMAN, bat_priv,
"batman packet queue full\n");
- return;
+ goto out;
}
}
@@ -118,7 +122,7 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
if (!forw_packet_aggr) {
if (!own_packet)
atomic_inc(&bat_priv->batman_queue_left);
- return;
+ goto out;
}
if ((atomic_read(&bat_priv->aggregated_ogms)) &&
@@ -133,7 +137,7 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
if (!own_packet)
atomic_inc(&bat_priv->batman_queue_left);
kfree(forw_packet_aggr);
- return;
+ goto out;
}
skb_reserve(forw_packet_aggr->skb, sizeof(struct ethhdr));
@@ -164,6 +168,10 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
queue_delayed_work(bat_event_workqueue,
&forw_packet_aggr->delayed_work,
send_time - jiffies);
+
+ return;
+out:
+ hardif_free_ref(if_incoming);
}
/* aggregate a new packet into the existing aggregation */
diff --git a/send.c b/send.c
index f30d0c6..6e83e23 100644
--- a/send.c
+++ b/send.c
@@ -377,6 +377,8 @@ static void forw_packet_free(struct forw_packet *forw_packet)
{
if (forw_packet->skb)
kfree_skb(forw_packet->skb);
+ if (forw_packet->if_incoming)
+ hardif_free_ref(forw_packet->if_incoming);
kfree(forw_packet);
}
--
1.7.5.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [B.A.T.M.A.N.] [RFC PATCHv2] batman-adv: Add missing hardif_free_ref in forw_packet_free
2011-05-11 12:44 ` [B.A.T.M.A.N.] [RFC PATCH] batman-adv: Add missing hardif_free_ref in forw_packet_free Sven Eckelmann
@ 2011-05-11 18:59 ` Sven Eckelmann
2011-05-14 15:26 ` Marek Lindner
2011-05-13 13:05 ` [B.A.T.M.A.N.] [RFC PATCH] " Antonio Quartulli
1 sibling, 1 reply; 5+ messages in thread
From: Sven Eckelmann @ 2011-05-11 18:59 UTC (permalink / raw)
To: b.a.t.m.a.n
add_bcast_packet_to_list increases the refcount for if_incoming but the
reference count is never decreased. The reference count must be
increased for all kinds of forwarded packets which have the primary
interface stored and forw_packet_free must decrease them. Also
purge_outstanding_packets has to invoke forw_packet_free when a work
item was really cancelled.
This was a regression introduced by
89079aec4c0c2c60a3a2561afcce62538cebd71d.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
I will leave the hard work to someone else (cleanup, fixing commit
message, ...)
aggregation.c | 14 +++++++++++---
send.c | 10 +++++++++-
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/aggregation.c b/aggregation.c
index 9b94590..a8c3203 100644
--- a/aggregation.c
+++ b/aggregation.c
@@ -23,6 +23,7 @@
#include "aggregation.h"
#include "send.h"
#include "routing.h"
+#include "hard-interface.h"
/* calculate the size of the tt information for a given packet */
static int tt_len(struct batman_packet *batman_packet)
@@ -105,12 +106,15 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
struct forw_packet *forw_packet_aggr;
unsigned char *skb_buff;
+ if (!atomic_inc_not_zero(&if_incoming->refcount))
+ return;
+
/* own packet should always be scheduled */
if (!own_packet) {
if (!atomic_dec_not_zero(&bat_priv->batman_queue_left)) {
bat_dbg(DBG_BATMAN, bat_priv,
"batman packet queue full\n");
- return;
+ goto out;
}
}
@@ -118,7 +122,7 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
if (!forw_packet_aggr) {
if (!own_packet)
atomic_inc(&bat_priv->batman_queue_left);
- return;
+ goto out;
}
if ((atomic_read(&bat_priv->aggregated_ogms)) &&
@@ -133,7 +137,7 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
if (!own_packet)
atomic_inc(&bat_priv->batman_queue_left);
kfree(forw_packet_aggr);
- return;
+ goto out;
}
skb_reserve(forw_packet_aggr->skb, sizeof(struct ethhdr));
@@ -164,6 +168,10 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
queue_delayed_work(bat_event_workqueue,
&forw_packet_aggr->delayed_work,
send_time - jiffies);
+
+ return;
+out:
+ hardif_free_ref(if_incoming);
}
/* aggregate a new packet into the existing aggregation */
diff --git a/send.c b/send.c
index f30d0c6..76f70c7 100644
--- a/send.c
+++ b/send.c
@@ -377,6 +377,8 @@ static void forw_packet_free(struct forw_packet *forw_packet)
{
if (forw_packet->skb)
kfree_skb(forw_packet->skb);
+ if (forw_packet->if_incoming)
+ hardif_free_ref(forw_packet->if_incoming);
kfree(forw_packet);
}
@@ -539,6 +541,7 @@ void purge_outstanding_packets(struct bat_priv *bat_priv,
{
struct forw_packet *forw_packet;
struct hlist_node *tmp_node, *safe_tmp_node;
+ bool pending;
if (hard_iface)
bat_dbg(DBG_BATMAN, bat_priv,
@@ -591,8 +594,13 @@ void purge_outstanding_packets(struct bat_priv *bat_priv,
* send_outstanding_bat_packet() will lock the list to
* delete the item from the list
*/
- cancel_delayed_work_sync(&forw_packet->delayed_work);
+ pending = cancel_delayed_work_sync(&forw_packet->delayed_work);
spin_lock_bh(&bat_priv->forw_bat_list_lock);
+
+ if (pending) {
+ hlist_del(&forw_packet->list);
+ forw_packet_free(forw_packet);
+ }
}
spin_unlock_bh(&bat_priv->forw_bat_list_lock);
}
--
1.7.5.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [B.A.T.M.A.N.] [RFC PATCH] batman-adv: Add missing hardif_free_ref in forw_packet_free
2011-05-11 12:44 ` [B.A.T.M.A.N.] [RFC PATCH] batman-adv: Add missing hardif_free_ref in forw_packet_free Sven Eckelmann
2011-05-11 18:59 ` [B.A.T.M.A.N.] [RFC PATCHv2] " Sven Eckelmann
@ 2011-05-13 13:05 ` Antonio Quartulli
1 sibling, 0 replies; 5+ messages in thread
From: Antonio Quartulli @ 2011-05-13 13:05 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
I tried the patch and it fixes the issu I reported!
Thanks Sven!
On Wed, May 11, 2011 at 02:44:37PM +0200, Sven Eckelmann wrote:
> add_bcast_packet_to_list increases the refcount for if_incoming but the
> reference count is never decreased. This is an regression introduced by
> 89079aec4c0c2c60a3a2561afcce62538cebd71d.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> This patch is not ready to be applied - just a first note. And before
> someone asks - no, it doesn't fix the problem.
>
> aggregation.c | 14 +++++++++++---
> send.c | 2 ++
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/aggregation.c b/aggregation.c
> index 9b94590..a8c3203 100644
> --- a/aggregation.c
> +++ b/aggregation.c
> @@ -23,6 +23,7 @@
> #include "aggregation.h"
> #include "send.h"
> #include "routing.h"
> +#include "hard-interface.h"
>
> /* calculate the size of the tt information for a given packet */
> static int tt_len(struct batman_packet *batman_packet)
> @@ -105,12 +106,15 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
> struct forw_packet *forw_packet_aggr;
> unsigned char *skb_buff;
>
> + if (!atomic_inc_not_zero(&if_incoming->refcount))
> + return;
> +
> /* own packet should always be scheduled */
> if (!own_packet) {
> if (!atomic_dec_not_zero(&bat_priv->batman_queue_left)) {
> bat_dbg(DBG_BATMAN, bat_priv,
> "batman packet queue full\n");
> - return;
> + goto out;
> }
> }
>
> @@ -118,7 +122,7 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
> if (!forw_packet_aggr) {
> if (!own_packet)
> atomic_inc(&bat_priv->batman_queue_left);
> - return;
> + goto out;
> }
>
> if ((atomic_read(&bat_priv->aggregated_ogms)) &&
> @@ -133,7 +137,7 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
> if (!own_packet)
> atomic_inc(&bat_priv->batman_queue_left);
> kfree(forw_packet_aggr);
> - return;
> + goto out;
> }
> skb_reserve(forw_packet_aggr->skb, sizeof(struct ethhdr));
>
> @@ -164,6 +168,10 @@ static void new_aggregated_packet(unsigned char *packet_buff, int packet_len,
> queue_delayed_work(bat_event_workqueue,
> &forw_packet_aggr->delayed_work,
> send_time - jiffies);
> +
> + return;
> +out:
> + hardif_free_ref(if_incoming);
> }
>
> /* aggregate a new packet into the existing aggregation */
> diff --git a/send.c b/send.c
> index f30d0c6..6e83e23 100644
> --- a/send.c
> +++ b/send.c
> @@ -377,6 +377,8 @@ static void forw_packet_free(struct forw_packet *forw_packet)
> {
> if (forw_packet->skb)
> kfree_skb(forw_packet->skb);
> + if (forw_packet->if_incoming)
> + hardif_free_ref(forw_packet->if_incoming);
> kfree(forw_packet);
> }
>
> --
> 1.7.5.1
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [B.A.T.M.A.N.] [RFC PATCHv2] batman-adv: Add missing hardif_free_ref in forw_packet_free
2011-05-11 18:59 ` [B.A.T.M.A.N.] [RFC PATCHv2] " Sven Eckelmann
@ 2011-05-14 15:26 ` Marek Lindner
0 siblings, 0 replies; 5+ messages in thread
From: Marek Lindner @ 2011-05-14 15:26 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
On Wednesday 11 May 2011 20:59:06 Sven Eckelmann wrote:
> add_bcast_packet_to_list increases the refcount for if_incoming but the
> reference count is never decreased. The reference count must be
> increased for all kinds of forwarded packets which have the primary
> interface stored and forw_packet_free must decrease them. Also
> purge_outstanding_packets has to invoke forw_packet_free when a work
> item was really cancelled.
Applied in revision d18e485.
Thanks,
Marek
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-05-14 15:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-11 0:17 [B.A.T.M.A.N.] bug - unregister_netdevice: usage_count = 1 Antonio Quartulli
2011-05-11 12:44 ` [B.A.T.M.A.N.] [RFC PATCH] batman-adv: Add missing hardif_free_ref in forw_packet_free Sven Eckelmann
2011-05-11 18:59 ` [B.A.T.M.A.N.] [RFC PATCHv2] " Sven Eckelmann
2011-05-14 15:26 ` Marek Lindner
2011-05-13 13:05 ` [B.A.T.M.A.N.] [RFC PATCH] " Antonio Quartulli
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.