* Re: [PATCH net-next] mlx4: only pull headers into skb head
From: David Miller @ 2014-09-06 5:35 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, amirv
In-Reply-To: <1409966985.11872.6.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 05 Sep 2014 18:29:45 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> Use the new fancy eth_get_headlen() to pull exactly the headers
> into skb->head.
>
> This speeds up GRE traffic (or more generally tunneled traffuc),
> as GRO can aggregate up to 17 MSS per GRO packet instead of 8.
>
> (Pulling too much data was forcing GRO to keep 2 frags per MSS)
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Looks great, applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH net v1 0/2] amd-xgbe: AMD XGBE driver fixes 2014-09-05
From: David Miller @ 2014-09-06 5:38 UTC (permalink / raw)
To: thomas.lendacky; +Cc: netdev
In-Reply-To: <20140905230224.27896.64932.stgit@tlendack-t1.amdoffice.net>
From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Fri, 5 Sep 2014 18:02:24 -0500
> The following series of patches includes fixes to the driver.
>
> - Proper access to 64 bit management counter registers
> - Enable all management counter registers to generate an interrupt when
> the counter threshold is reached
>
> This patch series is based on net.
Series applied, thanks Tom.
^ permalink raw reply
* [PATCH net-next v1 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Mahesh Bandewar @ 2014-09-06 6:35 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller
Cc: netdev, Mahesh Bandewar, Eric Dumazet, Maciej Zenczykowski
Earlier change to use usable slave array for TLB mode had an additional
performance advantage. So extending the same logic to all other modes
that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
Also consolidating this with the earlier TLB change.
The main idea is to build the usable slaves array in the control path
and use that array for slave selection during xmit operation.
Measured performance in a setup with a bond of 4x1G NICs with 200
instances of netperf for the modes involved (3ad, xor, tlb)
cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
Mode TPS-Before TPS-After
802.3ad : 468,694 493,101
TLB (lb=0): 392,583 392,965
XOR : 475,696 484,517
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
v1:
(a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
the slave that need to be removed.
(b) Freeing of array will assign NULL (to handle bond->down to bond->up
transition gracefully.
(c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
failure.
(d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
will populate the array even if these parameters are not used.
(e) 3AD: Should handle the ad_agg_selection_logic correctly.
drivers/net/bonding/bond_3ad.c | 79 ++++-----------------
drivers/net/bonding/bond_alb.c | 45 +-----------
drivers/net/bonding/bond_alb.h | 8 ---
drivers/net/bonding/bond_main.c | 150 ++++++++++++++++++++++++++++++++++++----
drivers/net/bonding/bonding.h | 8 +++
5 files changed, 161 insertions(+), 129 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index ee2c73a9de39..ba05c83d5d83 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1579,6 +1579,8 @@ static void ad_agg_selection_logic(struct aggregator *agg)
__disable_port(port);
}
}
+ if (bond_update_slave_arr(bond, NULL))
+ pr_err("Failed to build slave-array for 3ad mode.\n");
}
/* if the selected aggregator is of join individuals
@@ -1717,6 +1719,8 @@ static void ad_enable_collecting_distributing(struct port *port)
port->actor_port_number,
port->aggregator->aggregator_identifier);
__enable_port(port);
+ if (bond_update_slave_arr(port->slave->bond, NULL))
+ pr_err("Failed to build slave-array for 3ad mode.\n");
}
}
@@ -1733,6 +1737,8 @@ static void ad_disable_collecting_distributing(struct port *port)
port->actor_port_number,
port->aggregator->aggregator_identifier);
__disable_port(port);
+ if (bond_update_slave_arr(port->slave->bond, NULL))
+ pr_err("Failed to build slave-array for 3ad mode.\n");
}
}
@@ -1917,6 +1923,9 @@ void bond_3ad_unbind_slave(struct slave *slave)
__update_lacpdu_from_port(port);
ad_lacpdu_send(port);
+ if (bond_update_slave_arr(bond, slave))
+ pr_err("Failed to build slave-array for 3AD mode.\n");
+
/* check if this aggregator is occupied */
if (aggregator->lag_ports) {
/* check if there are other ports related to this aggregator
@@ -2311,6 +2320,9 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
*/
port->sm_vars |= AD_PORT_BEGIN;
+ if (bond_update_slave_arr(slave->bond, NULL))
+ pr_err("Failed to build slave-array for 3ad mode.\n");
+
__release_state_machine_lock(port);
}
@@ -2407,73 +2419,6 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
return ret;
}
-int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
-{
- struct bonding *bond = netdev_priv(dev);
- struct slave *slave, *first_ok_slave;
- struct aggregator *agg;
- struct ad_info ad_info;
- struct list_head *iter;
- int slaves_in_agg;
- int slave_agg_no;
- int agg_id;
-
- if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
- netdev_dbg(dev, "__bond_3ad_get_active_agg_info failed\n");
- goto err_free;
- }
-
- slaves_in_agg = ad_info.ports;
- agg_id = ad_info.aggregator_id;
-
- if (slaves_in_agg == 0) {
- netdev_dbg(dev, "active aggregator is empty\n");
- goto err_free;
- }
-
- slave_agg_no = bond_xmit_hash(bond, skb) % slaves_in_agg;
- first_ok_slave = NULL;
-
- bond_for_each_slave_rcu(bond, slave, iter) {
- agg = SLAVE_AD_INFO(slave)->port.aggregator;
- if (!agg || agg->aggregator_identifier != agg_id)
- continue;
-
- if (slave_agg_no >= 0) {
- if (!first_ok_slave && bond_slave_can_tx(slave))
- first_ok_slave = slave;
- slave_agg_no--;
- continue;
- }
-
- if (bond_slave_can_tx(slave)) {
- bond_dev_queue_xmit(bond, skb, slave->dev);
- goto out;
- }
- }
-
- if (slave_agg_no >= 0) {
- netdev_err(dev, "Couldn't find a slave to tx on for aggregator ID %d\n",
- agg_id);
- goto err_free;
- }
-
- /* we couldn't find any suitable slave after the agg_no, so use the
- * first suitable found, if found.
- */
- if (first_ok_slave)
- bond_dev_queue_xmit(bond, skb, first_ok_slave->dev);
- else
- goto err_free;
-
-out:
- return NETDEV_TX_OK;
-err_free:
- /* no suitable interface, frame not sent */
- dev_kfree_skb_any(skb);
- goto out;
-}
-
int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
struct slave *slave)
{
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 73c21e233131..334d92127baf 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -200,7 +200,6 @@ static int tlb_initialize(struct bonding *bond)
static void tlb_deinitialize(struct bonding *bond)
{
struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
- struct tlb_up_slave *arr;
_lock_tx_hashtbl_bh(bond);
@@ -208,10 +207,6 @@ static void tlb_deinitialize(struct bonding *bond)
bond_info->tx_hashtbl = NULL;
_unlock_tx_hashtbl_bh(bond);
-
- arr = rtnl_dereference(bond_info->slave_arr);
- if (arr)
- kfree_rcu(arr, rcu);
}
static long long compute_gap(struct slave *slave)
@@ -1409,39 +1404,9 @@ out:
return NETDEV_TX_OK;
}
-static int bond_tlb_update_slave_arr(struct bonding *bond,
- struct slave *skipslave)
-{
- struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
- struct slave *tx_slave;
- struct list_head *iter;
- struct tlb_up_slave *new_arr, *old_arr;
-
- new_arr = kzalloc(offsetof(struct tlb_up_slave, arr[bond->slave_cnt]),
- GFP_ATOMIC);
- if (!new_arr)
- return -ENOMEM;
-
- bond_for_each_slave(bond, tx_slave, iter) {
- if (!bond_slave_can_tx(tx_slave))
- continue;
- if (skipslave == tx_slave)
- continue;
- new_arr->arr[new_arr->count++] = tx_slave;
- }
-
- old_arr = rtnl_dereference(bond_info->slave_arr);
- rcu_assign_pointer(bond_info->slave_arr, new_arr);
- if (old_arr)
- kfree_rcu(old_arr, rcu);
-
- return 0;
-}
-
int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
- struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
struct ethhdr *eth_data;
struct slave *tx_slave = NULL;
u32 hash_index;
@@ -1462,9 +1427,9 @@ int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
hash_index & 0xFF,
skb->len);
} else {
- struct tlb_up_slave *slaves;
+ struct bond_up_slave *slaves;
- slaves = rcu_dereference(bond_info->slave_arr);
+ slaves = rcu_dereference(bond->slave_arr);
if (slaves && slaves->count)
tx_slave = slaves->arr[hash_index %
slaves->count];
@@ -1733,10 +1698,6 @@ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave)
rlb_clear_slave(bond, slave);
}
- if (bond_is_nondyn_tlb(bond))
- if (bond_tlb_update_slave_arr(bond, slave))
- pr_err("Failed to build slave-array for TLB mode.\n");
-
}
/* Caller must hold bond lock for read */
@@ -1762,7 +1723,7 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
}
if (bond_is_nondyn_tlb(bond)) {
- if (bond_tlb_update_slave_arr(bond, NULL))
+ if (bond_update_slave_arr(bond, NULL))
pr_err("Failed to build slave-array for TLB mode.\n");
}
}
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index aaeac61d03cf..5fc76c01636c 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -139,20 +139,12 @@ struct tlb_slave_info {
*/
};
-struct tlb_up_slave {
- unsigned int count;
- struct rcu_head rcu;
- struct slave *arr[0];
-};
-
struct alb_bond_info {
struct tlb_client_info *tx_hashtbl; /* Dynamically allocated */
spinlock_t tx_hashtbl_lock;
u32 unbalanced_load;
int tx_rebalance_counter;
int lp_counter;
- /* -------- non-dynamic tlb mode only ---------*/
- struct tlb_up_slave __rcu *slave_arr; /* Up slaves */
/* -------- rlb parameters -------- */
int rlb_enabled;
struct rlb_client_info *rx_hashtbl; /* Receive hash table */
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f0f5eab0fab1..43f066539dab 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1413,6 +1413,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
dev_mc_add(slave_dev, lacpdu_multicast);
}
+ if (BOND_MODE(bond) == BOND_MODE_XOR &&
+ bond_update_slave_arr(bond, NULL))
+ pr_err("Failed to build slave-array for XOR mode.\n");
+
res = vlan_vids_add_by_dev(slave_dev, bond_dev);
if (res) {
netdev_err(bond_dev, "Couldn't add bond vlan ids to %s\n",
@@ -1692,6 +1696,11 @@ static int __bond_release_one(struct net_device *bond_dev,
/* Inform AD package of unbinding of slave. */
if (BOND_MODE(bond) == BOND_MODE_8023AD)
bond_3ad_unbind_slave(slave);
+ else if (BOND_MODE(bond) == BOND_MODE_XOR ||
+ bond_is_nondyn_tlb(bond)) {
+ if (bond_update_slave_arr(bond, slave))
+ pr_err("Failed to build slave-array.\n");
+ }
write_unlock_bh(&bond->lock);
@@ -2009,6 +2018,10 @@ static void bond_miimon_commit(struct bonding *bond)
bond_alb_handle_link_change(bond, slave,
BOND_LINK_UP);
+ if (BOND_MODE(bond) == BOND_MODE_XOR &&
+ bond_update_slave_arr(bond, NULL))
+ pr_err("Failed to build slave-array for XOR mode.\n");
+
if (!bond->curr_active_slave ||
(slave == bond->primary_slave))
goto do_failover;
@@ -2037,6 +2050,10 @@ static void bond_miimon_commit(struct bonding *bond)
bond_alb_handle_link_change(bond, slave,
BOND_LINK_DOWN);
+ if (BOND_MODE(bond) == BOND_MODE_XOR &&
+ bond_update_slave_arr(bond, NULL))
+ pr_err("Failed to build slave-array for XOR mode.\n");
+
if (slave == rcu_access_pointer(bond->curr_active_slave))
goto do_failover;
@@ -2500,6 +2517,9 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
if (slave_state_changed) {
bond_slave_state_change(bond);
+ if (BOND_MODE(bond) == BOND_MODE_XOR &&
+ bond_update_slave_arr(bond, NULL))
+ pr_err("Failed to build slave-array for XOR mode.\n");
} else if (do_failover) {
/* the bond_select_active_slave must hold RTNL
* and curr_slave_lock for write.
@@ -2893,11 +2913,14 @@ static int bond_slave_netdev_event(unsigned long event,
if (old_duplex != slave->duplex)
bond_3ad_adapter_duplex_changed(slave);
}
+ if (BOND_MODE(bond) == BOND_MODE_XOR &&
+ bond_update_slave_arr(bond, NULL))
+ pr_err("Failed to build slave-array for XOR mode.\n");
break;
case NETDEV_DOWN:
- /*
- * ... Or is it this?
- */
+ if (BOND_MODE(bond) == BOND_MODE_XOR &&
+ bond_update_slave_arr(bond, NULL))
+ pr_err("Failed to build slave-array for XOR mode.\n");
break;
case NETDEV_CHANGEMTU:
/*
@@ -3143,12 +3166,17 @@ static int bond_open(struct net_device *bond_dev)
bond_3ad_initiate_agg_selection(bond, 1);
}
+ if (BOND_MODE(bond) == BOND_MODE_XOR &&
+ bond_update_slave_arr(bond, NULL))
+ pr_err("Failed to build slave-array for XOR mode.\n");
+
return 0;
}
static int bond_close(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
+ struct bond_up_slave *arr;
bond_work_cancel_all(bond);
bond->send_peer_notif = 0;
@@ -3156,6 +3184,12 @@ static int bond_close(struct net_device *bond_dev)
bond_alb_deinitialize(bond);
bond->recv_probe = NULL;
+ arr = rtnl_dereference(bond->slave_arr);
+ if (arr) {
+ kfree_rcu(arr, rcu);
+ RCU_INIT_POINTER(bond->slave_arr, NULL);
+ }
+
return 0;
}
@@ -3684,15 +3718,108 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
return NETDEV_TX_OK;
}
-/* In bond_xmit_xor() , we determine the output device by using a pre-
- * determined xmit_hash_policy(), If the selected device is not enabled,
- * find the next active slave.
+/* Build the usable slaves array in control path for modes that use xmit-hash
+ * to determine the slave interface -
+ * (a) BOND_MODE_8023AD
+ * (b) BOND_MODE_XOR
+ * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0
*/
-static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
+int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
{
- struct bonding *bond = netdev_priv(bond_dev);
+ struct slave *slave;
+ struct list_head *iter;
+ struct bond_up_slave *new_arr, *old_arr;
+ int slaves_in_agg;
+ int agg_id = 0;
+ int ret = 0;
+
+ new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
+ GFP_ATOMIC);
+ if (!new_arr) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+ struct ad_info ad_info;
- bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb) % bond->slave_cnt);
+ if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
+ pr_debug("bond_3ad_get_active_agg_info failed\n");
+ kfree_rcu(new_arr, rcu);
+ ret = -EINVAL;
+ goto out;
+ }
+ slaves_in_agg = ad_info.ports;
+ agg_id = ad_info.aggregator_id;
+ }
+ bond_for_each_slave(bond, slave, iter) {
+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+ struct aggregator *agg;
+
+ agg = SLAVE_AD_INFO(slave)->port.aggregator;
+ if (!agg || agg->aggregator_identifier != agg_id)
+ continue;
+ }
+ if (!bond_slave_can_tx(slave))
+ continue;
+ if (skipslave == slave)
+ continue;
+ new_arr->arr[new_arr->count++] = slave;
+ }
+
+ old_arr = rcu_dereference_protected(bond->slave_arr,
+ lockdep_rtnl_is_held() ||
+ lockdep_is_held(&bond->lock) ||
+ lockdep_is_held(&bond->curr_slave_lock));
+ rcu_assign_pointer(bond->slave_arr, new_arr);
+ if (old_arr)
+ kfree_rcu(old_arr, rcu);
+
+out:
+ if (ret != 0 && skipslave) {
+ int idx;
+
+ /* Rare situation where caller has asked to skip a specific
+ * slave but allocation failed (most likely!). In this sitation
+ * overwrite the skipslave entry in the array with the last
+ * entry from the array to avoid a situation where the xmit
+ * path may choose this to-be-skipped slave to send a packet
+ * out.
+ */
+ rcu_read_lock();
+ old_arr = rcu_dereference_protected(bond->slave_arr,
+ lockdep_is_held(&bond->lock));
+ for (idx = 0; idx < old_arr->count; idx++) {
+ if (skipslave == old_arr->arr[idx]) {
+ if (idx != old_arr->count - 1)
+ old_arr->arr[idx] =
+ old_arr->arr[old_arr->count-1];
+ old_arr->count--;
+ break;
+ }
+ }
+ rcu_read_unlock();
+ }
+ return ret;
+}
+
+/* Use this Xmit function for 3AD as well as XOR modes. The current
+ * usable slave array is formed in the control path. The xmit function
+ * just calculates hash and sends the packet out.
+ */
+int bond_3ad_xor_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct bonding *bond = netdev_priv(dev);
+ struct slave *slave;
+ struct bond_up_slave *slaves;
+
+ slaves = rcu_dereference(bond->slave_arr);
+ if (slaves && slaves->count) {
+ slave = slaves->arr[bond_xmit_hash(bond, skb) % slaves->count];
+ bond_dev_queue_xmit(bond, skb, slave->dev);
+ } else {
+ dev_kfree_skb_any(skb);
+ atomic_long_inc(&dev->tx_dropped);
+ }
return NETDEV_TX_OK;
}
@@ -3794,12 +3921,11 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
return bond_xmit_roundrobin(skb, dev);
case BOND_MODE_ACTIVEBACKUP:
return bond_xmit_activebackup(skb, dev);
+ case BOND_MODE_8023AD:
case BOND_MODE_XOR:
- return bond_xmit_xor(skb, dev);
+ return bond_3ad_xor_xmit(skb, dev);
case BOND_MODE_BROADCAST:
return bond_xmit_broadcast(skb, dev);
- case BOND_MODE_8023AD:
- return bond_3ad_xmit_xor(skb, dev);
case BOND_MODE_ALB:
return bond_alb_xmit(skb, dev);
case BOND_MODE_TLB:
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index aace510d08d1..4a6195c0de60 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -177,6 +177,12 @@ struct slave {
struct kobject kobj;
};
+struct bond_up_slave {
+ unsigned int count;
+ struct rcu_head rcu;
+ struct slave *arr[0];
+};
+
/*
* Link pseudo-state only used internally by monitors
*/
@@ -196,6 +202,7 @@ struct bonding {
struct slave __rcu *curr_active_slave;
struct slave __rcu *current_arp_slave;
struct slave *primary_slave;
+ struct bond_up_slave __rcu *slave_arr; /* Array of usable slaves */
bool force_primary;
s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
int (*recv_probe)(const struct sk_buff *, struct bonding *,
@@ -527,6 +534,7 @@ const char *bond_slave_link_status(s8 link);
struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
struct net_device *end_dev,
int level);
+int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
#ifdef CONFIG_PROC_FS
void bond_create_proc_entry(struct bonding *bond);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH net-next v1 1/2] bonding: display xmit_hash_policy for non-dynamic-tlb mode
From: Mahesh Bandewar @ 2014-09-06 6:35 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller
Cc: netdev, Mahesh Bandewar, Eric Dumazet, Maciej Zenczykowski
It's trivial fix to display xmit_hash_policy for this new TLB mode
since it uses transmit-hash-poilicy as part of bonding-master info
(/proc/net/bonding/<bonding-interface).
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
v1
Rebase
drivers/net/bonding/bond_procfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index de62c0385dfb..ebcf63a6ddf0 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -84,7 +84,8 @@ static void bond_info_show_master(struct seq_file *seq)
seq_printf(seq, "\n");
if (BOND_MODE(bond) == BOND_MODE_XOR ||
- BOND_MODE(bond) == BOND_MODE_8023AD) {
+ BOND_MODE(bond) == BOND_MODE_8023AD ||
+ bond_is_nondyn_tlb(bond)) {
optval = bond_opt_get_val(BOND_OPT_XMIT_HASH,
bond->params.xmit_policy);
seq_printf(seq, "Transmit Hash Policy: %s (%d)\n",
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* contact me
From: Harley Wang @ 2014-09-06 3:49 UTC (permalink / raw)
I wait for confirmation to transfer US$21.4M to you
__________ Information from ESET Mail Security, version of virus signature database 10226 (20140808) __________
The message was checked by ESET Mail Security.
http://www.eset.com
^ permalink raw reply
* [PATCH net-next 0/3] BPF updates
From: Daniel Borkmann @ 2014-09-06 9:42 UTC (permalink / raw)
To: davem; +Cc: ast, netdev
[ Set applies on top of current net-next but also on top of
Alexei's latest patches. Please see individual patches for
more details. ]
Daniel Borkmann (3):
net: bpf: consolidate JIT binary allocator
net: bpf: arm: address randomize and write protect JIT code
net: bpf: be friendly to kmemcheck
arch/arm/net/bpf_jit_32.c | 34 +++++++++++++++++++++------
arch/mips/net/bpf_jit.c | 2 +-
arch/powerpc/net/bpf_jit_comp.c | 2 +-
arch/s390/net/bpf_jit_comp.c | 47 ++++++++-----------------------------
arch/sparc/net/bpf_jit_comp.c | 2 +-
arch/x86/net/bpf_jit_comp.c | 52 ++++++++++-------------------------------
include/linux/filter.h | 19 ++++++++++++---
kernel/bpf/core.c | 39 +++++++++++++++++++++++++++++++
net/core/filter.c | 2 +-
9 files changed, 108 insertions(+), 91 deletions(-)
--
1.7.11.7
^ permalink raw reply
* [PATCH net-next 2/3] net: bpf: arm: address randomize and write protect JIT code
From: Daniel Borkmann @ 2014-09-06 9:42 UTC (permalink / raw)
To: davem; +Cc: ast, netdev, Mircea Gherzan
In-Reply-To: <1409996567-2170-1-git-send-email-dborkman@redhat.com>
This is the ARM variant for 314beb9bcab ("x86: bpf_jit_comp: secure bpf
jit against spraying attacks").
It is now possible to implement it due to commits 75374ad47c64 ("ARM: mm:
Define set_memory_* functions for ARM") and dca9aa92fc7c ("ARM: add
DEBUG_SET_MODULE_RONX option to Kconfig") which added infrastructure for
this facility.
Thus, this patch makes sure the BPF generated JIT code is marked RO, as
other kernel text sections, and also lets the generated JIT code start
at a pseudo random offset instead on a page boundary. The holes are filled
with illegal instructions.
JIT tested on armv7hl with BPF test suite.
Reference: http://mainisusuallyafunction.blogspot.com/2012/11/attacking-hardened-linux-systems-with.html
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
Cc: Mircea Gherzan <mgherzan@gmail.com>
---
arch/arm/net/bpf_jit_32.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index a76623b..2d1a5b9 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -12,7 +12,6 @@
#include <linux/compiler.h>
#include <linux/errno.h>
#include <linux/filter.h>
-#include <linux/moduleloader.h>
#include <linux/netdevice.h>
#include <linux/string.h>
#include <linux/slab.h>
@@ -174,6 +173,15 @@ static inline bool is_load_to_a(u16 inst)
}
}
+static void jit_fill_hole(void *area, unsigned int size)
+{
+ /* Insert illegal UND instructions. */
+ u32 *ptr, fill_ins = 0xe7ffffff;
+ /* We are guaranteed to have aligned memory. */
+ for (ptr = area; size >= sizeof(u32); size -= sizeof(u32))
+ *ptr++ = fill_ins;
+}
+
static void build_prologue(struct jit_ctx *ctx)
{
u16 reg_set = saved_regs(ctx);
@@ -859,9 +867,11 @@ b_epilogue:
void bpf_jit_compile(struct bpf_prog *fp)
{
+ struct bpf_binary_header *header;
struct jit_ctx ctx;
unsigned tmp_idx;
unsigned alloc_size;
+ u8 *target_ptr;
if (!bpf_jit_enable)
return;
@@ -897,13 +907,15 @@ void bpf_jit_compile(struct bpf_prog *fp)
/* there's nothing after the epilogue on ARMv7 */
build_epilogue(&ctx);
#endif
-
alloc_size = 4 * ctx.idx;
- ctx.target = module_alloc(alloc_size);
- if (unlikely(ctx.target == NULL))
+ header = bpf_jit_binary_alloc(alloc_size, &target_ptr,
+ 4, jit_fill_hole);
+ if (header == NULL)
goto out;
+ ctx.target = (u32 *) target_ptr;
ctx.idx = 0;
+
build_prologue(&ctx);
build_body(&ctx);
build_epilogue(&ctx);
@@ -919,6 +931,7 @@ void bpf_jit_compile(struct bpf_prog *fp)
/* there are 2 passes here */
bpf_jit_dump(fp->len, alloc_size, 2, ctx.target);
+ set_memory_ro((unsigned long)header, header->pages);
fp->bpf_func = (void *)ctx.target;
fp->jited = 1;
out:
@@ -928,8 +941,15 @@ out:
void bpf_jit_free(struct bpf_prog *fp)
{
- if (fp->jited)
- module_free(NULL, fp->bpf_func);
+ unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
+ struct bpf_binary_header *header = (void *)addr;
+
+ if (!fp->jited)
+ goto free_filter;
+
+ set_memory_rw(addr, header->pages);
+ bpf_jit_binary_free(header);
+free_filter:
bpf_prog_unlock_free(fp);
}
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 1/3] net: bpf: consolidate JIT binary allocator
From: Daniel Borkmann @ 2014-09-06 9:42 UTC (permalink / raw)
To: davem; +Cc: ast, netdev, Eric Dumazet, Heiko Carstens, Martin Schwidefsky
In-Reply-To: <1409996567-2170-1-git-send-email-dborkman@redhat.com>
Introduced in commit 314beb9bcabf ("x86: bpf_jit_comp: secure bpf jit
against spraying attacks") and later on replicated in aa2d2c73c21f
("s390/bpf,jit: address randomize and write protect jit code") for
s390 architecture, write protection for BPF JIT images got added and
a random start address of the JIT code, so that it's not on a page
boundary anymore.
Since both use a very similar allocator for the BPF binary header,
we can consolidate this code into the BPF core as it's mostly JIT
independant anyway.
This will also allow for future archs that support DEBUG_SET_MODULE_RONX
to just reuse instead of reimplementing it.
While reviewing the code, I think on s390, the alignment masking
seems not to be correct in it's current form, that is, we make sure
the first instruction starts at an even address as stated by commit
aa2d2c73c21f but masks the start with '& -2' while 2 byte-alignment
should rather be '& ~1'.
JIT tested on x86_64 and s390x with BPF test suite.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Alexei Starovoitov <ast@plumgrid.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
arch/s390/net/bpf_jit_comp.c | 45 ++++++++-------------------------------
arch/x86/net/bpf_jit_comp.c | 50 ++++++++++----------------------------------
include/linux/filter.h | 13 ++++++++++++
kernel/bpf/core.c | 39 ++++++++++++++++++++++++++++++++++
4 files changed, 72 insertions(+), 75 deletions(-)
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index f2833c5..b734f97 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -5,11 +5,9 @@
*
* Author(s): Martin Schwidefsky <schwidefsky@de.ibm.com>
*/
-#include <linux/moduleloader.h>
#include <linux/netdevice.h>
#include <linux/if_vlan.h>
#include <linux/filter.h>
-#include <linux/random.h>
#include <linux/init.h>
#include <asm/cacheflush.h>
#include <asm/facility.h>
@@ -148,6 +146,12 @@ struct bpf_jit {
ret; \
})
+static void bpf_jit_fill_hole(void *area, unsigned int size)
+{
+ /* Fill whole space with illegal instructions */
+ memset(area, 0, size);
+}
+
static void bpf_jit_prologue(struct bpf_jit *jit)
{
/* Save registers and create stack frame if necessary */
@@ -780,38 +784,6 @@ out:
return -1;
}
-/*
- * Note: for security reasons, bpf code will follow a randomly
- * sized amount of illegal instructions.
- */
-struct bpf_binary_header {
- unsigned int pages;
- u8 image[];
-};
-
-static struct bpf_binary_header *bpf_alloc_binary(unsigned int bpfsize,
- u8 **image_ptr)
-{
- struct bpf_binary_header *header;
- unsigned int sz, hole;
-
- /* Most BPF filters are really small, but if some of them fill a page,
- * allow at least 128 extra bytes for illegal instructions.
- */
- sz = round_up(bpfsize + sizeof(*header) + 128, PAGE_SIZE);
- header = module_alloc(sz);
- if (!header)
- return NULL;
- memset(header, 0, sz);
- header->pages = sz / PAGE_SIZE;
- hole = min(sz - (bpfsize + sizeof(*header)), PAGE_SIZE - sizeof(*header));
- /* Insert random number of illegal instructions before BPF code
- * and make sure the first instruction starts at an even address.
- */
- *image_ptr = &header->image[(prandom_u32() % hole) & -2];
- return header;
-}
-
void bpf_jit_compile(struct bpf_prog *fp)
{
struct bpf_binary_header *header = NULL;
@@ -850,7 +822,8 @@ void bpf_jit_compile(struct bpf_prog *fp)
size = prg_len + lit_len;
if (size >= BPF_SIZE_MAX)
goto out;
- header = bpf_alloc_binary(size, &jit.start);
+ header = bpf_jit_binary_alloc(size, &jit.start,
+ 2, bpf_jit_fill_hole);
if (!header)
goto out;
jit.prg = jit.mid = jit.start + prg_len;
@@ -884,7 +857,7 @@ void bpf_jit_free(struct bpf_prog *fp)
goto free_filter;
set_memory_rw(addr, header->pages);
- module_free(NULL, header);
+ bpf_jit_binary_free(header);
free_filter:
bpf_prog_unlock_free(fp);
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 06f8c17..9de0b54 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -8,12 +8,10 @@
* as published by the Free Software Foundation; version 2
* of the License.
*/
-#include <linux/moduleloader.h>
-#include <asm/cacheflush.h>
#include <linux/netdevice.h>
#include <linux/filter.h>
#include <linux/if_vlan.h>
-#include <linux/random.h>
+#include <asm/cacheflush.h>
int bpf_jit_enable __read_mostly;
@@ -109,39 +107,6 @@ static inline void bpf_flush_icache(void *start, void *end)
#define CHOOSE_LOAD_FUNC(K, func) \
((int)K < 0 ? ((int)K >= SKF_LL_OFF ? func##_negative_offset : func) : func##_positive_offset)
-struct bpf_binary_header {
- unsigned int pages;
- /* Note : for security reasons, bpf code will follow a randomly
- * sized amount of int3 instructions
- */
- u8 image[];
-};
-
-static struct bpf_binary_header *bpf_alloc_binary(unsigned int proglen,
- u8 **image_ptr)
-{
- unsigned int sz, hole;
- struct bpf_binary_header *header;
-
- /* Most of BPF filters are really small,
- * but if some of them fill a page, allow at least
- * 128 extra bytes to insert a random section of int3
- */
- sz = round_up(proglen + sizeof(*header) + 128, PAGE_SIZE);
- header = module_alloc(sz);
- if (!header)
- return NULL;
-
- memset(header, 0xcc, sz); /* fill whole space with int3 instructions */
-
- header->pages = sz / PAGE_SIZE;
- hole = min(sz - (proglen + sizeof(*header)), PAGE_SIZE - sizeof(*header));
-
- /* insert a random number of int3 instructions before BPF code */
- *image_ptr = &header->image[prandom_u32() % hole];
- return header;
-}
-
/* pick a register outside of BPF range for JIT internal work */
#define AUX_REG (MAX_BPF_REG + 1)
@@ -206,6 +171,12 @@ static inline u8 add_2reg(u8 byte, u32 dst_reg, u32 src_reg)
return byte + reg2hex[dst_reg] + (reg2hex[src_reg] << 3);
}
+static void jit_fill_hole(void *area, unsigned int size)
+{
+ /* fill whole space with int3 instructions */
+ memset(area, 0xcc, size);
+}
+
struct jit_context {
unsigned int cleanup_addr; /* epilogue code offset */
bool seen_ld_abs;
@@ -959,7 +930,7 @@ void bpf_int_jit_compile(struct bpf_prog *prog)
if (proglen <= 0) {
image = NULL;
if (header)
- module_free(NULL, header);
+ bpf_jit_binary_free(header);
goto out;
}
if (image) {
@@ -969,7 +940,8 @@ void bpf_int_jit_compile(struct bpf_prog *prog)
break;
}
if (proglen == oldproglen) {
- header = bpf_alloc_binary(proglen, &image);
+ header = bpf_jit_binary_alloc(proglen, &image,
+ 1, jit_fill_hole);
if (!header)
goto out;
}
@@ -998,7 +970,7 @@ void bpf_jit_free(struct bpf_prog *fp)
goto free_filter;
set_memory_rw(addr, header->pages);
- module_free(NULL, header);
+ bpf_jit_binary_free(header);
free_filter:
bpf_prog_unlock_free(fp);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 8f82ef3..868764f 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -289,6 +289,11 @@ struct sock_fprog_kern {
struct sock_filter *filter;
};
+struct bpf_binary_header {
+ unsigned int pages;
+ u8 image[];
+};
+
struct bpf_work_struct {
struct bpf_prog *prog;
struct work_struct work;
@@ -358,6 +363,14 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
gfp_t gfp_extra_flags);
void __bpf_prog_free(struct bpf_prog *fp);
+typedef void (*bpf_jit_fill_hole_t)(void *area, unsigned int size);
+
+struct bpf_binary_header *
+bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
+ unsigned int alignment,
+ bpf_jit_fill_hole_t bpf_fill_ill_insns);
+void bpf_jit_binary_free(struct bpf_binary_header *hdr);
+
static inline void bpf_prog_unlock_free(struct bpf_prog *fp)
{
bpf_prog_unlock_ro(fp);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 2c2bfaa..8ee520f 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -20,9 +20,12 @@
* Andi Kleen - Fix a few bad bugs and races.
* Kris Katterjohn - Added many additional checks in bpf_check_classic()
*/
+
#include <linux/filter.h>
#include <linux/skbuff.h>
#include <linux/vmalloc.h>
+#include <linux/random.h>
+#include <linux/moduleloader.h>
#include <asm/unaligned.h>
/* Registers */
@@ -125,6 +128,42 @@ void __bpf_prog_free(struct bpf_prog *fp)
}
EXPORT_SYMBOL_GPL(__bpf_prog_free);
+struct bpf_binary_header *
+bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
+ unsigned int alignment,
+ bpf_jit_fill_hole_t bpf_fill_ill_insns)
+{
+ struct bpf_binary_header *hdr;
+ unsigned int size, hole, start;
+
+ /* Most of BPF filters are really small, but if some of them
+ * fill a page, allow at least 128 extra bytes to insert a
+ * random section of illegal instructions.
+ */
+ size = round_up(proglen + sizeof(*hdr) + 128, PAGE_SIZE);
+ hdr = module_alloc(size);
+ if (hdr == NULL)
+ return NULL;
+
+ /* Fill space with illegal/arch-dep instructions. */
+ bpf_fill_ill_insns(hdr, size);
+
+ hdr->pages = size / PAGE_SIZE;
+ hole = min_t(unsigned int, size - (proglen + sizeof(*hdr)),
+ PAGE_SIZE - sizeof(*hdr));
+ start = (prandom_u32() % hole) & ~(alignment - 1);
+
+ /* Leave a random number of instructions before BPF code. */
+ *image_ptr = &hdr->image[start];
+
+ return hdr;
+}
+
+void bpf_jit_binary_free(struct bpf_binary_header *hdr)
+{
+ module_free(NULL, hdr);
+}
+
/* Base function for offset calculation. Needs to go into .text section,
* therefore keeping it non-static as well; will also be used by JITs
* anyway later on, so do not let the compiler omit it.
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 3/3] net: bpf: be friendly to kmemcheck
From: Daniel Borkmann @ 2014-09-06 9:42 UTC (permalink / raw)
To: davem; +Cc: ast, netdev, Mikulas Patocka
In-Reply-To: <1409996567-2170-1-git-send-email-dborkman@redhat.com>
Reported by Mikulas Patocka, kmemcheck currently barks out a
false positive since we don't have special kmemcheck annotation
for bitfields used in bpf_prog structure.
We currently have jited:1, len:31 and thus when accessing len
while CONFIG_KMEMCHECK enabled, kmemcheck throws a warning that
we're reading uninitialized memory.
As we don't need the whole bit universe for pages member, we
can just split it to u16 and use a bool flag for jited instead
of a bitfield.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
---
arch/arm/net/bpf_jit_32.c | 2 +-
arch/mips/net/bpf_jit.c | 2 +-
arch/powerpc/net/bpf_jit_comp.c | 2 +-
arch/s390/net/bpf_jit_comp.c | 2 +-
arch/sparc/net/bpf_jit_comp.c | 2 +-
arch/x86/net/bpf_jit_comp.c | 2 +-
include/linux/filter.h | 6 +++---
net/core/filter.c | 2 +-
8 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index 2d1a5b9..6b45f64 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -933,7 +933,7 @@ void bpf_jit_compile(struct bpf_prog *fp)
set_memory_ro((unsigned long)header, header->pages);
fp->bpf_func = (void *)ctx.target;
- fp->jited = 1;
+ fp->jited = true;
out:
kfree(ctx.offsets);
return;
diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c
index cfa83cf..0e97ccd 100644
--- a/arch/mips/net/bpf_jit.c
+++ b/arch/mips/net/bpf_jit.c
@@ -1417,7 +1417,7 @@ void bpf_jit_compile(struct bpf_prog *fp)
bpf_jit_dump(fp->len, alloc_size, 2, ctx.target);
fp->bpf_func = (void *)ctx.target;
- fp->jited = 1;
+ fp->jited = true;
out:
kfree(ctx.offsets);
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 40c53ff..cbae2df 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -686,7 +686,7 @@ void bpf_jit_compile(struct bpf_prog *fp)
((u64 *)image)[0] = (u64)code_base;
((u64 *)image)[1] = local_paca->kernel_toc;
fp->bpf_func = (void *)image;
- fp->jited = 1;
+ fp->jited = true;
}
out:
kfree(addrs);
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index b734f97..555f5c7 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -842,7 +842,7 @@ void bpf_jit_compile(struct bpf_prog *fp)
if (jit.start) {
set_memory_ro((unsigned long)header, header->pages);
fp->bpf_func = (void *) jit.start;
- fp->jited = 1;
+ fp->jited = true;
}
out:
kfree(addrs);
diff --git a/arch/sparc/net/bpf_jit_comp.c b/arch/sparc/net/bpf_jit_comp.c
index f7a736b..b2ad9dc 100644
--- a/arch/sparc/net/bpf_jit_comp.c
+++ b/arch/sparc/net/bpf_jit_comp.c
@@ -801,7 +801,7 @@ cond_branch: f_offset = addrs[i + filter[i].jf];
if (image) {
bpf_flush_icache(image, image + proglen);
fp->bpf_func = (void *)image;
- fp->jited = 1;
+ fp->jited = true;
}
out:
kfree(addrs);
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 9de0b54..d56cd1f 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -955,7 +955,7 @@ void bpf_int_jit_compile(struct bpf_prog *prog)
bpf_flush_icache(header, image + proglen);
set_memory_ro((unsigned long)header, header->pages);
prog->bpf_func = (void *)image;
- prog->jited = 1;
+ prog->jited = true;
}
out:
kfree(addrs);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 868764f..4b59ede 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -300,9 +300,9 @@ struct bpf_work_struct {
};
struct bpf_prog {
- u32 pages; /* Number of allocated pages */
- u32 jited:1, /* Is our filter JIT'ed? */
- len:31; /* Number of filter blocks */
+ u16 pages; /* Number of allocated pages */
+ bool jited; /* Is our filter JIT'ed? */
+ u32 len; /* Number of filter blocks */
struct sock_fprog_kern *orig_prog; /* Original BPF program */
struct bpf_work_struct *work; /* Deferred free work struct */
unsigned int (*bpf_func)(const struct sk_buff *skb,
diff --git a/net/core/filter.c b/net/core/filter.c
index fa5b7d0..dfc716f 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -972,7 +972,7 @@ static struct bpf_prog *bpf_prepare_filter(struct bpf_prog *fp)
int err;
fp->bpf_func = NULL;
- fp->jited = 0;
+ fp->jited = false;
err = bpf_check_classic(fp->insns, fp->len);
if (err) {
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next] bridge: Cleanup of unncessary check.
From: Rami Rosen @ 2014-09-06 10:08 UTC (permalink / raw)
To: davem; +Cc: netdev, Rami Rosen
This patch removes an unncessary check in the br_afspec() method of
br_netlink.c.
Signed-off-by: Rami Rosen <ramirose@gmail.com>
---
net/bridge/br_netlink.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index cb5fcf6..823db3a 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -257,9 +257,6 @@ static int br_afspec(struct net_bridge *br,
} else
err = br_vlan_add(br, vinfo->vid, vinfo->flags);
- if (err)
- break;
-
break;
case RTM_DELLINK:
--
1.9.0
^ permalink raw reply related
* Re: [RFC net-next 1/5] bonding: 3ad: use curr_slave_lock instead of bond->lock
From: Nikolay Aleksandrov @ 2014-09-06 10:19 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, vfalico, andy, davem
In-Reply-To: <27301.1409965688@localhost.localdomain>
On 09/06/2014 03:08 AM, Jay Vosburgh wrote:
> Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>
>> On 09/05/2014 10:37 PM, Jay Vosburgh wrote:
>>> Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>>>
>>>> In 3ad mode the only syncing needed by bond->lock is for the wq
>>>> and the recv handler, so change them to use curr_slave_lock.
>>>> There're no locking dependencies here as 3ad doesn't use
>>>> curr_slave_lock at all.
>>>
>>> One subtle aspect of the 3ad locking is that it's not really
>>> using the "read" property of the read lock with regard to the state
>>> machine; it's largely using it as a spin lock, because there is at most
>>> one reader and at most one writer in the full state machine code
>>> (although there are multiple reader possibilities elsewhere). The code
>>> would break if there actually were multiple read-lock holders in the
>>> full state machine or aggregator selection logic simultaneously.
>>>
>>> Because the state machine and incoming LACPDU cases both acquire
>>> the read lock for read, there is a separate per-port "state machine"
>>> spin lock to protect only the per-port fields that LACPDU and periodic
>>> state machine both touch. The incoming LACPDU case doesn't call into
>>> the full state machine, only the RX processing which can't go into agg
>>> selection, so this works.
>>>
>>> The agg selection can be entered via the unbind path or the
>>> periodic state machine (and only these two paths), and relies on the
>>> "one reader max" usage of the read lock to mutex the code paths that may
>>> enter agg selection.
>>>
>>> I suspect that what 3ad may need is a spin lock, not a read
>>> lock, because the multiple reader property isn't really being utilized;
>>> the incoming LACPDU and periodic state machine both acquire the read
>>> lock for read, but then acquire a second per-port spin lock. If the
>>> "big" (bond->lock currently) lock is a spin lock, then the per-port
>>> state machine lock is unnecessary, as the only purpose of the per-port
>>> lock is to mutex the one case that does have multiple readers.
>>>
>>> In actual practice I doubt there are multiple simultaneous
>>> readers very often; the periodic machine runs every 100 ms, but LACPDUs
>>> arrive for each port either every second or every 30 seconds (depending
>>> on admin configuration).
>>>
>>> Since contention on these locks is generally low, we're probably
>>> better off in the long run with something simpler to understand.
>>>
>>> So, what I'm kind of saying here is that this patch isn't a bad
>>> first step, but at least for the 3ad case, removal of the bond->lock
>>> itself doesn't really simplify the locking as much as could be done.
>>>
>>> Thoughts?
>>>
>>> -J
>>>
>>>
>>
>> Hi Jay,
>> That is a very good point, my main idea was to protect __bond_release_one
>> and the machine handler otherwise I'd have removed it altogether. I know
>> that this doesn't improve on the 3ad situation, I did it mostly to get rid
>> of bond->lock first. Going with a spinlock certainly makes sense there as
>> we don't spend much time inside and the contention is not high as you said
>> and would simplify the 3ad code so I like it :-)
>> I will include it in my bond-locking todo list and will post a follow-up
>> once I've cleared the details up as I'm speaking from the top of my head
>> right now, but first I'd like to clean the current lock use especially with
>> regard to curr_slave_lock and bring it to the necessary minimum. In the
>> long run I think that we either might be able to remove curr_slave_lock
>> completely or at least reduce it to ~ 3 places and with the spinlock that
>> you suggested here, we'll be definitely able to remove it from the 3ad code.
>
> I looked into curr_slave_lock some time ago, and as I recall
> there's not much it really protects that's not also covered by RTNL,
> since changes to the active slave are all happening under RTNL these
> days. And that was before the RCU conversion; with the current code,
> I'm not sure the curr_slave_lock is providing much mutexing beyond what
> we get from RTNL.
>
Right, that was one of the main things that drove me to start this, after
reviewing Mahesh's patches I realized most of the callers already had RTNL
and curr_slave_lock looked mostly redundant, and bond->lock completely
redundant.
> There are a couple of special cases, like the TLB rebalance in
> bond_alb_monitor, but that happens once every 10 seconds, and could just
> grab RTNL for this bit:
>
> if (slave == rcu_access_pointer(bond->curr_active_slave)) {
> SLAVE_TLB_INFO(slave).load =
> bond_info->unbalanced_load /
> BOND_TLB_REBALANCE_INTERVAL;
> bond_info->unbalanced_load = 0;
>
> the "unbalanced_load," now that I'm looking at it, might already
> have some race problems since it's now updated outside of bonding locks
> in bond_do_alb_xmit. It'll probably race with multiple bond_do_alb_xmit
> functions running simultaneously as well as the tx rebalance in
> bond_alb_monitor. I think the worst that will happen is that the tx
> traffic load is distributed suboptimally for 10 seconds.
>
Yes, I don't think this needs fixing, but we might revisit it once the rest
of the pieces are in place.
> The rlb_clear_slave case that acquires curr_slave_lock already
> also has RTNL, so I'm not sure that removing the curr_slave_lock will
> have any impact there, either. Many of the other curr_slave_lock
> holders bounce the curr_slave_lock to call into net/core functions (set
> promisc, change MAC, etc), so there's already reliance on RTNL.
>
Yes, rlb_clear_slave is not the problem, the problem if we remove
curr_slave_lock was that both rlb_clear_slave and bond_alb_monitor can be
transmitting packets at the same time or a mac swap might happen triggering
packet transmission while bond_alb_monitor() is also transmitting i.e.
pretty much everything that now gets curr_slave_lock for writing on the ALB
side and transmits could transmit with bond_alb_monitor(), that's what I
was trying to avoid.
> Separately, it also might be possible to combine the various
> per-mode special locks internally into a generic "mode lock" so the alb
> and rlb hashtbl lock and the 802.3ad state machine lock could be a
> single "bond->mode_lock" that mutexes whatever special sauce the active
> mode needs protected. Not sure if that's worth the trouble or not, but
> it seems plausible at first glance.
>
Yes, I had something similar in mind /actually called it sync_lock :)/, but
I only went as far as to use it for a few places that needed syncing beyond
RTNL after curr_slave_lock was removed, you've went one step further and
this certainly looks doable. I'll keep it in mind.
> -J
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
>
^ permalink raw reply
* Re: [PATCH v2] iproute2 tests: Fix problem with test running
From: Vadim Kochan @ 2014-09-06 10:31 UTC (permalink / raw)
To: netdev, Stephen Hemminger; +Cc: Vadim Kochan
In-Reply-To: <1409937036-30712-1-git-send-email-vadim4j@gmail.com>
I will re-send patch v2 with using "sudo -E".
On Fri, Sep 5, 2014 at 8:10 PM, Vadim Kochan <vadim4j@gmail.com> wrote:
> Tests were not allowed to be ran, the following
> issues were fixed:
> - creating the results folder before test running
> - sudo $PREFIX moved before variables definition which
> allow to pass them through the sudo to test script.
>
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> ---
> testsuite/Makefile | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/testsuite/Makefile b/testsuite/Makefile
> index 2a4e0ba..648c793 100644
> --- a/testsuite/Makefile
> +++ b/testsuite/Makefile
> @@ -1,6 +1,7 @@
> ## -- Config --
> DEV := lo
> PREFIX := sudo
> +RESULTS_DIR := results
> ## -- End Config --
>
> TESTS := $(patsubst tests/%,%,$(wildcard tests/*.t))
> @@ -23,23 +24,24 @@ listtests:
> alltests: $(TESTS)
>
> clean:
> - @rm -rf results/*
> + @rm -rf $(RESULTS_DIR)
>
> distclean: clean
> echo "Entering iproute2" && cd iproute2 && $(MAKE) distclean && cd ..;
>
> $(TESTS):
> + @mkdir -p $(RESULTS_DIR)
> @for i in $(IPVERS); do \
> o=`echo $$i | sed -e 's/iproute2\///'`; \
> echo -n "Running $@ [$$o/`uname -r`]: "; \
> - TC="$$i/tc/tc" IP="$$i/ip/ip" DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
> - ERRF="results/$@.$$o.err" $(KENV) $(PREFIX) tests/$@ > results/$@.$$o.out; \
> + $(PREFIX) TC="$$i/tc/tc" IP="$$i/ip/ip" DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
> + ERRF="$(RESULTS_DIR)/$@.$$o.err" $(KENV) tests/$@ > $(RESULTS_DIR)/$@.$$o.out; \
> if [ "$$?" = "127" ]; then \
> echo "SKIPPED"; \
> - elif [ -e "results/$@.$$o.err" ]; then \
> + elif [ -e "$(RESULTS_DIR)/$@.$$o.err" ]; then \
> echo "FAILED"; \
> else \
> echo "PASS"; \
> fi; \
> - dmesg > results/$@.$$o.dmesg; \
> + dmesg > $(RESULTS_DIR)/$@.$$o.dmesg; \
> done
> --
> 2.1.0
>
^ permalink raw reply
* [PATCH v2] iproute2 tests: Fix problem with test running
From: Vadim Kochan @ 2014-09-06 10:27 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
Tests were not allowed to be ran, the following
issues were fixed:
- creating the results folder before test running
- sudo $PREFIX moved before variables definition which
allow to pass them through the sudo to test script.
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
testsuite/Makefile | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/testsuite/Makefile b/testsuite/Makefile
index 2a4e0ba..b4ab15e 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -1,6 +1,7 @@
## -- Config --
DEV := lo
-PREFIX := sudo
+PREFIX := sudo -E
+RESULTS_DIR := results
## -- End Config --
TESTS := $(patsubst tests/%,%,$(wildcard tests/*.t))
@@ -23,23 +24,24 @@ listtests:
alltests: $(TESTS)
clean:
- @rm -rf results/*
+ @rm -rf $(RESULTS_DIR)
distclean: clean
echo "Entering iproute2" && cd iproute2 && $(MAKE) distclean && cd ..;
$(TESTS):
+ @mkdir -p $(RESULTS_DIR)
@for i in $(IPVERS); do \
o=`echo $$i | sed -e 's/iproute2\///'`; \
echo -n "Running $@ [$$o/`uname -r`]: "; \
TC="$$i/tc/tc" IP="$$i/ip/ip" DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
- ERRF="results/$@.$$o.err" $(KENV) $(PREFIX) tests/$@ > results/$@.$$o.out; \
+ ERRF="$(RESULTS_DIR)/$@.$$o.err" $(KENV) $(PREFIX) tests/$@ > $(RESULTS_DIR)/$@.$$o.out; \
if [ "$$?" = "127" ]; then \
echo "SKIPPED"; \
- elif [ -e "results/$@.$$o.err" ]; then \
+ elif [ -e "$(RESULTS_DIR)/$@.$$o.err" ]; then \
echo "FAILED"; \
else \
echo "PASS"; \
fi; \
- dmesg > results/$@.$$o.dmesg; \
+ dmesg > $(RESULTS_DIR)/$@.$$o.dmesg; \
done
--
2.1.0
^ permalink raw reply related
* Re: [NS] page allocation failure. order:2, mode:0x204020 while enabling Jumbo frame
From: Jorge Daniel Sequeira Matias @ 2014-09-06 10:29 UTC (permalink / raw)
To: ns-list@dsi.ist.utl.pt; +Cc: netdev@vger.kernel.org
In-Reply-To: <20140905184400.GA27606@calhariz.com>
Bom dia,
É pena que o título não tivesse começado por "bnx2x: ".
Não acredito que este problema seja transversal a todas à as placas de rede.
Brevemente teremos oportunidade de testar uma placa diferente, que vai sair de uma das brites.
Jorge
Sent from my iPhone
> On 05/09/2014, at 19:44, Jose M Calhariz <jose.calhariz@netvisao.pt> wrote:
>
>
> Hi
>
> I have a Debian 7.0 server currently running kernel 3.16 from
> kernel.org. This server is a client of nfsv3, iSCSI and use dmcrypt.
> When and only when enabling Jumbo Frames I get "page allocation
> failure: order:2, mode:0x204020", in the end of the email is a stack
> trace from this error.
>
> Sometimes the rate of "page allocation failure" is very big or the
> network stack stops, making the machine "network unreachable" to a
> nagios server.
>
>
> Sep 5 10:51:14 XXXXXX kernel: warn_alloc_failed: 5 callbacks suppressed
> Sep 5 10:51:14 XXXXXX kernel: kworker/0:1: page allocation failure: order:2, mode:0x204020
> Sep 5 10:51:14 XXXXXX kernel: CPU: 0 PID: 6178 Comm: kworker/0:1 Tainted: G I 3.16.0-dsi #9
> Sep 5 10:51:14 XXXXXX kernel: Hardware name: Dell Inc. PowerEdge R410/01V648, BIOS 1.12.0 07/30/2013
> Sep 5 10:51:14 XXXXXX kernel: Workqueue: kcryptd kcryptd_crypt [dm_crypt]
> Sep 5 10:51:14 XXXXXX kernel: 0000000000000000 0000000000000000 ffffffff814fd161 0000000000204020
> Sep 5 10:51:14 XXXXXX kernel: ffffffff811395d2 ffff88032fffac00 ffffffff818b04b0 0000000000000003
> Sep 5 10:51:14 XXXXXX kernel: 0000000000000018 000000012fffac18 ffff8801affff0a8 0000000000000096
> Sep 5 10:51:14 XXXXXX kernel: Call Trace:
> Sep 5 10:51:14 XXXXXX kernel: <IRQ> [<ffffffff814fd161>] ? dump_stack+0x41/0x51
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff811395d2>] ? warn_alloc_failed+0xf2/0x150
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff8113d930>] ? __alloc_pages_nodemask+0x8e0/0xb40
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff8118355a>] ? kmem_getpages+0x5a/0x110
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff8118522f>] ? fallback_alloc+0x1cf/0x220
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffffa019ef1d>] ? bnx2x_alloc_rx_data.isra.75+0x5d/0x1e0 [bnx2x]
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff811854bc>] ? __kmalloc+0x19c/0x1f0
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffffa019ef1d>] ? bnx2x_alloc_rx_data.isra.75+0x5d/0x1e0 [bnx2x]
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffffa019f894>] ? bnx2x_rx_int+0x7f4/0x1790 [bnx2x]
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff814005e8>] ? skb_release_head_state+0x48/0xf0
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff814006f9>] ? __kfree_skb+0x9/0xa0
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff81461f6d>] ? tcp_schedule_loss_probe+0x12d/0x1a0
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffffa01a25a8>] ? bnx2x_poll+0xe8/0x3b0 [bnx2x]
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff8140fc18>] ? net_rx_action+0x138/0x240
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff8106af4f>] ? __do_softirq+0xdf/0x270
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff8106b315>] ? irq_exit+0x95/0xa0
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff81015701>] ? do_IRQ+0x61/0x110
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff81503aed>] ? common_interrupt+0x6d/0x6d
> Sep 5 10:51:14 XXXXXX kernel: <EOI> [<ffffffffa0378c10>] ? _aesni_enc1+0x5c/0xac [aesni_intel]
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffffa0379170>] ? aesni_cbc_enc+0x20/0x40 [aesni_intel]
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffffa037ab24>] ? cbc_encrypt+0x74/0xd0 [aesni_intel]
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffffa02270fe>] ? __ablk_encrypt+0x3e/0x50 [ablk_helper]
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffffa069641b>] ? crypt_convert+0x20b/0x460 [dm_crypt]
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffffa069682b>] ? kcryptd_crypt+0x1bb/0x400 [dm_crypt]
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff81080adb>] ? process_one_work+0x16b/0x400
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff81081184>] ? worker_thread+0x114/0x510
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff814ff3f8>] ? __schedule+0x2c8/0x760
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff81081070>] ? rescuer_thread+0x2c0/0x2c0
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff810876cc>] ? kthread+0xbc/0xe0
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff81087610>] ? flush_kthread_worker+0x80/0x80
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff81502f3c>] ? ret_from_fork+0x7c/0xb0
> Sep 5 10:51:14 XXXXXX kernel: [<ffffffff81087610>] ? flush_kthread_worker+0x80/0x80
> Sep 5 10:51:14 XXXXXX kernel: Mem-Info:
> Sep 5 10:51:14 XXXXXX kernel: Node 0 Normal per-cpu:
> Sep 5 10:51:14 XXXXXX kernel: CPU 0: hi: 186, btch: 31 usd: 141
> Sep 5 10:51:14 XXXXXX kernel: CPU 1: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 2: hi: 186, btch: 31 usd: 178
> Sep 5 10:51:14 XXXXXX kernel: CPU 3: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 4: hi: 186, btch: 31 usd: 169
> Sep 5 10:51:14 XXXXXX kernel: CPU 5: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 6: hi: 186, btch: 31 usd: 181
> Sep 5 10:51:14 XXXXXX kernel: CPU 7: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: Node 1 DMA per-cpu:
> Sep 5 10:51:14 XXXXXX kernel: CPU 0: hi: 0, btch: 1 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 1: hi: 0, btch: 1 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 2: hi: 0, btch: 1 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 3: hi: 0, btch: 1 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 4: hi: 0, btch: 1 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 5: hi: 0, btch: 1 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 6: hi: 0, btch: 1 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 7: hi: 0, btch: 1 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: Node 1 DMA32 per-cpu:
> Sep 5 10:51:14 XXXXXX kernel: CPU 0: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 1: hi: 186, btch: 31 usd: 51
> Sep 5 10:51:14 XXXXXX kernel: CPU 2: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 3: hi: 186, btch: 31 usd: 36
> Sep 5 10:51:14 XXXXXX kernel: CPU 4: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 5: hi: 186, btch: 31 usd: 51
> Sep 5 10:51:14 XXXXXX kernel: CPU 6: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 7: hi: 186, btch: 31 usd: 59
> Sep 5 10:51:14 XXXXXX kernel: Node 1 Normal per-cpu:
> Sep 5 10:51:14 XXXXXX kernel: CPU 0: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 1: hi: 186, btch: 31 usd: 95
> Sep 5 10:51:14 XXXXXX kernel: CPU 2: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 3: hi: 186, btch: 31 usd: 100
> Sep 5 10:51:14 XXXXXX kernel: CPU 4: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 5: hi: 186, btch: 31 usd: 95
> Sep 5 10:51:14 XXXXXX kernel: CPU 6: hi: 186, btch: 31 usd: 0
> Sep 5 10:51:14 XXXXXX kernel: CPU 7: hi: 186, btch: 31 usd: 79
> Sep 5 10:51:14 XXXXXX kernel: active_anon:391978 inactive_anon:217234 isolated_anon:0
> Sep 5 10:51:14 XXXXXX kernel: active_file:405690 inactive_file:251497 isolated_file:0
> Sep 5 10:51:14 XXXXXX kernel: unevictable:2192 dirty:0 writeback:0 unstable:0
> Sep 5 10:51:14 XXXXXX kernel: free:482104 slab_reclaimable:827503 slab_unreclaimable:171240
> Sep 5 10:51:14 XXXXXX kernel: mapped:3469 shmem:99 pagetables:2164 bounce:0
> Sep 5 10:51:14 XXXXXX kernel: free_cma:0
> Sep 5 10:51:14 XXXXXX kernel: Node 0 Normal free:801464kB min:528080kB low:660100kB high:792120kB active_anon:1161624kB inactive_anon:387580kB active_file:534424kB inactive_file:291488kB unevictable:4940kB isolated(anon):0kB isolated(file):0kB present:6291456kB managed:6203092kB mlocked:4944kB dirty:0kB
> +writeback:0kB mapped:5116kB shmem:124kB slab_reclaimable:1923736kB slab_unreclaimable:386892kB kernel_stack:4368kB pagetables:5768kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:47 all_unreclaimable? no
> Sep 5 10:51:14 XXXXXX kernel: lowmem_reserve[]: 0 0 0 0
> Sep 5 10:51:14 XXXXXX kernel: Node 1 DMA free:15896kB min:1352kB low:1688kB high:2028kB active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:15980kB managed:15896kB mlocked:0kB dirty:0kB writeback:0kB mapped:0kB shmem:0kB
> +slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? yes
> Sep 5 10:51:14 XXXXXX kernel: lowmem_reserve[]: 0 3224 5952 5952
> Sep 5 10:51:14 XXXXXX kernel: Node 1 DMA32 free:804396kB min:281328kB low:351660kB high:421992kB active_anon:240284kB inactive_anon:288392kB active_file:558572kB inactive_file:412684kB unevictable:1972kB isolated(anon):0kB isolated(file):0kB present:3378660kB managed:3304628kB mlocked:1972kB dirty:0kB writeback:0kB
> +mapped:4144kB shmem:204kB slab_reclaimable:618324kB slab_unreclaimable:133696kB kernel_stack:1168kB pagetables:1512kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
> Sep 5 10:51:14 XXXXXX kernel: lowmem_reserve[]: 0 0 2727 2727
> Sep 5 10:51:14 XXXXXX kernel: Node 1 Normal free:306660kB min:237808kB low:297260kB high:356712kB active_anon:166004kB inactive_anon:192964kB active_file:529764kB inactive_file:301816kB unevictable:1856kB isolated(anon):0kB isolated(file):0kB present:2883584kB managed:2793404kB mlocked:1856kB dirty:0kB writeback:0kB
> +mapped:4616kB shmem:68kB slab_reclaimable:767952kB slab_unreclaimable:164372kB kernel_stack:1312kB pagetables:1376kB unstable:0kB bounce:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
> Sep 5 10:51:14 XXXXXX kernel: lowmem_reserve[]: 0 0 0 0
> Sep 5 10:51:14 XXXXXX kernel: Node 0 Normal: 186511*4kB (UEM) 5585*8kB (UEM) 421*16kB (UM) 1*32kB (M) 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 1*4096kB (R) = 801588kB
> Sep 5 10:51:14 XXXXXX kernel: Node 1 DMA: 0*4kB 1*8kB (U) 1*16kB (U) 0*32kB 2*64kB (U) 1*128kB (U) 1*256kB (U) 0*512kB 1*1024kB (U) 1*2048kB (R) 3*4096kB (M) = 15896kB
> Sep 5 10:51:14 XXXXXX kernel: Node 1 DMA32: 97773*4kB (EM) 49708*8kB (EM) 721*16kB (M) 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 1*4096kB (R) = 804388kB
> Sep 5 10:51:14 XXXXXX kernel: Node 1 Normal: 48558*4kB (EM) 13544*8kB (M) 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 1*4096kB (R) = 306680kB
> Sep 5 10:51:14 XXXXXX kernel: Node 0 hugepages_total=0 hugepages_free=0 hugepages_surp=0 hugepages_size=2048kB
> Sep 5 10:51:14 XXXXXX kernel: Node 1 hugepages_total=0 hugepages_free=0 hugepages_surp=0 hugepages_size=2048kB
> Sep 5 10:51:14 XXXXXX kernel: 658273 total pagecache pages
> Sep 5 10:51:14 XXXXXX kernel: 37 pages in swap cache
> Sep 5 10:51:14 XXXXXX kernel: Swap cache stats: add 499, delete 462, find 7/10
> Sep 5 10:51:14 XXXXXX kernel: Free swap = 37456928kB
> Sep 5 10:51:14 XXXXXX kernel: Total swap = 37458864kB
> Sep 5 10:51:14 XXXXXX kernel: 3142420 pages RAM
> Sep 5 10:51:14 XXXXXX kernel: 0 pages HighMem/MovableOnly
> Sep 5 10:51:14 XXXXXX kernel: 22545 pages reserved
> Sep 5 10:51:14 XXXXXX kernel: 0 pages hwpoisoned
>
>
>
> --
> --
> Citação: ato de repetir de modo errado as palavras alheias.
> -- Ambrose Bierce; jornalista e cientista americano.
> _______________________________________________
> ns-list mailing list
> ns-list@dsi.ist.utl.pt
> https://mlists.ist.utl.pt/mailman/listinfo/groups.ciist.ns-list
^ permalink raw reply
* Re: [PATCH net-next v1 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Nikolay Aleksandrov @ 2014-09-06 11:02 UTC (permalink / raw)
To: Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David Miller
Cc: netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <1409985356-2384-1-git-send-email-maheshb@google.com>
On 09/06/2014 08:35 AM, Mahesh Bandewar wrote:
> Earlier change to use usable slave array for TLB mode had an additional
> performance advantage. So extending the same logic to all other modes
> that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
> Also consolidating this with the earlier TLB change.
>
> The main idea is to build the usable slaves array in the control path
> and use that array for slave selection during xmit operation.
>
> Measured performance in a setup with a bond of 4x1G NICs with 200
> instances of netperf for the modes involved (3ad, xor, tlb)
> cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
>
> Mode TPS-Before TPS-After
>
> 802.3ad : 468,694 493,101
> TLB (lb=0): 392,583 392,965
> XOR : 475,696 484,517
>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
> v1:
> (a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
> the slave that need to be removed.
> (b) Freeing of array will assign NULL (to handle bond->down to bond->up
> transition gracefully.
> (c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
> failure.
> (d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
> will populate the array even if these parameters are not used.
> (e) 3AD: Should handle the ad_agg_selection_logic correctly.
>
> drivers/net/bonding/bond_3ad.c | 79 ++++-----------------
> drivers/net/bonding/bond_alb.c | 45 +-----------
> drivers/net/bonding/bond_alb.h | 8 ---
> drivers/net/bonding/bond_main.c | 150 ++++++++++++++++++++++++++++++++++++----
> drivers/net/bonding/bonding.h | 8 +++
> 5 files changed, 161 insertions(+), 129 deletions(-)
>
Hi Mahesh,
>From my last posts I revisited the bond_3ad_state_machine_handler() case
and I think I was wrong that the machine state lock would protect you since
it's different for every port so if the machine handler runs for port X and
something else executes for port Y without bond->lock for writing - race.
As I said in my last post primary_reselect is an ideal case for that even
though it's not used in 3ad, it can be altered and to cause a reselect of
the active slave thus rebuilding the slave_arr. Of course this is
theoretical, but I'd prefer not to have such bugs in the first place.
More notes below.
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index ee2c73a9de39..ba05c83d5d83 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -1579,6 +1579,8 @@ static void ad_agg_selection_logic(struct aggregator *agg)
> __disable_port(port);
> }
> }
> + if (bond_update_slave_arr(bond, NULL))
> + pr_err("Failed to build slave-array for 3ad mode.\n");
> }
>
> /* if the selected aggregator is of join individuals
> @@ -1717,6 +1719,8 @@ static void ad_enable_collecting_distributing(struct port *port)
> port->actor_port_number,
> port->aggregator->aggregator_identifier);
> __enable_port(port);
> + if (bond_update_slave_arr(port->slave->bond, NULL))
> + pr_err("Failed to build slave-array for 3ad mode.\n");
> }
> }
>
> @@ -1733,6 +1737,8 @@ static void ad_disable_collecting_distributing(struct port *port)
> port->actor_port_number,
> port->aggregator->aggregator_identifier);
> __disable_port(port);
> + if (bond_update_slave_arr(port->slave->bond, NULL))
> + pr_err("Failed to build slave-array for 3ad mode.\n");
> }
> }
>
> @@ -1917,6 +1923,9 @@ void bond_3ad_unbind_slave(struct slave *slave)
> __update_lacpdu_from_port(port);
> ad_lacpdu_send(port);
>
> + if (bond_update_slave_arr(bond, slave))
> + pr_err("Failed to build slave-array for 3AD mode.\n");
> +
> /* check if this aggregator is occupied */
> if (aggregator->lag_ports) {
> /* check if there are other ports related to this aggregator
> @@ -2311,6 +2320,9 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
> */
> port->sm_vars |= AD_PORT_BEGIN;
>
> + if (bond_update_slave_arr(slave->bond, NULL))
> + pr_err("Failed to build slave-array for 3ad mode.\n");
> +
> __release_state_machine_lock(port);
> }
>
> @@ -2407,73 +2419,6 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
> return ret;
> }
>
> -int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
> -{
> - struct bonding *bond = netdev_priv(dev);
> - struct slave *slave, *first_ok_slave;
> - struct aggregator *agg;
> - struct ad_info ad_info;
> - struct list_head *iter;
> - int slaves_in_agg;
> - int slave_agg_no;
> - int agg_id;
> -
> - if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
> - netdev_dbg(dev, "__bond_3ad_get_active_agg_info failed\n");
> - goto err_free;
> - }
> -
> - slaves_in_agg = ad_info.ports;
> - agg_id = ad_info.aggregator_id;
> -
> - if (slaves_in_agg == 0) {
> - netdev_dbg(dev, "active aggregator is empty\n");
> - goto err_free;
> - }
> -
> - slave_agg_no = bond_xmit_hash(bond, skb) % slaves_in_agg;
> - first_ok_slave = NULL;
> -
> - bond_for_each_slave_rcu(bond, slave, iter) {
> - agg = SLAVE_AD_INFO(slave)->port.aggregator;
> - if (!agg || agg->aggregator_identifier != agg_id)
> - continue;
> -
> - if (slave_agg_no >= 0) {
> - if (!first_ok_slave && bond_slave_can_tx(slave))
> - first_ok_slave = slave;
> - slave_agg_no--;
> - continue;
> - }
> -
> - if (bond_slave_can_tx(slave)) {
> - bond_dev_queue_xmit(bond, skb, slave->dev);
> - goto out;
> - }
> - }
> -
> - if (slave_agg_no >= 0) {
> - netdev_err(dev, "Couldn't find a slave to tx on for aggregator ID %d\n",
> - agg_id);
> - goto err_free;
> - }
> -
> - /* we couldn't find any suitable slave after the agg_no, so use the
> - * first suitable found, if found.
> - */
> - if (first_ok_slave)
> - bond_dev_queue_xmit(bond, skb, first_ok_slave->dev);
> - else
> - goto err_free;
> -
> -out:
> - return NETDEV_TX_OK;
> -err_free:
> - /* no suitable interface, frame not sent */
> - dev_kfree_skb_any(skb);
> - goto out;
> -}
> -
> int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
> struct slave *slave)
> {
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 73c21e233131..334d92127baf 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -200,7 +200,6 @@ static int tlb_initialize(struct bonding *bond)
> static void tlb_deinitialize(struct bonding *bond)
> {
> struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
> - struct tlb_up_slave *arr;
>
> _lock_tx_hashtbl_bh(bond);
>
> @@ -208,10 +207,6 @@ static void tlb_deinitialize(struct bonding *bond)
> bond_info->tx_hashtbl = NULL;
>
> _unlock_tx_hashtbl_bh(bond);
> -
> - arr = rtnl_dereference(bond_info->slave_arr);
> - if (arr)
> - kfree_rcu(arr, rcu);
> }
>
> static long long compute_gap(struct slave *slave)
> @@ -1409,39 +1404,9 @@ out:
> return NETDEV_TX_OK;
> }
>
> -static int bond_tlb_update_slave_arr(struct bonding *bond,
> - struct slave *skipslave)
> -{
> - struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
> - struct slave *tx_slave;
> - struct list_head *iter;
> - struct tlb_up_slave *new_arr, *old_arr;
> -
> - new_arr = kzalloc(offsetof(struct tlb_up_slave, arr[bond->slave_cnt]),
> - GFP_ATOMIC);
> - if (!new_arr)
> - return -ENOMEM;
> -
> - bond_for_each_slave(bond, tx_slave, iter) {
> - if (!bond_slave_can_tx(tx_slave))
> - continue;
> - if (skipslave == tx_slave)
> - continue;
> - new_arr->arr[new_arr->count++] = tx_slave;
> - }
> -
> - old_arr = rtnl_dereference(bond_info->slave_arr);
> - rcu_assign_pointer(bond_info->slave_arr, new_arr);
> - if (old_arr)
> - kfree_rcu(old_arr, rcu);
> -
> - return 0;
> -}
> -
> int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
> {
> struct bonding *bond = netdev_priv(bond_dev);
> - struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
> struct ethhdr *eth_data;
> struct slave *tx_slave = NULL;
> u32 hash_index;
> @@ -1462,9 +1427,9 @@ int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
> hash_index & 0xFF,
> skb->len);
> } else {
> - struct tlb_up_slave *slaves;
> + struct bond_up_slave *slaves;
>
> - slaves = rcu_dereference(bond_info->slave_arr);
> + slaves = rcu_dereference(bond->slave_arr);
> if (slaves && slaves->count)
> tx_slave = slaves->arr[hash_index %
> slaves->count];
> @@ -1733,10 +1698,6 @@ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave)
> rlb_clear_slave(bond, slave);
> }
>
> - if (bond_is_nondyn_tlb(bond))
> - if (bond_tlb_update_slave_arr(bond, slave))
> - pr_err("Failed to build slave-array for TLB mode.\n");
> -
> }
>
> /* Caller must hold bond lock for read */
> @@ -1762,7 +1723,7 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
> }
>
> if (bond_is_nondyn_tlb(bond)) {
> - if (bond_tlb_update_slave_arr(bond, NULL))
> + if (bond_update_slave_arr(bond, NULL))
> pr_err("Failed to build slave-array for TLB mode.\n");
> }
> }
> diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
> index aaeac61d03cf..5fc76c01636c 100644
> --- a/drivers/net/bonding/bond_alb.h
> +++ b/drivers/net/bonding/bond_alb.h
> @@ -139,20 +139,12 @@ struct tlb_slave_info {
> */
> };
>
> -struct tlb_up_slave {
> - unsigned int count;
> - struct rcu_head rcu;
> - struct slave *arr[0];
> -};
> -
> struct alb_bond_info {
> struct tlb_client_info *tx_hashtbl; /* Dynamically allocated */
> spinlock_t tx_hashtbl_lock;
> u32 unbalanced_load;
> int tx_rebalance_counter;
> int lp_counter;
> - /* -------- non-dynamic tlb mode only ---------*/
> - struct tlb_up_slave __rcu *slave_arr; /* Up slaves */
> /* -------- rlb parameters -------- */
> int rlb_enabled;
> struct rlb_client_info *rx_hashtbl; /* Receive hash table */
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index f0f5eab0fab1..43f066539dab 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1413,6 +1413,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> dev_mc_add(slave_dev, lacpdu_multicast);
> }
>
> + if (BOND_MODE(bond) == BOND_MODE_XOR &&
> + bond_update_slave_arr(bond, NULL))
> + pr_err("Failed to build slave-array for XOR mode.\n");
> +
^^^^^^^^^^^^^^
2 issues here:
1. a little bit after this you can find the following switch:
switch (BOND_MODE(bond)) {
that is meant for specific mode handling, I don't think you need to add
additional "if" here.
2. Why do you rebuild here, bond_master_upper_dev_link() hasn't been called
yet so the new slave isn't visible yet.
> res = vlan_vids_add_by_dev(slave_dev, bond_dev);
> if (res) {
> netdev_err(bond_dev, "Couldn't add bond vlan ids to %s\n",
> @@ -1692,6 +1696,11 @@ static int __bond_release_one(struct net_device *bond_dev,
> /* Inform AD package of unbinding of slave. */
> if (BOND_MODE(bond) == BOND_MODE_8023AD)
> bond_3ad_unbind_slave(slave);
> + else if (BOND_MODE(bond) == BOND_MODE_XOR ||
> + bond_is_nondyn_tlb(bond)) {
> + if (bond_update_slave_arr(bond, slave))
> + pr_err("Failed to build slave-array.\n");
> + }
Documentation/CodingStyle:
both branches must use braces.
>
> write_unlock_bh(&bond->lock);
>
> @@ -2009,6 +2018,10 @@ static void bond_miimon_commit(struct bonding *bond)
> bond_alb_handle_link_change(bond, slave,
> BOND_LINK_UP);
>
> + if (BOND_MODE(bond) == BOND_MODE_XOR &&
> + bond_update_slave_arr(bond, NULL))
> + pr_err("Failed to build slave-array for XOR mode.\n");
> +
> if (!bond->curr_active_slave ||
> (slave == bond->primary_slave))
> goto do_failover;
> @@ -2037,6 +2050,10 @@ static void bond_miimon_commit(struct bonding *bond)
> bond_alb_handle_link_change(bond, slave,
> BOND_LINK_DOWN);
>
> + if (BOND_MODE(bond) == BOND_MODE_XOR &&
> + bond_update_slave_arr(bond, NULL))
> + pr_err("Failed to build slave-array for XOR mode.\n");
> +
> if (slave == rcu_access_pointer(bond->curr_active_slave))
> goto do_failover;
>
> @@ -2500,6 +2517,9 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
>
> if (slave_state_changed) {
> bond_slave_state_change(bond);
> + if (BOND_MODE(bond) == BOND_MODE_XOR &&
> + bond_update_slave_arr(bond, NULL))
> + pr_err("Failed to build slave-array for XOR mode.\n");
> } else if (do_failover) {
> /* the bond_select_active_slave must hold RTNL
> * and curr_slave_lock for write.
> @@ -2893,11 +2913,14 @@ static int bond_slave_netdev_event(unsigned long event,
> if (old_duplex != slave->duplex)
> bond_3ad_adapter_duplex_changed(slave);
> }
> + if (BOND_MODE(bond) == BOND_MODE_XOR &&
> + bond_update_slave_arr(bond, NULL))
> + pr_err("Failed to build slave-array for XOR mode.\n");
> break;
> case NETDEV_DOWN:
> - /*
> - * ... Or is it this?
> - */
> + if (BOND_MODE(bond) == BOND_MODE_XOR &&
> + bond_update_slave_arr(bond, NULL))
> + pr_err("Failed to build slave-array for XOR mode.\n");
^^^^^^^^^^^^^^^^
In the case of a netdev event (up/down) does this only affect XOR mode ?
You could be right, just wanted to make sure we're not missing something :-)
> break;
> case NETDEV_CHANGEMTU:
> /*
> @@ -3143,12 +3166,17 @@ static int bond_open(struct net_device *bond_dev)
> bond_3ad_initiate_agg_selection(bond, 1);
> }
>
> + if (BOND_MODE(bond) == BOND_MODE_XOR &&
> + bond_update_slave_arr(bond, NULL))
> + pr_err("Failed to build slave-array for XOR mode.\n");
> +
> return 0;
> }
>
> static int bond_close(struct net_device *bond_dev)
> {
> struct bonding *bond = netdev_priv(bond_dev);
> + struct bond_up_slave *arr;
>
> bond_work_cancel_all(bond);
> bond->send_peer_notif = 0;
> @@ -3156,6 +3184,12 @@ static int bond_close(struct net_device *bond_dev)
> bond_alb_deinitialize(bond);
> bond->recv_probe = NULL;
>
> + arr = rtnl_dereference(bond->slave_arr);
> + if (arr) {
> + kfree_rcu(arr, rcu);
> + RCU_INIT_POINTER(bond->slave_arr, NULL);
> + }
> +
^^^^^^^^
Why do this in the first place ? I mean I could easily release a slave
while the bond is down and rebuild the slave_arr.
One more issue that I just saw is that you might be leaking memory as
ndo_uninit() is called for a device after dev_close_many() so you'll free
the array here, but bond_uninit() calls __bond_release_slave and will
rebuild it.
> return 0;
> }
>
> @@ -3684,15 +3718,108 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
> return NETDEV_TX_OK;
> }
>
> -/* In bond_xmit_xor() , we determine the output device by using a pre-
> - * determined xmit_hash_policy(), If the selected device is not enabled,
> - * find the next active slave.
> +/* Build the usable slaves array in control path for modes that use xmit-hash
> + * to determine the slave interface -
> + * (a) BOND_MODE_8023AD
> + * (b) BOND_MODE_XOR
> + * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0
> */
> -static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
> {
> - struct bonding *bond = netdev_priv(bond_dev);
> + struct slave *slave;
> + struct list_head *iter;
> + struct bond_up_slave *new_arr, *old_arr;
> + int slaves_in_agg;
> + int agg_id = 0;
> + int ret = 0;
> +
> + new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
> + GFP_ATOMIC);
> + if (!new_arr) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> + struct ad_info ad_info;
>
> - bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb) % bond->slave_cnt);
> + if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
> + pr_debug("bond_3ad_get_active_agg_info failed\n");
> + kfree_rcu(new_arr, rcu);
> + ret = -EINVAL;
> + goto out;
> + }
> + slaves_in_agg = ad_info.ports;
> + agg_id = ad_info.aggregator_id;
> + }
> + bond_for_each_slave(bond, slave, iter) {
> + if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> + struct aggregator *agg;
> +
> + agg = SLAVE_AD_INFO(slave)->port.aggregator;
> + if (!agg || agg->aggregator_identifier != agg_id)
> + continue;
> + }
> + if (!bond_slave_can_tx(slave))
> + continue;
> + if (skipslave == slave)
> + continue;
> + new_arr->arr[new_arr->count++] = slave;
> + }
> +
> + old_arr = rcu_dereference_protected(bond->slave_arr,
> + lockdep_rtnl_is_held() ||
> + lockdep_is_held(&bond->lock) ||
> + lockdep_is_held(&bond->curr_slave_lock));
> + rcu_assign_pointer(bond->slave_arr, new_arr);
> + if (old_arr)
> + kfree_rcu(old_arr, rcu);
> +
> +out:
> + if (ret != 0 && skipslave) {
> + int idx;
> +
> + /* Rare situation where caller has asked to skip a specific
> + * slave but allocation failed (most likely!). In this sitation
> + * overwrite the skipslave entry in the array with the last
> + * entry from the array to avoid a situation where the xmit
> + * path may choose this to-be-skipped slave to send a packet
> + * out.
> + */
> + rcu_read_lock();
^^^^^^^^^^^^^^
RCU ?
> + old_arr = rcu_dereference_protected(bond->slave_arr,
> + lockdep_is_held(&bond->lock));
^^^^^^^^
Only bond->lock ? This doesn't make any sense.
> + for (idx = 0; idx < old_arr->count; idx++) {
> + if (skipslave == old_arr->arr[idx]) {
> + if (idx != old_arr->count - 1)
You can drop the "if" and remove one level of indentation, if idx == count
- 1, then it'll overwrite itself (i.e. nothing) but count will still go down.
But I think there's a potential bigger problem here as in the case of
failure count might drop down to 0 but some transmitter might be pass the
check and at the modulus part and if count is re-fetched we might end up
with a div by zero.
> + old_arr->arr[idx] =
> + old_arr->arr[old_arr->count-1];
> + old_arr->count--;
> + break;
> + }
> + }
> + rcu_read_unlock();
> + }
> + return ret;
> +}
> +
> +/* Use this Xmit function for 3AD as well as XOR modes. The current
> + * usable slave array is formed in the control path. The xmit function
> + * just calculates hash and sends the packet out.
> + */
> +int bond_3ad_xor_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct bonding *bond = netdev_priv(dev);
> + struct slave *slave;
> + struct bond_up_slave *slaves;
> +
> + slaves = rcu_dereference(bond->slave_arr);
> + if (slaves && slaves->count) {
> + slave = slaves->arr[bond_xmit_hash(bond, skb) % slaves->count];
> + bond_dev_queue_xmit(bond, skb, slave->dev);
> + } else {
> + dev_kfree_skb_any(skb);
> + atomic_long_inc(&dev->tx_dropped);
> + }
>
> return NETDEV_TX_OK;
> }
> @@ -3794,12 +3921,11 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
> return bond_xmit_roundrobin(skb, dev);
> case BOND_MODE_ACTIVEBACKUP:
> return bond_xmit_activebackup(skb, dev);
> + case BOND_MODE_8023AD:
> case BOND_MODE_XOR:
> - return bond_xmit_xor(skb, dev);
> + return bond_3ad_xor_xmit(skb, dev);
> case BOND_MODE_BROADCAST:
> return bond_xmit_broadcast(skb, dev);
> - case BOND_MODE_8023AD:
> - return bond_3ad_xmit_xor(skb, dev);
> case BOND_MODE_ALB:
> return bond_alb_xmit(skb, dev);
> case BOND_MODE_TLB:
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index aace510d08d1..4a6195c0de60 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -177,6 +177,12 @@ struct slave {
> struct kobject kobj;
> };
>
> +struct bond_up_slave {
> + unsigned int count;
> + struct rcu_head rcu;
> + struct slave *arr[0];
> +};
> +
> /*
> * Link pseudo-state only used internally by monitors
> */
> @@ -196,6 +202,7 @@ struct bonding {
> struct slave __rcu *curr_active_slave;
> struct slave __rcu *current_arp_slave;
> struct slave *primary_slave;
> + struct bond_up_slave __rcu *slave_arr; /* Array of usable slaves */
> bool force_primary;
> s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
> int (*recv_probe)(const struct sk_buff *, struct bonding *,
> @@ -527,6 +534,7 @@ const char *bond_slave_link_status(s8 link);
> struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
> struct net_device *end_dev,
> int level);
> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
>
> #ifdef CONFIG_PROC_FS
> void bond_create_proc_entry(struct bonding *bond);
>
^ permalink raw reply
* [PATCH][net-next] openvswitch: change the data type of error status to atomic_long_t
From: roy.qing.li @ 2014-09-06 11:06 UTC (permalink / raw)
To: netdev; +Cc: pshelar
From: Li RongQing <roy.qing.li@gmail.com>
Change the date type of error status from u64 to atomic_long_t, and use atomic
operation, then remove the lock which is used to protect the error status.
The operation of atomic maybe faster than spin lock.
Cc: Pravin Shelar <pshelar@nicira.com>
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
net/openvswitch/vport.c | 25 ++++++++-----------------
net/openvswitch/vport.h | 10 ++++------
2 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index 6d8f2ec..f7e63f9 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -148,8 +148,6 @@ struct vport *ovs_vport_alloc(int priv_size, const struct vport_ops *ops,
return ERR_PTR(-ENOMEM);
}
- spin_lock_init(&vport->stats_lock);
-
return vport;
}
@@ -268,14 +266,10 @@ void ovs_vport_get_stats(struct vport *vport, struct ovs_vport_stats *stats)
* netdev-stats can be directly read over netlink-ioctl.
*/
- spin_lock_bh(&vport->stats_lock);
-
- stats->rx_errors = vport->err_stats.rx_errors;
- stats->tx_errors = vport->err_stats.tx_errors;
- stats->tx_dropped = vport->err_stats.tx_dropped;
- stats->rx_dropped = vport->err_stats.rx_dropped;
-
- spin_unlock_bh(&vport->stats_lock);
+ stats->rx_errors = atomic_long_read(&vport->err_stats.rx_errors);
+ stats->tx_errors = atomic_long_read(&vport->err_stats.tx_errors);
+ stats->tx_dropped = atomic_long_read(&vport->err_stats.tx_dropped);
+ stats->rx_dropped = atomic_long_read(&vport->err_stats.rx_dropped);
for_each_possible_cpu(i) {
const struct pcpu_sw_netstats *percpu_stats;
@@ -495,27 +489,24 @@ int ovs_vport_send(struct vport *vport, struct sk_buff *skb)
static void ovs_vport_record_error(struct vport *vport,
enum vport_err_type err_type)
{
- spin_lock(&vport->stats_lock);
-
switch (err_type) {
case VPORT_E_RX_DROPPED:
- vport->err_stats.rx_dropped++;
+ atomic_long_inc(&vport->err_stats.rx_dropped);
break;
case VPORT_E_RX_ERROR:
- vport->err_stats.rx_errors++;
+ atomic_long_inc(&vport->err_stats.rx_errors);
break;
case VPORT_E_TX_DROPPED:
- vport->err_stats.tx_dropped++;
+ atomic_long_inc(&vport->err_stats.tx_dropped);
break;
case VPORT_E_TX_ERROR:
- vport->err_stats.tx_errors++;
+ atomic_long_inc(&vport->err_stats.tx_errors);
break;
}
- spin_unlock(&vport->stats_lock);
}
static void free_vport_rcu(struct rcu_head *rcu)
diff --git a/net/openvswitch/vport.h b/net/openvswitch/vport.h
index 35f89d8..0d95b9f 100644
--- a/net/openvswitch/vport.h
+++ b/net/openvswitch/vport.h
@@ -62,10 +62,10 @@ int ovs_vport_send(struct vport *, struct sk_buff *);
/* The following definitions are for implementers of vport devices: */
struct vport_err_stats {
- u64 rx_dropped;
- u64 rx_errors;
- u64 tx_dropped;
- u64 tx_errors;
+ atomic_long_t rx_dropped;
+ atomic_long_t rx_errors;
+ atomic_long_t tx_dropped;
+ atomic_long_t tx_errors;
};
/**
* struct vport_portids - array of netlink portids of a vport.
@@ -93,7 +93,6 @@ struct vport_portids {
* @dp_hash_node: Element in @datapath->ports hash table in datapath.c.
* @ops: Class structure.
* @percpu_stats: Points to per-CPU statistics used and maintained by vport
- * @stats_lock: Protects @err_stats;
* @err_stats: Points to error statistics used and maintained by vport
*/
struct vport {
@@ -108,7 +107,6 @@ struct vport {
struct pcpu_sw_netstats __percpu *percpu_stats;
- spinlock_t stats_lock;
struct vport_err_stats err_stats;
};
--
1.7.10.4
^ permalink raw reply related
* [net-next 0/2][pull request] Intel Wired LAN Driver Updates 2014-09-06
From: Jeff Kirsher @ 2014-09-06 12:34 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene
This series contains updates to e1000 and igb.
Krzysztof provides a patch to cleanup the coding style in e1000 to quiet
checkpatch.pl warnings.
Todd adds two boolean flags to igb to allow for changes in the
advertised EEE speeds from ethtool.
The following are changes since commit cfecec56ae7c7c40f23fbdac04acee027ca3bd66:
mlx4: only pull headers into skb head
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Krzysztof Majzerowicz-Jaszcz (1):
e1000: e1000_ethertool.c coding style fixes
Todd Fujinaka (1):
igb: add flags to set eee advertisement mode
drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 155 ++++++++++++-----------
drivers/net/ethernet/intel/igb/e1000_82575.c | 31 ++++-
drivers/net/ethernet/intel/igb/e1000_82575.h | 4 +-
drivers/net/ethernet/intel/igb/igb_ethtool.c | 24 +++-
drivers/net/ethernet/intel/igb/igb_main.c | 8 +-
5 files changed, 128 insertions(+), 94 deletions(-)
--
1.9.3
^ permalink raw reply
* [net-next 2/2] igb: add flags to set eee advertisement mode
From: Jeff Kirsher @ 2014-09-06 12:34 UTC (permalink / raw)
To: davem; +Cc: Todd Fujinaka, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1410006842-26517-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Todd Fujinaka <todd.fujinaka@intel.com>
Change e1000_set_eee and e1000_set_eee_i35(0|4) to allow
changes in the advertised EEE speeds from ethtool. Adds two boolean
flags to e1000_set_eee_i35(0|4) to pass in advertised speed data.
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/e1000_82575.c | 31 +++++++++++++++++++++++-----
drivers/net/ethernet/intel/igb/e1000_82575.h | 4 ++--
drivers/net/ethernet/intel/igb/igb_ethtool.c | 24 ++++++++++++++-------
drivers/net/ethernet/intel/igb/igb_main.c | 8 +++----
4 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c
index 236a618..051ea94 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -2548,11 +2548,13 @@ s32 igb_read_emi_reg(struct e1000_hw *hw, u16 addr, u16 *data)
/**
* igb_set_eee_i350 - Enable/disable EEE support
* @hw: pointer to the HW structure
+ * @adv1G: boolean flag enabling 1G EEE advertisement
+ * @adv100m: boolean flag enabling 100M EEE advertisement
*
* Enable/disable EEE based on setting in dev_spec structure.
*
**/
-s32 igb_set_eee_i350(struct e1000_hw *hw)
+s32 igb_set_eee_i350(struct e1000_hw *hw, bool adv1G, bool adv100M)
{
u32 ipcnfg, eeer;
@@ -2566,7 +2568,16 @@ s32 igb_set_eee_i350(struct e1000_hw *hw)
if (!(hw->dev_spec._82575.eee_disable)) {
u32 eee_su = rd32(E1000_EEE_SU);
- ipcnfg |= (E1000_IPCNFG_EEE_1G_AN | E1000_IPCNFG_EEE_100M_AN);
+ if (adv100M)
+ ipcnfg |= E1000_IPCNFG_EEE_100M_AN;
+ else
+ ipcnfg &= ~E1000_IPCNFG_EEE_100M_AN;
+
+ if (adv1G)
+ ipcnfg |= E1000_IPCNFG_EEE_1G_AN;
+ else
+ ipcnfg &= ~E1000_IPCNFG_EEE_1G_AN;
+
eeer |= (E1000_EEER_TX_LPI_EN | E1000_EEER_RX_LPI_EN |
E1000_EEER_LPI_FC);
@@ -2593,11 +2604,13 @@ out:
/**
* igb_set_eee_i354 - Enable/disable EEE support
* @hw: pointer to the HW structure
+ * @adv1G: boolean flag enabling 1G EEE advertisement
+ * @adv100m: boolean flag enabling 100M EEE advertisement
*
* Enable/disable EEE legacy mode based on setting in dev_spec structure.
*
**/
-s32 igb_set_eee_i354(struct e1000_hw *hw)
+s32 igb_set_eee_i354(struct e1000_hw *hw, bool adv1G, bool adv100M)
{
struct e1000_phy_info *phy = &hw->phy;
s32 ret_val = 0;
@@ -2636,8 +2649,16 @@ s32 igb_set_eee_i354(struct e1000_hw *hw)
if (ret_val)
goto out;
- phy_data |= E1000_EEE_ADV_100_SUPPORTED |
- E1000_EEE_ADV_1000_SUPPORTED;
+ if (adv100M)
+ phy_data |= E1000_EEE_ADV_100_SUPPORTED;
+ else
+ phy_data &= ~E1000_EEE_ADV_100_SUPPORTED;
+
+ if (adv1G)
+ phy_data |= E1000_EEE_ADV_1000_SUPPORTED;
+ else
+ phy_data &= ~E1000_EEE_ADV_1000_SUPPORTED;
+
ret_val = igb_write_xmdio_reg(hw, E1000_EEE_ADV_ADDR_I354,
E1000_EEE_ADV_DEV_I354,
phy_data);
diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.h b/drivers/net/ethernet/intel/igb/e1000_82575.h
index b407c55..2154aea 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.h
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.h
@@ -263,8 +263,8 @@ void igb_vmdq_set_loopback_pf(struct e1000_hw *, bool);
void igb_vmdq_set_replication_pf(struct e1000_hw *, bool);
u16 igb_rxpbs_adjust_82580(u32 data);
s32 igb_read_emi_reg(struct e1000_hw *, u16 addr, u16 *data);
-s32 igb_set_eee_i350(struct e1000_hw *);
-s32 igb_set_eee_i354(struct e1000_hw *);
+s32 igb_set_eee_i350(struct e1000_hw *, bool adv1G, bool adv100M);
+s32 igb_set_eee_i354(struct e1000_hw *, bool adv1G, bool adv100M);
s32 igb_get_eee_status_i354(struct e1000_hw *hw, bool *status);
#define E1000_I2C_THERMAL_SENSOR_ADDR 0xF8
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index c737d1f..02cfd3b 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2675,6 +2675,7 @@ static int igb_set_eee(struct net_device *netdev,
struct igb_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
struct ethtool_eee eee_curr;
+ bool adv1g_eee = true, adv100m_eee = true;
s32 ret_val;
if ((hw->mac.type < e1000_i350) ||
@@ -2701,12 +2702,14 @@ static int igb_set_eee(struct net_device *netdev,
return -EINVAL;
}
- if (edata->advertised &
- ~(ADVERTISE_100_FULL | ADVERTISE_1000_FULL)) {
+ if (!edata->advertised || (edata->advertised &
+ ~(ADVERTISE_100_FULL | ADVERTISE_1000_FULL))) {
dev_err(&adapter->pdev->dev,
- "EEE Advertisement supports only 100Tx and or 100T full duplex\n");
+ "EEE Advertisement supports only 100Tx and/or 100T full duplex\n");
return -EINVAL;
}
+ adv100m_eee = !!(edata->advertised & ADVERTISE_100_FULL);
+ adv1g_eee = !!(edata->advertised & ADVERTISE_1000_FULL);
} else if (!edata->eee_enabled) {
dev_err(&adapter->pdev->dev,
@@ -2718,10 +2721,6 @@ static int igb_set_eee(struct net_device *netdev,
if (hw->dev_spec._82575.eee_disable != !edata->eee_enabled) {
hw->dev_spec._82575.eee_disable = !edata->eee_enabled;
adapter->flags |= IGB_FLAG_EEE;
- if (hw->mac.type == e1000_i350)
- igb_set_eee_i350(hw);
- else
- igb_set_eee_i354(hw);
/* reset link */
if (netif_running(netdev))
@@ -2730,6 +2729,17 @@ static int igb_set_eee(struct net_device *netdev,
igb_reset(adapter);
}
+ if (hw->mac.type == e1000_i354)
+ ret_val = igb_set_eee_i354(hw, adv1g_eee, adv100m_eee);
+ else
+ ret_val = igb_set_eee_i350(hw, adv1g_eee, adv100m_eee);
+
+ if (ret_val) {
+ dev_err(&adapter->pdev->dev,
+ "Problem setting EEE advertisement options\n");
+ return -EINVAL;
+ }
+
return 0;
}
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 4c023f0..6cf0c17 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2012,10 +2012,10 @@ void igb_reset(struct igb_adapter *adapter)
case e1000_i350:
case e1000_i210:
case e1000_i211:
- igb_set_eee_i350(hw);
+ igb_set_eee_i350(hw, true, true);
break;
case e1000_i354:
- igb_set_eee_i354(hw);
+ igb_set_eee_i354(hw, true, true);
break;
default:
break;
@@ -2619,7 +2619,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
case e1000_i210:
case e1000_i211:
/* Enable EEE for internal copper PHY devices */
- err = igb_set_eee_i350(hw);
+ err = igb_set_eee_i350(hw, true, true);
if ((!err) &&
(!hw->dev_spec._82575.eee_disable)) {
adapter->eee_advert =
@@ -2630,7 +2630,7 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
case e1000_i354:
if ((rd32(E1000_CTRL_EXT) &
E1000_CTRL_EXT_LINK_MODE_SGMII)) {
- err = igb_set_eee_i354(hw);
+ err = igb_set_eee_i354(hw, true, true);
if ((!err) &&
(!hw->dev_spec._82575.eee_disable)) {
adapter->eee_advert =
--
1.9.3
^ permalink raw reply related
* [net-next 1/2] e1000: e1000_ethertool.c coding style fixes
From: Jeff Kirsher @ 2014-09-06 12:34 UTC (permalink / raw)
To: davem
Cc: Krzysztof Majzerowicz-Jaszcz, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <1410006842-26517-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Krzysztof Majzerowicz-Jaszcz <cristos@vipserv.org>
Fixed many errors/warnings and checks in e1000_ethtool.c reported
by checkpatch.pl. Suggestions from Joe Perches and Alexander Duyck
applied as well
Signed-off-by: Krzysztof Majzerowicz-Jaszcz <cristos@vipserv.org>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 155 ++++++++++++-----------
1 file changed, 79 insertions(+), 76 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index cca5bca..9b50272 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -1,35 +1,30 @@
/*******************************************************************************
-
- Intel PRO/1000 Linux driver
- Copyright(c) 1999 - 2006 Intel Corporation.
-
- This program is free software; you can redistribute it and/or modify it
- under the terms and conditions of the GNU General Public License,
- version 2, as published by the Free Software Foundation.
-
- This program is distributed in the hope it will be useful, but WITHOUT
- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- more details.
-
- You should have received a copy of the GNU General Public License along with
- this program; if not, write to the Free Software Foundation, Inc.,
- 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
- The full GNU General Public License is included in this distribution in
- the file called "COPYING".
-
- Contact Information:
- Linux NICS <linux.nics@intel.com>
- e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
- Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
-
-*******************************************************************************/
+ * Intel PRO/1000 Linux driver
+ * Copyright(c) 1999 - 2006 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * The full GNU General Public License is included in this distribution in
+ * the file called "COPYING".
+ *
+ * Contact Information:
+ * Linux NICS <linux.nics@intel.com>
+ * e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
+ * Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
+ *
+ ******************************************************************************/
/* ethtool support for e1000 */
#include "e1000.h"
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
enum {NETDEV_STATS, E1000_STATS};
@@ -42,7 +37,7 @@ struct e1000_stats {
#define E1000_STAT(m) E1000_STATS, \
sizeof(((struct e1000_adapter *)0)->m), \
- offsetof(struct e1000_adapter, m)
+ offsetof(struct e1000_adapter, m)
#define E1000_NETDEV_STAT(m) NETDEV_STATS, \
sizeof(((struct net_device *)0)->m), \
offsetof(struct net_device, m)
@@ -104,6 +99,7 @@ static const char e1000_gstrings_test[][ETH_GSTRING_LEN] = {
"Interrupt test (offline)", "Loopback test (offline)",
"Link test (on/offline)"
};
+
#define E1000_TEST_LEN ARRAY_SIZE(e1000_gstrings_test)
static int e1000_get_settings(struct net_device *netdev,
@@ -113,7 +109,6 @@ static int e1000_get_settings(struct net_device *netdev,
struct e1000_hw *hw = &adapter->hw;
if (hw->media_type == e1000_media_type_copper) {
-
ecmd->supported = (SUPPORTED_10baseT_Half |
SUPPORTED_10baseT_Full |
SUPPORTED_100baseT_Half |
@@ -155,9 +150,8 @@ static int e1000_get_settings(struct net_device *netdev,
}
if (er32(STATUS) & E1000_STATUS_LU) {
-
e1000_get_speed_and_duplex(hw, &adapter->link_speed,
- &adapter->link_duplex);
+ &adapter->link_duplex);
ethtool_cmd_speed_set(ecmd, adapter->link_speed);
/* unfortunately FULL_DUPLEX != DUPLEX_FULL
@@ -247,9 +241,9 @@ static int e1000_set_settings(struct net_device *netdev,
if (netif_running(adapter->netdev)) {
e1000_down(adapter);
e1000_up(adapter);
- } else
+ } else {
e1000_reset(adapter);
-
+ }
clear_bit(__E1000_RESETTING, &adapter->flags);
return 0;
}
@@ -279,11 +273,11 @@ static void e1000_get_pauseparam(struct net_device *netdev,
pause->autoneg =
(adapter->fc_autoneg ? AUTONEG_ENABLE : AUTONEG_DISABLE);
- if (hw->fc == E1000_FC_RX_PAUSE)
+ if (hw->fc == E1000_FC_RX_PAUSE) {
pause->rx_pause = 1;
- else if (hw->fc == E1000_FC_TX_PAUSE)
+ } else if (hw->fc == E1000_FC_TX_PAUSE) {
pause->tx_pause = 1;
- else if (hw->fc == E1000_FC_FULL) {
+ } else if (hw->fc == E1000_FC_FULL) {
pause->rx_pause = 1;
pause->tx_pause = 1;
}
@@ -316,8 +310,9 @@ static int e1000_set_pauseparam(struct net_device *netdev,
if (netif_running(adapter->netdev)) {
e1000_down(adapter);
e1000_up(adapter);
- } else
+ } else {
e1000_reset(adapter);
+ }
} else
retval = ((hw->media_type == e1000_media_type_fiber) ?
e1000_setup_link(hw) : e1000_force_mac_fc(hw));
@@ -329,12 +324,14 @@ static int e1000_set_pauseparam(struct net_device *netdev,
static u32 e1000_get_msglevel(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
+
return adapter->msg_enable;
}
static void e1000_set_msglevel(struct net_device *netdev, u32 data)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
+
adapter->msg_enable = data;
}
@@ -526,7 +523,7 @@ static int e1000_set_eeprom(struct net_device *netdev,
* only the first byte of the word is being modified
*/
ret_val = e1000_read_eeprom(hw, last_word, 1,
- &eeprom_buff[last_word - first_word]);
+ &eeprom_buff[last_word - first_word]);
}
/* Device's eeprom is always little-endian, word addressable */
@@ -618,13 +615,12 @@ static int e1000_set_ringparam(struct net_device *netdev,
adapter->tx_ring = txdr;
adapter->rx_ring = rxdr;
- rxdr->count = max(ring->rx_pending,(u32)E1000_MIN_RXD);
- rxdr->count = min(rxdr->count,(u32)(mac_type < e1000_82544 ?
+ rxdr->count = max(ring->rx_pending, (u32)E1000_MIN_RXD);
+ rxdr->count = min(rxdr->count, (u32)(mac_type < e1000_82544 ?
E1000_MAX_RXD : E1000_MAX_82544_RXD));
rxdr->count = ALIGN(rxdr->count, REQ_RX_DESCRIPTOR_MULTIPLE);
-
- txdr->count = max(ring->tx_pending,(u32)E1000_MIN_TXD);
- txdr->count = min(txdr->count,(u32)(mac_type < e1000_82544 ?
+ txdr->count = max(ring->tx_pending, (u32)E1000_MIN_TXD);
+ txdr->count = min(txdr->count, (u32)(mac_type < e1000_82544 ?
E1000_MAX_TXD : E1000_MAX_82544_TXD));
txdr->count = ALIGN(txdr->count, REQ_TX_DESCRIPTOR_MULTIPLE);
@@ -680,8 +676,9 @@ static bool reg_pattern_test(struct e1000_adapter *adapter, u64 *data, int reg,
u32 mask, u32 write)
{
struct e1000_hw *hw = &adapter->hw;
- static const u32 test[] =
- {0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF};
+ static const u32 test[] = {
+ 0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF
+ };
u8 __iomem *address = hw->hw_addr + reg;
u32 read;
int i;
@@ -793,8 +790,8 @@ static int e1000_reg_test(struct e1000_adapter *adapter, u64 *data)
REG_PATTERN_TEST(TIDV, 0x0000FFFF, 0x0000FFFF);
value = E1000_RAR_ENTRIES;
for (i = 0; i < value; i++) {
- REG_PATTERN_TEST(RA + (((i << 1) + 1) << 2), 0x8003FFFF,
- 0xFFFFFFFF);
+ REG_PATTERN_TEST(RA + (((i << 1) + 1) << 2),
+ 0x8003FFFF, 0xFFFFFFFF);
}
} else {
REG_SET_AND_CHECK(RCTL, 0xFFFFFFFF, 0x01FFFFFF);
@@ -877,7 +874,6 @@ static int e1000_intr_test(struct e1000_adapter *adapter, u64 *data)
/* Test each interrupt */
for (; i < 10; i++) {
-
/* Interrupt to test */
mask = 1 << i;
@@ -1149,8 +1145,7 @@ static void e1000_phy_reset_clk_and_crs(struct e1000_adapter *adapter)
*/
e1000_read_phy_reg(hw, M88E1000_EXT_PHY_SPEC_CTRL, &phy_reg);
phy_reg |= M88E1000_EPSCR_TX_CLK_25;
- e1000_write_phy_reg(hw,
- M88E1000_EXT_PHY_SPEC_CTRL, phy_reg);
+ e1000_write_phy_reg(hw, M88E1000_EXT_PHY_SPEC_CTRL, phy_reg);
/* In addition, because of the s/w reset above, we need to enable
* CRS on TX. This must be set for both full and half duplex
@@ -1158,8 +1153,7 @@ static void e1000_phy_reset_clk_and_crs(struct e1000_adapter *adapter)
*/
e1000_read_phy_reg(hw, M88E1000_PHY_SPEC_CTRL, &phy_reg);
phy_reg |= M88E1000_PSCR_ASSERT_CRS_ON_TX;
- e1000_write_phy_reg(hw,
- M88E1000_PHY_SPEC_CTRL, phy_reg);
+ e1000_write_phy_reg(hw, M88E1000_PHY_SPEC_CTRL, phy_reg);
}
static int e1000_nonintegrated_phy_loopback(struct e1000_adapter *adapter)
@@ -1216,7 +1210,7 @@ static int e1000_nonintegrated_phy_loopback(struct e1000_adapter *adapter)
/* Check Phy Configuration */
e1000_read_phy_reg(hw, PHY_CTRL, &phy_reg);
if (phy_reg != 0x4100)
- return 9;
+ return 9;
e1000_read_phy_reg(hw, M88E1000_EXT_PHY_SPEC_CTRL, &phy_reg);
if (phy_reg != 0x0070)
@@ -1261,7 +1255,7 @@ static int e1000_integrated_phy_loopback(struct e1000_adapter *adapter)
E1000_CTRL_FD); /* Force Duplex to FULL */
if (hw->media_type == e1000_media_type_copper &&
- hw->phy_type == e1000_phy_m88)
+ hw->phy_type == e1000_phy_m88)
ctrl_reg |= E1000_CTRL_ILOS; /* Invert Loss of Signal */
else {
/* Set the ILOS bit on the fiber Nic is half
@@ -1299,7 +1293,7 @@ static int e1000_set_phy_loopback(struct e1000_adapter *adapter)
* attempt this 10 times.
*/
while (e1000_nonintegrated_phy_loopback(adapter) &&
- count++ < 10);
+ count++ < 10);
if (count < 11)
return 0;
}
@@ -1348,8 +1342,9 @@ static int e1000_setup_loopback_test(struct e1000_adapter *adapter)
ew32(RCTL, rctl);
return 0;
}
- } else if (hw->media_type == e1000_media_type_copper)
+ } else if (hw->media_type == e1000_media_type_copper) {
return e1000_set_phy_loopback(adapter);
+ }
return 7;
}
@@ -1395,9 +1390,9 @@ static int e1000_check_lbtest_frame(struct sk_buff *skb,
unsigned int frame_size)
{
frame_size &= ~1;
- if (*(skb->data + 3) == 0xFF) {
- if ((*(skb->data + frame_size / 2 + 10) == 0xBE) &&
- (*(skb->data + frame_size / 2 + 12) == 0xAF)) {
+ if (skb->data[3] == 0xFF) {
+ if (skb->data[frame_size / 2 + 10] == 0xBE &&
+ skb->data[frame_size / 2 + 12] == 0xAF) {
return 0;
}
}
@@ -1410,7 +1405,7 @@ static int e1000_run_loopback_test(struct e1000_adapter *adapter)
struct e1000_tx_ring *txdr = &adapter->test_tx_ring;
struct e1000_rx_ring *rxdr = &adapter->test_rx_ring;
struct pci_dev *pdev = adapter->pdev;
- int i, j, k, l, lc, good_cnt, ret_val=0;
+ int i, j, k, l, lc, good_cnt, ret_val = 0;
unsigned long time;
ew32(RDT, rxdr->count - 1);
@@ -1429,12 +1424,13 @@ static int e1000_run_loopback_test(struct e1000_adapter *adapter)
for (j = 0; j <= lc; j++) { /* loop count loop */
for (i = 0; i < 64; i++) { /* send the packets */
e1000_create_lbtest_frame(txdr->buffer_info[i].skb,
- 1024);
+ 1024);
dma_sync_single_for_device(&pdev->dev,
txdr->buffer_info[k].dma,
txdr->buffer_info[k].length,
DMA_TO_DEVICE);
- if (unlikely(++k == txdr->count)) k = 0;
+ if (unlikely(++k == txdr->count))
+ k = 0;
}
ew32(TDT, k);
E1000_WRITE_FLUSH();
@@ -1452,7 +1448,8 @@ static int e1000_run_loopback_test(struct e1000_adapter *adapter)
1024);
if (!ret_val)
good_cnt++;
- if (unlikely(++l == rxdr->count)) l = 0;
+ if (unlikely(++l == rxdr->count))
+ l = 0;
/* time + 20 msecs (200 msecs on 2.4) is more than
* enough time to complete the receives, if it's
* exceeded, break and error off
@@ -1494,6 +1491,7 @@ static int e1000_link_test(struct e1000_adapter *adapter, u64 *data)
*data = 0;
if (hw->media_type == e1000_media_type_internal_serdes) {
int i = 0;
+
hw->serdes_has_link = false;
/* On some blade server designs, link establishment
@@ -1512,9 +1510,8 @@ static int e1000_link_test(struct e1000_adapter *adapter, u64 *data)
if (hw->autoneg) /* if auto_neg is set wait for it */
msleep(4000);
- if (!(er32(STATUS) & E1000_STATUS_LU)) {
+ if (!(er32(STATUS) & E1000_STATUS_LU))
*data = 1;
- }
}
return *data;
}
@@ -1665,8 +1662,7 @@ static void e1000_get_wol(struct net_device *netdev,
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
- wol->supported = WAKE_UCAST | WAKE_MCAST |
- WAKE_BCAST | WAKE_MAGIC;
+ wol->supported = WAKE_UCAST | WAKE_MCAST | WAKE_BCAST | WAKE_MAGIC;
wol->wolopts = 0;
/* this function will set ->supported = 0 and return 1 if wol is not
@@ -1819,6 +1815,7 @@ static int e1000_set_coalesce(struct net_device *netdev,
static int e1000_nway_reset(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
+
if (netif_running(netdev))
e1000_reinit_locked(adapter);
return 0;
@@ -1830,22 +1827,29 @@ static void e1000_get_ethtool_stats(struct net_device *netdev,
struct e1000_adapter *adapter = netdev_priv(netdev);
int i;
char *p = NULL;
+ const struct e1000_stats *stat = e1000_gstrings_stats;
e1000_update_stats(adapter);
for (i = 0; i < E1000_GLOBAL_STATS_LEN; i++) {
- switch (e1000_gstrings_stats[i].type) {
+ switch (stat->type) {
case NETDEV_STATS:
- p = (char *) netdev +
- e1000_gstrings_stats[i].stat_offset;
+ p = (char *)netdev + stat->stat_offset;
break;
case E1000_STATS:
- p = (char *) adapter +
- e1000_gstrings_stats[i].stat_offset;
+ p = (char *)adapter + stat->stat_offset;
+ break;
+ default:
+ WARN_ONCE(1, "Invalid E1000 stat type: %u index %d\n",
+ stat->type, i);
break;
}
- data[i] = (e1000_gstrings_stats[i].sizeof_stat ==
- sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
+ if (stat->sizeof_stat == sizeof(u64))
+ data[i] = *(u64 *)p;
+ else
+ data[i] = *(u32 *)p;
+
+ stat++;
}
/* BUG_ON(i != E1000_STATS_LEN); */
}
@@ -1858,8 +1862,7 @@ static void e1000_get_strings(struct net_device *netdev, u32 stringset,
switch (stringset) {
case ETH_SS_TEST:
- memcpy(data, *e1000_gstrings_test,
- sizeof(e1000_gstrings_test));
+ memcpy(data, e1000_gstrings_test, sizeof(e1000_gstrings_test));
break;
case ETH_SS_STATS:
for (i = 0; i < E1000_GLOBAL_STATS_LEN; i++) {
--
1.9.3
^ permalink raw reply related
* Re: [RFC net-next PATCH V2 2/3] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Eric Dumazet @ 2014-09-06 12:55 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, David S. Miller, Tom Herbert, Hannes Frederic Sowa,
Florian Westphal, Daniel Borkmann, Jamal Hadi Salim,
Alexander Duyck, John Fastabend
In-Reply-To: <20140905102815.157fda5a@redhat.com>
On Fri, 2014-09-05 at 10:28 +0200, Jesper Dangaard Brouer wrote:
>
> So, I cannot see if it is a TSO packet via the skb_is_gso(skb) check?
> Will skb->len be the length of the full TSO packet size?
There is no difference between a TSO and GSO packet. They are what we
call a GSO packet.
The difference lies on the device ability to perform TSO.
If device is not able to send a TSO packet, packet is segmented by the
core networking stack.
^ permalink raw reply
* [PATCH net-next 0/6] bonding: get rid of bond->lock
From: Nikolay Aleksandrov @ 2014-09-06 13:59 UTC (permalink / raw)
To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
Hi all,
This patch-set removes the last users of bond->lock and converts the places
that needed it for sync to use curr_slave_lock or RCU as appropriate.
I've run this with lockdep and have stress-tested it via loading/unloading
and enslaving/releasing in parallel while outputting bond's proc, I didn't
see any issues. Please pay special attention to the procfs change, I've
done about an hour of stress-testing on it and have checked that the event
that causes the bonding to delete its proc entry (NETDEV_UNREGISTER) is
called before ndo_uninit() and the freeing of the dev so any readers will
sync with that. Also ran sparse checks and there were no splats.
Changes from the RFC:
use RCU in procfs instead of RTNL since RTNL might lead to a deadlock with
unloading and also is much slower. The bond destruction syncs with proc
via the proc locks. There's one new patch that converts primary_slave to
use RCU as it was necessary to fix a longstanding bugs in sysfs and
procfs and to make it easy to migrate bond's procfs to RCU. And of course
rebased on top of net-next current.
This is the first patch-set in a series that should simplify the bond's
locking requirements and will make it easier to define the locking
conditions necessary for the various paths. The goal is to rely on RTNL
and rcu alone, an extra lock would be needed in a few special cases that
would be documented very well.
Best regards,
Nikolay Aleksandrov
Nikolay Aleksandrov (6):
bonding: 3ad: use curr_slave_lock instead of bond->lock
bonding: alb: clean bond->lock
bonding: convert primary_slave to use RCU
bonding: procfs: clean bond->lock usage and use RCU
bonding: options: remove bond->lock usage
bonding: remove last users of bond->lock and bond->lock itself
drivers/net/bonding/bond_3ad.c | 9 ++--
drivers/net/bonding/bond_alb.c | 11 +----
drivers/net/bonding/bond_main.c | 93 ++++++++++++--------------------------
drivers/net/bonding/bond_netlink.c | 7 +--
drivers/net/bonding/bond_options.c | 27 ++---------
drivers/net/bonding/bond_procfs.c | 24 +++-------
drivers/net/bonding/bond_sysfs.c | 10 ++--
drivers/net/bonding/bonding.h | 10 ++--
8 files changed, 62 insertions(+), 129 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH net-next 1/6] bonding: 3ad: use curr_slave_lock instead of bond->lock
From: Nikolay Aleksandrov @ 2014-09-06 13:59 UTC (permalink / raw)
To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
In-Reply-To: <1410011971-24922-1-git-send-email-nikolay@redhat.com>
In 3ad mode the only syncing needed by bond->lock is for the wq
and the recv handler, so change them to use curr_slave_lock.
There're no locking dependencies here as 3ad doesn't use
curr_slave_lock at all.
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_3ad.c | 9 ++++-----
drivers/net/bonding/bond_main.c | 12 +++++++-----
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index ee2c73a9de39..5d27a6207384 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2057,7 +2057,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
struct port *port;
bool should_notify_rtnl = BOND_SLAVE_NOTIFY_LATER;
- read_lock(&bond->lock);
+ read_lock(&bond->curr_slave_lock);
rcu_read_lock();
/* check if there are any slaves */
@@ -2120,7 +2120,7 @@ re_arm:
}
}
rcu_read_unlock();
- read_unlock(&bond->lock);
+ read_unlock(&bond->curr_slave_lock);
if (should_notify_rtnl && rtnl_trylock()) {
bond_slave_state_notify(bond);
@@ -2395,7 +2395,6 @@ int __bond_3ad_get_active_agg_info(struct bonding *bond,
return 0;
}
-/* Wrapper used to hold bond->lock so no slave manipulation can occur */
int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
{
int ret;
@@ -2487,9 +2486,9 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
if (!lacpdu)
return ret;
- read_lock(&bond->lock);
+ read_lock(&bond->curr_slave_lock);
ret = bond_3ad_rx_indication(lacpdu, slave, skb->len);
- read_unlock(&bond->lock);
+ read_unlock(&bond->curr_slave_lock);
return ret;
}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f0f5eab0fab1..dcd331bd0c17 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1687,13 +1687,15 @@ static int __bond_release_one(struct net_device *bond_dev,
* for this slave anymore.
*/
netdev_rx_handler_unregister(slave_dev);
- write_lock_bh(&bond->lock);
- /* Inform AD package of unbinding of slave. */
- if (BOND_MODE(bond) == BOND_MODE_8023AD)
+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+ /* Sync against bond_3ad_rx_indication and
+ * bond_3ad_state_machine_handler
+ */
+ write_lock_bh(&bond->curr_slave_lock);
bond_3ad_unbind_slave(slave);
-
- write_unlock_bh(&bond->lock);
+ write_unlock_bh(&bond->curr_slave_lock);
+ }
netdev_info(bond_dev, "Releasing %s interface %s\n",
bond_is_active_slave(slave) ? "active" : "backup",
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 2/6] bonding: alb: clean bond->lock
From: Nikolay Aleksandrov @ 2014-09-06 13:59 UTC (permalink / raw)
To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
In-Reply-To: <1410011971-24922-1-git-send-email-nikolay@redhat.com>
We can remove the lock/unlock as it's no longer necessary since
RTNL should be held while calling bond_alb_set_mac_address().
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_alb.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 73c21e233131..028496205f39 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1775,8 +1775,7 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
* Set the bond->curr_active_slave to @new_slave and handle
* mac address swapping and promiscuity changes as needed.
*
- * If new_slave is NULL, caller must hold curr_slave_lock or
- * bond->lock for write.
+ * If new_slave is NULL, caller must hold curr_slave_lock for write
*
* If new_slave is not NULL, caller must hold RTNL, curr_slave_lock
* for write. Processing here may sleep, so no other locks may be held.
@@ -1857,12 +1856,8 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
write_lock_bh(&bond->curr_slave_lock);
}
-/*
- * Called with RTNL
- */
+/* Called with RTNL */
int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
- __acquires(&bond->lock)
- __releases(&bond->lock)
{
struct bonding *bond = netdev_priv(bond_dev);
struct sockaddr *sa = addr;
@@ -1895,14 +1890,12 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
} else {
alb_set_slave_mac_addr(curr_active, bond_dev->dev_addr);
- read_lock(&bond->lock);
alb_send_learning_packets(curr_active,
bond_dev->dev_addr, false);
if (bond->alb_info.rlb_enabled) {
/* inform clients mac address has changed */
rlb_req_update_slave_clients(bond, curr_active);
}
- read_unlock(&bond->lock);
}
return 0;
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 3/6] bonding: convert primary_slave to use RCU
From: Nikolay Aleksandrov @ 2014-09-06 13:59 UTC (permalink / raw)
To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
In-Reply-To: <1410011971-24922-1-git-send-email-nikolay@redhat.com>
This is necessary mainly for two bonding call sites: procfs and
sysfs as it was dereferenced without any real protection.
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
Some of the primary_slave change sites could use RCU_INIT_POINTER() instead
of rcu_assign_pointer as they don't need the barriers, but I've left it
like this to clearly show the intention, in case this is a problem we can
always switch them to RCU_INIT_POINTER().
drivers/net/bonding/bond_main.c | 42 +++++++++++++++++++++-----------------
drivers/net/bonding/bond_netlink.c | 7 ++++---
drivers/net/bonding/bond_options.c | 8 ++++----
drivers/net/bonding/bond_procfs.c | 8 ++++----
drivers/net/bonding/bond_sysfs.c | 10 ++++++---
drivers/net/bonding/bonding.h | 2 +-
6 files changed, 43 insertions(+), 34 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index dcd331bd0c17..629037f79213 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -708,7 +708,7 @@ out:
static bool bond_should_change_active(struct bonding *bond)
{
- struct slave *prim = bond->primary_slave;
+ struct slave *prim = rtnl_dereference(bond->primary_slave);
struct slave *curr = bond_deref_active_protected(bond);
if (!prim || !curr || curr->link != BOND_LINK_UP)
@@ -732,13 +732,14 @@ static bool bond_should_change_active(struct bonding *bond)
*/
static struct slave *bond_find_best_slave(struct bonding *bond)
{
- struct slave *slave, *bestslave = NULL;
+ struct slave *slave, *bestslave = NULL, *primary;
struct list_head *iter;
int mintime = bond->params.updelay;
- if (bond->primary_slave && bond->primary_slave->link == BOND_LINK_UP &&
+ primary = rtnl_dereference(bond->primary_slave);
+ if (primary && primary->link == BOND_LINK_UP &&
bond_should_change_active(bond))
- return bond->primary_slave;
+ return primary;
bond_for_each_slave(bond, slave, iter) {
if (slave->link == BOND_LINK_UP)
@@ -1482,7 +1483,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
if (bond_uses_primary(bond) && bond->params.primary[0]) {
/* if there is a primary slave, remember it */
if (strcmp(bond->params.primary, new_slave->dev->name) == 0) {
- bond->primary_slave = new_slave;
+ rcu_assign_pointer(bond->primary_slave, new_slave);
bond->force_primary = true;
}
}
@@ -1596,8 +1597,8 @@ err_detach:
bond_hw_addr_flush(bond_dev, slave_dev);
vlan_vids_del_by_dev(slave_dev, bond_dev);
- if (bond->primary_slave == new_slave)
- bond->primary_slave = NULL;
+ if (rcu_access_pointer(bond->primary_slave) == new_slave)
+ RCU_INIT_POINTER(bond->primary_slave, NULL);
if (rcu_access_pointer(bond->curr_active_slave) == new_slave) {
block_netpoll_tx();
write_lock_bh(&bond->curr_slave_lock);
@@ -1606,6 +1607,8 @@ err_detach:
write_unlock_bh(&bond->curr_slave_lock);
unblock_netpoll_tx();
}
+ /* either primary_slave or curr_active_slave might've changed */
+ synchronize_rcu();
slave_disable_netpoll(new_slave);
err_close:
@@ -1714,8 +1717,8 @@ static int __bond_release_one(struct net_device *bond_dev,
bond_dev->name, slave_dev->name);
}
- if (bond->primary_slave == slave)
- bond->primary_slave = NULL;
+ if (rtnl_dereference(bond->primary_slave) == slave)
+ RCU_INIT_POINTER(bond->primary_slave, NULL);
if (oldcurrent == slave) {
write_lock_bh(&bond->curr_slave_lock);
@@ -1976,7 +1979,7 @@ static int bond_miimon_inspect(struct bonding *bond)
static void bond_miimon_commit(struct bonding *bond)
{
struct list_head *iter;
- struct slave *slave;
+ struct slave *slave, *primary;
bond_for_each_slave(bond, slave, iter) {
switch (slave->new_link) {
@@ -1987,13 +1990,14 @@ static void bond_miimon_commit(struct bonding *bond)
slave->link = BOND_LINK_UP;
slave->last_link_up = jiffies;
+ primary = rtnl_dereference(bond->primary_slave);
if (BOND_MODE(bond) == BOND_MODE_8023AD) {
/* prevent it from being the active one */
bond_set_backup_slave(slave);
} else if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) {
/* make it immediately active */
bond_set_active_slave(slave);
- } else if (slave != bond->primary_slave) {
+ } else if (slave != primary) {
/* prevent it from being the active one */
bond_set_backup_slave(slave);
}
@@ -2011,8 +2015,7 @@ static void bond_miimon_commit(struct bonding *bond)
bond_alb_handle_link_change(bond, slave,
BOND_LINK_UP);
- if (!bond->curr_active_slave ||
- (slave == bond->primary_slave))
+ if (!bond->curr_active_slave || slave == primary)
goto do_failover;
continue;
@@ -2633,7 +2636,7 @@ static void bond_ab_arp_commit(struct bonding *bond)
slave->dev->name);
if (!rtnl_dereference(bond->curr_active_slave) ||
- (slave == bond->primary_slave))
+ slave == rtnl_dereference(bond->primary_slave))
goto do_failover;
}
@@ -2860,7 +2863,7 @@ static int bond_master_netdev_event(unsigned long event,
static int bond_slave_netdev_event(unsigned long event,
struct net_device *slave_dev)
{
- struct slave *slave = bond_slave_get_rtnl(slave_dev);
+ struct slave *slave = bond_slave_get_rtnl(slave_dev), *primary;
struct bonding *bond;
struct net_device *bond_dev;
u32 old_speed;
@@ -2874,6 +2877,7 @@ static int bond_slave_netdev_event(unsigned long event,
return NOTIFY_DONE;
bond_dev = slave->bond->dev;
bond = slave->bond;
+ primary = rtnl_dereference(bond->primary_slave);
switch (event) {
case NETDEV_UNREGISTER:
@@ -2921,18 +2925,18 @@ static int bond_slave_netdev_event(unsigned long event,
!bond->params.primary[0])
break;
- if (slave == bond->primary_slave) {
+ if (slave == primary) {
/* slave's name changed - he's no longer primary */
- bond->primary_slave = NULL;
+ RCU_INIT_POINTER(bond->primary_slave, NULL);
} else if (!strcmp(slave_dev->name, bond->params.primary)) {
/* we have a new primary slave */
- bond->primary_slave = slave;
+ rcu_assign_pointer(bond->primary_slave, slave);
} else { /* we didn't change primary - exit */
break;
}
netdev_info(bond->dev, "Primary slave changed to %s, reselecting active slave\n",
- bond->primary_slave ? slave_dev->name : "none");
+ primary ? slave_dev->name : "none");
block_netpoll_tx();
write_lock_bh(&bond->curr_slave_lock);
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index e1489d9df2a4..c13d83e15ace 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -443,6 +443,7 @@ static int bond_fill_info(struct sk_buff *skb,
unsigned int packets_per_slave;
int ifindex, i, targets_added;
struct nlattr *targets;
+ struct slave *primary;
if (nla_put_u8(skb, IFLA_BOND_MODE, BOND_MODE(bond)))
goto nla_put_failure;
@@ -492,9 +493,9 @@ static int bond_fill_info(struct sk_buff *skb,
bond->params.arp_all_targets))
goto nla_put_failure;
- if (bond->primary_slave &&
- nla_put_u32(skb, IFLA_BOND_PRIMARY,
- bond->primary_slave->dev->ifindex))
+ primary = rtnl_dereference(bond->primary_slave);
+ if (primary &&
+ nla_put_u32(skb, IFLA_BOND_PRIMARY, primary->dev->ifindex))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_PRIMARY_RESELECT,
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index d8dc17faa6b4..7c9e176baecc 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -1090,7 +1090,7 @@ static int bond_option_primary_set(struct bonding *bond,
/* check to see if we are clearing primary */
if (!strlen(primary)) {
netdev_info(bond->dev, "Setting primary slave to None\n");
- bond->primary_slave = NULL;
+ RCU_INIT_POINTER(bond->primary_slave, NULL);
memset(bond->params.primary, 0, sizeof(bond->params.primary));
bond_select_active_slave(bond);
goto out;
@@ -1100,16 +1100,16 @@ static int bond_option_primary_set(struct bonding *bond,
if (strncmp(slave->dev->name, primary, IFNAMSIZ) == 0) {
netdev_info(bond->dev, "Setting %s as primary slave\n",
slave->dev->name);
- bond->primary_slave = slave;
+ rcu_assign_pointer(bond->primary_slave, slave);
strcpy(bond->params.primary, slave->dev->name);
bond_select_active_slave(bond);
goto out;
}
}
- if (bond->primary_slave) {
+ if (rtnl_dereference(bond->primary_slave)) {
netdev_info(bond->dev, "Setting primary slave to None\n");
- bond->primary_slave = NULL;
+ RCU_INIT_POINTER(bond->primary_slave, NULL);
bond_select_active_slave(bond);
}
strncpy(bond->params.primary, primary, IFNAMSIZ);
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index de62c0385dfb..1a9fe1ba4c60 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -66,7 +66,7 @@ static void bond_info_show_master(struct seq_file *seq)
{
struct bonding *bond = seq->private;
const struct bond_opt_value *optval;
- struct slave *curr;
+ struct slave *curr, *primary;
int i;
curr = rcu_dereference(bond->curr_active_slave);
@@ -92,10 +92,10 @@ static void bond_info_show_master(struct seq_file *seq)
}
if (bond_uses_primary(bond)) {
+ primary = rcu_dereference(bond->primary_slave);
seq_printf(seq, "Primary Slave: %s",
- (bond->primary_slave) ?
- bond->primary_slave->dev->name : "None");
- if (bond->primary_slave) {
+ primary ? primary->dev->name : "None");
+ if (primary) {
optval = bond_opt_get_val(BOND_OPT_PRIMARY_RESELECT,
bond->params.primary_reselect);
seq_printf(seq, " (primary_reselect %s)",
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 98db8edd9c75..5555517284db 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -425,11 +425,15 @@ static ssize_t bonding_show_primary(struct device *d,
struct device_attribute *attr,
char *buf)
{
- int count = 0;
struct bonding *bond = to_bond(d);
+ struct slave *primary;
+ int count = 0;
- if (bond->primary_slave)
- count = sprintf(buf, "%s\n", bond->primary_slave->dev->name);
+ rcu_read_lock();
+ primary = rcu_dereference(bond->primary_slave);
+ if (primary)
+ count = sprintf(buf, "%s\n", primary->dev->name);
+ rcu_read_unlock();
return count;
}
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index aace510d08d1..c798561a6f01 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -195,7 +195,7 @@ struct bonding {
struct net_device *dev; /* first - useful for panic debug */
struct slave __rcu *curr_active_slave;
struct slave __rcu *current_arp_slave;
- struct slave *primary_slave;
+ struct slave __rcu *primary_slave;
bool force_primary;
s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
int (*recv_probe)(const struct sk_buff *, struct bonding *,
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 4/6] bonding: procfs: clean bond->lock usage and use RCU
From: Nikolay Aleksandrov @ 2014-09-06 13:59 UTC (permalink / raw)
To: netdev; +Cc: vfalico, j.vosburgh, andy, davem, Nikolay Aleksandrov
In-Reply-To: <1410011971-24922-1-git-send-email-nikolay@redhat.com>
Use RCU to protect against slave release, the proc show function will sync
with the bond destruction by the proc locks and the fact that the bond is
released after NETDEV_UNREGISTER which causes the bonding to remove the
proc entry.
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_procfs.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 1a9fe1ba4c60..bb09d0442aa8 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -7,21 +7,18 @@
static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos)
__acquires(RCU)
- __acquires(&bond->lock)
{
struct bonding *bond = seq->private;
struct list_head *iter;
struct slave *slave;
loff_t off = 0;
- /* make sure the bond won't be taken away */
rcu_read_lock();
- read_lock(&bond->lock);
if (*pos == 0)
return SEQ_START_TOKEN;
- bond_for_each_slave(bond, slave, iter)
+ bond_for_each_slave_rcu(bond, slave, iter)
if (++off == *pos)
return slave;
@@ -37,12 +34,9 @@ static void *bond_info_seq_next(struct seq_file *seq, void *v, loff_t *pos)
++*pos;
if (v == SEQ_START_TOKEN)
- return bond_first_slave(bond);
+ return bond_first_slave_rcu(bond);
- if (bond_is_last_slave(bond, v))
- return NULL;
-
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
if (found)
return slave;
if (slave == v)
@@ -53,12 +47,8 @@ static void *bond_info_seq_next(struct seq_file *seq, void *v, loff_t *pos)
}
static void bond_info_seq_stop(struct seq_file *seq, void *v)
- __releases(&bond->lock)
__releases(RCU)
{
- struct bonding *bond = seq->private;
-
- read_unlock(&bond->lock);
rcu_read_unlock();
}
--
1.9.3
^ permalink raw reply related
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