Netdev List
 help / color / mirror / Atom feed
* [PATCH v4 net-next 06/27] bonding: modify bond_get_slave_by_dev() to use neighbours
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
  To: netdev; +Cc: jiri, Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>

It should be used under rtnl/bonding lock, so use the non-RCU version.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---

Notes:
    v3  -> v4:
    No change.
    
    v2  -> v3:
    No change.
    
    v1  -> v2:
    No changes.
    
    RFC -> v1:
    Move the patch right after we've populated ->private with bonding.

 drivers/net/bonding/bonding.h | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 03cf3fd..597a4ec 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -276,13 +276,8 @@ struct bonding {
 static inline struct slave *bond_get_slave_by_dev(struct bonding *bond,
 						  struct net_device *slave_dev)
 {
-	struct slave *slave = NULL;
-
-	bond_for_each_slave(bond, slave)
-		if (slave->dev == slave_dev)
-			return slave;
-
-	return NULL;
+	return (struct slave *)netdev_lower_dev_get_private(bond->dev,
+							    slave_dev);
 }
 
 static inline struct bonding *bond_get_bond_by_slave(struct slave *slave)
-- 
1.8.4

^ permalink raw reply related

* [PATCH v4 net-next 04/27] net: add netdev_adjacent->private and allow to use it
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
  To: netdev
  Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
	Alexander Duyck
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>

Currently, even though we can access any linked device, we can't attach
anything to it, which is vital to properly manage them.

To fix this, add a new void *private to netdev_adjacent and functions
setting/getting it (per link), so that we can save, per example, bonding's
slave structures there, per slave device.

netdev_master_upper_dev_link_private(dev, upper_dev, private) links dev to
upper dev and populates the neighbour link only with private.

netdev_lower_dev_get_private{,_rcu}() returns the private, if found.

CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---

Notes:
    v3  -> v4:
    No change.
    
    v2  -> v3:
    No change.
    
    v1  -> v2:
    No changes.
    
    RFC -> v1:
    rename neighbour_dev_list/all_dev_list to adj_list/all_adj_list and rework
    the naming of functions - to make the use the pattern
    netdev_(all_)(lower|upper)_* .

 include/linux/netdevice.h |  7 +++++
 net/core/dev.c            | 68 +++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 514045c..75d5bea 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2839,8 +2839,15 @@ extern int netdev_upper_dev_link(struct net_device *dev,
 				 struct net_device *upper_dev);
 extern int netdev_master_upper_dev_link(struct net_device *dev,
 					struct net_device *upper_dev);
+extern int netdev_master_upper_dev_link_private(struct net_device *dev,
+						struct net_device *upper_dev,
+						void *private);
 extern void netdev_upper_dev_unlink(struct net_device *dev,
 				    struct net_device *upper_dev);
+extern void *netdev_lower_dev_get_private_rcu(struct net_device *dev,
+					      struct net_device *lower_dev);
+extern void *netdev_lower_dev_get_private(struct net_device *dev,
+					  struct net_device *lower_dev);
 extern int skb_checksum_help(struct sk_buff *skb);
 extern struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
 	netdev_features_t features, bool tx_path);
diff --git a/net/core/dev.c b/net/core/dev.c
index 9290f09..c69ab74 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4376,6 +4376,9 @@ struct netdev_adjacent {
 	/* counter for the number of times this device was added to us */
 	u16 ref_nr;
 
+	/* private field for the users */
+	void *private;
+
 	struct list_head list;
 	struct rcu_head rcu;
 };
@@ -4510,7 +4513,7 @@ EXPORT_SYMBOL(netdev_master_upper_dev_get_rcu);
 static int __netdev_adjacent_dev_insert(struct net_device *dev,
 					struct net_device *adj_dev,
 					struct list_head *dev_list,
-					bool master)
+					void *private, bool master)
 {
 	struct netdev_adjacent *adj;
 
@@ -4528,6 +4531,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
 	adj->dev = adj_dev;
 	adj->master = master;
 	adj->ref_nr = 1;
+	adj->private = private;
 	dev_hold(adj_dev);
 
 	pr_debug("dev_hold for %s, because of link added from %s to %s\n",
@@ -4574,15 +4578,17 @@ int __netdev_adjacent_dev_link_lists(struct net_device *dev,
 				     struct net_device *upper_dev,
 				     struct list_head *up_list,
 				     struct list_head *down_list,
-				     bool master)
+				     void *private, bool master)
 {
 	int ret;
 
-	ret = __netdev_adjacent_dev_insert(dev, upper_dev, up_list, master);
+	ret = __netdev_adjacent_dev_insert(dev, upper_dev, up_list, private,
+					   master);
 	if (ret)
 		return ret;
 
-	ret = __netdev_adjacent_dev_insert(upper_dev, dev, down_list, false);
+	ret = __netdev_adjacent_dev_insert(upper_dev, dev, down_list, private,
+					   false);
 	if (ret) {
 		__netdev_adjacent_dev_remove(dev, upper_dev, up_list);
 		return ret;
@@ -4597,7 +4603,7 @@ int __netdev_adjacent_dev_link(struct net_device *dev,
 	return __netdev_adjacent_dev_link_lists(dev, upper_dev,
 						&dev->all_adj_list.upper,
 						&upper_dev->all_adj_list.lower,
-						false);
+						NULL, false);
 }
 
 void __netdev_adjacent_dev_unlink_lists(struct net_device *dev,
@@ -4619,7 +4625,7 @@ void __netdev_adjacent_dev_unlink(struct net_device *dev,
 
 int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
 					 struct net_device *upper_dev,
-					 bool master)
+					 void *private, bool master)
 {
 	int ret = __netdev_adjacent_dev_link(dev, upper_dev);
 
@@ -4629,7 +4635,7 @@ int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
 	ret = __netdev_adjacent_dev_link_lists(dev, upper_dev,
 					       &dev->adj_list.upper,
 					       &upper_dev->adj_list.lower,
-					       master);
+					       private, master);
 	if (ret) {
 		__netdev_adjacent_dev_unlink(dev, upper_dev);
 		return ret;
@@ -4648,7 +4654,8 @@ void __netdev_adjacent_dev_unlink_neighbour(struct net_device *dev,
 }
 
 static int __netdev_upper_dev_link(struct net_device *dev,
-				   struct net_device *upper_dev, bool master)
+				   struct net_device *upper_dev, bool master,
+				   void *private)
 {
 	struct netdev_adjacent *i, *j, *to_i, *to_j;
 	int ret = 0;
@@ -4668,7 +4675,8 @@ static int __netdev_upper_dev_link(struct net_device *dev,
 	if (master && netdev_master_upper_dev_get(dev))
 		return -EBUSY;
 
-	ret = __netdev_adjacent_dev_link_neighbour(dev, upper_dev, master);
+	ret = __netdev_adjacent_dev_link_neighbour(dev, upper_dev, private,
+						   master);
 	if (ret)
 		return ret;
 
@@ -4759,7 +4767,7 @@ rollback_mesh:
 int netdev_upper_dev_link(struct net_device *dev,
 			  struct net_device *upper_dev)
 {
-	return __netdev_upper_dev_link(dev, upper_dev, false);
+	return __netdev_upper_dev_link(dev, upper_dev, false, NULL);
 }
 EXPORT_SYMBOL(netdev_upper_dev_link);
 
@@ -4777,10 +4785,18 @@ EXPORT_SYMBOL(netdev_upper_dev_link);
 int netdev_master_upper_dev_link(struct net_device *dev,
 				 struct net_device *upper_dev)
 {
-	return __netdev_upper_dev_link(dev, upper_dev, true);
+	return __netdev_upper_dev_link(dev, upper_dev, true, NULL);
 }
 EXPORT_SYMBOL(netdev_master_upper_dev_link);
 
+int netdev_master_upper_dev_link_private(struct net_device *dev,
+					 struct net_device *upper_dev,
+					 void *private)
+{
+	return __netdev_upper_dev_link(dev, upper_dev, true, private);
+}
+EXPORT_SYMBOL(netdev_master_upper_dev_link_private);
+
 /**
  * netdev_upper_dev_unlink - Removes a link to upper device
  * @dev: device
@@ -4818,6 +4834,36 @@ void netdev_upper_dev_unlink(struct net_device *dev,
 }
 EXPORT_SYMBOL(netdev_upper_dev_unlink);
 
+void *netdev_lower_dev_get_private_rcu(struct net_device *dev,
+				       struct net_device *lower_dev)
+{
+	struct netdev_adjacent *lower;
+
+	if (!lower_dev)
+		return NULL;
+	lower = __netdev_find_adj_rcu(dev, lower_dev, &dev->adj_list.lower);
+	if (!lower)
+		return NULL;
+
+	return lower->private;
+}
+EXPORT_SYMBOL(netdev_lower_dev_get_private_rcu);
+
+void *netdev_lower_dev_get_private(struct net_device *dev,
+				   struct net_device *lower_dev)
+{
+	struct netdev_adjacent *lower;
+
+	if (!lower_dev)
+		return NULL;
+	lower = __netdev_find_adj(dev, lower_dev, &dev->adj_list.lower);
+	if (!lower)
+		return NULL;
+
+	return lower->private;
+}
+EXPORT_SYMBOL(netdev_lower_dev_get_private);
+
 static void dev_change_rx_flags(struct net_device *dev, int flags)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
-- 
1.8.4

^ permalink raw reply related

* [PATCH v4 net-next 00/27] bonding: use neighbours instead of own lists
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
  To: netdev
  Cc: jiri, Jay Vosburgh, Andy Gospodarek, Dimitris Michailidis,
	David S. Miller, Patrick McHardy, Eric Dumazet, Alexander Duyck,
	Veaceslav Falico

This patchset introduces all the needed infrastructure, on top of current
adjacent lists, to be able to remove bond's slave_list/slave->list. The
overhead in memory/CPU is minimal, and after the patchset bonding can rely
on its slave-related functions, given the proper locking. I've done some
netperf benchmarks on a vm, and the delta was about 0.1gbps for 35gbps as a
whole, so no speed fluctuations.

It also automatically creates lower/upper and master symlinks in dev's
sysfs directory.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Dimitris Michailidis <dm@chelsio.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Patrick McHardy <kaber@trash.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>

---
v3  -> v4:
Per the feedback received, fix the comments to be kernel-doc style.
As Ben Hutchings suggested, make the 'get next private after a
specific private' function bonding-specific - it has virtually no uses
outside bonding (and even in bonding I already have some patches of
removing it).
Also, remove the stupid 'bool upper' logic from everywhere in the logic -
as Ben suggested, it's easier to pass the lists directly - and not only
easier, it saved several hundred lines of code modified.
Finally, make netdev_for_each_lower_private() safe for removal of the
current member - it can be easily achieved and doesn't really need two
versions, as in lists.

v2  -> v3:
Fix the commit subject for patch 08 - forgot the _continue in
bond_for_each_slave_continue_reverse(), suggested by Cong Wang.
This version is already ready for the inclusion, hopefully.
Moved the changelog in cover letter to the bottom and cleaned it
up a bit.

v1  -> v2:
Fix a bug when we could remove try to remove a neighbour twice - it was
possible if we've had this neighbour already and added another devices
which had this neighbour in its lists. Fix it by following the rule - there
can be only one neighbour link, so add also _remove_neighbour() functions
to the generic _remove()s to handle the neighbour only removing, and fix
its refcounting while adding (all in patch 01).
Also, fix a vlan bug when it called unregister netdevice before unlinking -
it could possibly lead to a subte race condition - cause it was
unregistered but still in global-accessible list.
Rearrange sysfs/vlan patches so that first we fix vlan and only after we
add sysfs stuff.

RFC -> v1:
I've added proper, consistent naming for all variables/functions, uninlined
some helpers to get better backtraces, just in case (overhead is minimal,
no hot paths), rearranged patches for better review, dropped bondings
->prev and bond_for_each_slave_continue() functionality - to be able to
RCUify it easier, and renamed slave_* sysfs links to lower_* sysfs links to
maintain upper/lower naming. I've also dropped, thanks to bonding cleanup,
some heavy and ugly/intrusive patches.

---
 drivers/net/bonding/bond_3ad.c                  |  54 ++--
 drivers/net/bonding/bond_alb.c                  |  81 ++---
 drivers/net/bonding/bond_alb.h                  |   4 +-
 drivers/net/bonding/bond_main.c                 | 296 +++++++++--------
 drivers/net/bonding/bond_procfs.c               |   5 +-
 drivers/net/bonding/bond_sysfs.c                |  62 +---
 drivers/net/bonding/bonding.h                   | 101 +++---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c |   3 +-
 include/linux/netdevice.h                       |  55 +++-
 net/8021q/vlan.c                                |  18 +-
 net/core/dev.c                                  | 403 +++++++++++++++++-------
 11 files changed, 638 insertions(+), 444 deletions(-)

^ permalink raw reply

* [PATCH v4 net-next 03/27] net: add RCU variant to search for netdev_adjacent link
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
  To: netdev
  Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
	Alexander Duyck, Cong Wang
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>

Currently we have only the RTNL flavour, however we can traverse it while
holding only RCU, so add the RCU search. Add an RCU variant that uses
list_head * as an argument, so that it can be universally used afterwards.

CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Cong Wang <amwang@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---

Notes:
    v3  -> v4:
    Use the list argument, instead of bool upper.
    
    v2  -> v3:
    No change.
    
    v1  -> v2:
    No changes.
    
    RFC -> v1:
    rename neighbour_dev_list/all_dev_list to adj_list/all_adj_list and rework
    the naming of functions - to make the use the pattern
    netdev_(all_)(lower|upper)_* . Uninline functions to get better stack
    traces - inline doesn't give much speed improvement, but this way the stack
    traces are meaningful. Remove the unused (through the whole patchset)
    functions - they can easily be added afterwards.

 net/core/dev.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 9a395e0..9290f09 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4380,6 +4380,19 @@ struct netdev_adjacent {
 	struct rcu_head rcu;
 };
 
+static struct netdev_adjacent *__netdev_find_adj_rcu(struct net_device *dev,
+						     struct net_device *adj_dev,
+						     struct list_head *adj_list)
+{
+	struct netdev_adjacent *adj;
+
+	list_for_each_entry_rcu(adj, adj_list, list) {
+		if (adj->dev == adj_dev)
+			return adj;
+	}
+	return NULL;
+}
+
 static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
 						 struct net_device *adj_dev,
 						 struct list_head *adj_list)
-- 
1.8.4

^ permalink raw reply related

* [PATCH v4 net-next 02/27] net: add adj_list to save only neighbours
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
  To: netdev
  Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
	Alexander Duyck, Cong Wang
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>

Currently, we distinguish neighbours (first-level linked devices) from
non-neighbours by the neighbour bool in the netdev_adjacent. This could be
quite time-consuming in case we would like to traverse *only* through
neighbours - cause we'd have to traverse through all devices and check for
this flag, and in a (quite common) scenario where we have lots of vlans on
top of bridge, which is on top of a bond - the bonding would have to go
through all those vlans to get its upper neighbour linked devices.

This situation is really unpleasant, cause there are already a lot of cases
when a device with slaves needs to go through them in hot path.

To fix this, introduce a new upper/lower device lists structure -
adj_list, which contains only the neighbours. It works always in
pair with the all_adj_list structure (renamed from upper/lower_dev_list),
i.e. both of them contain the same links, only that all_adj_list contains
also non-neighbour device links. It's really a small change visible,
currently, only for __netdev_adjacent_dev_insert/remove(), and doesn't
change the main linked logic at all.

Also, add some comments a fix a name collision in
netdev_for_each_upper_dev_rcu() and rework the naming by the following
rules:

netdev_(all_)(upper|lower)_*

If "all_" is present, then we work with the whole list of upper/lower
devices, otherwise - only with direct neighbours. Uninline functions - to
get better stack traces.

CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Cong Wang <amwang@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---

Notes:
    v3  -> v4
    Remove the 'bool upper' and, instead, transmit pointers to the lists
    directly.
    
    v2  -> v3:
    No change.
    
    v1  -> v2:
    Fix a bug when we've tried to add a non-neighbour dev1 to dev2, while dev2
    already had dev1 as a neighbour - we shouldn't care about neighbour's
    ref_nr at all cause we can only add one - so just add/remove it via
    different functions. And it became more readable.
    
    RFC -> v1:
    rename neighbour_dev_list/all_dev_list to adj_list/all_adj_list and rework
    the naming of functions - to make the use the pattern
    netdev_(all_)(lower|upper)_* . Uninline functions to get better stack
    traces - inline doesn't give much speed improvement, but this way the stack
    traces are meaningful.

 drivers/net/bonding/bond_alb.c  |   2 +-
 drivers/net/bonding/bond_main.c |  10 +-
 include/linux/netdevice.h       |  28 ++++--
 net/core/dev.c                  | 203 ++++++++++++++++++++--------------------
 4 files changed, 129 insertions(+), 114 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index f428ef57..8524e33 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1019,7 +1019,7 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
 
 	/* loop through vlans and send one packet for each */
 	rcu_read_lock();
-	netdev_for_each_upper_dev_rcu(bond->dev, upper, iter) {
+	netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
 		if (upper->priv_flags & IFF_802_1Q_VLAN)
 			alb_send_lp_vid(slave, mac_addr,
 					vlan_dev_vlan_id(upper));
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 55bbb8b..91c4ab8 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2267,7 +2267,7 @@ static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
 		return true;
 
 	rcu_read_lock();
-	netdev_for_each_upper_dev_rcu(bond->dev, upper, iter) {
+	netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
 		if (ip == bond_confirm_addr(upper, 0, ip)) {
 			ret = true;
 			break;
@@ -2342,10 +2342,12 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 		 *
 		 * TODO: QinQ?
 		 */
-		netdev_for_each_upper_dev_rcu(bond->dev, vlan_upper, vlan_iter) {
+		netdev_for_each_all_upper_dev_rcu(bond->dev, vlan_upper,
+						  vlan_iter) {
 			if (!is_vlan_dev(vlan_upper))
 				continue;
-			netdev_for_each_upper_dev_rcu(vlan_upper, upper, iter) {
+			netdev_for_each_all_upper_dev_rcu(vlan_upper, upper,
+							  iter) {
 				if (upper == rt->dst.dev) {
 					vlan_id = vlan_dev_vlan_id(vlan_upper);
 					rcu_read_unlock();
@@ -2358,7 +2360,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 		 * our upper vlans, then just search for any dev that
 		 * matches, and in case it's a vlan - save the id
 		 */
-		netdev_for_each_upper_dev_rcu(bond->dev, upper, iter) {
+		netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
 			if (upper == rt->dst.dev) {
 				/* if it's a vlan - get its VID */
 				if (is_vlan_dev(upper))
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3de49ac..514045c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1143,8 +1143,18 @@ struct net_device {
 	struct list_head	dev_list;
 	struct list_head	napi_list;
 	struct list_head	unreg_list;
-	struct list_head	upper_dev_list; /* List of upper devices */
-	struct list_head	lower_dev_list;
+
+	/* directly linked devices, like slaves for bonding */
+	struct {
+		struct list_head upper;
+		struct list_head lower;
+	} adj_list;
+
+	/* all linked devices, *including* neighbours */
+	struct {
+		struct list_head upper;
+		struct list_head lower;
+	} all_adj_list;
 
 
 	/* currently active device features */
@@ -2813,15 +2823,15 @@ extern int		bpf_jit_enable;
 extern bool netdev_has_upper_dev(struct net_device *dev,
 				 struct net_device *upper_dev);
 extern bool netdev_has_any_upper_dev(struct net_device *dev);
-extern struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
-							struct list_head **iter);
+extern struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
+							    struct list_head **iter);
 
 /* iterate through upper list, must be called under RCU read lock */
-#define netdev_for_each_upper_dev_rcu(dev, upper, iter) \
-	for (iter = &(dev)->upper_dev_list, \
-	     upper = netdev_upper_get_next_dev_rcu(dev, &(iter)); \
-	     upper; \
-	     upper = netdev_upper_get_next_dev_rcu(dev, &(iter)))
+#define netdev_for_each_all_upper_dev_rcu(dev, updev, iter) \
+	for (iter = &(dev)->all_adj_list.upper, \
+	     updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter)); \
+	     updev; \
+	     updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter)))
 
 extern struct net_device *netdev_master_upper_dev_get(struct net_device *dev);
 extern struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 9be7937..9a395e0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4373,9 +4373,6 @@ struct netdev_adjacent {
 	/* upper master flag, there can only be one master device per list */
 	bool master;
 
-	/* indicates that this dev is our first-level lower/upper device */
-	bool neighbour;
-
 	/* counter for the number of times this device was added to us */
 	u16 ref_nr;
 
@@ -4385,29 +4382,17 @@ struct netdev_adjacent {
 
 static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
 						 struct net_device *adj_dev,
-						 struct list_head *dev_list)
+						 struct list_head *adj_list)
 {
 	struct netdev_adjacent *adj;
 
-	list_for_each_entry(adj, dev_list, list) {
+	list_for_each_entry(adj, adj_list, list) {
 		if (adj->dev == adj_dev)
 			return adj;
 	}
 	return NULL;
 }
 
-static inline struct netdev_adjacent *__netdev_find_upper(struct net_device *dev,
-							  struct net_device *udev)
-{
-	return __netdev_find_adj(dev, udev, &dev->upper_dev_list);
-}
-
-static inline struct netdev_adjacent *__netdev_find_lower(struct net_device *dev,
-							  struct net_device *ldev)
-{
-	return __netdev_find_adj(dev, ldev, &dev->lower_dev_list);
-}
-
 /**
  * netdev_has_upper_dev - Check if device is linked to an upper device
  * @dev: device
@@ -4422,7 +4407,7 @@ bool netdev_has_upper_dev(struct net_device *dev,
 {
 	ASSERT_RTNL();
 
-	return __netdev_find_upper(dev, upper_dev);
+	return __netdev_find_adj(dev, upper_dev, &dev->all_adj_list.upper);
 }
 EXPORT_SYMBOL(netdev_has_upper_dev);
 
@@ -4437,7 +4422,7 @@ bool netdev_has_any_upper_dev(struct net_device *dev)
 {
 	ASSERT_RTNL();
 
-	return !list_empty(&dev->upper_dev_list);
+	return !list_empty(&dev->all_adj_list.upper);
 }
 EXPORT_SYMBOL(netdev_has_any_upper_dev);
 
@@ -4454,10 +4439,10 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
 
 	ASSERT_RTNL();
 
-	if (list_empty(&dev->upper_dev_list))
+	if (list_empty(&dev->adj_list.upper))
 		return NULL;
 
-	upper = list_first_entry(&dev->upper_dev_list,
+	upper = list_first_entry(&dev->adj_list.upper,
 				 struct netdev_adjacent, list);
 	if (likely(upper->master))
 		return upper->dev;
@@ -4465,15 +4450,15 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
 }
 EXPORT_SYMBOL(netdev_master_upper_dev_get);
 
-/* netdev_upper_get_next_dev_rcu - Get the next dev from upper list
+/* netdev_all_upper_get_next_dev_rcu - Get the next dev from upper list
  * @dev: device
  * @iter: list_head ** of the current position
  *
  * Gets the next device from the dev's upper list, starting from iter
  * position. The caller must hold RCU read lock.
  */
-struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
-						 struct list_head **iter)
+struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
+						     struct list_head **iter)
 {
 	struct netdev_adjacent *upper;
 
@@ -4481,14 +4466,14 @@ struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
 
 	upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
 
-	if (&upper->list == &dev->upper_dev_list)
+	if (&upper->list == &dev->all_adj_list.upper)
 		return NULL;
 
 	*iter = &upper->list;
 
 	return upper->dev;
 }
-EXPORT_SYMBOL(netdev_upper_get_next_dev_rcu);
+EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu);
 
 /**
  * netdev_master_upper_dev_get_rcu - Get master upper device
@@ -4501,7 +4486,7 @@ struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev)
 {
 	struct netdev_adjacent *upper;
 
-	upper = list_first_or_null_rcu(&dev->upper_dev_list,
+	upper = list_first_or_null_rcu(&dev->adj_list.upper,
 				       struct netdev_adjacent, list);
 	if (upper && likely(upper->master))
 		return upper->dev;
@@ -4512,14 +4497,13 @@ EXPORT_SYMBOL(netdev_master_upper_dev_get_rcu);
 static int __netdev_adjacent_dev_insert(struct net_device *dev,
 					struct net_device *adj_dev,
 					struct list_head *dev_list,
-					bool neighbour, bool master)
+					bool master)
 {
 	struct netdev_adjacent *adj;
 
 	adj = __netdev_find_adj(dev, adj_dev, dev_list);
 
 	if (adj) {
-		BUG_ON(neighbour);
 		adj->ref_nr++;
 		return 0;
 	}
@@ -4530,13 +4514,11 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
 
 	adj->dev = adj_dev;
 	adj->master = master;
-	adj->neighbour = neighbour;
 	adj->ref_nr = 1;
-
 	dev_hold(adj_dev);
-	pr_debug("dev_hold for %s, because of %s link added from %s to %s\n",
-		 adj_dev->name, dev_list == &dev->upper_dev_list ?
-		 "upper" : "lower", dev->name, adj_dev->name);
+
+	pr_debug("dev_hold for %s, because of link added from %s to %s\n",
+		 adj_dev->name, dev->name, adj_dev->name);
 
 	/* Ensure that master link is always the first item in list. */
 	if (master)
@@ -4547,22 +4529,6 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
 	return 0;
 }
 
-static inline int __netdev_upper_dev_insert(struct net_device *dev,
-					    struct net_device *udev,
-					    bool master, bool neighbour)
-{
-	return __netdev_adjacent_dev_insert(dev, udev, &dev->upper_dev_list,
-					    neighbour, master);
-}
-
-static inline int __netdev_lower_dev_insert(struct net_device *dev,
-					    struct net_device *ldev,
-					    bool neighbour)
-{
-	return __netdev_adjacent_dev_insert(dev, ldev, &dev->lower_dev_list,
-					    neighbour, false);
-}
-
 void __netdev_adjacent_dev_remove(struct net_device *dev,
 				  struct net_device *adj_dev,
 				  struct list_head *dev_list)
@@ -4571,73 +4537,102 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
 
 	adj = __netdev_find_adj(dev, adj_dev, dev_list);
 
-	if (!adj)
+	if (!adj) {
+		pr_err("tried to remove device %s from %s\n",
+		       dev->name, adj_dev->name);
 		BUG();
+	}
 
 	if (adj->ref_nr > 1) {
+		pr_debug("%s to %s ref_nr-- = %d\n", dev->name, adj_dev->name,
+			 adj->ref_nr-1);
 		adj->ref_nr--;
 		return;
 	}
 
 	list_del_rcu(&adj->list);
-	pr_debug("dev_put for %s, because of %s link removed from %s to %s\n",
-		 adj_dev->name, dev_list == &dev->upper_dev_list ?
-		 "upper" : "lower", dev->name, adj_dev->name);
+	pr_debug("dev_put for %s, because link removed from %s to %s\n",
+		 adj_dev->name, dev->name, adj_dev->name);
 	dev_put(adj_dev);
 	kfree_rcu(adj, rcu);
 }
 
-static inline void __netdev_upper_dev_remove(struct net_device *dev,
-					     struct net_device *udev)
-{
-	return __netdev_adjacent_dev_remove(dev, udev, &dev->upper_dev_list);
-}
-
-static inline void __netdev_lower_dev_remove(struct net_device *dev,
-					     struct net_device *ldev)
-{
-	return __netdev_adjacent_dev_remove(dev, ldev, &dev->lower_dev_list);
-}
-
-int __netdev_adjacent_dev_insert_link(struct net_device *dev,
-				      struct net_device *upper_dev,
-				      bool master, bool neighbour)
+int __netdev_adjacent_dev_link_lists(struct net_device *dev,
+				     struct net_device *upper_dev,
+				     struct list_head *up_list,
+				     struct list_head *down_list,
+				     bool master)
 {
 	int ret;
 
-	ret = __netdev_upper_dev_insert(dev, upper_dev, master, neighbour);
+	ret = __netdev_adjacent_dev_insert(dev, upper_dev, up_list, master);
 	if (ret)
 		return ret;
 
-	ret = __netdev_lower_dev_insert(upper_dev, dev, neighbour);
+	ret = __netdev_adjacent_dev_insert(upper_dev, dev, down_list, false);
 	if (ret) {
-		__netdev_upper_dev_remove(dev, upper_dev);
+		__netdev_adjacent_dev_remove(dev, upper_dev, up_list);
 		return ret;
 	}
 
 	return 0;
 }
 
-static inline int __netdev_adjacent_dev_link(struct net_device *dev,
-					     struct net_device *udev)
+int __netdev_adjacent_dev_link(struct net_device *dev,
+			       struct net_device *upper_dev)
 {
-	return __netdev_adjacent_dev_insert_link(dev, udev, false, false);
+	return __netdev_adjacent_dev_link_lists(dev, upper_dev,
+						&dev->all_adj_list.upper,
+						&upper_dev->all_adj_list.lower,
+						false);
 }
 
-static inline int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
-						       struct net_device *udev,
-						       bool master)
+void __netdev_adjacent_dev_unlink_lists(struct net_device *dev,
+					struct net_device *upper_dev,
+					struct list_head *up_list,
+					struct list_head *down_list)
 {
-	return __netdev_adjacent_dev_insert_link(dev, udev, master, true);
+	__netdev_adjacent_dev_remove(dev, upper_dev, up_list);
+	__netdev_adjacent_dev_remove(upper_dev, dev, down_list);
 }
 
 void __netdev_adjacent_dev_unlink(struct net_device *dev,
 				  struct net_device *upper_dev)
 {
-	__netdev_upper_dev_remove(dev, upper_dev);
-	__netdev_lower_dev_remove(upper_dev, dev);
+	__netdev_adjacent_dev_unlink_lists(dev, upper_dev,
+					   &dev->all_adj_list.upper,
+					   &upper_dev->all_adj_list.lower);
+}
+
+int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
+					 struct net_device *upper_dev,
+					 bool master)
+{
+	int ret = __netdev_adjacent_dev_link(dev, upper_dev);
+
+	if (ret)
+		return ret;
+
+	ret = __netdev_adjacent_dev_link_lists(dev, upper_dev,
+					       &dev->adj_list.upper,
+					       &upper_dev->adj_list.lower,
+					       master);
+	if (ret) {
+		__netdev_adjacent_dev_unlink(dev, upper_dev);
+		return ret;
+	}
+
+	return 0;
 }
 
+void __netdev_adjacent_dev_unlink_neighbour(struct net_device *dev,
+					    struct net_device *upper_dev)
+{
+	__netdev_adjacent_dev_unlink(dev, upper_dev);
+	__netdev_adjacent_dev_unlink_lists(dev, upper_dev,
+					   &dev->adj_list.upper,
+					   &upper_dev->adj_list.lower);
+}
 
 static int __netdev_upper_dev_link(struct net_device *dev,
 				   struct net_device *upper_dev, bool master)
@@ -4651,10 +4646,10 @@ static int __netdev_upper_dev_link(struct net_device *dev,
 		return -EBUSY;
 
 	/* To prevent loops, check if dev is not upper device to upper_dev. */
-	if (__netdev_find_upper(upper_dev, dev))
+	if (__netdev_find_adj(upper_dev, dev, &upper_dev->all_adj_list.upper))
 		return -EBUSY;
 
-	if (__netdev_find_upper(dev, upper_dev))
+	if (__netdev_find_adj(dev, upper_dev, &dev->all_adj_list.upper))
 		return -EEXIST;
 
 	if (master && netdev_master_upper_dev_get(dev))
@@ -4665,12 +4660,14 @@ static int __netdev_upper_dev_link(struct net_device *dev,
 		return ret;
 
 	/* Now that we linked these devs, make all the upper_dev's
-	 * upper_dev_list visible to every dev's lower_dev_list and vice
+	 * all_adj_list.upper visible to every dev's all_adj_list.lower an
 	 * versa, and don't forget the devices itself. All of these
 	 * links are non-neighbours.
 	 */
-	list_for_each_entry(i, &dev->lower_dev_list, list) {
-		list_for_each_entry(j, &upper_dev->upper_dev_list, list) {
+	list_for_each_entry(i, &dev->all_adj_list.lower, list) {
+		list_for_each_entry(j, &upper_dev->all_adj_list.upper, list) {
+			pr_debug("Interlinking %s with %s, non-neighbour\n",
+				 i->dev->name, j->dev->name);
 			ret = __netdev_adjacent_dev_link(i->dev, j->dev);
 			if (ret)
 				goto rollback_mesh;
@@ -4678,14 +4675,18 @@ static int __netdev_upper_dev_link(struct net_device *dev,
 	}
 
 	/* add dev to every upper_dev's upper device */
-	list_for_each_entry(i, &upper_dev->upper_dev_list, list) {
+	list_for_each_entry(i, &upper_dev->all_adj_list.upper, list) {
+		pr_debug("linking %s's upper device %s with %s\n",
+			 upper_dev->name, i->dev->name, dev->name);
 		ret = __netdev_adjacent_dev_link(dev, i->dev);
 		if (ret)
 			goto rollback_upper_mesh;
 	}
 
 	/* add upper_dev to every dev's lower device */
-	list_for_each_entry(i, &dev->lower_dev_list, list) {
+	list_for_each_entry(i, &dev->all_adj_list.lower, list) {
+		pr_debug("linking %s's lower device %s with %s\n", dev->name,
+			 i->dev->name, upper_dev->name);
 		ret = __netdev_adjacent_dev_link(i->dev, upper_dev);
 		if (ret)
 			goto rollback_lower_mesh;
@@ -4696,7 +4697,7 @@ static int __netdev_upper_dev_link(struct net_device *dev,
 
 rollback_lower_mesh:
 	to_i = i;
-	list_for_each_entry(i, &dev->lower_dev_list, list) {
+	list_for_each_entry(i, &dev->all_adj_list.lower, list) {
 		if (i == to_i)
 			break;
 		__netdev_adjacent_dev_unlink(i->dev, upper_dev);
@@ -4706,7 +4707,7 @@ rollback_lower_mesh:
 
 rollback_upper_mesh:
 	to_i = i;
-	list_for_each_entry(i, &upper_dev->upper_dev_list, list) {
+	list_for_each_entry(i, &upper_dev->all_adj_list.upper, list) {
 		if (i == to_i)
 			break;
 		__netdev_adjacent_dev_unlink(dev, i->dev);
@@ -4717,8 +4718,8 @@ rollback_upper_mesh:
 rollback_mesh:
 	to_i = i;
 	to_j = j;
-	list_for_each_entry(i, &dev->lower_dev_list, list) {
-		list_for_each_entry(j, &upper_dev->upper_dev_list, list) {
+	list_for_each_entry(i, &dev->all_adj_list.lower, list) {
+		list_for_each_entry(j, &upper_dev->all_adj_list.upper, list) {
 			if (i == to_i && j == to_j)
 				break;
 			__netdev_adjacent_dev_unlink(i->dev, j->dev);
@@ -4727,7 +4728,7 @@ rollback_mesh:
 			break;
 	}
 
-	__netdev_adjacent_dev_unlink(dev, upper_dev);
+	__netdev_adjacent_dev_unlink_neighbour(dev, upper_dev);
 
 	return ret;
 }
@@ -4781,23 +4782,23 @@ void netdev_upper_dev_unlink(struct net_device *dev,
 	struct netdev_adjacent *i, *j;
 	ASSERT_RTNL();
 
-	__netdev_adjacent_dev_unlink(dev, upper_dev);
+	__netdev_adjacent_dev_unlink_neighbour(dev, upper_dev);
 
 	/* Here is the tricky part. We must remove all dev's lower
 	 * devices from all upper_dev's upper devices and vice
 	 * versa, to maintain the graph relationship.
 	 */
-	list_for_each_entry(i, &dev->lower_dev_list, list)
-		list_for_each_entry(j, &upper_dev->upper_dev_list, list)
+	list_for_each_entry(i, &dev->all_adj_list.lower, list)
+		list_for_each_entry(j, &upper_dev->all_adj_list.upper, list)
 			__netdev_adjacent_dev_unlink(i->dev, j->dev);
 
 	/* remove also the devices itself from lower/upper device
 	 * list
 	 */
-	list_for_each_entry(i, &dev->lower_dev_list, list)
+	list_for_each_entry(i, &dev->all_adj_list.lower, list)
 		__netdev_adjacent_dev_unlink(i->dev, upper_dev);
 
-	list_for_each_entry(i, &upper_dev->upper_dev_list, list)
+	list_for_each_entry(i, &upper_dev->all_adj_list.upper, list)
 		__netdev_adjacent_dev_unlink(dev, i->dev);
 
 	call_netdevice_notifiers(NETDEV_CHANGEUPPER, dev);
@@ -6059,8 +6060,10 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	INIT_LIST_HEAD(&dev->napi_list);
 	INIT_LIST_HEAD(&dev->unreg_list);
 	INIT_LIST_HEAD(&dev->link_watch_list);
-	INIT_LIST_HEAD(&dev->upper_dev_list);
-	INIT_LIST_HEAD(&dev->lower_dev_list);
+	INIT_LIST_HEAD(&dev->adj_list.upper);
+	INIT_LIST_HEAD(&dev->adj_list.lower);
+	INIT_LIST_HEAD(&dev->all_adj_list.upper);
+	INIT_LIST_HEAD(&dev->all_adj_list.lower);
 	dev->priv_flags = IFF_XMIT_DST_RELEASE;
 	setup(dev);
 
-- 
1.8.4

^ permalink raw reply related

* [PATCH v4 net-next 01/27] net: use lists as arguments instead of bool upper
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
  To: netdev
  Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
	Alexander Duyck, Cong Wang
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>

Currently we make use of bool upper when we want to specify if we want to
work with upper/lower list. It's, however, harder to read, debug and
occupies a lot more code.

Fix this by just passing the correct upper/lower_dev_list list_head pointer
instead of bool upper, and work internally with it.

CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Cong Wang <amwang@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---

Notes:
    RFC -> v4:
    New patch.

 net/core/dev.c | 54 ++++++++++++++++++++++--------------------------------
 1 file changed, 22 insertions(+), 32 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 5c713f2..9be7937 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4385,12 +4385,9 @@ struct netdev_adjacent {
 
 static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
 						 struct net_device *adj_dev,
-						 bool upper)
+						 struct list_head *dev_list)
 {
 	struct netdev_adjacent *adj;
-	struct list_head *dev_list;
-
-	dev_list = upper ? &dev->upper_dev_list : &dev->lower_dev_list;
 
 	list_for_each_entry(adj, dev_list, list) {
 		if (adj->dev == adj_dev)
@@ -4402,13 +4399,13 @@ static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
 static inline struct netdev_adjacent *__netdev_find_upper(struct net_device *dev,
 							  struct net_device *udev)
 {
-	return __netdev_find_adj(dev, udev, true);
+	return __netdev_find_adj(dev, udev, &dev->upper_dev_list);
 }
 
 static inline struct netdev_adjacent *__netdev_find_lower(struct net_device *dev,
 							  struct net_device *ldev)
 {
-	return __netdev_find_adj(dev, ldev, false);
+	return __netdev_find_adj(dev, ldev, &dev->lower_dev_list);
 }
 
 /**
@@ -4514,12 +4511,12 @@ EXPORT_SYMBOL(netdev_master_upper_dev_get_rcu);
 
 static int __netdev_adjacent_dev_insert(struct net_device *dev,
 					struct net_device *adj_dev,
-					bool neighbour, bool master,
-					bool upper)
+					struct list_head *dev_list,
+					bool neighbour, bool master)
 {
 	struct netdev_adjacent *adj;
 
-	adj = __netdev_find_adj(dev, adj_dev, upper);
+	adj = __netdev_find_adj(dev, adj_dev, dev_list);
 
 	if (adj) {
 		BUG_ON(neighbour);
@@ -4538,19 +4535,14 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
 
 	dev_hold(adj_dev);
 	pr_debug("dev_hold for %s, because of %s link added from %s to %s\n",
-		 adj_dev->name, upper ? "upper" : "lower", dev->name,
-		 adj_dev->name);
+		 adj_dev->name, dev_list == &dev->upper_dev_list ?
+		 "upper" : "lower", dev->name, adj_dev->name);
 
-	if (!upper) {
-		list_add_tail_rcu(&adj->list, &dev->lower_dev_list);
-		return 0;
-	}
-
-	/* Ensure that master upper link is always the first item in list. */
+	/* Ensure that master link is always the first item in list. */
 	if (master)
-		list_add_rcu(&adj->list, &dev->upper_dev_list);
+		list_add_rcu(&adj->list, dev_list);
 	else
-		list_add_tail_rcu(&adj->list, &dev->upper_dev_list);
+		list_add_tail_rcu(&adj->list, dev_list);
 
 	return 0;
 }
@@ -4559,27 +4551,25 @@ static inline int __netdev_upper_dev_insert(struct net_device *dev,
 					    struct net_device *udev,
 					    bool master, bool neighbour)
 {
-	return __netdev_adjacent_dev_insert(dev, udev, neighbour, master,
-					    true);
+	return __netdev_adjacent_dev_insert(dev, udev, &dev->upper_dev_list,
+					    neighbour, master);
 }
 
 static inline int __netdev_lower_dev_insert(struct net_device *dev,
 					    struct net_device *ldev,
 					    bool neighbour)
 {
-	return __netdev_adjacent_dev_insert(dev, ldev, neighbour, false,
-					    false);
+	return __netdev_adjacent_dev_insert(dev, ldev, &dev->lower_dev_list,
+					    neighbour, false);
 }
 
 void __netdev_adjacent_dev_remove(struct net_device *dev,
-				  struct net_device *adj_dev, bool upper)
+				  struct net_device *adj_dev,
+				  struct list_head *dev_list)
 {
 	struct netdev_adjacent *adj;
 
-	if (upper)
-		adj = __netdev_find_upper(dev, adj_dev);
-	else
-		adj = __netdev_find_lower(dev, adj_dev);
+	adj = __netdev_find_adj(dev, adj_dev, dev_list);
 
 	if (!adj)
 		BUG();
@@ -4591,8 +4581,8 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
 
 	list_del_rcu(&adj->list);
 	pr_debug("dev_put for %s, because of %s link removed from %s to %s\n",
-		 adj_dev->name, upper ? "upper" : "lower", dev->name,
-		 adj_dev->name);
+		 adj_dev->name, dev_list == &dev->upper_dev_list ?
+		 "upper" : "lower", dev->name, adj_dev->name);
 	dev_put(adj_dev);
 	kfree_rcu(adj, rcu);
 }
@@ -4600,13 +4590,13 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
 static inline void __netdev_upper_dev_remove(struct net_device *dev,
 					     struct net_device *udev)
 {
-	return __netdev_adjacent_dev_remove(dev, udev, true);
+	return __netdev_adjacent_dev_remove(dev, udev, &dev->upper_dev_list);
 }
 
 static inline void __netdev_lower_dev_remove(struct net_device *dev,
 					     struct net_device *ldev)
 {
-	return __netdev_adjacent_dev_remove(dev, ldev, false);
+	return __netdev_adjacent_dev_remove(dev, ldev, &dev->lower_dev_list);
 }
 
 int __netdev_adjacent_dev_insert_link(struct net_device *dev,
-- 
1.8.4

^ permalink raw reply related

* Re: [PATCH net-next] xfrm: Simplify SA looking up when using wildcard source address
From: Steffen Klassert @ 2013-09-24 11:45 UTC (permalink / raw)
  To: Fan Du; +Cc: davem, netdev
In-Reply-To: <1379927917-17365-1-git-send-email-fan.du@windriver.com>

On Mon, Sep 23, 2013 at 05:18:37PM +0800, Fan Du wrote:
> I'm not quite sure I get this "wildcard source address" right,
> IMHO if a host needs to protect every traffic for a given remote host,
> then the source address is wildcard address, i.e. all ZEROs.
> (Please correct me if I'm bloodly wrong。。。)

The above does not belong to a commit message, really.
If you are not sure and you want comments on your patch,
mark your patch as RFC. You should be sure that your patch
is correct when you submit, at least in the moment you
send it. I know that this can change a second after,
but in that moment you should be sure.

> 
> Here is the argument if above statement stands true:
> __xfrm4/6_state_addr_check is a four steps check, all we need to do
> is checking whether the destination address match. Passing saddr from
> flow is worst option, as the checking needs to reach the fourth step.
> 
> So, simply this process by only checking destination address only when
> using wildcard source address for looking up SAs.
> 
> Signed-off-by: Fan Du <fan.du@windriver.com>
> ---

If you have further comments on your patch that should not be
included in the commit message, you can add them here.

>  include/net/xfrm.h    |   31 +++++++++++++++++++++++++++++++
>  net/xfrm/xfrm_state.c |    2 +-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index e253bf0..fdb9343 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1282,6 +1282,37 @@ xfrm_state_addr_check(const struct xfrm_state *x,
>  }
>  
>  static __inline__ int
> +__xfrm4_state_daddr_check(const struct xfrm_state *x,
> +                                const xfrm_address_t *daddr)
> +{
> +        return ((daddr->a4 == x->id.daddr.a4) ? 1 : 0);
> +}
> +
> +static __inline__ int
> +__xfrm6_state_daddr_check(const struct xfrm_state *x,
> +                         const xfrm_address_t *daddr)
> +{
> +        if (ipv6_addr_equal((struct in6_addr *)daddr, (struct in6_addr *)&x->id.daddr))
> +                return 1;
> +        else 
> +                return 0;
> +}
> +
> +static __inline__ int
> +xfrm_state_daddr_check(const struct xfrm_state *x,
> +                      const xfrm_address_t *daddr,
> +                      unsigned short family)
> +{
> +        switch (family) {
> +        case AF_INET:
> +                return __xfrm4_state_daddr_check(x, daddr);
> +        case AF_INET6:
> +                return __xfrm6_state_daddr_check(x, daddr);
> +        }    
> +        return 0;
> +}

You used whitespaces where you should use tabs in the whole patch.
Please do the formating right to avoid cleanup patches.

^ permalink raw reply

* Re: [PATCH net 0/4] bridge: Fix problems around the PVID
From: Toshiaki Makita @ 2013-09-24 11:45 UTC (permalink / raw)
  To: vyasevic
  Cc: Toshiaki Makita, David Miller, netdev, Fernando Luis Vazquez Cao,
	Patrick McHardy
In-Reply-To: <524052FC.5010301@redhat.com>

On Mon, 2013-09-23 at 10:41 -0400, Vlad Yasevich wrote:
> On 09/17/2013 04:12 AM, Toshiaki Makita wrote:
> > On Mon, 2013-09-16 at 13:49 -0400, Vlad Yasevich wrote:
> >> On 09/13/2013 08:06 AM, Toshiaki Makita wrote:
> >>> On Thu, 2013-09-12 at 16:00 -0400, David Miller wrote:
> >>>> From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> >>>> Date: Tue, 10 Sep 2013 19:27:54 +0900
> >>>>
> >>>>> There seem to be some undesirable behaviors related with PVID.
> >>>>> 1. It has no effect assigning PVID to a port. PVID cannot be applied
> >>>>> to any frame regardless of whether we set it or not.
> >>>>> 2. FDB entries learned via frames applied PVID are registered with
> >>>>> VID 0 rather than VID value of PVID.
> >>>>> 3. We can set 0 or 4095 as a PVID that are not allowed in IEEE 802.1Q.
> >>>>> This leads interoperational problems such as sending frames with VID
> >>>>> 4095, which is not allowed in IEEE 802.1Q, and treating frames with VID
> >>>>> 0 as they belong to VLAN 0, which is expected to be handled as they have
> >>>>> no VID according to IEEE 802.1Q.
> >>>>>
> >>>>> Note: 2nd and 3rd problems are potential and not exposed unless 1st problem
> >>>>> is fixed, because we cannot activate PVID due to it.
> >>>>
> >>>> Please work out the issues in patch #2 with Vlad and resubmit this
> >>>> series.
> >>>>
> >>>> Thank you.
> >>>
> >>> I'm hovering between whether we should fix the issue by changing vlan 0
> >>> interface behavior in 8021q module or enabling a bridge port to sending
> >>> priority-tagged frames, or another better way.
> >>>
> >>> If you could comment it, I'd appreciate it :)
> >>>
> >>>
> >>> BTW, I think what is discussed in patch #2 is another problem about
> >>> handling priority-tags, and it exists without this patch set applied.
> >>> It looks like that we should prepare another patch set than this to fix
> >>> that problem.
> >>>
> >>> Should I include patches that fix the priority-tags problem in this
> >>> patch set and resubmit them all together?
> >>>
> >>
> >> I am thinking that we might need to do it in bridge and it looks like
> >> the simplest way to do it is to have default priority regeneration table
> >> (table 6-5 from 802.1Q doc).
> >>
> >> That way I think we would conform to the spec.
> >>
> >> -vlad
> >
> > Unfortunately I don't think the default priority regeneration table
> > resolves the problem because IEEE 802.1Q says that a VLAN-aware bridge
> > can transmit untagged or VLAN-tagged frames only (the end of section 7.5
> > and 8.1.7).
> >
> > No mechanism to send priority-tagged frames is found as far as I can see
> > the standard. I think the regenerated priority is used for outgoing PCP
> > field only if egress policy is not untagged (i.e. transmitting as
> > VLAN-tagged), and unused if untagged (Section 6.9.2 3rd/4th Paragraph).
> >
> > If we want to transmit priority-tagged frames from a bridge port, I
> > think we need to implement a new (optional) feature that is above the
> > standard, as I stated previously.
> >
> > How do you feel about adding a per-port policy that enables a bridge to
> > send priority-tagged frames instead of untagged frames when egress
> > policy for the port is untagged?
> > With this change, we can transmit frames for a given vlan as either all
> > untagged, all priority-tagged or all VLAN-tagged.
> 
> That would work.  What I am thinking is that we do it by special casing
> the vid 0 egress policy specification.  Let it be untagged by default 
> and if it is tagged, then we preserve the priority field and forward
> it on.
> 
> This keeps the API stable and doesn't require user/admin from knowing 
> exactly what happens.  Default operation conforms to the spec and allows
> simple change to make it backward-compatible.
> 
> What do you think.  I've done a simple prototype of this an it seems to 
> work with the VMs I am testing with.

Are you saying that
- by default, set the 0th bit of untagged_bitmap; and
- if we unset the 0th bit and set the "vid"th bit, we transmit frames
classified as belonging to VLAN "vid" as priority-tagged?

If so, though it's attractive to keep current API, I'm worried about if
it could be a bit confusing and not intuitive for kernel/iproute2
developers that VID 0 has a special meaning only in the egress policy.
Wouldn't it be better to adding a new member to struct net_port_vlans
instead of using VID 0 of untagged_bitmap?

Or are you saying that we use a new flag in struct net_port_vlans but
use the BRIDGE_VLAN_INFO_UNTAGGED bit with VID 0 in netlink to set the
flag?

Even in that case, I'm afraid that it might be confusing for developers
for the same reason. We are going to prohibit to specify VID with 0 (and
4095) in adding/deleting a FDB entry or a vlan filtering entry, but it
would allow us to use VID 0 only when a vlan filtering entry is
configured.
I am thinking a new nlattr is a straightforward approach to configure
it.

Thanks,

Toshiaki Makita

> 
> -vlad
> 
> >
> > Thanks,
> >
> > Toshiaki Makita
> >
> >>
> >>>
> >>> Thanks,
> >>>
> >>> Toshiaki Makita
> >>>
> >>>>
> >>>> --
> >>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >>>> the body of a message to majordomo@vger.kernel.org
> >>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>>
> >>>
> >>>
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >

^ permalink raw reply

* Re: [net 5/6] i40e: better return values
From: Joe Perches @ 2013-09-24 11:34 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Jesse Brandeburg, netdev, gospo, sassmann
In-Reply-To: <1380015910-25927-6-git-send-email-jeffrey.t.kirsher@intel.com>

On Tue, 2013-09-24 at 02:45 -0700, Jeff Kirsher wrote:

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
[]
> @@ -3339,9 +3345,7 @@ static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
>  	/* Traffic class index starts from zero so
>  	 * increment to return the actual count
>  	 */
> -	num_tc++;
> -
> -	return num_tc;
> +	return num_tc++;

Ick.  post_increment problem.

	return ++num_tc;

There's nothing wrong with the original code
unless this is a bugfix which should be documented
better than "better return values".

^ permalink raw reply

* Re: [PATCH 12/19] wireless: Change variable type to bool
From: Kalle Valo @ 2013-09-24 10:54 UTC (permalink / raw)
  To: Peter Senna Tschudin
  Cc: Larry.Finger, chaoming_li, linville, linux-wireless, netdev,
	linux-kernel, kernel-janitors
In-Reply-To: <1379802471-30252-12-git-send-email-peter.senna@gmail.com>

Peter Senna Tschudin <peter.senna@gmail.com> writes:

> The variable continual is only assigned the values true and false.
> Change its type to bool.
>
> The simplified semantic patch that find this problem is as
> follows (http://coccinelle.lip6.fr/):
>
> @exists@
> type T;
> identifier b;
> @@
> - T
> + bool
>   b = ...;
>   ... when any
>   b = \(true\|false\)
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
>  drivers/net/wireless/rtlwifi/efuse.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please prefix the patch title with "rtlwifi:". We use "wireless:" for
changes to net/wireless.

-- 
Kalle Valo

^ permalink raw reply

* Re: linux-next: manual merge of the ipsec-next tree with the net-next tree
From: Steffen Klassert @ 2013-09-24 10:25 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: linux-next, linux-kernel, Fan Du, Joe Perches, David Miller,
	netdev
In-Reply-To: <20130924121629.f818475ddf308b0494cfce2a@canb.auug.org.au>

Hi Stephen.

On Tue, Sep 24, 2013 at 12:16:29PM +1000, Stephen Rothwell wrote:
> Hi Steffen,
> 
> Today's linux-next merge of the ipsec-next tree got a conflict in
> include/net/xfrm.h between commit d511337a1eda ("xfrm.h: Remove extern
> from function prototypes") from the net-next tree and commit aba826958830
> ("{ipv4,xfrm}: Introduce xfrm_tunnel_notifier for xfrm tunnel mode
> callback") from the ipsec-next tree.
> 

Thanks for the information, I'll do a rebase of the ipsec-next
tree tomorrow.

^ permalink raw reply

* [net 5/6] i40e: better return values
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Joe Perches,
	Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

As mentioned by Joe Perches, clean up return values in some functions
making sure to have consistent return types, not mixing types.

A couple of Joe's comments suggested returning void, but since
the functions in question are ndo defined, the return values are fixed.
So make a comment in the header that notes this is a function called by
net_device_ops.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Joe Perches <joe@perches.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 37 ++++++++++++++++-------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 60c7152..4cbedbd 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1787,6 +1787,8 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid)
  * i40e_vsi_kill_vlan - Remove vsi membership for given vlan
  * @vsi: the vsi being configured
  * @vid: vlan id to be removed (0 = untagged only , -1 = any)
+ *
+ * Return: 0 on success or negative otherwise
  **/
 int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
 {
@@ -1860,37 +1862,39 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
  * i40e_vlan_rx_add_vid - Add a vlan id filter to HW offload
  * @netdev: network interface to be adjusted
  * @vid: vlan id to be added
+ *
+ * net_device_ops implementation for adding vlan ids
  **/
 static int i40e_vlan_rx_add_vid(struct net_device *netdev,
 				__always_unused __be16 proto, u16 vid)
 {
 	struct i40e_netdev_priv *np = netdev_priv(netdev);
 	struct i40e_vsi *vsi = np->vsi;
-	int ret;
+	int ret = 0;
 
 	if (vid > 4095)
-		return 0;
+		return -EINVAL;
+
+	netdev_info(netdev, "adding %pM vid=%d\n", netdev->dev_addr, vid);
 
-	netdev_info(vsi->netdev, "adding %pM vid=%d\n",
-		    netdev->dev_addr, vid);
 	/* If the network stack called us with vid = 0, we should
 	 * indicate to i40e_vsi_add_vlan() that we want to receive
 	 * any traffic (i.e. with any vlan tag, or untagged)
 	 */
 	ret = i40e_vsi_add_vlan(vsi, vid ? vid : I40E_VLAN_ANY);
 
-	if (!ret) {
-		if (vid < VLAN_N_VID)
-			set_bit(vid, vsi->active_vlans);
-	}
+	if (!ret && (vid < VLAN_N_VID))
+		set_bit(vid, vsi->active_vlans);
 
-	return 0;
+	return ret;
 }
 
 /**
  * i40e_vlan_rx_kill_vid - Remove a vlan id filter from HW offload
  * @netdev: network interface to be adjusted
  * @vid: vlan id to be removed
+ *
+ * net_device_ops implementation for adding vlan ids
  **/
 static int i40e_vlan_rx_kill_vid(struct net_device *netdev,
 				 __always_unused __be16 proto, u16 vid)
@@ -1898,15 +1902,16 @@ static int i40e_vlan_rx_kill_vid(struct net_device *netdev,
 	struct i40e_netdev_priv *np = netdev_priv(netdev);
 	struct i40e_vsi *vsi = np->vsi;
 
-	netdev_info(vsi->netdev, "removing %pM vid=%d\n",
-		    netdev->dev_addr, vid);
+	netdev_info(netdev, "removing %pM vid=%d\n", netdev->dev_addr, vid);
+
 	/* return code is ignored as there is nothing a user
 	 * can do about failure to remove and a log message was
-	 * already printed from another function
+	 * already printed from the other function
 	 */
 	i40e_vsi_kill_vlan(vsi, vid);
 
 	clear_bit(vid, vsi->active_vlans);
+
 	return 0;
 }
 
@@ -3324,7 +3329,8 @@ static void i40e_pf_unquiesce_all_vsi(struct i40e_pf *pf)
  **/
 static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
 {
-	int num_tc = 0, i;
+	u8 num_tc = 0;
+	int i;
 
 	/* Scan the ETS Config Priority Table to find
 	 * traffic class enabled for a given priority
@@ -3339,9 +3345,7 @@ static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
 	/* Traffic class index starts from zero so
 	 * increment to return the actual count
 	 */
-	num_tc++;
-
-	return num_tc;
+	return num_tc++;
 }
 
 /**
@@ -3491,6 +3495,7 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
 		/* 3 bits out of 4 for each TC */
 		vsi->bw_ets_max_quanta[i] = (u8)((tc_bw_max >> (i*4)) & 0x7);
 	}
+
 	return 0;
 }
 
-- 
1.8.3.1

^ permalink raw reply related

* [net 6/6] i40e: clean up coccicheck reported errors
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

coccicheck shows:

drivers/net/ethernet/intel/i40e/i40e_adminq.c:704:2-8: Replace memcpy
with struct assignment
drivers/net/ethernet/intel/i40e/i40e_adminq.c:763:1-7: Replace memcpy
with struct assignment
drivers/net/ethernet/intel/i40e/i40e_adminq.c:810:2-8: Replace memcpy
with struct assignment
drivers/net/ethernet/intel/i40e/i40e_common.c:510:2-8: Replace memcpy
with struct assignment

Fix each of them with a *a = *b;

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_adminq.c | 7 +++----
 drivers/net/ethernet/intel/i40e/i40e_common.c | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index 0c524fa..cfef7fc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -701,8 +701,7 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
 
 	details = I40E_ADMINQ_DETAILS(hw->aq.asq, hw->aq.asq.next_to_use);
 	if (cmd_details) {
-		memcpy(details, cmd_details,
-		       sizeof(struct i40e_asq_cmd_details));
+		*details = *cmd_details;
 
 		/* If the cmd_details are defined copy the cookie.  The
 		 * cpu_to_le32 is not needed here because the data is ignored
@@ -760,7 +759,7 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
 	desc_on_ring = I40E_ADMINQ_DESC(hw->aq.asq, hw->aq.asq.next_to_use);
 
 	/* if the desc is available copy the temp desc to the right place */
-	memcpy(desc_on_ring, desc, sizeof(struct i40e_aq_desc));
+	*desc_on_ring = *desc;
 
 	/* if buff is not NULL assume indirect command */
 	if (buff != NULL) {
@@ -807,7 +806,7 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
 
 	/* if ready, copy the desc back to temp */
 	if (i40e_asq_done(hw)) {
-		memcpy(desc, desc_on_ring, sizeof(struct i40e_aq_desc));
+		*desc = *desc_on_ring;
 		if (buff != NULL)
 			memcpy(buff, dma_buff->va, buff_size);
 		retval = le16_to_cpu(desc->retval);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index c21df7b..1e4ea13 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -507,7 +507,7 @@ i40e_status i40e_aq_get_link_info(struct i40e_hw *hw,
 
 	/* save link status information */
 	if (link)
-		memcpy(link, hw_link_info, sizeof(struct i40e_link_status));
+		*link = *hw_link_info;
 
 	/* flag cleared so helper functions don't call AQ again */
 	hw->phy.get_link_info = false;
-- 
1.8.3.1

^ permalink raw reply related

* [net 4/6] i40e: convert ret to aq_ret
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

When calling admin queue functions the driver should use aq_ret
variable to help make clear that the return value is not a regular
return variable.

This allows for clean up of the return types that were previously
converted to int.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 107 ++++++++++++++--------------
 1 file changed, 52 insertions(+), 55 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 865bc6b..60c7152 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1388,7 +1388,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 	bool add_happened = false;
 	int filter_list_len = 0;
 	u32 changed_flags = 0;
-	i40e_status ret = 0;
+	i40e_status aq_ret = 0;
 	struct i40e_pf *pf;
 	int num_add = 0;
 	int num_del = 0;
@@ -1449,28 +1449,28 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 
 			/* flush a full buffer */
 			if (num_del == filter_list_len) {
-				ret = i40e_aq_remove_macvlan(&pf->hw,
+				aq_ret = i40e_aq_remove_macvlan(&pf->hw,
 					    vsi->seid, del_list, num_del,
 					    NULL);
 				num_del = 0;
 				memset(del_list, 0, sizeof(*del_list));
 
-				if (ret)
+				if (aq_ret)
 					dev_info(&pf->pdev->dev,
 						 "ignoring delete macvlan error, err %d, aq_err %d while flushing a full buffer\n",
-						 ret,
+						 aq_ret,
 						 pf->hw.aq.asq_last_status);
 			}
 		}
 		if (num_del) {
-			ret = i40e_aq_remove_macvlan(&pf->hw, vsi->seid,
+			aq_ret = i40e_aq_remove_macvlan(&pf->hw, vsi->seid,
 						     del_list, num_del, NULL);
 			num_del = 0;
 
-			if (ret)
+			if (aq_ret)
 				dev_info(&pf->pdev->dev,
 					 "ignoring delete macvlan error, err %d, aq_err %d\n",
-					 ret, pf->hw.aq.asq_last_status);
+					 aq_ret, pf->hw.aq.asq_last_status);
 		}
 
 		kfree(del_list);
@@ -1515,32 +1515,30 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 
 			/* flush a full buffer */
 			if (num_add == filter_list_len) {
-				ret = i40e_aq_add_macvlan(&pf->hw,
-							  vsi->seid,
-							  add_list,
-							  num_add,
-							  NULL);
+				aq_ret = i40e_aq_add_macvlan(&pf->hw, vsi->seid,
+							     add_list, num_add,
+							     NULL);
 				num_add = 0;
 
-				if (ret)
+				if (aq_ret)
 					break;
 				memset(add_list, 0, sizeof(*add_list));
 			}
 		}
 		if (num_add) {
-			ret = i40e_aq_add_macvlan(&pf->hw, vsi->seid,
-						  add_list, num_add, NULL);
+			aq_ret = i40e_aq_add_macvlan(&pf->hw, vsi->seid,
+						     add_list, num_add, NULL);
 			num_add = 0;
 		}
 		kfree(add_list);
 		add_list = NULL;
 
-		if (add_happened && (!ret)) {
+		if (add_happened && (!aq_ret)) {
 			/* do nothing */;
-		} else if (add_happened && (ret)) {
+		} else if (add_happened && (aq_ret)) {
 			dev_info(&pf->pdev->dev,
 				 "add filter failed, err %d, aq_err %d\n",
-				 ret, pf->hw.aq.asq_last_status);
+				 aq_ret, pf->hw.aq.asq_last_status);
 			if ((pf->hw.aq.asq_last_status == I40E_AQ_RC_ENOSPC) &&
 			    !test_bit(__I40E_FILTER_OVERFLOW_PROMISC,
 				      &vsi->state)) {
@@ -1556,28 +1554,27 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 	if (changed_flags & IFF_ALLMULTI) {
 		bool cur_multipromisc;
 		cur_multipromisc = !!(vsi->current_netdev_flags & IFF_ALLMULTI);
-		ret = i40e_aq_set_vsi_multicast_promiscuous(&vsi->back->hw,
-							    vsi->seid,
-							    cur_multipromisc,
-							    NULL);
-		if (ret)
+		aq_ret = i40e_aq_set_vsi_multicast_promiscuous(&vsi->back->hw,
+							       vsi->seid,
+							       cur_multipromisc,
+							       NULL);
+		if (aq_ret)
 			dev_info(&pf->pdev->dev,
 				 "set multi promisc failed, err %d, aq_err %d\n",
-				 ret, pf->hw.aq.asq_last_status);
+				 aq_ret, pf->hw.aq.asq_last_status);
 	}
 	if ((changed_flags & IFF_PROMISC) || promisc_forced_on) {
 		bool cur_promisc;
 		cur_promisc = (!!(vsi->current_netdev_flags & IFF_PROMISC) ||
 			       test_bit(__I40E_FILTER_OVERFLOW_PROMISC,
 					&vsi->state));
-		ret = i40e_aq_set_vsi_unicast_promiscuous(&vsi->back->hw,
-							  vsi->seid,
-							  cur_promisc,
-							  NULL);
-		if (ret)
+		aq_ret = i40e_aq_set_vsi_unicast_promiscuous(&vsi->back->hw,
+							     vsi->seid,
+							     cur_promisc, NULL);
+		if (aq_ret)
 			dev_info(&pf->pdev->dev,
 				 "set uni promisc failed, err %d, aq_err %d\n",
-				 ret, pf->hw.aq.asq_last_status);
+				 aq_ret, pf->hw.aq.asq_last_status);
 	}
 
 	clear_bit(__I40E_CONFIG_BUSY, &vsi->state);
@@ -1936,10 +1933,10 @@ static void i40e_restore_vlan(struct i40e_vsi *vsi)
  * @vsi: the vsi being adjusted
  * @vid: the vlan id to set as a PVID
  **/
-i40e_status i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
+int i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
 {
 	struct i40e_vsi_context ctxt;
-	i40e_status ret;
+	i40e_status aq_ret;
 
 	vsi->info.valid_sections = cpu_to_le16(I40E_AQ_VSI_PROP_VLAN_VALID);
 	vsi->info.pvid = cpu_to_le16(vid);
@@ -1948,14 +1945,15 @@ i40e_status i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
 
 	ctxt.seid = vsi->seid;
 	memcpy(&ctxt.info, &vsi->info, sizeof(vsi->info));
-	ret = i40e_aq_update_vsi_params(&vsi->back->hw, &ctxt, NULL);
-	if (ret) {
+	aq_ret = i40e_aq_update_vsi_params(&vsi->back->hw, &ctxt, NULL);
+	if (aq_ret) {
 		dev_info(&vsi->back->pdev->dev,
 			 "%s: update vsi failed, aq_err=%d\n",
 			 __func__, vsi->back->hw.aq.asq_last_status);
+		return -ENOENT;
 	}
 
-	return ret;
+	return 0;
 }
 
 /**
@@ -3451,28 +3449,27 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
 	struct i40e_aqc_query_vsi_bw_config_resp bw_config = {0};
 	struct i40e_pf *pf = vsi->back;
 	struct i40e_hw *hw = &pf->hw;
+	i40e_status aq_ret;
 	u32 tc_bw_max;
-	int ret;
 	int i;
 
 	/* Get the VSI level BW configuration */
-	ret = i40e_aq_query_vsi_bw_config(hw, vsi->seid, &bw_config, NULL);
-	if (ret) {
+	aq_ret = i40e_aq_query_vsi_bw_config(hw, vsi->seid, &bw_config, NULL);
+	if (aq_ret) {
 		dev_info(&pf->pdev->dev,
 			 "couldn't get pf vsi bw config, err %d, aq_err %d\n",
-			 ret, pf->hw.aq.asq_last_status);
-		return ret;
+			 aq_ret, pf->hw.aq.asq_last_status);
+		return -EINVAL;
 	}
 
 	/* Get the VSI level BW configuration per TC */
-	ret = i40e_aq_query_vsi_ets_sla_config(hw, vsi->seid,
-					       &bw_ets_config,
-					       NULL);
-	if (ret) {
+	aq_ret = i40e_aq_query_vsi_ets_sla_config(hw, vsi->seid, &bw_ets_config,
+					          NULL);
+	if (aq_ret) {
 		dev_info(&pf->pdev->dev,
 			 "couldn't get pf vsi ets bw config, err %d, aq_err %d\n",
-			 ret, pf->hw.aq.asq_last_status);
-		return ret;
+			 aq_ret, pf->hw.aq.asq_last_status);
+		return -EINVAL;
 	}
 
 	if (bw_config.tc_valid_bits != bw_ets_config.tc_valid_bits) {
@@ -3494,7 +3491,7 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
 		/* 3 bits out of 4 for each TC */
 		vsi->bw_ets_max_quanta[i] = (u8)((tc_bw_max >> (i*4)) & 0x7);
 	}
-	return ret;
+	return 0;
 }
 
 /**
@@ -3505,30 +3502,30 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
  *
  * Returns 0 on success, negative value on failure
  **/
-static int i40e_vsi_configure_bw_alloc(struct i40e_vsi *vsi,
-				       u8 enabled_tc,
+static int i40e_vsi_configure_bw_alloc(struct i40e_vsi *vsi, u8 enabled_tc,
 				       u8 *bw_share)
 {
 	struct i40e_aqc_configure_vsi_tc_bw_data bw_data;
-	int i, ret = 0;
+	i40e_status aq_ret;
+	int i;
 
 	bw_data.tc_valid_bits = enabled_tc;
 	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
 		bw_data.tc_bw_credits[i] = bw_share[i];
 
-	ret = i40e_aq_config_vsi_tc_bw(&vsi->back->hw, vsi->seid,
-				       &bw_data, NULL);
-	if (ret) {
+	aq_ret = i40e_aq_config_vsi_tc_bw(&vsi->back->hw, vsi->seid, &bw_data,
+					  NULL);
+	if (aq_ret) {
 		dev_info(&vsi->back->pdev->dev,
 			 "%s: AQ command Config VSI BW allocation per TC failed = %d\n",
 			 __func__, vsi->back->hw.aq.asq_last_status);
-		return ret;
+		return -EINVAL;
 	}
 
 	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
 		vsi->info.qs_handle[i] = bw_data.qs_handles[i];
 
-	return ret;
+	return 0;
 }
 
 /**
-- 
1.8.3.1

^ permalink raw reply related

* [net 2/6] i40e: use common failure flow
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Joe Perches,
	Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

As mentioned by Joe Perches, we should be using
foo = alloc(...)
if (!foo)
	return -ENOMEM;

return 0;

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Joe Perches <joe@perches.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 601d482..67f8fd5 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -101,10 +101,10 @@ int i40e_allocate_dma_mem_d(struct i40e_hw *hw, struct i40e_dma_mem *mem,
 	mem->size = ALIGN(size, alignment);
 	mem->va = dma_zalloc_coherent(&pf->pdev->dev, mem->size,
 				      &mem->pa, GFP_KERNEL);
-	if (mem->va)
-		return 0;
+	if (!mem->va)
+		return -ENOMEM;
 
-	return -ENOMEM;
+	return 0;
 }
 
 /**
@@ -136,10 +136,10 @@ int i40e_allocate_virt_mem_d(struct i40e_hw *hw, struct i40e_virt_mem *mem,
 	mem->size = size;
 	mem->va = kzalloc(size, GFP_KERNEL);
 
-	if (mem->va)
-		return 0;
+	if (!mem->va)
+		return -ENOMEM;
 
-	return -ENOMEM;
+	return 0;
 }
 
 /**
-- 
1.8.3.1

^ permalink raw reply related

* [net 3/6] i40e: small clean ups from review
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Joe Perches,
	Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

As mentioned by Joe Perches clean up a loop flow.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Joe Perches <joe@perches.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 67f8fd5..865bc6b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -174,8 +174,7 @@ static int i40e_get_lump(struct i40e_pf *pf, struct i40e_lump_tracking *pile,
 			 u16 needed, u16 id)
 {
 	int ret = -ENOMEM;
-	int i = 0;
-	int j = 0;
+	int i, j;
 
 	if (!pile || needed == 0 || id >= I40E_PILE_VALID_BIT) {
 		dev_info(&pf->pdev->dev,
@@ -186,7 +185,7 @@ static int i40e_get_lump(struct i40e_pf *pf, struct i40e_lump_tracking *pile,
 
 	/* start the linear search with an imperfect hint */
 	i = pile->search_hint;
-	while (i < pile->num_entries && ret < 0) {
+	while (i < pile->num_entries) {
 		/* skip already allocated entries */
 		if (pile->list[i] & I40E_PILE_VALID_BIT) {
 			i++;
@@ -205,6 +204,7 @@ static int i40e_get_lump(struct i40e_pf *pf, struct i40e_lump_tracking *pile,
 				pile->list[i+j] = id | I40E_PILE_VALID_BIT;
 			ret = i;
 			pile->search_hint = i + j;
+			break;
 		} else {
 			/* not enough, so skip over it and continue looking */
 			i += j;
-- 
1.8.3.1

^ permalink raw reply related

* [net 0/6][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann

This series contains updates to igb and i40e.

Todd provides a fix for 82580 devices in igb, where the ethtool
loopback test was missing 82580 copper devices.

Jesse provides five fixes/cleanups to i40e based on feedback from
Joe Perches and the community.

The following are changes since commit 9fe34f5d920b183ec063550e0f4ec854aa373316:
  mrp: add periodictimer to allow retries when packets get lost
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master

Todd Fujinaka (1):
  igb: Fix ethtool loopback test for 82580 copper

Jesse Brandeburg (5):
  i40e: use common failure flow
  i40e: small clean ups from review
  i40e: convert ret to aq_ret
  i40e: better return values
  i40e: clean up coccicheck reported errors

 drivers/net/ethernet/intel/i40e/i40e_adminq.c |   7 +-
 drivers/net/ethernet/intel/i40e/i40e_common.c |   2 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 162 +++++++++++++-------------
 drivers/net/ethernet/intel/igb/igb_ethtool.c  |   3 +
 4 files changed, 89 insertions(+), 85 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [net 1/6] igb: Fix ethtool loopback test for 82580 copper
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Todd Fujinaka, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Todd Fujinaka <todd.fujinaka@intel.com>

Add back 82580 loopback tests to ethtool.

Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 48cbc83..86d5142 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -1607,6 +1607,9 @@ static int igb_integrated_phy_loopback(struct igb_adapter *adapter)
 			igb_write_phy_reg(hw, I347AT4_PAGE_SELECT, 0);
 			igb_write_phy_reg(hw, PHY_CONTROL, 0x4140);
 		}
+	} else if (hw->phy.type == e1000_phy_82580) {
+		/* enable MII loopback */
+		igb_write_phy_reg(hw, I82580_PHY_LBK_CTRL, 0x8041);
 	}
 
 	/* add small delay to avoid loopback test failure */
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH net-next] xen-netback: improve ring effeciency for guest RX
From: Ian Campbell @ 2013-09-24  9:41 UTC (permalink / raw)
  To: Wei Liu; +Cc: netdev, xen-devel, Xi Xiong, Matt Wilson, Annie Li, David Vrabel
In-Reply-To: <1379873024-32132-1-git-send-email-wei.liu2@citrix.com>

On Sun, 2013-09-22 at 19:03 +0100, Wei Liu wrote:
> There was a bug that netback routines netbk/xenvif_skb_count_slots and
> netbk/xenvif_gop_frag_copy disagreed with each other, which caused
> netback to push wrong number of responses to netfront, which caused
> netfront to eventually crash. The bug was fixed in 6e43fc04a
> ("xen-netback: count number required slots for an skb more carefully").
> 
> Commit 6e43fc04a focused on backport-ability. The drawback with the
> existing packing scheme is that the ring is not used effeciently, as
> stated in 6e43fc04a.
> 
> skb->data like:
>     |        1111|222222222222|3333        |
> 
> is arranged as:
>     |1111        |222222222222|3333        |
> 
> If we can do this:
>     |111122222222|22223333    |
> That would save one ring slot, which improves ring effeciency.
> 
> This patch effectively reverts 6e43fc04a. That patch made count_slots
> agree with gop_frag_copy, while this patch goes the other way around --
> make gop_frag_copy agree with count_slots. The end result is that they
> still agree with each other, and the ring is now arranged like:
>     |111122222222|22223333    |
> 
> The patch that improves packing was first posted by Xi Xong and Matt
> Wilson. I only rebase it on top of net-next and rewrite commit message,
> so I retain all their SoBs. For more infomation about the original bug
> please refer to email listed below and commit message of 6e43fc04a.
> 
> Original patch:
> http://lists.xen.org/archives/html/xen-devel/2013-07/msg00760.html
> 
> Signed-off-by: Xi Xiong <xixiong@amazon.com>
> Reviewed-by: Matt Wilson <msw@amazon.com>
> [ msw: minor code cleanups, rewrote commit message, adjusted code
>   to count RX slots instead of meta structures ]
> Signed-off-by: Matt Wilson <msw@amazon.com>
> Cc: Annie Li <annie.li@oracle.com>
> Cc: Wei Liu <wei.liu2@citrix.com>

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

> [ liuw: rebased on top of net-next tree, rewrote commit message, coding
>   style cleanup. ]
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> ---
>  drivers/net/xen-netback/netback.c |  144 ++++++++++++++++---------------------
>  1 file changed, 61 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index f3e591c..d0b0feb 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -47,6 +47,14 @@
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/page.h>
>  
> +/* SKB control block overlay is used to store useful information when
> + * doing guest RX.
> + */
> +struct skb_cb_overlay {
> +	int meta_slots_used;
> +	int peek_slots_count;
> +};
> +
>  /* Provide an option to disable split event channels at load time as
>   * event channels are limited resource. Split event channels are
>   * enabled by default.
> @@ -212,49 +220,6 @@ static bool start_new_rx_buffer(int offset, unsigned long size, int head)
>  	return false;
>  }
>  
> -struct xenvif_count_slot_state {
> -	unsigned long copy_off;
> -	bool head;
> -};
> -
> -unsigned int xenvif_count_frag_slots(struct xenvif *vif,
> -				     unsigned long offset, unsigned long size,
> -				     struct xenvif_count_slot_state *state)
> -{
> -	unsigned count = 0;
> -
> -	offset &= ~PAGE_MASK;
> -
> -	while (size > 0) {
> -		unsigned long bytes;
> -
> -		bytes = PAGE_SIZE - offset;
> -
> -		if (bytes > size)
> -			bytes = size;
> -
> -		if (start_new_rx_buffer(state->copy_off, bytes, state->head)) {
> -			count++;
> -			state->copy_off = 0;
> -		}
> -
> -		if (state->copy_off + bytes > MAX_BUFFER_OFFSET)
> -			bytes = MAX_BUFFER_OFFSET - state->copy_off;
> -
> -		state->copy_off += bytes;
> -
> -		offset += bytes;
> -		size -= bytes;
> -
> -		if (offset == PAGE_SIZE)
> -			offset = 0;
> -
> -		state->head = false;
> -	}
> -
> -	return count;
> -}
> -
>  /*
>   * Figure out how many ring slots we're going to need to send @skb to
>   * the guest. This function is essentially a dry run of
> @@ -262,40 +227,53 @@ unsigned int xenvif_count_frag_slots(struct xenvif *vif,
>   */
>  unsigned int xenvif_count_skb_slots(struct xenvif *vif, struct sk_buff *skb)
>  {
> -	struct xenvif_count_slot_state state;
>  	unsigned int count;
> -	unsigned char *data;
> -	unsigned i;
> +	int i, copy_off;
> +	struct skb_cb_overlay *sco;
>  
> -	state.head = true;
> -	state.copy_off = 0;
> +	count = DIV_ROUND_UP(skb_headlen(skb), PAGE_SIZE);
>  
> -	/* Slot for the first (partial) page of data. */
> -	count = 1;
> +	copy_off = skb_headlen(skb) % PAGE_SIZE;
>  
> -	/* Need a slot for the GSO prefix for GSO extra data? */
>  	if (skb_shinfo(skb)->gso_size)
>  		count++;
>  
> -	data = skb->data;
> -	while (data < skb_tail_pointer(skb)) {
> -		unsigned long offset = offset_in_page(data);
> -		unsigned long size = PAGE_SIZE - offset;
> +	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> +		unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
> +		unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
> +		unsigned long bytes;
>  
> -		if (data + size > skb_tail_pointer(skb))
> -			size = skb_tail_pointer(skb) - data;
> +		offset &= ~PAGE_MASK;
>  
> -		count += xenvif_count_frag_slots(vif, offset, size, &state);
> +		while (size > 0) {
> +			BUG_ON(offset >= PAGE_SIZE);
> +			BUG_ON(copy_off > MAX_BUFFER_OFFSET);
>  
> -		data += size;
> -	}
> +			bytes = PAGE_SIZE - offset;
>  
> -	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> -		unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
> -		unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
> +			if (bytes > size)
> +				bytes = size;
> +
> +			if (start_new_rx_buffer(copy_off, bytes, 0)) {
> +				count++;
> +				copy_off = 0;
> +			}
>  
> -		count += xenvif_count_frag_slots(vif, offset, size, &state);
> +			if (copy_off + bytes > MAX_BUFFER_OFFSET)
> +				bytes = MAX_BUFFER_OFFSET - copy_off;
> +
> +			copy_off += bytes;
> +
> +			offset += bytes;
> +			size -= bytes;
> +
> +			if (offset == PAGE_SIZE)
> +				offset = 0;
> +		}
>  	}
> +
> +	sco = (struct skb_cb_overlay *)skb->cb;
> +	sco->peek_slots_count = count;
>  	return count;
>  }
>  
> @@ -327,14 +305,11 @@ static struct xenvif_rx_meta *get_next_rx_buffer(struct xenvif *vif,
>  	return meta;
>  }
>  
> -/*
> - * Set up the grant operations for this fragment. If it's a flipping
> - * interface, we also set up the unmap request from here.
> - */
> +/* Set up the grant operations for this fragment. */
>  static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
>  				 struct netrx_pending_operations *npo,
>  				 struct page *page, unsigned long size,
> -				 unsigned long offset, int *head)
> +				 unsigned long offset, int head, int *first)
>  {
>  	struct gnttab_copy *copy_gop;
>  	struct xenvif_rx_meta *meta;
> @@ -358,12 +333,12 @@ static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
>  		if (bytes > size)
>  			bytes = size;
>  
> -		if (start_new_rx_buffer(npo->copy_off, bytes, *head)) {
> +		if (start_new_rx_buffer(npo->copy_off, bytes, head)) {
>  			/*
>  			 * Netfront requires there to be some data in the head
>  			 * buffer.
>  			 */
> -			BUG_ON(*head);
> +			BUG_ON(*first);
>  
>  			meta = get_next_rx_buffer(vif, npo);
>  		}
> @@ -397,10 +372,10 @@ static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
>  		}
>  
>  		/* Leave a gap for the GSO descriptor. */
> -		if (*head && skb_shinfo(skb)->gso_size && !vif->gso_prefix)
> +		if (*first && skb_shinfo(skb)->gso_size && !vif->gso_prefix)
>  			vif->rx.req_cons++;
>  
> -		*head = 0; /* There must be something in this buffer now. */
> +		*first = 0; /* There must be something in this buffer now. */
>  
>  	}
>  }
> @@ -426,7 +401,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
>  	struct xen_netif_rx_request *req;
>  	struct xenvif_rx_meta *meta;
>  	unsigned char *data;
> -	int head = 1;
> +	int first = 1;
>  	int old_meta_prod;
>  
>  	old_meta_prod = npo->meta_prod;
> @@ -462,7 +437,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
>  			len = skb_tail_pointer(skb) - data;
>  
>  		xenvif_gop_frag_copy(vif, skb, npo,
> -				     virt_to_page(data), len, offset, &head);
> +				     virt_to_page(data), len, offset, 1, &first);
>  		data += len;
>  	}
>  
> @@ -471,7 +446,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
>  				     skb_frag_page(&skb_shinfo(skb)->frags[i]),
>  				     skb_frag_size(&skb_shinfo(skb)->frags[i]),
>  				     skb_shinfo(skb)->frags[i].page_offset,
> -				     &head);
> +				     0, &first);
>  	}
>  
>  	return npo->meta_prod - old_meta_prod;
> @@ -529,10 +504,6 @@ static void xenvif_add_frag_responses(struct xenvif *vif, int status,
>  	}
>  }
>  
> -struct skb_cb_overlay {
> -	int meta_slots_used;
> -};
> -
>  static void xenvif_kick_thread(struct xenvif *vif)
>  {
>  	wake_up(&vif->wq);
> @@ -563,19 +534,26 @@ void xenvif_rx_action(struct xenvif *vif)
>  	count = 0;
>  
>  	while ((skb = skb_dequeue(&vif->rx_queue)) != NULL) {
> +		RING_IDX old_rx_req_cons;
> +
>  		vif = netdev_priv(skb->dev);
>  		nr_frags = skb_shinfo(skb)->nr_frags;
>  
> +		old_rx_req_cons = vif->rx.req_cons;
>  		sco = (struct skb_cb_overlay *)skb->cb;
>  		sco->meta_slots_used = xenvif_gop_skb(skb, &npo);
>  
> -		count += nr_frags + 1;
> +		count += vif->rx.req_cons - old_rx_req_cons;
>  
>  		__skb_queue_tail(&rxq, skb);
>  
> +		skb = skb_peek(&vif->rx_queue);
> +		if (skb == NULL)
> +			break;
> +		sco = (struct skb_cb_overlay *)skb->cb;
> +
>  		/* Filled the batch queue? */
> -		/* XXX FIXME: RX path dependent on MAX_SKB_FRAGS */
> -		if (count + MAX_SKB_FRAGS >= XEN_NETIF_RX_RING_SIZE)
> +		if (count + sco->peek_slots_count >= XEN_NETIF_RX_RING_SIZE)
>  			break;
>  	}
>  

^ permalink raw reply

* [PATCH net-next 5/5] tipc: correct return value of link_cmd_set_value routine
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy
In-Reply-To: <1380014868-2797-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

link_cmd_set_value() takes commands for link, bearer and media related
configuration. Genereally the function returns 0 when a command is
recognized, and -EINVAL when it is not. However, in the switch for link
related commands it returns 0 even when the command is unrecognized. This
will sometimes make it look as if a failed configuration command has been
successful, but has otherwise no negative effects.

We remove this anomaly by returning -EINVAL even for link commands. We also
rework all three switches to make them  conforming to common kernel coding
style.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/link.c |   28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 40521ae..8bbe4ca 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -2647,6 +2647,7 @@ static int link_cmd_set_value(const char *name, u32 new_value, u16 cmd)
 	struct tipc_link *l_ptr;
 	struct tipc_bearer *b_ptr;
 	struct tipc_media *m_ptr;
+	int res = 0;
 
 	l_ptr = link_find_link(name, &node);
 	if (l_ptr) {
@@ -2669,9 +2670,12 @@ static int link_cmd_set_value(const char *name, u32 new_value, u16 cmd)
 		case TIPC_CMD_SET_LINK_WINDOW:
 			tipc_link_set_queue_limits(l_ptr, new_value);
 			break;
+		default:
+			res = -EINVAL;
+			break;
 		}
 		tipc_node_unlock(node);
-		return 0;
+		return res;
 	}
 
 	b_ptr = tipc_bearer_find(name);
@@ -2679,15 +2683,18 @@ static int link_cmd_set_value(const char *name, u32 new_value, u16 cmd)
 		switch (cmd) {
 		case TIPC_CMD_SET_LINK_TOL:
 			b_ptr->tolerance = new_value;
-			return 0;
+			break;
 		case TIPC_CMD_SET_LINK_PRI:
 			b_ptr->priority = new_value;
-			return 0;
+			break;
 		case TIPC_CMD_SET_LINK_WINDOW:
 			b_ptr->window = new_value;
-			return 0;
+			break;
+		default:
+			res = -EINVAL;
+			break;
 		}
-		return -EINVAL;
+		return res;
 	}
 
 	m_ptr = tipc_media_find(name);
@@ -2696,15 +2703,18 @@ static int link_cmd_set_value(const char *name, u32 new_value, u16 cmd)
 	switch (cmd) {
 	case TIPC_CMD_SET_LINK_TOL:
 		m_ptr->tolerance = new_value;
-		return 0;
+		break;
 	case TIPC_CMD_SET_LINK_PRI:
 		m_ptr->priority = new_value;
-		return 0;
+		break;
 	case TIPC_CMD_SET_LINK_WINDOW:
 		m_ptr->window = new_value;
-		return 0;
+		break;
+	default:
+		res = -EINVAL;
+		break;
 	}
-	return -EINVAL;
+	return res;
 }
 
 struct sk_buff *tipc_link_cmd_config(const void *req_tlv_area, int req_tlv_space,
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 4/5] tipc: correct return value of recv_msg routine
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy
In-Reply-To: <1380014868-2797-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

Currently, rcv_msg() always returns zero on a packet delivery upcall
from net_device.

To make its behavior more compliant with the way this API should be
used, we change this to let it return NET_RX_SUCCESS (which is zero
anyway) when it is able to handle the packet, and NET_RX_DROP otherwise.
The latter does not imply any functional change, it only enables the
driver to keep more accurate statistics about the fate of delivered
packets.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/eth_media.c |    6 +++---
 net/tipc/ib_media.c  |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index c36c938..f80d59f 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -132,18 +132,18 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 
 	if (!net_eq(dev_net(dev), &init_net)) {
 		kfree_skb(buf);
-		return 0;
+		return NET_RX_DROP;
 	}
 
 	if (likely(eb_ptr->bearer)) {
 		if (likely(buf->pkt_type <= PACKET_BROADCAST)) {
 			buf->next = NULL;
 			tipc_recv_msg(buf, eb_ptr->bearer);
-			return 0;
+			return NET_RX_SUCCESS;
 		}
 	}
 	kfree_skb(buf);
-	return 0;
+	return NET_RX_DROP;
 }
 
 /**
diff --git a/net/tipc/ib_media.c b/net/tipc/ib_media.c
index 20b1aa4..c139892 100644
--- a/net/tipc/ib_media.c
+++ b/net/tipc/ib_media.c
@@ -125,18 +125,18 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 
 	if (!net_eq(dev_net(dev), &init_net)) {
 		kfree_skb(buf);
-		return 0;
+		return NET_RX_DROP;
 	}
 
 	if (likely(ib_ptr->bearer)) {
 		if (likely(buf->pkt_type <= PACKET_BROADCAST)) {
 			buf->next = NULL;
 			tipc_recv_msg(buf, ib_ptr->bearer);
-			return 0;
+			return NET_RX_SUCCESS;
 		}
 	}
 	kfree_skb(buf);
-	return 0;
+	return NET_RX_DROP;
 }
 
 /**
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 3/5] tipc: avoid unnecessary lookup for tipc bearer instance
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy
In-Reply-To: <1380014868-2797-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

tipc_block_bearer() currently takes a bearer name (const char*)
as argument. This requires the function to make a lookup to find
the pointer to the corresponding bearer struct. In the current
code base this is not necessary, since the only two callers
(tipc_continue(),recv_notification()) already have validated
copies of this pointer, and hence can pass it directly in the
function call.

We change tipc_block_bearer() to directly take struct tipc_bearer*
as argument instead.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/bearer.c    |   14 +++-----------
 net/tipc/bearer.h    |    2 +-
 net/tipc/eth_media.c |    6 +++---
 net/tipc/ib_media.c  |    6 +++---
 4 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 09faa55..3f9707a 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -420,23 +420,15 @@ exit:
 }
 
 /**
- * tipc_block_bearer - Block the bearer with the given name, and reset all its links
+ * tipc_block_bearer - Block the bearer, and reset all its links
  */
-int tipc_block_bearer(const char *name)
+int tipc_block_bearer(struct tipc_bearer *b_ptr)
 {
-	struct tipc_bearer *b_ptr = NULL;
 	struct tipc_link *l_ptr;
 	struct tipc_link *temp_l_ptr;
 
 	read_lock_bh(&tipc_net_lock);
-	b_ptr = tipc_bearer_find(name);
-	if (!b_ptr) {
-		pr_warn("Attempt to block unknown bearer <%s>\n", name);
-		read_unlock_bh(&tipc_net_lock);
-		return -EINVAL;
-	}
-
-	pr_info("Blocking bearer <%s>\n", name);
+	pr_info("Blocking bearer <%s>\n", b_ptr->name);
 	spin_lock_bh(&b_ptr->lock);
 	b_ptr->blocked = 1;
 	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
index f800e63..e5e04be 100644
--- a/net/tipc/bearer.h
+++ b/net/tipc/bearer.h
@@ -163,7 +163,7 @@ int tipc_register_media(struct tipc_media *m_ptr);
 
 void tipc_recv_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr);
 
-int  tipc_block_bearer(const char *name);
+int  tipc_block_bearer(struct tipc_bearer *b_ptr);
 void tipc_continue(struct tipc_bearer *tb_ptr);
 
 int tipc_enable_bearer(const char *bearer_name, u32 disc_domain, u32 priority);
diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index e048d49..c36c938 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -265,17 +265,17 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 		if (netif_carrier_ok(dev))
 			tipc_continue(eb_ptr->bearer);
 		else
-			tipc_block_bearer(eb_ptr->bearer->name);
+			tipc_block_bearer(eb_ptr->bearer);
 		break;
 	case NETDEV_UP:
 		tipc_continue(eb_ptr->bearer);
 		break;
 	case NETDEV_DOWN:
-		tipc_block_bearer(eb_ptr->bearer->name);
+		tipc_block_bearer(eb_ptr->bearer);
 		break;
 	case NETDEV_CHANGEMTU:
 	case NETDEV_CHANGEADDR:
-		tipc_block_bearer(eb_ptr->bearer->name);
+		tipc_block_bearer(eb_ptr->bearer);
 		tipc_continue(eb_ptr->bearer);
 		break;
 	case NETDEV_UNREGISTER:
diff --git a/net/tipc/ib_media.c b/net/tipc/ib_media.c
index 5545145..20b1aa4 100644
--- a/net/tipc/ib_media.c
+++ b/net/tipc/ib_media.c
@@ -258,17 +258,17 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 		if (netif_carrier_ok(dev))
 			tipc_continue(ib_ptr->bearer);
 		else
-			tipc_block_bearer(ib_ptr->bearer->name);
+			tipc_block_bearer(ib_ptr->bearer);
 		break;
 	case NETDEV_UP:
 		tipc_continue(ib_ptr->bearer);
 		break;
 	case NETDEV_DOWN:
-		tipc_block_bearer(ib_ptr->bearer->name);
+		tipc_block_bearer(ib_ptr->bearer);
 		break;
 	case NETDEV_CHANGEMTU:
 	case NETDEV_CHANGEADDR:
-		tipc_block_bearer(ib_ptr->bearer->name);
+		tipc_block_bearer(ib_ptr->bearer);
 		tipc_continue(ib_ptr->bearer);
 		break;
 	case NETDEV_UNREGISTER:
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 2/5] tipc: make bearer and media naming consistent
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy
In-Reply-To: <1380014868-2797-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

TIPC 'bearer' exists as an abstract concept, while 'media'
is deemed a specific implementation of a bearer, such as Ethernet
or Infiniband media. When a component inside TIPC wants to control
a specific media, it only needs to access the generic bearer API
to achieve this. However, in the current media implementations,
the 'bearer' name is also extensively used in media specific
function and variable names.

This may create confusion, so we choose to replace the term 'bearer'
with 'media' in all function names, variable names, and prefixes
where this is what really is meant.

Note that this change is cosmetic only, and no runtime behaviour
changes are made here.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/bearer.c    |    4 ++--
 net/tipc/bearer.h    |    8 ++++----
 net/tipc/eth_media.c |   56 +++++++++++++++++++++++++-------------------------
 net/tipc/ib_media.c  |   46 ++++++++++++++++++++---------------------
 4 files changed, 57 insertions(+), 57 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 609c30c..09faa55 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -387,7 +387,7 @@ restart:
 
 	b_ptr = &tipc_bearers[bearer_id];
 	strcpy(b_ptr->name, name);
-	res = m_ptr->enable_bearer(b_ptr);
+	res = m_ptr->enable_media(b_ptr);
 	if (res) {
 		pr_warn("Bearer <%s> rejected, enable failure (%d)\n",
 			name, -res);
@@ -465,7 +465,7 @@ static void bearer_disable(struct tipc_bearer *b_ptr)
 	pr_info("Disabling bearer <%s>\n", b_ptr->name);
 	spin_lock_bh(&b_ptr->lock);
 	b_ptr->blocked = 1;
-	b_ptr->media->disable_bearer(b_ptr);
+	b_ptr->media->disable_media(b_ptr);
 	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
 		tipc_link_delete(l_ptr);
 	}
diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
index 09c869a..f800e63 100644
--- a/net/tipc/bearer.h
+++ b/net/tipc/bearer.h
@@ -75,8 +75,8 @@ struct tipc_bearer;
 /**
  * struct tipc_media - TIPC media information available to internal users
  * @send_msg: routine which handles buffer transmission
- * @enable_bearer: routine which enables a bearer
- * @disable_bearer: routine which disables a bearer
+ * @enable_media: routine which enables a media
+ * @disable_media: routine which disables a media
  * @addr2str: routine which converts media address to string
  * @addr2msg: routine which converts media address to protocol message area
  * @msg2addr: routine which converts media address from protocol message area
@@ -91,8 +91,8 @@ struct tipc_media {
 	int (*send_msg)(struct sk_buff *buf,
 			struct tipc_bearer *b_ptr,
 			struct tipc_media_addr *dest);
-	int (*enable_bearer)(struct tipc_bearer *b_ptr);
-	void (*disable_bearer)(struct tipc_bearer *b_ptr);
+	int (*enable_media)(struct tipc_bearer *b_ptr);
+	void (*disable_media)(struct tipc_bearer *b_ptr);
 	int (*addr2str)(struct tipc_media_addr *a, char *str_buf, int str_size);
 	int (*addr2msg)(struct tipc_media_addr *a, char *msg_area);
 	int (*msg2addr)(const struct tipc_bearer *b_ptr,
diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index 40ea40c..e048d49 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -2,7 +2,7 @@
  * net/tipc/eth_media.c: Ethernet bearer support for TIPC
  *
  * Copyright (c) 2001-2007, Ericsson AB
- * Copyright (c) 2005-2008, 2011, Wind River Systems
+ * Copyright (c) 2005-2008, 2011-2013, Wind River Systems
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -37,19 +37,19 @@
 #include "core.h"
 #include "bearer.h"
 
-#define MAX_ETH_BEARERS		MAX_BEARERS
+#define MAX_ETH_MEDIA		MAX_BEARERS
 
 #define ETH_ADDR_OFFSET	4	/* message header offset of MAC address */
 
 /**
- * struct eth_bearer - Ethernet bearer data structure
+ * struct eth_media - Ethernet bearer data structure
  * @bearer: ptr to associated "generic" bearer structure
  * @dev: ptr to associated Ethernet network device
  * @tipc_packet_type: used in binding TIPC to Ethernet driver
  * @setup: work item used when enabling bearer
  * @cleanup: work item used when disabling bearer
  */
-struct eth_bearer {
+struct eth_media {
 	struct tipc_bearer *bearer;
 	struct net_device *dev;
 	struct packet_type tipc_packet_type;
@@ -58,7 +58,7 @@ struct eth_bearer {
 };
 
 static struct tipc_media eth_media_info;
-static struct eth_bearer eth_bearers[MAX_ETH_BEARERS];
+static struct eth_media eth_media_array[MAX_ETH_MEDIA];
 static int eth_started;
 
 static int recv_notification(struct notifier_block *nb, unsigned long evt,
@@ -100,7 +100,7 @@ static int send_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr,
 	if (!clone)
 		return 0;
 
-	dev = ((struct eth_bearer *)(tb_ptr->usr_handle))->dev;
+	dev = ((struct eth_media *)(tb_ptr->usr_handle))->dev;
 	delta = dev->hard_header_len - skb_headroom(buf);
 
 	if ((delta > 0) &&
@@ -128,7 +128,7 @@ static int send_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr,
 static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 		    struct packet_type *pt, struct net_device *orig_dev)
 {
-	struct eth_bearer *eb_ptr = (struct eth_bearer *)pt->af_packet_priv;
+	struct eth_media *eb_ptr = (struct eth_media *)pt->af_packet_priv;
 
 	if (!net_eq(dev_net(dev), &init_net)) {
 		kfree_skb(buf);
@@ -147,24 +147,24 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 }
 
 /**
- * setup_bearer - setup association between Ethernet bearer and interface
+ * setup_media - setup association between Ethernet bearer and interface
  */
-static void setup_bearer(struct work_struct *work)
+static void setup_media(struct work_struct *work)
 {
-	struct eth_bearer *eb_ptr =
-		container_of(work, struct eth_bearer, setup);
+	struct eth_media *eb_ptr =
+		container_of(work, struct eth_media, setup);
 
 	dev_add_pack(&eb_ptr->tipc_packet_type);
 }
 
 /**
- * enable_bearer - attach TIPC bearer to an Ethernet interface
+ * enable_media - attach TIPC bearer to an Ethernet interface
  */
-static int enable_bearer(struct tipc_bearer *tb_ptr)
+static int enable_media(struct tipc_bearer *tb_ptr)
 {
 	struct net_device *dev;
-	struct eth_bearer *eb_ptr = &eth_bearers[0];
-	struct eth_bearer *stop = &eth_bearers[MAX_ETH_BEARERS];
+	struct eth_media *eb_ptr = &eth_media_array[0];
+	struct eth_media *stop = &eth_media_array[MAX_ETH_MEDIA];
 	char *driver_name = strchr((const char *)tb_ptr->name, ':') + 1;
 	int pending_dev = 0;
 
@@ -188,7 +188,7 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
 	eb_ptr->tipc_packet_type.func = recv_msg;
 	eb_ptr->tipc_packet_type.af_packet_priv = eb_ptr;
 	INIT_LIST_HEAD(&(eb_ptr->tipc_packet_type.list));
-	INIT_WORK(&eb_ptr->setup, setup_bearer);
+	INIT_WORK(&eb_ptr->setup, setup_media);
 	schedule_work(&eb_ptr->setup);
 
 	/* Associate TIPC bearer with Ethernet bearer */
@@ -205,14 +205,14 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
 }
 
 /**
- * cleanup_bearer - break association between Ethernet bearer and interface
+ * cleanup_media - break association between Ethernet bearer and interface
  *
  * This routine must be invoked from a work queue because it can sleep.
  */
-static void cleanup_bearer(struct work_struct *work)
+static void cleanup_media(struct work_struct *work)
 {
-	struct eth_bearer *eb_ptr =
-		container_of(work, struct eth_bearer, cleanup);
+	struct eth_media *eb_ptr =
+		container_of(work, struct eth_media, cleanup);
 
 	dev_remove_pack(&eb_ptr->tipc_packet_type);
 	dev_put(eb_ptr->dev);
@@ -220,18 +220,18 @@ static void cleanup_bearer(struct work_struct *work)
 }
 
 /**
- * disable_bearer - detach TIPC bearer from an Ethernet interface
+ * disable_media - detach TIPC bearer from an Ethernet interface
  *
  * Mark Ethernet bearer as inactive so that incoming buffers are thrown away,
  * then get worker thread to complete bearer cleanup.  (Can't do cleanup
  * here because cleanup code needs to sleep and caller holds spinlocks.)
  */
-static void disable_bearer(struct tipc_bearer *tb_ptr)
+static void disable_media(struct tipc_bearer *tb_ptr)
 {
-	struct eth_bearer *eb_ptr = (struct eth_bearer *)tb_ptr->usr_handle;
+	struct eth_media *eb_ptr = (struct eth_media *)tb_ptr->usr_handle;
 
 	eb_ptr->bearer = NULL;
-	INIT_WORK(&eb_ptr->cleanup, cleanup_bearer);
+	INIT_WORK(&eb_ptr->cleanup, cleanup_media);
 	schedule_work(&eb_ptr->cleanup);
 }
 
@@ -245,8 +245,8 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 			     void *ptr)
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
-	struct eth_bearer *eb_ptr = &eth_bearers[0];
-	struct eth_bearer *stop = &eth_bearers[MAX_ETH_BEARERS];
+	struct eth_media *eb_ptr = &eth_media_array[0];
+	struct eth_media *stop = &eth_media_array[MAX_ETH_MEDIA];
 
 	if (!net_eq(dev_net(dev), &init_net))
 		return NOTIFY_DONE;
@@ -327,8 +327,8 @@ static int eth_msg2addr(const struct tipc_bearer *tb_ptr,
  */
 static struct tipc_media eth_media_info = {
 	.send_msg	= send_msg,
-	.enable_bearer	= enable_bearer,
-	.disable_bearer	= disable_bearer,
+	.enable_media	= enable_media,
+	.disable_media	= disable_media,
 	.addr2str	= eth_addr2str,
 	.addr2msg	= eth_addr2msg,
 	.msg2addr	= eth_msg2addr,
diff --git a/net/tipc/ib_media.c b/net/tipc/ib_media.c
index 9934a32..5545145 100644
--- a/net/tipc/ib_media.c
+++ b/net/tipc/ib_media.c
@@ -42,17 +42,17 @@
 #include "core.h"
 #include "bearer.h"
 
-#define MAX_IB_BEARERS		MAX_BEARERS
+#define MAX_IB_MEDIA		MAX_BEARERS
 
 /**
- * struct ib_bearer - Infiniband bearer data structure
+ * struct ib_media - Infiniband media data structure
  * @bearer: ptr to associated "generic" bearer structure
  * @dev: ptr to associated Infiniband network device
  * @tipc_packet_type: used in binding TIPC to Infiniband driver
  * @cleanup: work item used when disabling bearer
  */
 
-struct ib_bearer {
+struct ib_media {
 	struct tipc_bearer *bearer;
 	struct net_device *dev;
 	struct packet_type tipc_packet_type;
@@ -61,7 +61,7 @@ struct ib_bearer {
 };
 
 static struct tipc_media ib_media_info;
-static struct ib_bearer ib_bearers[MAX_IB_BEARERS];
+static struct ib_media ib_media_array[MAX_IB_MEDIA];
 static int ib_started;
 
 /**
@@ -93,7 +93,7 @@ static int send_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr,
 	if (!clone)
 		return 0;
 
-	dev = ((struct ib_bearer *)(tb_ptr->usr_handle))->dev;
+	dev = ((struct ib_media *)(tb_ptr->usr_handle))->dev;
 	delta = dev->hard_header_len - skb_headroom(buf);
 
 	if ((delta > 0) &&
@@ -121,7 +121,7 @@ static int send_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr,
 static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 		    struct packet_type *pt, struct net_device *orig_dev)
 {
-	struct ib_bearer *ib_ptr = (struct ib_bearer *)pt->af_packet_priv;
+	struct ib_media *ib_ptr = (struct ib_media *)pt->af_packet_priv;
 
 	if (!net_eq(dev_net(dev), &init_net)) {
 		kfree_skb(buf);
@@ -142,22 +142,22 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 /**
  * setup_bearer - setup association between InfiniBand bearer and interface
  */
-static void setup_bearer(struct work_struct *work)
+static void setup_media(struct work_struct *work)
 {
-	struct ib_bearer *ib_ptr =
-		container_of(work, struct ib_bearer, setup);
+	struct ib_media *ib_ptr =
+		container_of(work, struct ib_media, setup);
 
 	dev_add_pack(&ib_ptr->tipc_packet_type);
 }
 
 /**
- * enable_bearer - attach TIPC bearer to an InfiniBand interface
+ * enable_media - attach TIPC bearer to an InfiniBand interface
  */
-static int enable_bearer(struct tipc_bearer *tb_ptr)
+static int enable_media(struct tipc_bearer *tb_ptr)
 {
 	struct net_device *dev;
-	struct ib_bearer *ib_ptr = &ib_bearers[0];
-	struct ib_bearer *stop = &ib_bearers[MAX_IB_BEARERS];
+	struct ib_media *ib_ptr = &ib_media_array[0];
+	struct ib_media *stop = &ib_media_array[MAX_IB_MEDIA];
 	char *driver_name = strchr((const char *)tb_ptr->name, ':') + 1;
 	int pending_dev = 0;
 
@@ -181,7 +181,7 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
 	ib_ptr->tipc_packet_type.func = recv_msg;
 	ib_ptr->tipc_packet_type.af_packet_priv = ib_ptr;
 	INIT_LIST_HEAD(&(ib_ptr->tipc_packet_type.list));
-	INIT_WORK(&ib_ptr->setup, setup_bearer);
+	INIT_WORK(&ib_ptr->setup, setup_media);
 	schedule_work(&ib_ptr->setup);
 
 	/* Associate TIPC bearer with InfiniBand bearer */
@@ -204,8 +204,8 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
  */
 static void cleanup_bearer(struct work_struct *work)
 {
-	struct ib_bearer *ib_ptr =
-		container_of(work, struct ib_bearer, cleanup);
+	struct ib_media *ib_ptr =
+		container_of(work, struct ib_media, cleanup);
 
 	dev_remove_pack(&ib_ptr->tipc_packet_type);
 	dev_put(ib_ptr->dev);
@@ -213,15 +213,15 @@ static void cleanup_bearer(struct work_struct *work)
 }
 
 /**
- * disable_bearer - detach TIPC bearer from an InfiniBand interface
+ * disable_media - detach TIPC bearer from an InfiniBand interface
  *
  * Mark InfiniBand bearer as inactive so that incoming buffers are thrown away,
  * then get worker thread to complete bearer cleanup.  (Can't do cleanup
  * here because cleanup code needs to sleep and caller holds spinlocks.)
  */
-static void disable_bearer(struct tipc_bearer *tb_ptr)
+static void disable_media(struct tipc_bearer *tb_ptr)
 {
-	struct ib_bearer *ib_ptr = (struct ib_bearer *)tb_ptr->usr_handle;
+	struct ib_media *ib_ptr = (struct ib_media *)tb_ptr->usr_handle;
 
 	ib_ptr->bearer = NULL;
 	INIT_WORK(&ib_ptr->cleanup, cleanup_bearer);
@@ -238,8 +238,8 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 			     void *ptr)
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
-	struct ib_bearer *ib_ptr = &ib_bearers[0];
-	struct ib_bearer *stop = &ib_bearers[MAX_IB_BEARERS];
+	struct ib_media *ib_ptr = &ib_media_array[0];
+	struct ib_media *stop = &ib_media_array[MAX_IB_MEDIA];
 
 	if (!net_eq(dev_net(dev), &init_net))
 		return NOTIFY_DONE;
@@ -323,8 +323,8 @@ static int ib_msg2addr(const struct tipc_bearer *tb_ptr,
  */
 static struct tipc_media ib_media_info = {
 	.send_msg	= send_msg,
-	.enable_bearer	= enable_bearer,
-	.disable_bearer	= disable_bearer,
+	.enable_media	= enable_media,
+	.disable_media	= disable_media,
 	.addr2str	= ib_addr2str,
 	.addr2msg	= ib_addr2msg,
 	.msg2addr	= ib_msg2addr,
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 0/5] tipc: some small patches
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy

Some small and relatively straightforward patches that fix a number of
minor issues. The patches are functionally unrelated to each other.

Ying Xue (5):
  tipc: silence sparse warnings
  tipc: make bearer and media naming consistent
  tipc: avoid unnecessary lookup for tipc bearer instance
  tipc: correct return value of recv_msg routine
  tipc: correct return value of link_cmd_set_value routine

 net/tipc/bearer.c    |   18 ++++---------
 net/tipc/bearer.h    |   10 ++++----
 net/tipc/eth_media.c |   68 +++++++++++++++++++++++++-------------------------
 net/tipc/ib_media.c  |   58 +++++++++++++++++++++---------------------
 net/tipc/link.c      |   33 ++++++++++++++++--------
 net/tipc/msg.c       |    4 +--
 net/tipc/socket.c    |    6 ++---
 7 files changed, 100 insertions(+), 97 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH net-next 1/5] tipc: silence sparse warnings
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Andreas Bofjäll, Jon Maloy
In-Reply-To: <1380014868-2797-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

Eliminate below sparse warnings:

net/tipc/link.c:1210:37: warning: cast removes address space of expression
net/tipc/link.c:1218:59: warning: incorrect type in argument 2 (different address spaces)
net/tipc/link.c:1218:59:    expected void const [noderef] <asn:1>*from
net/tipc/link.c:1218:59:    got unsigned char const [usertype] *[assigned] sect_crs
net/tipc/msg.c:96:61: warning: incorrect type in argument 3 (different address spaces)
net/tipc/msg.c:96:61:    expected void const *from
net/tipc/msg.c:96:61:    got void [noderef] <asn:1>*const iov_base
net/tipc/socket.c:341:49: warning: Using plain integer as NULL pointer
net/tipc/socket.c:1371:36: warning: Using plain integer as NULL pointer
net/tipc/socket.c:1694:57: warning: Using plain integer as NULL pointer

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Andreas Bofjäll <andreas.bofjall@ericsson.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/link.c   |    5 +++--
 net/tipc/msg.c    |    4 ++--
 net/tipc/socket.c |    6 +++---
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 0cc3d90..40521ae 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1165,7 +1165,7 @@ static int link_send_sections_long(struct tipc_port *sender,
 	struct tipc_msg fragm_hdr;
 	struct sk_buff *buf, *buf_chain, *prev;
 	u32 fragm_crs, fragm_rest, hsz, sect_rest;
-	const unchar *sect_crs;
+	const unchar __user *sect_crs;
 	int curr_sect;
 	u32 fragm_no;
 	int res = 0;
@@ -1207,7 +1207,8 @@ again:
 
 		if (!sect_rest) {
 			sect_rest = msg_sect[++curr_sect].iov_len;
-			sect_crs = (const unchar *)msg_sect[curr_sect].iov_base;
+			sect_crs =
+			  (const unchar __user *)msg_sect[curr_sect].iov_base;
 		}
 
 		if (sect_rest < fragm_rest)
diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index ced60e2..37cfb57 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -93,8 +93,8 @@ int tipc_msg_build(struct tipc_msg *hdr, struct iovec const *msg_sect,
 	skb_copy_to_linear_data(*buf, hdr, hsz);
 	for (res = 1, cnt = 0; res && (cnt < num_sect); cnt++) {
 		skb_copy_to_linear_data_offset(*buf, pos,
-					       msg_sect[cnt].iov_base,
-					       msg_sect[cnt].iov_len);
+			(const void __force *)msg_sect[cnt].iov_base,
+			msg_sect[cnt].iov_len);
 		pos += msg_sect[cnt].iov_len;
 	}
 	if (likely(res))
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 6cc7ddd..0ff921d 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -338,7 +338,7 @@ static int release(struct socket *sock)
 		buf = __skb_dequeue(&sk->sk_receive_queue);
 		if (buf == NULL)
 			break;
-		if (TIPC_SKB_CB(buf)->handle != 0)
+		if (TIPC_SKB_CB(buf)->handle != NULL)
 			kfree_skb(buf);
 		else {
 			if ((sock->state == SS_CONNECTING) ||
@@ -1368,7 +1368,7 @@ static u32 filter_rcv(struct sock *sk, struct sk_buff *buf)
 		return TIPC_ERR_OVERLOAD;
 
 	/* Enqueue message */
-	TIPC_SKB_CB(buf)->handle = 0;
+	TIPC_SKB_CB(buf)->handle = NULL;
 	__skb_queue_tail(&sk->sk_receive_queue, buf);
 	skb_set_owner_r(buf, sk);
 
@@ -1691,7 +1691,7 @@ restart:
 		/* Disconnect and send a 'FIN+' or 'FIN-' message to peer */
 		buf = __skb_dequeue(&sk->sk_receive_queue);
 		if (buf) {
-			if (TIPC_SKB_CB(buf)->handle != 0) {
+			if (TIPC_SKB_CB(buf)->handle != NULL) {
 				kfree_skb(buf);
 				goto restart;
 			}
-- 
1.7.9.5

^ permalink raw reply related


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