* [PATCH net-next v2 0/10] bonding: rebuild the lock use for bond monitor
From: Ding Tianhong @ 2013-11-08 2:07 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
Now the bond slave list is not protected by bond lock, only by RTNL,
but the monitor still use the bond lock to protect the slave list,
it is useless, according to the Veaceslav's opinion, there were
three way to fix the protect problem:
1. add bond_master_upper_dev_link() and bond_upper_dev_unlink()
in bond->lock, but it is unsafe to call call_netdevice_notifiers()
in write lock.
2. remove unused bond->lock for monitor function, only use the exist
rtnl lock(), it will take performance loss in fast path.
3. use RCU to protect the slave list, of course, performance is better,
but in slow path, it is ignored.
obviously the solution 1 is not fit here, I will consider the 2 and 3
solution. My principle is simple, if in fast path, RCU is better,
otherwise in slow path, both is well, but according to the Jay Vosburgh's
opinion, the monitor will loss performace if use RTNL to protect the all
slave list, so remove the bond lock and replace with RCU.
The second problem is the curr_slave_lock for bond, it is too old and
unwanted in many place, because the curr_active_slave would only be
changed in 3 place:
1. enslave slave.
2. release slave.
3. change active slave.
all above were already holding bond lock, RTNL and curr_slave_lock
together, it is tedious and no need to add so mach lock, when change
the curr_active_slave, you have to hold the RTNL and curr_slave_lock
together, and when you read the curr_active_slave, RTNL or curr_slave_lock,
any one of them is no problem.
for the stability, I did not change the logic for the monitor,
all change is clear and simple, I have test the patch set for lockdep,
it work well and stability.
v2. accept the Jay Vosburgh's opinion, remove the RTNL and replace with RCU,
also add some rcu function for bond use, so the patch set reach 10.
Best Regards
Ding Tianhong
Ding Tianhong (10):
bonding: remove the no effect lock for bond_select_active_slave()
bonding: rebuild the lock use for bond_mii_monitor()
bonding: rebuild the lock use for bond_alb_monitor()
bonding: rebuild the lock use for bond_loadbalance_arp_mon()
net: add and export netdev_adjacent_get_private_rcu()
bonding: rebuild the lock use for bond_activebackup_arp_mon()
bonding: rebuild the lock use for bond_3ad_state_machine_handler()
bonding: remove unwanted lock for bond_option_active_slave_set()
bonding: remvoe unwanted lock for bond enslave and release
bonding: remove unwanted lock for bond_store_primaryxxx()
drivers/net/bonding/bond_3ad.c | 21 +++---
drivers/net/bonding/bond_alb.c | 33 ++++-----
drivers/net/bonding/bond_main.c | 137 +++++++++++++++++--------------------
drivers/net/bonding/bond_options.c | 2 -
drivers/net/bonding/bond_sysfs.c | 4 --
drivers/net/bonding/bonding.h | 13 ++++
include/linux/netdevice.h | 1 +
net/core/dev.c | 10 +++
8 files changed, 108 insertions(+), 113 deletions(-)
--
1.8.2.1
^ permalink raw reply
* [PATCH net-next v2 2/10] bonding: rebuild the lock use for bond_mii_monitor()
From: Ding Tianhong @ 2013-11-08 2:07 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_mii_monitor() still use bond lock to protect bond slave list,
it is no effect, I have 2 way to fix the problem, move the RTNL to the
top of the function, or add RCU to protect the bond_has_slaves() and
bond_miimon_inspect(), according to the Jay Vosburgh's opinion, 10 times
one second is a truely big performance loss if use RTNL to protect the
whole function, so I would take the advice and use RCU to protect the
two functions, of course it need to add more modify, the
bond_has_slave_rcu() is add for RCU use, and the bond_for_each_slave
need to replace with bond_for_each_slave_rcu in bond_miimon_inspect.
I move the peer notify before the queue_delayed_work(), and obviously
it is no need to lock the RTNL twice if call bond_miimon_commit() and
peer notify together, other path is no logic change, I think the
performance is better than before.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 40 ++++++++++++++++++++--------------------
drivers/net/bonding/bonding.h | 6 ++++++
2 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ba18719..def489d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1913,7 +1913,7 @@ static int bond_miimon_inspect(struct bonding *bond)
ignore_updelay = !bond->curr_active_slave ? true : false;
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
slave->new_link = BOND_LINK_NOCHANGE;
link_state = bond_check_dev_link(bond, slave->dev, 0);
@@ -2111,47 +2111,47 @@ void bond_mii_monitor(struct work_struct *work)
bool should_notify_peers = false;
unsigned long delay;
- read_lock(&bond->lock);
-
delay = msecs_to_jiffies(bond->params.miimon);
- if (!bond_has_slaves(bond))
+ rcu_read_lock();
+
+ if (!bond_has_slaves_rcu(bond)) {
+ rcu_read_unlock();
goto re_arm;
+ }
should_notify_peers = bond_should_notify_peers(bond);
if (bond_miimon_inspect(bond)) {
- read_unlock(&bond->lock);
+ rcu_read_unlock();
/* Race avoidance with bond_close cancel of workqueue */
if (!rtnl_trylock()) {
- read_lock(&bond->lock);
delay = 1;
- should_notify_peers = false;
goto re_arm;
}
- read_lock(&bond->lock);
-
bond_miimon_commit(bond);
- read_unlock(&bond->lock);
+ if (should_notify_peers)
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+ bond->dev);
+
rtnl_unlock(); /* might sleep, hold no other locks */
- read_lock(&bond->lock);
+ } else {
+ rcu_read_unlock();
+ if (should_notify_peers) {
+ if (!rtnl_trylock())
+ goto re_arm;
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+ bond->dev);
+ rtnl_unlock();
+ }
}
re_arm:
if (bond->params.miimon)
queue_delayed_work(bond->wq, &bond->mii_work, delay);
-
- read_unlock(&bond->lock);
-
- if (should_notify_peers) {
- if (!rtnl_trylock())
- return;
- call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
- rtnl_unlock();
- }
}
static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 046a605..deb9738 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -81,6 +81,12 @@
#define bond_has_slaves(bond) !list_empty(bond_slave_list(bond))
+#define bond_has_slaves_rcu(bond) \
+ ({struct list_head *__ptr = (bond_slave_list(bond)); \
+ struct list_head *__next = ACCESS_ONCE(__ptr->next); \
+ __ptr != __next; \
+ })
+
/* IMPORTANT: bond_first/last_slave can return NULL in case of an empty list */
#define bond_first_slave(bond) \
(bond_has_slaves(bond) ? \
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v2 1/10] bonding: remove the no effect lock for bond_select_active_slave()
From: Ding Tianhong @ 2013-11-08 2:07 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond slave list was no longer protected by bond lock and only
protected by RTNL or RCU, so anywhere that use bond lock to protect
slave list is meaningless.
The curr_active_slave could only be changed in 3 place:
1. enslave slave.
2. release slave.
3. change_active_slave.
all above place were holding bond lock, RTNL and curr_slave_lock
together, it is tedious and meaningless, obviously bond lock is no
need here, but RTNL or curr_slave_lock is needed, so if you want
to access active slave, you have to choose one lock, RTNL or
curr_slave_lock, if RTNL is exist, no need to add curr_slave_lock,
otherwise curr_slave_lock is better, because of the performance.
there are several place calling bond_select_active_slave() and
bond_change_active_slave(), the next step I will clean these place
and remove the no effect lock.
there are some document changed together when update the function.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_alb.c | 12 +++---------
drivers/net/bonding/bond_main.c | 15 +++------------
2 files changed, 6 insertions(+), 21 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 0287240..1fae915 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -470,7 +470,7 @@ static void rlb_teach_disabled_mac_on_primary(struct bonding *bond, u8 addr[])
/* slave being removed should not be active at this point
*
- * Caller must hold bond lock for read
+ * Caller must hold rtnl.
*/
static void rlb_clear_slave(struct bonding *bond, struct slave *slave)
{
@@ -1680,14 +1680,11 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
* If new_slave is NULL, caller must hold curr_slave_lock or
* bond->lock for write.
*
- * If new_slave is not NULL, caller must hold RTNL, bond->lock for
- * read and curr_slave_lock for write. Processing here may sleep, so
- * no other locks may be held.
+ * 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.
*/
void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave)
__releases(&bond->curr_slave_lock)
- __releases(&bond->lock)
- __acquires(&bond->lock)
__acquires(&bond->curr_slave_lock)
{
struct slave *swap_slave;
@@ -1723,7 +1720,6 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
tlb_clear_slave(bond, new_slave, 1);
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
ASSERT_RTNL();
@@ -1749,11 +1745,9 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
/* swap mac address */
alb_swap_mac_addr(swap_slave, new_slave);
alb_fasten_mac_swap(bond, swap_slave, new_slave);
- read_lock(&bond->lock);
} else {
/* set the new_slave to the bond mac address */
alb_set_slave_mac_addr(new_slave, bond->dev->dev_addr);
- read_lock(&bond->lock);
alb_send_learning_packets(new_slave, bond->dev->dev_addr);
}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a141f40..ba18719 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -691,14 +691,12 @@ static void bond_set_dev_addr(struct net_device *bond_dev,
*
* Perform special MAC address swapping for fail_over_mac settings
*
- * Called with RTNL, bond->lock for read, curr_slave_lock for write_bh.
+ * Called with RTNL, curr_slave_lock for write_bh.
*/
static void bond_do_fail_over_mac(struct bonding *bond,
struct slave *new_active,
struct slave *old_active)
__releases(&bond->curr_slave_lock)
- __releases(&bond->lock)
- __acquires(&bond->lock)
__acquires(&bond->curr_slave_lock)
{
u8 tmp_mac[ETH_ALEN];
@@ -709,9 +707,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
case BOND_FOM_ACTIVE:
if (new_active) {
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
bond_set_dev_addr(bond->dev, new_active->dev);
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
}
break;
@@ -725,7 +721,6 @@ static void bond_do_fail_over_mac(struct bonding *bond,
return;
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
if (old_active) {
memcpy(tmp_mac, new_active->dev->dev_addr, ETH_ALEN);
@@ -755,7 +750,6 @@ static void bond_do_fail_over_mac(struct bonding *bond,
pr_err("%s: Error %d setting MAC of slave %s\n",
bond->dev->name, -rv, new_active->dev->name);
out:
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
break;
default:
@@ -840,8 +834,7 @@ static bool bond_should_notify_peers(struct bonding *bond)
* because it is apparently the best available slave we have, even though its
* updelay hasn't timed out yet.
*
- * If new_active is not NULL, caller must hold bond->lock for read and
- * curr_slave_lock for write_bh.
+ * If new_active is not NULL, caller must hold curr_slave_lock for write_bh.
*/
void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
{
@@ -910,14 +903,12 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
}
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
if (should_notify_peers)
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
bond->dev);
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
}
}
@@ -943,7 +934,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
* - The primary_slave has got its link back.
* - A slave has got its link back and there's no old curr_active_slave.
*
- * Caller must hold bond->lock for read and curr_slave_lock for write_bh.
+ * Caller must hold curr_slave_lock for write_bh.
*/
void bond_select_active_slave(struct bonding *bond)
{
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v2 3/10] bonding: rebuild the lock use for bond_alb_monitor()
From: Ding Tianhong @ 2013-11-08 2:07 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_alb_monitor use bond lock to protect the bond slave list,
it is no effect here, we need to use RTNL or RCU to replace bond lock,
the bond_alb_monitor will called 10 times one second, RTNL may loss
performance here, so the bond lock replace with RCU to protect the
bond slave list, also the RTNL is preserved, the logic of the monitor
did not changed.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_alb.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 1fae915..ffdb91b 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -816,7 +816,7 @@ static void rlb_rebalance(struct bonding *bond)
for (; hash_index != RLB_NULL_INDEX;
hash_index = client_info->used_next) {
client_info = &(bond_info->rx_hashtbl[hash_index]);
- assigned_slave = rlb_next_rx_slave(bond);
+ assigned_slave = __rlb_next_rx_slave(bond);
if (assigned_slave && (client_info->slave != assigned_slave)) {
client_info->slave = assigned_slave;
client_info->ntt = 1;
@@ -1495,9 +1495,10 @@ void bond_alb_monitor(struct work_struct *work)
struct list_head *iter;
struct slave *slave;
- read_lock(&bond->lock);
+ rcu_read_lock();
- if (!bond_has_slaves(bond)) {
+ if (!bond_has_slaves_rcu(bond)) {
+ rcu_read_unlock();
bond_info->tx_rebalance_counter = 0;
bond_info->lp_counter = 0;
goto re_arm;
@@ -1528,7 +1529,7 @@ void bond_alb_monitor(struct work_struct *work)
read_lock(&bond->curr_slave_lock);
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
tlb_clear_slave(bond, slave, 1);
if (slave == bond->curr_active_slave) {
SLAVE_TLB_INFO(slave).load =
@@ -1552,11 +1553,9 @@ void bond_alb_monitor(struct work_struct *work)
* dev_set_promiscuity requires rtnl and
* nothing else. Avoid race with bond_close.
*/
- read_unlock(&bond->lock);
- if (!rtnl_trylock()) {
- read_lock(&bond->lock);
+ rcu_read_unlock();
+ if (!rtnl_trylock())
goto re_arm;
- }
bond_info->rlb_promisc_timeout_counter = 0;
@@ -1568,7 +1567,7 @@ void bond_alb_monitor(struct work_struct *work)
bond_info->primary_is_promisc = 0;
rtnl_unlock();
- read_lock(&bond->lock);
+ rcu_read_lock();
}
if (bond_info->rlb_rebalance) {
@@ -1590,11 +1589,9 @@ void bond_alb_monitor(struct work_struct *work)
}
}
}
-
+ rcu_read_unlock();
re_arm:
queue_delayed_work(bond->wq, &bond->alb_work, alb_delta_in_ticks);
-
- read_unlock(&bond->lock);
}
/* assumption: called before the slave is attached to the bond
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v2 4/10] bonding: rebuild the lock use for bond_loadbalance_arp_mon()
From: Ding Tianhong @ 2013-11-08 2:07 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_loadbalance_arp_mon() use the bond lock to protect the
bond slave list, it is no effect, so I could use RTNL or RCU to
replace it, considering the performance impact, the RCU is more
better here, so the bond lock replace with the RCU.
The bond_select_active_slave() need RTNL and curr_slave_lock
together, but there is no RTNL lock here, so add a rtnl_rtylock.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index def489d..759dcd0 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2407,12 +2407,14 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
struct list_head *iter;
int do_failover = 0;
- read_lock(&bond->lock);
+ rcu_read_lock();
- if (!bond_has_slaves(bond))
+ if (!bond_has_slaves_rcu(bond)) {
+ rcu_read_unlock();
goto re_arm;
+ }
- oldcurrent = bond->curr_active_slave;
+ oldcurrent = ACCESS_ONCE(bond->curr_active_slave);
/* see if any of the previous devices are up now (i.e. they have
* xmt and rcv traffic). the curr_active_slave does not come into
* the picture unless it is null. also, slave->jiffies is not needed
@@ -2421,7 +2423,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
* TODO: what about up/down delay in arp mode? it wasn't here before
* so it can wait
*/
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
unsigned long trans_start = dev_trans_start(slave->dev);
if (slave->link != BOND_LINK_UP) {
@@ -2483,7 +2485,14 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
bond_arp_send_all(bond, slave);
}
+ rcu_read_unlock();
+
if (do_failover) {
+ /* the bond_select_active_slave must hold RTNL
+ * and curr_slave_lock for write.
+ */
+ if (!rtnl_trylock())
+ goto re_arm;
block_netpoll_tx();
write_lock_bh(&bond->curr_slave_lock);
@@ -2491,14 +2500,13 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
write_unlock_bh(&bond->curr_slave_lock);
unblock_netpoll_tx();
+ rtnl_unlock();
}
re_arm:
if (bond->params.arp_interval)
queue_delayed_work(bond->wq, &bond->arp_work,
msecs_to_jiffies(bond->params.arp_interval));
-
- read_unlock(&bond->lock);
}
/*
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v2 7/10] bonding: rebuild the lock use for bond_3ad_state_machine_handler()
From: Ding Tianhong @ 2013-11-08 2:08 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_3ad_state_machine_handler() use the bond lock to protect
the bond slave list and slave port, so as the before patch said,
I remove bond lock and replace with RCU.
The nots in the function is still too old, clean up the nots.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_3ad.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 187b1b7..09edccf61 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2068,18 +2068,18 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
struct slave *slave;
struct port *port;
- read_lock(&bond->lock);
+ rcu_read_lock();
- //check if there are any slaves
- if (!bond_has_slaves(bond))
+ /* check if there are any slaves */
+ if (!bond_has_slaves_rcu(bond))
goto re_arm;
- // check if agg_select_timer timer after initialize is timed out
+ /* check if agg_select_timer timer after initialize is timed out */
if (BOND_AD_INFO(bond).agg_select_timer && !(--BOND_AD_INFO(bond).agg_select_timer)) {
- slave = bond_first_slave(bond);
+ slave = bond_first_slave_rcu(bond);
port = slave ? &(SLAVE_AD_INFO(slave).port) : NULL;
- // select the active aggregator for the bond
+ /* select the active aggregator for the bond */
if (port) {
if (!port->slave) {
pr_warning("%s: Warning: bond's first port is uninitialized\n",
@@ -2093,8 +2093,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
bond_3ad_set_carrier(bond);
}
- // for each port run the state machines
- bond_for_each_slave(bond, slave, iter) {
+ /* for each port run the state machines */
+ bond_for_each_slave_rcu(bond, slave, iter) {
port = &(SLAVE_AD_INFO(slave).port);
if (!port->slave) {
pr_warning("%s: Warning: Found an uninitialized port\n",
@@ -2114,7 +2114,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
ad_mux_machine(port);
ad_tx_machine(port);
- // turn off the BEGIN bit, since we already handled it
+ /* turn off the BEGIN bit, since we already handled it */
if (port->sm_vars & AD_PORT_BEGIN)
port->sm_vars &= ~AD_PORT_BEGIN;
@@ -2122,9 +2122,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
}
re_arm:
+ rcu_read_unlock();
queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
-
- read_unlock(&bond->lock);
}
/**
--
1.8.2.1
^ permalink raw reply related
* Re: [PATCH net-next] inet: fix a UFO regression
From: Eric Dumazet @ 2013-11-08 2:08 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David Miller, netdev, Hannes Frederic Sowa
In-Reply-To: <CAMEtUuynjTj_eteupaSx1hbZs4OyVtc=fDZAwAFSL24qK8tFLA@mail.gmail.com>
On Thu, 2013-11-07 at 17:39 -0800, Alexei Starovoitov wrote:
> as we discussed this will break vxlan :)
> Please move udpfrag assignment before line:
> segs = ops->callbacks.gso_segment(skb, features);
> since skb_udp_tunnel_segment() does skb->encapsulation = 0
> then both ufo and vxlan should be fine
Oh that's right, thanks again, I'll send a V2
^ permalink raw reply
* [PATCH net-next v2 8/10] bonding: remove unwanted lock for bond_option_active_slave_set()
From: Ding Tianhong @ 2013-11-08 2:08 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_option_active_slave_set() is always called in RTNL,
the RTNL could protect bond slave list, so remove the unwanted
bond lock.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_options.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 9a5223c..ae4cd04 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -101,7 +101,6 @@ int bond_option_active_slave_set(struct bonding *bond,
}
block_netpoll_tx();
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
/* check to see if we are clearing active */
@@ -136,7 +135,6 @@ int bond_option_active_slave_set(struct bonding *bond,
}
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
unblock_netpoll_tx();
return ret;
}
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v2 10/10] bonding: remove unwanted lock for bond_store_primaryxxx()
From: Ding Tianhong @ 2013-11-08 2:08 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_select_active_slave() will not release and acquire
bond lock, so it is no need to read the bond lock for them,
and the bond_store_primaryxxx() is in RTNL, remove the
unwanted lock.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_sysfs.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 47749c9..25ef533 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1068,7 +1068,6 @@ static ssize_t bonding_store_primary(struct device *d,
if (!rtnl_trylock())
return restart_syscall();
block_netpoll_tx();
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
if (!USES_PRIMARY(bond->params.mode)) {
@@ -1108,7 +1107,6 @@ static ssize_t bonding_store_primary(struct device *d,
bond->dev->name, ifname, bond->dev->name);
out:
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
unblock_netpoll_tx();
rtnl_unlock();
@@ -1156,11 +1154,9 @@ static ssize_t bonding_store_primary_reselect(struct device *d,
new_value);
block_netpoll_tx();
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
unblock_netpoll_tx();
out:
rtnl_unlock();
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v2 6/10] bonding: rebuild the lock use for bond_activebackup_arp_mon()
From: Ding Tianhong @ 2013-11-08 2:08 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_activebackup_arp_mon() use the bond lock for read to
protect the slave list, it is no effect, and the RTNL is only
called for bond_ab_arp_commit() and peer notify, for the performance
better, use RCU instead of the bond lock, because the bond slave
list need to called in RCU, add a new bond_first_slave_rcu()
to get the first slave in RCU protection.
When bond_ab_arp_inspect() and should_notify_peers is true, the
RTNL will called twice, it is a loss of performance, so make the
two RTNL together to avoid performance loss.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 35 +++++++++++++++++++----------------
drivers/net/bonding/bonding.h | 7 +++++++
2 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 759dcd0..b48ca55 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2524,7 +2524,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
struct slave *slave;
int commit = 0;
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
slave->new_link = BOND_LINK_NOCHANGE;
last_rx = slave_last_rx(bond, slave);
@@ -2586,7 +2586,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
* Called to commit link state changes noted by inspection step of
* active-backup mode ARP monitor.
*
- * Called with RTNL and bond->lock for read.
+ * Called with RTNL hold.
*/
static void bond_ab_arp_commit(struct bonding *bond)
{
@@ -2661,7 +2661,7 @@ do_failover:
/*
* Send ARP probes for active-backup mode ARP monitor.
*
- * Called with bond->lock held for read.
+ * Called with rcu_read_lock hold.
*/
static void bond_ab_arp_probe(struct bonding *bond)
{
@@ -2690,14 +2690,14 @@ static void bond_ab_arp_probe(struct bonding *bond)
*/
if (!bond->current_arp_slave) {
- bond->current_arp_slave = bond_first_slave(bond);
+ bond->current_arp_slave = bond_first_slave_rcu(bond);
if (!bond->current_arp_slave)
return;
}
bond_set_slave_inactive_flags(bond->current_arp_slave);
- bond_for_each_slave(bond, slave, iter) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
if (!found && !before && IS_UP(slave->dev))
before = slave;
@@ -2745,43 +2745,46 @@ void bond_activebackup_arp_mon(struct work_struct *work)
bool should_notify_peers = false;
int delta_in_ticks;
- read_lock(&bond->lock);
-
delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
- if (!bond_has_slaves(bond))
+ rcu_read_lock();
+
+ if (!bond_has_slaves_rcu(bond)) {
+ rcu_read_unlock();
goto re_arm;
+ }
should_notify_peers = bond_should_notify_peers(bond);
if (bond_ab_arp_inspect(bond)) {
- read_unlock(&bond->lock);
+ rcu_read_unlock();
/* Race avoidance with bond_close flush of workqueue */
if (!rtnl_trylock()) {
- read_lock(&bond->lock);
delta_in_ticks = 1;
should_notify_peers = false;
goto re_arm;
}
- read_lock(&bond->lock);
-
bond_ab_arp_commit(bond);
- read_unlock(&bond->lock);
+ if (should_notify_peers) {
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+ bond->dev);
+ should_notify_peers = false;
+ }
+
rtnl_unlock();
- read_lock(&bond->lock);
+ rcu_read_lock();
}
bond_ab_arp_probe(bond);
+ rcu_read_unlock();
re_arm:
if (bond->params.arp_interval)
queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
- read_unlock(&bond->lock);
-
if (should_notify_peers) {
if (!rtnl_trylock())
return;
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index deb9738..90b745c 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -97,6 +97,13 @@
netdev_adjacent_get_private(bond_slave_list(bond)->prev) : \
NULL)
+#define bond_first_slave_rcu(bond) \
+ ({struct list_head *__ptr = (bond_slave_list(bond)); \
+ struct list_head *__next = ACCESS_ONCE(__ptr->next); \
+ likely(__ptr != __next) ? \
+ netdev_adjacent_get_private_rcu(__next) : NULL; \
+ })
+
#define bond_is_first_slave(bond, pos) (pos == bond_first_slave(bond))
#define bond_is_last_slave(bond, pos) (pos == bond_last_slave(bond))
--
1.8.2.1
^ permalink raw reply related
* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-08 2:17 UTC (permalink / raw)
To: David Miller
Cc: bhutchings, eric.dumazet, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131107.170651.1819930710384730745.davem@davemloft.net>
On Thu, Nov 07, 2013 at 05:06:51PM -0500, David Miller wrote:
>
> Aside from the segmentation issues, Eric's patch was a nice cleanup
> which also made it such that we'd be able to get features back when
> the blocking condition gets removed.
>
> Given all of this, would you like me to revert his change for now?
Oh no let's keep it as it is indeed a great cleanup and improvement
for LRO. We can however remove the bits that relate to GRO.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH net-next v2 9/10] bonding: remvoe unwanted lock for bond enslave and release
From: Ding Tianhong @ 2013-11-08 2:08 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_change_active_slave() and bond_select_active_slave()
do't need bond lock anymore, so remove the unwanted bond lock
for these two functions.
The bond_select_active_slave() will release and acquire
curr_slave_lock, so the curr_slave_lock need to protect
the function.
In bond enslave and bond release, the bond slave list is also
protected by RTNL, so bond lock is no need to exist, remove
the lock and clean the functions.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 27 ++++++---------------------
1 file changed, 6 insertions(+), 21 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b48ca55..be163e2 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1579,11 +1579,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
bond_set_carrier(bond);
if (USES_PRIMARY(bond->params.mode)) {
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
}
pr_info("%s: enslaving %s as a%s interface with a%s link.\n",
@@ -1603,19 +1601,13 @@ err_detach:
bond_hw_addr_flush(bond_dev, slave_dev);
vlan_vids_del_by_dev(slave_dev, bond_dev);
- write_lock_bh(&bond->lock);
if (bond->primary_slave == new_slave)
bond->primary_slave = NULL;
if (bond->curr_active_slave == new_slave) {
- bond_change_active_slave(bond, NULL);
- write_unlock_bh(&bond->lock);
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
+ bond_change_active_slave(bond, NULL);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
- } else {
- write_unlock_bh(&bond->lock);
}
slave_disable_netpoll(new_slave);
@@ -1680,20 +1672,16 @@ static int __bond_release_one(struct net_device *bond_dev,
}
block_netpoll_tx();
- write_lock_bh(&bond->lock);
slave = bond_get_slave_by_dev(bond, slave_dev);
if (!slave) {
/* not a slave of this bond */
pr_info("%s: %s not enslaved\n",
bond_dev->name, slave_dev->name);
- write_unlock_bh(&bond->lock);
unblock_netpoll_tx();
return -EINVAL;
}
- write_unlock_bh(&bond->lock);
-
/* release the slave from its bond */
bond->slave_cnt--;
@@ -1711,6 +1699,7 @@ static int __bond_release_one(struct net_device *bond_dev,
*/
bond_3ad_unbind_slave(slave);
}
+ write_unlock_bh(&bond->lock);
pr_info("%s: releasing %s interface %s\n",
bond_dev->name,
@@ -1733,8 +1722,11 @@ static int __bond_release_one(struct net_device *bond_dev,
if (bond->primary_slave == slave)
bond->primary_slave = NULL;
- if (oldcurrent == slave)
+ if (oldcurrent == slave) {
+ write_lock_bh(&bond->curr_slave_lock);
bond_change_active_slave(bond, NULL);
+ write_unlock_bh(&bond->curr_slave_lock);
+ }
if (bond_is_lb(bond)) {
/* Must be called only after the slave has been
@@ -1742,9 +1734,7 @@ static int __bond_release_one(struct net_device *bond_dev,
* has been cleared (if our_slave == old_current),
* but before a new active slave is selected.
*/
- write_unlock_bh(&bond->lock);
bond_alb_deinit_slave(bond, slave);
- write_lock_bh(&bond->lock);
}
if (all) {
@@ -1755,15 +1745,11 @@ static int __bond_release_one(struct net_device *bond_dev,
* is no concern that another slave add/remove event
* will interfere.
*/
- write_unlock_bh(&bond->lock);
- read_lock(&bond->lock);
write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
- write_lock_bh(&bond->lock);
}
if (!bond_has_slaves(bond)) {
@@ -1778,7 +1764,6 @@ static int __bond_release_one(struct net_device *bond_dev,
}
}
- write_unlock_bh(&bond->lock);
unblock_netpoll_tx();
synchronize_rcu();
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next v2] net_sched: tbf: support of 64bit rates
From: Yang Yingliang @ 2013-11-08 2:23 UTC (permalink / raw)
To: davem, netdev; +Cc: eric.dumazet, jhs, sergei.shtylyov
With psched_ratecfg_precompute(), tbf can deal with 64bit rates.
Add two new attributes so that tc can use them to break the 32bit
limit.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Suggested-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
include/uapi/linux/pkt_sched.h | 2 ++
net/sched/sch_tbf.c | 22 ++++++++++++++++++----
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index f2624b5..307f293 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -171,6 +171,8 @@ enum {
TCA_TBF_PARMS,
TCA_TBF_RTAB,
TCA_TBF_PTAB,
+ TCA_TBF_RATE64,
+ TCA_TBF_PRATE64,
__TCA_TBF_MAX,
};
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index b057122..68f9859 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -266,20 +266,23 @@ static const struct nla_policy tbf_policy[TCA_TBF_MAX + 1] = {
[TCA_TBF_PARMS] = { .len = sizeof(struct tc_tbf_qopt) },
[TCA_TBF_RTAB] = { .type = NLA_BINARY, .len = TC_RTAB_SIZE },
[TCA_TBF_PTAB] = { .type = NLA_BINARY, .len = TC_RTAB_SIZE },
+ [TCA_TBF_RATE64] = { .type = NLA_U64 },
+ [TCA_TBF_PRATE64] = { .type = NLA_U64 },
};
static int tbf_change(struct Qdisc *sch, struct nlattr *opt)
{
int err;
struct tbf_sched_data *q = qdisc_priv(sch);
- struct nlattr *tb[TCA_TBF_PTAB + 1];
+ struct nlattr *tb[TCA_TBF_MAX + 1];
struct tc_tbf_qopt *qopt;
struct qdisc_rate_table *rtab = NULL;
struct qdisc_rate_table *ptab = NULL;
struct Qdisc *child = NULL;
int max_size, n;
+ u64 rate64 = 0, prate64 = 0;
- err = nla_parse_nested(tb, TCA_TBF_PTAB, opt, tbf_policy);
+ err = nla_parse_nested(tb, TCA_TBF_MAX, opt, tbf_policy);
if (err < 0)
return err;
@@ -341,9 +344,13 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt)
q->tokens = q->buffer;
q->ptokens = q->mtu;
- psched_ratecfg_precompute(&q->rate, &rtab->rate, 0);
+ if (tb[TCA_TBF_RATE64])
+ rate64 = nla_get_u64(tb[TCA_TBF_RATE64]);
+ psched_ratecfg_precompute(&q->rate, &rtab->rate, rate64);
if (ptab) {
- psched_ratecfg_precompute(&q->peak, &ptab->rate, 0);
+ if (tb[TCA_TBF_PRATE64])
+ prate64 = nla_get_u64(tb[TCA_TBF_PRATE64]);
+ psched_ratecfg_precompute(&q->peak, &ptab->rate, prate64);
q->peak_present = true;
} else {
q->peak_present = false;
@@ -402,6 +409,13 @@ static int tbf_dump(struct Qdisc *sch, struct sk_buff *skb)
opt.buffer = PSCHED_NS2TICKS(q->buffer);
if (nla_put(skb, TCA_TBF_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
+ if (q->rate.rate_bytes_ps >= (1ULL << 32) &&
+ nla_put_u64(skb, TCA_TBF_RATE64, q->rate.rate_bytes_ps))
+ goto nla_put_failure;
+ if (q->peak_present &&
+ q->peak.rate_bytes_ps >= (1ULL << 32) &&
+ nla_put_u64(skb, TCA_TBF_PRATE64, q->peak.rate_bytes_ps))
+ goto nla_put_failure;
nla_nest_end(skb, nest);
return skb->len;
--
1.7.12
^ permalink raw reply related
* [PATCH net-next v2 5/10] net: add and export netdev_adjacent_get_private_rcu()
From: Ding Tianhong @ 2013-11-08 2:08 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
In bonding, the bond lock could not protect slave list any more,
the bond lock will be replace with RTNL or RCU, sometimes
bonding needs netdev_adjacent_get_private_rcu() to get a slave in
RCU protect, so add a new function and export it.
Suggested-by: Jay Vosburgh <fubar@us.ibm.com>
Suggested-by: Veaceslav Falico <vfalico@redhat.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
include/linux/netdevice.h | 1 +
net/core/dev.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e6353ca..c27bb4a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2840,6 +2840,7 @@ void *netdev_lower_get_next_private_rcu(struct net_device *dev,
priv = netdev_lower_get_next_private_rcu(dev, &(iter)))
void *netdev_adjacent_get_private(struct list_head *adj_list);
+void *netdev_adjacent_get_private_rcu(struct list_head *adj_list);
struct net_device *netdev_master_upper_dev_get(struct net_device *dev);
struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev);
int netdev_upper_dev_link(struct net_device *dev, struct net_device *upper_dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 0e61365..92613a2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4478,6 +4478,16 @@ void *netdev_adjacent_get_private(struct list_head *adj_list)
}
EXPORT_SYMBOL(netdev_adjacent_get_private);
+void *netdev_adjacent_get_private_rcu(struct list_head *adj_list)
+{
+ struct netdev_adjacent *adj;
+
+ adj = list_entry_rcu(adj_list, struct netdev_adjacent, list);
+
+ return adj->private;
+}
+EXPORT_SYMBOL(netdev_adjacent_get_private_rcu);
+
/**
* netdev_all_upper_get_next_dev_rcu - Get the next dev from upper list
* @dev: device
--
1.8.2.1
^ permalink raw reply related
* [PATCH v2 net-next] inet: fix a UFO regression
From: Eric Dumazet @ 2013-11-08 2:32 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David Miller, netdev, Hannes Frederic Sowa
In-Reply-To: <1383871465.9412.118.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
While testing virtio_net and skb_segment() changes, Hannes reported
that UFO was sending wrong frames.
It appears this was introduced by a recent commit :
8c3a897bfab1 ("inet: restore gso for vxlan")
The old condition to perform IP frag was :
tunnel = !!skb->encapsulation;
...
if (!tunnel && proto == IPPROTO_UDP) {
So the new one should be :
udpfrag = !skb->encapsulation && proto == IPPROTO_UDP;
...
if (udpfrag) {
Initialization of udpfrag must be done before call
to ops->callbacks.gso_segment(skb, features), as
skb_udp_tunnel_segment() clears skb->encapsulation
(We want udpfrag to be true for UFO, false for VXLAN)
With help from Alexei Starovoitov
Reported-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
---
v2: move the udpfrag init, as spotted by Alexei.
net/ipv4/af_inet.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 09d78d4a3cff..68af9aac91d0 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1299,6 +1299,9 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
segs = ERR_PTR(-EPROTONOSUPPORT);
+ /* Note : following gso_segment() might change skb->encapsulation */
+ udpfrag = !skb->encapsulation && proto == IPPROTO_UDP;
+
ops = rcu_dereference(inet_offloads[proto]);
if (likely(ops && ops->callbacks.gso_segment))
segs = ops->callbacks.gso_segment(skb, features);
@@ -1306,7 +1309,6 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
if (IS_ERR_OR_NULL(segs))
goto out;
- udpfrag = !!skb->encapsulation && proto == IPPROTO_UDP;
skb = segs;
do {
iph = (struct iphdr *)(skb_mac_header(skb) + nhoff);
^ permalink raw reply related
* Re: [PATCH net-next v2] net_sched: tbf: support of 64bit rates
From: Eric Dumazet @ 2013-11-08 2:34 UTC (permalink / raw)
To: Yang Yingliang; +Cc: davem, netdev, jhs, sergei.shtylyov
In-Reply-To: <1383877414-9488-1-git-send-email-yangyingliang@huawei.com>
On Fri, 2013-11-08 at 10:23 +0800, Yang Yingliang wrote:
> With psched_ratecfg_precompute(), tbf can deal with 64bit rates.
> Add two new attributes so that tc can use them to break the 32bit
> limit.
>
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> Suggested-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> ---
> include/uapi/linux/pkt_sched.h | 2 ++
> net/sched/sch_tbf.c | 22 ++++++++++++++++++----
> 2 files changed, 20 insertions(+), 4 deletions(-)
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks !
^ permalink raw reply
* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-08 2:42 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131108021758.GA13467@gondor.apana.org.au>
On Fri, 2013-11-08 at 10:17 +0800, Herbert Xu wrote:
> Oh no let's keep it as it is indeed a great cleanup and improvement
> for LRO. We can however remove the bits that relate to GRO.
What about doing a benchmark first ?
I benchmarked the 'locally delivered' traffic, not the forwarding one.
That's why I am not comfortable of having large GRO packets that need
to be segmented.
You tell us hardware advantage of TSO is negligible, this is not
what I observe today on many platforms.
A normal TSO packets with 16 MSS setups a ~17 DMA descriptors,
while GSO requires 2 DMA descriptors per MSS, plus a lot of overhead
in sk_buff allocation/deallocation.
^ permalink raw reply
* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-08 2:51 UTC (permalink / raw)
To: Herbert Xu
Cc: David Miller, bhutchings, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383878554.9412.153.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, 2013-11-07 at 18:42 -0800, Eric Dumazet wrote:
> A normal TSO packets with 16 MSS setups a ~17 DMA descriptors,
> while GSO requires 2 DMA descriptors per MSS, plus a lot of overhead
> in sk_buff allocation/deallocation.
Not mentioning fact that a 64KB packet is adding latencies, since high
prio packets have to wait the whole preceding 64KB packet has left the
host.
^ permalink raw reply
* [PATCH net-next 0/5] sctp: fix and clean up
From: Wang Weidong @ 2013-11-08 2:55 UTC (permalink / raw)
To: vyasevich, nhorman; +Cc: dingtianhong, davem, linux-sctp, netdev
This patch series include: remove the duplicate initialize,
remove else path, fix one lost free, convert func to boolean,
fix some typos.
Wang Weidong (5):
sctp: remove the duplicate initialize
sctp: remove the else path
sctp: if we done update ssnmap, free the new ssnmap
sctp: convert sctp_peer_needs_update to boolean
sctp: fix some typos in associola.c
net/sctp/associola.c | 63 ++++++++++------------------------------------------
1 file changed, 12 insertions(+), 51 deletions(-)
--
1.7.12
^ permalink raw reply
* [PATCH net-next 4/5] sctp: convert sctp_peer_needs_update to boolean
From: Wang Weidong @ 2013-11-08 2:55 UTC (permalink / raw)
To: vyasevich, nhorman; +Cc: dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <1383879310-22792-1-git-send-email-wangweidong1@huawei.com>
sctp_peer_needs_update only return 0 or 1.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/associola.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index bca1ad1..c6cfff6 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1355,7 +1355,7 @@ void sctp_assoc_sync_pmtu(struct sock *sk, struct sctp_association *asoc)
}
/* Should we send a SACK to update our peer? */
-static inline int sctp_peer_needs_update(struct sctp_association *asoc)
+static inline bool sctp_peer_needs_update(struct sctp_association *asoc)
{
struct net *net = sock_net(asoc->base.sk);
switch (asoc->state) {
@@ -1367,12 +1367,12 @@ static inline int sctp_peer_needs_update(struct sctp_association *asoc)
((asoc->rwnd - asoc->a_rwnd) >= max_t(__u32,
(asoc->base.sk->sk_rcvbuf >> net->sctp.rwnd_upd_shift),
asoc->pathmtu)))
- return 1;
+ return true;
break;
default:
break;
}
- return 0;
+ return false;
}
/* Increase asoc's rwnd by len and send any window update SACK if needed. */
--
1.7.12
^ permalink raw reply related
* [PATCH net-next 5/5] sctp: fix some typos in associola.c
From: Wang Weidong @ 2013-11-08 2:55 UTC (permalink / raw)
To: vyasevich, nhorman; +Cc: dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <1383879310-22792-1-git-send-email-wangweidong1@huawei.com>
fix some typos
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/associola.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index c6cfff6..fadf31d 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -128,7 +128,7 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
*/
asoc->param_flags = sp->param_flags;
- /* Initialize the maximum mumber of new data packets that can be sent
+ /* Initialize the maximum number of new data packets that can be sent
* in a burst.
*/
asoc->max_burst = sp->max_burst;
@@ -231,7 +231,7 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
/* Assume that the peer will tell us if he recognizes ASCONF
* as part of INIT exchange.
- * The sctp_addip_noauth option is there for backward compatibilty
+ * The sctp_addip_noauth option is there for backward compatibility
* and will revert old behavior.
*/
if (net->sctp.addip_noauth)
@@ -1227,7 +1227,7 @@ void sctp_assoc_update(struct sctp_association *asoc,
if (new->ssnmap)
sctp_ssnmap_free(new->ssnmap);
- /* SCTP-AUTH: Save the peer parameters from the new assocaitions
+ /* SCTP-AUTH: Save the peer parameters from the new associations
* and also move the association shared keys over
*/
kfree(asoc->peer.peer_random);
@@ -1452,7 +1452,7 @@ void sctp_assoc_rwnd_decrease(struct sctp_association *asoc, unsigned int len)
/* If we've reached or overflowed our receive buffer, announce
* a 0 rwnd if rwnd would still be positive. Store the
- * the pottential pressure overflow so that the window can be restored
+ * the potential pressure overflow so that the window can be restored
* back to original value.
*/
if (rx_count >= asoc->base.sk->sk_rcvbuf)
--
1.7.12
^ permalink raw reply related
* [PATCH net-next 3/5] sctp: if we done update ssnmap, free the new ssnmap
From: Wang Weidong @ 2013-11-08 2:55 UTC (permalink / raw)
To: vyasevich, nhorman; +Cc: dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <1383879310-22792-1-git-send-email-wangweidong1@huawei.com>
I found that free the old parameters after update the peer parameters.
So we should free the new ssnmap which not updated.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/associola.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 2d53d4c..bca1ad1 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1223,6 +1223,10 @@ void sctp_assoc_update(struct sctp_association *asoc,
}
}
+ /* if we don't update the ssnmap, we should free it. */
+ if (new->ssnmap)
+ sctp_ssnmap_free(new->ssnmap);
+
/* SCTP-AUTH: Save the peer parameters from the new assocaitions
* and also move the association shared keys over
*/
--
1.7.12
^ permalink raw reply related
* [PATCH net-next 2/5] sctp: remove the else path
From: Wang Weidong @ 2013-11-08 2:55 UTC (permalink / raw)
To: vyasevich, nhorman; +Cc: dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <1383879310-22792-1-git-send-email-wangweidong1@huawei.com>
Make the code more simplification.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/associola.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 667f984..2d53d4c 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -954,15 +954,13 @@ int sctp_cmp_addr_exact(const union sctp_addr *ss1,
*/
struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
{
- struct sctp_chunk *chunk;
+ struct sctp_chunk *chunk = NULL;
/* Send ECNE if needed.
* Not being able to allocate a chunk here is not deadly.
*/
if (asoc->need_ecne)
chunk = sctp_make_ecne(asoc, asoc->last_ecne_tsn);
- else
- chunk = NULL;
return chunk;
}
--
1.7.12
^ permalink raw reply related
* Re: [PATCH net-next 2/5] sctp: remove the else path
From: Joe Perches @ 2013-11-08 3:00 UTC (permalink / raw)
To: Wang Weidong; +Cc: vyasevich, nhorman, dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <1383879310-22792-3-git-send-email-wangweidong1@huawei.com>
On Fri, 2013-11-08 at 10:55 +0800, Wang Weidong wrote:
> Make the code more simplification.
>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
> net/sctp/associola.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 667f984..2d53d4c 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -954,15 +954,13 @@ int sctp_cmp_addr_exact(const union sctp_addr *ss1,
> */
> struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
> {
> - struct sctp_chunk *chunk;
> + struct sctp_chunk *chunk = NULL;
>
> /* Send ECNE if needed.
> * Not being able to allocate a chunk here is not deadly.
> */
> if (asoc->need_ecne)
> chunk = sctp_make_ecne(asoc, asoc->last_ecne_tsn);
> - else
> - chunk = NULL;
>
> return chunk;
> }
If you really want to make the code simple how about:
struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
{
if (!asoc->need_ecne)
return NULL;
return sctp_make_ecne(asoc, asoc->last_ecne_tsn);
}
^ permalink raw reply
* [PATCH net-next 1/5] sctp: remove the duplicate initialize
From: Wang Weidong @ 2013-11-08 2:55 UTC (permalink / raw)
To: vyasevich, nhorman; +Cc: dingtianhong, davem, linux-sctp, netdev
In-Reply-To: <1383879310-22792-1-git-send-email-wangweidong1@huawei.com>
kzalloc had initialize the allocated memroy. Therefore, remove the
initialize with 0 and the memset.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/sctp/associola.c | 41 -----------------------------------------
1 file changed, 41 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index cef5099..667f984 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -90,14 +90,12 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
/* Initialize the object handling fields. */
atomic_set(&asoc->base.refcnt, 1);
- asoc->base.dead = false;
/* Initialize the bind addr area. */
sctp_bind_addr_init(&asoc->base.bind_addr, ep->base.bind_addr.port);
asoc->state = SCTP_STATE_CLOSED;
asoc->cookie_life = ms_to_ktime(sp->assocparams.sasoc_cookie_life);
- asoc->frag_point = 0;
asoc->user_frag = sp->user_frag;
/* Set the association max_retrans and RTO values from the
@@ -110,8 +108,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
asoc->rto_max = msecs_to_jiffies(sp->rtoinfo.srto_max);
asoc->rto_min = msecs_to_jiffies(sp->rtoinfo.srto_min);
- asoc->overall_error_count = 0;
-
/* Initialize the association's heartbeat interval based on the
* sock configured value.
*/
@@ -138,12 +134,9 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
asoc->max_burst = sp->max_burst;
/* initialize association timers */
- asoc->timeouts[SCTP_EVENT_TIMEOUT_NONE] = 0;
asoc->timeouts[SCTP_EVENT_TIMEOUT_T1_COOKIE] = asoc->rto_initial;
asoc->timeouts[SCTP_EVENT_TIMEOUT_T1_INIT] = asoc->rto_initial;
asoc->timeouts[SCTP_EVENT_TIMEOUT_T2_SHUTDOWN] = asoc->rto_initial;
- asoc->timeouts[SCTP_EVENT_TIMEOUT_T3_RTX] = 0;
- asoc->timeouts[SCTP_EVENT_TIMEOUT_T4_RTO] = 0;
/* sctpimpguide Section 2.12.2
* If the 'T5-shutdown-guard' timer is used, it SHOULD be set to the
@@ -152,7 +145,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
asoc->timeouts[SCTP_EVENT_TIMEOUT_T5_SHUTDOWN_GUARD]
= 5 * asoc->rto_max;
- asoc->timeouts[SCTP_EVENT_TIMEOUT_HEARTBEAT] = 0;
asoc->timeouts[SCTP_EVENT_TIMEOUT_SACK] = asoc->sackdelay;
asoc->timeouts[SCTP_EVENT_TIMEOUT_AUTOCLOSE] =
min_t(unsigned long, sp->autoclose, net->sctp.max_autoclose) * HZ;
@@ -173,11 +165,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
asoc->max_init_timeo =
msecs_to_jiffies(sp->initmsg.sinit_max_init_timeo);
- /* Allocate storage for the ssnmap after the inbound and outbound
- * streams have been negotiated during Init.
- */
- asoc->ssnmap = NULL;
-
/* Set the local window size for receive.
* This is also the rcvbuf space per association.
* RFC 6 - A SCTP receiver MUST be able to receive a minimum of
@@ -190,25 +177,15 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
asoc->a_rwnd = asoc->rwnd;
- asoc->rwnd_over = 0;
- asoc->rwnd_press = 0;
-
/* Use my own max window until I learn something better. */
asoc->peer.rwnd = SCTP_DEFAULT_MAXWINDOW;
- /* Set the sndbuf size for transmit. */
- asoc->sndbuf_used = 0;
-
/* Initialize the receive memory counter */
atomic_set(&asoc->rmem_alloc, 0);
init_waitqueue_head(&asoc->wait);
asoc->c.my_vtag = sctp_generate_tag(ep);
- asoc->peer.i.init_tag = 0; /* INIT needs a vtag of 0. */
- asoc->c.peer_vtag = 0;
- asoc->c.my_ttag = 0;
- asoc->c.peer_ttag = 0;
asoc->c.my_port = ep->base.bind_addr.port;
asoc->c.initial_tsn = sctp_generate_tsn(ep);
@@ -219,7 +196,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
asoc->adv_peer_ack_point = asoc->ctsn_ack_point;
asoc->highest_sacked = asoc->ctsn_ack_point;
asoc->last_cwr_tsn = asoc->ctsn_ack_point;
- asoc->unack_data = 0;
/* ADDIP Section 4.1 Asconf Chunk Procedures
*
@@ -238,7 +214,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
/* Make an empty list of remote transport addresses. */
INIT_LIST_HEAD(&asoc->peer.transport_addr_list);
- asoc->peer.transport_count = 0;
/* RFC 2960 5.1 Normal Establishment of an Association
*
@@ -252,7 +227,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
* already received one packet.]
*/
asoc->peer.sack_needed = 1;
- asoc->peer.sack_cnt = 0;
asoc->peer.sack_generation = 1;
/* Assume that the peer will tell us if he recognizes ASCONF
@@ -260,12 +234,8 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
* The sctp_addip_noauth option is there for backward compatibilty
* and will revert old behavior.
*/
- asoc->peer.asconf_capable = 0;
if (net->sctp.addip_noauth)
asoc->peer.asconf_capable = 1;
- asoc->asconf_addr_del_pending = NULL;
- asoc->src_out_of_asoc_ok = 0;
- asoc->new_transport = NULL;
/* Create an input queue. */
sctp_inq_init(&asoc->base.inqueue);
@@ -277,12 +247,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
if (!sctp_ulpq_init(&asoc->ulpq, asoc))
goto fail_init;
- memset(&asoc->peer.tsn_map, 0, sizeof(struct sctp_tsnmap));
-
- asoc->need_ecne = 0;
-
- asoc->assoc_id = 0;
-
/* Assume that peer would support both address types unless we are
* told otherwise.
*/
@@ -300,9 +264,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
asoc->default_timetolive = sp->default_timetolive;
asoc->default_rcv_context = sp->default_rcv_context;
- /* SCTP_GET_ASSOC_STATS COUNTERS */
- memset(&asoc->stats, 0, sizeof(struct sctp_priv_assoc_stats));
-
/* AUTH related initializations */
INIT_LIST_HEAD(&asoc->endpoint_shared_keys);
err = sctp_auth_asoc_copy_shkeys(ep, asoc, gfp);
@@ -310,9 +271,7 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
goto fail_init;
asoc->active_key_id = ep->active_key_id;
- asoc->asoc_shared_key = NULL;
- asoc->default_hmac_id = 0;
/* Save the hmacs and chunks list into this association */
if (ep->auth_hmacs_list)
memcpy(asoc->c.auth_hmacs, ep->auth_hmacs_list,
--
1.7.12
^ 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