Commit 6cca9adb authored by David S. Miller's avatar David S. Miller

Merge branch 'bonding-cleanups'

Nikolay Aleksandrov says:

====================
bonding: style, comment and assertion changes

This is a small and simple patch-set that doesn't introduce (hopefully) any
functional changes, but only stylistic and semantic ones.
Patch 01 simply uses the already provided __rlb_next_rx_slave function inside
rlb_next_rx_slave(), thus removing the duplication of code.
Patch 02 changes all comments that I could find to netdev style, removes
some outdated ones and fixes a few more small cosmetic issues (new line
after declaration, braces around if; else and such)
Patch 03 removes one extra ASSERT_RTNL() because we already have it in the
parent function and consolidates two other ASSERT_RTNL()s to the function
that is exported and supposed to be called with RTNL anyway.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 43702406 e0974585
...@@ -297,15 +297,14 @@ static u16 __get_link_speed(struct port *port) ...@@ -297,15 +297,14 @@ static u16 __get_link_speed(struct port *port)
static u8 __get_duplex(struct port *port) static u8 __get_duplex(struct port *port)
{ {
struct slave *slave = port->slave; struct slave *slave = port->slave;
u8 retval; u8 retval;
/* handling a special case: when the configuration starts with /* handling a special case: when the configuration starts with
* link down, it sets the duplex to 0. * link down, it sets the duplex to 0.
*/ */
if (slave->link != BOND_LINK_UP) if (slave->link != BOND_LINK_UP) {
retval = 0x0; retval = 0x0;
else { } else {
switch (slave->duplex) { switch (slave->duplex) {
case DUPLEX_FULL: case DUPLEX_FULL:
retval = 0x1; retval = 0x1;
......
...@@ -261,14 +261,15 @@ static struct slave *tlb_choose_channel(struct bonding *bond, u32 hash_index, ...@@ -261,14 +261,15 @@ static struct slave *tlb_choose_channel(struct bonding *bond, u32 hash_index,
u32 skb_len) u32 skb_len)
{ {
struct slave *tx_slave; struct slave *tx_slave;
/*
* We don't need to disable softirq here, becase /* We don't need to disable softirq here, becase
* tlb_choose_channel() is only called by bond_alb_xmit() * tlb_choose_channel() is only called by bond_alb_xmit()
* which already has softirq disabled. * which already has softirq disabled.
*/ */
spin_lock(&bond->mode_lock); spin_lock(&bond->mode_lock);
tx_slave = __tlb_choose_channel(bond, hash_index, skb_len); tx_slave = __tlb_choose_channel(bond, hash_index, skb_len);
spin_unlock(&bond->mode_lock); spin_unlock(&bond->mode_lock);
return tx_slave; return tx_slave;
} }
...@@ -334,14 +335,15 @@ static int rlb_arp_recv(const struct sk_buff *skb, struct bonding *bond, ...@@ -334,14 +335,15 @@ static int rlb_arp_recv(const struct sk_buff *skb, struct bonding *bond,
return RX_HANDLER_ANOTHER; return RX_HANDLER_ANOTHER;
} }
static struct slave *rlb_next_rx_slave(struct bonding *bond) /* Caller must hold rcu_read_lock() */
static struct slave *__rlb_next_rx_slave(struct bonding *bond)
{ {
struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
struct slave *before = NULL, *rx_slave = NULL, *slave; struct slave *before = NULL, *rx_slave = NULL, *slave;
struct list_head *iter; struct list_head *iter;
bool found = false; bool found = false;
bond_for_each_slave(bond, slave, iter) { bond_for_each_slave_rcu(bond, slave, iter) {
if (!bond_slave_can_tx(slave)) if (!bond_slave_can_tx(slave))
continue; continue;
if (!found) { if (!found) {
...@@ -366,35 +368,16 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond) ...@@ -366,35 +368,16 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond)
return rx_slave; return rx_slave;
} }
/* Caller must hold rcu_read_lock() */ /* Caller must hold RTNL, rcu_read_lock is obtained only to silence checkers */
static struct slave *__rlb_next_rx_slave(struct bonding *bond) static struct slave *rlb_next_rx_slave(struct bonding *bond)
{ {
struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); struct slave *rx_slave;
struct slave *before = NULL, *rx_slave = NULL, *slave;
struct list_head *iter;
bool found = false;
bond_for_each_slave_rcu(bond, slave, iter) { ASSERT_RTNL();
if (!bond_slave_can_tx(slave))
continue;
if (!found) {
if (!before || before->speed < slave->speed)
before = slave;
} else {
if (!rx_slave || rx_slave->speed < slave->speed)
rx_slave = slave;
}
if (slave == bond_info->rx_slave)
found = true;
}
/* we didn't find anything after the current or we have something
* better before and up to the current slave
*/
if (!rx_slave || (before && rx_slave->speed < before->speed))
rx_slave = before;
if (rx_slave) rcu_read_lock();
bond_info->rx_slave = rx_slave; rx_slave = __rlb_next_rx_slave(bond);
rcu_read_unlock();
return rx_slave; return rx_slave;
} }
...@@ -587,7 +570,7 @@ static void rlb_req_update_subnet_clients(struct bonding *bond, __be32 src_ip) ...@@ -587,7 +570,7 @@ static void rlb_req_update_subnet_clients(struct bonding *bond, __be32 src_ip)
netdev_err(bond->dev, "found a client with no channel in the client's hash table\n"); netdev_err(bond->dev, "found a client with no channel in the client's hash table\n");
continue; continue;
} }
/*update all clients using this src_ip, that are not assigned /* update all clients using this src_ip, that are not assigned
* to the team's address (curr_active_slave) and have a known * to the team's address (curr_active_slave) and have a known
* unicast mac address. * unicast mac address.
*/ */
...@@ -713,9 +696,7 @@ static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond) ...@@ -713,9 +696,7 @@ static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond)
return NULL; return NULL;
if (arp->op_code == htons(ARPOP_REPLY)) { if (arp->op_code == htons(ARPOP_REPLY)) {
/* the arp must be sent on the selected /* the arp must be sent on the selected rx channel */
* rx channel
*/
tx_slave = rlb_choose_channel(skb, bond); tx_slave = rlb_choose_channel(skb, bond);
if (tx_slave) if (tx_slave)
ether_addr_copy(arp->mac_src, tx_slave->dev->dev_addr); ether_addr_copy(arp->mac_src, tx_slave->dev->dev_addr);
...@@ -774,7 +755,7 @@ static void rlb_rebalance(struct bonding *bond) ...@@ -774,7 +755,7 @@ static void rlb_rebalance(struct bonding *bond)
spin_unlock_bh(&bond->mode_lock); spin_unlock_bh(&bond->mode_lock);
} }
/* Caller must hold rx_hashtbl lock */ /* Caller must hold mode_lock */
static void rlb_init_table_entry_dst(struct rlb_client_info *entry) static void rlb_init_table_entry_dst(struct rlb_client_info *entry)
{ {
entry->used_next = RLB_NULL_INDEX; entry->used_next = RLB_NULL_INDEX;
...@@ -863,7 +844,8 @@ static void rlb_src_link(struct bonding *bond, u32 ip_src_hash, u32 ip_dst_hash) ...@@ -863,7 +844,8 @@ static void rlb_src_link(struct bonding *bond, u32 ip_src_hash, u32 ip_dst_hash)
} }
/* deletes all rx_hashtbl entries with arp->ip_src if their mac_src does /* deletes all rx_hashtbl entries with arp->ip_src if their mac_src does
* not match arp->mac_src */ * not match arp->mac_src
*/
static void rlb_purge_src_ip(struct bonding *bond, struct arp_pkt *arp) static void rlb_purge_src_ip(struct bonding *bond, struct arp_pkt *arp)
{ {
struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
...@@ -1040,8 +1022,9 @@ static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[]) ...@@ -1040,8 +1022,9 @@ static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[])
return 0; return 0;
} }
/* for rlb each slave must have a unique hw mac addresses so that */ /* for rlb each slave must have a unique hw mac addresses so that
/* each slave will receive packets destined to a different mac */ * each slave will receive packets destined to a different mac
*/
memcpy(s_addr.sa_data, addr, dev->addr_len); memcpy(s_addr.sa_data, addr, dev->addr_len);
s_addr.sa_family = dev->type; s_addr.sa_family = dev->type;
if (dev_set_mac_address(dev, &s_addr)) { if (dev_set_mac_address(dev, &s_addr)) {
...@@ -1052,13 +1035,10 @@ static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[]) ...@@ -1052,13 +1035,10 @@ static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[])
return 0; return 0;
} }
/* /* Swap MAC addresses between two slaves.
* Swap MAC addresses between two slaves.
* *
* Called with RTNL held, and no other locks. * Called with RTNL held, and no other locks.
*
*/ */
static void alb_swap_mac_addr(struct slave *slave1, struct slave *slave2) static void alb_swap_mac_addr(struct slave *slave1, struct slave *slave2)
{ {
u8 tmp_mac_addr[ETH_ALEN]; u8 tmp_mac_addr[ETH_ALEN];
...@@ -1069,8 +1049,7 @@ static void alb_swap_mac_addr(struct slave *slave1, struct slave *slave2) ...@@ -1069,8 +1049,7 @@ static void alb_swap_mac_addr(struct slave *slave1, struct slave *slave2)
} }
/* /* Send learning packets after MAC address swap.
* Send learning packets after MAC address swap.
* *
* Called with RTNL and no other locks * Called with RTNL and no other locks
*/ */
...@@ -1143,7 +1122,6 @@ static void alb_change_hw_addr_on_detach(struct bonding *bond, struct slave *sla ...@@ -1143,7 +1122,6 @@ static void alb_change_hw_addr_on_detach(struct bonding *bond, struct slave *sla
found_slave = bond_slave_has_mac(bond, slave->perm_hwaddr); found_slave = bond_slave_has_mac(bond, slave->perm_hwaddr);
if (found_slave) { if (found_slave) {
/* locking: needs RTNL and nothing else */
alb_swap_mac_addr(slave, found_slave); alb_swap_mac_addr(slave, found_slave);
alb_fasten_mac_swap(bond, slave, found_slave); alb_fasten_mac_swap(bond, slave, found_slave);
} }
...@@ -1192,7 +1170,8 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav ...@@ -1192,7 +1170,8 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
return 0; return 0;
/* Try setting slave mac to bond address and fall-through /* Try setting slave mac to bond address and fall-through
to code handling that situation below... */ * to code handling that situation below...
*/
alb_set_slave_mac_addr(slave, bond->dev->dev_addr); alb_set_slave_mac_addr(slave, bond->dev->dev_addr);
} }
...@@ -1300,7 +1279,6 @@ int bond_alb_initialize(struct bonding *bond, int rlb_enabled) ...@@ -1300,7 +1279,6 @@ int bond_alb_initialize(struct bonding *bond, int rlb_enabled)
if (rlb_enabled) { if (rlb_enabled) {
bond->alb_info.rlb_enabled = 1; bond->alb_info.rlb_enabled = 1;
/* initialize rlb */
res = rlb_initialize(bond); res = rlb_initialize(bond);
if (res) { if (res) {
tlb_deinitialize(bond); tlb_deinitialize(bond);
...@@ -1572,13 +1550,11 @@ void bond_alb_monitor(struct work_struct *work) ...@@ -1572,13 +1550,11 @@ void bond_alb_monitor(struct work_struct *work)
bond_info->tx_rebalance_counter = 0; bond_info->tx_rebalance_counter = 0;
} }
/* handle rlb stuff */
if (bond_info->rlb_enabled) { if (bond_info->rlb_enabled) {
if (bond_info->primary_is_promisc && if (bond_info->primary_is_promisc &&
(++bond_info->rlb_promisc_timeout_counter >= RLB_PROMISC_TIMEOUT)) { (++bond_info->rlb_promisc_timeout_counter >= RLB_PROMISC_TIMEOUT)) {
/* /* dev_set_promiscuity requires rtnl and
* dev_set_promiscuity requires rtnl and
* nothing else. Avoid race with bond_close. * nothing else. Avoid race with bond_close.
*/ */
rcu_read_unlock(); rcu_read_unlock();
...@@ -1648,8 +1624,7 @@ int bond_alb_init_slave(struct bonding *bond, struct slave *slave) ...@@ -1648,8 +1624,7 @@ int bond_alb_init_slave(struct bonding *bond, struct slave *slave)
return 0; return 0;
} }
/* /* Remove slave from tlb and rlb hash tables, and fix up MAC addresses
* Remove slave from tlb and rlb hash tables, and fix up MAC addresses
* if necessary. * if necessary.
* *
* Caller must hold RTNL and no other locks * Caller must hold RTNL and no other locks
...@@ -1736,8 +1711,7 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave ...@@ -1736,8 +1711,7 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
if (!swap_slave) if (!swap_slave)
swap_slave = bond_slave_has_mac(bond, bond->dev->dev_addr); swap_slave = bond_slave_has_mac(bond, bond->dev->dev_addr);
/* /* Arrange for swap_slave and new_slave to temporarily be
* Arrange for swap_slave and new_slave to temporarily be
* ignored so we can mess with their MAC addresses without * ignored so we can mess with their MAC addresses without
* fear of interference from transmit activity. * fear of interference from transmit activity.
*/ */
......
...@@ -13,9 +13,7 @@ ...@@ -13,9 +13,7 @@
static struct dentry *bonding_debug_root; static struct dentry *bonding_debug_root;
/* /* Show RLB hash table */
* Show RLB hash table
*/
static int bond_debug_rlb_hash_show(struct seq_file *m, void *v) static int bond_debug_rlb_hash_show(struct seq_file *m, void *v)
{ {
struct bonding *bond = m->private; struct bonding *bond = m->private;
......
This diff is collapsed.
...@@ -91,7 +91,6 @@ static struct net_device *bond_get_by_name(struct bond_net *bn, const char *ifna ...@@ -91,7 +91,6 @@ static struct net_device *bond_get_by_name(struct bond_net *bn, const char *ifna
* creates and deletes entire bonds. * creates and deletes entire bonds.
* *
* The class parameter is ignored. * The class parameter is ignored.
*
*/ */
static ssize_t bonding_store_bonds(struct class *cls, static ssize_t bonding_store_bonds(struct class *cls,
struct class_attribute *attr, struct class_attribute *attr,
......
...@@ -197,7 +197,8 @@ struct bonding { ...@@ -197,7 +197,8 @@ struct bonding {
struct slave *); struct slave *);
/* mode_lock is used for mode-specific locking needs, currently used by: /* mode_lock is used for mode-specific locking needs, currently used by:
* 3ad mode (4) - protect against running bond_3ad_unbind_slave() and * 3ad mode (4) - protect against running bond_3ad_unbind_slave() and
* bond_3ad_state_machine_handler() concurrently. * bond_3ad_state_machine_handler() concurrently and also
* the access to the state machine shared variables.
* TLB mode (5) - to sync the use and modifications of its hash table * TLB mode (5) - to sync the use and modifications of its hash table
* ALB mode (6) - to sync the use and modifications of its hash table * ALB mode (6) - to sync the use and modifications of its hash table
*/ */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment