Netdev List
 help / color / mirror / Atom feed
* Re: linux-next: manual merge of the arm-soc tree with the net-next tree
From: Stephen Rothwell @ 2012-11-13  9:08 UTC (permalink / raw)
  To: Joachim Eastwood
  Cc: Olof Johansson, Arnd Bergmann, linux-arm-kernel, linux-next,
	linux-kernel, Jean-Christophe PLAGNIOL-VILLARD, David Miller,
	netdev
In-Reply-To: <CAGhQ9Vx3Y3j0qqkEif09HebFyR+1UYmSbp=3jwuXnoiuWBdymQ@mail.gmail.com>

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

Hi Joachim,

On Tue, 13 Nov 2012 08:21:26 +0100 Joachim  Eastwood <manabian@gmail.com> wrote:
>
> The platform_data/atmel.h include shouldn't be necessary since the
> driver already includes platform_data/macb.h.
> 
> Otherwise the fix up looks correct.

Thanks, I have fixed up my fix for tomorrow.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* pull request: batman-adv 2012-11-13
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r

Hello David,

this is another set of changes intended for net-next/linux-3.8.
In this patchset you have patch 10/11 which is a roaming handling procedure
re-arrangement and patches 4,5,6/11 by Simon Wunderlich which improve the Bridge
Loop Avoidance set-up phase. The rest is mostly clean-up work.

Let me know if there is any problem!

Thanks a lot,
	Antonio


The following changes since commit 9fafd65ad407d4e0c96919a325f568dd95d032af:

  ipv6 ndisc: Use pre-defined in6addr_linklocal_allnodes. (2012-11-12 15:23:21 -0500)

are available in the git repository at:

  git://git.open-mesh.org/linux-merge.git tags/batman-adv-for-davem

for you to fetch changes up to 153f5177b998c04c8459bc591cd4e5204fe66a1c:

  batman-adv: Remove instant overwritten variable initialization (2012-11-13 10:06:08 +0100)

----------------------------------------------------------------
Included changes:
- hash computation improvements
- Bridge Loop Avoidance set-up phase optimisations
- Roaming handling code redesign
- some code cleanups

----------------------------------------------------------------
Antonio Quartulli (6):
      batman-adv: fix wrong spinlock inline comment
      batman-adv: prevent using any virtual device created on batman-adv as hard-interface
      batman-adv: substitute tt_poss_change with a per-tt_entry flag
      batman-adv: refactor code to simplify long lines
      batman-adv: refactor tt_global_del_struct()
      batman-adv: roaming handling mechanism redesign

Simon Wunderlich (4):
      batman-adv: don't rely on positions in struct for hashing
      batman-adv: send announcement when backbone gw is registered
      batman-adv: allow bla traffic only after first worker period
      batman-adv: wait multiple periods before activating bla

Sven Eckelmann (1):
      batman-adv: Remove instant overwritten variable initialization

 net/batman-adv/bridge_loop_avoidance.c |  71 ++++++---
 net/batman-adv/hard-interface.c        |  41 ++++-
 net/batman-adv/main.h                  |   1 +
 net/batman-adv/originator.c            |   1 -
 net/batman-adv/routing.c               | 173 +++++++++++++++------
 net/batman-adv/soft-interface.c        |   1 -
 net/batman-adv/sysfs.c                 |   2 +-
 net/batman-adv/translation-table.c     | 276 ++++++++++++++++++++++-----------
 net/batman-adv/translation-table.h     |   2 +
 net/batman-adv/types.h                 |  11 +-
 10 files changed, 406 insertions(+), 173 deletions(-)

^ permalink raw reply

* [PATCH 01/11] batman-adv: don't rely on positions in struct for hashing
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, Simon Wunderlich
In-Reply-To: <1352798139-19458-1-git-send-email-ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>

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

The hash functions in the bridge loop avoidance code expects the
VLAN vid to be right after the mac address, but this is not guaranteed.

Fix this by explicitly hashing over the right fields of the struct.

Reported-by: Marek Lindner <lindner_marek-LWAfsSFWpa4@public.gmane.org>
Signed-off-by: Simon Wunderlich <siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX@public.gmane.org>
Signed-off-by: Antonio Quartulli <ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>
---
 net/batman-adv/bridge_loop_avoidance.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 29a5542..decc042 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -37,18 +37,26 @@ static void batadv_bla_periodic_work(struct work_struct *work);
 static void batadv_bla_send_announce(struct batadv_priv *bat_priv,
 				     struct batadv_backbone_gw *backbone_gw);
 
+static inline void hash_bytes(uint32_t *hash, void *data, uint32_t size)
+{
+	const unsigned char *key = data;
+	int i;
+
+	for (i = 0; i < size; i++) {
+		*hash += key[i];
+		*hash += (*hash << 10);
+		*hash ^= (*hash >> 6);
+	}
+}
+
 /* return the index of the claim */
 static inline uint32_t batadv_choose_claim(const void *data, uint32_t size)
 {
-	const unsigned char *key = data;
+	struct batadv_claim *claim = (struct batadv_claim *)data;
 	uint32_t hash = 0;
-	size_t i;
 
-	for (i = 0; i < ETH_ALEN + sizeof(short); i++) {
-		hash += key[i];
-		hash += (hash << 10);
-		hash ^= (hash >> 6);
-	}
+	hash_bytes(&hash, &claim->addr, sizeof(claim->addr));
+	hash_bytes(&hash, &claim->vid, sizeof(claim->vid));
 
 	hash += (hash << 3);
 	hash ^= (hash >> 11);
@@ -61,15 +69,11 @@ static inline uint32_t batadv_choose_claim(const void *data, uint32_t size)
 static inline uint32_t batadv_choose_backbone_gw(const void *data,
 						 uint32_t size)
 {
-	const unsigned char *key = data;
+	struct batadv_claim *claim = (struct batadv_claim *)data;
 	uint32_t hash = 0;
-	size_t i;
 
-	for (i = 0; i < ETH_ALEN + sizeof(short); i++) {
-		hash += key[i];
-		hash += (hash << 10);
-		hash ^= (hash >> 6);
-	}
+	hash_bytes(&hash, &claim->addr, sizeof(claim->addr));
+	hash_bytes(&hash, &claim->vid, sizeof(claim->vid));
 
 	hash += (hash << 3);
 	hash ^= (hash >> 11);
-- 
1.8.0

^ permalink raw reply related

* [PATCH 02/11] batman-adv: fix wrong spinlock inline comment
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1352798139-19458-1-git-send-email-ordex@autistici.org>

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 8ce16c1..e8a1b18 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -303,7 +303,7 @@ struct batadv_priv {
 	struct hlist_head forw_bcast_list;
 	struct batadv_hashtable *orig_hash;
 	spinlock_t forw_bat_list_lock; /* protects forw_bat_list */
-	spinlock_t forw_bcast_list_lock; /* protects  */
+	spinlock_t forw_bcast_list_lock; /* protects forw_bcast_list */
 	struct delayed_work orig_work;
 	struct batadv_hard_iface __rcu *primary_if;  /* rcu protected pointer */
 	struct batadv_algo_ops *bat_algo_ops;
-- 
1.8.0

^ permalink raw reply related

* [PATCH 03/11] batman-adv: prevent using any virtual device created on batman-adv as hard-interface
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1352798139-19458-1-git-send-email-ordex@autistici.org>

Any virtual device created on top of a batman-adv mesh interface must be
prevented to be used to create a new mesh network (this would lead to an
unwanted batman-over-batman configuration)

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/hard-interface.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 6b7a5d3..365ed74 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -59,6 +59,45 @@ out:
 	return hard_iface;
 }
 
+/**
+ * batadv_is_on_batman_iface - check if a device is a batman iface descendant
+ * @net_dev: the device to check
+ *
+ * If the user creates any virtual device on top of a batman-adv interface, it
+ * is important to prevent this new interface to be used to create a new mesh
+ * network (this behaviour would lead to a batman-over-batman configuration).
+ * This function recursively checks all the fathers of the device passed as
+ * argument looking for a batman-adv soft interface.
+ *
+ * Returns true if the device is descendant of a batman-adv mesh interface (or
+ * if it is a batman-adv interface itself), false otherwise
+ */
+static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
+{
+	struct net_device *parent_dev;
+	bool ret;
+
+	/* check if this is a batman-adv mesh interface */
+	if (batadv_softif_is_valid(net_dev))
+		return true;
+
+	/* no more parents..stop recursion */
+	if (net_dev->iflink == net_dev->ifindex)
+		return false;
+
+	/* recurse over the parent device */
+	parent_dev = dev_get_by_index(&init_net, net_dev->iflink);
+	/* if we got a NULL parent_dev there is something broken.. */
+	if (WARN(!parent_dev, "Cannot find parent device"))
+		return false;
+
+	ret = batadv_is_on_batman_iface(parent_dev);
+
+	if (parent_dev)
+		dev_put(parent_dev);
+	return ret;
+}
+
 static int batadv_is_valid_iface(const struct net_device *net_dev)
 {
 	if (net_dev->flags & IFF_LOOPBACK)
@@ -71,7 +110,7 @@ static int batadv_is_valid_iface(const struct net_device *net_dev)
 		return 0;
 
 	/* no batman over batman */
-	if (batadv_softif_is_valid(net_dev))
+	if (batadv_is_on_batman_iface(net_dev))
 		return 0;
 
 	return 1;
-- 
1.8.0

^ permalink raw reply related

* [PATCH 04/11] batman-adv: send announcement when backbone gw is registered
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem
  Cc: netdev, b.a.t.m.a.n, Simon Wunderlich, Simon Wunderlich,
	Antonio Quartulli
In-Reply-To: <1352798139-19458-1-git-send-email-ordex@autistici.org>

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

To avoid loops in the startup phase until the first announcement is
sent, send an announcement immediately as soon as a backbone gw is
added.

This may happen due to various reasons, e.g. a packet passes the rx
or tx path.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/bridge_loop_avoidance.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index decc042..d1a817f 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -366,7 +366,7 @@ out:
  */
 static struct batadv_backbone_gw *
 batadv_bla_get_backbone_gw(struct batadv_priv *bat_priv, uint8_t *orig,
-			   short vid)
+			   short vid, bool own_backbone)
 {
 	struct batadv_backbone_gw *entry;
 	struct batadv_orig_node *orig_node;
@@ -413,6 +413,10 @@ batadv_bla_get_backbone_gw(struct batadv_priv *bat_priv, uint8_t *orig,
 					  "became a backbone gateway");
 		batadv_orig_node_free_ref(orig_node);
 	}
+
+	if (own_backbone)
+		batadv_bla_send_announce(bat_priv, entry);
+
 	return entry;
 }
 
@@ -428,7 +432,7 @@ batadv_bla_update_own_backbone_gw(struct batadv_priv *bat_priv,
 
 	backbone_gw = batadv_bla_get_backbone_gw(bat_priv,
 						 primary_if->net_dev->dev_addr,
-						 vid);
+						 vid, true);
 	if (unlikely(!backbone_gw))
 		return;
 
@@ -636,7 +640,8 @@ static int batadv_handle_announce(struct batadv_priv *bat_priv,
 	if (memcmp(an_addr, batadv_announce_mac, 4) != 0)
 		return 0;
 
-	backbone_gw = batadv_bla_get_backbone_gw(bat_priv, backbone_addr, vid);
+	backbone_gw = batadv_bla_get_backbone_gw(bat_priv, backbone_addr, vid,
+						 false);
 
 	if (unlikely(!backbone_gw))
 		return 1;
@@ -734,7 +739,8 @@ static int batadv_handle_claim(struct batadv_priv *bat_priv,
 
 	/* register the gateway if not yet available, and add the claim. */
 
-	backbone_gw = batadv_bla_get_backbone_gw(bat_priv, backbone_addr, vid);
+	backbone_gw = batadv_bla_get_backbone_gw(bat_priv, backbone_addr, vid,
+						 false);
 
 	if (unlikely(!backbone_gw))
 		return 1;
-- 
1.8.0

^ permalink raw reply related

* [PATCH 05/11] batman-adv: allow bla traffic only after first worker period
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem
  Cc: netdev, b.a.t.m.a.n, Simon Wunderlich, Simon Wunderlich,
	Antonio Quartulli
In-Reply-To: <1352798139-19458-1-git-send-email-ordex@autistici.org>

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

When adding a backbone gateway for the first time, it might not yet
be known in the backbone, and therefore we should not forward
broadcasts yet. This behaviour is the same as when sending a request
to another backbone gw because of a CRC mismatch. The backbone gw
will operate normal after the next periodic bla work.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/bridge_loop_avoidance.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index d1a817f..77ee47e 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -414,9 +414,14 @@ batadv_bla_get_backbone_gw(struct batadv_priv *bat_priv, uint8_t *orig,
 		batadv_orig_node_free_ref(orig_node);
 	}
 
-	if (own_backbone)
+	if (own_backbone) {
 		batadv_bla_send_announce(bat_priv, entry);
 
+		/* this will be decreased in the worker thread */
+		atomic_inc(&entry->request_sent);
+		atomic_inc(&bat_priv->bla.num_requests);
+	}
+
 	return entry;
 }
 
@@ -1150,6 +1155,19 @@ static void batadv_bla_periodic_work(struct work_struct *work)
 			backbone_gw->lasttime = jiffies;
 
 			batadv_bla_send_announce(bat_priv, backbone_gw);
+
+			/* request_sent is only set after creation to avoid
+			 * problems when we are not yet known as backbone gw
+			 * in the backbone.
+			 *
+			 * We can reset this now and allow traffic again.
+			 */
+
+			if (atomic_read(&backbone_gw->request_sent) == 0)
+				continue;
+
+			atomic_dec(&backbone_gw->bat_priv->bla.num_requests);
+			atomic_set(&backbone_gw->request_sent, 0);
 		}
 		rcu_read_unlock();
 	}
-- 
1.8.0

^ permalink raw reply related

* [PATCH 06/11] batman-adv: wait multiple periods before activating bla
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem
  Cc: netdev, b.a.t.m.a.n, Simon Wunderlich, Simon Wunderlich,
	Antonio Quartulli
In-Reply-To: <1352798139-19458-1-git-send-email-ordex@autistici.org>

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

For some reasons (bridge forward delay, network device setup order, etc)
the initial bridge loop avoidance announcement packets may be lost. This
may lead to problems in finding other backbone gws, and therfore create
loops in the startup time.

Fix this by extending the waiting periods to 3 (define can be changed)
before allowing broadcast traffic.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/bridge_loop_avoidance.c | 9 ++++++++-
 net/batman-adv/main.h                  | 1 +
 net/batman-adv/types.h                 | 1 +
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 77ee47e..415343f 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -390,6 +390,7 @@ batadv_bla_get_backbone_gw(struct batadv_priv *bat_priv, uint8_t *orig,
 	entry->crc = BATADV_BLA_CRC_INIT;
 	entry->bat_priv = bat_priv;
 	atomic_set(&entry->request_sent, 0);
+	atomic_set(&entry->wait_periods, 0);
 	memcpy(entry->orig, orig, ETH_ALEN);
 
 	/* one for the hash, one for returning */
@@ -419,6 +420,7 @@ batadv_bla_get_backbone_gw(struct batadv_priv *bat_priv, uint8_t *orig,
 
 		/* this will be decreased in the worker thread */
 		atomic_inc(&entry->request_sent);
+		atomic_set(&entry->wait_periods, BATADV_BLA_WAIT_PERIODS);
 		atomic_inc(&bat_priv->bla.num_requests);
 	}
 
@@ -1160,12 +1162,17 @@ static void batadv_bla_periodic_work(struct work_struct *work)
 			 * problems when we are not yet known as backbone gw
 			 * in the backbone.
 			 *
-			 * We can reset this now and allow traffic again.
+			 * We can reset this now after we waited some periods
+			 * to give bridge forward delays and bla group forming
+			 * some grace time.
 			 */
 
 			if (atomic_read(&backbone_gw->request_sent) == 0)
 				continue;
 
+			if (!atomic_dec_and_test(&backbone_gw->wait_periods))
+				continue;
+
 			atomic_dec(&backbone_gw->bat_priv->bla.num_requests);
 			atomic_set(&backbone_gw->request_sent, 0);
 		}
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index 240c74f..8f149bb 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -95,6 +95,7 @@
 #define BATADV_BLA_PERIOD_LENGTH	10000	/* 10 seconds */
 #define BATADV_BLA_BACKBONE_TIMEOUT	(BATADV_BLA_PERIOD_LENGTH * 3)
 #define BATADV_BLA_CLAIM_TIMEOUT	(BATADV_BLA_PERIOD_LENGTH * 10)
+#define BATADV_BLA_WAIT_PERIODS		3
 
 #define BATADV_DUPLIST_SIZE		16
 #define BATADV_DUPLIST_TIMEOUT		500	/* 500 ms */
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index e8a1b18..fa09961 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -369,6 +369,7 @@ struct batadv_backbone_gw {
 	struct hlist_node hash_entry;
 	struct batadv_priv *bat_priv;
 	unsigned long lasttime;	/* last time we heard of this backbone gw */
+	atomic_t wait_periods;
 	atomic_t request_sent;
 	atomic_t refcount;
 	struct rcu_head rcu;
-- 
1.8.0

^ permalink raw reply related

* [PATCH 07/11] batman-adv: substitute tt_poss_change with a per-tt_entry flag
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1352798139-19458-1-git-send-email-ordex@autistici.org>

tt_poss_change is a node-wide flag which tells whether the node is in a roaming
state (a client recently moved to/away from it) in order to let it apply special
re-routing rules. However this flag does not give a clear idea of the current
state because it is not possible to understand *which client* is actually
involved in the roaming. For this reason a better approach has been chosen:
instead of using a node-wide variable, the roaming state is now given by a
per-tt_entry ROAM flag which, in case of packet coming through the node, tells
the node whether the real destination is in roaming state or not.

With this flag change, batadv_check_unicast_ttvn() has also been rearranged in
order to better fit the new re-routing logic and to be much more readable.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/originator.c        |   1 -
 net/batman-adv/routing.c           | 179 ++++++++++++++++++++++++++-----------
 net/batman-adv/soft-interface.c    |   1 -
 net/batman-adv/translation-table.c |  46 +++++++---
 net/batman-adv/translation-table.h |   2 +
 net/batman-adv/types.h             |   8 --
 6 files changed, 159 insertions(+), 78 deletions(-)

diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
index 84930a4..8c32cf1 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -221,7 +221,6 @@ struct batadv_orig_node *batadv_get_orig_node(struct batadv_priv *bat_priv,
 	atomic_set(&orig_node->refcount, 2);
 
 	orig_node->tt_initialised = false;
-	orig_node->tt_poss_change = false;
 	orig_node->bat_priv = bat_priv;
 	memcpy(orig_node->orig, addr, ETH_ALEN);
 	batadv_dat_init_orig_node_addr(orig_node);
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 32aa4d4..78d6572 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -711,12 +711,6 @@ int batadv_recv_roam_adv(struct sk_buff *skb, struct batadv_hard_iface *recv_if)
 			     BATADV_TT_CLIENT_ROAM,
 			     atomic_read(&orig_node->last_ttvn) + 1);
 
-	/* Roaming phase starts: I have new information but the ttvn has not
-	 * been incremented yet. This flag will make me check all the incoming
-	 * packets for the correct destination.
-	 */
-	bat_priv->tt.poss_change = true;
-
 	batadv_orig_node_free_ref(orig_node);
 out:
 	/* returning NET_RX_DROP will make the caller function kfree the skb */
@@ -899,14 +893,67 @@ out:
 	return ret;
 }
 
+/**
+ * batadv_reroute_unicast_packet - update the unicast header for re-routing
+ * @bat_priv: the bat priv with all the soft interface information
+ * @unicast_packet: the unicast header to be updated
+ * @dst_addr: the payload destination
+ *
+ * Search the translation table for dst_addr and update the unicast header with
+ * the new corresponding information (originator address where the destination
+ * client currently is and its known TTVN)
+ *
+ * Returns true if the packet header has been updated, false otherwise
+ */
+static bool
+batadv_reroute_unicast_packet(struct batadv_priv *bat_priv,
+			      struct batadv_unicast_packet *unicast_packet,
+			      uint8_t *dst_addr)
+{
+	struct batadv_orig_node *orig_node = NULL;
+	struct batadv_hard_iface *primary_if = NULL;
+	bool ret = false;
+	uint8_t *orig_addr, orig_ttvn;
+
+	if (batadv_is_my_client(bat_priv, dst_addr)) {
+		primary_if = batadv_primary_if_get_selected(bat_priv);
+		if (!primary_if)
+			goto out;
+		orig_addr = primary_if->net_dev->dev_addr;
+		orig_ttvn = (uint8_t)atomic_read(&bat_priv->tt.vn);
+	} else {
+		orig_node = batadv_transtable_search(bat_priv, NULL, dst_addr);
+		if (!orig_node)
+			goto out;
+
+		if (batadv_compare_eth(orig_node->orig, unicast_packet->dest))
+			goto out;
+
+		orig_addr = orig_node->orig;
+		orig_ttvn = (uint8_t)atomic_read(&orig_node->last_ttvn);
+	}
+
+	/* update the packet header */
+	memcpy(unicast_packet->dest, orig_addr, ETH_ALEN);
+	unicast_packet->ttvn = orig_ttvn;
+
+	ret = true;
+out:
+	if (primary_if)
+		batadv_hardif_free_ref(primary_if);
+	if (orig_node)
+		batadv_orig_node_free_ref(orig_node);
+
+	return ret;
+}
+
 static int batadv_check_unicast_ttvn(struct batadv_priv *bat_priv,
 				     struct sk_buff *skb) {
-	uint8_t curr_ttvn;
+	uint8_t curr_ttvn, old_ttvn;
 	struct batadv_orig_node *orig_node;
 	struct ethhdr *ethhdr;
 	struct batadv_hard_iface *primary_if;
 	struct batadv_unicast_packet *unicast_packet;
-	bool tt_poss_change;
 	int is_old_ttvn;
 
 	/* check if there is enough data before accessing it */
@@ -918,65 +965,89 @@ static int batadv_check_unicast_ttvn(struct batadv_priv *bat_priv,
 		return 0;
 
 	unicast_packet = (struct batadv_unicast_packet *)skb->data;
+	ethhdr = (struct ethhdr *)(skb->data + sizeof(*unicast_packet));
 
-	if (batadv_is_my_mac(unicast_packet->dest)) {
-		tt_poss_change = bat_priv->tt.poss_change;
-		curr_ttvn = (uint8_t)atomic_read(&bat_priv->tt.vn);
-	} else {
+	/* check if the destination client was served by this node and it is now
+	 * roaming. In this case, it means that the node has got a ROAM_ADV
+	 * message and that it knows the new destination in the mesh to re-route
+	 * the packet to
+	 */
+	if (batadv_tt_local_client_is_roaming(bat_priv, ethhdr->h_dest)) {
+		if (batadv_reroute_unicast_packet(bat_priv, unicast_packet,
+						  ethhdr->h_dest))
+			net_ratelimited_function(batadv_dbg, BATADV_DBG_TT,
+						 bat_priv,
+						 "Rerouting unicast packet to %pM (dst=%pM): Local Roaming\n",
+						 unicast_packet->dest,
+						 ethhdr->h_dest);
+		/* at this point the mesh destination should have been
+		 * substituted with the originator address found in the global
+		 * table. If not, let the packet go untouched anyway because
+		 * there is nothing the node can do
+		 */
+		return 1;
+	}
+
+	/* retrieve the TTVN known by this node for the packet destination. This
+	 * value is used later to check if the node which sent (or re-routed
+	 * last time) the packet had an updated information or not
+	 */
+	curr_ttvn = (uint8_t)atomic_read(&bat_priv->tt.vn);
+	if (!batadv_is_my_mac(unicast_packet->dest)) {
 		orig_node = batadv_orig_hash_find(bat_priv,
 						  unicast_packet->dest);
-
+		/* if it is not possible to find the orig_node representing the
+		 * destination, the packet can immediately be dropped as it will
+		 * not be possible to deliver it
+		 */
 		if (!orig_node)
 			return 0;
 
 		curr_ttvn = (uint8_t)atomic_read(&orig_node->last_ttvn);
-		tt_poss_change = orig_node->tt_poss_change;
 		batadv_orig_node_free_ref(orig_node);
 	}
 
-	/* Check whether I have to reroute the packet */
+	/* check if the TTVN contained in the packet is fresher than what the
+	 * node knows
+	 */
 	is_old_ttvn = batadv_seq_before(unicast_packet->ttvn, curr_ttvn);
-	if (is_old_ttvn || tt_poss_change) {
-		/* check if there is enough data before accessing it */
-		if (pskb_may_pull(skb, sizeof(struct batadv_unicast_packet) +
-				  ETH_HLEN) < 0)
-			return 0;
-
-		ethhdr = (struct ethhdr *)(skb->data + sizeof(*unicast_packet));
-
-		/* we don't have an updated route for this client, so we should
-		 * not try to reroute the packet!!
-		 */
-		if (batadv_tt_global_client_is_roaming(bat_priv,
-						       ethhdr->h_dest))
-			return 1;
-
-		orig_node = batadv_transtable_search(bat_priv, NULL,
-						     ethhdr->h_dest);
-
-		if (!orig_node) {
-			if (!batadv_is_my_client(bat_priv, ethhdr->h_dest))
-				return 0;
-			primary_if = batadv_primary_if_get_selected(bat_priv);
-			if (!primary_if)
-				return 0;
-			memcpy(unicast_packet->dest,
-			       primary_if->net_dev->dev_addr, ETH_ALEN);
-			batadv_hardif_free_ref(primary_if);
-		} else {
-			memcpy(unicast_packet->dest, orig_node->orig,
-			       ETH_ALEN);
-			curr_ttvn = (uint8_t)atomic_read(&orig_node->last_ttvn);
-			batadv_orig_node_free_ref(orig_node);
-		}
-
+	if (!is_old_ttvn)
+		return 1;
+
+	old_ttvn = unicast_packet->ttvn;
+	/* the packet was forged based on outdated network information. Its
+	 * destination can possibly be updated and forwarded towards the new
+	 * target host
+	 */
+	if (batadv_reroute_unicast_packet(bat_priv, unicast_packet,
+					  ethhdr->h_dest)) {
 		net_ratelimited_function(batadv_dbg, BATADV_DBG_TT, bat_priv,
-					 "TTVN mismatch (old_ttvn %u new_ttvn %u)! Rerouting unicast packet (for %pM) to %pM\n",
-					 unicast_packet->ttvn, curr_ttvn,
-					 ethhdr->h_dest, unicast_packet->dest);
-
-		unicast_packet->ttvn = curr_ttvn;
+					 "Rerouting unicast packet to %pM (dst=%pM): TTVN mismatch old_ttvn=%u new_ttvn=%u\n",
+					 unicast_packet->dest, ethhdr->h_dest,
+					 old_ttvn, curr_ttvn);
+		return 1;
 	}
+
+	/* the packet has not been re-routed: either the destination is
+	 * currently served by this node or there is no destination at all and
+	 * it is possible to drop the packet
+	 */
+	if (!batadv_is_my_client(bat_priv, ethhdr->h_dest))
+		return 0;
+
+	/* update the header in order to let the packet be delivered to this
+	 * node's soft interface
+	 */
+	primary_if = batadv_primary_if_get_selected(bat_priv);
+	if (!primary_if)
+		return 0;
+
+	memcpy(unicast_packet->dest, primary_if->net_dev->dev_addr, ETH_ALEN);
+
+	batadv_hardif_free_ref(primary_if);
+
+	unicast_packet->ttvn = curr_ttvn;
+
 	return 1;
 }
 
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index c283d87..2d1f895 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -505,7 +505,6 @@ struct net_device *batadv_softif_create(const char *name)
 #endif
 	bat_priv->tt.last_changeset = NULL;
 	bat_priv->tt.last_changeset_len = 0;
-	bat_priv->tt.poss_change = false;
 
 	bat_priv->primary_if = NULL;
 	bat_priv->num_ifaces = 0;
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index f8b9c32..d741851 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -306,8 +306,6 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
 		head = &tt_global_entry->orig_list;
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(orig_entry, node, head, list) {
-			orig_entry->orig_node->tt_poss_change = true;
-
 			batadv_send_roam_adv(bat_priv,
 					     tt_global_entry->common.addr,
 					     orig_entry->orig_node);
@@ -512,8 +510,11 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv,
 	curr_flags = tt_local_entry->common.flags;
 
 	flags = BATADV_TT_CLIENT_DEL;
-	if (roaming)
+	if (roaming) {
 		flags |= BATADV_TT_CLIENT_ROAM;
+		/* mark the local client as ROAMed */
+		tt_local_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
+	}
 
 	batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags, message);
 
@@ -1945,7 +1946,8 @@ bool batadv_is_my_client(struct batadv_priv *bat_priv, const uint8_t *addr)
 	/* Check if the client has been logically deleted (but is kept for
 	 * consistency purpose)
 	 */
-	if (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING)
+	if ((tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) ||
+	    (tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM))
 		goto out;
 	ret = true;
 out:
@@ -1996,10 +1998,6 @@ void batadv_handle_tt_response(struct batadv_priv *bat_priv,
 
 	/* Recalculate the CRC for this orig_node and store it */
 	orig_node->tt_crc = batadv_tt_global_crc(bat_priv, orig_node);
-	/* Roaming phase is over: tables are in sync again. I can
-	 * unset the flag
-	 */
-	orig_node->tt_poss_change = false;
 out:
 	if (orig_node)
 		batadv_orig_node_free_ref(orig_node);
@@ -2290,7 +2288,6 @@ static int batadv_tt_commit_changes(struct batadv_priv *bat_priv,
 	batadv_dbg(BATADV_DBG_TT, bat_priv,
 		   "Local changes committed, updating to ttvn %u\n",
 		   (uint8_t)atomic_read(&bat_priv->tt.vn));
-	bat_priv->tt.poss_change = false;
 
 	/* reset the sending counter */
 	atomic_set(&bat_priv->tt.ogm_append_cnt, BATADV_TT_OGM_APPEND_MAX);
@@ -2402,11 +2399,6 @@ void batadv_tt_update_orig(struct batadv_priv *bat_priv,
 		 */
 		if (orig_node->tt_crc != tt_crc)
 			goto request_table;
-
-		/* Roaming phase is over: tables are in sync again. I can
-		 * unset the flag
-		 */
-		orig_node->tt_poss_change = false;
 	} else {
 		/* if we missed more than one change or our tables are not
 		 * in sync anymore -> request fresh tt data
@@ -2445,6 +2437,32 @@ out:
 	return ret;
 }
 
+/**
+ * batadv_tt_local_client_is_roaming - tells whether the client is roaming
+ * @bat_priv: the bat priv with all the soft interface information
+ * @addr: the MAC address of the local client to query
+ *
+ * Returns true if the local client is known to be roaming (it is not served by
+ * this node anymore) or not. If yes, the client is still present in the table
+ * to keep the latter consistent with the node TTVN
+ */
+bool batadv_tt_local_client_is_roaming(struct batadv_priv *bat_priv,
+				       uint8_t *addr)
+{
+	struct batadv_tt_local_entry *tt_local_entry;
+	bool ret = false;
+
+	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr);
+	if (!tt_local_entry)
+		goto out;
+
+	ret = tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM;
+	batadv_tt_local_entry_free_ref(tt_local_entry);
+out:
+	return ret;
+
+}
+
 bool batadv_tt_add_temporary_global_entry(struct batadv_priv *bat_priv,
 					  struct batadv_orig_node *orig_node,
 					  const unsigned char *addr)
diff --git a/net/batman-adv/translation-table.h b/net/batman-adv/translation-table.h
index 9fa4fe4..46d4451 100644
--- a/net/batman-adv/translation-table.h
+++ b/net/batman-adv/translation-table.h
@@ -59,6 +59,8 @@ int batadv_tt_append_diff(struct batadv_priv *bat_priv,
 			  int packet_min_len);
 bool batadv_tt_global_client_is_roaming(struct batadv_priv *bat_priv,
 					uint8_t *addr);
+bool batadv_tt_local_client_is_roaming(struct batadv_priv *bat_priv,
+				       uint8_t *addr);
 bool batadv_tt_add_temporary_global_entry(struct batadv_priv *bat_priv,
 					  struct batadv_orig_node *orig_node,
 					  const unsigned char *addr);
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index fa09961..7b3d0d7 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -101,13 +101,6 @@ struct batadv_orig_node {
 	spinlock_t tt_buff_lock; /* protects tt_buff */
 	atomic_t tt_size;
 	bool tt_initialised;
-	/* The tt_poss_change flag is used to detect an ongoing roaming phase.
-	 * If true, then I sent a Roaming_adv to this orig_node and I have to
-	 * inspect every packet directed to it to check whether it is still
-	 * the true destination or not. This flag will be reset to false as
-	 * soon as I receive a new TTVN from this orig_node
-	 */
-	bool tt_poss_change;
 	uint32_t last_real_seqno;
 	uint8_t last_ttl;
 	DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
@@ -212,7 +205,6 @@ struct batadv_priv_tt {
 	atomic_t vn;
 	atomic_t ogm_append_cnt;
 	atomic_t local_changes;
-	bool poss_change;
 	struct list_head changes_list;
 	struct batadv_hashtable *local_hash;
 	struct batadv_hashtable *global_hash;
-- 
1.8.0

^ permalink raw reply related

* [PATCH 08/11] batman-adv: refactor code to simplify long lines
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1352798139-19458-1-git-send-email-ordex@autistici.org>

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/translation-table.c | 62 ++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index d741851..c24e604 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -242,86 +242,84 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
 			 int ifindex)
 {
 	struct batadv_priv *bat_priv = netdev_priv(soft_iface);
-	struct batadv_tt_local_entry *tt_local_entry = NULL;
-	struct batadv_tt_global_entry *tt_global_entry = NULL;
+	struct batadv_tt_local_entry *tt_local = NULL;
+	struct batadv_tt_global_entry *tt_global = NULL;
 	struct hlist_head *head;
 	struct hlist_node *node;
 	struct batadv_tt_orig_list_entry *orig_entry;
 	int hash_added;
 
-	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr);
+	tt_local = batadv_tt_local_hash_find(bat_priv, addr);
 
-	if (tt_local_entry) {
-		tt_local_entry->last_seen = jiffies;
+	if (tt_local) {
+		tt_local->last_seen = jiffies;
 		/* possibly unset the BATADV_TT_CLIENT_PENDING flag */
-		tt_local_entry->common.flags &= ~BATADV_TT_CLIENT_PENDING;
+		tt_local->common.flags &= ~BATADV_TT_CLIENT_PENDING;
 		goto out;
 	}
 
-	tt_local_entry = kmalloc(sizeof(*tt_local_entry), GFP_ATOMIC);
-	if (!tt_local_entry)
+	tt_local = kmalloc(sizeof(*tt_local), GFP_ATOMIC);
+	if (!tt_local)
 		goto out;
 
 	batadv_dbg(BATADV_DBG_TT, bat_priv,
 		   "Creating new local tt entry: %pM (ttvn: %d)\n", addr,
 		   (uint8_t)atomic_read(&bat_priv->tt.vn));
 
-	memcpy(tt_local_entry->common.addr, addr, ETH_ALEN);
-	tt_local_entry->common.flags = BATADV_NO_FLAGS;
+	memcpy(tt_local->common.addr, addr, ETH_ALEN);
+	tt_local->common.flags = BATADV_NO_FLAGS;
 	if (batadv_is_wifi_iface(ifindex))
-		tt_local_entry->common.flags |= BATADV_TT_CLIENT_WIFI;
-	atomic_set(&tt_local_entry->common.refcount, 2);
-	tt_local_entry->last_seen = jiffies;
-	tt_local_entry->common.added_at = tt_local_entry->last_seen;
+		tt_local->common.flags |= BATADV_TT_CLIENT_WIFI;
+	atomic_set(&tt_local->common.refcount, 2);
+	tt_local->last_seen = jiffies;
+	tt_local->common.added_at = tt_local->last_seen;
 
 	/* the batman interface mac address should never be purged */
 	if (batadv_compare_eth(addr, soft_iface->dev_addr))
-		tt_local_entry->common.flags |= BATADV_TT_CLIENT_NOPURGE;
+		tt_local->common.flags |= BATADV_TT_CLIENT_NOPURGE;
 
 	/* The local entry has to be marked as NEW to avoid to send it in
 	 * a full table response going out before the next ttvn increment
 	 * (consistency check)
 	 */
-	tt_local_entry->common.flags |= BATADV_TT_CLIENT_NEW;
+	tt_local->common.flags |= BATADV_TT_CLIENT_NEW;
 
 	hash_added = batadv_hash_add(bat_priv->tt.local_hash, batadv_compare_tt,
-				     batadv_choose_orig,
-				     &tt_local_entry->common,
-				     &tt_local_entry->common.hash_entry);
+				     batadv_choose_orig, &tt_local->common,
+				     &tt_local->common.hash_entry);
 
 	if (unlikely(hash_added != 0)) {
 		/* remove the reference for the hash */
-		batadv_tt_local_entry_free_ref(tt_local_entry);
+		batadv_tt_local_entry_free_ref(tt_local);
 		goto out;
 	}
 
-	batadv_tt_local_event(bat_priv, addr, tt_local_entry->common.flags);
+	batadv_tt_local_event(bat_priv, addr, tt_local->common.flags);
 
 	/* remove address from global hash if present */
-	tt_global_entry = batadv_tt_global_hash_find(bat_priv, addr);
+	tt_global = batadv_tt_global_hash_find(bat_priv, addr);
 
 	/* Check whether it is a roaming! */
-	if (tt_global_entry) {
+	if (tt_global) {
 		/* These node are probably going to update their tt table */
-		head = &tt_global_entry->orig_list;
+		head = &tt_global->orig_list;
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(orig_entry, node, head, list) {
-			batadv_send_roam_adv(bat_priv,
-					     tt_global_entry->common.addr,
+			batadv_send_roam_adv(bat_priv, tt_global->common.addr,
 					     orig_entry->orig_node);
 		}
 		rcu_read_unlock();
 		/* The global entry has to be marked as ROAMING and
 		 * has to be kept for consistency purpose
 		 */
-		tt_global_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
-		tt_global_entry->roam_at = jiffies;
+		tt_global->common.flags |= BATADV_TT_CLIENT_ROAM;
+		tt_global->roam_at = jiffies;
 	}
 out:
-	if (tt_local_entry)
-		batadv_tt_local_entry_free_ref(tt_local_entry);
-	if (tt_global_entry)
-		batadv_tt_global_entry_free_ref(tt_global_entry);
+	if (tt_local)
+		batadv_tt_local_entry_free_ref(tt_local);
+	if (tt_global)
+		batadv_tt_global_entry_free_ref(tt_global);
 }
 
 static void batadv_tt_realloc_packet_buff(unsigned char **packet_buff,
-- 
1.8.0

^ permalink raw reply related

* [PATCH 09/11] batman-adv: refactor tt_global_del_struct()
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1352798139-19458-1-git-send-email-ordex@autistici.org>

batadv_tt_global_del_struct() function is not properly named.
Having a more meaningful name which reflects the current behavior helps other
developers to easily understand what it does.

A parameter has also been renamed in order to let the function header better fit
the 80-chars line-width

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/translation-table.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index c24e604..cb8433a 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -927,18 +927,17 @@ batadv_tt_global_del_orig_entry(struct batadv_priv *bat_priv,
 	spin_unlock_bh(&tt_global_entry->list_lock);
 }
 
-static void
-batadv_tt_global_del_struct(struct batadv_priv *bat_priv,
-			    struct batadv_tt_global_entry *tt_global_entry,
-			    const char *message)
+static void batadv_tt_global_free(struct batadv_priv *bat_priv,
+				  struct batadv_tt_global_entry *tt_global,
+				  const char *message)
 {
 	batadv_dbg(BATADV_DBG_TT, bat_priv,
 		   "Deleting global tt entry %pM: %s\n",
-		   tt_global_entry->common.addr, message);
+		   tt_global->common.addr, message);
 
 	batadv_hash_remove(bat_priv->tt.global_hash, batadv_compare_tt,
-			   batadv_choose_orig, tt_global_entry->common.addr);
-	batadv_tt_global_entry_free_ref(tt_global_entry);
+			   batadv_choose_orig, tt_global->common.addr);
+	batadv_tt_global_entry_free_ref(tt_global);
 
 }
 
@@ -1002,8 +1001,8 @@ static void batadv_tt_global_del(struct batadv_priv *bat_priv,
 						orig_node, message);
 
 		if (hlist_empty(&tt_global_entry->orig_list))
-			batadv_tt_global_del_struct(bat_priv, tt_global_entry,
-						    message);
+			batadv_tt_global_free(bat_priv, tt_global_entry,
+					      message);
 
 		goto out;
 	}
@@ -1026,7 +1025,7 @@ static void batadv_tt_global_del(struct batadv_priv *bat_priv,
 	if (local_entry) {
 		/* local entry exists, case 2: client roamed to us. */
 		batadv_tt_global_del_orig_list(tt_global_entry);
-		batadv_tt_global_del_struct(bat_priv, tt_global_entry, message);
+		batadv_tt_global_free(bat_priv, tt_global_entry, message);
 	} else
 		/* no local entry exists, case 1: check for roaming */
 		batadv_tt_global_del_roaming(bat_priv, tt_global_entry,
-- 
1.8.0

^ permalink raw reply related

* [PATCH 10/11] batman-adv: roaming handling mechanism redesign
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1352798139-19458-1-git-send-email-ordex@autistici.org>

This patch allows clients to roam multiple times within the same
originator-interval.

To enable this new feature two key aspects that have been introduced:
1) packets are always directed to the node that was originally
serving the roamed client which will then re-route the data
to the correct destination at any point in time;
2) the client flags handling mechanism has been properly modified
in order to allow multiple roamings withinin the same orig-int.
Therefore flags are now set properly even in this scenario.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/translation-table.c | 157 ++++++++++++++++++++++++++++---------
 1 file changed, 118 insertions(+), 39 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index cb8433a..dd2c254 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -238,6 +238,20 @@ static int batadv_tt_local_init(struct batadv_priv *bat_priv)
 	return 0;
 }
 
+static void batadv_tt_global_free(struct batadv_priv *bat_priv,
+				  struct batadv_tt_global_entry *tt_global,
+				  const char *message)
+{
+	batadv_dbg(BATADV_DBG_TT, bat_priv,
+		   "Deleting global tt entry %pM: %s\n",
+		   tt_global->common.addr, message);
+
+	batadv_hash_remove(bat_priv->tt.global_hash, batadv_compare_tt,
+			   batadv_choose_orig, tt_global->common.addr);
+	batadv_tt_global_entry_free_ref(tt_global);
+
+}
+
 void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
 			 int ifindex)
 {
@@ -248,14 +262,38 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
 	struct hlist_node *node;
 	struct batadv_tt_orig_list_entry *orig_entry;
 	int hash_added;
+	bool roamed_back = false;
 
 	tt_local = batadv_tt_local_hash_find(bat_priv, addr);
+	tt_global = batadv_tt_global_hash_find(bat_priv, addr);
 
 	if (tt_local) {
 		tt_local->last_seen = jiffies;
-		/* possibly unset the BATADV_TT_CLIENT_PENDING flag */
-		tt_local->common.flags &= ~BATADV_TT_CLIENT_PENDING;
-		goto out;
+		if (tt_local->common.flags & BATADV_TT_CLIENT_PENDING) {
+			batadv_dbg(BATADV_DBG_TT, bat_priv,
+				   "Re-adding pending client %pM\n", addr);
+			/* whatever the reason why the PENDING flag was set,
+			 * this is a client which was enqueued to be removed in
+			 * this orig_interval. Since it popped up again, the
+			 * flag can be reset like it was never enqueued
+			 */
+			tt_local->common.flags &= ~BATADV_TT_CLIENT_PENDING;
+			goto add_event;
+		}
+
+		if (tt_local->common.flags & BATADV_TT_CLIENT_ROAM) {
+			batadv_dbg(BATADV_DBG_TT, bat_priv,
+				   "Roaming client %pM came back to its original location\n",
+				   addr);
+			/* the ROAM flag is set because this client roamed away
+			 * and the node got a roaming_advertisement message. Now
+			 * that the client popped up again at its original
+			 * location such flag can be unset
+			 */
+			tt_local->common.flags &= ~BATADV_TT_CLIENT_ROAM;
+			roamed_back = true;
+		}
+		goto check_roaming;
 	}
 
 	tt_local = kmalloc(sizeof(*tt_local), GFP_ATOMIC);
@@ -294,13 +332,14 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
 		goto out;
 	}
 
+add_event:
 	batadv_tt_local_event(bat_priv, addr, tt_local->common.flags);
 
-	/* remove address from global hash if present */
-	tt_global = batadv_tt_global_hash_find(bat_priv, addr);
-
-	/* Check whether it is a roaming! */
-	if (tt_global) {
+check_roaming:
+	/* Check whether it is a roaming, but don't do anything if the roaming
+	 * process has already been handled
+	 */
+	if (tt_global && !(tt_global->common.flags & BATADV_TT_CLIENT_ROAM)) {
 		/* These node are probably going to update their tt table */
 		head = &tt_global->orig_list;
 		rcu_read_lock();
@@ -309,12 +348,19 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
 					     orig_entry->orig_node);
 		}
 		rcu_read_unlock();
-		/* The global entry has to be marked as ROAMING and
-		 * has to be kept for consistency purpose
-		 */
-		tt_global->common.flags |= BATADV_TT_CLIENT_ROAM;
-		tt_global->roam_at = jiffies;
+		if (roamed_back) {
+			batadv_tt_global_free(bat_priv, tt_global,
+					      "Roaming canceled");
+			tt_global = NULL;
+		} else {
+			/* The global entry has to be marked as ROAMING and
+			 * has to be kept for consistency purpose
+			 */
+			tt_global->common.flags |= BATADV_TT_CLIENT_ROAM;
+			tt_global->roam_at = jiffies;
+		}
 	}
+
 out:
 	if (tt_local)
 		batadv_tt_local_entry_free_ref(tt_local);
@@ -508,13 +554,28 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv,
 	curr_flags = tt_local_entry->common.flags;
 
 	flags = BATADV_TT_CLIENT_DEL;
+	/* if this global entry addition is due to a roaming, the node has to
+	 * mark the local entry as "roamed" in order to correctly reroute
+	 * packets later
+	 */
 	if (roaming) {
 		flags |= BATADV_TT_CLIENT_ROAM;
 		/* mark the local client as ROAMed */
 		tt_local_entry->common.flags |= BATADV_TT_CLIENT_ROAM;
 	}
 
-	batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags, message);
+	if (!(tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW)) {
+		batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags,
+					    message);
+		goto out;
+	}
+	/* if this client has been added right now, it is possible to
+	 * immediately purge it
+	 */
+	batadv_tt_local_event(bat_priv, tt_local_entry->common.addr,
+			      curr_flags | BATADV_TT_CLIENT_DEL);
+	hlist_del_rcu(&tt_local_entry->common.hash_entry);
+	batadv_tt_local_entry_free_ref(tt_local_entry);
 
 out:
 	if (tt_local_entry)
@@ -724,12 +785,22 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 			 uint8_t ttvn)
 {
 	struct batadv_tt_global_entry *tt_global_entry = NULL;
+	struct batadv_tt_local_entry *tt_local_entry = NULL;
 	int ret = 0;
 	int hash_added;
 	struct batadv_tt_common_entry *common;
 	uint16_t local_flags;
 
 	tt_global_entry = batadv_tt_global_hash_find(bat_priv, tt_addr);
+	tt_local_entry = batadv_tt_local_hash_find(bat_priv, tt_addr);
+
+	/* if the node already has a local client for this entry, it has to wait
+	 * for a roaming advertisement instead of manually messing up the global
+	 * table
+	 */
+	if ((flags & BATADV_TT_CLIENT_TEMP) && tt_local_entry &&
+	    !(tt_local_entry->common.flags & BATADV_TT_CLIENT_NEW))
+		goto out;
 
 	if (!tt_global_entry) {
 		tt_global_entry = kzalloc(sizeof(*tt_global_entry), GFP_ATOMIC);
@@ -764,19 +835,31 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 			goto out_remove;
 		}
 	} else {
+		common = &tt_global_entry->common;
 		/* If there is already a global entry, we can use this one for
 		 * our processing.
-		 * But if we are trying to add a temporary client we can exit
-		 * directly because the temporary information should never
-		 * override any already known client state (whatever it is)
+		 * But if we are trying to add a temporary client then here are
+		 * two options at this point:
+		 * 1) the global client is not a temporary client: the global
+		 *    client has to be left as it is, temporary information
+		 *    should never override any already known client state
+		 * 2) the global client is a temporary client: purge the
+		 *    originator list and add the new one orig_entry
 		 */
-		if (flags & BATADV_TT_CLIENT_TEMP)
-			goto out;
+		if (flags & BATADV_TT_CLIENT_TEMP) {
+			if (!(common->flags & BATADV_TT_CLIENT_TEMP))
+				goto out;
+			if (batadv_tt_global_entry_has_orig(tt_global_entry,
+							    orig_node))
+				goto out_remove;
+			batadv_tt_global_del_orig_list(tt_global_entry);
+			goto add_orig_entry;
+		}
 
 		/* if the client was temporary added before receiving the first
 		 * OGM announcing it, we have to clear the TEMP flag
 		 */
-		tt_global_entry->common.flags &= ~BATADV_TT_CLIENT_TEMP;
+		common->flags &= ~BATADV_TT_CLIENT_TEMP;
 
 		/* If there is the BATADV_TT_CLIENT_ROAM flag set, there is only
 		 * one originator left in the list and we previously received a
@@ -785,18 +868,19 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 		 * We should first delete the old originator before adding the
 		 * new one.
 		 */
-		if (tt_global_entry->common.flags & BATADV_TT_CLIENT_ROAM) {
+		if (common->flags & BATADV_TT_CLIENT_ROAM) {
 			batadv_tt_global_del_orig_list(tt_global_entry);
-			tt_global_entry->common.flags &= ~BATADV_TT_CLIENT_ROAM;
+			common->flags &= ~BATADV_TT_CLIENT_ROAM;
 			tt_global_entry->roam_at = 0;
 		}
 	}
+add_orig_entry:
 	/* add the new orig_entry (if needed) or update it */
 	batadv_tt_global_orig_entry_add(tt_global_entry, orig_node, ttvn);
 
 	batadv_dbg(BATADV_DBG_TT, bat_priv,
 		   "Creating new global tt entry: %pM (via %pM)\n",
-		   tt_global_entry->common.addr, orig_node->orig);
+		   common->addr, orig_node->orig);
 	ret = 1;
 
 out_remove:
@@ -804,12 +888,20 @@ out_remove:
 	/* remove address from local hash if present */
 	local_flags = batadv_tt_local_remove(bat_priv, tt_addr,
 					     "global tt received",
-					     flags & BATADV_TT_CLIENT_ROAM);
+					     !!(flags & BATADV_TT_CLIENT_ROAM));
 	tt_global_entry->common.flags |= local_flags & BATADV_TT_CLIENT_WIFI;
 
+	if (!(flags & BATADV_TT_CLIENT_ROAM))
+		/* this is a normal global add. Therefore the client is not in a
+		 * roaming state anymore.
+		 */
+		tt_global_entry->common.flags &= ~BATADV_TT_CLIENT_ROAM;
+
 out:
 	if (tt_global_entry)
 		batadv_tt_global_entry_free_ref(tt_global_entry);
+	if (tt_local_entry)
+		batadv_tt_local_entry_free_ref(tt_local_entry);
 	return ret;
 }
 
@@ -927,20 +1019,6 @@ batadv_tt_global_del_orig_entry(struct batadv_priv *bat_priv,
 	spin_unlock_bh(&tt_global_entry->list_lock);
 }
 
-static void batadv_tt_global_free(struct batadv_priv *bat_priv,
-				  struct batadv_tt_global_entry *tt_global,
-				  const char *message)
-{
-	batadv_dbg(BATADV_DBG_TT, bat_priv,
-		   "Deleting global tt entry %pM: %s\n",
-		   tt_global->common.addr, message);
-
-	batadv_hash_remove(bat_priv->tt.global_hash, batadv_compare_tt,
-			   batadv_choose_orig, tt_global->common.addr);
-	batadv_tt_global_entry_free_ref(tt_global);
-
-}
-
 /* If the client is to be deleted, we check if it is the last origantor entry
  * within tt_global entry. If yes, we set the BATADV_TT_CLIENT_ROAM flag and the
  * timer, otherwise we simply remove the originator scheduled for deletion.
@@ -1204,7 +1282,8 @@ struct batadv_orig_node *batadv_transtable_search(struct batadv_priv *bat_priv,
 
 	if (src && atomic_read(&bat_priv->ap_isolation)) {
 		tt_local_entry = batadv_tt_local_hash_find(bat_priv, src);
-		if (!tt_local_entry)
+		if (!tt_local_entry ||
+		    (tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING))
 			goto out;
 	}
 
-- 
1.8.0

^ permalink raw reply related

* [PATCH 11/11] batman-adv: Remove instant overwritten variable initialization
From: Antonio Quartulli @ 2012-11-13  9:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Marek Lindner,
	Antonio Quartulli
In-Reply-To: <1352798139-19458-1-git-send-email-ordex@autistici.org>

From: Sven Eckelmann <sven@narfation.org>

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/sysfs.c             |  2 +-
 net/batman-adv/translation-table.c | 20 ++++++++++----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index fa3cc1a..84a55cb 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -688,7 +688,7 @@ int batadv_throw_uevent(struct batadv_priv *bat_priv, enum batadv_uev_type type,
 			enum batadv_uev_action action, const char *data)
 {
 	int ret = -ENOMEM;
-	struct batadv_hard_iface *primary_if = NULL;
+	struct batadv_hard_iface *primary_if;
 	struct kobject *bat_kobj;
 	char *uevent_env[4] = { NULL, NULL, NULL, NULL };
 
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index dd2c254..9f5705f 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -256,8 +256,8 @@ void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
 			 int ifindex)
 {
 	struct batadv_priv *bat_priv = netdev_priv(soft_iface);
-	struct batadv_tt_local_entry *tt_local = NULL;
-	struct batadv_tt_global_entry *tt_global = NULL;
+	struct batadv_tt_local_entry *tt_local;
+	struct batadv_tt_global_entry *tt_global;
 	struct hlist_head *head;
 	struct hlist_node *node;
 	struct batadv_tt_orig_list_entry *orig_entry;
@@ -544,7 +544,7 @@ uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv,
 				const uint8_t *addr, const char *message,
 				bool roaming)
 {
-	struct batadv_tt_local_entry *tt_local_entry = NULL;
+	struct batadv_tt_local_entry *tt_local_entry;
 	uint16_t flags, curr_flags = BATADV_NO_FLAGS;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr);
@@ -784,8 +784,8 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 			 const unsigned char *tt_addr, uint8_t flags,
 			 uint8_t ttvn)
 {
-	struct batadv_tt_global_entry *tt_global_entry = NULL;
-	struct batadv_tt_local_entry *tt_local_entry = NULL;
+	struct batadv_tt_global_entry *tt_global_entry;
+	struct batadv_tt_local_entry *tt_local_entry;
 	int ret = 0;
 	int hash_added;
 	struct batadv_tt_common_entry *common;
@@ -1067,7 +1067,7 @@ static void batadv_tt_global_del(struct batadv_priv *bat_priv,
 				 const unsigned char *addr,
 				 const char *message, bool roaming)
 {
-	struct batadv_tt_global_entry *tt_global_entry = NULL;
+	struct batadv_tt_global_entry *tt_global_entry;
 	struct batadv_tt_local_entry *local_entry = NULL;
 
 	tt_global_entry = batadv_tt_global_hash_find(bat_priv, addr);
@@ -1670,7 +1670,7 @@ static bool
 batadv_send_other_tt_response(struct batadv_priv *bat_priv,
 			      struct batadv_tt_query_packet *tt_request)
 {
-	struct batadv_orig_node *req_dst_orig_node = NULL;
+	struct batadv_orig_node *req_dst_orig_node;
 	struct batadv_orig_node *res_dst_orig_node = NULL;
 	struct batadv_neigh_node *neigh_node = NULL;
 	struct batadv_hard_iface *primary_if = NULL;
@@ -1805,7 +1805,7 @@ static bool
 batadv_send_my_tt_response(struct batadv_priv *bat_priv,
 			   struct batadv_tt_query_packet *tt_request)
 {
-	struct batadv_orig_node *orig_node = NULL;
+	struct batadv_orig_node *orig_node;
 	struct batadv_neigh_node *neigh_node = NULL;
 	struct batadv_hard_iface *primary_if = NULL;
 	uint8_t my_ttvn, req_ttvn, ttvn;
@@ -1971,7 +1971,7 @@ static void _batadv_tt_update_changes(struct batadv_priv *bat_priv,
 static void batadv_tt_fill_gtable(struct batadv_priv *bat_priv,
 				  struct batadv_tt_query_packet *tt_response)
 {
-	struct batadv_orig_node *orig_node = NULL;
+	struct batadv_orig_node *orig_node;
 
 	orig_node = batadv_orig_hash_find(bat_priv, tt_response->src);
 	if (!orig_node)
@@ -2013,7 +2013,7 @@ static void batadv_tt_update_changes(struct batadv_priv *bat_priv,
 
 bool batadv_is_my_client(struct batadv_priv *bat_priv, const uint8_t *addr)
 {
-	struct batadv_tt_local_entry *tt_local_entry = NULL;
+	struct batadv_tt_local_entry *tt_local_entry;
 	bool ret = false;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr);
-- 
1.8.0

^ permalink raw reply related

* Re: [PATCH v4 3/9] net: xfrm: use __this_cpu_read per-cpu helper
From: Shan Wei @ 2012-11-13  9:33 UTC (permalink / raw)
  To: Steffen Klassert, Christoph Lameter
  Cc: David Miller, NetDev, Herbert Xu, Kernel-Maillist
In-Reply-To: <20121113072101.GG22290@secunet.com>

Steffen Klassert said, at 2012/11/13 15:21:
> 
> This should just fetch the tfm pointer, so why exactly __this_cpu_read
> is better than __this_cpu_ptr? Please keep in mind that performance is
> not the most important thing here. It's much more important that it
> works in any case.
 
[0/9] describes why we should submit this series patches.
you are not included in original mail.

this_cpu_ptr relocates and address. this_cpu_read() relocates the address
and performs the fetch. If you want to operate on rda(defined as per_cpu) 
then you can only use this_cpu_ptr. this_cpu_read() saves you more instructions
since it can do the relocation and the fetch in one instruction.

More info, please refer to http://www.spinics.net/lists/kernel/msg1435153.html

^ permalink raw reply

* Re: [PATCH 3/3] netfilter: ipv6: add getsockopt to retrieve origdst
From: Pablo Neira Ayuso @ 2012-11-13  9:39 UTC (permalink / raw)
  To: Florian Westphal; +Cc: YOSHIFUJI Hideaki, netfilter-devel, davem, netdev
In-Reply-To: <20121113085930.GE20678@breakpoint.cc>

On Tue, Nov 13, 2012 at 09:59:30AM +0100, Florian Westphal wrote:
> YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> wrote:
> > pablo@netfilter.org wrote:
> > > From: Florian Westphal <fw@strlen.de>
> > > +	sin6.sin6_scope_id = sk->sk_bound_dev_if;
> > > +
> > > +	nf_ct_put(ct);
> > > +	return copy_to_user(user, &sin6, sizeof(sin6)) ? -EFAULT : 0;
> > > +}
> > > +
> > 
> > I think we should set sin6_scope_id to sk->sk_bound_dev_if only if the
> > destination is link-local address.
> 
> Right, I see that getpeer/sockname has this test.
> 
> Dave, please either apply the patch as is (I'd submit a fixup)
> or discard i, I'll then send a V2 with added
> ipv6_addr_type(&sin->sin6_addr) & IPV6_ADDR_LINKLOCAL
> test to Pablo.

Please, send me a follow-up patch, I'll apply and request a new pull
request.

Thanks.

^ permalink raw reply

* Re: [PATCH v4 9/9] net: batman-adv: use per_cpu_add helper
From: Marek Lindner @ 2012-11-13  9:55 UTC (permalink / raw)
  To: b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
  Cc: NetDev, Kernel-Maillist, siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX,
	Shan Wei, Christoph Lameter, David Miller
In-Reply-To: <50A1A816.3070900-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Tuesday, November 13, 2012 09:53:26 Shan Wei wrote:
> From: Shan Wei <davidshan-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
> 
> this_cpu_add is an atomic operation.
> and be more faster than per_cpu_ptr operation.
> 
> Signed-off-by: Shan Wei <davidshan-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
> Reviewed-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
> ---
> v4: no changes vs v3.
> ---
>  net/batman-adv/main.h |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)

Applied in our tree (revision f8b19e1). We will pass along this patch through 
our standard workflow. No need to resend this patch in the future.

Thanks,
Marek

^ permalink raw reply

* Re: [PATCH v4 9/9] net: batman-adv: use per_cpu_add helper
From: Shan Wei @ 2012-11-13 10:25 UTC (permalink / raw)
  To: Marek Lindner
  Cc: NetDev, b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
	Kernel-Maillist, siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX,
	Christoph Lameter, David Miller
In-Reply-To: <201211131755.13299.lindner_marek-LWAfsSFWpa4@public.gmane.org>

Marek Lindner said, at 2012/11/13 17:55:
>>  net/batman-adv/main.h |    4 +---
>>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> Applied in our tree (revision f8b19e1). We will pass along this patch through 
> our standard workflow. No need to resend this patch in the future.

OK thanks~

> 
> Thanks,
> Marek
> 

^ permalink raw reply

* Re: [REGRESSION, v3.7-rc5, bisected] 100% CPU usage in softirqd, unable to shutdown(Internet mail)
From: dannyfeng(冯小天) @ 2012-11-13 10:26 UTC (permalink / raw)
  To: Lekensteyn, David S. Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <5934660.4HuQirPM4Z@al>


________________________________________
发件人: Lekensteyn [lekensteyn@gmail.com]
发送时间: 2012年11月13日 5:20
收件人: dannyfeng(冯小天); David S. Miller
Cc: netdev@vger.kernel.org
主题: [REGRESSION, v3.7-rc5, bisected] 100% CPU usage in softirqd, unable to shutdown(Internet mail)

Hi,

After upgrading from 3.7-rc4 to 3.7-rc5 I found that I was unable to suspend
without locking up the system afterwards. Neither was I able to shutdown as it
would simply hang where it should halt. The second suspend/resume in a session
would make Networkmanager hang.

When looking in my process list, I saw that softirqd was using one full CPU
core. Watching the contents of /proc/softirqs showed that the tasklet number
would rapidly increase.

I got this message when trying to suspend for the second time in a session:

 Freezing user space processes ...
 Freezing of tasks failed after 20.01 seconds (1 tasks refusing to freeze,
wq_busy=0):
 NetworkManager  R  running task        0   332      1 0x00000004
  ffff88023169d628 ffffffff81549b86 ffff8802316e4470 ffff88023169dfd8
  ffff88023169dfd8 ffff88023169dfd8 ffff8802316e4470 ffff8802316e4470
  ffff88023169d698 ffff88023bc92a80 ffff88022fd3db70 ffff88022fd3dc90
 Call Trace:
  [<ffffffff81549b9e>] ? __schedule+0x13e/0x760
  [<ffffffff8154a4f9>] schedule+0x29/0x70
  [<ffffffff810722aa>] sys_sched_yield+0x4a/0x60
  [<ffffffff8154a7c2>] yield+0x32/0x40
  [<ffffffff810451f5>] tasklet_kill+0x35/0x80
  [<ffffffffa017c2f3>] jme_close+0xd3/0x850 [jme]
  [<ffffffff8146325d>] __dev_close_many+0x7d/0xc0
  [<ffffffff814632cd>] __dev_close+0x2d/0x40
  [<ffffffff81469551>] __dev_change_flags+0xa1/0x180
  [<ffffffff814696e8>] dev_change_flags+0x28/0x70
  [<ffffffff81475b68>] do_setlink+0x378/0xa00
  [<ffffffff81078a56>] ? find_busiest_group+0x36/0x490
  [<ffffffff812d5821>] ? nla_parse+0x31/0xe0
  [<ffffffff812d5821>] ? nla_parse+0x31/0xe0
  [<ffffffff81477e6e>] rtnl_newlink+0x36e/0x590
  [<ffffffff81286e16>] ? apparmor_capable+0x26/0x90
  [<ffffffff81477694>] rtnetlink_rcv_msg+0x114/0x300
  [<ffffffff8114d1c3>] ? __kmalloc_node_track_caller+0x63/0x1b0
  [<ffffffff8145b65b>] ? __alloc_skb+0x8b/0x290
  [<ffffffff81477580>] ? __rtnl_unlock+0x20/0x20
  [<ffffffff8148f471>] netlink_rcv_skb+0xb1/0xc0
  [<ffffffff814748f5>] rtnetlink_rcv+0x25/0x40
  [<ffffffff8148ed8b>] netlink_unicast+0x19b/0x220
  [<ffffffff8148f111>] netlink_sendmsg+0x301/0x3c0
  [<ffffffff8144f8ec>] sock_sendmsg+0xbc/0xf0
  [<ffffffff81450797>] ? sock_recvmsg+0xd7/0x110
  [<ffffffff8145045c>] __sys_sendmsg+0x3ac/0x3c0
  [<ffffffff810854dc>] ? ktime_get_ts+0x4c/0xf0
  [<ffffffff81452699>] sys_sendmsg+0x49/0x90
  [<ffffffff81553906>] system_call_fastpath+0x1a/0x1f

Bisecting leads to:
commit 175c0dffef310fc7d7f026ca4a7682beb2fbd8ec
Author: Xiaotian Feng <xtfeng@gmail.com>
Date:   Wed Oct 31 00:29:57 2012 +0000

    drivers/net: use tasklet_kill in device remove/close process

    Some driver uses tasklet_disable in device remove/close process,
    tasklet_disable will inc tasklet->count and return. If the tasklet
    is not handled yet because some softirq pressure, the tasklet will
    placed on the tasklet_vec, never have a chance to excute. This might
    lead to ksoftirqd heavy loaded, wakeup with pending_softirq, but
    tasklet is disabled. tasklet_kill should be used in this case.

    Signed-off-by: Xiaotian Feng <dannyfeng@tencent.com>
    Cc: "David S. Miller" <davem@davemloft.net>
    Cc: netdev@vger.kernel.org
    Signed-off-by: David S. Miller <davem@davemloft.net>
(if it wasn't obvious, I have an Ethernet device that needs the "jme" driver,
04:00.5 Ethernet controller [0200]: JMicron Technology Corp. JMC250 PCI
Express Gigabit Ethernet Controller [197b:0250] (rev 03))

Since 3.7, I sometimes get the below messages during suspend, but it would
never hang:
smpboot: CPU 2 is now offline
NOHZ: local_softirq_pending 02
NOHZ: local_softirq_pending 202
NOHZ: local_softirq_pending 202
NOHZ: local_softirq_pending 02
NOHZ: local_softirq_pending 202
NOHZ: local_softirq_pending 202
smpboot: CPU 3 is now offline

Time for a revert or do you have an other proposed fix?

I'm sorry, I didn't noticed that jme_open use tasklet_enable, which may reuse a killed tasklet...
Could you please try following patch?
---
diff --git a/drivers/net/ethernet/jme.c b/drivers/net/ethernet/jme.c
index 92317e9..c0314c1 100644
--- a/drivers/net/ethernet/jme.c
+++ b/drivers/net/ethernet/jme.c
@@ -1860,10 +1860,14 @@ jme_open(struct net_device *netdev)
        jme_clear_pm(jme);
        JME_NAPI_ENABLE(jme);
 
-       tasklet_enable(&jme->linkch_task);
-       tasklet_enable(&jme->txclean_task);
-       tasklet_hi_enable(&jme->rxclean_task);
-       tasklet_hi_enable(&jme->rxempty_task);
+       tasklet_init(&jme->linkch_task, jme_link_change_tasklet,
+                     (unsigned long) jme);
+       tasklet_init(&jme->txclean_task, jme_tx_clean_tasklet,
+                     (unsigned long) jme);
+       tasklet_init(&jme->rxclean_task, jme_rx_clean_tasklet,
+                     (unsigned long) jme);
+       tasklet_init(&jme->rxempty_task, jme_rx_empty_tasklet,
+                     (unsigned long) jme);
 
        rc = jme_request_irq(jme);
        if (rc)
@@ -3079,22 +3083,6 @@ jme_init_one(struct pci_dev *pdev,
        tasklet_init(&jme->pcc_task,
                     jme_pcc_tasklet,
                     (unsigned long) jme);
-       tasklet_init(&jme->linkch_task,
-                    jme_link_change_tasklet,
-                    (unsigned long) jme);
-       tasklet_init(&jme->txclean_task,
-                    jme_tx_clean_tasklet,
-                    (unsigned long) jme);
-       tasklet_init(&jme->rxclean_task,
-                    jme_rx_clean_tasklet,
-                    (unsigned long) jme);
-       tasklet_init(&jme->rxempty_task,
-                    jme_rx_empty_tasklet,
-                    (unsigned long) jme);
-       tasklet_disable_nosync(&jme->linkch_task);
-       tasklet_disable_nosync(&jme->txclean_task);
-       tasklet_disable_nosync(&jme->rxclean_task);
-       tasklet_disable_nosync(&jme->rxempty_task);
        jme->dpi.cur = PCC_P1;
 
        jme->reg_ghc = 0;


Regards,
Peter


^ permalink raw reply related

* Re: [PATCH v4 3/9] net: xfrm: use __this_cpu_read per-cpu helper
From: Steffen Klassert @ 2012-11-13 10:48 UTC (permalink / raw)
  To: Shan Wei
  Cc: Christoph Lameter, David Miller, NetDev, Herbert Xu,
	Kernel-Maillist
In-Reply-To: <50A213DF.1040103@gmail.com>

On Tue, Nov 13, 2012 at 05:33:19PM +0800, Shan Wei wrote:
> Steffen Klassert said, at 2012/11/13 15:21:
> > 
> > This should just fetch the tfm pointer, so why exactly __this_cpu_read
> > is better than __this_cpu_ptr? Please keep in mind that performance is
> > not the most important thing here. It's much more important that it
> > works in any case.
>  
> [0/9] describes why we should submit this series patches.
> you are not included in original mail.
> 
> this_cpu_ptr relocates and address. this_cpu_read() relocates the address
> and performs the fetch. If you want to operate on rda(defined as per_cpu) 
> then you can only use this_cpu_ptr. this_cpu_read() saves you more instructions
> since it can do the relocation and the fetch in one instruction.
> 

Ok, so please add a commit message to describe your changes.

Thanks.

^ permalink raw reply

* Re: [PATCH v4] Network driver for the Armada 370 and Armada XP ARM Marvell SoCs
From: Thomas Petazzoni @ 2012-11-13 11:34 UTC (permalink / raw)
  To: Francois Romieu
  Cc: Eric Dumazet, David S. Miller, Lennert Buytenhek, netdev,
	linux-arm-kernel, Jason Cooper, Andrew Lunn, Gregory Clement,
	Lior Amsalem, Maen Suleiman
In-Reply-To: <20121103115321.GA4539@electric-eye.fr.zoreil.com>

François,

Thanks for your detailed review. I have a few comments/questions below
on specific topics.

On Sat, 3 Nov 2012 12:53:21 +0100, Francois Romieu wrote:
> > +	if (rxq->descs == NULL) {
> > +		netdev_err(pp->dev,
> > +			   "rxQ=%d: Can't allocate %d bytes for %d RX descr\n",
> > +			   rxq->id, rxq->size * MVNETA_DESC_ALIGNED_SIZE,
> > +			   rxq->size);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	BUG_ON(rxq->descs !=
> > +	       PTR_ALIGN(rxq->descs, MVNETA_CPU_D_CACHE_LINE_SIZE));
> 
> There is no reason to crash.

Well, there is a reason: the hardware will not work properly if
rxq->descs is not aligned on a MVNETA_CPU_D_CACHE_LINE_SIZE boundary.
So one solution is to over-allocated to guarantee the alignment, but
since practically speaking MVNETA_CPU_D_CACHE_LINE_SIZE=32 and
dma_alloc_coherent() returns things that seem at least 32 bytes
aligned, it sounded overkill to include more code to fix a problem that
doesn't exist. This BUG_ON() is here solely for the purpose of noisily
letting the user know if this implicit assumption on the alignment of
dma_alloc_coherent() allocated buffer changes in the future. I can turn
this into an error if you prefer:

	if (rxq->descs != PTR_ALIGN(rxq->descs, MVNETA_CPU_D_CACHE_LINE_SIZE)) {
		netdev_err(pp->dev, "improper buffer alignement assumption, driver needs fixing\n");
		dma_free_coherent(...);
		return -EINVAL;
	}

> (...]
> > +static int mvneta_txq_init(struct mvneta_port *pp,
> > +			   struct mvneta_tx_queue *txq)
> > +{
> > +	txq->size = pp->tx_ring_size;
> > +
> > +	/* Allocate memory for TX descriptors */
> > +	txq->descs = dma_alloc_coherent(pp->dev->dev.parent,
> > +					txq->size * MVNETA_DESC_ALIGNED_SIZE,
> > +					&txq->descs_phys,
> > +					DMA_BIDIRECTIONAL);
> 
> 					&txq->descs_phys, DMA_BIDIRECTIONAL);

Aaah, thanks for pointing this one! It should have been GFP_KERNEL, not
DMA_BIDIRECTIONAL here.

> > +	if (txq->descs == NULL) {
> > +		netdev_err(pp->dev,
> > +			   "txQ=%d: Can't allocate %d bytes for %d TX descr\n",
> > +			   txq->id, txq->size * MVNETA_DESC_ALIGNED_SIZE,
> > +			   txq->size);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	/* Make sure descriptor address is cache line size aligned  */
> > +	BUG_ON(txq->descs !=
> > +	       PTR_ALIGN(txq->descs, MVNETA_CPU_D_CACHE_LINE_SIZE));
> 
> There is no reason to crash.

Same as above :-)

> [...]
> > +static int mvneta_setup_rxqs(struct mvneta_port *pp)
> > +{
> > +	int queue;
> > +
> > +	for (queue = 0; queue < rxq_number; queue++) {
> > +		int err = mvneta_rxq_init(pp, &pp->rxqs[queue]);
> > +		if (err) {
> > +			netdev_err(pp->dev,
> > +				   "%s: can't create RxQ rxq=%d\n",
> 
> 			netdev_err(pp->dev, "%s: can't create RxQ rxq=%d\n",
> 
> > +				   __func__, queue);
> > +			mvneta_cleanup_rxqs(pp);
> > +			return -ENODEV;
> 
> mvneta_rxq_init should return a proper error code and it should be
> propagated (option: break instead of return and mvneta_setup_rxqs scoped
> err variable)

Besides turning the "return -ENODEV;" into "return err;", I don't see
what is the other problem here? mvneta_rxq_init() properly returns
-ENOMEM where there is a memory allocation failure, and
mvneta_cleanup_rxqs() properly cleans up *all* initialized rxqs. Is
there something I'm missing?

Thanks again for your review!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH] tcp: tcp_replace_ts_recent() should not be called from tcp_validate_incoming()
From: Romain Francoise @ 2012-11-13 12:02 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Neal Cardwell, Yuchung Cheng,
	Nandita Dukkipati, H.K. Jerry Chu
In-Reply-To: <1352793951.6185.28.camel@edumazet-glaptop>

Eric Dumazet <eric.dumazet@gmail.com> writes:

> We added support for RFC 5962 in latest kernels but TCP fails
> to perform exhaustive check of ACK sequence.

You probably meant 5961...

^ permalink raw reply

* Re: [PATCH v5 3/9] net: xfrm: use __this_cpu_read per-cpu helper
From: Shan Wei @ 2012-11-13 12:36 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: Christoph Lameter, David Miller, NetDev, Herbert Xu,
	Kernel-Maillist
In-Reply-To: <20121113104839.GJ22290@secunet.com>

Steffen Klassert said, at 2012/11/13 18:48:
> 
> Ok, so please add a commit message to describe your changes.
> 
> Thanks.
> 

[PATCH v5] net: xfrm: use __this_cpu_read per-cpu helper

this_cpu_ptr/this_cpu_read is faster than per_cpu_ptr(p, smp_processor_id()) 
and can reduce  memory accesses.
The latter helper needs to find the offset for current cpu,
and needs more assembler instructions which objdump shows in following. 

this_cpu_ptr relocates and address. this_cpu_read() relocates the address
and performs the fetch. this_cpu_read() saves you more instructions
since it can do the relocation and the fetch in one instruction.

per_cpu_ptr(p, smp_processor_id()):
  1e:   65 8b 04 25 00 00 00 00         mov    %gs:0x0,%eax
  26:   48 98                           cltq
  28:   31 f6                           xor    %esi,%esi
  2a:   48 c7 c7 00 00 00 00            mov    $0x0,%rdi
  31:   48 8b 04 c5 00 00 00 00         mov    0x0(,%rax,8),%rax
  39:   c7 44 10 04 14 00 00 00         movl   $0x14,0x4(%rax,%rdx,1)

this_cpu_ptr(p)
  1e:   65 48 03 14 25 00 00 00 00      add    %gs:0x0,%rdx
  27:   31 f6                           xor    %esi,%esi
  29:   c7 42 04 14 00 00 00            movl   $0x14,0x4(%rdx)
  30:   48 c7 c7 00 00 00 00            mov    $0x0,%rdi

Signed-off-by: Shan Wei <davidshan@tencent.com>
---
 net/xfrm/xfrm_ipcomp.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index e5246fb..2906d52 100644
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -276,18 +276,16 @@ static struct crypto_comp * __percpu *ipcomp_alloc_tfms(const char *alg_name)
 	struct crypto_comp * __percpu *tfms;
 	int cpu;
 
-	/* This can be any valid CPU ID so we don't need locking. */
-	cpu = raw_smp_processor_id();
 
 	list_for_each_entry(pos, &ipcomp_tfms_list, list) {
 		struct crypto_comp *tfm;
 
-		tfms = pos->tfms;
-		tfm = *per_cpu_ptr(tfms, cpu);
+		/* This can be any valid CPU ID so we don't need locking. */
+		tfm = __this_cpu_read(*pos->tfms);
 
 		if (!strcmp(crypto_comp_name(tfm), alg_name)) {
 			pos->users++;
-			return tfms;
+			return pos->tfms;
 		}
 	}
 
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH 0/3] netfilter updates for net-next
From: Pablo Neira Ayuso @ 2012-11-13 12:40 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1352765203-4160-1-git-send-email-pablo@netfilter.org>

On Tue, Nov 13, 2012 at 01:06:40AM +0100, pablo@netfilter.org wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> Hi David,
> 
> The following three patches contain updates for your net-next tree,
> they include:
> 
> * Little cleanup for IPVS the use of a strange notation to assign the
>   conntrack object, from Alan Cox.
> 
> * getsockopt support to obtain the original IPv6 address after NAT,
>   similar to the one that IPv4 provides, from Florian Westphal.
> 
> * Another little cleanup for nf_nat to save a couple of lines by using
>   PTR_RET, from Wu Fengguang.

Please, hold on with this pull request. We need a follow-up patch to
resolve an issue regarding the getsockopt to obtain original address
with IPv6 NAT.

I'll send you a new pull request.

Sorry for the inconvenience.

^ permalink raw reply

* [PATCH] net: cdc_ncm: add Huawei devices
From: Bjørn Mork @ 2012-11-13 13:19 UTC (permalink / raw)
  To: netdev
  Cc: linux-usb, Oliver Neukum, Alexey ORISHKO, Tommy Cheng, Olof Ermis,
	Bjørn Mork

A number of Huawei 3G and LTE modems implement a CDC NCM function,
including the necessary functional descriptors, but using a non
standard interface layout and class/subclass/protocol codes.

These devices can be handled by this driver with only a minor
change to the probing logic, allowing a single combined control
and data interface.  This works because the devices
- include a CDC Union descriptor labelling the combined
  interface as both master and slave, and
- have an alternate setting #1 for the bulk endpoints on the
  combined interface.

The 3G/LTE network connection is managed by vendor specific AT
commands on a serial function in the same composite device.
Handling the managment function is out of the scope of this
driver.  It will be handled by an appropriate USB serial
driver.

Reported-and-Tested-by: Olof Ermis <olof.ermis@gmail.com>
Reported-and-Tested-by: Tommy Cheng <tommy7765@yahoo.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
Hello David,

This patch had to touch more than just the device id table to
deal with the combined NCM interface on these devices.  The
changes will not affect any currently supported devices because
(ctx->data != ctx->control) always is true for those.

I am therefore requesting that this patch is put in the stable
queue like other device additons.

I guess it may be too late for v3.7, but FWIW I have verified
that the patch applies cleanly to both the current net-next/master
and to net/master, and will not cause merge conflicts either way.
I have also verified that it applies cleanly to the v3.6.6 stable
release.


Bjørn

 drivers/net/usb/cdc_ncm.c |   22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4cd582a..74fab1a 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -540,10 +540,12 @@ advance:
 	    (ctx->ether_desc == NULL) || (ctx->control != intf))
 		goto error;
 
-	/* claim interfaces, if any */
-	temp = usb_driver_claim_interface(driver, ctx->data, dev);
-	if (temp)
-		goto error;
+	/* claim data interface, if different from control */
+	if (ctx->data != ctx->control) {
+		temp = usb_driver_claim_interface(driver, ctx->data, dev);
+		if (temp)
+			goto error;
+	}
 
 	iface_no = ctx->data->cur_altsetting->desc.bInterfaceNumber;
 
@@ -623,6 +625,10 @@ static void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
 
 	tasklet_kill(&ctx->bh);
 
+	/* handle devices with combined control and data interface */
+	if (ctx->control == ctx->data)
+		ctx->data = NULL;
+
 	/* disconnect master --> disconnect slave */
 	if (intf == ctx->control && ctx->data) {
 		usb_set_intfdata(ctx->data, NULL);
@@ -1245,6 +1251,14 @@ static const struct usb_device_id cdc_devs[] = {
 	  .driver_info = (unsigned long) &wwan_info,
 	},
 
+	/* Huawei NCM devices disguised as vendor specific */
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x16),
+	  .driver_info = (unsigned long)&wwan_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x46),
+	  .driver_info = (unsigned long)&wwan_info,
+	},
+
 	/* Generic CDC-NCM devices */
 	{ USB_INTERFACE_INFO(USB_CLASS_COMM,
 		USB_CDC_SUBCLASS_NCM, USB_CDC_PROTO_NONE),
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] tcp: tcp_replace_ts_recent() should not be called from tcp_validate_incoming()
From: Eric Dumazet @ 2012-11-13 13:28 UTC (permalink / raw)
  To: Romain Francoise
  Cc: David Miller, netdev, Neal Cardwell, Yuchung Cheng,
	Nandita Dukkipati, H.K. Jerry Chu
In-Reply-To: <87k3tp6783.fsf@silenus.orebokech.com>

On Tue, 2012-11-13 at 13:02 +0100, Romain Francoise wrote:
> Eric Dumazet <eric.dumazet@gmail.com> writes:
> 
> > We added support for RFC 5962 in latest kernels but TCP fails
> > to perform exhaustive check of ACK sequence.
> 
> You probably meant 5961...

Oops you're right ;)

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox