| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 11 Sep 2014 15:08:15 -0700
From: Mahesh Bandewar <maheshb@...gle.com>
To: Nikolay Aleksandrov <nikolay@...hat.com>
Cc: Jay Vosburgh <j.vosburgh@...il.com>,
Veaceslav Falico <vfalico@...hat.com>,
Andy Gospodarek <andy@...yhouse.net>,
David Miller <davem@...emloft.net>,
netdev <netdev@...r.kernel.org>,
Eric Dumazet <edumazet@...gle.com>,
Maciej Zenczykowski <maze@...gle.com>
Subject: Re: [PATCH net-next v3 2/2] bonding: Simplify the xmit function for
modes that use xmit_hash
some how my earlier mail bounced back (formatting issues, I suppose!).
So it's a resend.
On Thu, Sep 11, 2014 at 2:27 PM, Mahesh Bandewar <maheshb@...gle.com> wrote:
>
> On Thu, Sep 11, 2014 at 2:39 AM, Nikolay Aleksandrov <nikolay@...hat.com> wrote:
> > On 11/09/14 06:16, 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@...gle.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.
> >> v2:
> >> (a) Removed rcu_read_{un}lock() calls from array manipulation code.
> >> (b) Slave link-events now refresh array for all these modes.
> >> (c) Moved free-array call from bond_close() to bond_uninit().
> >> v3:
> >> (a) Fixed null pointer dereference.
> >> (b) Removed bond->lock lockdep dependency.
> >>
> > Hello Mahesh,
> > You should've given me time to respond, the reason I wrote this:
> > "First a question, if a bond device in XOR mode is up and we enslave a
> > single
> > slave how would it start transmitting ? Same question, if we are enslaving
> > a
> > second device then the array will be rebuild with only the first upon
> > NETDEV_UP
> > (of course all this is in the case miimon is 0).
> > The NETDEV_UP upon enslave happens before the slave is linked in."
> > was not because I wanted you to remove the slave rebuilding from the
> > NETDEV_UP/DOWN events, but because I didn't see how would a slave start
> > transmitting in XOR mode after enslaving, and I just tested it - it doesn't
> > since the slave array never gets rebuilt. The NETDEV_UP event is carried by
> > the dev_open() done in bond_enslave() earlier so the bond_set_carrier() in
> > the end isn't of much importance in most cases, simply do the following and
> > you'll see:
> > modprobe bonding mode=2
> > ip set bond0 up
> > ifenslave bond0 eth0
> >
> > Try to transmit anything and watch on the other side, you won't be able to
> > see anything as there's no slave array. My second question was given all
> > this, if you enslave any subsequent slaves, will it start transmitting ? But
> > I just tested this scenario and it still doesn't as the array doesn't get
> > built.
>
OK I tried that and I don't see what you have observed -
Here is the log -
~# modprobe bonding mode=2
~# ip link set bond0 up
[ 133.537516] New slave count=0
~# [add ip addr]
~# [add default route]
~# ifenslave bond0 eth0
~# [ 211.044852] Adding slave=eth0
[ 211.047826] New slave count=1
~# ifenslave bond0 eth1
[ 723.795877] Adding slave=eth0
[ 723.798853] Adding slave=eth1
[ 723.801824] New slave count=2
I have added some instrumentation to see what is happening and
following are the couple of changes (basically printfs!) -
@@ -3730,13 +3736,16 @@ int bond_update_slave_arr(struct bonding
*bond, struct slave *skipslave)
continue;
if (skipslave == slave)
continue;
+pr_err("Adding slave=%s\n", slave->dev->name);
new_arr->arr[new_arr->count++] = slave;
}
old_arr = rcu_dereference_protected(bond->slave_arr,
lockdep_rtnl_is_held() ||
lockdep_is_held(&bond->curr_slave_lock));
rcu_assign_pointer(bond->slave_arr, new_arr);
+pr_err("New slave count=%d\n", new_arr->count);
if (old_arr)
kfree_rcu(old_arr, rcu);
> > A few suggestions below, nothing serious though.
> >
> >
> >> drivers/net/bonding/bond_3ad.c | 76 +++-----------------
> >> drivers/net/bonding/bond_alb.c | 51 ++------------
> >> drivers/net/bonding/bond_alb.h | 8 ---
> >> drivers/net/bonding/bond_main.c | 152
> >> ++++++++++++++++++++++++++++++++++++----
> >> drivers/net/bonding/bonding.h | 8 +++
> >> 5 files changed, 164 insertions(+), 131 deletions(-)
> >>
> >> diff --git a/drivers/net/bonding/bond_3ad.c
> >> b/drivers/net/bonding/bond_3ad.c
> >> index 5d27a6207384..516075f0a740 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");
> >> }
> >> }
> >>
> >> @@ -2311,6 +2317,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);
> >> }
> >>
> >> @@ -2406,73 +2415,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 028496205f39..dbac0ceb17f6 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,12 +1427,14 @@ 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;
> >> + unsigned int count;
> >>
> >> - slaves =
> >> rcu_dereference(bond_info->slave_arr);
> >> - if (slaves && slaves->count)
> >> + slaves = rcu_dereference(bond->slave_arr);
> >> + count = slaves ? slaves->count : 0;
> >> + if (count)
> >
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > In both of these cases (slaves & slaves->count) you could use likely() as in
> > the case we have slaves to transmit, it'd be advantageous and will be hit
> > every time. The cases that we get here and don't have a slave_arr/count are
> > mostly 2:
> > 1. The slaves got released between the start of xmit and this part
> > 2. They were not eligible so the array got emptied
> > In both of these cases we don't care about the fallout path since
> > transmission isn't possible anyway.
> > Also another minor thing is that usually in the cases where we want to fetch
> > a variable only once ACCESS_ONCE() is used as a weak compiler barrier to
> > make sure the compiler doesn't optimize out something. The others can
> > correct me if I'm wrong, but I think in this case it's a good precaution for
> > slaves->count.
> > These are merely suggestions, I might be wrong.
> >
Will do.
> >
> >> tx_slave = slaves->arr[hash_index
> >> %
> >> -
> >> slaves->count];
> >> + count];
> >> }
> >> break;
> >> }
> >> @@ -1733,10 +1700,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 +1725,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 b43b2df9e5d1..4412c458939d 100644
> >> --- a/drivers/net/bonding/bond_main.c
> >> +++ b/drivers/net/bonding/bond_main.c
> >> @@ -1700,6 +1700,10 @@ static int __bond_release_one(struct net_device
> >> *bond_dev,
> >> write_unlock_bh(&bond->curr_slave_lock);
> >> }
> >>
> >> + if (bond_mode_uses_xmit_hash(bond) &&
> >> + bond_update_slave_arr(bond, slave))
> >> + pr_err("Failed to build slave-array.\n");
> >> +
> >> netdev_info(bond_dev, "Releasing %s interface %s\n",
> >> bond_is_active_slave(slave) ? "active" : "backup",
> >> slave_dev->name);
> >> @@ -2015,6 +2019,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 == primary)
> >> goto do_failover;
> >>
> >> @@ -2042,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;
> >>
> >> @@ -2505,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.
> >> @@ -2899,11 +2914,23 @@ static int bond_slave_netdev_event(unsigned long
> >> event,
> >> if (old_duplex != slave->duplex)
> >> bond_3ad_adapter_duplex_changed(slave);
> >> }
> >> + /* Refresh slave-array if applicable!
> >> + * If the setuo does not use miimon or arpmon
> >> (mode-specific!),
> >> + * then these event will not cause the slave-array to be
> >> + * refreshed. This will cause xmit to use a slave that is
> >> not
> >> + * usable. Avoid such situation by refeshing the array at
> >> these
> >> + * events. If these (miimon/arpmon) parameters are
> >> configured
> >> + * then array gets refreshed twice and that should be
> >> fine!
> >> + */
> >> + if (bond_mode_uses_xmit_hash(bond) &&
> >> + 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?
> >> - */
> >> + /* Refresh slave-array if applicable! */
> >> + if (bond_mode_uses_xmit_hash(bond) &&
> >> + bond_update_slave_arr(bond, NULL))
> >> + pr_err("Failed to build slave-array for XOR
> >> mode.\n");
> >> break;
> >> case NETDEV_CHANGEMTU:
> >> /*
> >> @@ -3147,6 +3174,10 @@ static int bond_open(struct net_device *bond_dev)
> >> bond_3ad_initiate_agg_selection(bond, 1);
> >> }
> >>
> >> + if (bond_mode_uses_xmit_hash(bond) &&
> >> + bond_update_slave_arr(bond, NULL))
> >> + pr_err("Failed to build slave-array for XOR mode.\n");
> >> +
> >> return 0;
> >> }
> >>
> >> @@ -3654,15 +3685,106 @@ 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;
> >> +
> >> + 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;
> >>
> >> - bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb) %
> >> bond->slave_cnt);
> >> + 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->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!). BTW this is
> >> + * only possible when the call is initiated from
> >> + * __bond_release_one(). 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.
> >> + */
> >> + old_arr = rtnl_dereference(bond->slave_arr);
> >> + for (idx = 0; idx < old_arr->count; idx++) {
> >> + if (skipslave == old_arr->arr[idx]) {
> >> + old_arr->arr[idx] =
> >> + old_arr->arr[old_arr->count-1];
> >> + old_arr->count--;
> >> + break;
> >> + }
> >> + }
> >> + }
> >> + 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;
> >> + unsigned int count;
> >> +
> >> + slaves = rcu_dereference(bond->slave_arr);
> >> + count = slaves ? slaves->count : 0;
> >> + if (count) {
> >
> > ^^^^^^^^^^^^^^
> > The same comment as above applies here, too.
> >
> >
> >> + slave = slaves->arr[bond_xmit_hash(bond, skb) % 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;
> >> }
> >> @@ -3764,12 +3886,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:
> >> @@ -3947,6 +4068,7 @@ static void bond_uninit(struct net_device *bond_dev)
> >> struct bonding *bond = netdev_priv(bond_dev);
> >> struct list_head *iter;
> >> struct slave *slave;
> >> + struct bond_up_slave *arr;
> >>
> >> bond_netpoll_cleanup(bond_dev);
> >>
> >> @@ -3955,6 +4077,12 @@ static void bond_uninit(struct net_device
> >> *bond_dev)
> >> __bond_release_one(bond_dev, slave->dev, true);
> >> netdev_info(bond_dev, "Released all slaves\n");
> >>
> >> + arr = rtnl_dereference(bond->slave_arr);
> >> + if (arr) {
> >> + kfree_rcu(arr, rcu);
> >> + RCU_INIT_POINTER(bond->slave_arr, NULL);
> >> + }
> >> +
> >> list_del(&bond->bond_list);
> >>
> >> bond_debug_unregister(bond);
> >> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> >> index 8375133dd347..78d6d3b7a780 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
> >> */
> >> @@ -193,6 +199,7 @@ struct bonding {
> >> struct slave __rcu *curr_active_slave;
> >> struct slave __rcu *current_arp_slave;
> >> struct slave __rcu *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 *,
> >> @@ -530,6 +537,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);
> >>
> >
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists