Netdev List
 help / color / mirror / Atom feed
* Re: [net-next 00/15][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2014-01-06 18:49 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, gospo, sassmann
In-Reply-To: <20140106.132628.418605275145035482.davem@davemloft.net>

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

On Mon, 2014-01-06 at 13:26 -0500, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Mon,  6 Jan 2014 04:30:21 -0800
> 
> > This series contains updates to i40e only.
>  ...
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
> 
> Pulled, please make sure the coding style feedback does actually
> get addressed.
> 
> Thanks Jeff.

Yes, I will make sure it is in one of the upcoming series.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH net-next 1/2] bridge: use spin_lock_bh() in br_multicast_set_hash_max
From: Scott Feldman @ 2014-01-06 19:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, roopa, bridge, curt, shm

From: Curt Brune <curt@cumulusnetworks.com>

br_multicast_set_hash_max() is called from process context in
net/bridge/br_sysfs_br.c by the sysfs store_hash_max() function.

br_multicast_set_hash_max() calls spin_lock(&br->multicast_lock),
which can deadlock the CPU if a softirq that also tries to take the
same lock interrupts br_multicast_set_hash_max() while the lock is
held .  This can happen quite easily when any of the bridge multicast
timers expire, which try to take the same lock.

The fix here is to use spin_lock_bh(), preventing other softirqs from
executing on this CPU.

Steps to reproduce:

1. Create a bridge with several interfaces (I used 4).
2. Set the "multicast query interval" to a low number, like 2.
3. Enable the bridge as a multicast querier.
4. Repeatedly set the bridge hash_max parameter via sysfs.

  # brctl addbr br0
  # brctl addif br0 eth1 eth2 eth3 eth4
  # brctl setmcqi br0 2
  # brctl setmcquerier br0 1

  # while true ; do echo 4096 > /sys/class/net/br0/bridge/hash_max; done

Signed-off-by: Curt Brune <curt@cumulusnetworks.com>
Signed-off-by: Scott Feldman <sfeldma@cumulusnetworks.com>
---
 net/bridge/br_multicast.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 4c214b2..ef66365 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1998,7 +1998,7 @@ int br_multicast_set_hash_max(struct net_bridge *br, unsigned long val)
 	u32 old;
 	struct net_bridge_mdb_htable *mdb;
 
-	spin_lock(&br->multicast_lock);
+	spin_lock_bh(&br->multicast_lock);
 	if (!netif_running(br->dev))
 		goto unlock;
 
@@ -2030,7 +2030,7 @@ rollback:
 	}
 
 unlock:
-	spin_unlock(&br->multicast_lock);
+	spin_unlock_bh(&br->multicast_lock);
 
 	return err;
 }

^ permalink raw reply related

* [PATCH net-next 0/2] bridge: spin_lock fix plus sysfs cleanup
From: Scott Feldman @ 2014-01-06 19:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, roopa, bridge, curt, shm

The following series implements:

- br_multicast_set_hash_max() is called from process context (via sysfs)
  and needs spin_lock_bh to disable bhs.

- cleanup bridge sysfs definitions by using DEVICE_ATTR_xx macros

---

Curt Brune (1):
      bridge: use spin_lock_bh() in br_multicast_set_hash_max

Scott Feldman (1):
      bridge: use DEVICE_ATTR_xx macros


 net/bridge/br_multicast.c |    4 -
 net/bridge/br_sysfs_br.c  |  249 ++++++++++++++++++++-------------------------
 2 files changed, 110 insertions(+), 143 deletions(-)

-- 
Signature

^ permalink raw reply

* [PATCH net-next 2/2] bridge: use DEVICE_ATTR_xx macros
From: Scott Feldman @ 2014-01-06 19:00 UTC (permalink / raw)
  To: stephen; +Cc: netdev, roopa, bridge, curt, shm

Use DEVICE_ATTR_RO/RW macros to simplify bridge sysfs attribute definitions.

Signed-off-by: Scott Feldman <sfeldma@cumulusnetworks.com>
---
 net/bridge/br_sysfs_br.c |  249 ++++++++++++++++++++--------------------------
 1 file changed, 108 insertions(+), 141 deletions(-)

diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 3b9637f..8dac6555 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -49,53 +49,51 @@ static ssize_t store_bridge_parm(struct device *d,
 }
 
 
-static ssize_t show_forward_delay(struct device *d,
+static ssize_t forward_delay_show(struct device *d,
 				  struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
 	return sprintf(buf, "%lu\n", jiffies_to_clock_t(br->forward_delay));
 }
 
-static ssize_t store_forward_delay(struct device *d,
+static ssize_t forward_delay_store(struct device *d,
 				   struct device_attribute *attr,
 				   const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, br_set_forward_delay);
 }
-static DEVICE_ATTR(forward_delay, S_IRUGO | S_IWUSR,
-		   show_forward_delay, store_forward_delay);
+static DEVICE_ATTR_RW(forward_delay);
 
-static ssize_t show_hello_time(struct device *d, struct device_attribute *attr,
+static ssize_t hello_time_show(struct device *d, struct device_attribute *attr,
 			       char *buf)
 {
 	return sprintf(buf, "%lu\n",
 		       jiffies_to_clock_t(to_bridge(d)->hello_time));
 }
 
-static ssize_t store_hello_time(struct device *d,
+static ssize_t hello_time_store(struct device *d,
 				struct device_attribute *attr, const char *buf,
 				size_t len)
 {
 	return store_bridge_parm(d, buf, len, br_set_hello_time);
 }
-static DEVICE_ATTR(hello_time, S_IRUGO | S_IWUSR, show_hello_time,
-		   store_hello_time);
+static DEVICE_ATTR_RW(hello_time);
 
-static ssize_t show_max_age(struct device *d, struct device_attribute *attr,
+static ssize_t max_age_show(struct device *d, struct device_attribute *attr,
 			    char *buf)
 {
 	return sprintf(buf, "%lu\n",
 		       jiffies_to_clock_t(to_bridge(d)->max_age));
 }
 
-static ssize_t store_max_age(struct device *d, struct device_attribute *attr,
+static ssize_t max_age_store(struct device *d, struct device_attribute *attr,
 			     const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, br_set_max_age);
 }
-static DEVICE_ATTR(max_age, S_IRUGO | S_IWUSR, show_max_age, store_max_age);
+static DEVICE_ATTR_RW(max_age);
 
-static ssize_t show_ageing_time(struct device *d,
+static ssize_t ageing_time_show(struct device *d,
 				struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -108,16 +106,15 @@ static int set_ageing_time(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_ageing_time(struct device *d,
+static ssize_t ageing_time_store(struct device *d,
 				 struct device_attribute *attr,
 				 const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_ageing_time);
 }
-static DEVICE_ATTR(ageing_time, S_IRUGO | S_IWUSR, show_ageing_time,
-		   store_ageing_time);
+static DEVICE_ATTR_RW(ageing_time);
 
-static ssize_t show_stp_state(struct device *d,
+static ssize_t stp_state_show(struct device *d,
 			      struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -125,7 +122,7 @@ static ssize_t show_stp_state(struct device *d,
 }
 
 
-static ssize_t store_stp_state(struct device *d,
+static ssize_t stp_state_store(struct device *d,
 			       struct device_attribute *attr, const char *buf,
 			       size_t len)
 {
@@ -147,20 +144,21 @@ static ssize_t store_stp_state(struct device *d,
 
 	return len;
 }
-static DEVICE_ATTR(stp_state, S_IRUGO | S_IWUSR, show_stp_state,
-		   store_stp_state);
+static DEVICE_ATTR_RW(stp_state);
 
-static ssize_t show_group_fwd_mask(struct device *d,
-			      struct device_attribute *attr, char *buf)
+static ssize_t group_fwd_mask_show(struct device *d,
+				   struct device_attribute *attr,
+				   char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
 	return sprintf(buf, "%#x\n", br->group_fwd_mask);
 }
 
 
-static ssize_t store_group_fwd_mask(struct device *d,
-			       struct device_attribute *attr, const char *buf,
-			       size_t len)
+static ssize_t group_fwd_mask_store(struct device *d,
+				    struct device_attribute *attr,
+				    const char *buf,
+				    size_t len)
 {
 	struct net_bridge *br = to_bridge(d);
 	char *endp;
@@ -180,10 +178,9 @@ static ssize_t store_group_fwd_mask(struct device *d,
 
 	return len;
 }
-static DEVICE_ATTR(group_fwd_mask, S_IRUGO | S_IWUSR, show_group_fwd_mask,
-		   store_group_fwd_mask);
+static DEVICE_ATTR_RW(group_fwd_mask);
 
-static ssize_t show_priority(struct device *d, struct device_attribute *attr,
+static ssize_t priority_show(struct device *d, struct device_attribute *attr,
 			     char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -197,93 +194,91 @@ static int set_priority(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_priority(struct device *d, struct device_attribute *attr,
-			       const char *buf, size_t len)
+static ssize_t priority_store(struct device *d, struct device_attribute *attr,
+			      const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_priority);
 }
-static DEVICE_ATTR(priority, S_IRUGO | S_IWUSR, show_priority, store_priority);
+static DEVICE_ATTR_RW(priority);
 
-static ssize_t show_root_id(struct device *d, struct device_attribute *attr,
+static ssize_t root_id_show(struct device *d, struct device_attribute *attr,
 			    char *buf)
 {
 	return br_show_bridge_id(buf, &to_bridge(d)->designated_root);
 }
-static DEVICE_ATTR(root_id, S_IRUGO, show_root_id, NULL);
+static DEVICE_ATTR_RO(root_id);
 
-static ssize_t show_bridge_id(struct device *d, struct device_attribute *attr,
+static ssize_t bridge_id_show(struct device *d, struct device_attribute *attr,
 			      char *buf)
 {
 	return br_show_bridge_id(buf, &to_bridge(d)->bridge_id);
 }
-static DEVICE_ATTR(bridge_id, S_IRUGO, show_bridge_id, NULL);
+static DEVICE_ATTR_RO(bridge_id);
 
-static ssize_t show_root_port(struct device *d, struct device_attribute *attr,
+static ssize_t root_port_show(struct device *d, struct device_attribute *attr,
 			      char *buf)
 {
 	return sprintf(buf, "%d\n", to_bridge(d)->root_port);
 }
-static DEVICE_ATTR(root_port, S_IRUGO, show_root_port, NULL);
+static DEVICE_ATTR_RO(root_port);
 
-static ssize_t show_root_path_cost(struct device *d,
+static ssize_t root_path_cost_show(struct device *d,
 				   struct device_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%d\n", to_bridge(d)->root_path_cost);
 }
-static DEVICE_ATTR(root_path_cost, S_IRUGO, show_root_path_cost, NULL);
+static DEVICE_ATTR_RO(root_path_cost);
 
-static ssize_t show_topology_change(struct device *d,
+static ssize_t topology_change_show(struct device *d,
 				    struct device_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%d\n", to_bridge(d)->topology_change);
 }
-static DEVICE_ATTR(topology_change, S_IRUGO, show_topology_change, NULL);
+static DEVICE_ATTR_RO(topology_change);
 
-static ssize_t show_topology_change_detected(struct device *d,
+static ssize_t topology_change_detected_show(struct device *d,
 					     struct device_attribute *attr,
 					     char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
 	return sprintf(buf, "%d\n", br->topology_change_detected);
 }
-static DEVICE_ATTR(topology_change_detected, S_IRUGO,
-		   show_topology_change_detected, NULL);
+static DEVICE_ATTR_RO(topology_change_detected);
 
-static ssize_t show_hello_timer(struct device *d,
+static ssize_t hello_timer_show(struct device *d,
 				struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
 	return sprintf(buf, "%ld\n", br_timer_value(&br->hello_timer));
 }
-static DEVICE_ATTR(hello_timer, S_IRUGO, show_hello_timer, NULL);
+static DEVICE_ATTR_RO(hello_timer);
 
-static ssize_t show_tcn_timer(struct device *d, struct device_attribute *attr,
+static ssize_t tcn_timer_show(struct device *d, struct device_attribute *attr,
 			      char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
 	return sprintf(buf, "%ld\n", br_timer_value(&br->tcn_timer));
 }
-static DEVICE_ATTR(tcn_timer, S_IRUGO, show_tcn_timer, NULL);
+static DEVICE_ATTR_RO(tcn_timer);
 
-static ssize_t show_topology_change_timer(struct device *d,
+static ssize_t topology_change_timer_show(struct device *d,
 					  struct device_attribute *attr,
 					  char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
 	return sprintf(buf, "%ld\n", br_timer_value(&br->topology_change_timer));
 }
-static DEVICE_ATTR(topology_change_timer, S_IRUGO, show_topology_change_timer,
-		   NULL);
+static DEVICE_ATTR_RO(topology_change_timer);
 
-static ssize_t show_gc_timer(struct device *d, struct device_attribute *attr,
+static ssize_t gc_timer_show(struct device *d, struct device_attribute *attr,
 			     char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
 	return sprintf(buf, "%ld\n", br_timer_value(&br->gc_timer));
 }
-static DEVICE_ATTR(gc_timer, S_IRUGO, show_gc_timer, NULL);
+static DEVICE_ATTR_RO(gc_timer);
 
-static ssize_t show_group_addr(struct device *d,
+static ssize_t group_addr_show(struct device *d,
 			       struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -293,7 +288,7 @@ static ssize_t show_group_addr(struct device *d,
 		       br->group_addr[4], br->group_addr[5]);
 }
 
-static ssize_t store_group_addr(struct device *d,
+static ssize_t group_addr_store(struct device *d,
 				struct device_attribute *attr,
 				const char *buf, size_t len)
 {
@@ -324,10 +319,9 @@ static ssize_t store_group_addr(struct device *d,
 	return len;
 }
 
-static DEVICE_ATTR(group_addr, S_IRUGO | S_IWUSR,
-		   show_group_addr, store_group_addr);
+static DEVICE_ATTR_RW(group_addr);
 
-static ssize_t store_flush(struct device *d,
+static ssize_t flush_store(struct device *d,
 			   struct device_attribute *attr,
 			   const char *buf, size_t len)
 {
@@ -339,26 +333,25 @@ static ssize_t store_flush(struct device *d,
 	br_fdb_flush(br);
 	return len;
 }
-static DEVICE_ATTR(flush, S_IWUSR, NULL, store_flush);
+static DEVICE_ATTR_WO(flush);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
-static ssize_t show_multicast_router(struct device *d,
+static ssize_t multicast_router_show(struct device *d,
 				     struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
 	return sprintf(buf, "%d\n", br->multicast_router);
 }
 
-static ssize_t store_multicast_router(struct device *d,
+static ssize_t multicast_router_store(struct device *d,
 				      struct device_attribute *attr,
 				      const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, br_multicast_set_router);
 }
-static DEVICE_ATTR(multicast_router, S_IRUGO | S_IWUSR, show_multicast_router,
-		   store_multicast_router);
+static DEVICE_ATTR_RW(multicast_router);
 
-static ssize_t show_multicast_snooping(struct device *d,
+static ssize_t multicast_snooping_show(struct device *d,
 				       struct device_attribute *attr,
 				       char *buf)
 {
@@ -366,18 +359,17 @@ static ssize_t show_multicast_snooping(struct device *d,
 	return sprintf(buf, "%d\n", !br->multicast_disabled);
 }
 
-static ssize_t store_multicast_snooping(struct device *d,
+static ssize_t multicast_snooping_store(struct device *d,
 					struct device_attribute *attr,
 					const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, br_multicast_toggle);
 }
-static DEVICE_ATTR(multicast_snooping, S_IRUGO | S_IWUSR,
-		   show_multicast_snooping, store_multicast_snooping);
+static DEVICE_ATTR_RW(multicast_snooping);
 
-static ssize_t show_multicast_query_use_ifaddr(struct device *d,
-				      struct device_attribute *attr,
-				      char *buf)
+static ssize_t multicast_query_use_ifaddr_show(struct device *d,
+					       struct device_attribute *attr,
+					       char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
 	return sprintf(buf, "%d\n", br->multicast_query_use_ifaddr);
@@ -390,17 +382,15 @@ static int set_query_use_ifaddr(struct net_bridge *br, unsigned long val)
 }
 
 static ssize_t
-store_multicast_query_use_ifaddr(struct device *d,
+multicast_query_use_ifaddr_store(struct device *d,
 				 struct device_attribute *attr,
 				 const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_query_use_ifaddr);
 }
-static DEVICE_ATTR(multicast_query_use_ifaddr, S_IRUGO | S_IWUSR,
-		   show_multicast_query_use_ifaddr,
-		   store_multicast_query_use_ifaddr);
+static DEVICE_ATTR_RW(multicast_query_use_ifaddr);
 
-static ssize_t show_multicast_querier(struct device *d,
+static ssize_t multicast_querier_show(struct device *d,
 				      struct device_attribute *attr,
 				      char *buf)
 {
@@ -408,16 +398,15 @@ static ssize_t show_multicast_querier(struct device *d,
 	return sprintf(buf, "%d\n", br->multicast_querier);
 }
 
-static ssize_t store_multicast_querier(struct device *d,
+static ssize_t multicast_querier_store(struct device *d,
 				       struct device_attribute *attr,
 				       const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, br_multicast_set_querier);
 }
-static DEVICE_ATTR(multicast_querier, S_IRUGO | S_IWUSR,
-		   show_multicast_querier, store_multicast_querier);
+static DEVICE_ATTR_RW(multicast_querier);
 
-static ssize_t show_hash_elasticity(struct device *d,
+static ssize_t hash_elasticity_show(struct device *d,
 				    struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -430,31 +419,29 @@ static int set_elasticity(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_hash_elasticity(struct device *d,
+static ssize_t hash_elasticity_store(struct device *d,
 				     struct device_attribute *attr,
 				     const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_elasticity);
 }
-static DEVICE_ATTR(hash_elasticity, S_IRUGO | S_IWUSR, show_hash_elasticity,
-		   store_hash_elasticity);
+static DEVICE_ATTR_RW(hash_elasticity);
 
-static ssize_t show_hash_max(struct device *d, struct device_attribute *attr,
+static ssize_t hash_max_show(struct device *d, struct device_attribute *attr,
 			     char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
 	return sprintf(buf, "%u\n", br->hash_max);
 }
 
-static ssize_t store_hash_max(struct device *d, struct device_attribute *attr,
+static ssize_t hash_max_store(struct device *d, struct device_attribute *attr,
 			      const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, br_multicast_set_hash_max);
 }
-static DEVICE_ATTR(hash_max, S_IRUGO | S_IWUSR, show_hash_max,
-		   store_hash_max);
+static DEVICE_ATTR_RW(hash_max);
 
-static ssize_t show_multicast_last_member_count(struct device *d,
+static ssize_t multicast_last_member_count_show(struct device *d,
 						struct device_attribute *attr,
 						char *buf)
 {
@@ -468,17 +455,15 @@ static int set_last_member_count(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_multicast_last_member_count(struct device *d,
+static ssize_t multicast_last_member_count_store(struct device *d,
 						 struct device_attribute *attr,
 						 const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_last_member_count);
 }
-static DEVICE_ATTR(multicast_last_member_count, S_IRUGO | S_IWUSR,
-		   show_multicast_last_member_count,
-		   store_multicast_last_member_count);
+static DEVICE_ATTR_RW(multicast_last_member_count);
 
-static ssize_t show_multicast_startup_query_count(
+static ssize_t multicast_startup_query_count_show(
 	struct device *d, struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -491,17 +476,15 @@ static int set_startup_query_count(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_multicast_startup_query_count(
+static ssize_t multicast_startup_query_count_store(
 	struct device *d, struct device_attribute *attr, const char *buf,
 	size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_startup_query_count);
 }
-static DEVICE_ATTR(multicast_startup_query_count, S_IRUGO | S_IWUSR,
-		   show_multicast_startup_query_count,
-		   store_multicast_startup_query_count);
+static DEVICE_ATTR_RW(multicast_startup_query_count);
 
-static ssize_t show_multicast_last_member_interval(
+static ssize_t multicast_last_member_interval_show(
 	struct device *d, struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -515,17 +498,15 @@ static int set_last_member_interval(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_multicast_last_member_interval(
+static ssize_t multicast_last_member_interval_store(
 	struct device *d, struct device_attribute *attr, const char *buf,
 	size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_last_member_interval);
 }
-static DEVICE_ATTR(multicast_last_member_interval, S_IRUGO | S_IWUSR,
-		   show_multicast_last_member_interval,
-		   store_multicast_last_member_interval);
+static DEVICE_ATTR_RW(multicast_last_member_interval);
 
-static ssize_t show_multicast_membership_interval(
+static ssize_t multicast_membership_interval_show(
 	struct device *d, struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -539,17 +520,15 @@ static int set_membership_interval(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_multicast_membership_interval(
+static ssize_t multicast_membership_interval_store(
 	struct device *d, struct device_attribute *attr, const char *buf,
 	size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_membership_interval);
 }
-static DEVICE_ATTR(multicast_membership_interval, S_IRUGO | S_IWUSR,
-		   show_multicast_membership_interval,
-		   store_multicast_membership_interval);
+static DEVICE_ATTR_RW(multicast_membership_interval);
 
-static ssize_t show_multicast_querier_interval(struct device *d,
+static ssize_t multicast_querier_interval_show(struct device *d,
 					       struct device_attribute *attr,
 					       char *buf)
 {
@@ -564,17 +543,15 @@ static int set_querier_interval(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_multicast_querier_interval(struct device *d,
+static ssize_t multicast_querier_interval_store(struct device *d,
 						struct device_attribute *attr,
 						const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_querier_interval);
 }
-static DEVICE_ATTR(multicast_querier_interval, S_IRUGO | S_IWUSR,
-		   show_multicast_querier_interval,
-		   store_multicast_querier_interval);
+static DEVICE_ATTR_RW(multicast_querier_interval);
 
-static ssize_t show_multicast_query_interval(struct device *d,
+static ssize_t multicast_query_interval_show(struct device *d,
 					     struct device_attribute *attr,
 					     char *buf)
 {
@@ -589,17 +566,15 @@ static int set_query_interval(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_multicast_query_interval(struct device *d,
+static ssize_t multicast_query_interval_store(struct device *d,
 					      struct device_attribute *attr,
 					      const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_query_interval);
 }
-static DEVICE_ATTR(multicast_query_interval, S_IRUGO | S_IWUSR,
-		   show_multicast_query_interval,
-		   store_multicast_query_interval);
+static DEVICE_ATTR_RW(multicast_query_interval);
 
-static ssize_t show_multicast_query_response_interval(
+static ssize_t multicast_query_response_interval_show(
 	struct device *d, struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -614,17 +589,15 @@ static int set_query_response_interval(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_multicast_query_response_interval(
+static ssize_t multicast_query_response_interval_store(
 	struct device *d, struct device_attribute *attr, const char *buf,
 	size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_query_response_interval);
 }
-static DEVICE_ATTR(multicast_query_response_interval, S_IRUGO | S_IWUSR,
-		   show_multicast_query_response_interval,
-		   store_multicast_query_response_interval);
+static DEVICE_ATTR_RW(multicast_query_response_interval);
 
-static ssize_t show_multicast_startup_query_interval(
+static ssize_t multicast_startup_query_interval_show(
 	struct device *d, struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -639,18 +612,16 @@ static int set_startup_query_interval(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_multicast_startup_query_interval(
+static ssize_t multicast_startup_query_interval_store(
 	struct device *d, struct device_attribute *attr, const char *buf,
 	size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_startup_query_interval);
 }
-static DEVICE_ATTR(multicast_startup_query_interval, S_IRUGO | S_IWUSR,
-		   show_multicast_startup_query_interval,
-		   store_multicast_startup_query_interval);
+static DEVICE_ATTR_RW(multicast_startup_query_interval);
 #endif
 #ifdef CONFIG_BRIDGE_NETFILTER
-static ssize_t show_nf_call_iptables(
+static ssize_t nf_call_iptables_show(
 	struct device *d, struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -663,16 +634,15 @@ static int set_nf_call_iptables(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_nf_call_iptables(
+static ssize_t nf_call_iptables_store(
 	struct device *d, struct device_attribute *attr, const char *buf,
 	size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_nf_call_iptables);
 }
-static DEVICE_ATTR(nf_call_iptables, S_IRUGO | S_IWUSR,
-		   show_nf_call_iptables, store_nf_call_iptables);
+static DEVICE_ATTR_RW(nf_call_iptables);
 
-static ssize_t show_nf_call_ip6tables(
+static ssize_t nf_call_ip6tables_show(
 	struct device *d, struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -685,16 +655,15 @@ static int set_nf_call_ip6tables(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_nf_call_ip6tables(
+static ssize_t nf_call_ip6tables_store(
 	struct device *d, struct device_attribute *attr, const char *buf,
 	size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_nf_call_ip6tables);
 }
-static DEVICE_ATTR(nf_call_ip6tables, S_IRUGO | S_IWUSR,
-		   show_nf_call_ip6tables, store_nf_call_ip6tables);
+static DEVICE_ATTR_RW(nf_call_ip6tables);
 
-static ssize_t show_nf_call_arptables(
+static ssize_t nf_call_arptables_show(
 	struct device *d, struct device_attribute *attr, char *buf)
 {
 	struct net_bridge *br = to_bridge(d);
@@ -707,17 +676,16 @@ static int set_nf_call_arptables(struct net_bridge *br, unsigned long val)
 	return 0;
 }
 
-static ssize_t store_nf_call_arptables(
+static ssize_t nf_call_arptables_store(
 	struct device *d, struct device_attribute *attr, const char *buf,
 	size_t len)
 {
 	return store_bridge_parm(d, buf, len, set_nf_call_arptables);
 }
-static DEVICE_ATTR(nf_call_arptables, S_IRUGO | S_IWUSR,
-		   show_nf_call_arptables, store_nf_call_arptables);
+static DEVICE_ATTR_RW(nf_call_arptables);
 #endif
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
-static ssize_t show_vlan_filtering(struct device *d,
+static ssize_t vlan_filtering_show(struct device *d,
 				   struct device_attribute *attr,
 				   char *buf)
 {
@@ -725,14 +693,13 @@ static ssize_t show_vlan_filtering(struct device *d,
 	return sprintf(buf, "%d\n", br->vlan_enabled);
 }
 
-static ssize_t store_vlan_filtering(struct device *d,
+static ssize_t vlan_filtering_store(struct device *d,
 				    struct device_attribute *attr,
 				    const char *buf, size_t len)
 {
 	return store_bridge_parm(d, buf, len, br_vlan_filter_toggle);
 }
-static DEVICE_ATTR(vlan_filtering, S_IRUGO | S_IWUSR,
-		   show_vlan_filtering, store_vlan_filtering);
+static DEVICE_ATTR_RW(vlan_filtering);
 #endif
 
 static struct attribute *bridge_attrs[] = {

^ permalink raw reply related

* Re: [PATCH v3 4/5] alx: add alx_get_stats64 operation
From: Ben Hutchings @ 2014-01-06 19:07 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: davem, johannes, stephen, netdev
In-Reply-To: <1389026023-18743-5-git-send-email-sd@queasysnail.net>

On Mon, 2014-01-06 at 17:33 +0100, Sabrina Dubroca wrote:
[...]
> --- a/drivers/net/ethernet/atheros/alx/main.c
> +++ b/drivers/net/ethernet/atheros/alx/main.c
> @@ -1166,10 +1166,54 @@ static void alx_poll_controller(struct net_device *netdev)
>  }
>  #endif
>  
> +static struct rtnl_link_stats64 *alx_get_stats64(struct net_device *dev,
> +					struct rtnl_link_stats64 *net_stats)
> +{
> +	struct alx_priv *alx = netdev_priv(dev);
> +	struct alx_hw_stats *hw_stats = &alx->hw.stats;
> +
> +	spin_lock(&alx->stats_lock);
> +
> +	alx_update_hw_stats(&alx->hw);
> +
> +	net_stats->tx_packets = hw_stats->tx_ok;

I think this should be set to hw_stats->tx_ok + net_stats->tx_errors
(after you set tx_errors).

> +	net_stats->tx_bytes   = hw_stats->tx_byte_cnt;
> +	net_stats->rx_packets = hw_stats->rx_ok;

Similarly, I think this should be hw_stats->rx_ok +
net_stats->rx_errors.

> +	net_stats->rx_bytes   = hw_stats->rx_byte_cnt;
> +	net_stats->multicast  = hw_stats->rx_mcast;
> +	net_stats->collisions = hw_stats->tx_single_col +
> +				hw_stats->tx_multi_col * 2 +

I would expect this to count the number of packets that had collisions
rather than total number of collisions (which you're only guessing at by
using * 2).

> +				hw_stats->tx_late_col + hw_stats->tx_abort_col;
> +
> +	net_stats->rx_errors  = hw_stats->rx_frag + hw_stats->rx_fcs_err +
> +				hw_stats->rx_len_err + hw_stats->rx_ov_sz +
> +				hw_stats->rx_ov_rrd + hw_stats->rx_align_err;
> +
> +	net_stats->rx_fifo_errors   = hw_stats->rx_ov_rxf;
> +	net_stats->rx_length_errors = hw_stats->rx_len_err;
> +	net_stats->rx_crc_errors    = hw_stats->rx_fcs_err;
> +	net_stats->rx_frame_errors  = hw_stats->rx_align_err;
> +	net_stats->rx_over_errors   = hw_stats->rx_ov_rrd + hw_stats->rx_ov_rxf;

rx_over_errors is commented as 'receiver ring buff overflow' and
ifconfig includes it in the 'frame' error count.  I think it is intended
to count frames which are too large for on-chip RX buffers and should
always be 0 for devices that do RX DMA.

Each error should contribute to at most one specific error stat, so
don't count rx_ov_rxf in both rx_fifo_errors and rx_over_errors.  I
would guess rx_fifo_errors is the right counter.

I don't know what rx_ov_rrd represents, but if it's the number of
packets dropped because the RX descriptor ring was empty then it should
be counted in rx_dropped not rx_over_errors.

> +	net_stats->rx_missed_errors = hw_stats->rx_ov_rrd + hw_stats->rx_ov_rxf;
[...]

This counter is commented as 'receiver missed packet'.  I think this
means the MAC detected SOF but was somehow too busy to receive the
packet, but I'm not sure.  I don't think these hardware counters match
the description, and certainly they shouldn't be counted here as well as
the other specific error stats.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH v3 5/5] alx: add stats to ethtool
From: Ben Hutchings @ 2014-01-06 19:08 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: davem, johannes, stephen, netdev
In-Reply-To: <1389026023-18743-6-git-send-email-sd@queasysnail.net>

On Mon, 2014-01-06 at 17:33 +0100, Sabrina Dubroca wrote:
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>

Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>

> ---
>  drivers/net/ethernet/atheros/alx/ethtool.c | 101 +++++++++++++++++++++++++++++
>  drivers/net/ethernet/atheros/alx/hw.h      |   7 +-
>  2 files changed, 107 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c b/drivers/net/ethernet/atheros/alx/ethtool.c
> index 45b3650..08e22df 100644
> --- a/drivers/net/ethernet/atheros/alx/ethtool.c
> +++ b/drivers/net/ethernet/atheros/alx/ethtool.c
> @@ -46,6 +46,66 @@
>  #include "reg.h"
>  #include "hw.h"
>  
> +/* The order of these strings must match the order of the fields in
> + * struct alx_hw_stats
> + * See hw.h
> + */
> +static const char alx_gstrings_stats[][ETH_GSTRING_LEN] = {
> +	"rx_packets",
> +	"rx_bcast_packets",
> +	"rx_mcast_packets",
> +	"rx_pause_packets",
> +	"rx_ctrl_packets",
> +	"rx_fcs_errors",
> +	"rx_length_errors",
> +	"rx_bytes",
> +	"rx_runt_packets",
> +	"rx_fragments",
> +	"rx_64B_or_less_packets",
> +	"rx_65B_to_127B_packets",
> +	"rx_128B_to_255B_packets",
> +	"rx_256B_to_511B_packets",
> +	"rx_512B_to_1023B_packets",
> +	"rx_1024B_to_1518B_packets",
> +	"rx_1519B_to_mtu_packets",
> +	"rx_oversize_packets",
> +	"rx_rxf_ov_drop_packets",
> +	"rx_rrd_ov_drop_packets",
> +	"rx_align_errors",
> +	"rx_bcast_bytes",
> +	"rx_mcast_bytes",
> +	"rx_address_errors",
> +	"tx_packets",
> +	"tx_bcast_packets",
> +	"tx_mcast_packets",
> +	"tx_pause_packets",
> +	"tx_exc_defer_packets",
> +	"tx_ctrl_packets",
> +	"tx_defer_packets",
> +	"tx_bytes",
> +	"tx_64B_or_less_packets",
> +	"tx_65B_to_127B_packets",
> +	"tx_128B_to_255B_packets",
> +	"tx_256B_to_511B_packets",
> +	"tx_512B_to_1023B_packets",
> +	"tx_1024B_to_1518B_packets",
> +	"tx_1519B_to_mtu_packets",
> +	"tx_single_collision",
> +	"tx_multiple_collisions",
> +	"tx_late_collision",
> +	"tx_abort_collision",
> +	"tx_underrun",
> +	"tx_trd_eop",
> +	"tx_length_errors",
> +	"tx_trunc_packets",
> +	"tx_bcast_bytes",
> +	"tx_mcast_bytes",
> +	"tx_update",
> +};
> +
> +#define ALX_NUM_STATS ARRAY_SIZE(alx_gstrings_stats)
> +
> +
>  static u32 alx_get_supported_speeds(struct alx_hw *hw)
>  {
>  	u32 supported = SUPPORTED_10baseT_Half |
> @@ -201,6 +261,44 @@ static void alx_set_msglevel(struct net_device *netdev, u32 data)
>  	alx->msg_enable = data;
>  }
>  
> +static void alx_get_ethtool_stats(struct net_device *netdev,
> +				  struct ethtool_stats *estats, u64 *data)
> +{
> +	struct alx_priv *alx = netdev_priv(netdev);
> +	struct alx_hw *hw = &alx->hw;
> +
> +	spin_lock(&alx->stats_lock);
> +
> +	alx_update_hw_stats(hw);
> +	BUILD_BUG_ON(sizeof(hw->stats) - offsetof(struct alx_hw_stats, rx_ok) <
> +		     ALX_NUM_STATS * sizeof(u64));
> +	memcpy(data, &hw->stats.rx_ok, ALX_NUM_STATS * sizeof(u64));
> +
> +	spin_unlock(&alx->stats_lock);
> +}
> +
> +static void alx_get_strings(struct net_device *netdev, u32 stringset, u8 *buf)
> +{
> +	switch (stringset) {
> +	case ETH_SS_STATS:
> +		memcpy(buf, &alx_gstrings_stats, sizeof(alx_gstrings_stats));
> +		break;
> +	default:
> +		WARN_ON(1);
> +		break;
> +	}
> +}
> +
> +static int alx_get_sset_count(struct net_device *netdev, int sset)
> +{
> +	switch (sset) {
> +	case ETH_SS_STATS:
> +		return ALX_NUM_STATS;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  const struct ethtool_ops alx_ethtool_ops = {
>  	.get_settings	= alx_get_settings,
>  	.set_settings	= alx_set_settings,
> @@ -209,4 +307,7 @@ const struct ethtool_ops alx_ethtool_ops = {
>  	.get_msglevel	= alx_get_msglevel,
>  	.set_msglevel	= alx_set_msglevel,
>  	.get_link	= ethtool_op_get_link,
> +	.get_strings	= alx_get_strings,
> +	.get_sset_count	= alx_get_sset_count,
> +	.get_ethtool_stats	= alx_get_ethtool_stats,
>  };
> diff --git a/drivers/net/ethernet/atheros/alx/hw.h b/drivers/net/ethernet/atheros/alx/hw.h
> index a97edfb..5978253 100644
> --- a/drivers/net/ethernet/atheros/alx/hw.h
> +++ b/drivers/net/ethernet/atheros/alx/hw.h
> @@ -381,7 +381,12 @@ struct alx_rrd {
>  				 ALX_ISR_RX_Q6 | \
>  				 ALX_ISR_RX_Q7)
>  
> -/* Statistics counters collected by the MAC */
> +/* Statistics counters collected by the MAC
> + *
> + * The order of the fields must match the strings in alx_gstrings_stats
> + * All stats fields should be u64
> + * See ethtool.c
> + */
>  struct alx_hw_stats {
>  	/* rx */
>  	u64 rx_ok;

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH net-next 1/2] bridge: use spin_lock_bh() in br_multicast_set_hash_max
From: Cong Wang @ 2014-01-06 19:11 UTC (permalink / raw)
  To: Scott Feldman; +Cc: stephen, netdev, roopa, bridge, curt, shm
In-Reply-To: <20140106190032.10912.1521.stgit@monster-03.cumulusnetworks.com>

On Mon, Jan 6, 2014 at 11:00 AM, Scott Feldman
<sfeldma@cumulusnetworks.com> wrote:
> From: Curt Brune <curt@cumulusnetworks.com>
>
> br_multicast_set_hash_max() is called from process context in
> net/bridge/br_sysfs_br.c by the sysfs store_hash_max() function.
>
> br_multicast_set_hash_max() calls spin_lock(&br->multicast_lock),
> which can deadlock the CPU if a softirq that also tries to take the
> same lock interrupts br_multicast_set_hash_max() while the lock is
> held .  This can happen quite easily when any of the bridge multicast
> timers expire, which try to take the same lock.
>
> The fix here is to use spin_lock_bh(), preventing other softirqs from
> executing on this CPU.
>
> Steps to reproduce:
>
> 1. Create a bridge with several interfaces (I used 4).
> 2. Set the "multicast query interval" to a low number, like 2.
> 3. Enable the bridge as a multicast querier.
> 4. Repeatedly set the bridge hash_max parameter via sysfs.
>
>   # brctl addbr br0
>   # brctl addif br0 eth1 eth2 eth3 eth4
>   # brctl setmcqi br0 2
>   # brctl setmcquerier br0 1
>
>   # while true ; do echo 4096 > /sys/class/net/br0/bridge/hash_max; done
>


I think this should probably go to net instead of net-next,
and -stable too.

^ permalink raw reply

* Re: [PATCH v3.5 18/19] rtlwifi: slight optimization of addr compare
From: John W. Linville @ 2014-01-06 19:53 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Ding Tianhong, Larry Finger, Chaoming Li, linux-wireless, Netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <52BC2576.3090601@cogentembedded.com>

On Thu, Dec 26, 2013 at 04:47:50PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 26-12-2013 15:41, Ding Tianhong wrote:
> 
> >Use possibly more efficient ether_addr_equal_unaligned
> >instead of memcmp.
> 
> >Cc: Larry Finger <Larry.Finger@lwfinger.net>
> >Cc: Chaoming Li <chaoming_li@realsil.com.cn>
> >Cc: John W. Linville <linville@tuxdriver.com>
> >Cc: linux-wireless@vger.kernel.org
> >Cc: netdev@vger.kernel.org
> >Cc: linux-kernel@vger.kernel.org
> >Signed-off-by: Weilong Chen <chenweilong@huawei.com>
> >Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> >---
> >  drivers/net/wireless/rtlwifi/cam.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> >diff --git a/drivers/net/wireless/rtlwifi/cam.c b/drivers/net/wireless/rtlwifi/cam.c
> >index 0e510f7..0276153 100644
> >--- a/drivers/net/wireless/rtlwifi/cam.c
> >+++ b/drivers/net/wireless/rtlwifi/cam.c
> [...]
> >@@ -335,7 +335,7 @@ void rtl_cam_del_entry(struct ieee80211_hw *hw, u8 *sta_addr)
> >  		addr = rtlpriv->sec.hwsec_cam_sta_addr[i];
> >  		bitmap = (rtlpriv->sec.hwsec_cam_bitmap) >> i;
> >  		if (((bitmap & BIT(0)) == BIT(0)) &&
> >-		    (memcmp(addr, sta_addr, ETH_ALEN) == 0)) {
> >+		    (ether_addr_equal_unaligned(addr, sta_addr))) {
> 
>    It's pointless to enclose function call in parens, again.

I'll correct that issue when merging the patch.

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [PATCH v2 00/16] wl1251 patches from linux-n900 tree
From: John W. Linville @ 2014-01-06 20:00 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Luciano Coelho, linux-wireless, netdev, linux-kernel,
	freemangordon, aaro.koskinen, pavel, sre, joni.lapilainen,
	Johannes Berg, Felipe Contreras
In-Reply-To: <201312311047.30147@pali>

On Tue, Dec 31, 2013 at 10:47:29AM +0100, Pali Rohár wrote:
> On Sunday 08 December 2013 10:24:58 Pali Rohár wrote:
> > Hello, I'm sending wl1251 patches from linux-n900 tree [1] for
> > comments. More patches come from David's monitor & packet
> > injection work. Patches are tested with 3.12 rc5 kernel on
> > Nokia N900.
> > 
> > Second version contains new patch for fixing NULL pointer
> > dereference which sometimes cause kernel panic and fixes code
> > suggested by Pavel Machek.
 
> So far, there are no new comments for patches 01, 03-13 and 16. 
> Are there any other problems with that patches or can be accepted 
> for upstream?
 
They are probably fine to merge now.  Of course, I was still deleting
wl12xx patches when they were originally posted, so I'll need someone
to resend the patches to me...

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply
From: David Miller @ 2014-01-06 20:11 UTC (permalink / raw)
  To: hannes; +Cc: fx.lebail, netdev, kuznet, jmorris, yoshfuji, kaber
In-Reply-To: <20140105230505.GA29910@order.stressinduktion.org>

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Mon, 6 Jan 2014 00:05:05 +0100

> On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
>> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
>> index 5d42009..65c8619 100644
>> --- a/net/ipv6/icmp.c
>> +++ b/net/ipv6/icmp.c
>> @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
>>  
>>  	saddr = &ipv6_hdr(skb)->daddr;
>>  
>> -	if (!ipv6_unicast_destination(skb))
>> +	if (!ipv6_unicast_destination(skb) &&
>> +	    !(net->ipv6.anycast_src_echo_reply &&
>> +	      ipv6_chk_acast_addr(net, NULL, saddr)))
>>  		saddr = NULL;
> 
> I am not sure why you left out the device at ipv6_chk_acast_addr?
> 
> IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
> scope global but need to check the interface for scope link.
> 
> It is already possible via setsockopt JOIN_ANYCAST that an ll address is
> anycast on another interface which may not be checked here.

I think we should pass a valid device in unless it breaks something
obvious.

Thanks.

^ permalink raw reply

* Re: [PATCH v5 net-next] net: pkt_sched: PIE AQM scheme
From: David Miller @ 2014-01-06 20:13 UTC (permalink / raw)
  To: subramanian.vijay; +Cc: netdev, shemminger, mysuryan, dave.taht
In-Reply-To: <1388885635-19939-1-git-send-email-subramanian.vijay@gmail.com>

From: Vijay Subramanian <subramanian.vijay@gmail.com>
Date: Sat,  4 Jan 2014 17:33:55 -0800

> From: Vijay Subramanian <vijaynsu@cisco.com>
> 
> Proportional Integral controller Enhanced (PIE) is a scheduler to address the
> bufferbloat problem.
 ...
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> Signed-off-by: Mythili Prabhu <mysuryan@cisco.com>

Applied, thanks.

^ permalink raw reply

* Re: [net-next 00/15][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2014-01-06 20:14 UTC (permalink / raw)
  To: David Miller, Sergei Shtylyov; +Cc: netdev, gospo, sassmann
In-Reply-To: <20140106.132628.418605275145035482.davem@davemloft.net>

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

On Mon, 2014-01-06 at 13:26 -0500, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Mon,  6 Jan 2014 04:30:21 -0800
> 
> > This series contains updates to i40e only.
>  ...
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
> 
> Pulled, please make sure the coding style feedback does actually
> get addressed.
> 
> Thanks Jeff.

Just to be clear, this coding style will be fixed in patch 13 of the
next series I am sending out.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: 答复: Re: [Patch] netxen_nic: Bugfix for wrong RUNNING status of NX3031 NICs with no link
From: Ben Hutchings @ 2014-01-06 20:24 UTC (permalink / raw)
  To: jiang.biao2
  Cc: David Miller, li.fengmao, manish.chopra, netdev, rajesh.borundia,
	sony.chacko, wang.liang82, cai.qu, long.chun
In-Reply-To: <OF9AC42061.4BF09F2E-ON48257C55.00253C5A-48257C55.0027274D@zte.com.cn>

On Fri, 2014-01-03 at 15:07 +0800, jiang.biao2@zte.com.cn wrote:
> >>> You cannot do this, because at the very exact moment you call
> >>> register_netdevice() the device can be brought up and once the
> >>> device is up the link can be brought up.
> 
> >>> Therefore if you invoke netif_carrier_off() right after
> >>> register_netdevice(), it can cancel out a legitimate
> >>> netif_carrier_on() call.
> 
> >> register_netdevice() should be call in probe(), at that time, is
> >> it impossible to bring up device because the driver loading has
> >> not finished?
> 
> > So far as the networking core is concerned, when register_netdevice() is
> > called the device is ready to use.  But you can do:
> 
> > rtnl_lock();
> > register_netdevice();
> > netif_carrier_off();
> > rtnl_unlock();
> 
> > However, as I said before, the current order in netxen_nic works and
> > only older kernel versions require you to call netif_carrier_off() after
> > registering the device.
> 
> >> Even that, the netif_carrier_off() will be called before
> >> netif_carrier_on(), and that will not be a problem.
> 
> >> Besides, the other net drivers(e.g igb, e1000, e1000e) I saw all
> >> call netif_carrier_off() after register_netdev(). There is no such
> >> problems in these drivers.
> 
> > It is not easy to hit the race condition but it is real.
> 
> But the problem I depicted really exists in the current netxen_nic driver,
> you can easily reproduce it following the reproduction steps.

'Current' as in Linux 3.12/3.13-rc7?

I don't know where I would find a Netxen card now.  So I can't test
netxen_nic myself, but I can assure you the sfc driver does things in
the same order and it works correctly.

> Can the following modification fix both the problem I faced and the race
> you worried?
> 
> netif_carrier_off();
> netdev->operstate=IF_OPER_DOWN;
> register_netdevice();

register_netdevice() calls linkwatch_init_dev() which sets the operstate
if necessary.  You should not set it directly.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH v2 00/16] wl1251 patches from linux-n900 tree
From: Johannes Berg @ 2014-01-06 20:26 UTC (permalink / raw)
  To: John W. Linville
  Cc: Pali Rohár, Luciano Coelho, linux-wireless, netdev,
	linux-kernel, freemangordon, aaro.koskinen, pavel, sre,
	joni.lapilainen, Felipe Contreras
In-Reply-To: <20140106200050.GG10885@tuxdriver.com>

On Mon, 2014-01-06 at 15:00 -0500, John W. Linville wrote:
> On Tue, Dec 31, 2013 at 10:47:29AM +0100, Pali Rohár wrote:
> > On Sunday 08 December 2013 10:24:58 Pali Rohár wrote:
> > > Hello, I'm sending wl1251 patches from linux-n900 tree [1] for
> > > comments. More patches come from David's monitor & packet
> > > injection work. Patches are tested with 3.12 rc5 kernel on
> > > Nokia N900.
> > > 
> > > Second version contains new patch for fixing NULL pointer
> > > dereference which sometimes cause kernel panic and fixes code
> > > suggested by Pavel Machek.
>  
> > So far, there are no new comments for patches 01, 03-13 and 16. 
> > Are there any other problems with that patches or can be accepted 
> > for upstream?
>  
> They are probably fine to merge now.  Of course, I was still deleting
> wl12xx patches when they were originally posted, so I'll need someone
> to resend the patches to me...

But please please don't CC half the world again!

johannes

^ permalink raw reply

* tx-nocache-copy performance
From: Benjamin Poirier @ 2014-01-06 20:27 UTC (permalink / raw)
  To: Tom Herbert; +Cc: netdev

Hi Tom,

In commit "c6e1a0d net: Allow no-cache copy from user on transmit
(v3.0-rc1)" you introduced the tx-nocache-copy performance optimization
and set it to on by default. I've tried to reproduce your testcase, as
well as a few more, but I did not find any performance improvement from
turning on tx-nocache-copy. Do you think tx-nocache-copy is still a
worthwhile optimization and it should remain on by default? In which
situations does it help?

I've ran latency tests similar to the ones you described in the commit
log. I've also tested how the option affects single stream throughput
tests. According to the results I obtained, it seems that
tx-nocache-copy has either no impact (in the latency test) or a negative
impact (in the throughput test).

My test results follow. I tested using 3.12.6 on one Intel Xeon W3565
and one i7 920 connected by ixgbe adapters. The results are from the
Xeon, but they're similar on the i7. All numbers report the mean±stddev
over 10 runs of 10s.

1) latency tests similar to what you described
There is no statistically significant difference between tx-nocache-copy
on/off.
nic irqs spread out (one queue per cpu)

200x netperf -r 1400,1
tx-nocache-copy off
        692000±1000 tps
        50/90/95/99% latency (us): 275±2/643.8±0.4/799±1/2474.4±0.3
tx-nocache-copy on
        693000±1000 tps
        50/90/95/99% latency (us): 274±1/644.1±0.7/800±2/2474.5±0.7

200x netperf -r 14000,14000
tx-nocache-copy off
        86450±80 tps
        50/90/95/99% latency (us): 334.37±0.02/838±1/2100±20/3990±40
tx-nocache-copy on
        86110±60 tps
        50/90/95/99% latency (us): 334.28±0.01/837±2/2110±20/3990±20

2) single stream throughput tests
tx-nocache-copy leads to higher service demand

                        throughput  cpu0        cpu1        demand
                        (Gb/s)      (Gcycle)    (Gcycle)    (cycle/B)

nic irqs and netperf on cpu0 (1x netperf -T0,0 -t omni -- -d send)

tx-nocache-copy off     9402±5      9.4±0.2                 0.80±0.01
tx-nocache-copy on      9403±3      9.85±0.04               0.838±0.004

nic irqs on cpu0, netperf on cpu1 (1x netperf -T1,1 -t omni -- -d send)

tx-nocache-copy off     9401±5      5.83±0.03   5.0±0.1     0.923±0.007
tx-nocache-copy on      9404±2      5.74±0.03   5.523±0.009 0.958±0.002

-Benjamin

^ permalink raw reply

* Re: inconsistency of ethtool feature names for get vs. set
From: Ben Hutchings @ 2014-01-06 20:39 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: Or Gerlitz, netdev
In-Reply-To: <CAJZOPZJZVH-ajrytOT9u70s4rK+n38ssDvEz9zEc0NQn2zywOQ@mail.gmail.com>

On Thu, 2014-01-02 at 22:12 +0200, Or Gerlitz wrote:
> On Thu, Jan 2, 2014 at 8:41 PM, Ben Hutchings <bhutchings@solarflare.com> wrote:
> 
> > Just to be clear, I do see that there's a problem here but the fix may
> > have to be mostly or entirely in documentation rather than code.
> 
> Ben, I am fine with at least starting with documenting this and see
> where it takes us, can you pick it up?

I might do but don't assume I will.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH net-next] net-gre-gro: Add GRE support to the GRO stack
From: David Miller @ 2014-01-06 20:43 UTC (permalink / raw)
  To: hkchu; +Cc: edumazet, herbert, ogerlitz, netdev
In-Reply-To: <1388896679-23152-1-git-send-email-hkchu@google.com>

From: "H.K. Jerry Chu" <hkchu@google.com>
Date: Sat,  4 Jan 2014 20:37:59 -0800

> This patch built on top of Commit 299603e8370a93dd5d8e8d800f0dff1ce2c53d36
> ("net-gro: Prepare GRO stack for the upcoming tunneling support") to add
> the support of the standard GRE (RFC1701/RFC2784/RFC2890) to the GRO
> stack. It also serves as an example for supporting other encapsulation
> protocols in the GRO stack in the future.
...

Applied, but you guys do realize:

> diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
> index 4b81e91..f8c49ce 100644
> --- a/net/ipv4/Makefile
> +++ b/net/ipv4/Makefile
> @@ -11,7 +11,7 @@ obj-y     := route.o inetpeer.o protocol.o \
>  	     tcp_offload.o datagram.o raw.o udp.o udplite.o \
>  	     udp_offload.o arp.o icmp.o devinet.o af_inet.o igmp.o \
>  	     fib_frontend.o fib_semantics.o fib_trie.o \
> -	     inet_fragment.o ping.o ip_tunnel_core.o
> +	     inet_fragment.o ping.o ip_tunnel_core.o gre_offload.o
>  
>  obj-$(CONFIG_NET_IP_TUNNEL) += ip_tunnel.o
>  obj-$(CONFIG_SYSCTL) += sysctl_net_ipv4.o
> @@ -19,7 +19,7 @@ obj-$(CONFIG_PROC_FS) += proc.o
>  obj-$(CONFIG_IP_MULTIPLE_TABLES) += fib_rules.o
>  obj-$(CONFIG_IP_MROUTE) += ipmr.o
>  obj-$(CONFIG_NET_IPIP) += ipip.o
> -gre-y := gre_demux.o gre_offload.o
> +gre-y := gre_demux.o
>  obj-$(CONFIG_NET_IPGRE_DEMUX) += gre.o
>  obj-$(CONFIG_NET_IPGRE) += ip_gre.o
>  obj-$(CONFIG_NET_IPVTI) += ip_vti.o

That the gre offload bits won't be registered unless GRE is enabled and
initialized/loaded, right?

^ permalink raw reply

* Re: [PATCH net 1/2] macvlan: forbid L2 fowarding offload for macvtap
From: David Miller @ 2014-01-06 20:47 UTC (permalink / raw)
  To: jasowang; +Cc: netdev, linux-kernel, mst, john.r.fastabend, nhorman
In-Reply-To: <1388978467-2075-1-git-send-email-jasowang@redhat.com>

From: Jason Wang <jasowang@redhat.com>
Date: Mon,  6 Jan 2014 11:21:06 +0800

> L2 fowarding offload will bypass the rx handler of real device. This will make
> the packet could not be forwarded to macvtap device. Another problem is the
> dev_hard_start_xmit() called for macvtap does not have any synchronization.
> 
> Fix this by forbidding L2 forwarding for macvtap.
> 
> Cc: John Fastabend <john.r.fastabend@intel.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

I think I agree with Neil that the rx_handler change might be the best
way to fix this.  That change seems to have a lot of nice unintended
side effects, no?

^ permalink raw reply

* Re: [PATCH v3] isdn: Drop big endian cpp checks from telespci and hfc_pci drivers
From: David Miller @ 2014-01-06 20:51 UTC (permalink / raw)
  To: linux; +Cc: isdn, netdev, linux-kernel
In-Reply-To: <1388982699-11514-1-git-send-email-linux@roeck-us.net>

From: Guenter Roeck <linux@roeck-us.net>
Date: Sun,  5 Jan 2014 20:31:39 -0800

> With arm:allmodconfig, building the Teles PCI driver fails with
> 
> telespci.c:294:2: error: #error "not running on big endian machines now"
> 
> Similar, building the driver for HFC PCI-Bus cards fails with
> 
> hfc_pci.c:1647:2: error: #error "not running on big endian machines now"
> 
> Remove the big endian cpp check from both drivers to fix the build errors.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH v3.5 18/19] rtlwifi: slight optimization of addr compare
From: John W. Linville @ 2014-01-06 20:51 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Ding Tianhong, Larry Finger, Chaoming Li, linux-wireless, Netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <20140106195302.GF10885@tuxdriver.com>

On Mon, Jan 06, 2014 at 02:53:02PM -0500, John W. Linville wrote:
> On Thu, Dec 26, 2013 at 04:47:50PM +0400, Sergei Shtylyov wrote:
> > Hello.
> > 
> > On 26-12-2013 15:41, Ding Tianhong wrote:
> > 
> > >Use possibly more efficient ether_addr_equal_unaligned
> > >instead of memcmp.
> > 
> > >Cc: Larry Finger <Larry.Finger@lwfinger.net>
> > >Cc: Chaoming Li <chaoming_li@realsil.com.cn>
> > >Cc: John W. Linville <linville@tuxdriver.com>
> > >Cc: linux-wireless@vger.kernel.org
> > >Cc: netdev@vger.kernel.org
> > >Cc: linux-kernel@vger.kernel.org
> > >Signed-off-by: Weilong Chen <chenweilong@huawei.com>
> > >Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> > >---
> > >  drivers/net/wireless/rtlwifi/cam.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > >diff --git a/drivers/net/wireless/rtlwifi/cam.c b/drivers/net/wireless/rtlwifi/cam.c
> > >index 0e510f7..0276153 100644
> > >--- a/drivers/net/wireless/rtlwifi/cam.c
> > >+++ b/drivers/net/wireless/rtlwifi/cam.c
> > [...]
> > >@@ -335,7 +335,7 @@ void rtl_cam_del_entry(struct ieee80211_hw *hw, u8 *sta_addr)
> > >  		addr = rtlpriv->sec.hwsec_cam_sta_addr[i];
> > >  		bitmap = (rtlpriv->sec.hwsec_cam_bitmap) >> i;
> > >  		if (((bitmap & BIT(0)) == BIT(0)) &&
> > >-		    (memcmp(addr, sta_addr, ETH_ALEN) == 0)) {
> > >+		    (ether_addr_equal_unaligned(addr, sta_addr))) {
> > 
> >    It's pointless to enclose function call in parens, again.
> 
> I'll correct that issue when merging the patch.

Well, that was the plan...  But since ether_addr_equal_unaligned
isn't in the wireless trees at the moment, it will have to wait or go
through another tree that has that definition.  Feel free to respin
with the extra parenthesis removed...

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [PATCH] netfilter: nf_conntrack: release conntrack from rcu callback
From: Andrew Vagin @ 2014-01-06 20:54 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Andrey Vagin, netfilter-devel, netfilter, coreteam, netdev,
	linux-kernel, vvs, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik, David S. Miller, Cyrill Gorcunov
In-Reply-To: <20140106170235.GJ28854@breakpoint.cc>

On Mon, Jan 06, 2014 at 06:02:35PM +0100, Florian Westphal wrote:
> Andrey Vagin <avagin@openvz.org> wrote:
> > Lets look at destroy_conntrack:
> > 
> > hlist_nulls_del_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode);
> > ...
> > nf_conntrack_free(ct)
> > 	kmem_cache_free(net->ct.nf_conntrack_cachep, ct);
> > 
> > The hash is protected by rcu, so readers look up conntracks without
> > locks.
> > A conntrack is removed from the hash, but in this moment a few readers
> > still can use the conntrack, so if we call kmem_cache_free now, all
> > readers will read released object.
> > 
> > Bellow you can find more tricky race condition of three tasks.
> > 
> > task 1			task 2			task 3
> > 			nf_conntrack_find_get
> > 			 ____nf_conntrack_find
> > destroy_conntrack
> >  hlist_nulls_del_rcu
> >  nf_conntrack_free
> >  kmem_cache_free
> > 						__nf_conntrack_alloc
> > 						 kmem_cache_alloc
> > 						 memset(&ct->tuplehash[IP_CT_DIR_MAX],
> > 			 if (nf_ct_is_dying(ct))
> > 
> > In this case the task 2 will not understand, that it uses a wrong
> > conntrack.
> 
> Can you elaborate?
> Yes, nf_ct_is_dying(ct) might be called for the wrong conntrack.
> 
> But, in case we _think_ that its the right one we call
> nf_ct_tuple_equal() to verify we indeed found the right one:

Ok. task3 creates a new contrack and nf_ct_tuple_equal() returns true on
it. Looks like it's possible. In this case we have two threads with one
unitialized contrack. It's really bad, because the code supposes that
conntrack can not be initialized in two threads concurrently. For
example BUG can be triggered from nf_nat_setup_info():

BUG_ON(nf_nat_initialized(ct, maniptype));


> 
>        h = ____nf_conntrack_find(net, zone, tuple, hash);
>        if (h) { // might be released right now, but page won't go away (SLAB_BY_RCU)

I did not take SLAB_BY_RCU into account. Thank you. But it doesn't say,
that we don't have the race condition here. It explains why we don't
have really bad situations, when a completely wrong contract is
used. We always use a "right" conntrack, but sometimes it is
uninitialized and here is a problem.

The race window is tiny, because usually we check that conntrack is not
initialized and only then we execute its initialization. We don't hold
any locks in these moments.

Task2					| Task3
if (!nf_nat_initialized(ct))		|
					| if (!nf_nat_initialized(ct)
 alloc_null_binding			|
					|  alloc_null_binding
  nf_nat_setup_info			|
   ct->status |= IPS_SRC_NAT_DONE	|
					|   nf_nat_setup_info
					|    BUG_ON(nf_nat_initialized(ct));

>                 ct = nf_ct_tuplehash_to_ctrack(h);
>                 if (unlikely(nf_ct_is_dying(ct) ||
>                              !atomic_inc_not_zero(&ct->ct_general.use)))
> 			// which means we should hit this path (0 ref).
>                         h = NULL;
>                 else {
> 			// otherwise, it cannot go away from under us, since
> 			// we own a reference now.
>                         if (unlikely(!nf_ct_tuple_equal(tuple, &h->tuple) ||
>                                      nf_ct_zone(ct) != zone)) {
> 			// if we get here, the entry got recycled on other cpu
> 			// for a different tuple, we can bail out and drop
> 			// the reference safely and re-try the lookup
>                                 nf_ct_put(ct);
>                                 goto begin;
>                         }
>                 }

Thanks,
Andrey

^ permalink raw reply

* Re: tx-nocache-copy performance
From: Eric Dumazet @ 2014-01-06 20:57 UTC (permalink / raw)
  To: Benjamin Poirier; +Cc: Tom Herbert, netdev
In-Reply-To: <20140106202754.GA5877@d2.synalogic.ca>

On Mon, 2014-01-06 at 15:27 -0500, Benjamin Poirier wrote:
> Hi Tom,
> 
> In commit "c6e1a0d net: Allow no-cache copy from user on transmit
> (v3.0-rc1)" you introduced the tx-nocache-copy performance optimization
> and set it to on by default. I've tried to reproduce your testcase, as
> well as a few more, but I did not find any performance improvement from
> turning on tx-nocache-copy. Do you think tx-nocache-copy is still a
> worthwhile optimization and it should remain on by default? In which
> situations does it help?
> 
> I've ran latency tests similar to the ones you described in the commit
> log. I've also tested how the option affects single stream throughput
> tests. According to the results I obtained, it seems that
> tx-nocache-copy has either no impact (in the latency test) or a negative
> impact (in the throughput test).
> 
> My test results follow. I tested using 3.12.6 on one Intel Xeon W3565
> and one i7 920 connected by ixgbe adapters. The results are from the
> Xeon, but they're similar on the i7. All numbers report the mean±stddev
> over 10 runs of 10s.
> 
> 1) latency tests similar to what you described
> There is no statistically significant difference between tx-nocache-copy
> on/off.
> nic irqs spread out (one queue per cpu)
> 
> 200x netperf -r 1400,1
> tx-nocache-copy off
>         692000±1000 tps
>         50/90/95/99% latency (us): 275±2/643.8±0.4/799±1/2474.4±0.3
> tx-nocache-copy on
>         693000±1000 tps
>         50/90/95/99% latency (us): 274±1/644.1±0.7/800±2/2474.5±0.7
> 
> 200x netperf -r 14000,14000
> tx-nocache-copy off
>         86450±80 tps
>         50/90/95/99% latency (us): 334.37±0.02/838±1/2100±20/3990±40
> tx-nocache-copy on
>         86110±60 tps
>         50/90/95/99% latency (us): 334.28±0.01/837±2/2110±20/3990±20
> 
> 2) single stream throughput tests
> tx-nocache-copy leads to higher service demand
> 
>                         throughput  cpu0        cpu1        demand
>                         (Gb/s)      (Gcycle)    (Gcycle)    (cycle/B)
> 
> nic irqs and netperf on cpu0 (1x netperf -T0,0 -t omni -- -d send)
> 
> tx-nocache-copy off     9402±5      9.4±0.2                 0.80±0.01
> tx-nocache-copy on      9403±3      9.85±0.04               0.838±0.004
> 
> nic irqs on cpu0, netperf on cpu1 (1x netperf -T1,1 -t omni -- -d send)
> 
> tx-nocache-copy off     9401±5      5.83±0.03   5.0±0.1     0.923±0.007
> tx-nocache-copy on      9404±2      5.74±0.03   5.523±0.009 0.958±0.002
> 
> -Benjamin

We raised this point last year, for same reasons.

http://www.spinics.net/lists/netdev/msg226501.html

I agree we should revert the default.

I'll post new perf data using latest net-next kernel and a bnx2x NIC.

^ permalink raw reply

* Re: tx-nocache-copy performance
From: Tom Herbert @ 2014-01-06 20:59 UTC (permalink / raw)
  To: Benjamin Poirier; +Cc: Linux Netdev List
In-Reply-To: <20140106202754.GA5877@d2.synalogic.ca>

On Mon, Jan 6, 2014 at 12:27 PM, Benjamin Poirier <bpoirier@suse.de> wrote:
> Hi Tom,
>
> In commit "c6e1a0d net: Allow no-cache copy from user on transmit
> (v3.0-rc1)" you introduced the tx-nocache-copy performance optimization
> and set it to on by default. I've tried to reproduce your testcase, as
> well as a few more, but I did not find any performance improvement from
> turning on tx-nocache-copy. Do you think tx-nocache-copy is still a
> worthwhile optimization and it should remain on by default? In which
> situations does it help?
>
Unfortunately, I think this is probably not a worthwhile optimization
at this point. The benefits should manifest themselves under high
networking load and high CPU load where we are getting a lot of
pressure on the cache, the non-temporal copy should alleviate that
case. In reality, I suspect that rep movsq is more efficient that
movntq's so the advantages of skipping the cache might be wiped out.
It would be nice if Intel had a movntsq instruction!

btw, I still believe it would be a win if we could use vmsplice to
mitigate the copy altogether, unfortunately no one has yet to come up
with an interface to reliably reclaim buffers :-(.

> I've ran latency tests similar to the ones you described in the commit
> log. I've also tested how the option affects single stream throughput
> tests. According to the results I obtained, it seems that
> tx-nocache-copy has either no impact (in the latency test) or a negative
> impact (in the throughput test).
>
> My test results follow. I tested using 3.12.6 on one Intel Xeon W3565
> and one i7 920 connected by ixgbe adapters. The results are from the
> Xeon, but they're similar on the i7. All numbers report the meanąstddev
> over 10 runs of 10s.
>
> 1) latency tests similar to what you described
> There is no statistically significant difference between tx-nocache-copy
> on/off.
> nic irqs spread out (one queue per cpu)
>
> 200x netperf -r 1400,1
> tx-nocache-copy off
>         692000ą1000 tps
>         50/90/95/99% latency (us): 275ą2/643.8ą0.4/799ą1/2474.4ą0.3
> tx-nocache-copy on
>         693000ą1000 tps
>         50/90/95/99% latency (us): 274ą1/644.1ą0.7/800ą2/2474.5ą0.7
>
> 200x netperf -r 14000,14000
> tx-nocache-copy off
>         86450ą80 tps
>         50/90/95/99% latency (us): 334.37ą0.02/838ą1/2100ą20/3990ą40
> tx-nocache-copy on
>         86110ą60 tps
>         50/90/95/99% latency (us): 334.28ą0.01/837ą2/2110ą20/3990ą20
>
> 2) single stream throughput tests
> tx-nocache-copy leads to higher service demand
>
>                         throughput  cpu0        cpu1        demand
>                         (Gb/s)      (Gcycle)    (Gcycle)    (cycle/B)
>
> nic irqs and netperf on cpu0 (1x netperf -T0,0 -t omni -- -d send)
>
> tx-nocache-copy off     9402ą5      9.4ą0.2                 0.80ą0.01
> tx-nocache-copy on      9403ą3      9.85ą0.04               0.838ą0.004
>
> nic irqs on cpu0, netperf on cpu1 (1x netperf -T1,1 -t omni -- -d send)
>
> tx-nocache-copy off     9401ą5      5.83ą0.03   5.0ą0.1     0.923ą0.007
> tx-nocache-copy on      9404ą2      5.74ą0.03   5.523ą0.009 0.958ą0.002
>
> -Benjamin

^ permalink raw reply

* Re: [PATCH net-next v3 1/3] ipv4: introduce ip_dst_mtu_secure and protect forwarding path against pmtu spoofing
From: David Miller @ 2014-01-06 21:08 UTC (permalink / raw)
  To: hannes; +Cc: netdev, eric.dumazet, johnwheffner, steffen.klassert, fweimer
In-Reply-To: <20140106084827.GA5766@order.stressinduktion.org>

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Mon, 6 Jan 2014 09:48:27 +0100

> I want to put these patches up for discussion again. I did more testing
> to them (xfrm, tunnel), did some rewording and rewrote the IPv6 patch.

I'm still thinking about this series.

I would have been nice if you provided a "0/3" introductory posting
discussing your point of view, and giving a more detailed analysis of
your view of the situation so that John and others have something to
reply to.

Thanks.

^ permalink raw reply

* Re: [PATCH 3/4] tcp: metrics: Delete all entries matching a certain destination
From: David Miller @ 2014-01-06 21:10 UTC (permalink / raw)
  To: christoph.paasch; +Cc: netdev, eric.dumazet, ja
In-Reply-To: <20140102091823.GA31178@cpaasch-mac>

From: Christoph Paasch <christoph.paasch@uclouvain.be>
Date: Thu, 2 Jan 2014 10:18:23 +0100

> Do you mean that if no source-IP is given in the netlink command that all
> entries matching the dst should be deleted or rather only one of them (and that it
> would be non-deterministic which one)?

You would delete all of them, it's the only way to stay compatible with
the current code.

> Because, if I delete all of them, then "ip tcp_metrics flush PREFIX" of
> today's iproute2 will complain, because iproute2 expects that for each entry of
> "ip tcp_metrics show" a delete-call must be done.
> 
> But, the non-deterministic case also feels a bit odd to me.

iproute2 should not expect that if it doesn't specify a specific
source address, in fact it doesn't even know how to currently.  I
can't think of any other reasonable behavior.

^ 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