Netdev List
 help / color / mirror / Atom feed
* [PATCH 06/16] batman-adv: Set special lockdep classes to avoid lockdep warning
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

From: Sven Eckelmann <sven@narfation.org>

Transmissions over batman-adv devices always start another nested transmission
over devices attached to the batman-adv interface. These devices usually use
the ethernet lockdep class for the tx_queue lock which is also set by default
for all batman-adv devices. Lockdep will detect a nested locking attempt of two
locks with the same class and warn about a possible deadlock.

This is the default and expected behavior and should not alarm the locking
correctness prove mechanism. Therefore, the locks for all netdevice specific tx
queues get a special batman-adv lock class to avoid a false positive for each
transmission.

Reported-by: Linus Luessing <linus.luessing@web.de>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/soft-interface.c | 44 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index b9a28d2..948860a 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -347,7 +347,51 @@ out:
 	return;
 }
 
+/* batman-adv network devices have devices nesting below it and are a special
+ * "super class" of normal network devices; split their locks off into a
+ * separate class since they always nest.
+ */
+static struct lock_class_key batadv_netdev_xmit_lock_key;
+static struct lock_class_key batadv_netdev_addr_lock_key;
+
+/**
+ * batadv_set_lockdep_class_one - Set lockdep class for a single tx queue
+ * @dev: device which owns the tx queue
+ * @txq: tx queue to modify
+ * @_unused: always NULL
+ */
+static void batadv_set_lockdep_class_one(struct net_device *dev,
+					 struct netdev_queue *txq,
+					 void *_unused)
+{
+	lockdep_set_class(&txq->_xmit_lock, &batadv_netdev_xmit_lock_key);
+}
+
+/**
+ * batadv_set_lockdep_class - Set txq and addr_list lockdep class
+ * @dev: network device to modify
+ */
+static void batadv_set_lockdep_class(struct net_device *dev)
+{
+	lockdep_set_class(&dev->addr_list_lock, &batadv_netdev_addr_lock_key);
+	netdev_for_each_tx_queue(dev, batadv_set_lockdep_class_one, NULL);
+}
+
+/**
+ * batadv_softif_init - Late stage initialization of soft interface
+ * @dev: registered network device to modify
+ *
+ * Returns error code on failures
+ */
+static int batadv_softif_init(struct net_device *dev)
+{
+	batadv_set_lockdep_class(dev);
+
+	return 0;
+}
+
 static const struct net_device_ops batadv_netdev_ops = {
+	.ndo_init = batadv_softif_init,
 	.ndo_open = batadv_interface_open,
 	.ndo_stop = batadv_interface_release,
 	.ndo_get_stats = batadv_interface_stats,
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 07/16] batman-adv: Remove extra check in batadv_bit_get_packet
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

From: Sven Eckelmann <sven@narfation.org>

batadv_bit_get_packet checks for all common situations before it decides that
the new received packet indicates that the host was restarted. This extra
condition check at the end of the function is not necessary because this
condition is always true.

This patch addresses Coverity #712296: Logically dead code

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/bitarray.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/net/batman-adv/bitarray.c b/net/batman-adv/bitarray.c
index aea174c..5453b17 100644
--- a/net/batman-adv/bitarray.c
+++ b/net/batman-adv/bitarray.c
@@ -79,20 +79,17 @@ int batadv_bit_get_packet(void *priv, unsigned long *seq_bits,
 	 * or the old packet got delayed somewhere in the network. The
 	 * packet should be dropped without calling this function if the
 	 * seqno window is protected.
+	 *
+	 * seq_num_diff <= -BATADV_TQ_LOCAL_WINDOW_SIZE
+	 * or
+	 * seq_num_diff >= BATADV_EXPECTED_SEQNO_RANGE
 	 */
-	if (seq_num_diff <= -BATADV_TQ_LOCAL_WINDOW_SIZE ||
-	    seq_num_diff >= BATADV_EXPECTED_SEQNO_RANGE) {
+	batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
+		   "Other host probably restarted!\n");
 
-		batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
-			   "Other host probably restarted!\n");
+	bitmap_zero(seq_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
+	if (set_mark)
+		batadv_set_bit(seq_bits, 0);
 
-		bitmap_zero(seq_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
-		if (set_mark)
-			batadv_set_bit(seq_bits, 0);
-
-		return 1;
-	}
-
-	/* never reached */
-	return 0;
+	return 1;
 }
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 08/16] batman-adv: Check return value of try_module_get
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

From: Sven Eckelmann <sven@narfation.org>

New operations should not be started when they need an increased module
reference counter and try_module_get failed.

This patch addresses Coverity #712284: Unchecked return value

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/debugfs.c     |  6 ++++--
 net/batman-adv/icmp_socket.c | 12 ++++++++----
 net/batman-adv/main.c        | 10 ----------
 net/batman-adv/main.h        |  2 --
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c
index 391d4fb..bd032bc 100644
--- a/net/batman-adv/debugfs.c
+++ b/net/batman-adv/debugfs.c
@@ -99,15 +99,17 @@ int batadv_debug_log(struct batadv_priv *bat_priv, const char *fmt, ...)
 
 static int batadv_log_open(struct inode *inode, struct file *file)
 {
+	if (!try_module_get(THIS_MODULE))
+		return -EBUSY;
+
 	nonseekable_open(inode, file);
 	file->private_data = inode->i_private;
-	batadv_inc_module_count();
 	return 0;
 }
 
 static int batadv_log_release(struct inode *inode, struct file *file)
 {
-	batadv_dec_module_count();
+	module_put(THIS_MODULE);
 	return 0;
 }
 
diff --git a/net/batman-adv/icmp_socket.c b/net/batman-adv/icmp_socket.c
index bde3cf747..5874c0e 100644
--- a/net/batman-adv/icmp_socket.c
+++ b/net/batman-adv/icmp_socket.c
@@ -42,12 +42,16 @@ static int batadv_socket_open(struct inode *inode, struct file *file)
 	unsigned int i;
 	struct batadv_socket_client *socket_client;
 
+	if (!try_module_get(THIS_MODULE))
+		return -EBUSY;
+
 	nonseekable_open(inode, file);
 
 	socket_client = kmalloc(sizeof(*socket_client), GFP_KERNEL);
-
-	if (!socket_client)
+	if (!socket_client) {
+		module_put(THIS_MODULE);
 		return -ENOMEM;
+	}
 
 	for (i = 0; i < ARRAY_SIZE(batadv_socket_client_hash); i++) {
 		if (!batadv_socket_client_hash[i]) {
@@ -59,6 +63,7 @@ static int batadv_socket_open(struct inode *inode, struct file *file)
 	if (i == ARRAY_SIZE(batadv_socket_client_hash)) {
 		pr_err("Error - can't add another packet client: maximum number of clients reached\n");
 		kfree(socket_client);
+		module_put(THIS_MODULE);
 		return -EXFULL;
 	}
 
@@ -71,7 +76,6 @@ static int batadv_socket_open(struct inode *inode, struct file *file)
 
 	file->private_data = socket_client;
 
-	batadv_inc_module_count();
 	return 0;
 }
 
@@ -96,7 +100,7 @@ static int batadv_socket_release(struct inode *inode, struct file *file)
 	spin_unlock_bh(&socket_client->lock);
 
 	kfree(socket_client);
-	batadv_dec_module_count();
+	module_put(THIS_MODULE);
 
 	return 0;
 }
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index a4a337d..f9bcfa1 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -160,16 +160,6 @@ void batadv_mesh_free(struct net_device *soft_iface)
 	atomic_set(&bat_priv->mesh_state, BATADV_MESH_INACTIVE);
 }
 
-void batadv_inc_module_count(void)
-{
-	try_module_get(THIS_MODULE);
-}
-
-void batadv_dec_module_count(void)
-{
-	module_put(THIS_MODULE);
-}
-
 int batadv_is_my_mac(const uint8_t *addr)
 {
 	const struct batadv_hard_iface *hard_iface;
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index 5c0be5b..9b94f05 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -150,8 +150,6 @@ extern struct workqueue_struct *batadv_event_workqueue;
 
 int batadv_mesh_init(struct net_device *soft_iface);
 void batadv_mesh_free(struct net_device *soft_iface);
-void batadv_inc_module_count(void);
-void batadv_dec_module_count(void);
 int batadv_is_my_mac(const uint8_t *addr);
 struct batadv_hard_iface *
 batadv_seq_print_text_primary_if_get(struct seq_file *seq);
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 09/16] batman-adv: Only increase refcounter once for alternate router
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

From: Sven Eckelmann <sven@narfation.org>

The test whether we can use a router for alternating bonding should only be
done once because it is already known that it is still usable and will not be
deleted from the list soon.

This patch addresses Coverity #712285: Unchecked return value

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/routing.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 8bdafc8..1ac072d 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -549,25 +549,18 @@ batadv_find_ifalter_router(struct batadv_orig_node *primary_orig,
 		if (tmp_neigh_node->if_incoming == recv_if)
 			continue;
 
+		if (router && tmp_neigh_node->tq_avg <= router->tq_avg)
+			continue;
+
 		if (!atomic_inc_not_zero(&tmp_neigh_node->refcount))
 			continue;
 
-		/* if we don't have a router yet
-		 * or this one is better, choose it.
-		 */
-		if ((!router) ||
-		    (tmp_neigh_node->tq_avg > router->tq_avg)) {
-			/* decrement refcount of
-			 * previously selected router
-			 */
-			if (router)
-				batadv_neigh_node_free_ref(router);
+		/* decrement refcount of previously selected router */
+		if (router)
+			batadv_neigh_node_free_ref(router);
 
-			router = tmp_neigh_node;
-			atomic_inc_not_zero(&router->refcount);
-		}
-
-		batadv_neigh_node_free_ref(tmp_neigh_node);
+		/* we found a better router (or at least one valid router) */
+		router = tmp_neigh_node;
 	}
 
 	/* use the first candidate if nothing was found. */
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 10/16] batman-adv: don't allow ECTP traffic on batman-adv
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem
  Cc: netdev, b.a.t.m.a.n, Simon Wunderlich, Simon Wunderlich,
	Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

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

We have seen this to break networks when used with bridge loop
avoidance. As we can't see any benefit from sending these ancient frames
via our mesh, we just drop them.

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

diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 948860a..22bc651 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -146,8 +146,10 @@ static int batadv_interface_tx(struct sk_buff *skb,
 	struct batadv_bcast_packet *bcast_packet;
 	struct vlan_ethhdr *vhdr;
 	__be16 ethertype = __constant_htons(BATADV_ETH_P_BATMAN);
-	static const uint8_t stp_addr[ETH_ALEN] = {0x01, 0x80, 0xC2, 0x00, 0x00,
-						   0x00};
+	static const uint8_t stp_addr[ETH_ALEN] = {0x01, 0x80, 0xC2, 0x00,
+						   0x00, 0x00};
+	static const uint8_t ectp_addr[ETH_ALEN] = {0xCF, 0x00, 0x00, 0x00,
+						    0x00, 0x00};
 	unsigned int header_len = 0;
 	int data_len = skb->len, ret;
 	short vid __maybe_unused = -1;
@@ -180,10 +182,16 @@ static int batadv_interface_tx(struct sk_buff *skb,
 
 	/* don't accept stp packets. STP does not help in meshes.
 	 * better use the bridge loop avoidance ...
+	 *
+	 * The same goes for ECTP sent at least by some Cisco Switches,
+	 * it might confuse the mesh when used with bridge loop avoidance.
 	 */
 	if (batadv_compare_eth(ethhdr->h_dest, stp_addr))
 		goto dropped;
 
+	if (batadv_compare_eth(ethhdr->h_dest, ectp_addr))
+		goto dropped;
+
 	if (is_multicast_ether_addr(ethhdr->h_dest)) {
 		do_bcast = true;
 
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 11/16] batman-adv: properly store the roaming time
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

in case of a new global entry added because of roaming, the roam_at field must
be properly initiated with the current time. This value will be later use to
purge this entry out on time out (if nobody claims it). Instead roam_at field
is now set to zero in this situation leading to an immediate purging of the
related entry.

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

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 42a74e3..0ac39d5 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -726,6 +726,12 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 
 		common->flags = flags;
 		tt_global_entry->roam_at = 0;
+		/* node must store current time in case of roaming. This is
+		 * needed to purge this entry out on timeout (if nobody claims
+		 * it)
+		 */
+		if (flags & BATADV_TT_CLIENT_ROAM)
+			tt_global_entry->roam_at = jiffies;
 		atomic_set(&common->refcount, 2);
 		common->added_at = jiffies;
 
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 13/16] batman-adv: check for more space before accessing the skb
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

In batadv_check_unicast_ttvn() the code accesses both the unicast header and the
Ethernet header in the payload. For this reason pskb_may_pull() must be invoked
to check for the required space.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/routing.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 456a0a9..46dd5b4 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -908,8 +908,12 @@ static int batadv_check_unicast_ttvn(struct batadv_priv *bat_priv,
 	bool tt_poss_change;
 	int is_old_ttvn;
 
-	/* I could need to modify it */
-	if (skb_cow(skb, sizeof(struct batadv_unicast_packet)) < 0)
+	/* check if there is enough data before accessing it */
+	if (pskb_may_pull(skb, sizeof(*unicast_packet) + ETH_HLEN) < 0)
+		return 0;
+
+	/* create a copy of the skb (in case of for re-routing) to modify it. */
+	if (skb_cow(skb, sizeof(*unicast_packet)) < 0)
 		return 0;
 
 	unicast_packet = (struct batadv_unicast_packet *)skb->data;
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 12/16] batman-adv: print packets re-routing on DBG_TT and ratelimit it
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

To simplify TranslationTable debugging it is better to print the packet
rerouting message on the DBG_TT log level. In this way a developer interested in
packets rerouting doesn't need to filter it out of the whole ROUTES log.

Moreover, since this message will appear for each rerouted message, it is now
"ratelimited".

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/routing.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 1ac072d..456a0a9 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -965,10 +965,10 @@ static int batadv_check_unicast_ttvn(struct batadv_priv *bat_priv,
 			batadv_orig_node_free_ref(orig_node);
 		}
 
-		batadv_dbg(BATADV_DBG_ROUTES, 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);
+		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;
 	}
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 15/16] batman-adv: pass the WIFI flag from the local to global entry
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

in case of client roaming a new global entry is added while a corresponding
local one is still present. In this case the node can safely pass the WIFI flag
from the local to the global entry.

This change is required to let the AP-isolation correctly working in case of
roaming: if a generic WIFI client C roams from node A to B, A adds a global
entry for C without adding any WIFI flag. The latter will be set only later,
once A has received C's advertisement from B. In this time period the
AP-Isolation (if enabled) would not correctly work since C is not marked as
WIFI, so allowing it to communicate with other WIFI clients.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/translation-table.c | 31 +++++++++++++++++++++++++------
 net/batman-adv/translation-table.h |  6 +++---
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index c61209f..a570d95 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -489,24 +489,39 @@ batadv_tt_local_set_pending(struct batadv_priv *bat_priv,
 		   tt_local_entry->common.addr, message);
 }
 
-void batadv_tt_local_remove(struct batadv_priv *bat_priv, const uint8_t *addr,
-			    const char *message, bool roaming)
+/**
+ * batadv_tt_local_remove - logically remove an entry from the local table
+ * @bat_priv: the bat priv with all the soft interface information
+ * @addr: the MAC address of the client to remove
+ * @message: message to append to the log on deletion
+ * @roaming: true if the deletion is due to a roaming event
+ *
+ * Returns the flags assigned to the local entry before being deleted
+ */
+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;
-	uint16_t flags;
+	uint16_t flags, curr_flags = BATADV_NO_FLAGS;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr);
 	if (!tt_local_entry)
 		goto out;
 
+	curr_flags = tt_local_entry->common.flags;
+
 	flags = BATADV_TT_CLIENT_DEL;
 	if (roaming)
 		flags |= BATADV_TT_CLIENT_ROAM;
 
 	batadv_tt_local_set_pending(bat_priv, tt_local_entry, flags, message);
+
 out:
 	if (tt_local_entry)
 		batadv_tt_local_entry_free_ref(tt_local_entry);
+
+	return curr_flags;
 }
 
 static void batadv_tt_local_purge_list(struct batadv_priv *bat_priv,
@@ -713,6 +728,7 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 	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);
 
@@ -785,10 +801,13 @@ int batadv_tt_global_add(struct batadv_priv *bat_priv,
 	ret = 1;
 
 out_remove:
+
 	/* remove address from local hash if present */
-	batadv_tt_local_remove(bat_priv, tt_global_entry->common.addr,
-			       "global tt received",
-			       flags & BATADV_TT_CLIENT_ROAM);
+	local_flags = batadv_tt_local_remove(bat_priv, tt_addr,
+					     "global tt received",
+					     flags & BATADV_TT_CLIENT_ROAM);
+	tt_global_entry->common.flags |= local_flags & BATADV_TT_CLIENT_WIFI;
+
 out:
 	if (tt_global_entry)
 		batadv_tt_global_entry_free_ref(tt_global_entry);
diff --git a/net/batman-adv/translation-table.h b/net/batman-adv/translation-table.h
index 811fffd..9fa4fe4 100644
--- a/net/batman-adv/translation-table.h
+++ b/net/batman-adv/translation-table.h
@@ -24,9 +24,9 @@ int batadv_tt_len(int changes_num);
 int batadv_tt_init(struct batadv_priv *bat_priv);
 void batadv_tt_local_add(struct net_device *soft_iface, const uint8_t *addr,
 			 int ifindex);
-void batadv_tt_local_remove(struct batadv_priv *bat_priv,
-			    const uint8_t *addr, const char *message,
-			    bool roaming);
+uint16_t batadv_tt_local_remove(struct batadv_priv *bat_priv,
+				const uint8_t *addr, const char *message,
+				bool roaming);
 int batadv_tt_local_seq_print_text(struct seq_file *seq, void *offset);
 void batadv_tt_global_add_orig(struct batadv_priv *bat_priv,
 			       struct batadv_orig_node *orig_node,
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 14/16] batman-adv: properly convert flag into a boolean value
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

In order to properly convert a bitwise AND to a boolean value, the whole
expression must be prepended by "!!".

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

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 0ac39d5..c61209f 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -2420,7 +2420,7 @@ bool batadv_tt_global_client_is_roaming(struct batadv_priv *bat_priv,
 	if (!tt_global_entry)
 		goto out;
 
-	ret = tt_global_entry->common.flags & BATADV_TT_CLIENT_ROAM;
+	ret = !!(tt_global_entry->common.flags & BATADV_TT_CLIENT_ROAM);
 	batadv_tt_global_entry_free_ref(tt_global_entry);
 out:
 	return ret;
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 16/16] batman-adv: add kernel-doc for enum batadv_dbg_level
From: Antonio Quartulli @ 2012-10-29  8:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli
In-Reply-To: <1351501097-1289-1-git-send-email-ordex@autistici.org>

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 net/batman-adv/main.h | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index 9b94f05..897ba6a 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -165,12 +165,19 @@ int batadv_algo_register(struct batadv_algo_ops *bat_algo_ops);
 int batadv_algo_select(struct batadv_priv *bat_priv, char *name);
 int batadv_algo_seq_print_text(struct seq_file *seq, void *offset);
 
-/* all messages related to routing / flooding / broadcasting / etc */
+/**
+ * enum batadv_dbg_level - available log levels
+ * @BATADV_DBG_BATMAN: OGM and TQ computations related messages
+ * @BATADV_DBG_ROUTES: route added / changed / deleted
+ * @BATADV_DBG_TT: translation table messages
+ * @BATADV_DBG_BLA: bridge loop avoidance messages
+ * @BATADV_DBG_ALL: the union of all the above log levels
+ */
 enum batadv_dbg_level {
 	BATADV_DBG_BATMAN = BIT(0),
-	BATADV_DBG_ROUTES = BIT(1), /* route added / changed / deleted */
-	BATADV_DBG_TT	  = BIT(2), /* translation table operations */
-	BATADV_DBG_BLA    = BIT(3), /* bridge loop avoidance */
+	BATADV_DBG_ROUTES = BIT(1),
+	BATADV_DBG_TT	  = BIT(2),
+	BATADV_DBG_BLA    = BIT(3),
 	BATADV_DBG_ALL    = 15,
 };
 
-- 
1.7.12.4

^ permalink raw reply related

* Re: [Patch net-next] ipv6: remove another useless NULL check
From: Cong Wang @ 2012-10-29  9:08 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, David S. Miller
In-Reply-To: <1351499588.7394.198.camel@edumazet-glaptop>

On Mon, 2012-10-29 at 09:33 +0100, Eric Dumazet wrote:
> On Mon, 2012-10-29 at 16:16 +0800, Cong Wang wrote:
> > When 'rt' is NULL, '&rt->dst' is NULL too because >dst
> > is always the first field of 'rt'. And dst_release accepts
> > NULL.
> > 
> > Cc: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Signed-off-by: Cong Wang <amwang@redhat.com>
> > 
> > ---
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index 8f0b12a..adeb479 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -2027,8 +2027,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
> >  			addrconf_prefix_route(&pinfo->prefix, pinfo->prefix_len,
> >  					      dev, expires, flags);
> >  		}
> > -		if (rt)
> > -			dst_release(&rt->dst);
> > +		dst_release(&rt->dst);
> >  	}
> >  
> >  	/* Try to figure out our local address for this prefix */
> 
> 
> 
> Could you instead introduce a helper for that, like we have in IPv4.

Sure, that is better.

^ permalink raw reply

* Re: [PATCH net-next] ipv4: avoid a test in ip_rt_put()
From: Cong Wang @ 2012-10-29  9:09 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1351499603.7394.199.camel@edumazet-glaptop>

On Mon, 2012-10-29 at 09:33 +0100, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> We can save a test in ip_rt_put(), considering dst_release() accepts
> a NULL parameter, and dst is first element in rtable.
> 
> Add a BUILD_BUG_ON() to catch any change that could break this
> assertion.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Cong Wang <amwang@redhat.com>

Acked-by: Cong Wang <amwang@redhat.com>

Thanks!

^ permalink raw reply

* [PATCH] bonding: fix bond-6-mode change MAC of arp reply from vif to  cause Domu's network unreachable intermittently
From: zheng.li @ 2012-10-29  9:11 UTC (permalink / raw)
  To: netdev, Jay Vosburgh, Andy Gospodarek
  Cc: linux-kernel, David S. Miller, Joe Jin, zheng.x.li

This is a fix for a bug in bond_alb.c
Rate of reproduced:100%
Scenario: set Dom0 to bond 6 mode, Domu communicate with Dom0 through
vif which is in bridge mode. The Dom0's bridge of xenbr0 contains vif
and bond0, bond0 contains eth0 and eth1. You can just need to ping a
host which is in same LAN on
Domu, some of packets will be lost intermittently.
Analyse: When Dom0 set bond mode to 6(alb), the bond_alb will change MAC
of every arp reply in rlb_arp_xmit function to affect receive packets,
it is ok for normal NIC, but it's wrong to Domu, when Domu send arp
reply through vif of Dom0, bond of alb replace Domu's MAC in arp reply
with NIC's MAC address, that will cause remote host send packets to Domu
using real NIC's MAC instead of Domu's MAC. Domu
can't receive the packets whose dst MAC is not Domu's MAC.

Signed-off-by: Zheng Li <zheng.x.li@oracle.com>
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/net/bonding/bond_alb.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index e15cc11..d6b134a 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -700,7 +700,18 @@ static struct slave *rlb_arp_xmit(struct sk_buff
*skb, struct bonding *bond)
 		*/
 		tx_slave = rlb_choose_channel(skb, bond);
 		if (tx_slave) {
-			memcpy(arp->mac_src,tx_slave->dev->dev_addr, ETH_ALEN);
+			struct slave *tmp_slave = NULL;
+			int i = 0, found_mac = 0;
+			bond_for_each_slave(bond, tmp_slave, i) {
+				if (ether_addr_equal_64bits(arp->mac_src,
+						tmp_slave->dev->dev_addr)) {
+					found_mac = 1;
+					break;
+				}
+			}
+			if (found_mac)
+				memcpy(arp->mac_src, tx_slave->dev->dev_addr,
+					ETH_ALEN);
 		}
 		pr_debug("Server sent ARP Reply packet\n");
 	} else if (arp->op_code == htons(ARPOP_REQUEST)) {
-- 
1.7.6.5

^ permalink raw reply related

* [PATCH net-next] ipv6: remove useless test in dev_forward_change()
From: Nicolas Dichtel @ 2012-10-29  9:24 UTC (permalink / raw)
  To: fengguang.wu; +Cc: netdev, davem, Nicolas Dichtel
In-Reply-To: <20121029015709.GA18567@localhost>

idev->dev cannot be NULL.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/ipv6/addrconf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 8f0b12a..c9b1cf3 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -607,7 +607,7 @@ static void dev_forward_change(struct inet6_dev *idev)
 	dev = idev->dev;
 	if (idev->cnf.forwarding)
 		dev_disable_lro(dev);
-	if (dev && (dev->flags & IFF_MULTICAST)) {
+	if (dev->flags & IFF_MULTICAST) {
 		if (idev->cnf.forwarding)
 			ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
 		else
-- 
1.7.12

^ permalink raw reply related

* Re: [PATCH net-next] tcp: better retrans tracking for defer-accept
From: Julian Anastasov @ 2012-10-29  9:21 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Vijay Subramanian, netdev, ncardwell,
	Venkat Venkatsubra, Elliott Hughes, Yuchung Cheng
In-Reply-To: <1351454568.30380.630.camel@edumazet-glaptop>


	Hello,

On Sun, 28 Oct 2012, Eric Dumazet wrote:

> On Sun, 2012-10-28 at 18:51 +0200, Julian Anastasov wrote:
> 
> > 	The idea is:
> > 
> > - expire request_sock as before, based on num_timeout with
> > the idea to catch many SYN retransmissions and to reduce
> > SYN-ACK rate from 2*SYN_rate to 1*SYN_rate, up to
> > tcp_synack_retries SYN-ACKs
> > 
> > - num_retrans accounts sent SYN-ACKs, they can be sent in
> > response to SYN retr or from timer. If num_retrans increases
> > faster than num_timeout it means client uses lower
> > TCP_TIMEOUT_INIT value and sending SYN-ACKs from
> > tcp_check_req is enough because we apply tcp_synack_retries
> > once as a SYN-ACK limit and second time as expiration
> > period.
> > 
> > - If we get 10 SYNs in 1 second, we will send 5 SYN-ACKs
> > immediately (will be restricted in tcp_check_req), from
> > second +1 to +31 we will not send SYN-ACKs if
> > tcp_synack_retries is reached, we will wait for ACK and
> > for more SYNs to drop, silently. Finally, at +63 we expire
> > the request_sock. inet_csk_reqsk_queue_prune still
> > can reduce the expiration period (thresh value) under load.
> > 
> > 	Of course, this is material for separate patch,
> > if idea is liked at all.
> > 
> > Regards
> 
> On a SYNFLOOD attack, we end up sending one SYNACK per SYN message
> anyway ?

	I assume you are referring to want_cookie set by
tcp_syn_flood_action

> If we want to address a non SYNFLOOD attack, why not resetting
> req->expire when we send a SYNACK to a retransmitted SYN ?
> 
> tcp_check_req()
> ...
> 	if (!inet_rtx_syn_ack(sk, req)) {
> 		req->expire = jiffies +
> 			min(TCP_TIMEOUT_INIT << req->num_timeout,
> 			    TCP_RTO_MAX);
> 	}

	This is a good trick, only that it affects the
TCP_DEFER_ACCEPT timing, num_timeout will be increased
in different time.

	But your example reveals another place where
we can optimize: time_after_eq(now, req->expires)
check in inet_csk_reqsk_queue_prune() is immune to
the thresh reduction on load, we do not touch for long
periods of time entries with num_timeout above the desired
thresh. May be we have to use:

time_after_eq(now, req->expires) ||
(req->num_timeout > thresh && !inet_rsk(req)->acked)

	for immediate expire of requests before end of
current period because they already exceed the threshold.
We avoid it only for defer_accept entries with received ACK.
But such checks are not cheap if most of the time we are
not under load.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [PATCH net-next] ipv6: remove useless test in dev_forward_change()
From: Cong Wang @ 2012-10-29  9:22 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1351502649-3876-1-git-send-email-nicolas.dichtel@6wind.com>

On Mon, 29 Oct 2012 at 09:24 GMT, Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> idev->dev cannot be NULL.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

I sent a same patch before you:

http://marc.info/?l=linux-netdev&m=135148225303417&w=2

:)

^ permalink raw reply

* Re: # (c) 2007, Joe Perches <joe@perches.com>
From: Vaibhav Hiremath @ 2012-10-29  9:25 UTC (permalink / raw)
  To: netdev; +Cc: paul, linux-omap, linux-arm-kernel
In-Reply-To: <1351498881-32482-1-git-send-email-hvaibhav@ti.com>



On 10/29/2012 1:51 PM, Vaibhav Hiremath wrote:

Ignore this patch/mail, I screwed up with my command while sending patches.
Sorry for noise.

Thanks,
Vaibhav

^ permalink raw reply

* Re: [PATCH net-next] ipv6: remove useless test in dev_forward_change()
From: Eric Dumazet @ 2012-10-29  9:31 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev
In-Reply-To: <k6lhsd$jui$1@ger.gmane.org>

On Mon, 2012-10-29 at 09:22 +0000, Cong Wang wrote:
> On Mon, 29 Oct 2012 at 09:24 GMT, Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> > idev->dev cannot be NULL.
> >
> > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> 
> I sent a same patch before you:
> 
> http://marc.info/?l=linux-netdev&m=135148225303417&w=2
> 
> :)

Yes, and it got proper attribution to Fengguang Wu

Thanks

^ permalink raw reply

* [Patch net-next] ipv6: introduce ip6_rt_put()
From: Cong Wang @ 2012-10-29  9:55 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet, David S. Miller, Cong Wang

As suggested by Eric, we could introduce a helper function
for ipv6 too, to avoid checking if rt is NULL before
dst_release().

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>

---
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 20210d7..d1327e4 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -213,6 +213,15 @@ static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
 	dst_hold(new);
 }
 
+static inline void ip6_rt_put(struct rt6_info *rt)
+{
+	/* dst_release() accepts a NULL parameter.
+	 * We rely on dst being first structure in struct rt6_info
+	 */
+	BUILD_BUG_ON(offsetof(struct rt6_info, dst) != 0);
+	dst_release(&rt->dst);
+}
+
 struct fib6_walker_t {
 	struct list_head lh;
 	struct fib6_node *root, *node;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 8f0b12a..690a6ab 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -699,7 +699,7 @@ void inet6_ifa_finish_destroy(struct inet6_ifaddr *ifp)
 		pr_warn("Freeing alive inet6 address %p\n", ifp);
 		return;
 	}
-	dst_release(&ifp->rt->dst);
+	ip6_rt_put(&ifp->rt);
 
 	kfree_rcu(ifp, rcu);
 }
@@ -951,7 +951,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 				rt6_set_expires(rt, expires);
 			}
 		}
-		dst_release(&rt->dst);
+		ip6_rt_put(rt);
 	}
 
 	/* clean up prefsrc entries */
@@ -2027,8 +2027,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 			addrconf_prefix_route(&pinfo->prefix, pinfo->prefix_len,
 					      dev, expires, flags);
 		}
-		if (rt)
-			dst_release(&rt->dst);
+		ip6_rt_put(rt);
 	}
 
 	/* Try to figure out our local address for this prefix */
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index d9fb911..2e1a432 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -100,7 +100,7 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp,
 		goto out;
 	}
 again:
-	dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	rt = NULL;
 	goto out;
 
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 92f8e48..b19ed51 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -163,7 +163,7 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 		rt = rt6_lookup(net, addr, NULL, 0, 0);
 		if (rt) {
 			dev = rt->dst.dev;
-			dst_release(&rt->dst);
+			ip6_rt_put(rt);
 		}
 	} else
 		dev = dev_get_by_index_rcu(net, ifindex);
@@ -260,7 +260,7 @@ static struct inet6_dev *ip6_mc_find_dev_rcu(struct net *net,
 
 		if (rt) {
 			dev = rt->dst.dev;
-			dst_release(&rt->dst);
+			ip6_rt_put(rt);
 		}
 	} else
 		dev = dev_get_by_index_rcu(net, ifindex);
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index ff36194..ae0cf81 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1145,7 +1145,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 			ND_PRINTK(0, err,
 				  "RA: %s got default router without neighbour\n",
 				  __func__);
-			dst_release(&rt->dst);
+			ip6_rt_put(rt);
 			return;
 		}
 	}
@@ -1170,7 +1170,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 			ND_PRINTK(0, err,
 				  "RA: %s got default router without neighbour\n",
 				  __func__);
-			dst_release(&rt->dst);
+			ip6_rt_put(rt);
 			return;
 		}
 		neigh->flags |= NTF_ROUTER;
@@ -1326,8 +1326,7 @@ skip_routeinfo:
 		ND_PRINTK(2, warn, "RA: invalid RA options\n");
 	}
 out:
-	if (rt)
-		dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	if (neigh)
 		neigh_release(neigh);
 }
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c42650c..d9badb2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -732,7 +732,7 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
 		else
 			rt6_set_expires(rt, jiffies + HZ * lifetime);
 
-		dst_release(&rt->dst);
+		ip6_rt_put(rt);
 	}
 	return 0;
 }
@@ -948,7 +948,7 @@ restart:
 	else
 		goto out2;
 
-	dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	rt = nrt ? : net->ipv6.ip6_null_entry;
 
 	dst_hold(&rt->dst);
@@ -965,7 +965,7 @@ restart:
 	 * Race condition! In the gap, when table->tb6_lock was
 	 * released someone could insert this route.  Relookup.
 	 */
-	dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	goto relookup;
 
 out:
@@ -1576,7 +1576,7 @@ int ip6_route_add(struct fib6_config *cfg)
 				goto out;
 			if (dev) {
 				if (dev != grt->dst.dev) {
-					dst_release(&grt->dst);
+					ip6_rt_put(grt);
 					goto out;
 				}
 			} else {
@@ -1587,7 +1587,7 @@ int ip6_route_add(struct fib6_config *cfg)
 			}
 			if (!(grt->rt6i_flags & RTF_GATEWAY))
 				err = 0;
-			dst_release(&grt->dst);
+			ip6_rt_put(grt);
 
 			if (err)
 				goto out;
@@ -1673,7 +1673,7 @@ static int __ip6_del_rt(struct rt6_info *rt, struct nl_info *info)
 	write_unlock_bh(&table->tb6_lock);
 
 out:
-	dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	return err;
 }
 
@@ -2726,7 +2726,7 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (!skb) {
-		dst_release(&rt->dst);
+		ip6_rt_put(rt);
 		err = -ENOBUFS;
 		goto errout;
 	}

^ permalink raw reply related

* Re: [Patch net-next] ipv6: introduce ip6_rt_put()
From: Cong Wang @ 2012-10-29  9:59 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet, David S. Miller
In-Reply-To: <1351504521-15003-1-git-send-email-amwang@redhat.com>

On Mon, 2012-10-29 at 17:55 +0800, Cong Wang wrote:
> -       dst_release(&ifp->rt->dst);
> +       ip6_rt_put(&ifp->rt); 

Oops, should be ip6_rt_put(ifp->rt), don't know why gcc doesn't warn
me...

^ permalink raw reply

* Re: [PATCH net-next] ipv6: remove useless test in dev_forward_change()
From: Nicolas Dichtel @ 2012-10-29 10:07 UTC (permalink / raw)
  To: davem; +Cc: fengguang.wu, netdev
In-Reply-To: <1351502649-3876-1-git-send-email-nicolas.dichtel@6wind.com>

Please ignore, the patch has already be sent by Cong Wang <amwang@redhat.com>.

Le 29/10/2012 10:24, Nicolas Dichtel a écrit :
> idev->dev cannot be NULL.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>   net/ipv6/addrconf.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 8f0b12a..c9b1cf3 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -607,7 +607,7 @@ static void dev_forward_change(struct inet6_dev *idev)
>   	dev = idev->dev;
>   	if (idev->cnf.forwarding)
>   		dev_disable_lro(dev);
> -	if (dev && (dev->flags & IFF_MULTICAST)) {
> +	if (dev->flags & IFF_MULTICAST) {
>   		if (idev->cnf.forwarding)
>   			ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
>   		else
>

^ permalink raw reply

* [Patch net-next v2] ipv6: introduce ip6_rt_put()
From: Cong Wang @ 2012-10-29 10:13 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet, David S. Miller, Cong Wang

V2:
fix a typo
more dst_release() -> ip6_rt_put()

As suggested by Eric, we could introduce a helper function
for ipv6 too, to avoid checking if rt is NULL before
dst_release().

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>

---
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 20210d7..d1327e4 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -213,6 +213,15 @@ static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
 	dst_hold(new);
 }
 
+static inline void ip6_rt_put(struct rt6_info *rt)
+{
+	/* dst_release() accepts a NULL parameter.
+	 * We rely on dst being first structure in struct rt6_info
+	 */
+	BUILD_BUG_ON(offsetof(struct rt6_info, dst) != 0);
+	dst_release(&rt->dst);
+}
+
 struct fib6_walker_t {
 	struct list_head lh;
 	struct fib6_node *root, *node;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 8f0b12a..5f46ace 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -699,7 +699,7 @@ void inet6_ifa_finish_destroy(struct inet6_ifaddr *ifp)
 		pr_warn("Freeing alive inet6 address %p\n", ifp);
 		return;
 	}
-	dst_release(&ifp->rt->dst);
+	ip6_rt_put(ifp->rt);
 
 	kfree_rcu(ifp, rcu);
 }
@@ -951,7 +951,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 				rt6_set_expires(rt, expires);
 			}
 		}
-		dst_release(&rt->dst);
+		ip6_rt_put(rt);
 	}
 
 	/* clean up prefsrc entries */
@@ -2027,8 +2027,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 			addrconf_prefix_route(&pinfo->prefix, pinfo->prefix_len,
 					      dev, expires, flags);
 		}
-		if (rt)
-			dst_release(&rt->dst);
+		ip6_rt_put(rt);
 	}
 
 	/* Try to figure out our local address for this prefix */
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index cdf02be..4963c76 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -84,7 +84,7 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 		rt = rt6_lookup(net, addr, NULL, 0, 0);
 		if (rt) {
 			dev = rt->dst.dev;
-			dst_release(&rt->dst);
+			ip6_rt_put(rt);
 		} else if (ishost) {
 			err = -EADDRNOTAVAIL;
 			goto error;
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index d9fb911..2e1a432 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -100,7 +100,7 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp,
 		goto out;
 	}
 again:
-	dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	rt = NULL;
 	goto out;
 
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 0185679..bbe2e7b 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1069,7 +1069,7 @@ static void ip6gre_tnl_link_config(struct ip6_tnl *t, int set_mtu)
 					dev->mtu = IPV6_MIN_MTU;
 			}
 		}
-		dst_release(&rt->dst);
+		ip6_rt_put(rt);
 	}
 
 	t->hlen = addend;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index aece3e7..9d9853c 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -756,7 +756,7 @@ int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 		if (err == 0) {
 			IP6_INC_STATS(net, ip6_dst_idev(&rt->dst),
 				      IPSTATS_MIB_FRAGOKS);
-			dst_release(&rt->dst);
+			ip6_rt_put(rt);
 			return 0;
 		}
 
@@ -768,7 +768,7 @@ int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 
 		IP6_INC_STATS(net, ip6_dst_idev(&rt->dst),
 			      IPSTATS_MIB_FRAGFAILS);
-		dst_release(&rt->dst);
+		ip6_rt_put(rt);
 		return err;
 
 slow_path_clean:
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index cb7e2de..09482f7 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -663,8 +663,7 @@ ip6ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 
 		icmpv6_send(skb2, rel_type, rel_code, rel_info);
 
-		if (rt)
-			dst_release(&rt->dst);
+		ip6_rt_put(rt);
 
 		kfree_skb(skb2);
 	}
@@ -1208,7 +1207,7 @@ static void ip6_tnl_link_config(struct ip6_tnl *t)
 			if (dev->mtu < IPV6_MIN_MTU)
 				dev->mtu = IPV6_MIN_MTU;
 		}
-		dst_release(&rt->dst);
+		ip6_rt_put(rt);
 	}
 }
 
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 92f8e48..b19ed51 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -163,7 +163,7 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 		rt = rt6_lookup(net, addr, NULL, 0, 0);
 		if (rt) {
 			dev = rt->dst.dev;
-			dst_release(&rt->dst);
+			ip6_rt_put(rt);
 		}
 	} else
 		dev = dev_get_by_index_rcu(net, ifindex);
@@ -260,7 +260,7 @@ static struct inet6_dev *ip6_mc_find_dev_rcu(struct net *net,
 
 		if (rt) {
 			dev = rt->dst.dev;
-			dst_release(&rt->dst);
+			ip6_rt_put(rt);
 		}
 	} else
 		dev = dev_get_by_index_rcu(net, ifindex);
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index ff36194..ae0cf81 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1145,7 +1145,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 			ND_PRINTK(0, err,
 				  "RA: %s got default router without neighbour\n",
 				  __func__);
-			dst_release(&rt->dst);
+			ip6_rt_put(rt);
 			return;
 		}
 	}
@@ -1170,7 +1170,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 			ND_PRINTK(0, err,
 				  "RA: %s got default router without neighbour\n",
 				  __func__);
-			dst_release(&rt->dst);
+			ip6_rt_put(rt);
 			return;
 		}
 		neigh->flags |= NTF_ROUTER;
@@ -1326,8 +1326,7 @@ skip_routeinfo:
 		ND_PRINTK(2, warn, "RA: invalid RA options\n");
 	}
 out:
-	if (rt)
-		dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	if (neigh)
 		neigh_release(neigh);
 }
diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
index 5d1d8b0..5060d54 100644
--- a/net/ipv6/netfilter/ip6t_rpfilter.c
+++ b/net/ipv6/netfilter/ip6t_rpfilter.c
@@ -67,7 +67,7 @@ static bool rpfilter_lookup_reverse6(const struct sk_buff *skb,
 	if (rt->rt6i_idev->dev == dev || (flags & XT_RPFILTER_LOOSE))
 		ret = true;
  out:
-	dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	return ret;
 }
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c42650c..d9badb2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -732,7 +732,7 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
 		else
 			rt6_set_expires(rt, jiffies + HZ * lifetime);
 
-		dst_release(&rt->dst);
+		ip6_rt_put(rt);
 	}
 	return 0;
 }
@@ -948,7 +948,7 @@ restart:
 	else
 		goto out2;
 
-	dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	rt = nrt ? : net->ipv6.ip6_null_entry;
 
 	dst_hold(&rt->dst);
@@ -965,7 +965,7 @@ restart:
 	 * Race condition! In the gap, when table->tb6_lock was
 	 * released someone could insert this route.  Relookup.
 	 */
-	dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	goto relookup;
 
 out:
@@ -1576,7 +1576,7 @@ int ip6_route_add(struct fib6_config *cfg)
 				goto out;
 			if (dev) {
 				if (dev != grt->dst.dev) {
-					dst_release(&grt->dst);
+					ip6_rt_put(grt);
 					goto out;
 				}
 			} else {
@@ -1587,7 +1587,7 @@ int ip6_route_add(struct fib6_config *cfg)
 			}
 			if (!(grt->rt6i_flags & RTF_GATEWAY))
 				err = 0;
-			dst_release(&grt->dst);
+			ip6_rt_put(grt);
 
 			if (err)
 				goto out;
@@ -1673,7 +1673,7 @@ static int __ip6_del_rt(struct rt6_info *rt, struct nl_info *info)
 	write_unlock_bh(&table->tb6_lock);
 
 out:
-	dst_release(&rt->dst);
+	ip6_rt_put(rt);
 	return err;
 }
 
@@ -2726,7 +2726,7 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (!skb) {
-		dst_release(&rt->dst);
+		ip6_rt_put(rt);
 		err = -ENOBUFS;
 		goto errout;
 	}

^ permalink raw reply related

* Re: [PATCH RFC] pkt_sched: enable QFQ to support TSO/GSO
From: Cong Wang @ 2012-10-29 11:08 UTC (permalink / raw)
  To: Paolo Valente
  Cc: jhs, davem, shemminger, linux-kernel, netdev, rizzo, fchecconi
In-Reply-To: <20121029085106.GA3733@paolo-ThinkPad-W520>

On 10/29/2012 04:51 PM, Paolo Valente wrote:
> Hi,
> if the max packet size for some class (configured through tc) is
> violated by the actual size of the packets of that class, then QFQ
> would not schedule classes correctly, and the data structures
> implementing the bucket lists may get corrupted. This problem occurs
> with TSO/GSO even if the max packet size is set to the MTU, and is,
> e.g., the cause of the failure reported in [1]. Two patches have been
> proposed to solve this problem in [2], one of them is a preliminary
> version of this patch.
>
> This patch addresses the above issues by: 1) setting QFQ parameters to
> proper values for supporting TSO/GSO (in particular, setting the
> maximum possible packet size to 64KB), 2) automatically increasing the
> max packet size for a class, lmax, when a packet with a larger size
> than the current value of lmax arrives (a notification is also appended
> to the log).
>
> The drawback of the first point is that the maximum weight for a class
> is now limited to 4096, which is equal to 1/16 of the maximum weight
> sum.
>
> Finally, this patch also forcibly caps the timestamps of a class if
> they are too high to be stored in the bucket list. This capping, taken
> from QFQ+ [3], handles the unfrequent case described in the comment to
> the function slot_insert.
>
> [1] http://marc.info/?l=linux-netdev&m=134968777902077&w=2
> [2] http://marc.info/?l=linux-netdev&m=135096573507936&w=2
> [3] http://marc.info/?l=linux-netdev&m=134902691421670&w=2
>
> Signed-off-by: Paolo Valente <paolo.valente@unimore.it>


Please respect people who helps you to test it:

Tested-by: Cong Wang <amwang@redhat.com>

I tested it again just in case...

By the way, one nit below:


> +	if (unlikely(cl->lmax < qdisc_pkt_len(skb))) {
> +		pr_notice("qfq: increasing maxpkt from %u to %u for class %u",
> +			  cl->lmax, qdisc_pkt_len(skb), cl->common.classid);
> +		qfq_update_reactivate_class(q, cl, cl->inv_w,
> +					    qdisc_pkt_len(skb), 0);
> +	}


Seems the log level KERN_NOTICE is overkill, normal users don't care 
about these information, so s/pr_notice/pr_debug/.


Thanks!

^ permalink raw reply

* Re: [PATCH RFC] pkt_sched: enable QFQ to support TSO/GSO
From: Paolo Valente @ 2012-10-29 11:24 UTC (permalink / raw)
  To: Cong Wang; +Cc: jhs, davem, shemminger, linux-kernel, netdev, rizzo, fchecconi
In-Reply-To: <508E63AD.20803@gmail.com>

Il 29/10/2012 12:08, Cong Wang ha scritto:
> On 10/29/2012 04:51 PM, Paolo Valente wrote:
>> Hi,
>> if the max packet size for some class (configured through tc) is
>> violated by the actual size of the packets of that class, then QFQ
>> would not schedule classes correctly, and the data structures
>> implementing the bucket lists may get corrupted. This problem occurs
>> with TSO/GSO even if the max packet size is set to the MTU, and is,
>> e.g., the cause of the failure reported in [1]. Two patches have been
>> proposed to solve this problem in [2], one of them is a preliminary
>> version of this patch.
>>
>> This patch addresses the above issues by: 1) setting QFQ parameters to
>> proper values for supporting TSO/GSO (in particular, setting the
>> maximum possible packet size to 64KB), 2) automatically increasing the
>> max packet size for a class, lmax, when a packet with a larger size
>> than the current value of lmax arrives (a notification is also appended
>> to the log).
>>
>> The drawback of the first point is that the maximum weight for a class
>> is now limited to 4096, which is equal to 1/16 of the maximum weight
>> sum.
>>
>> Finally, this patch also forcibly caps the timestamps of a class if
>> they are too high to be stored in the bucket list. This capping, taken
>> from QFQ+ [3], handles the unfrequent case described in the comment to
>> the function slot_insert.
>>
>> [1] http://marc.info/?l=linux-netdev&m=134968777902077&w=2
>> [2] http://marc.info/?l=linux-netdev&m=135096573507936&w=2
>> [3] http://marc.info/?l=linux-netdev&m=134902691421670&w=2
>>
>> Signed-off-by: Paolo Valente <paolo.valente@unimore.it>
>
>
> Please respect people who helps you to test it:
>
> Tested-by: Cong Wang <amwang@redhat.com>
Oops, really sorry about that. I did not mean to hide your contribution. 
I am just not familiar with the process.
>
> I tested it again just in case...
Thanks again
>
> By the way, one nit below:
>
>
>> +    if (unlikely(cl->lmax < qdisc_pkt_len(skb))) {
>> +        pr_notice("qfq: increasing maxpkt from %u to %u for class %u",
>> +              cl->lmax, qdisc_pkt_len(skb), cl->common.classid);
>> +        qfq_update_reactivate_class(q, cl, cl->inv_w,
>> +                        qdisc_pkt_len(skb), 0);
>> +    }
>
>
> Seems the log level KERN_NOTICE is overkill, normal users don't care
> about these information, so s/pr_notice/pr_debug/.
>
Thanks, I will integrate this and possible other suggestions in a new 
version of the patch.
>
> Thanks!
>
>


-- 
-----------------------------------------------------------
| Paolo Valente              |                            |
| Algogroup                  |                            |
| Dip. Ing. Informazione     | tel:   +39 059 2056318     |
| Via Vignolese 905/b        | fax:   +39 059 2056129     |
| 41125 Modena - Italy       |                            |
|     home:  http://algo.ing.unimo.it/people/paolo/       |
-----------------------------------------------------------

^ 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