netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Simon Wunderlich <sw@simonwunderlich.de>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, b.a.t.m.a.n@lists.open-mesh.org,
	Sven Eckelmann <sven@narfation.org>,
	Marek Lindner <mareklindner@neomailbox.ch>,
	Simon Wunderlich <sw@simonwunderlich.de>
Subject: [PATCH 19/19] batman-adv: Fix consistency of update route messages
Date: Fri, 12 Aug 2016 10:56:55 +0200	[thread overview]
Message-ID: <1470992215-11009-20-git-send-email-sw@simonwunderlich.de> (raw)
In-Reply-To: <1470992215-11009-1-git-send-email-sw@simonwunderlich.de>

From: Sven Eckelmann <sven@narfation.org>

The debug messages of _batadv_update_route were printed before the actual
route change is done. At this point it is not really known which
curr_router will be replaced. Thus the messages could print the wrong
operation.

Printing the debug messages after the operation was done avoids this
problem.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/routing.c | 43 +++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 7602c00..610f2c4 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -74,11 +74,23 @@ static void _batadv_update_route(struct batadv_priv *bat_priv,
 	if (!orig_ifinfo)
 		return;
 
-	rcu_read_lock();
-	curr_router = rcu_dereference(orig_ifinfo->router);
-	if (curr_router && !kref_get_unless_zero(&curr_router->refcount))
-		curr_router = NULL;
-	rcu_read_unlock();
+	spin_lock_bh(&orig_node->neigh_list_lock);
+	/* curr_router used earlier may not be the current orig_ifinfo->router
+	 * anymore because it was dereferenced outside of the neigh_list_lock
+	 * protected region. After the new best neighbor has replace the current
+	 * best neighbor the reference counter needs to decrease. Consequently,
+	 * the code needs to ensure the curr_router variable contains a pointer
+	 * to the replaced best neighbor.
+	 */
+	curr_router = rcu_dereference_protected(orig_ifinfo->router, true);
+
+	/* increase refcount of new best neighbor */
+	if (neigh_node)
+		kref_get(&neigh_node->refcount);
+
+	rcu_assign_pointer(orig_ifinfo->router, neigh_node);
+	spin_unlock_bh(&orig_node->neigh_list_lock);
+	batadv_orig_ifinfo_put(orig_ifinfo);
 
 	/* route deleted */
 	if ((curr_router) && (!neigh_node)) {
@@ -100,27 +112,6 @@ static void _batadv_update_route(struct batadv_priv *bat_priv,
 			   curr_router->addr);
 	}
 
-	if (curr_router)
-		batadv_neigh_node_put(curr_router);
-
-	spin_lock_bh(&orig_node->neigh_list_lock);
-	/* curr_router used earlier may not be the current orig_ifinfo->router
-	 * anymore because it was dereferenced outside of the neigh_list_lock
-	 * protected region. After the new best neighbor has replace the current
-	 * best neighbor the reference counter needs to decrease. Consequently,
-	 * the code needs to ensure the curr_router variable contains a pointer
-	 * to the replaced best neighbor.
-	 */
-	curr_router = rcu_dereference_protected(orig_ifinfo->router, true);
-
-	/* increase refcount of new best neighbor */
-	if (neigh_node)
-		kref_get(&neigh_node->refcount);
-
-	rcu_assign_pointer(orig_ifinfo->router, neigh_node);
-	spin_unlock_bh(&orig_node->neigh_list_lock);
-	batadv_orig_ifinfo_put(orig_ifinfo);
-
 	/* decrease refcount of previous best neighbor */
 	if (curr_router)
 		batadv_neigh_node_put(curr_router);
-- 
2.8.1

      parent reply	other threads:[~2016-08-12  8:57 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-12  8:56 [PATCH 00/19] pull request for net-next: batman-adv 2016-08-12 Simon Wunderlich
2016-08-12  8:56 ` [PATCH 02/19] batman-adv: Document optional batadv_algo_ops Simon Wunderlich
2016-08-12  8:56 ` [PATCH 05/19] batman-adv: Modify mesh_iface outside sysfs context Simon Wunderlich
     [not found] ` <1470992215-11009-1-git-send-email-sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
2016-08-12  8:56   ` [PATCH 01/19] batman-adv: Start new development cycle Simon Wunderlich
2016-08-12  8:56   ` [PATCH 03/19] batman-adv: Define module rtnl link name Simon Wunderlich
2016-08-12  8:56   ` [PATCH 04/19] batman-adv: Use rtnl link in device creation example Simon Wunderlich
2016-08-12  8:56   ` [PATCH 06/19] batman-adv: Revert "postpone sysfs removal when unregistering" Simon Wunderlich
2016-08-12  8:56   ` [PATCH 07/19] batman-adv: Avoid sysfs name collision for netns moves Simon Wunderlich
2016-08-12  8:56   ` [PATCH 08/19] batman-adv: Remove unused primary_if and bat_priv variables Simon Wunderlich
2016-08-12  8:56   ` [PATCH 09/19] batman-adv: make the GW selection class algorithm specific Simon Wunderlich
2016-08-12  8:56   ` [PATCH 10/19] batman-adv: make GW election code protocol specific Simon Wunderlich
2016-08-12  8:56   ` [PATCH 11/19] batman-adv: B.A.T.M.A.N. V - implement GW selection logic Simon Wunderlich
2016-08-13  3:56   ` [PATCH 00/19] pull request for net-next: batman-adv 2016-08-12 David Miller
2016-08-12  8:56 ` [PATCH 12/19] batman-adv: disable sysfs knobs when GW-mode is not implemented Simon Wunderlich
2016-08-12  8:56 ` [PATCH 13/19] batman-adv: iv_ogm, Reduce code duplication Simon Wunderlich
2016-08-12  8:56 ` [PATCH 14/19] batman-adv: fix boolreturn.cocci warnings Simon Wunderlich
2016-08-12  8:56 ` [PATCH 15/19] batman-adv: Introduce forward packet creation helper Simon Wunderlich
2016-08-12  8:56 ` [PATCH 16/19] batman-adv: use kmem_cache for translation table Simon Wunderlich
2016-08-12  8:56 ` [PATCH 17/19] batman-adv: Remove orig_node reference handling from send_skb_unicast Simon Wunderlich
2016-08-12  8:56 ` [PATCH 18/19] batman-adv: Use bitwise instead of arithmetic operator for flags Simon Wunderlich
2016-08-12  8:56 ` Simon Wunderlich [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1470992215-11009-20-git-send-email-sw@simonwunderlich.de \
    --to=sw@simonwunderlich.de \
    --cc=b.a.t.m.a.n@lists.open-mesh.org \
    --cc=davem@davemloft.net \
    --cc=mareklindner@neomailbox.ch \
    --cc=netdev@vger.kernel.org \
    --cc=sven@narfation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).