Every time that find_router() is invoked, if_status has to be compared with IF_ACTIVE. Moving this comparison inside find_router() will avoid to write it each time.
Signed-off-by: Antonio Quartulli ordex@autistici.org --- routing.c | 4 +++- unicast.c | 3 --- 2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/routing.c b/routing.c index 49f5715..c875164 100644 --- a/routing.c +++ b/routing.c @@ -1237,7 +1237,6 @@ struct neigh_node *find_router(struct bat_priv *bat_priv,
/* find the orig_node which has the primary interface. might * even be the same as our router_orig in many cases */ - if (compare_eth(router_orig->primary_addr, router_orig->orig)) { primary_orig_node = router_orig; } else { @@ -1266,6 +1265,9 @@ struct neigh_node *find_router(struct bat_priv *bat_priv, router = find_ifalter_router(primary_orig_node, recv_if);
return_router: + if (router && router->if_incoming->if_status != IF_ACTIVE) + router = NULL; + rcu_read_unlock(); return router; } diff --git a/unicast.c b/unicast.c index b46cbf1..1be53d7 100644 --- a/unicast.c +++ b/unicast.c @@ -314,9 +314,6 @@ find_router: if (!neigh_node) goto out;
- if (neigh_node->if_incoming->if_status != IF_ACTIVE) - goto out; - if (my_skb_head_push(skb, sizeof(struct unicast_packet)) < 0) goto out;
On Thursday 05 May 2011 10:21:42 Antonio Quartulli wrote:
return_router:
if (router && router->if_incoming->if_status != IF_ACTIVE)
router = NULL;
rcu_read_unlock(); return router;
You are breaking the reference counting of 'router' here. While looking at your patch I found another refcount imbalance. Check the patch I just posted (Fix refcount imbalance in find_router).
Regards, Marek
On gio, mag 05, 2011 at 02:20:10 +0200, Marek Lindner wrote:
On Thursday 05 May 2011 10:21:42 Antonio Quartulli wrote:
return_router:
if (router && router->if_incoming->if_status != IF_ACTIVE)
router = NULL;
rcu_read_unlock(); return router;
You are breaking the reference counting of 'router' here. While looking at your patch I found another refcount imbalance. Check the patch I just posted (Fix refcount imbalance in find_router).
I see :) And thanks for reviewing.
Regards,
Every time that find_router() is invoked, if_status has to be compared with IF_ACTIVE. Moving this comparison inside find_router() will avoid to write it each time.
Signed-off-by: Antonio Quartulli ordex@autistici.org --- In this version the reference counting of 'router' has been corrected.
routing.c | 3 +++ unicast.c | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/routing.c b/routing.c index bb1c3ec..8c403ce 100644 --- a/routing.c +++ b/routing.c @@ -1240,6 +1240,9 @@ struct neigh_node *find_router(struct bat_priv *bat_priv, router = find_ifalter_router(primary_orig_node, recv_if);
return_router: + if (router && router->if_incoming->if_status != IF_ACTIVE) + goto err_unlock; + rcu_read_unlock(); return router; err_unlock: diff --git a/unicast.c b/unicast.c index 19c3daf..bab6076 100644 --- a/unicast.c +++ b/unicast.c @@ -314,9 +314,6 @@ find_router: if (!neigh_node) goto out;
- if (neigh_node->if_incoming->if_status != IF_ACTIVE) - goto out; - if (my_skb_head_push(skb, sizeof(struct unicast_packet)) < 0) goto out;
On Sunday 08 May 2011 20:52:57 Antonio Quartulli wrote:
Every time that find_router() is invoked, if_status has to be compared with IF_ACTIVE. Moving this comparison inside find_router() will avoid to write it each time.
Applied in revision de7607d.
Thanks, Marek
b.a.t.m.a.n@lists.open-mesh.org