* [PATCH v3 net-next 01/27] net: add adj_list to save only neighbours
From: Veaceslav Falico @ 2013-09-17 0:46 UTC (permalink / raw)
To: netdev
Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
Alexander Duyck, Cong Wang
In-Reply-To: <1379378812-18346-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:
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 | 219 ++++++++++++++++++++++++++++------------
4 files changed, 179 insertions(+), 80 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 91f179d..c3dcc6b 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 39e5b1c..72bdb8b 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 041b42a..2a944e5 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 5c713f2..f69a8ab 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,30 +4382,47 @@ struct netdev_adjacent {
static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
struct net_device *adj_dev,
- bool upper)
+ bool upper, bool neighbour)
{
struct netdev_adjacent *adj;
- struct list_head *dev_list;
+ struct list_head *adj_list;
- dev_list = upper ? &dev->upper_dev_list : &dev->lower_dev_list;
+ if (neighbour)
+ adj_list = upper ? &dev->adj_list.upper :
+ &dev->adj_list.lower;
+ else
+ adj_list = upper ? &dev->all_adj_list.upper :
+ &dev->all_adj_list.lower;
- 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)
+static struct netdev_adjacent *__netdev_all_upper_find(struct net_device *dev,
+ struct net_device *udev)
+{
+ return __netdev_find_adj(dev, udev, true, false);
+}
+
+static struct netdev_adjacent *__netdev_all_lower_find(struct net_device *dev,
+ struct net_device *ldev)
{
- return __netdev_find_adj(dev, udev, true);
+ return __netdev_find_adj(dev, ldev, false, false);
}
-static inline struct netdev_adjacent *__netdev_find_lower(struct net_device *dev,
- struct net_device *ldev)
+static struct netdev_adjacent *__netdev_upper_find(struct net_device *dev,
+ struct net_device *udev)
{
- return __netdev_find_adj(dev, ldev, false);
+ return __netdev_find_adj(dev, udev, true, true);
+}
+
+static struct netdev_adjacent *__netdev_lower_find(struct net_device *dev,
+ struct net_device *ldev)
+{
+ return __netdev_find_adj(dev, ldev, false, true);
}
/**
@@ -4425,7 +4439,7 @@ bool netdev_has_upper_dev(struct net_device *dev,
{
ASSERT_RTNL();
- return __netdev_find_upper(dev, upper_dev);
+ return __netdev_all_upper_find(dev, upper_dev);
}
EXPORT_SYMBOL(netdev_has_upper_dev);
@@ -4440,7 +4454,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);
@@ -4457,10 +4471,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;
@@ -4468,15 +4482,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;
@@ -4484,14 +4498,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
@@ -4504,7 +4518,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;
@@ -4517,11 +4531,12 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
bool neighbour, bool master,
bool upper)
{
- struct netdev_adjacent *adj;
+ struct netdev_adjacent *adj, *neigh = NULL;
- adj = __netdev_find_adj(dev, adj_dev, upper);
+ adj = __netdev_find_adj(dev, adj_dev, upper, false);
if (adj) {
+ /* we cannot insert a neighbour device twice */
BUG_ON(neighbour);
adj->ref_nr++;
return 0;
@@ -4533,58 +4548,103 @@ 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);
+
+ if (neighbour) {
+ neigh = kmalloc(sizeof(*neigh), GFP_KERNEL);
+ if (!neigh) {
+ kfree(adj);
+ return -ENOMEM;
+ }
+ neigh->dev = adj_dev;
+ neigh->master = master;
+ neigh->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, upper ? "upper" : "lower", dev->name,
adj_dev->name);
+ if (neigh)
+ pr_debug("dev_hold for %s, because of %s link added from %s to %s (neighbour)\n",
+ adj_dev->name, upper ? "upper" : "lower", dev->name,
+ adj_dev->name);
if (!upper) {
- list_add_tail_rcu(&adj->list, &dev->lower_dev_list);
+ if (neigh)
+ list_add_tail_rcu(&neigh->list,
+ &dev->adj_list.lower);
+ list_add_tail_rcu(&adj->list, &dev->all_adj_list.lower);
return 0;
}
/* Ensure that master upper link is always the first item in list. */
- if (master)
- list_add_rcu(&adj->list, &dev->upper_dev_list);
- else
- list_add_tail_rcu(&adj->list, &dev->upper_dev_list);
+ if (master) {
+ if (neigh)
+ list_add_rcu(&neigh->list,
+ &dev->adj_list.upper);
+ list_add_rcu(&adj->list, &dev->all_adj_list.upper);
+ } else {
+ if (neigh)
+ list_add_tail_rcu(&neigh->list,
+ &dev->adj_list.upper);
+ list_add_tail_rcu(&adj->list, &dev->all_adj_list.upper);
+ }
return 0;
}
-static inline int __netdev_upper_dev_insert(struct net_device *dev,
- struct net_device *udev,
- bool master, bool neighbour)
+static 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);
}
-static inline int __netdev_lower_dev_insert(struct net_device *dev,
- struct net_device *ldev,
- bool neighbour)
+static 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);
}
void __netdev_adjacent_dev_remove(struct net_device *dev,
- struct net_device *adj_dev, bool upper)
+ struct net_device *adj_dev, bool upper,
+ bool is_neigh)
{
- struct netdev_adjacent *adj;
+ struct netdev_adjacent *adj, *neighbour;
- if (upper)
- adj = __netdev_find_upper(dev, adj_dev);
- else
- adj = __netdev_find_lower(dev, adj_dev);
+ if (upper) {
+ adj = __netdev_all_upper_find(dev, adj_dev);
+ neighbour = __netdev_upper_find(dev, adj_dev);
+ } else {
+ adj = __netdev_all_lower_find(dev, adj_dev);
+ neighbour = __netdev_lower_find(dev, adj_dev);
+ }
- if (!adj)
+ if (!adj) {
+ pr_err("tried to remove %s device %s from %s\n",
+ upper ? "upper" : "lower", dev->name, adj_dev->name);
BUG();
+ }
+
+ if (is_neigh) {
+ BUG_ON(!neighbour || neighbour->ref_nr > 1);
+ pr_debug("dev_put for %s, because of %s link removed from %s to %s (neighbour)\n",
+ adj_dev->name, upper ? "upper" : "lower", dev->name,
+ adj_dev->name);
+ list_del_rcu(&neighbour->list);
+ dev_put(adj_dev);
+ kfree_rcu(neighbour, rcu);
+ }
if (adj->ref_nr > 1) {
+ pr_debug("rec_cnt-- for link to %s, because of %s link removed from %s to %s, remains %d\n",
+ adj_dev->name, upper ? "upper" : "lower", dev->name,
+ adj_dev->name, adj->ref_nr-1);
adj->ref_nr--;
return;
}
@@ -4600,13 +4660,25 @@ 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, true, false);
+}
+
+static inline void __netdev_upper_dev_remove_neighbour(struct net_device *dev,
+ struct net_device *udev)
+{
+ return __netdev_adjacent_dev_remove(dev, udev, true, true);
}
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, false, false);
+}
+
+static inline void __netdev_lower_dev_remove_neighbour(struct net_device *dev,
+ struct net_device *ldev)
+{
+ return __netdev_adjacent_dev_remove(dev, ldev, false, true);
}
int __netdev_adjacent_dev_insert_link(struct net_device *dev,
@@ -4648,6 +4720,13 @@ void __netdev_adjacent_dev_unlink(struct net_device *dev,
__netdev_lower_dev_remove(upper_dev, dev);
}
+void __netdev_adjacent_dev_unlink_neighbour(struct net_device *dev,
+ struct net_device *upper_dev)
+{
+ __netdev_upper_dev_remove_neighbour(dev, upper_dev);
+ __netdev_lower_dev_remove_neighbour(upper_dev, dev);
+}
+
static int __netdev_upper_dev_link(struct net_device *dev,
struct net_device *upper_dev, bool master)
@@ -4661,10 +4740,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_all_upper_find(upper_dev, dev))
return -EBUSY;
- if (__netdev_find_upper(dev, upper_dev))
+ if (__netdev_all_upper_find(dev, upper_dev))
return -EEXIST;
if (master && netdev_master_upper_dev_get(dev))
@@ -4675,12 +4754,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;
@@ -4688,14 +4769,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;
@@ -4706,7 +4791,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);
@@ -4716,7 +4801,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);
@@ -4727,8 +4812,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);
@@ -4737,7 +4822,7 @@ rollback_mesh:
break;
}
- __netdev_adjacent_dev_unlink(dev, upper_dev);
+ __netdev_adjacent_dev_unlink_neighbour(dev, upper_dev);
return ret;
}
@@ -4791,23 +4876,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);
@@ -6069,8 +6154,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 v3 net-next 03/27] net: uninline netdev neighbour functions
From: Veaceslav Falico @ 2013-09-17 0:46 UTC (permalink / raw)
To: netdev
Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
Alexander Duyck
In-Reply-To: <1379378812-18346-1-git-send-email-vfalico@redhat.com>
They don't give almost any speed/size advantage, however it's really
useful to have them in the backtrace.
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:
v2 -> v3:
No change.
v1 -> v2:
No changes.
RFC -> v1:
New patch.
net/core/dev.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 75eabef..d277081 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4684,25 +4684,25 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
kfree_rcu(adj, rcu);
}
-static inline void __netdev_upper_dev_remove(struct net_device *dev,
- struct net_device *udev)
+static void __netdev_upper_dev_remove(struct net_device *dev,
+ struct net_device *udev)
{
return __netdev_adjacent_dev_remove(dev, udev, true, false);
}
-static inline void __netdev_upper_dev_remove_neighbour(struct net_device *dev,
+static void __netdev_upper_dev_remove_neighbour(struct net_device *dev,
struct net_device *udev)
{
return __netdev_adjacent_dev_remove(dev, udev, true, true);
}
-static inline void __netdev_lower_dev_remove(struct net_device *dev,
- struct net_device *ldev)
+static void __netdev_lower_dev_remove(struct net_device *dev,
+ struct net_device *ldev)
{
return __netdev_adjacent_dev_remove(dev, ldev, false, false);
}
-static inline void __netdev_lower_dev_remove_neighbour(struct net_device *dev,
+static void __netdev_lower_dev_remove_neighbour(struct net_device *dev,
struct net_device *ldev)
{
return __netdev_adjacent_dev_remove(dev, ldev, false, true);
@@ -4727,15 +4727,15 @@ int __netdev_adjacent_dev_insert_link(struct net_device *dev,
return 0;
}
-static inline int __netdev_adjacent_dev_link(struct net_device *dev,
- struct net_device *udev)
+static int __netdev_adjacent_dev_link(struct net_device *dev,
+ struct net_device *udev)
{
return __netdev_adjacent_dev_insert_link(dev, udev, false, false);
}
-static inline int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
- struct net_device *udev,
- bool master)
+static int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
+ struct net_device *udev,
+ bool master)
{
return __netdev_adjacent_dev_insert_link(dev, udev, master, true);
}
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 02/27] net: add RCU variant to search for netdev_adjacent link
From: Veaceslav Falico @ 2013-09-17 0:46 UTC (permalink / raw)
To: netdev
Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
Alexander Duyck, Cong Wang
In-Reply-To: <1379378812-18346-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 only one function that will be
used further, other functions can be added easily afterwards, if anyone
would need them.
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:
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 | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index f69a8ab..75eabef 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4380,6 +4380,33 @@ struct netdev_adjacent {
struct rcu_head rcu;
};
+static struct netdev_adjacent *__netdev_find_adj_rcu(struct net_device *dev,
+ struct net_device *adj_dev,
+ bool upper, bool neighbour)
+{
+ struct netdev_adjacent *adj;
+ struct list_head *adj_list;
+
+ if (neighbour)
+ adj_list = upper ? &dev->adj_list.upper :
+ &dev->adj_list.lower;
+ else
+ adj_list = upper ? &dev->all_adj_list.upper :
+ &dev->all_adj_list.lower;
+
+ list_for_each_entry_rcu(adj, adj_list, list) {
+ if (adj->dev == adj_dev)
+ return adj;
+ }
+ return NULL;
+}
+
+static struct netdev_adjacent *__netdev_lower_find_rcu(struct net_device *dev,
+ struct net_device *ldev)
+{
+ return __netdev_find_adj_rcu(dev, ldev, false, true);
+}
+
static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
struct net_device *adj_dev,
bool upper, bool neighbour)
--
1.8.4
^ permalink raw reply related
* Re: [PATCH 1/1] net: race condition when removing virtual net_device
From: Eric W. Biederman @ 2013-09-17 0:25 UTC (permalink / raw)
To: Francesco Ruggeri
Cc: David S. Miller, Eric Dumazet, Jiri Pirko, Alexander Duyck,
Cong Wang, netdev
In-Reply-To: <CA+HUmGih9akzhpRrb_0WEapi4jGcuSV8qO==QeRWHoqnxzFyng@mail.gmail.com>
Francesco Ruggeri <fruggeri@aristanetworks.com> writes:
> On Mon, Sep 16, 2013 at 3:45 AM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>
>> I believe the weird reordering of veth devices is solved or largely
>> solved by:
>>
>> commit f45a5c267da35174e22cec955093a7513dc1623d
>
> I looked at d0e2c55e and f45a5c26 in the past and I do not think they
> are related to the problem I see, which I think is more related to the
> infrastructure. See more below.
If everything is in the same batch they definitely relate, as it stops
refiling one of the devices past the point where it's loopback device
is destroyed.
It is not the fundamental problem but it is related and those fixes.
>>> and this is the sequence after pushing the loopback_devs to the tail
>>> of net_todo_list:
>>>
>>> unregister_netdevice_queue: v0 (ns ffff8800379f8000)
>>> unregister_netdevice_queue: v1 (ns ffff8800378c0b00)
>>> unregister_netdevice_queue: lo (ns ffff8800379f8000)
>>> unregister_netdevice_queue: v1 (ns ffff8800378c0b00)
>>> unregister_netdevice_queue: v0 (ns ffff8800379f8000)
>>> unregister_netdevice_queue: lo (ns ffff8800378c0b00)
>>> unregister_netdevice_many: lo (ns ffff8800379f8000) v1 (ns
>>> ffff8800378c0b00) v0 (ns ffff8800379f8000) lo (ns ffff8800378c0b00)
>>> netdev_run_todo: v1 (ns ffff8800378c0b00) v0 (ns ffff8800379f8000) lo
>>> (ns ffff8800379f8000) lo (ns ffff8800378c0b00)
>>
>> I am scratching my head. That isn't what I would expect from putting
>> loopback devices at the end of the list.
>
> The way to read it is as follows:
> unregister_netdevice_many gets the sequence lo0 v1 v0 lo1 (lo0 is the
> occurrence of "lo" with the same namespace as v0, etc.).
> As the interfaces are unlisted the loopback_devs are queued at the end
> of net_todo_list, so when netdev_run_todo executes net_todo_list is v1
> v0 lo0 lo1.
My reading jumbled the unregister_netdevice_many line and the
netdev_run_todo line. So I was seeing v0, lo, v1, v0, lo, lo.
But since both loop devices are now at the end your that part of your
patch looks ok.
>> Even more I am not comfortable with any situation where preserving the
>> order in which the devices are unregistered is not sufficient to make
>> things work correctly. That quickly gets into fragile layering
>> problems, and I don't know how anyone would be able to maintain that.
>>
>
> I agree with your general statement, but unfortunately the order in
> which the interfaces are unregistered is not preserved even in the
> current code, since dev_close_many already rearranges them based on
> whether they are UP or not.
Ugh. That is a bug.
I have sent a patch to solve this by simply not reordering the device
deletions as that is easier to think about. And more robust if we
happen to care about anything besides the loopback device. Reording
happens so seldom I can't imagine it being tested properly.
I have also sent a patch to set loopback_dev to NULL so that it is more
likely we will see your class of bugs.
I think I see bugs with the handling of rtmsg_ifinfo, and
netpoll_rx_enable/disable in dev_close_many as well. But those are
significantly less serious issues.
If you could verify that my patch to dev_close solves the ordering
issues you were seeing I would appreciate that.
>> I still don't see a better solution than dev_change_net_namespace,
>> for the network devices that have this problem. Network devices are not
>> supposed to be findable after the dance at the start of cleanup_net.
>>
>
> I think the problem we are seeing is an infrastructure one. If I can restate it:
>
> 1) When destroying a namespace all net_devices in it should be
> disposed of before any non-device pernet subsystems exit. The existing
> code tries to do this but it does not take into consideration
> net_devices that may have been unlisted from the namespace by another
> process which is still in the process of destroying them (specifically
> in netdev_run_todo/netdev_wait_allrefs).
The existing code makes the assumption that is not possible to find
a network device and manipulate it outside the network namespace in
any way (outside of the network namespace cleanup methods) after the
network namespace reference count has dropped to 0 and the network
namespace is no longer in the network namespace list.
That assumption is clearly not true for veth pairs, vlans, macvlans and
the like, and it is an infrastructure problem.
The question is what is the most robust comprehensible and maintinable
fix? (That doesn't penalize the fast path of course).
> There is also another issue, separate but related (my diffs did not
> try to address it):
>
> 2) dev_change_net_namespace does not have the equivalent of
> netdev_wait_allrefs. If rebroadcasts are in fact needed when
> destroying a net_device then the same should apply when the net_device
> is moved out of a namespace. With existing code a net_device can be
> moved to a different namespace before its refcount drops to 0. Any
> later broadcasts will not trigger any action in the original namespace
> where they should have occurred.
Not having the broadcast in dev_change_net_namespace is not a bug. The
repeated broadcast is most definitely there for hysterical raisins, and
with a good audit of dev_hold/dev_put call sites can most definitely be
removed. At a practical level the repeated broadcast is just acting as
a hammer and saying to subsystems you messed up now let this device go.
Let go! Let go! Let go! And there is a slight hope that something will
let go when told multiple times.
> I suspect that this is not catastrophic but its effects could be
> subtle. This is another reason why I would avoid using
> dev_change_net_namespace as a driver level solution for 1) above,
> besides the fact that we would have to apply it to all drivers
> affected by this (veth, vlan, macvlan, maybe more).
We don't avoid the rebroadcast and wait, with dev_change_net_namespace,
they are just performed in a different network namespace. So if
something is wrong we will know.
>> It might be possible to actually build asynchronous network device
>> unregistration and opportunistick batching. Aka cleanup network devices
>> much like we clean up network namespaces now, and by funnelling them all
>> into a single threaded work queue that would probably stop the races, as
>> well as improve performance. I don't have the energy to do that right
>> now unfortunately :(
>>
>
> That would probably solve it, but as you suggest it will take a
> considerable amount of work and code re-structuring.
And it may be worth it for the speed up you get when destroying 384 mlag
ports.
Eric
^ permalink raw reply
* [PATCH net 5/5] sfc: Reinitialise and re-validate datapath caps after MC reboot
From: Ben Hutchings @ 2013-09-17 0:13 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1379376611.1945.11.camel@bwh-desktop.uk.level5networks.com>
After an MC reboot, the datapath may be running a different firmware
variant and have different capabilities. It is critical that we know
the current capabilities so that we can pass valid flags to
MC_CMD_INIT_EVQ.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/ef10.c | 10 ++++++++++
drivers/net/ethernet/sfc/nic.h | 3 +++
2 files changed, 13 insertions(+)
diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index a4956b8..9f18ae9 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -343,6 +343,13 @@ static int efx_ef10_init_nic(struct efx_nic *efx)
struct efx_ef10_nic_data *nic_data = efx->nic_data;
int rc;
+ if (nic_data->must_check_datapath_caps) {
+ rc = efx_ef10_init_datapath_caps(efx);
+ if (rc)
+ return rc;
+ nic_data->must_check_datapath_caps = false;
+ }
+
if (nic_data->must_realloc_vis) {
/* We cannot let the number of VIs change now */
rc = efx_ef10_alloc_vis(efx, nic_data->n_allocated_vis,
@@ -711,6 +718,9 @@ static int efx_ef10_mcdi_poll_reboot(struct efx_nic *efx)
nic_data->must_restore_filters = true;
nic_data->rx_rss_context = EFX_EF10_RSS_CONTEXT_INVALID;
+ /* The datapath firmware might have been changed */
+ nic_data->must_check_datapath_caps = true;
+
/* MAC statistics have been cleared on the NIC; clear the local
* statistic that we update with efx_update_diff_stat().
*/
diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h
index 4b1e188..fda29d3 100644
--- a/drivers/net/ethernet/sfc/nic.h
+++ b/drivers/net/ethernet/sfc/nic.h
@@ -400,6 +400,8 @@ enum {
* @rx_rss_context: Firmware handle for our RSS context
* @stats: Hardware statistics
* @workaround_35388: Flag: firmware supports workaround for bug 35388
+ * @must_check_datapath_caps: Flag: @datapath_caps needs to be revalidated
+ * after MC reboot
* @datapath_caps: Capabilities of datapath firmware (FLAGS1 field of
* %MC_CMD_GET_CAPABILITIES response)
*/
@@ -413,6 +415,7 @@ struct efx_ef10_nic_data {
u32 rx_rss_context;
u64 stats[EF10_STAT_COUNT];
bool workaround_35388;
+ bool must_check_datapath_caps;
u32 datapath_caps;
};
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net 4/5] sfc: Clean up validation of datapath capabilities
From: Ben Hutchings @ 2013-09-17 0:13 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1379376611.1945.11.camel@bwh-desktop.uk.level5networks.com>
Rename efx_ef10_init_capabilities() to the more specific
efx_ef10_init_datapath_caps().
Stop accepting short responses to MC_CMD_GET_CAPABILITIES; we
don't need to support pre-production firmware.
Move the check for RX prefix support from efx_ef10_probe() into
efx_ef10_init_datapath_caps() and use consistent error messages
for missing TSO support and missing RX prefix support.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/ef10.c | 41 ++++++++++++++++++++++-------------------
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 80a6eea..a4956b8 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -94,7 +94,7 @@ static unsigned int efx_ef10_mem_map_size(struct efx_nic *efx)
return resource_size(&efx->pci_dev->resource[EFX_MEM_BAR]);
}
-static int efx_ef10_init_capabilities(struct efx_nic *efx)
+static int efx_ef10_init_datapath_caps(struct efx_nic *efx)
{
MCDI_DECLARE_BUF(outbuf, MC_CMD_GET_CAPABILITIES_OUT_LEN);
struct efx_ef10_nic_data *nic_data = efx->nic_data;
@@ -107,16 +107,27 @@ static int efx_ef10_init_capabilities(struct efx_nic *efx)
outbuf, sizeof(outbuf), &outlen);
if (rc)
return rc;
+ if (outlen < sizeof(outbuf)) {
+ netif_err(efx, drv, efx->net_dev,
+ "unable to read datapath firmware capabilities\n");
+ return -EIO;
+ }
- if (outlen >= sizeof(outbuf)) {
- nic_data->datapath_caps =
- MCDI_DWORD(outbuf, GET_CAPABILITIES_OUT_FLAGS1);
- if (!(nic_data->datapath_caps &
- (1 << MC_CMD_GET_CAPABILITIES_OUT_TX_TSO_LBN))) {
- netif_err(efx, drv, efx->net_dev,
- "Capabilities don't indicate TSO support.\n");
- return -ENODEV;
- }
+ nic_data->datapath_caps =
+ MCDI_DWORD(outbuf, GET_CAPABILITIES_OUT_FLAGS1);
+
+ if (!(nic_data->datapath_caps &
+ (1 << MC_CMD_GET_CAPABILITIES_OUT_TX_TSO_LBN))) {
+ netif_err(efx, drv, efx->net_dev,
+ "current firmware does not support TSO\n");
+ return -ENODEV;
+ }
+
+ if (!(nic_data->datapath_caps &
+ (1 << MC_CMD_GET_CAPABILITIES_OUT_RX_PREFIX_LEN_14_LBN))) {
+ netif_err(efx, probe, efx->net_dev,
+ "current firmware does not support an RX prefix\n");
+ return -ENODEV;
}
return 0;
@@ -217,21 +228,13 @@ static int efx_ef10_probe(struct efx_nic *efx)
if (rc)
goto fail3;
- rc = efx_ef10_init_capabilities(efx);
+ rc = efx_ef10_init_datapath_caps(efx);
if (rc < 0)
goto fail3;
efx->rx_packet_len_offset =
ES_DZ_RX_PREFIX_PKTLEN_OFST - ES_DZ_RX_PREFIX_SIZE;
- if (!(nic_data->datapath_caps &
- (1 << MC_CMD_GET_CAPABILITIES_OUT_RX_PREFIX_LEN_14_LBN))) {
- netif_err(efx, probe, efx->net_dev,
- "current firmware does not support an RX prefix\n");
- rc = -ENODEV;
- goto fail3;
- }
-
rc = efx_mcdi_port_get_number(efx);
if (rc < 0)
goto fail3;
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net 3/5] sfc: Reset derived rx_bad_bytes statistic when EF10 MC is rebooted
From: Ben Hutchings @ 2013-09-17 0:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1379376611.1945.11.camel@bwh-desktop.uk.level5networks.com>
If the MC reboots then the stats it reports to us will have been
reset. We need to reset ours to get efx_update_diff_stat() working
properly.
(This is a re-run of commit 876be083b669 'sfc: Reset driver's
MAC stats after MC reboot seen'.)
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/ef10.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 357a6e5..80a6eea 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -708,6 +708,11 @@ static int efx_ef10_mcdi_poll_reboot(struct efx_nic *efx)
nic_data->must_restore_filters = true;
nic_data->rx_rss_context = EFX_EF10_RSS_CONTEXT_INVALID;
+ /* MAC statistics have been cleared on the NIC; clear the local
+ * statistic that we update with efx_update_diff_stat().
+ */
+ nic_data->stats[EF10_STAT_rx_bad_bytes] = 0;
+
return -EIO;
}
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net 2/5] sfc: Disable PTP on EF10 until we're ready to handle inline RX timestamps
From: Ben Hutchings @ 2013-09-17 0:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1379376611.1945.11.camel@bwh-desktop.uk.level5networks.com>
Unlike Siena where timestamping is provided by a peripheral, EF10
delivers RX timestamps in the packet prefix. However the driver
doesn't yet support this.
We are also creating a PHC device for each EF10 function, even though
the clock is really shared between all of them.
Disable hardware PTP/timestamping support until it's complete.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/ef10.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 5f42313..357a6e5 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -260,8 +260,6 @@ static int efx_ef10_probe(struct efx_nic *efx)
if (rc)
goto fail3;
- efx_ptp_probe(efx);
-
return 0;
fail3:
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net 1/5] sfc: Minimal support for 40G link speed
From: Ben Hutchings @ 2013-09-17 0:11 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1379376611.1945.11.camel@bwh-desktop.uk.level5networks.com>
Accept and handle 40G link events.
Accept ethtool link settings of speed == 40000 && duplex, and set the
appropriate MCDI PHY capability.
This does not include reporting of 40G media types, as those have not
yet been assigned numbers in the MCDI protocol.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/Kconfig | 2 +-
drivers/net/ethernet/sfc/mcdi_port.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig
index 8b71525..0889212 100644
--- a/drivers/net/ethernet/sfc/Kconfig
+++ b/drivers/net/ethernet/sfc/Kconfig
@@ -7,7 +7,7 @@ config SFC
select I2C_ALGOBIT
select PTP_1588_CLOCK
---help---
- This driver supports 10-gigabit Ethernet cards based on
+ This driver supports 10/40-gigabit Ethernet cards based on
the Solarflare SFC4000, SFC9000-family and SFC9100-family
controllers.
diff --git a/drivers/net/ethernet/sfc/mcdi_port.c b/drivers/net/ethernet/sfc/mcdi_port.c
index 8d33da6..7b6be61 100644
--- a/drivers/net/ethernet/sfc/mcdi_port.c
+++ b/drivers/net/ethernet/sfc/mcdi_port.c
@@ -556,6 +556,7 @@ static int efx_mcdi_phy_set_settings(struct efx_nic *efx, struct ethtool_cmd *ec
case 100: caps = 1 << MC_CMD_PHY_CAP_100FDX_LBN; break;
case 1000: caps = 1 << MC_CMD_PHY_CAP_1000FDX_LBN; break;
case 10000: caps = 1 << MC_CMD_PHY_CAP_10000FDX_LBN; break;
+ case 40000: caps = 1 << MC_CMD_PHY_CAP_40000FDX_LBN; break;
default: return -EINVAL;
}
} else {
@@ -841,6 +842,7 @@ static unsigned int efx_mcdi_event_link_speed[] = {
[MCDI_EVENT_LINKCHANGE_SPEED_100M] = 100,
[MCDI_EVENT_LINKCHANGE_SPEED_1G] = 1000,
[MCDI_EVENT_LINKCHANGE_SPEED_10G] = 10000,
+ [MCDI_EVENT_LINKCHANGE_SPEED_40G] = 40000,
};
void efx_mcdi_process_link_change(struct efx_nic *efx, efx_qword_t *ev)
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Pull request: sfc 2013-09-17
From: Ben Hutchings @ 2013-09-17 0:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
The following changes since commit ba39767288661c01ac7797b6b981f73b6513cf55:
Merge branch 'qlcnic' (2013-08-31 22:35:26 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc.git sfc-3.12
for you to fetch changes up to a915ccc9f2c80aa4c329262d89f93ea16d8792c9:
sfc: Reinitialise and re-validate datapath caps after MC reboot (2013-09-11 15:29:53 +0100)
Some bug fixes and future-proofing for the recently added SFC9120
support:
1. Minimal support for the 40G configuration.
2. Disable the incomplete PTP/hardware timestamping support.
3. Reset MAC stats properly after a firmware upgrade.
4. Re-check the datapath firmware capabilities after the controller is
reset.
Ben.
----------------------------------------------------------------
Ben Hutchings (5):
sfc: Minimal support for 40G link speed
sfc: Disable PTP on EF10 until we're ready to handle inline RX timestamps
sfc: Reset derived rx_bad_bytes statistic when EF10 MC is rebooted
sfc: Clean up validation of datapath capabilities
sfc: Reinitialise and re-validate datapath caps after MC reboot
drivers/net/ethernet/sfc/Kconfig | 2 +-
drivers/net/ethernet/sfc/ef10.c | 58 +++++++++++++++++++++++-------------
drivers/net/ethernet/sfc/mcdi_port.c | 2 ++
drivers/net/ethernet/sfc/nic.h | 3 ++
4 files changed, 43 insertions(+), 22 deletions(-)
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH net-next] net loopback: Set loopback_dev to NULL when freed
From: Eric W. Biederman @ 2013-09-16 23:52 UTC (permalink / raw)
To: David S. Miller
Cc: Eric Dumazet, Jiri Pirko, Alexander Duyck, Cong Wang, netdev,
Francesco Ruggeri
In-Reply-To: <CA+HUmGih9akzhpRrb_0WEapi4jGcuSV8qO==QeRWHoqnxzFyng@mail.gmail.com>
It has recently turned up that we have a number of long standing bugs
in the network stack cleanup code with use of the loopback device
after it has been freed that have not turned up because in most cases
the storage allocated to the loopback device is not reused, when those
accesses happen.
Set looback_dev to NULL to trigger oopses instead of silent data corrupt
when we hit this class of bug.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
drivers/net/loopback.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index fcbf680..a17d85a 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -146,6 +146,7 @@ static int loopback_dev_init(struct net_device *dev)
static void loopback_dev_free(struct net_device *dev)
{
+ dev_net(dev)->loopback_dev = NULL;
free_percpu(dev->lstats);
free_netdev(dev);
}
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH 2/8 V2] rtlwifi: rtl8192de: Fix smatch warnings in rtl8192de/hw.c
From: Kalle Valo @ 2013-09-16 21:44 UTC (permalink / raw)
To: Larry Finger
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1379357722-17687-3-git-send-email-Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org> writes:
> Smatch lists the following:
> CHECK drivers/net/wireless/rtlwifi/rtl8192de/hw.c
> drivers/net/wireless/rtlwifi/rtl8192de/hw.c:1200 rtl92de_set_qos() info: ignoring unreachable code.
> drivers/net/wireless/rtlwifi/rtl8192de/hw.c:1200 rtl92de_set_qos() info: ignoring unreachable code.
>
> Dead code is commented out. It has not been deleted in case I find a need for
> it later.
We should not have any commented out code. It's better to remove it and
if it's ever needed it can be found from the git history.
--
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH] mwifiex: Remove casting the return value which is a void pointer
From: Bing Zhao @ 2013-09-16 21:26 UTC (permalink / raw)
To: Jingoo Han, 'John W. Linville'
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
'David Miller'
In-Reply-To: <004d01cead1d$3058dcf0$910a96d0$%han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Hi Jingoo,
Thanks for the patch.
> Casting the return value which is a void pointer is redundant.
> The conversion from void pointer to any other pointer type is
> guaranteed by the C programming language.
>
> Signed-off-by: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Acked-by: Bing Zhao <bzhao-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Thanks,
Bing
> ---
> drivers/net/wireless/mwifiex/pcie.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH stable] ipv6: restrict neighbor entry creation to output flow
From: Jiri Pirko @ 2013-09-16 21:21 UTC (permalink / raw)
To: David Miller; +Cc: hannes, mleitner, netdev, dbanerje, yoshfuji
In-Reply-To: <20130909.132900.704139320855868470.davem@davemloft.net>
Mon, Sep 09, 2013 at 07:29:00PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Mon, 9 Sep 2013 14:17:19 +0200
>
>> When do you plan to push this to stable maintainers?
>
>Jiri, it's in the -stable queue and that means I will send it at an
>appropriate time.
>
>I'm working on a submission at the moment, but every extra query like
>your's that I have to answer takes up time I could be spending on it.
I'm sorry Dave. I'm just worried. I don't want this patch to get lost.
I expected this patch to be part of:
http://git.kernel.org/cgit/linux/kernel/git/bwh/linux-3.2.y-queue.git/commit/?id=dadcb7672a124159630f1f756dea8323bc4976bd
But it is not there.
I'm sorry again :/
Jiri
^ permalink raw reply
* Re: [patch 1/4] drivers/net/ethernet/ibm/ehea/ehea_main.c: add alias entry for portN properties
From: Thadeu Lima de Souza Cascardo @ 2013-09-16 21:01 UTC (permalink / raw)
To: David Miller; +Cc: akpm, netdev, ohering, jeffm, jslaby
In-Reply-To: <20130916205543.GA14221@oc0268524204.ibm.com>
On Mon, Sep 16, 2013 at 05:55:43PM -0300, Thadeu Lima de Souza Cascardo wrote:
> On Fri, Sep 13, 2013 at 07:58:57PM -0400, David Miller wrote:
> > From: akpm@linux-foundation.org
> > Date: Fri, 13 Sep 2013 14:52:01 -0700
> >
> > > From: Olaf Hering <ohering@suse.com>
> > > Subject: drivers/net/ethernet/ibm/ehea/ehea_main.c: add alias entry for portN properties
> > >
> > > Use separate table for alias entries in the ehea module, otherwise the
> > > probe() function will operate on the separate ports instead of the
> > > lhea-"root" entry of the device-tree
> > >
> > > Addresses https://bugzilla.novell.com/show_bug.cgi?id=435215
> > >
> > > Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> > > Signed-off-by: Olaf Hering <ohering@suse.com>
> > > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > > Cc: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
> > > Cc: "David S. Miller" <davem@davemloft.net>
> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> >
> > This can definitely have consequences and in particular potentially cause
> > a device to not get probed properly.
> >
> > Therefore I want an ehea driver maintainer to review and ACK this before
> > I apply it.
> >
> > Thanks.
> >
>
> After taking a glance at the patch, it doesn't seem to change probing,
> since it keeps the same table for the driver itself. It seems this only
> changes alias, so the driver will be loaded under some device-tree
> layouts not currently matched by the current alias.
>
> That last part is the one that bothers me. I still need to find a system
> where the current modalias won't work and this patch is needed. I'll see
> if I can put more effort into that and find a system as described in the
> bug.
>
> Regards.
> Thadeu Cascardo.
Sorry for the noise. I just checked again the bug, and had to read
between the lines that this issue might happen with the generation of
initrd, when the scripts check for /sys/class/net/eth0/device/modalias,
which links to the port device at
/sys/devices/ibmebus/23c00400.lhea/port0/.
I think that should be clarified in the log message. Besides that, since
this has also been in the field for a long time:
Acked-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Regards.
Cascardo.
^ permalink raw reply
* Re: [patch 1/4] drivers/net/ethernet/ibm/ehea/ehea_main.c: add alias entry for portN properties
From: Thadeu Lima de Souza Cascardo @ 2013-09-16 20:55 UTC (permalink / raw)
To: David Miller; +Cc: akpm, netdev, ohering, jeffm, jslaby
In-Reply-To: <20130913.195857.1842211951469893139.davem@davemloft.net>
On Fri, Sep 13, 2013 at 07:58:57PM -0400, David Miller wrote:
> From: akpm@linux-foundation.org
> Date: Fri, 13 Sep 2013 14:52:01 -0700
>
> > From: Olaf Hering <ohering@suse.com>
> > Subject: drivers/net/ethernet/ibm/ehea/ehea_main.c: add alias entry for portN properties
> >
> > Use separate table for alias entries in the ehea module, otherwise the
> > probe() function will operate on the separate ports instead of the
> > lhea-"root" entry of the device-tree
> >
> > Addresses https://bugzilla.novell.com/show_bug.cgi?id=435215
> >
> > Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> > Signed-off-by: Olaf Hering <ohering@suse.com>
> > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > Cc: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>
> This can definitely have consequences and in particular potentially cause
> a device to not get probed properly.
>
> Therefore I want an ehea driver maintainer to review and ACK this before
> I apply it.
>
> Thanks.
>
After taking a glance at the patch, it doesn't seem to change probing,
since it keeps the same table for the driver itself. It seems this only
changes alias, so the driver will be loaded under some device-tree
layouts not currently matched by the current alias.
That last part is the one that bothers me. I still need to find a system
where the current modalias won't work and this patch is needed. I'll see
if I can put more effort into that and find a system as described in the
bug.
Regards.
Thadeu Cascardo.
^ permalink raw reply
* Re: [PATCH v2.39 7/7] datapath: Add basic MPLS support to kernel
From: Ben Pfaff @ 2013-09-16 20:46 UTC (permalink / raw)
To: Jesse Gross
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, Ravi K, netdev,
Isaku Yamahata
In-Reply-To: <CAEP_g=8FBQPZ=C6G39YdHRzG57m5MqfXSZFAX2S_KLHRwfzc2w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Sep 16, 2013 at 03:38:21PM -0500, Jesse Gross wrote:
> - Ben, do you want to take over for the userspace portions of the
> series on the assumption that the above comments can be fixed fairly
> easily?
Yes, that's fine. Simon, I guess that you are looking at Jesse's
comments about mpls_depth from earlier?
^ permalink raw reply
* Re: [PATCH v2.39 7/7] datapath: Add basic MPLS support to kernel
From: Jesse Gross @ 2013-09-16 20:38 UTC (permalink / raw)
To: Simon Horman, Ben Pfaff, Pravin Shelar
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev, Ravi K,
Isaku Yamahata
In-Reply-To: <1378711207-29890-8-git-send-email-horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
On Mon, Sep 9, 2013 at 12:20 AM, Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org> wrote:
> diff --git a/datapath/actions.c b/datapath/actions.c
> index 6741d81..2335014 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> +/* Push MPLS after the ethernet header. */
> +static int push_mpls(struct sk_buff *skb,
> + const struct ovs_action_push_mpls *mpls)
[...]
> + /* update mac_len for subsequent pop_mpls actions. */
> + skb->mac_len += VLAN_HLEN;
Is this supposed to be part of put_vlan()?
[...]
> + if (skb->ip_summed == CHECKSUM_COMPLETE)
> + skb->csum = csum_add(skb->csum, csum_partial(new_mpls_lse,
> + MPLS_HLEN, 0));
> +
> + hdr = (struct ethhdr *)(skb_mac_header(skb));
I think you could simplify this by using eth_hdr().
> @@ -616,6 +736,13 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb)
> goto out_loop;
> }
>
> + /* Needed to initialise inner protocol on kernels older
> + * than v3.11 where skb->inner_protocol is not present
> + * and compatibility code uses the OVS_CB(skb) to store
> + * the inner protocol.
> + */
> + ovs_skb_set_inner_protocol(skb, skb->protocol);
The comment makes it sound like this code should just be deleted when
upstreaming. However, I believe that we still need to initialize this
field, right? Is this the best place do it or should it be conditional
on adding a first MPLS header? (i.e. what happens if inner_protocol is
already set and the packet simply passes through OVS?)
> diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
> index 215a47e..69f24d1 100644
> --- a/datapath/vport-netdev.c
> +++ b/datapath/vport-netdev.c
> @@ -287,8 +291,17 @@ static int netdev_send(struct vport *vport, struct sk_buff *skb)
> + inner_protocol = ovs_skb_get_inner_protocol(skb);
> + if (eth_p_mpls(skb->protocol) && !eth_p_mpls(inner_protocol))
> + mpls = true;
> +
> + if (vlan_tx_tag_present(skb) && !dev_supports_vlan_tx(skb->dev))
> + vlan = true;
A couple of thoughts on this:
- I'm not sure that the check on inner_protocol is right since I
don't think it will be set to an MPLS EtherType in any real situation.
I think you can just drop it.
- Can we simplify the loop by just inserting the vlan tag (if
necessary), calling skb_gso_segment(), and sending the packet? I think
it should be possible now that our skb_gso_segment() backport is more
robust and it would make things easier to read.
- Can we have some kind of version check for MPLS, similar to what we
do for VLAN, so that we don't call into this on kernels >= 3.11?
These are all pretty targeted comments though, as was the one in the
previous kernel patch so I am basically happy with this overall.
To move forward with this:
- Pravin, would you mind double checking the GSO compat code?
- Ben, do you want to take over for the userspace portions of the
series on the assumption that the above comments can be fixed fairly
easily?
^ permalink raw reply
* PATCH: periodictimer for mrp.c
From: Noel Burton-Krahn @ 2013-09-16 20:32 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]
(originally to "David S. Miller" <davem@davemloft.net>, CC'ed to
netdev. Resent in text/plain)
I ran into a problem when testing MVRP support in net/mrp.c: MRP
would transmit JoinIn messages before the network interface was fully
up, the packets would be lost, and MRP would never retry. It looks
like the MRP implementation was missing the periodictimer defined in
[802.1Q-2011] to retry if packets get lost. The attached patch patch
adds the periodictimer.
Note: As far as I can tell in [802.1Q-2011], the periodic timer never
turns off. It fires every second, causing MRP to bounce from state QA
to AA and back. You might think that would stop when the registrar
responds with an JoinIn, but there's no state in the MRP state table
to ignore the periodic timer after getting a reply. The result is MRP
sends a JoinIn message every second. This may not be desirable, but
it's what the spec requires.
[802.1Q-2011]
http://standards.ieee.org/findstds/standard/802.1Q-2011.html
Does this look OK to you?
--
Noel
[-- Attachment #2: mrp-periodictimer.patch --]
[-- Type: application/octet-stream, Size: 2955 bytes --]
diff --git a/include/net/mrp.h b/include/net/mrp.h
index 4fbf02a..0f7558b 100644
--- a/include/net/mrp.h
+++ b/include/net/mrp.h
@@ -112,6 +112,7 @@ struct mrp_applicant {
struct mrp_application *app;
struct net_device *dev;
struct timer_list join_timer;
+ struct timer_list periodic_timer;
spinlock_t lock;
struct sk_buff_head queue;
diff --git a/net/802/mrp.c b/net/802/mrp.c
index 1eb05d8..97566ec 100644
--- a/net/802/mrp.c
+++ b/net/802/mrp.c
@@ -24,6 +24,11 @@
static unsigned int mrp_join_time __read_mostly = 200;
module_param(mrp_join_time, uint, 0644);
MODULE_PARM_DESC(mrp_join_time, "Join time in ms (default 200ms)");
+
+static unsigned int mrp_periodic_time __read_mostly = 1000;
+module_param(mrp_periodic_time, uint, 0644);
+MODULE_PARM_DESC(mrp_periodic_time, "Periodic time in ms (default 1s)");
+
MODULE_LICENSE("GPL");
static const u8
@@ -595,6 +600,43 @@ static void mrp_join_timer(unsigned long data)
mrp_join_timer_arm(app);
}
+static void mrp_periodic_timer_arm(struct mrp_applicant *app)
+{
+ unsigned long delay;
+
+ delay = (u64)msecs_to_jiffies(mrp_periodic_time);
+ mod_timer(&app->periodic_timer, jiffies + delay);
+}
+
+
+/*
+ Added periodic timer from [802.1Q-2011] to retry if MRP loses
+ messages. MRP used to lose JoinIn messages and never retry if it
+ sent messages before the interface becomes ready.
+
+ The periodic timer from the 802.1Q spec never turns off. It fires
+ every second, causing MRP to bounce from state QA to AA and back.
+ You might think that would stop when the registrar responds with an
+ JoinIn, but there's no state in the MRP state table to ignore the
+ periodic timer after getting a reply. The result is MRP sends a
+ JoinIn message every second. This may not be desirable, but it's
+ what the spec requires.
+
+ [802.1Q-2011]
+ http://standards.ieee.org/findstds/standard/802.1Q-2011.html
+*/
+static void mrp_periodic_timer(unsigned long data)
+{
+ struct mrp_applicant *app = (struct mrp_applicant *)data;
+
+ spin_lock(&app->lock);
+ mrp_mad_event(app, MRP_EVENT_PERIODIC);
+ mrp_pdu_queue(app);
+ spin_unlock(&app->lock);
+
+ mrp_periodic_timer_arm(app);
+}
+
static int mrp_pdu_parse_end_mark(struct sk_buff *skb, int *offset)
{
__be16 endmark;
@@ -845,6 +887,8 @@ int mrp_init_applicant(struct net_device *dev, struct mrp_application *appl)
rcu_assign_pointer(dev->mrp_port->applicants[appl->type], app);
setup_timer(&app->join_timer, mrp_join_timer, (unsigned long)app);
mrp_join_timer_arm(app);
+ setup_timer(&app->periodic_timer, mrp_periodic_timer, (unsigned long)app);
+ mrp_periodic_timer_arm(app);
return 0;
err3:
@@ -870,6 +914,7 @@ void mrp_uninit_applicant(struct net_device *dev, struct mrp_application *appl)
* all pending messages before the applicant is gone.
*/
del_timer_sync(&app->join_timer);
+ del_timer_sync(&app->periodic_timer);
spin_lock_bh(&app->lock);
mrp_mad_event(app, MRP_EVENT_TX);
^ permalink raw reply related
* Re: [PATCH 1/1] net: race condition when removing virtual net_device
From: Francesco Ruggeri @ 2013-09-16 20:30 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David S. Miller, Eric Dumazet, Jiri Pirko, Alexander Duyck,
Cong Wang, netdev
In-Reply-To: <87six5kpu5.fsf@xmission.com>
On Mon, Sep 16, 2013 at 3:45 AM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> I believe the weird reordering of veth devices is solved or largely
> solved by:
>
> commit f45a5c267da35174e22cec955093a7513dc1623d
I looked at d0e2c55e and f45a5c26 in the past and I do not think they
are related to the problem I see, which I think is more related to the
infrastructure. See more below.
>
>> and this is the sequence after pushing the loopback_devs to the tail
>> of net_todo_list:
>>
>> unregister_netdevice_queue: v0 (ns ffff8800379f8000)
>> unregister_netdevice_queue: v1 (ns ffff8800378c0b00)
>> unregister_netdevice_queue: lo (ns ffff8800379f8000)
>> unregister_netdevice_queue: v1 (ns ffff8800378c0b00)
>> unregister_netdevice_queue: v0 (ns ffff8800379f8000)
>> unregister_netdevice_queue: lo (ns ffff8800378c0b00)
>> unregister_netdevice_many: lo (ns ffff8800379f8000) v1 (ns
>> ffff8800378c0b00) v0 (ns ffff8800379f8000) lo (ns ffff8800378c0b00)
>> netdev_run_todo: v1 (ns ffff8800378c0b00) v0 (ns ffff8800379f8000) lo
>> (ns ffff8800379f8000) lo (ns ffff8800378c0b00)
>
> I am scratching my head. That isn't what I would expect from putting
> loopback devices at the end of the list.
The way to read it is as follows:
unregister_netdevice_many gets the sequence lo0 v1 v0 lo1 (lo0 is the
occurrence of "lo" with the same namespace as v0, etc.).
As the interfaces are unlisted the loopback_devs are queued at the end
of net_todo_list, so when netdev_run_todo executes net_todo_list is v1
v0 lo0 lo1.
>
> Even more I am not comfortable with any situation where preserving the
> order in which the devices are unregistered is not sufficient to make
> things work correctly. That quickly gets into fragile layering
> problems, and I don't know how anyone would be able to maintain that.
>
I agree with your general statement, but unfortunately the order in
which the interfaces are unregistered is not preserved even in the
current code, since dev_close_many already rearranges them based on
whether they are UP or not.
If I delete a namespace with only lo and v0 this is the order in which
they are released depending on their state. If lo is DOWN then it will
be processed before v0 in netdev_run_todo.
====== lo down, v0 down
unregister_netdevice_queue: v0 (ns ffff880037bb0b00)
unregister_netdevice_queue: lo (ns ffff880037bb0b00)
unregister_netdevice_many: v0 (ns ffff880037bb0b00) lo (ns ffff880037bb0b00)
netdev_run_todo: lo (ns ffff880037bb0b00) v0 (ns ffff880037bb0b00)
====== lo up, v0 down
unregister_netdevice_queue: v0 (ns ffff8800378a9600)
unregister_netdevice_queue: lo (ns ffff8800378a9600)
unregister_netdevice_many: v0 (ns ffff8800378a9600) lo (ns ffff8800378a9600)
netdev_run_todo: v0 (ns ffff8800378a9600) lo (ns ffff8800378a9600)
====== lo down, v0 up
unregister_netdevice_queue: v0 (ns ffff880037bb0b00)
unregister_netdevice_queue: lo (ns ffff880037bb0b00)
unregister_netdevice_many: v0 (ns ffff880037bb0b00) lo (ns ffff880037bb0b00)
netdev_run_todo: lo (ns ffff880037bb0b00) v0 (ns ffff880037bb0b00)
====== lo up, v0 up
unregister_netdevice_queue: v0 (ns ffff8800378a9600)
unregister_netdevice_queue: lo (ns ffff8800378a9600)
unregister_netdevice_many: v0 (ns ffff8800378a9600) lo (ns ffff8800378a9600)
netdev_run_todo: v0 (ns ffff8800378a9600) lo (ns ffff8800378a9600)
> I still don't see a better solution than dev_change_net_namespace,
> for the network devices that have this problem. Network devices are not
> supposed to be findable after the dance at the start of cleanup_net.
>
I think the problem we are seeing is an infrastructure one. If I can restate it:
1) When destroying a namespace all net_devices in it should be
disposed of before any non-device pernet subsystems exit. The existing
code tries to do this but it does not take into consideration
net_devices that may have been unlisted from the namespace by another
process which is still in the process of destroying them (specifically
in netdev_run_todo/netdev_wait_allrefs).
There is also another issue, separate but related (my diffs did not
try to address it):
2) dev_change_net_namespace does not have the equivalent of
netdev_wait_allrefs. If rebroadcasts are in fact needed when
destroying a net_device then the same should apply when the net_device
is moved out of a namespace. With existing code a net_device can be
moved to a different namespace before its refcount drops to 0. Any
later broadcasts will not trigger any action in the original namespace
where they should have occurred.
I suspect that this is not catastrophic but its effects could be
subtle. This is another reason why I would avoid using
dev_change_net_namespace as a driver level solution for 1) above,
besides the fact that we would have to apply it to all drivers
affected by this (veth, vlan, macvlan, maybe more).
> It might be possible to actually build asynchronous network device
> unregistration and opportunistick batching. Aka cleanup network devices
> much like we clean up network namespaces now, and by funnelling them all
> into a single threaded work queue that would probably stop the races, as
> well as improve performance. I don't have the energy to do that right
> now unfortunately :(
>
That would probably solve it, but as you suggest it will take a
considerable amount of work and code re-structuring.
Francesco
^ permalink raw reply
* Re: [PATCH] ethernet/arc/arc_emac: Fix huge delays in large file copies
From: greg Kroah-Hartman @ 2013-09-16 19:40 UTC (permalink / raw)
To: Vineet Gupta
Cc: David Miller, netdev, Alexey.Brodkin, linux-kernel,
stable@vger.kernel.org
In-Reply-To: <52369A94.5060900@synopsys.com>
On Mon, Sep 16, 2013 at 11:13:48AM +0530, Vineet Gupta wrote:
> On 09/10/2013 11:57 AM, Vineet Gupta wrote:
> > On 09/05/2013 11:55 PM, David Miller wrote:
> >> From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
> >> Date: Wed, 4 Sep 2013 17:17:15 +0530
> >>
> >>> copying large files to a NFS mounted host was taking absurdly large
> >>> time.
> >>>
> >>> Turns out that TX BD reclaim had a sublte bug.
> >>>
> >>> Loop starts off from @txbd_dirty cursor and stops when it hits a BD
> >>> still in use by controller. However when it stops it needs to keep the
> >>> cursor at that very BD to resume scanning in next iteration. However it
> >>> was erroneously incrementing the cursor, causing the next scan(s) to
> >>> fail too, unless the BD chain was completely drained out.
> >>>
> >>> [ARCLinux]$ ls -l -sh /disk/log.txt
> >>> 17976 -rw-r--r-- 1 root root 17.5M Sep /disk/log.txt
> >>>
> >>> ========== Before =====================
> >>> [ARCLinux]$ time cp /disk/log.txt /mnt/.
> >>> real 31m 7.95s
> >>> user 0m 0.00s
> >>> sys 0m 0.10s
> >>>
> >>> ========== After =====================
> >>> [ARCLinux]$ time cp /disk/log.txt /mnt/.
> >>> real 0m 24.33s
> >>> user 0m 0.00s
> >>> sys 0m 0.19s
> >>>
> >>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> >>
> >> Applied.
> >>
> >
> > Hi Greg,
> >
> > This needs a stable backport (3.11).
> > Mainline commit 27082ee1b92f4d41e78b85
> >
> > Thx,
> > -Vineet
>
> Hi Greg,
>
> I didn't spot this one in your stable-queue for 3.11.
> Please apply.
Network patches for the stable tree needs to go through the networking
maintainer. Please let them know about this and they will forward it on
to me if needed.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 5/8 V2] [PATCH 5/7: rtlwifi: Fix smatch warning in pci.c
From: Sergei Shtylyov @ 2013-09-16 19:11 UTC (permalink / raw)
To: Larry Finger; +Cc: linville, linux-wireless, netdev
In-Reply-To: <5237562D.4080509@cogentembedded.com>
On 09/16/2013 11:04 PM, Sergei Shtylyov wrote:
>> Smatch reports the following:
>> CHECK drivers/net/wireless/rtlwifi/pci.c
>> drivers/net/wireless/rtlwifi/pci.c:739 _rtl_pci_rx_interrupt() warn:
>> assigning (-98) to unsigned variable 'stats.noise'
>> This variable is not used and is removed.
> It is not a variable but a structure field initializer you're removing.
Meant to also write that your subject is spoiled but forgot. :-)
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 4/8 V2] rtlwifi: rtl8192_common: Fix smatch errors and warnings in rtl8192c/dm_common.c
From: Sergei Shtylyov @ 2013-09-16 19:10 UTC (permalink / raw)
To: Larry Finger; +Cc: linville, linux-wireless, netdev
In-Reply-To: <523756DB.90507@lwfinger.net>
Hello.
On 09/16/2013 11:07 PM, Larry Finger wrote:
>>> Smatch lists the following:
>>> CHECK drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
>>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:551 rtl92c_dm_pwdb_monitor()
>>> info: ignoring unreachable code.
>>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:551 rtl92c_dm_pwdb_monitor()
>>> info: ignoring unreachable code.
>>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:870
>>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>>> 'txpwr_level' 2 <= 2
>>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:870
>>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>>> 'txpwr_level' 2 <= 2
>>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:882
>>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>>> 'txpwr_level' 2 <= 2
>>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:883
>>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>>> 'txpwr_level' 2 <= 2
>>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:891
>>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>>> 'txpwr_level' 2 <= 2
>>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:892
>>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>>> 'txpwr_level' 2 <= 2
>>> The unreachable code message is fixed by commenting out the code that follows
>>> a return.
>> You've commented out the whole function body, where is the *return* you're
>> talking about?
> The return is in the middle of the function body just after the variable
> declarations. It does not show in the diff listing, but it is there. What
> should I do?
Just describe what you really did, I guess.
> Larry
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 6/8 V2] rtlwifi: Fix smatch warnings in usb.c
From: Sergei Shtylyov @ 2013-09-16 19:07 UTC (permalink / raw)
To: Larry Finger
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1379357722-17687-7-git-send-email-Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
On 09/16/2013 10:55 PM, Larry Finger wrote:
> Smatch displays the following:
> CHECK drivers/net/wireless/rtlwifi/usb.c
> drivers/net/wireless/rtlwifi/usb.c:458 _rtl_usb_rx_process_agg() warn: assigning (-98) to unsigned variable 'stats.noise'
> drivers/net/wireless/rtlwifi/usb.c:503 _rtl_usb_rx_process_noagg() warn: assigning (-98) to unsigned variable 'stats.noise'
> drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: ignoring unreachable code.
> drivers/net/wireless/rtlwifi/usb.c:596 _rtl_rx_get_padding() info: ignoring unreachable code.
> The negative number to an unsigned quantity is fixed by removing the variable
> as it is no longer used.
You're removing only structure field initializer, not a variable.
> The unreachable code info is fixed by including the
> appropriate section inside #ifdef .. #endif constructions.
> Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
> ---
> drivers/net/wireless/rtlwifi/usb.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> diff --git a/drivers/net/wireless/rtlwifi/usb.c b/drivers/net/wireless/rtlwifi/usb.c
> index e56778c..60cb0b4 100644
> --- a/drivers/net/wireless/rtlwifi/usb.c
> +++ b/drivers/net/wireless/rtlwifi/usb.c
> @@ -455,7 +455,6 @@ static void _rtl_usb_rx_process_agg(struct ieee80211_hw *hw,
> struct ieee80211_rx_status rx_status = {0};
> struct rtl_stats stats = {
> .signal = 0,
> - .noise = -98,
> .rate = 0,
> };
>
> @@ -498,7 +497,6 @@ static void _rtl_usb_rx_process_noagg(struct ieee80211_hw *hw,
> struct ieee80211_rx_status rx_status = {0};
> struct rtl_stats stats = {
> .signal = 0,
> - .noise = -98,
> .rate = 0,
> };
>
> @@ -582,12 +580,15 @@ static void _rtl_rx_work(unsigned long param)
> static unsigned int _rtl_rx_get_padding(struct ieee80211_hdr *hdr,
> unsigned int len)
> {
> +#if NET_IP_ALIGN != 0
> unsigned int padding = 0;
> +#endif
>
> /* make function no-op when possible */
> - if (NET_IP_ALIGN == 0 || len < sizeof(*hdr))
> + if (NET_IP_ALIGN == 0 || len < sizeof(struct ieee80211_hdr))
Hm, I thought you were going to remove this collateral change.
> return 0;
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 4/8 V2] rtlwifi: rtl8192_common: Fix smatch errors and warnings in rtl8192c/dm_common.c
From: Larry Finger @ 2013-09-16 19:07 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linville, linux-wireless, netdev
In-Reply-To: <523755D1.2010003@cogentembedded.com>
On 09/16/2013 02:02 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 09/16/2013 10:55 PM, Larry Finger wrote:
>
>> Smatch lists the following:
>> CHECK drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c
>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:551 rtl92c_dm_pwdb_monitor()
>> info: ignoring unreachable code.
>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:551 rtl92c_dm_pwdb_monitor()
>> info: ignoring unreachable code.
>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:870
>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>> 'txpwr_level' 2 <= 2
>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:870
>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>> 'txpwr_level' 2 <= 2
>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:882
>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>> 'txpwr_level' 2 <= 2
>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:883
>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>> 'txpwr_level' 2 <= 2
>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:891
>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>> 'txpwr_level' 2 <= 2
>> drivers/net/wireless/rtlwifi/rtl8192c/dm_common.c:892
>> rtl92c_dm_txpower_tracking_callback_thermalmeter() error: buffer overflow
>> 'txpwr_level' 2 <= 2
>
>> The unreachable code message is fixed by commenting out the code that follows
>> a return.
>
> You've commented out the whole function body, where is the *return* you're
> talking about?
The return is in the middle of the function body just after the variable
declarations. It does not show in the diff listing, but it is there. What should
I do?
Larry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox