From 2f268f129c2d1a05d297fe3ee34d393f862d2b22 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:07 +0200 Subject: net: add adj_list to save only neighbours Currently, we distinguish neighbours (first-level linked devices) from non-neighbours by the neighbour bool in the netdev_adjacent. This could be quite time-consuming in case we would like to traverse *only* through neighbours - cause we'd have to traverse through all devices and check for this flag, and in a (quite common) scenario where we have lots of vlans on top of bridge, which is on top of a bond - the bonding would have to go through all those vlans to get its upper neighbour linked devices. This situation is really unpleasant, cause there are already a lot of cases when a device with slaves needs to go through them in hot path. To fix this, introduce a new upper/lower device lists structure - adj_list, which contains only the neighbours. It works always in pair with the all_adj_list structure (renamed from upper/lower_dev_list), i.e. both of them contain the same links, only that all_adj_list contains also non-neighbour device links. It's really a small change visible, currently, only for __netdev_adjacent_dev_insert/remove(), and doesn't change the main linked logic at all. Also, add some comments a fix a name collision in netdev_for_each_upper_dev_rcu() and rework the naming by the following rules: netdev_(all_)(upper|lower)_* If "all_" is present, then we work with the whole list of upper/lower devices, otherwise - only with direct neighbours. Uninline functions - to get better stack traces. CC: "David S. Miller" CC: Eric Dumazet CC: Jiri Pirko CC: Alexander Duyck CC: Cong Wang Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_alb.c | 2 +- drivers/net/bonding/bond_main.c | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index f428ef574372..8524e33e6754 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -1019,7 +1019,7 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[]) /* loop through vlans and send one packet for each */ rcu_read_lock(); - netdev_for_each_upper_dev_rcu(bond->dev, upper, iter) { + netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) { if (upper->priv_flags & IFF_802_1Q_VLAN) alb_send_lp_vid(slave, mac_addr, vlan_dev_vlan_id(upper)); diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 55bbb8b8200c..91c4ab8913b1 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2267,7 +2267,7 @@ static bool bond_has_this_ip(struct bonding *bond, __be32 ip) return true; rcu_read_lock(); - netdev_for_each_upper_dev_rcu(bond->dev, upper, iter) { + netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) { if (ip == bond_confirm_addr(upper, 0, ip)) { ret = true; break; @@ -2342,10 +2342,12 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave) * * TODO: QinQ? */ - netdev_for_each_upper_dev_rcu(bond->dev, vlan_upper, vlan_iter) { + netdev_for_each_all_upper_dev_rcu(bond->dev, vlan_upper, + vlan_iter) { if (!is_vlan_dev(vlan_upper)) continue; - netdev_for_each_upper_dev_rcu(vlan_upper, upper, iter) { + netdev_for_each_all_upper_dev_rcu(vlan_upper, upper, + iter) { if (upper == rt->dst.dev) { vlan_id = vlan_dev_vlan_id(vlan_upper); rcu_read_unlock(); @@ -2358,7 +2360,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave) * our upper vlans, then just search for any dev that * matches, and in case it's a vlan - save the id */ - netdev_for_each_upper_dev_rcu(bond->dev, upper, iter) { + netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) { if (upper == rt->dst.dev) { /* if it's a vlan - get its VID */ if (is_vlan_dev(upper)) -- cgit v1.2.3 From 1f718f0f4f97145f4072d2d72dcf85069ca7226d Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:10 +0200 Subject: bonding: populate neighbour's private on enslave Use the new provided function when attaching the lower slave to populate its ->private with struct slave *new_slave. Also, move it to the end to be able to 'find' it only after it was completely initialized, and deinitialize in the first place on release. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 91c4ab8913b1..8e41416a1d52 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1233,11 +1233,12 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) } static int bond_master_upper_dev_link(struct net_device *bond_dev, - struct net_device *slave_dev) + struct net_device *slave_dev, + struct slave *slave) { int err; - err = netdev_master_upper_dev_link(slave_dev, bond_dev); + err = netdev_master_upper_dev_link_private(slave_dev, bond_dev, slave); if (err) return err; slave_dev->flags |= IFF_SLAVE; @@ -1413,17 +1414,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) } } - res = bond_master_upper_dev_link(bond_dev, slave_dev); - if (res) { - pr_debug("Error %d calling bond_master_upper_dev_link\n", res); - goto err_restore_mac; - } - /* open the slave since the application closed it */ res = dev_open(slave_dev); if (res) { pr_debug("Opening slave %s failed\n", slave_dev->name); - goto err_unset_master; + goto err_restore_mac; } new_slave->bond = bond; @@ -1637,6 +1632,13 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) goto err_dest_symlinks; } + res = bond_master_upper_dev_link(bond_dev, slave_dev, new_slave); + if (res) { + pr_debug("Error %d calling bond_master_upper_dev_link\n", res); + goto err_unregister; + } + + pr_info("%s: enslaving %s as a%s interface with a%s link.\n", bond_dev->name, slave_dev->name, bond_is_active_slave(new_slave) ? "n active" : " backup", @@ -1646,6 +1648,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) return 0; /* Undo stages on error */ +err_unregister: + netdev_rx_handler_unregister(slave_dev); + err_dest_symlinks: bond_destroy_slave_symlinks(bond_dev, slave_dev); @@ -1675,9 +1680,6 @@ err_close: slave_dev->priv_flags &= ~IFF_BONDING; dev_close(slave_dev); -err_unset_master: - bond_upper_dev_unlink(bond_dev, slave_dev); - err_restore_mac: if (!bond->params.fail_over_mac) { /* XXX TODO - fom follow mode needs to change master's @@ -1748,6 +1750,8 @@ static int __bond_release_one(struct net_device *bond_dev, } write_unlock_bh(&bond->lock); + + bond_upper_dev_unlink(bond_dev, slave_dev); /* unregister rx_handler early so bond_handle_frame wouldn't be called * for this slave anymore. */ @@ -1866,8 +1870,6 @@ static int __bond_release_one(struct net_device *bond_dev, bond_hw_addr_flush(bond_dev, slave_dev); } - bond_upper_dev_unlink(bond_dev, slave_dev); - slave_disable_netpoll(slave); /* close slave before restoring its mac address */ -- cgit v1.2.3 From 46bb4807b5d95a049b008efd639ff8942970d815 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:11 +0200 Subject: bonding: modify bond_get_slave_by_dev() to use neighbours It should be used under rtnl/bonding lock, so use the non-RCU version. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bonding.h | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 03cf3fd14490..90f4847b264b 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -276,13 +276,7 @@ struct bonding { static inline struct slave *bond_get_slave_by_dev(struct bonding *bond, struct net_device *slave_dev) { - struct slave *slave = NULL; - - bond_for_each_slave(bond, slave) - if (slave->dev == slave_dev) - return slave; - - return NULL; + return netdev_lower_dev_get_private(bond->dev, slave_dev); } static inline struct bonding *bond_get_bond_by_slave(struct slave *slave) -- cgit v1.2.3 From 81f23b13ac985e9a3cfb889c690695a8932e02c2 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:13 +0200 Subject: bonding: remove bond_for_each_slave_continue_reverse() We only use it in rollback scenarios and can easily use the standart bond_for_each_dev() instead. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_alb.c | 14 ++++++++------ drivers/net/bonding/bond_main.c | 34 ++++++++++++++++++++++------------ drivers/net/bonding/bonding.h | 10 ---------- 3 files changed, 30 insertions(+), 28 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index 8524e33e6754..6437657375d7 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -1246,9 +1246,9 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav */ static int alb_set_mac_address(struct bonding *bond, void *addr) { - char tmp_addr[ETH_ALEN]; - struct slave *slave; + struct slave *slave, *rollback_slave; struct sockaddr sa; + char tmp_addr[ETH_ALEN]; int res; if (bond->alb_info.rlb_enabled) @@ -1274,10 +1274,12 @@ unwind: sa.sa_family = bond->dev->type; /* unwind from head to the slave that failed */ - bond_for_each_slave_continue_reverse(bond, slave) { - memcpy(tmp_addr, slave->dev->dev_addr, ETH_ALEN); - dev_set_mac_address(slave->dev, &sa); - memcpy(slave->dev->dev_addr, tmp_addr, ETH_ALEN); + bond_for_each_slave(bond, rollback_slave) { + if (rollback_slave == slave) + break; + memcpy(tmp_addr, rollback_slave->dev->dev_addr, ETH_ALEN); + dev_set_mac_address(rollback_slave->dev, &sa); + memcpy(rollback_slave->dev->dev_addr, tmp_addr, ETH_ALEN); } return res; diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 8e41416a1d52..d94b6c16537d 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -332,7 +332,7 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev, __be16 proto, u16 vid) { struct bonding *bond = netdev_priv(bond_dev); - struct slave *slave; + struct slave *slave, *rollback_slave; int res; bond_for_each_slave(bond, slave) { @@ -344,9 +344,13 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev, return 0; unwind: - /* unwind from the slave that failed */ - bond_for_each_slave_continue_reverse(bond, slave) - vlan_vid_del(slave->dev, proto, vid); + /* unwind to the slave that failed */ + bond_for_each_slave(bond, rollback_slave) { + if (rollback_slave == slave) + break; + + vlan_vid_del(rollback_slave->dev, proto, vid); + } return res; } @@ -3468,7 +3472,7 @@ static int bond_neigh_setup(struct net_device *dev, static int bond_change_mtu(struct net_device *bond_dev, int new_mtu) { struct bonding *bond = netdev_priv(bond_dev); - struct slave *slave; + struct slave *slave, *rollback_slave; int res = 0; pr_debug("bond=%p, name=%s, new_mtu=%d\n", bond, @@ -3517,13 +3521,16 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu) unwind: /* unwind from head to the slave that failed */ - bond_for_each_slave_continue_reverse(bond, slave) { + bond_for_each_slave(bond, rollback_slave) { int tmp_res; - tmp_res = dev_set_mtu(slave->dev, bond_dev->mtu); + if (rollback_slave == slave) + break; + + tmp_res = dev_set_mtu(rollback_slave->dev, bond_dev->mtu); if (tmp_res) { pr_debug("unwind err %d dev %s\n", - tmp_res, slave->dev->name); + tmp_res, rollback_slave->dev->name); } } @@ -3540,8 +3547,8 @@ unwind: static int bond_set_mac_address(struct net_device *bond_dev, void *addr) { struct bonding *bond = netdev_priv(bond_dev); + struct slave *slave, *rollback_slave; struct sockaddr *sa = addr, tmp_sa; - struct slave *slave; int res = 0; if (bond->params.mode == BOND_MODE_ALB) @@ -3607,13 +3614,16 @@ unwind: tmp_sa.sa_family = bond_dev->type; /* unwind from head to the slave that failed */ - bond_for_each_slave_continue_reverse(bond, slave) { + bond_for_each_slave(bond, rollback_slave) { int tmp_res; - tmp_res = dev_set_mac_address(slave->dev, &tmp_sa); + if (rollback_slave == slave) + break; + + tmp_res = dev_set_mac_address(rollback_slave->dev, &tmp_sa); if (tmp_res) { pr_debug("unwind err %d dev %s\n", - tmp_res, slave->dev->name); + tmp_res, rollback_slave->dev->name); } } diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 90f4847b264b..0b0c3df21a89 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -120,16 +120,6 @@ #define bond_for_each_slave_rcu(bond, pos) \ list_for_each_entry_rcu(pos, &(bond)->slave_list, list) -/** - * bond_for_each_slave_reverse - iterate in reverse from a given position - * @bond: the bond holding this list - * @pos: slave to continue from - * - * Caller must hold bond->lock - */ -#define bond_for_each_slave_continue_reverse(bond, pos) \ - list_for_each_entry_continue_reverse(pos, &(bond)->slave_list, list) - #ifdef CONFIG_NET_POLL_CONTROLLER extern atomic_t netpoll_block_tx; -- cgit v1.2.3 From 9caff1e7b761c28018bf1858f6661439b4055f51 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:14 +0200 Subject: bonding: make bond_for_each_slave() use lower neighbour's private It needs a list_head *iter, so add it wherever needed. Use both non-rcu and rcu variants. CC: Jay Vosburgh CC: Andy Gospodarek CC: Dimitris Michailidis Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 6 +- drivers/net/bonding/bond_alb.c | 18 ++++-- drivers/net/bonding/bond_main.c | 86 ++++++++++++++++--------- drivers/net/bonding/bond_procfs.c | 5 +- drivers/net/bonding/bond_sysfs.c | 23 ++++--- drivers/net/bonding/bonding.h | 12 ++-- drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 3 +- 7 files changed, 98 insertions(+), 55 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 0d8f427ade93..3847aee34968 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -2419,6 +2419,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev) { struct slave *slave, *start_at; struct bonding *bond = netdev_priv(dev); + struct list_head *iter; int slave_agg_no; int slaves_in_agg; int agg_id; @@ -2444,7 +2445,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev) slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator; if (agg && (agg->aggregator_identifier == agg_id)) { @@ -2515,11 +2516,12 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond, void bond_3ad_update_lacp_rate(struct bonding *bond) { struct port *port = NULL; + struct list_head *iter; struct slave *slave; int lacp_fast; lacp_fast = bond->params.lacp_fast; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { port = &(SLAVE_AD_INFO(slave).port); __get_state_machine_lock(port); if (lacp_fast) diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index 6437657375d7..f4929cee21a6 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -223,13 +223,14 @@ static long long compute_gap(struct slave *slave) static struct slave *tlb_get_least_loaded_slave(struct bonding *bond) { struct slave *slave, *least_loaded; + struct list_head *iter; long long max_gap; least_loaded = NULL; max_gap = LLONG_MIN; /* Find the slave with the largest gap */ - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (SLAVE_IS_OK(slave)) { long long gap = compute_gap(slave); @@ -1172,8 +1173,9 @@ static void alb_change_hw_addr_on_detach(struct bonding *bond, struct slave *sla */ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slave *slave) { - struct slave *tmp_slave1, *free_mac_slave = NULL; struct slave *has_bond_addr = bond->curr_active_slave; + struct slave *tmp_slave1, *free_mac_slave = NULL; + struct list_head *iter; if (list_empty(&bond->slave_list)) { /* this is the first slave */ @@ -1196,7 +1198,7 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav /* The slave's address is equal to the address of the bond. * Search for a spare address in the bond for this slave. */ - bond_for_each_slave(bond, tmp_slave1) { + bond_for_each_slave(bond, tmp_slave1, iter) { if (!bond_slave_has_mac(bond, tmp_slave1->perm_hwaddr)) { /* no slave has tmp_slave1's perm addr * as its curr addr @@ -1247,6 +1249,7 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav static int alb_set_mac_address(struct bonding *bond, void *addr) { struct slave *slave, *rollback_slave; + struct list_head *iter; struct sockaddr sa; char tmp_addr[ETH_ALEN]; int res; @@ -1254,7 +1257,7 @@ static int alb_set_mac_address(struct bonding *bond, void *addr) if (bond->alb_info.rlb_enabled) return 0; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { /* save net_device's current hw address */ memcpy(tmp_addr, slave->dev->dev_addr, ETH_ALEN); @@ -1274,7 +1277,7 @@ unwind: sa.sa_family = bond->dev->type; /* unwind from head to the slave that failed */ - bond_for_each_slave(bond, rollback_slave) { + bond_for_each_slave(bond, rollback_slave, iter) { if (rollback_slave == slave) break; memcpy(tmp_addr, rollback_slave->dev->dev_addr, ETH_ALEN); @@ -1460,6 +1463,7 @@ void bond_alb_monitor(struct work_struct *work) struct bonding *bond = container_of(work, struct bonding, alb_work.work); struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); + struct list_head *iter; struct slave *slave; read_lock(&bond->lock); @@ -1482,7 +1486,7 @@ void bond_alb_monitor(struct work_struct *work) */ read_lock(&bond->curr_slave_lock); - bond_for_each_slave(bond, slave) + bond_for_each_slave(bond, slave, iter) alb_send_learning_packets(slave, slave->dev->dev_addr); read_unlock(&bond->curr_slave_lock); @@ -1495,7 +1499,7 @@ void bond_alb_monitor(struct work_struct *work) read_lock(&bond->curr_slave_lock); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { tlb_clear_slave(bond, slave, 1); if (slave == bond->curr_active_slave) { SLAVE_TLB_INFO(slave).load = diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index d94b6c16537d..9064e24de35a 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -333,9 +333,10 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev, { struct bonding *bond = netdev_priv(bond_dev); struct slave *slave, *rollback_slave; + struct list_head *iter; int res; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { res = vlan_vid_add(slave->dev, proto, vid); if (res) goto unwind; @@ -345,7 +346,7 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev, unwind: /* unwind to the slave that failed */ - bond_for_each_slave(bond, rollback_slave) { + bond_for_each_slave(bond, rollback_slave, iter) { if (rollback_slave == slave) break; @@ -364,9 +365,10 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev, __be16 proto, u16 vid) { struct bonding *bond = netdev_priv(bond_dev); + struct list_head *iter; struct slave *slave; - bond_for_each_slave(bond, slave) + bond_for_each_slave(bond, slave, iter) vlan_vid_del(slave->dev, proto, vid); if (bond_is_lb(bond)) @@ -386,6 +388,7 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev, */ static int bond_set_carrier(struct bonding *bond) { + struct list_head *iter; struct slave *slave; if (list_empty(&bond->slave_list)) @@ -394,7 +397,7 @@ static int bond_set_carrier(struct bonding *bond) if (bond->params.mode == BOND_MODE_8023AD) return bond_3ad_set_carrier(bond); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (slave->link == BOND_LINK_UP) { if (!netif_carrier_ok(bond->dev)) { netif_carrier_on(bond->dev); @@ -526,7 +529,9 @@ static int bond_check_dev_link(struct bonding *bond, */ static int bond_set_promiscuity(struct bonding *bond, int inc) { + struct list_head *iter; int err = 0; + if (USES_PRIMARY(bond->params.mode)) { /* write lock already acquired */ if (bond->curr_active_slave) { @@ -536,7 +541,7 @@ static int bond_set_promiscuity(struct bonding *bond, int inc) } else { struct slave *slave; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { err = dev_set_promiscuity(slave->dev, inc); if (err) return err; @@ -550,7 +555,9 @@ static int bond_set_promiscuity(struct bonding *bond, int inc) */ static int bond_set_allmulti(struct bonding *bond, int inc) { + struct list_head *iter; int err = 0; + if (USES_PRIMARY(bond->params.mode)) { /* write lock already acquired */ if (bond->curr_active_slave) { @@ -560,7 +567,7 @@ static int bond_set_allmulti(struct bonding *bond, int inc) } else { struct slave *slave; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { err = dev_set_allmulti(slave->dev, inc); if (err) return err; @@ -1050,9 +1057,10 @@ static void bond_poll_controller(struct net_device *bond_dev) static void bond_netpoll_cleanup(struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); + struct list_head *iter; struct slave *slave; - bond_for_each_slave(bond, slave) + bond_for_each_slave(bond, slave, iter) if (IS_UP(slave->dev)) slave_disable_netpoll(slave); } @@ -1060,10 +1068,11 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev) static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni, gfp_t gfp) { struct bonding *bond = netdev_priv(dev); + struct list_head *iter; struct slave *slave; int err = 0; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { err = slave_enable_netpoll(slave); if (err) { bond_netpoll_cleanup(dev); @@ -1091,6 +1100,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev, netdev_features_t features) { struct bonding *bond = netdev_priv(dev); + struct list_head *iter; netdev_features_t mask; struct slave *slave; @@ -1104,7 +1114,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev, features &= ~NETIF_F_ONE_FOR_ALL; features |= NETIF_F_ALL_FOR_ALL; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { features = netdev_increment_features(features, slave->dev->features, mask); @@ -1122,16 +1132,17 @@ static void bond_compute_features(struct bonding *bond) { unsigned int flags, dst_release_flag = IFF_XMIT_DST_RELEASE; netdev_features_t vlan_features = BOND_VLAN_FEATURES; + struct net_device *bond_dev = bond->dev; + struct list_head *iter; + struct slave *slave; unsigned short max_hard_header_len = ETH_HLEN; unsigned int gso_max_size = GSO_MAX_SIZE; - struct net_device *bond_dev = bond->dev; u16 gso_max_segs = GSO_MAX_SEGS; - struct slave *slave; if (list_empty(&bond->slave_list)) goto done; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { vlan_features = netdev_increment_features(vlan_features, slave->dev->vlan_features, BOND_VLAN_FEATURES); @@ -1993,11 +2004,12 @@ static int bond_info_query(struct net_device *bond_dev, struct ifbond *info) static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *info) { struct bonding *bond = netdev_priv(bond_dev); + struct list_head *iter; int i = 0, res = -ENODEV; struct slave *slave; read_lock(&bond->lock); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (i++ == (int)info->slave_id) { res = 0; strcpy(info->slave_name, slave->dev->name); @@ -2018,12 +2030,13 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in static int bond_miimon_inspect(struct bonding *bond) { int link_state, commit = 0; + struct list_head *iter; struct slave *slave; bool ignore_updelay; ignore_updelay = !bond->curr_active_slave ? true : false; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { slave->new_link = BOND_LINK_NOCHANGE; link_state = bond_check_dev_link(bond, slave->dev, 0); @@ -2117,9 +2130,10 @@ static int bond_miimon_inspect(struct bonding *bond) static void bond_miimon_commit(struct bonding *bond) { + struct list_head *iter; struct slave *slave; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { switch (slave->new_link) { case BOND_LINK_NOCHANGE: continue; @@ -2513,6 +2527,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work) struct bonding *bond = container_of(work, struct bonding, arp_work.work); struct slave *slave, *oldcurrent; + struct list_head *iter; int do_failover = 0; read_lock(&bond->lock); @@ -2529,7 +2544,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) { + bond_for_each_slave(bond, slave, iter) { unsigned long trans_start = dev_trans_start(slave->dev); if (slave->link != BOND_LINK_UP) { @@ -2620,10 +2635,11 @@ re_arm: static int bond_ab_arp_inspect(struct bonding *bond) { unsigned long trans_start, last_rx; + struct list_head *iter; struct slave *slave; int commit = 0; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { slave->new_link = BOND_LINK_NOCHANGE; last_rx = slave_last_rx(bond, slave); @@ -2690,9 +2706,10 @@ static int bond_ab_arp_inspect(struct bonding *bond) static void bond_ab_arp_commit(struct bonding *bond) { unsigned long trans_start; + struct list_head *iter; struct slave *slave; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { switch (slave->new_link) { case BOND_LINK_NOCHANGE: continue; @@ -3156,13 +3173,14 @@ static void bond_work_cancel_all(struct bonding *bond) static int bond_open(struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); + struct list_head *iter; struct slave *slave; /* reset slave->backup and slave->inactive */ read_lock(&bond->lock); if (!list_empty(&bond->slave_list)) { read_lock(&bond->curr_slave_lock); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP) && (slave != bond->curr_active_slave)) { bond_set_slave_inactive_flags(slave); @@ -3222,12 +3240,13 @@ static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev, { struct bonding *bond = netdev_priv(bond_dev); struct rtnl_link_stats64 temp; + struct list_head *iter; struct slave *slave; memset(stats, 0, sizeof(*stats)); read_lock_bh(&bond->lock); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { const struct rtnl_link_stats64 *sstats = dev_get_stats(slave->dev, &temp); @@ -3394,6 +3413,7 @@ static void bond_change_rx_flags(struct net_device *bond_dev, int change) static void bond_set_rx_mode(struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); + struct list_head *iter; struct slave *slave; ASSERT_RTNL(); @@ -3405,7 +3425,7 @@ static void bond_set_rx_mode(struct net_device *bond_dev) dev_mc_sync(slave->dev, bond_dev); } } else { - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { dev_uc_sync_multiple(slave->dev, bond_dev); dev_mc_sync_multiple(slave->dev, bond_dev); } @@ -3473,6 +3493,7 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu) { struct bonding *bond = netdev_priv(bond_dev); struct slave *slave, *rollback_slave; + struct list_head *iter; int res = 0; pr_debug("bond=%p, name=%s, new_mtu=%d\n", bond, @@ -3493,7 +3514,7 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu) * call to the base driver. */ - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { pr_debug("s %p s->p %p c_m %p\n", slave, bond_prev_slave(bond, slave), @@ -3521,7 +3542,7 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu) unwind: /* unwind from head to the slave that failed */ - bond_for_each_slave(bond, rollback_slave) { + bond_for_each_slave(bond, rollback_slave, iter) { int tmp_res; if (rollback_slave == slave) @@ -3549,6 +3570,7 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr) struct bonding *bond = netdev_priv(bond_dev); struct slave *slave, *rollback_slave; struct sockaddr *sa = addr, tmp_sa; + struct list_head *iter; int res = 0; if (bond->params.mode == BOND_MODE_ALB) @@ -3582,7 +3604,7 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr) * call to the base driver. */ - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { const struct net_device_ops *slave_ops = slave->dev->netdev_ops; pr_debug("slave %p %s\n", slave, slave->dev->name); @@ -3614,7 +3636,7 @@ unwind: tmp_sa.sa_family = bond_dev->type; /* unwind from head to the slave that failed */ - bond_for_each_slave(bond, rollback_slave) { + bond_for_each_slave(bond, rollback_slave, iter) { int tmp_res; if (rollback_slave == slave) @@ -3642,11 +3664,12 @@ unwind: */ void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id) { + struct list_head *iter; struct slave *slave; int i = slave_id; /* Here we start from the slave with slave_id */ - bond_for_each_slave_rcu(bond, slave) { + bond_for_each_slave_rcu(bond, slave, iter) { if (--i < 0) { if (slave_can_tx(slave)) { bond_dev_queue_xmit(bond, skb, slave->dev); @@ -3657,7 +3680,7 @@ void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id) /* Here we start from the first slave up to slave_id */ i = slave_id; - bond_for_each_slave_rcu(bond, slave) { + bond_for_each_slave_rcu(bond, slave, iter) { if (--i < 0) break; if (slave_can_tx(slave)) { @@ -3734,8 +3757,9 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); struct slave *slave = NULL; + struct list_head *iter; - bond_for_each_slave_rcu(bond, slave) { + bond_for_each_slave_rcu(bond, slave, iter) { if (bond_is_last_slave(bond, slave)) break; if (IS_UP(slave->dev) && slave->link == BOND_LINK_UP) { @@ -3784,13 +3808,14 @@ static inline int bond_slave_override(struct bonding *bond, { struct slave *slave = NULL; struct slave *check_slave; + struct list_head *iter; int res = 1; if (!skb->queue_mapping) return 1; /* Find out if any slaves have the same mapping as this skb. */ - bond_for_each_slave_rcu(bond, check_slave) { + bond_for_each_slave_rcu(bond, check_slave, iter) { if (check_slave->queue_id == skb->queue_mapping) { slave = check_slave; break; @@ -3922,6 +3947,7 @@ static int bond_ethtool_get_settings(struct net_device *bond_dev, { struct bonding *bond = netdev_priv(bond_dev); unsigned long speed = 0; + struct list_head *iter; struct slave *slave; ecmd->duplex = DUPLEX_UNKNOWN; @@ -3933,7 +3959,7 @@ static int bond_ethtool_get_settings(struct net_device *bond_dev, * this is an accurate maximum. */ read_lock(&bond->lock); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (SLAVE_IS_OK(slave)) { if (slave->speed != SPEED_UNKNOWN) speed += slave->speed; diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c index 20a6ee25bb63..7af5646e4410 100644 --- a/drivers/net/bonding/bond_procfs.c +++ b/drivers/net/bonding/bond_procfs.c @@ -10,8 +10,9 @@ static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos) __acquires(&bond->lock) { struct bonding *bond = seq->private; - loff_t off = 0; + struct list_head *iter; struct slave *slave; + loff_t off = 0; /* make sure the bond won't be taken away */ rcu_read_lock(); @@ -20,7 +21,7 @@ static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos) if (*pos == 0) return SEQ_START_TOKEN; - bond_for_each_slave(bond, slave) + bond_for_each_slave(bond, slave, iter) if (++off == *pos) return slave; diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index c29b836749b6..e747415b5cb0 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -210,11 +210,12 @@ static ssize_t bonding_show_slaves(struct device *d, struct device_attribute *attr, char *buf) { struct bonding *bond = to_bond(d); + struct list_head *iter; struct slave *slave; int res = 0; read_lock(&bond->lock); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (res > (PAGE_SIZE - IFNAMSIZ)) { /* not enough space for another interface name */ if ((PAGE_SIZE - res) > 10) @@ -656,6 +657,7 @@ static ssize_t bonding_store_arp_targets(struct device *d, const char *buf, size_t count) { struct bonding *bond = to_bond(d); + struct list_head *iter; struct slave *slave; __be32 newtarget, *targets; unsigned long *targets_rx; @@ -688,7 +690,7 @@ static ssize_t bonding_store_arp_targets(struct device *d, &newtarget); /* not to race with bond_arp_rcv */ write_lock_bh(&bond->lock); - bond_for_each_slave(bond, slave) + bond_for_each_slave(bond, slave, iter) slave->target_last_arp_rx[ind] = jiffies; targets[ind] = newtarget; write_unlock_bh(&bond->lock); @@ -714,7 +716,7 @@ static ssize_t bonding_store_arp_targets(struct device *d, &newtarget); write_lock_bh(&bond->lock); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { targets_rx = slave->target_last_arp_rx; j = ind; for (; (j < BOND_MAX_ARP_TARGETS-1) && targets[j+1]; j++) @@ -1111,6 +1113,7 @@ static ssize_t bonding_store_primary(struct device *d, const char *buf, size_t count) { struct bonding *bond = to_bond(d); + struct list_head *iter; char ifname[IFNAMSIZ]; struct slave *slave; @@ -1138,7 +1141,7 @@ static ssize_t bonding_store_primary(struct device *d, goto out; } - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (strncmp(slave->dev->name, ifname, IFNAMSIZ) == 0) { pr_info("%s: Setting %s as primary slave.\n", bond->dev->name, slave->dev->name); @@ -1286,6 +1289,7 @@ static ssize_t bonding_store_active_slave(struct device *d, { struct slave *slave, *old_active, *new_active; struct bonding *bond = to_bond(d); + struct list_head *iter; char ifname[IFNAMSIZ]; if (!rtnl_trylock()) @@ -1313,7 +1317,7 @@ static ssize_t bonding_store_active_slave(struct device *d, goto out; } - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (strncmp(slave->dev->name, ifname, IFNAMSIZ) == 0) { old_active = bond->curr_active_slave; new_active = slave; @@ -1493,6 +1497,7 @@ static ssize_t bonding_show_queue_id(struct device *d, char *buf) { struct bonding *bond = to_bond(d); + struct list_head *iter; struct slave *slave; int res = 0; @@ -1500,7 +1505,7 @@ static ssize_t bonding_show_queue_id(struct device *d, return restart_syscall(); read_lock(&bond->lock); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (res > (PAGE_SIZE - IFNAMSIZ - 6)) { /* not enough space for another interface_name:queue_id pair */ if ((PAGE_SIZE - res) > 10) @@ -1529,6 +1534,7 @@ static ssize_t bonding_store_queue_id(struct device *d, { struct slave *slave, *update_slave; struct bonding *bond = to_bond(d); + struct list_head *iter; u16 qid; int ret = count; char *delim; @@ -1565,7 +1571,7 @@ static ssize_t bonding_store_queue_id(struct device *d, /* Search for thes slave and check for duplicate qids */ update_slave = NULL; - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (sdev == slave->dev) /* * We don't need to check the matching @@ -1619,6 +1625,7 @@ static ssize_t bonding_store_slaves_active(struct device *d, { struct bonding *bond = to_bond(d); int new_value, ret = count; + struct list_head *iter; struct slave *slave; if (sscanf(buf, "%d", &new_value) != 1) { @@ -1641,7 +1648,7 @@ static ssize_t bonding_store_slaves_active(struct device *d, } read_lock(&bond->lock); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (!bond_is_active_slave(slave)) { if (new_value) slave->inactive = 0; diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 0b0c3df21a89..96f571dd0bb6 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -110,15 +110,16 @@ * bond_for_each_slave - iterate over all slaves * @bond: the bond holding this list * @pos: current slave + * @iter: list_head * iterator * * Caller must hold bond->lock */ -#define bond_for_each_slave(bond, pos) \ - list_for_each_entry(pos, &(bond)->slave_list, list) +#define bond_for_each_slave(bond, pos, iter) \ + netdev_for_each_lower_private((bond)->dev, pos, iter) /* Caller must have rcu_read_lock */ -#define bond_for_each_slave_rcu(bond, pos) \ - list_for_each_entry_rcu(pos, &(bond)->slave_list, list) +#define bond_for_each_slave_rcu(bond, pos, iter) \ + netdev_for_each_lower_private_rcu((bond)->dev, pos, iter) #ifdef CONFIG_NET_POLL_CONTROLLER extern atomic_t netpoll_block_tx; @@ -476,9 +477,10 @@ static inline void bond_destroy_proc_dir(struct bond_net *bn) static inline struct slave *bond_slave_has_mac(struct bonding *bond, const u8 *mac) { + struct list_head *iter; struct slave *tmp; - bond_for_each_slave(bond, tmp) + bond_for_each_slave(bond, tmp, iter) if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr)) return tmp; diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c index c73cabdbd4c0..85d0cda5fbfa 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c @@ -3983,6 +3983,7 @@ static int cxgb4_inet6addr_handler(struct notifier_block *this, struct net_device *event_dev; int ret = NOTIFY_DONE; struct bonding *bond = netdev_priv(ifa->idev->dev); + struct list_head *iter; struct slave *slave; struct pci_dev *first_pdev = NULL; @@ -3995,7 +3996,7 @@ static int cxgb4_inet6addr_handler(struct notifier_block *this, * in all of them only once. */ read_lock(&bond->lock); - bond_for_each_slave(bond, slave) { + bond_for_each_slave(bond, slave, iter) { if (!first_pdev) { ret = clip_add(slave->dev, ifa, event); /* If clip_add is success then only initialize -- cgit v1.2.3 From 544a028e65a9dadc13c3d12fb009b4bcd5338a9f Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:15 +0200 Subject: bonding: use bond_for_each_slave() in bond_uninit() We're safe agains removal there, cause we use neighbours primitives. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 9064e24de35a..85e99aedabb1 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4090,12 +4090,13 @@ static void bond_setup(struct net_device *bond_dev) static void bond_uninit(struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); - struct slave *slave, *tmp_slave; + struct list_head *iter; + struct slave *slave; bond_netpoll_cleanup(bond_dev); /* Release the bonded slaves */ - list_for_each_entry_safe(slave, tmp_slave, &bond->slave_list, list) + bond_for_each_slave(bond, slave, iter) __bond_release_one(bond_dev, slave->dev, true); pr_info("%s: released all slaves\n", bond_dev->name); -- cgit v1.2.3 From c33d78874eeb6c28909158719043fa2a5fd18f0a Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:16 +0200 Subject: bonding: rework bond_3ad_xmit_xor() to use bond_for_each_slave() only Currently, there are two loops - first we find the first slave in an aggregator after the xmit_hash_policy() returned number, and after that we loop from that slave, over bonding head, and till that slave to find any suitable slave to send the packet through. Replace it by just one bond_for_each_slave() loop, which first loops through the requested number of slaves, saving the first suitable one, and after that we've hit the requested number of slaves to skip - search for any up slave to send the packet through. If we don't find such kind of slave - then just send the packet through the first suitable slave found. Logic remains unchainged, and we skip two loops. Also, refactor it a bit for readability. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 46 ++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 24 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 3847aee34968..c861ee7e65ff 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -2417,15 +2417,15 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info) int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev) { - struct slave *slave, *start_at; 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 slave_agg_no; int slaves_in_agg; - int agg_id; - int i; - struct ad_info ad_info; + int slave_agg_no; int res = 1; + int agg_id; read_lock(&bond->lock); if (__bond_3ad_get_active_agg_info(bond, &ad_info)) { @@ -2438,20 +2438,28 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev) agg_id = ad_info.aggregator_id; if (slaves_in_agg == 0) { - /*the aggregator is empty*/ pr_debug("%s: Error: active aggregator is empty\n", dev->name); goto out; } slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg); + first_ok_slave = NULL; bond_for_each_slave(bond, slave, iter) { - struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator; + agg = SLAVE_AD_INFO(slave).port.aggregator; + if (!agg || agg->aggregator_identifier != agg_id) + continue; - if (agg && (agg->aggregator_identifier == agg_id)) { + if (slave_agg_no >= 0) { + if (!first_ok_slave && SLAVE_IS_OK(slave)) + first_ok_slave = slave; slave_agg_no--; - if (slave_agg_no < 0) - break; + continue; + } + + if (SLAVE_IS_OK(slave)) { + res = bond_dev_queue_xmit(bond, skb, slave->dev); + goto out; } } @@ -2461,20 +2469,10 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev) goto out; } - start_at = slave; - - bond_for_each_slave_from(bond, slave, i, start_at) { - int slave_agg_id = 0; - struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator; - - if (agg) - slave_agg_id = agg->aggregator_identifier; - - if (SLAVE_IS_OK(slave) && agg && (slave_agg_id == agg_id)) { - res = bond_dev_queue_xmit(bond, skb, slave->dev); - break; - } - } + /* we couldn't find any suitable slave after the agg_no, so use the + * first suitable found, if found. */ + if (first_ok_slave) + res = bond_dev_queue_xmit(bond, skb, first_ok_slave->dev); out: read_unlock(&bond->lock); -- cgit v1.2.3 From 6475ae4ceea2f430db1daabf6460a9f36bc97438 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:17 +0200 Subject: bonding: rework rlb_next_rx_slave() to use bond_for_each_slave() Currently, we're using bond_for_each_slave_from(), which is really hard to implement under RCU and/or neighbour list. Remove it and use bond_for_each_slave() instead, taking care of the last used slave. Also, rename next_rx_slave to rx_slave and store the current (last) rx_slave. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_alb.c | 41 +++++++++++++++++++++-------------------- drivers/net/bonding/bond_alb.h | 4 +--- 2 files changed, 22 insertions(+), 23 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index f4929cee21a6..c5b85ad5554d 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -383,30 +383,31 @@ out: static struct slave *rlb_next_rx_slave(struct bonding *bond) { struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); - struct slave *rx_slave, *slave, *start_at; - int i = 0; - - if (bond_info->next_rx_slave) - start_at = bond_info->next_rx_slave; - else - start_at = bond_first_slave(bond); - - rx_slave = NULL; + struct slave *before = NULL, *rx_slave = NULL, *slave; + struct list_head *iter; + bool found = false; - bond_for_each_slave_from(bond, slave, i, start_at) { - if (SLAVE_IS_OK(slave)) { - if (!rx_slave) { - rx_slave = slave; - } else if (slave->speed > rx_slave->speed) { + bond_for_each_slave(bond, slave, iter) { + if (!SLAVE_IS_OK(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) { - slave = bond_next_slave(bond, rx_slave); - bond_info->next_rx_slave = slave; - } + if (rx_slave) + bond_info->rx_slave = rx_slave; return rx_slave; } @@ -1611,7 +1612,7 @@ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave) tlb_clear_slave(bond, slave, 0); if (bond->alb_info.rlb_enabled) { - bond->alb_info.next_rx_slave = NULL; + bond->alb_info.rx_slave = NULL; rlb_clear_slave(bond, slave); } } diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h index c5eff5dafdfe..4226044efd08 100644 --- a/drivers/net/bonding/bond_alb.h +++ b/drivers/net/bonding/bond_alb.h @@ -154,9 +154,7 @@ struct alb_bond_info { u8 rx_ntt; /* flag - need to transmit * to all rx clients */ - struct slave *next_rx_slave;/* next slave to be assigned - * to a new rx client for - */ + struct slave *rx_slave;/* last slave to xmit from */ u8 primary_is_promisc; /* boolean */ u32 rlb_promisc_timeout_counter;/* counts primary * promiscuity time -- cgit v1.2.3 From 77140d2951432487d012dbcdcf124168eafc49ca Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:18 +0200 Subject: bonding: rework bond_find_best_slave() to use bond_for_each_slave() bond_find_best_slave() does not have to be balanced - i.e. return the slave that is *after* some other slave, but rather return the best slave that suits, except of bond->primary_slave - in which case we just return it if it's suitable. After that we just look through all the slaves and return either first up slave or the slave whose link came back earliest. We also don't care about curr_active_slave lock cause we use it in bond_should_change_active() only and there we take it right away - i.e. it won't go away. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 43 ++++++++++++----------------------------- 1 file changed, 12 insertions(+), 31 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 85e99aedabb1..6abbfaca0b93 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -785,43 +785,24 @@ static bool bond_should_change_active(struct bonding *bond) /** * find_best_interface - select the best available slave to be the active one * @bond: our bonding struct - * - * Warning: Caller must hold curr_slave_lock for writing. */ static struct slave *bond_find_best_slave(struct bonding *bond) { - struct slave *new_active, *old_active; - struct slave *bestslave = NULL; + struct slave *slave, *bestslave = NULL; + struct list_head *iter; int mintime = bond->params.updelay; - int i; - new_active = bond->curr_active_slave; - - if (!new_active) { /* there were no active slaves left */ - new_active = bond_first_slave(bond); - if (!new_active) - return NULL; /* still no slave, return NULL */ - } + if (bond->primary_slave && bond->primary_slave->link == BOND_LINK_UP && + bond_should_change_active(bond)) + return bond->primary_slave; - if ((bond->primary_slave) && - bond->primary_slave->link == BOND_LINK_UP && - bond_should_change_active(bond)) { - new_active = bond->primary_slave; - } - - /* remember where to stop iterating over the slaves */ - old_active = new_active; - - bond_for_each_slave_from(bond, new_active, i, old_active) { - if (new_active->link == BOND_LINK_UP) { - return new_active; - } else if (new_active->link == BOND_LINK_BACK && - IS_UP(new_active->dev)) { - /* link up, but waiting for stabilization */ - if (new_active->delay < mintime) { - mintime = new_active->delay; - bestslave = new_active; - } + bond_for_each_slave(bond, slave, iter) { + if (slave->link == BOND_LINK_UP) + return slave; + if (slave->link == BOND_LINK_BACK && IS_UP(slave->dev) && + slave->delay < mintime) { + mintime = slave->delay; + bestslave = slave; } } -- cgit v1.2.3 From 4087df87b868cef0dd19a9409b40fb9415503552 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:19 +0200 Subject: bonding: rework bond_ab_arp_probe() to use bond_for_each_slave() Currently it uses the hard-to-rcuify bond_for_each_slave_from(), and also it doesn't check every slave for disrepencies between the actual IS_UP(slave) and the slave->link == BOND_LINK_UP, but only till we find the next suitable slave. Fix this by using bond_for_each_slave() and storing the first good slave in *before till we find the current_arp_slave, after that we store the first good slave in new_slave. If new_slave is empty - use the slave stored in before, and if it's also empty - then we didn't find any suitable slave. Also, in the meanwhile, check for each slave status. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 6abbfaca0b93..3c96b1b10ba4 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2761,8 +2761,9 @@ do_failover: */ static void bond_ab_arp_probe(struct bonding *bond) { - struct slave *slave, *next_slave; - int i; + struct slave *slave, *before = NULL, *new_slave = NULL; + struct list_head *iter; + bool found = false; read_lock(&bond->curr_slave_lock); @@ -2792,18 +2793,12 @@ static void bond_ab_arp_probe(struct bonding *bond) bond_set_slave_inactive_flags(bond->current_arp_slave); - /* search for next candidate */ - next_slave = bond_next_slave(bond, bond->current_arp_slave); - bond_for_each_slave_from(bond, slave, i, next_slave) { - if (IS_UP(slave->dev)) { - slave->link = BOND_LINK_BACK; - bond_set_slave_active_flags(slave); - bond_arp_send_all(bond, slave); - slave->jiffies = jiffies; - bond->current_arp_slave = slave; - break; - } + bond_for_each_slave(bond, slave, iter) { + if (!found && !before && IS_UP(slave->dev)) + before = slave; + if (found && !new_slave && IS_UP(slave->dev)) + new_slave = slave; /* if the link state is up at this point, we * mark it down - this can happen if we have * simultaneous link failures and @@ -2811,7 +2806,7 @@ static void bond_ab_arp_probe(struct bonding *bond) * one the current slave so it is still marked * up when it is actually down */ - if (slave->link == BOND_LINK_UP) { + if (!IS_UP(slave->dev) && slave->link == BOND_LINK_UP) { slave->link = BOND_LINK_DOWN; if (slave->link_failure_count < UINT_MAX) slave->link_failure_count++; @@ -2821,7 +2816,22 @@ static void bond_ab_arp_probe(struct bonding *bond) pr_info("%s: backup interface %s is now down.\n", bond->dev->name, slave->dev->name); } + if (slave == bond->current_arp_slave) + found = true; } + + if (!new_slave && before) + new_slave = before; + + if (!new_slave) + return; + + new_slave->link = BOND_LINK_BACK; + bond_set_slave_active_flags(new_slave); + bond_arp_send_all(bond, new_slave); + new_slave->jiffies = jiffies; + bond->current_arp_slave = new_slave; + } void bond_activebackup_arp_mon(struct work_struct *work) -- cgit v1.2.3 From b386c58b85b23c3a73baa014d01d2943ca8260cd Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:20 +0200 Subject: bonding: remove unused bond_for_each_slave_from() It has no users, so we can remove it. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bonding.h | 13 ------------- 1 file changed, 13 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 96f571dd0bb6..7c8a4b128f65 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -93,19 +93,6 @@ (bond_is_first_slave(bond, pos) ? bond_last_slave(bond) : \ bond_to_slave((pos)->list.prev)) -/** - * bond_for_each_slave_from - iterate the slaves list from a starting point - * @bond: the bond holding this list. - * @pos: current slave. - * @cnt: counter for max number of moves - * @start: starting point. - * - * Caller must hold bond->lock - */ -#define bond_for_each_slave_from(bond, pos, cnt, start) \ - for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \ - cnt++, pos = bond_next_slave(bond, pos)) - /** * bond_for_each_slave - iterate over all slaves * @bond: the bond holding this list -- cgit v1.2.3 From 0965a1f3f8757a2c20a16a83bc18279009d79a26 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:21 +0200 Subject: bonding: add bond_has_slaves() and use it Currently we verify if we have slaves by checking if bond->slave_list is empty. Create a define bond_has_slaves() and use it, a bit more readable and easier to change in the future. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 2 +- drivers/net/bonding/bond_alb.c | 8 ++++---- drivers/net/bonding/bond_main.c | 32 ++++++++++++++++---------------- drivers/net/bonding/bond_sysfs.c | 4 ++-- drivers/net/bonding/bonding.h | 2 ++ 5 files changed, 25 insertions(+), 23 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index c861ee7e65ff..1337eafe4311 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -2117,7 +2117,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work) read_lock(&bond->lock); //check if there are any slaves - if (list_empty(&bond->slave_list)) + if (!bond_has_slaves(bond)) goto re_arm; // check if agg_select_timer timer after initialize is timed out diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index c5b85ad5554d..e96041816b5b 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -1178,7 +1178,7 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav struct slave *tmp_slave1, *free_mac_slave = NULL; struct list_head *iter; - if (list_empty(&bond->slave_list)) { + if (!bond_has_slaves(bond)) { /* this is the first slave */ return 0; } @@ -1469,7 +1469,7 @@ void bond_alb_monitor(struct work_struct *work) read_lock(&bond->lock); - if (list_empty(&bond->slave_list)) { + if (!bond_has_slaves(bond)) { bond_info->tx_rebalance_counter = 0; bond_info->lp_counter = 0; goto re_arm; @@ -1606,7 +1606,7 @@ int bond_alb_init_slave(struct bonding *bond, struct slave *slave) */ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave) { - if (!list_empty(&bond->slave_list)) + if (bond_has_slaves(bond)) alb_change_hw_addr_on_detach(bond, slave); tlb_clear_slave(bond, slave, 0); @@ -1676,7 +1676,7 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave swap_slave = bond->curr_active_slave; rcu_assign_pointer(bond->curr_active_slave, new_slave); - if (!new_slave || list_empty(&bond->slave_list)) + if (!new_slave || !bond_has_slaves(bond)) return; /* set the new curr_active_slave to the bonds mac address diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 3c96b1b10ba4..06ffc8ace54c 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -391,7 +391,7 @@ static int bond_set_carrier(struct bonding *bond) struct list_head *iter; struct slave *slave; - if (list_empty(&bond->slave_list)) + if (!bond_has_slaves(bond)) goto down; if (bond->params.mode == BOND_MODE_8023AD) @@ -1085,7 +1085,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev, netdev_features_t mask; struct slave *slave; - if (list_empty(&bond->slave_list)) { + if (!bond_has_slaves(bond)) { /* Disable adding VLANs to empty bond. But why? --mq */ features |= NETIF_F_VLAN_CHALLENGED; return features; @@ -1120,7 +1120,7 @@ static void bond_compute_features(struct bonding *bond) unsigned int gso_max_size = GSO_MAX_SIZE; u16 gso_max_segs = GSO_MAX_SEGS; - if (list_empty(&bond->slave_list)) + if (!bond_has_slaves(bond)) goto done; bond_for_each_slave(bond, slave, iter) { @@ -1310,7 +1310,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) * bond ether type mutual exclusion - don't allow slaves of dissimilar * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the same bond */ - if (list_empty(&bond->slave_list)) { + if (!bond_has_slaves(bond)) { if (bond_dev->type != slave_dev->type) { pr_debug("%s: change device type from %d to %d\n", bond_dev->name, @@ -1349,7 +1349,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) } if (slave_ops->ndo_set_mac_address == NULL) { - if (list_empty(&bond->slave_list)) { + if (!bond_has_slaves(bond)) { pr_warning("%s: Warning: The first slave device specified does not support setting the MAC address. Setting fail_over_mac to active.", bond_dev->name); bond->params.fail_over_mac = BOND_FOM_ACTIVE; @@ -1365,7 +1365,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) /* If this is the first slave, then we need to set the master's hardware * address to be the same as the slave's. */ - if (list_empty(&bond->slave_list) && + if (!bond_has_slaves(bond) && bond->dev->addr_assign_type == NET_ADDR_RANDOM) bond_set_dev_addr(bond->dev, slave_dev); @@ -1696,7 +1696,7 @@ err_free: err_undo_flags: bond_compute_features(bond); /* Enslave of first slave has failed and we need to fix master's mac */ - if (list_empty(&bond->slave_list) && + if (!bond_has_slaves(bond) && ether_addr_equal(bond_dev->dev_addr, slave_dev->dev_addr)) eth_hw_addr_random(bond_dev); @@ -1776,7 +1776,7 @@ static int __bond_release_one(struct net_device *bond_dev, if (!all && !bond->params.fail_over_mac) { if (ether_addr_equal(bond_dev->dev_addr, slave->perm_hwaddr) && - !list_empty(&bond->slave_list)) + bond_has_slaves(bond)) pr_warn("%s: Warning: the permanent HWaddr of %s - %pM - is still in use by %s. Set the HWaddr of %s to a different address to avoid conflicts.\n", bond_dev->name, slave_dev->name, slave->perm_hwaddr, @@ -1819,7 +1819,7 @@ static int __bond_release_one(struct net_device *bond_dev, write_lock_bh(&bond->lock); } - if (list_empty(&bond->slave_list)) { + if (!bond_has_slaves(bond)) { bond_set_carrier(bond); eth_hw_addr_random(bond_dev); @@ -1835,7 +1835,7 @@ static int __bond_release_one(struct net_device *bond_dev, unblock_netpoll_tx(); synchronize_rcu(); - if (list_empty(&bond->slave_list)) { + if (!bond_has_slaves(bond)) { call_netdevice_notifiers(NETDEV_CHANGEADDR, bond->dev); call_netdevice_notifiers(NETDEV_RELEASE, bond->dev); } @@ -1904,7 +1904,7 @@ static int bond_release_and_destroy(struct net_device *bond_dev, int ret; ret = bond_release(bond_dev, slave_dev); - if (ret == 0 && list_empty(&bond->slave_list)) { + if (ret == 0 && !bond_has_slaves(bond)) { bond_dev->priv_flags |= IFF_DISABLE_NETPOLL; pr_info("%s: destroying bond %s.\n", bond_dev->name, bond_dev->name); @@ -2219,7 +2219,7 @@ void bond_mii_monitor(struct work_struct *work) delay = msecs_to_jiffies(bond->params.miimon); - if (list_empty(&bond->slave_list)) + if (!bond_has_slaves(bond)) goto re_arm; should_notify_peers = bond_should_notify_peers(bond); @@ -2513,7 +2513,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work) read_lock(&bond->lock); - if (list_empty(&bond->slave_list)) + if (!bond_has_slaves(bond)) goto re_arm; oldcurrent = bond->curr_active_slave; @@ -2845,7 +2845,7 @@ void bond_activebackup_arp_mon(struct work_struct *work) delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval); - if (list_empty(&bond->slave_list)) + if (!bond_has_slaves(bond)) goto re_arm; should_notify_peers = bond_should_notify_peers(bond); @@ -3169,7 +3169,7 @@ static int bond_open(struct net_device *bond_dev) /* reset slave->backup and slave->inactive */ read_lock(&bond->lock); - if (!list_empty(&bond->slave_list)) { + if (bond_has_slaves(bond)) { read_lock(&bond->curr_slave_lock); bond_for_each_slave(bond, slave, iter) { if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP) @@ -3892,7 +3892,7 @@ static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev) return NETDEV_TX_BUSY; rcu_read_lock(); - if (!list_empty(&bond->slave_list)) + if (bond_has_slaves(bond)) ret = __bond_start_xmit(skb, dev); else kfree_skb(skb); diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index e747415b5cb0..04d95d6f6c63 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -327,7 +327,7 @@ static ssize_t bonding_store_mode(struct device *d, goto out; } - if (!list_empty(&bond->slave_list)) { + if (bond_has_slaves(bond)) { pr_err("unable to update mode of %s because it has slaves.\n", bond->dev->name); ret = -EPERM; @@ -523,7 +523,7 @@ static ssize_t bonding_store_fail_over_mac(struct device *d, if (!rtnl_trylock()) return restart_syscall(); - if (!list_empty(&bond->slave_list)) { + if (bond_has_slaves(bond)) { pr_err("%s: Can't alter fail_over_mac with slaves in bond.\n", bond->dev->name); ret = -EPERM; diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 7c8a4b128f65..bcef15ec3459 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -72,6 +72,8 @@ res; }) /* slave list primitives */ +#define bond_has_slaves(bond) !list_empty(&(bond)->slave_list) + #define bond_to_slave(ptr) list_entry(ptr, struct slave, list) /* IMPORTANT: bond_first/last_slave can return NULL in case of an empty list */ -- cgit v1.2.3 From 70039aa7c6c182c488ec23a9669d9f6b21aebe16 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:22 +0200 Subject: bonding: convert bond_has_slaves() to use the neighbour list The same way as it was used for its own slave_list. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bonding.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index bcef15ec3459..170c7fce1b44 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -72,7 +72,7 @@ res; }) /* slave list primitives */ -#define bond_has_slaves(bond) !list_empty(&(bond)->slave_list) +#define bond_has_slaves(bond) !list_empty(&(bond)->dev->adj_list.lower) #define bond_to_slave(ptr) list_entry(ptr, struct slave, list) -- cgit v1.2.3 From 5a52405a30abf70a60312ad4231385699f09cb85 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:24 +0200 Subject: bonding: convert first/last slave logic to use neighbours For that, use netdev_adjacent_get_private(list_head) on bond's lower neighbour list members. Also, add a small macro - bond_slave_list(bond), which returns the bond list via neighbour list. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bonding.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 170c7fce1b44..3eb464cb8744 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -72,19 +72,24 @@ res; }) /* slave list primitives */ -#define bond_has_slaves(bond) !list_empty(&(bond)->dev->adj_list.lower) +#define bond_slave_list(bond) (&(bond)->dev->adj_list.lower) + +#define bond_has_slaves(bond) !list_empty(bond_slave_list(bond)) #define bond_to_slave(ptr) list_entry(ptr, struct slave, list) /* IMPORTANT: bond_first/last_slave can return NULL in case of an empty list */ #define bond_first_slave(bond) \ - list_first_entry_or_null(&(bond)->slave_list, struct slave, list) + (bond_has_slaves(bond) ? \ + netdev_adjacent_get_private(bond_slave_list(bond)->next) : \ + NULL) #define bond_last_slave(bond) \ - (list_empty(&(bond)->slave_list) ? NULL : \ - bond_to_slave((bond)->slave_list.prev)) + (bond_has_slaves(bond) ? \ + netdev_adjacent_get_private(bond_slave_list(bond)->prev) : \ + NULL) -#define bond_is_first_slave(bond, pos) ((pos)->list.prev == &(bond)->slave_list) -#define bond_is_last_slave(bond, pos) ((pos)->list.next == &(bond)->slave_list) +#define bond_is_first_slave(bond, pos) (pos == bond_first_slave(bond)) +#define bond_is_last_slave(bond, pos) (pos == bond_last_slave(bond)) /* Since bond_first/last_slave can return NULL, these can return NULL too */ #define bond_next_slave(bond, pos) \ -- cgit v1.2.3 From c8c23903f12a62708606b5cdba8cd8550cd6bdcd Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:25 +0200 Subject: bonding: remove bond_prev_slave() We don't really need it, and it's really hard to RCUify the list->prev. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 9 +++------ drivers/net/bonding/bonding.h | 4 ---- 2 files changed, 3 insertions(+), 10 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 06ffc8ace54c..6aa345a6a0bc 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1255,7 +1255,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) { struct bonding *bond = netdev_priv(bond_dev); const struct net_device_ops *slave_ops = slave_dev->netdev_ops; - struct slave *new_slave = NULL; + struct slave *new_slave = NULL, *prev_slave; struct sockaddr addr; int link_reporting; int res = 0, i; @@ -1472,6 +1472,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) write_lock_bh(&bond->lock); + prev_slave = bond_last_slave(bond); bond_attach_slave(bond, new_slave); new_slave->delay = 0; @@ -1566,9 +1567,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) */ bond_3ad_initialize(bond, 1000/AD_TIMER_INTERVAL); } else { - struct slave *prev_slave; - - prev_slave = bond_prev_slave(bond, new_slave); SLAVE_AD_INFO(new_slave).id = SLAVE_AD_INFO(prev_slave).id + 1; } @@ -3506,9 +3504,8 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu) */ bond_for_each_slave(bond, slave, iter) { - pr_debug("s %p s->p %p c_m %p\n", + pr_debug("s %p c_m %p\n", slave, - bond_prev_slave(bond, slave), slave->dev->netdev_ops->ndo_change_mtu); res = dev_set_mtu(slave->dev, new_mtu); diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 3eb464cb8744..454d6affa06a 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -96,10 +96,6 @@ (bond_is_last_slave(bond, pos) ? bond_first_slave(bond) : \ bond_to_slave((pos)->list.next)) -#define bond_prev_slave(bond, pos) \ - (bond_is_first_slave(bond, pos) ? bond_last_slave(bond) : \ - bond_to_slave((pos)->list.prev)) - /** * bond_for_each_slave - iterate over all slaves * @bond: the bond holding this list -- cgit v1.2.3 From 18e1e9bc5d1b9b89853a23aaeeed39686a95551b Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:26 +0200 Subject: bonding: add __bond_next_slave() which uses neighbours Add a new function, __bond_next_slave(), which uses neighbours to find the next slave after the slave provided. It will be further used to gradually go start using neighbour netdev_adjacent infrastructure instead of bonding's own lists. CC: Jay Vosburgh CC: Andy Gospodarek CC: Ben Hutchings Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bonding.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 454d6affa06a..4a3fbe307ee8 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -249,6 +249,34 @@ struct bonding { #define bond_slave_get_rtnl(dev) \ ((struct slave *) rtnl_dereference(dev->rx_handler_data)) +/** + * __bond_next_slave - get the next slave after the one provided + * @bond - bonding struct + * @slave - the slave provided + * + * Returns the next slave after the slave provided, first slave if the + * slave provided is the last slave and NULL if slave is not found + */ +static inline struct slave *__bond_next_slave(struct bonding *bond, + struct slave *slave) +{ + struct slave *slave_iter; + struct list_head *iter; + bool found = false; + + netdev_for_each_lower_private(bond->dev, slave_iter, iter) { + if (found) + return slave_iter; + if (slave_iter == slave) + found = true; + } + + if (found) + return bond_first_slave(bond); + + return NULL; +} + /** * Returns NULL if the net_device does not belong to any of the bond's slaves * -- cgit v1.2.3 From 344f3297629e409675120998d1cfe4d21c34fbaa Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:27 +0200 Subject: bonding: use neighbours for bond_next_slave() Use the new function __bond_next_slave(). CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bonding.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 4a3fbe307ee8..f070557c21ea 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -76,8 +76,6 @@ #define bond_has_slaves(bond) !list_empty(bond_slave_list(bond)) -#define bond_to_slave(ptr) list_entry(ptr, struct slave, list) - /* IMPORTANT: bond_first/last_slave can return NULL in case of an empty list */ #define bond_first_slave(bond) \ (bond_has_slaves(bond) ? \ @@ -92,9 +90,7 @@ #define bond_is_last_slave(bond, pos) (pos == bond_last_slave(bond)) /* Since bond_first/last_slave can return NULL, these can return NULL too */ -#define bond_next_slave(bond, pos) \ - (bond_is_last_slave(bond, pos) ? bond_first_slave(bond) : \ - bond_to_slave((pos)->list.next)) +#define bond_next_slave(bond, pos) __bond_next_slave(bond, pos) /** * bond_for_each_slave - iterate over all slaves -- cgit v1.2.3 From 4fee991a4689ca64abea90814e0b3c6d30a3c62f Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:28 +0200 Subject: bonding: remove slave lists And all the initialization. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 4 ---- drivers/net/bonding/bonding.h | 2 -- 2 files changed, 6 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 6aa345a6a0bc..d49404509814 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -972,7 +972,6 @@ void bond_select_active_slave(struct bonding *bond) */ static void bond_attach_slave(struct bonding *bond, struct slave *new_slave) { - list_add_tail_rcu(&new_slave->list, &bond->slave_list); bond->slave_cnt++; } @@ -988,7 +987,6 @@ static void bond_attach_slave(struct bonding *bond, struct slave *new_slave) */ static void bond_detach_slave(struct bonding *bond, struct slave *slave) { - list_del_rcu(&slave->list); bond->slave_cnt--; } @@ -1374,7 +1372,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) res = -ENOMEM; goto err_undo_flags; } - INIT_LIST_HEAD(&new_slave->list); /* * Set the new_slave's queue_id to be zero. Queue ID mapping * is set via sysfs or module option if desired. @@ -4022,7 +4019,6 @@ static void bond_setup(struct net_device *bond_dev) /* initialize rwlocks */ rwlock_init(&bond->lock); rwlock_init(&bond->curr_slave_lock); - INIT_LIST_HEAD(&bond->slave_list); bond->params = bonding_defaults; /* Initialize pointers */ diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index f070557c21ea..e5c32a61dacc 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -165,7 +165,6 @@ struct bond_parm_tbl { struct slave { struct net_device *dev; /* first - useful for panic debug */ - struct list_head list; struct bonding *bond; /* our master */ int delay; unsigned long jiffies; @@ -205,7 +204,6 @@ struct slave { */ struct bonding { struct net_device *dev; /* first - useful for panic debug */ - struct list_head slave_list; struct slave *curr_active_slave; struct slave *current_arp_slave; struct slave *primary_slave; -- cgit v1.2.3 From 842d67a7b34ea735155812ecf0671a481284f358 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:31 +0200 Subject: net: expose the master link to sysfs, and remove it from bond Currently, we can have only one master upper neighbour, so it would be useful to create a symlink to it in the sysfs device directory, the way that bonding now does it, for every device. Lower devices from bridge/team/etc will automagically get it, so we could rely on it. Also, remove the same functionality from bonding. CC: Jay Vosburgh CC: Andy Gospodarek CC: "David S. Miller" CC: Eric Dumazet CC: Jiri Pirko CC: Alexander Duyck Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_sysfs.c | 20 +++----------------- net/core/dev.c | 19 +++++++++++++++++-- 2 files changed, 20 insertions(+), 19 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index 04d95d6f6c63..1c5724672204 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -172,24 +172,11 @@ int bond_create_slave_symlinks(struct net_device *master, struct net_device *slave) { char linkname[IFNAMSIZ+7]; - int ret = 0; - /* first, create a link from the slave back to the master */ - ret = sysfs_create_link(&(slave->dev.kobj), &(master->dev.kobj), - "master"); - if (ret) - return ret; - /* next, create a link from the master to the slave */ + /* create a link from the master to the slave */ sprintf(linkname, "slave_%s", slave->name); - ret = sysfs_create_link(&(master->dev.kobj), &(slave->dev.kobj), - linkname); - - /* free the master link created earlier in case of error */ - if (ret) - sysfs_remove_link(&(slave->dev.kobj), "master"); - - return ret; - + return sysfs_create_link(&(master->dev.kobj), &(slave->dev.kobj), + linkname); } void bond_destroy_slave_symlinks(struct net_device *master, @@ -197,7 +184,6 @@ void bond_destroy_slave_symlinks(struct net_device *master, { char linkname[IFNAMSIZ+7]; - sysfs_remove_link(&(slave->dev.kobj), "master"); sprintf(linkname, "slave_%s", slave->name); sysfs_remove_link(&(master->dev.kobj), linkname); } diff --git a/net/core/dev.c b/net/core/dev.c index acc11810805f..de443ee1b046 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4584,6 +4584,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, void *private, bool master) { struct netdev_adjacent *adj; + int ret; adj = __netdev_find_adj(dev, adj_dev, dev_list); @@ -4606,12 +4607,23 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, adj_dev->name, dev->name, adj_dev->name); /* Ensure that master link is always the first item in list. */ - if (master) + if (master) { + ret = sysfs_create_link(&(dev->dev.kobj), + &(adj_dev->dev.kobj), "master"); + if (ret) + goto free_adj; + list_add_rcu(&adj->list, dev_list); - else + } else { list_add_tail_rcu(&adj->list, dev_list); + } return 0; + +free_adj: + kfree(adj); + + return ret; } void __netdev_adjacent_dev_remove(struct net_device *dev, @@ -4635,6 +4647,9 @@ void __netdev_adjacent_dev_remove(struct net_device *dev, return; } + if (adj->master) + sysfs_remove_link(&(dev->dev.kobj), "master"); + list_del_rcu(&adj->list); pr_debug("dev_put for %s, because link removed from %s to %s\n", adj_dev->name, dev->name, adj_dev->name); -- cgit v1.2.3 From 5831d66e8097aedfa3bc35941cf265ada2352317 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Wed, 25 Sep 2013 09:20:32 +0200 Subject: net: create sysfs symlinks for neighbour devices Also, remove the same functionality from bonding - it will be already done for any device that links to its lower/upper neighbour. The links will be created for dev's kobject, and will look like lower_eth0 for lower device eth0 and upper_bridge0 for upper device bridge0. CC: Jay Vosburgh CC: Andy Gospodarek CC: "David S. Miller" CC: Eric Dumazet CC: Jiri Pirko CC: Alexander Duyck Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 11 +---------- drivers/net/bonding/bond_sysfs.c | 21 --------------------- drivers/net/bonding/bonding.h | 2 -- net/core/dev.c | 35 ++++++++++++++++++++++++++++++++++- 4 files changed, 35 insertions(+), 34 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index d49404509814..d5c3153226b7 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1612,15 +1612,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) read_unlock(&bond->lock); - res = bond_create_slave_symlinks(bond_dev, slave_dev); - if (res) - goto err_detach; - res = netdev_rx_handler_register(slave_dev, bond_handle_frame, new_slave); if (res) { pr_debug("Error %d calling netdev_rx_handler_register\n", res); - goto err_dest_symlinks; + goto err_detach; } res = bond_master_upper_dev_link(bond_dev, slave_dev, new_slave); @@ -1642,9 +1638,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) err_unregister: netdev_rx_handler_unregister(slave_dev); -err_dest_symlinks: - bond_destroy_slave_symlinks(bond_dev, slave_dev); - err_detach: if (!USES_PRIMARY(bond->params.mode)) bond_hw_addr_flush(bond_dev, slave_dev); @@ -1842,8 +1835,6 @@ static int __bond_release_one(struct net_device *bond_dev, bond_dev->name, slave_dev->name, bond_dev->name); /* must do this from outside any spinlocks */ - bond_destroy_slave_symlinks(bond_dev, slave_dev); - vlan_vids_del_by_dev(slave_dev, bond_dev); /* If the mode USES_PRIMARY, then this cases was handled above by diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index 1c5724672204..e06c644470b1 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -168,27 +168,6 @@ static const struct class_attribute class_attr_bonding_masters = { .namespace = bonding_namespace, }; -int bond_create_slave_symlinks(struct net_device *master, - struct net_device *slave) -{ - char linkname[IFNAMSIZ+7]; - - /* create a link from the master to the slave */ - sprintf(linkname, "slave_%s", slave->name); - return sysfs_create_link(&(master->dev.kobj), &(slave->dev.kobj), - linkname); -} - -void bond_destroy_slave_symlinks(struct net_device *master, - struct net_device *slave) -{ - char linkname[IFNAMSIZ+7]; - - sprintf(linkname, "slave_%s", slave->name); - sysfs_remove_link(&(master->dev.kobj), linkname); -} - - /* * Show the slaves in the current bond. */ diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index e5c32a61dacc..5b71601666cd 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -436,8 +436,6 @@ int bond_create(struct net *net, const char *name); int bond_create_sysfs(struct bond_net *net); void bond_destroy_sysfs(struct bond_net *net); void bond_prepare_sysfs_group(struct bonding *bond); -int bond_create_slave_symlinks(struct net_device *master, struct net_device *slave); -void bond_destroy_slave_symlinks(struct net_device *master, struct net_device *slave); int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev); int bond_release(struct net_device *bond_dev, struct net_device *slave_dev); void bond_mii_monitor(struct work_struct *); diff --git a/net/core/dev.c b/net/core/dev.c index de443ee1b046..25ab6fe80da2 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4584,6 +4584,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, void *private, bool master) { struct netdev_adjacent *adj; + char linkname[IFNAMSIZ+7]; int ret; adj = __netdev_find_adj(dev, adj_dev, dev_list); @@ -4606,12 +4607,26 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, pr_debug("dev_hold for %s, because of link added from %s to %s\n", adj_dev->name, dev->name, adj_dev->name); + if (dev_list == &dev->adj_list.lower) { + sprintf(linkname, "lower_%s", adj_dev->name); + ret = sysfs_create_link(&(dev->dev.kobj), + &(adj_dev->dev.kobj), linkname); + if (ret) + goto free_adj; + } else if (dev_list == &dev->adj_list.upper) { + sprintf(linkname, "upper_%s", adj_dev->name); + ret = sysfs_create_link(&(dev->dev.kobj), + &(adj_dev->dev.kobj), linkname); + if (ret) + goto free_adj; + } + /* Ensure that master link is always the first item in list. */ if (master) { ret = sysfs_create_link(&(dev->dev.kobj), &(adj_dev->dev.kobj), "master"); if (ret) - goto free_adj; + goto remove_symlinks; list_add_rcu(&adj->list, dev_list); } else { @@ -4620,6 +4635,15 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev, return 0; +remove_symlinks: + if (dev_list == &dev->adj_list.lower) { + sprintf(linkname, "lower_%s", adj_dev->name); + sysfs_remove_link(&(dev->dev.kobj), linkname); + } else if (dev_list == &dev->adj_list.upper) { + sprintf(linkname, "upper_%s", adj_dev->name); + sysfs_remove_link(&(dev->dev.kobj), linkname); + } + free_adj: kfree(adj); @@ -4631,6 +4655,7 @@ void __netdev_adjacent_dev_remove(struct net_device *dev, struct list_head *dev_list) { struct netdev_adjacent *adj; + char linkname[IFNAMSIZ+7]; adj = __netdev_find_adj(dev, adj_dev, dev_list); @@ -4650,6 +4675,14 @@ void __netdev_adjacent_dev_remove(struct net_device *dev, if (adj->master) sysfs_remove_link(&(dev->dev.kobj), "master"); + if (dev_list == &dev->adj_list.lower) { + sprintf(linkname, "lower_%s", adj_dev->name); + sysfs_remove_link(&(dev->dev.kobj), linkname); + } else if (dev_list == &dev->adj_list.upper) { + sprintf(linkname, "upper_%s", adj_dev->name); + sysfs_remove_link(&(dev->dev.kobj), linkname); + } + list_del_rcu(&adj->list); pr_debug("dev_put for %s, because link removed from %s to %s\n", adj_dev->name, dev->name, adj_dev->name); -- cgit v1.2.3 From 58292cbe6669d74498a5f08db13e57cb3bcfb81d Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Wed, 11 Sep 2013 22:29:04 -0400 Subject: sysfs: make attr namespace interface less convoluted sysfs ns (namespace) implementation became more convoluted than necessary while trying to hide ns information from visible interface. The relatively recent attr ns support is a good example. * attr ns tag is determined by sysfs_ops->namespace() callback while dir tag is determined by kobj_type->namespace(). The placement is arbitrary. * Instead of performing operations with explicit ns tag, the namespace callback is routed through sysfs_attr_ns(), sysfs_ops->namespace(), class_attr_namespace(), class_attr->namespace(). It's not simpler in any sense. The only thing this convolution does is traversing the whole stack backwards. The namespace callbacks are unncessary because the operations involved are inherently synchronous. The information can be provided in in straight-forward top-down direction and reversing that direction is unnecessary and against basic design principles. This backward interface is unnecessarily convoluted and hinders properly separating out sysfs from driver model / kobject for proper layering. This patch updates attr ns support such that * sysfs_ops->namespace() and class_attr->namespace() are dropped. * sysfs_{create|remove}_file_ns(), which take explicit @ns param, are added and sysfs_{create|remove}_file() are now simple wrappers around the ns aware functions. * ns handling is dropped from sysfs_chmod_file(). Nobody uses it at this point. sysfs_chmod_file_ns() can be added later if necessary. * Explicit @ns is propagated through class_{create|remove}_file_ns() and netdev_class_{create|remove}_file_ns(). * driver/net/bonding which is currently the only user of attr namespace is updated to use netdev_class_{create|remove}_file_ns() with @bh->net as the ns tag instead of using the namespace callback. This patch should be an equivalent conversion without any functional difference. It makes the code easier to follow, reduces lines of code a bit and helps proper separation and layering. Signed-off-by: Tejun Heo Cc: Eric W. Biederman Cc: Kay Sievers Acked-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- drivers/base/class.c | 29 ++++-------- drivers/net/bonding/bond_sysfs.c | 14 ++---- fs/sysfs/file.c | 95 ++++++++++------------------------------ fs/sysfs/group.c | 7 +-- fs/sysfs/sysfs.h | 5 ++- include/linux/device.h | 24 +++++++--- include/linux/netdevice.h | 16 ++++++- include/linux/sysfs.h | 31 +++++++++---- net/core/net-sysfs.c | 14 +++--- 9 files changed, 106 insertions(+), 129 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/base/class.c b/drivers/base/class.c index 8b7818b80056..f96f70419a78 100644 --- a/drivers/base/class.c +++ b/drivers/base/class.c @@ -47,18 +47,6 @@ static ssize_t class_attr_store(struct kobject *kobj, struct attribute *attr, return ret; } -static const void *class_attr_namespace(struct kobject *kobj, - const struct attribute *attr) -{ - struct class_attribute *class_attr = to_class_attr(attr); - struct subsys_private *cp = to_subsys_private(kobj); - const void *ns = NULL; - - if (class_attr->namespace) - ns = class_attr->namespace(cp->class, class_attr); - return ns; -} - static void class_release(struct kobject *kobj) { struct subsys_private *cp = to_subsys_private(kobj); @@ -86,7 +74,6 @@ static const struct kobj_ns_type_operations *class_child_ns_type(struct kobject static const struct sysfs_ops class_sysfs_ops = { .show = class_attr_show, .store = class_attr_store, - .namespace = class_attr_namespace, }; static struct kobj_type class_ktype = { @@ -99,21 +86,23 @@ static struct kobj_type class_ktype = { static struct kset *class_kset; -int class_create_file(struct class *cls, const struct class_attribute *attr) +int class_create_file_ns(struct class *cls, const struct class_attribute *attr, + const void *ns) { int error; if (cls) - error = sysfs_create_file(&cls->p->subsys.kobj, - &attr->attr); + error = sysfs_create_file_ns(&cls->p->subsys.kobj, + &attr->attr, ns); else error = -EINVAL; return error; } -void class_remove_file(struct class *cls, const struct class_attribute *attr) +void class_remove_file_ns(struct class *cls, const struct class_attribute *attr, + const void *ns) { if (cls) - sysfs_remove_file(&cls->p->subsys.kobj, &attr->attr); + sysfs_remove_file_ns(&cls->p->subsys.kobj, &attr->attr, ns); } static struct class *class_get(struct class *cls) @@ -600,8 +589,8 @@ int __init classes_init(void) return 0; } -EXPORT_SYMBOL_GPL(class_create_file); -EXPORT_SYMBOL_GPL(class_remove_file); +EXPORT_SYMBOL_GPL(class_create_file_ns); +EXPORT_SYMBOL_GPL(class_remove_file_ns); EXPORT_SYMBOL_GPL(class_unregister); EXPORT_SYMBOL_GPL(class_destroy); diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index c29b836749b6..ec9b6460a38d 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -149,14 +149,6 @@ err_no_cmd: return -EPERM; } -static const void *bonding_namespace(struct class *cls, - const struct class_attribute *attr) -{ - const struct bond_net *bn = - container_of(attr, struct bond_net, class_attr_bonding_masters); - return bn->net; -} - /* class attribute for bond_masters file. This ends up in /sys/class/net */ static const struct class_attribute class_attr_bonding_masters = { .attr = { @@ -165,7 +157,6 @@ static const struct class_attribute class_attr_bonding_masters = { }, .show = bonding_show_bonds, .store = bonding_store_bonds, - .namespace = bonding_namespace, }; int bond_create_slave_symlinks(struct net_device *master, @@ -1787,7 +1778,8 @@ int bond_create_sysfs(struct bond_net *bn) bn->class_attr_bonding_masters = class_attr_bonding_masters; sysfs_attr_init(&bn->class_attr_bonding_masters.attr); - ret = netdev_class_create_file(&bn->class_attr_bonding_masters); + ret = netdev_class_create_file_ns(&bn->class_attr_bonding_masters, + bn->net); /* * Permit multiple loads of the module by ignoring failures to * create the bonding_masters sysfs file. Bonding devices @@ -1817,7 +1809,7 @@ int bond_create_sysfs(struct bond_net *bn) */ void bond_destroy_sysfs(struct bond_net *bn) { - netdev_class_remove_file(&bn->class_attr_bonding_masters); + netdev_class_remove_file_ns(&bn->class_attr_bonding_masters, bn->net); } /* diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 15ef5eb13663..e784340f1599 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -485,58 +485,15 @@ const struct file_operations sysfs_file_operations = { .poll = sysfs_poll, }; -static int sysfs_attr_ns(struct kobject *kobj, const struct attribute *attr, - const void **pns) -{ - struct sysfs_dirent *dir_sd = kobj->sd; - const struct sysfs_ops *ops; - const void *ns = NULL; - int err; - - if (!dir_sd) { - WARN(1, KERN_ERR "sysfs: kobject %s without dirent\n", - kobject_name(kobj)); - return -ENOENT; - } - - err = 0; - if (!sysfs_ns_type(dir_sd)) - goto out; - - err = -EINVAL; - if (!kobj->ktype) - goto out; - ops = kobj->ktype->sysfs_ops; - if (!ops) - goto out; - if (!ops->namespace) - goto out; - - err = 0; - ns = ops->namespace(kobj, attr); -out: - if (err) { - WARN(1, KERN_ERR - "missing sysfs namespace attribute operation for kobject: %s\n", - kobject_name(kobj)); - } - *pns = ns; - return err; -} - -int sysfs_add_file_mode(struct sysfs_dirent *dir_sd, - const struct attribute *attr, int type, umode_t amode) +int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd, + const struct attribute *attr, int type, + umode_t amode, const void *ns) { umode_t mode = (amode & S_IALLUGO) | S_IFREG; struct sysfs_addrm_cxt acxt; struct sysfs_dirent *sd; - const void *ns; int rc; - rc = sysfs_attr_ns(dir_sd->s_dir.kobj, attr, &ns); - if (rc) - return rc; - sd = sysfs_new_dirent(attr->name, mode, type); if (!sd) return -ENOMEM; @@ -559,23 +516,25 @@ int sysfs_add_file_mode(struct sysfs_dirent *dir_sd, int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr, int type) { - return sysfs_add_file_mode(dir_sd, attr, type, attr->mode); + return sysfs_add_file_mode_ns(dir_sd, attr, type, attr->mode, NULL); } - /** - * sysfs_create_file - create an attribute file for an object. - * @kobj: object we're creating for. - * @attr: attribute descriptor. + * sysfs_create_file_ns - create an attribute file for an object with custom ns + * @kobj: object we're creating for + * @attr: attribute descriptor + * @ns: namespace the new file should belong to */ -int sysfs_create_file(struct kobject *kobj, const struct attribute *attr) +int sysfs_create_file_ns(struct kobject *kobj, const struct attribute *attr, + const void *ns) { BUG_ON(!kobj || !kobj->sd || !attr); - return sysfs_add_file(kobj->sd, attr, SYSFS_KOBJ_ATTR); + return sysfs_add_file_mode_ns(kobj->sd, attr, SYSFS_KOBJ_ATTR, + attr->mode, ns); } -EXPORT_SYMBOL_GPL(sysfs_create_file); +EXPORT_SYMBOL_GPL(sysfs_create_file_ns); int sysfs_create_files(struct kobject *kobj, const struct attribute **ptr) { @@ -630,17 +589,12 @@ int sysfs_chmod_file(struct kobject *kobj, const struct attribute *attr, { struct sysfs_dirent *sd; struct iattr newattrs; - const void *ns; int rc; - rc = sysfs_attr_ns(kobj, attr, &ns); - if (rc) - return rc; - mutex_lock(&sysfs_mutex); rc = -ENOENT; - sd = sysfs_find_dirent(kobj->sd, ns, attr->name); + sd = sysfs_find_dirent(kobj->sd, NULL, attr->name); if (!sd) goto out; @@ -655,22 +609,21 @@ int sysfs_chmod_file(struct kobject *kobj, const struct attribute *attr, EXPORT_SYMBOL_GPL(sysfs_chmod_file); /** - * sysfs_remove_file - remove an object attribute. - * @kobj: object we're acting for. - * @attr: attribute descriptor. + * sysfs_remove_file_ns - remove an object attribute with a custom ns tag + * @kobj: object we're acting for + * @attr: attribute descriptor + * @ns: namespace tag of the file to remove * - * Hash the attribute name and kill the victim. + * Hash the attribute name and namespace tag and kill the victim. */ -void sysfs_remove_file(struct kobject *kobj, const struct attribute *attr) +void sysfs_remove_file_ns(struct kobject *kobj, const struct attribute *attr, + const void *ns) { - const void *ns; - - if (sysfs_attr_ns(kobj, attr, &ns)) - return; + struct sysfs_dirent *dir_sd = kobj->sd; - sysfs_hash_and_remove(kobj->sd, ns, attr->name); + sysfs_hash_and_remove(dir_sd, ns, attr->name); } -EXPORT_SYMBOL_GPL(sysfs_remove_file); +EXPORT_SYMBOL_GPL(sysfs_remove_file_ns); void sysfs_remove_files(struct kobject *kobj, const struct attribute **ptr) { diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c index 5f92cd2f61c1..25c78f23dae8 100644 --- a/fs/sysfs/group.c +++ b/fs/sysfs/group.c @@ -56,9 +56,10 @@ static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj, if (!mode) continue; } - error = sysfs_add_file_mode(dir_sd, *attr, - SYSFS_KOBJ_ATTR, - (*attr)->mode | mode); + error = sysfs_add_file_mode_ns(dir_sd, *attr, + SYSFS_KOBJ_ATTR, + (*attr)->mode | mode, + NULL); if (unlikely(error)) break; } diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index b6deca3e301d..a96da2559db2 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -230,8 +230,9 @@ extern const struct file_operations sysfs_file_operations; int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr, int type); -int sysfs_add_file_mode(struct sysfs_dirent *dir_sd, - const struct attribute *attr, int type, umode_t amode); +int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd, + const struct attribute *attr, int type, + umode_t amode, const void *ns); /* * bin.c */ diff --git a/include/linux/device.h b/include/linux/device.h index 2a9d6ed59579..ce690ea34547 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -427,8 +427,6 @@ struct class_attribute { char *buf); ssize_t (*store)(struct class *class, struct class_attribute *attr, const char *buf, size_t count); - const void *(*namespace)(struct class *class, - const struct class_attribute *attr); }; #define CLASS_ATTR(_name, _mode, _show, _store) \ @@ -438,10 +436,24 @@ struct class_attribute { #define CLASS_ATTR_RO(_name) \ struct class_attribute class_attr_##_name = __ATTR_RO(_name) -extern int __must_check class_create_file(struct class *class, - const struct class_attribute *attr); -extern void class_remove_file(struct class *class, - const struct class_attribute *attr); +extern int __must_check class_create_file_ns(struct class *class, + const struct class_attribute *attr, + const void *ns); +extern void class_remove_file_ns(struct class *class, + const struct class_attribute *attr, + const void *ns); + +static inline int __must_check class_create_file(struct class *class, + const struct class_attribute *attr) +{ + return class_create_file_ns(class, attr, NULL); +} + +static inline void class_remove_file(struct class *class, + const struct class_attribute *attr) +{ + return class_remove_file_ns(class, attr, NULL); +} /* Simple class attribute that is just a static string */ struct class_attribute_string { diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 3de49aca4519..42421ed49a47 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2873,8 +2873,20 @@ extern int __init dev_proc_init(void); #define dev_proc_init() 0 #endif -extern int netdev_class_create_file(struct class_attribute *class_attr); -extern void netdev_class_remove_file(struct class_attribute *class_attr); +extern int netdev_class_create_file_ns(struct class_attribute *class_attr, + const void *ns); +extern void netdev_class_remove_file_ns(struct class_attribute *class_attr, + const void *ns); + +static inline int netdev_class_create_file(struct class_attribute *class_attr) +{ + return netdev_class_create_file_ns(class_attr, NULL); +} + +static inline void netdev_class_remove_file(struct class_attribute *class_attr) +{ + netdev_class_remove_file_ns(class_attr, NULL); +} extern struct kobj_ns_type_operations net_ns_type_operations; diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 11baec7c9b26..82f7fac78e77 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -173,7 +173,6 @@ struct bin_attribute bin_attr_##_name = __BIN_ATTR_RW(_name, _size) struct sysfs_ops { ssize_t (*show)(struct kobject *, struct attribute *, char *); ssize_t (*store)(struct kobject *, struct attribute *, const char *, size_t); - const void *(*namespace)(struct kobject *, const struct attribute *); }; struct sysfs_dirent; @@ -189,13 +188,15 @@ int __must_check sysfs_rename_dir(struct kobject *kobj, const char *new_name); int __must_check sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj); -int __must_check sysfs_create_file(struct kobject *kobj, - const struct attribute *attr); +int __must_check sysfs_create_file_ns(struct kobject *kobj, + const struct attribute *attr, + const void *ns); int __must_check sysfs_create_files(struct kobject *kobj, const struct attribute **attr); int __must_check sysfs_chmod_file(struct kobject *kobj, const struct attribute *attr, umode_t mode); -void sysfs_remove_file(struct kobject *kobj, const struct attribute *attr); +void sysfs_remove_file_ns(struct kobject *kobj, const struct attribute *attr, + const void *ns); void sysfs_remove_files(struct kobject *kobj, const struct attribute **attr); int __must_check sysfs_create_bin_file(struct kobject *kobj, @@ -277,8 +278,9 @@ static inline int sysfs_move_dir(struct kobject *kobj, return 0; } -static inline int sysfs_create_file(struct kobject *kobj, - const struct attribute *attr) +static inline int sysfs_create_file_ns(struct kobject *kobj, + const struct attribute *attr, + const void *ns) { return 0; } @@ -295,8 +297,9 @@ static inline int sysfs_chmod_file(struct kobject *kobj, return 0; } -static inline void sysfs_remove_file(struct kobject *kobj, - const struct attribute *attr) +static inline void sysfs_remove_file_ns(struct kobject *kobj, + const struct attribute *attr, + const void *ns) { } @@ -435,4 +438,16 @@ static inline int __must_check sysfs_init(void) #endif /* CONFIG_SYSFS */ +static inline int __must_check sysfs_create_file(struct kobject *kobj, + const struct attribute *attr) +{ + return sysfs_create_file_ns(kobj, attr, NULL); +} + +static inline void sysfs_remove_file(struct kobject *kobj, + const struct attribute *attr) +{ + return sysfs_remove_file_ns(kobj, attr, NULL); +} + #endif /* _SYSFS_H_ */ diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index d954b56b4e47..325dee863e46 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -1344,17 +1344,19 @@ int netdev_register_kobject(struct net_device *net) return error; } -int netdev_class_create_file(struct class_attribute *class_attr) +int netdev_class_create_file_ns(struct class_attribute *class_attr, + const void *ns) { - return class_create_file(&net_class, class_attr); + return class_create_file_ns(&net_class, class_attr, ns); } -EXPORT_SYMBOL(netdev_class_create_file); +EXPORT_SYMBOL(netdev_class_create_file_ns); -void netdev_class_remove_file(struct class_attribute *class_attr) +void netdev_class_remove_file_ns(struct class_attribute *class_attr, + const void *ns) { - class_remove_file(&net_class, class_attr); + class_remove_file_ns(&net_class, class_attr, ns); } -EXPORT_SYMBOL(netdev_class_remove_file); +EXPORT_SYMBOL(netdev_class_remove_file_ns); int netdev_kobject_init(void) { -- cgit v1.2.3 From 23c147e026bbb41dd26a2bda0404a95ea951072f Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 15:10:57 +0200 Subject: bonding: correctly verify for the first slave in bond_enslave After commit 1f718f0f4f97145f4072d2d72dcf85069ca7226d ("bonding: populate neighbour's private on enslave"), we've moved the actual 'linking' in the end of the function - so that, once linked, the slave is ready to be used, and is not still in the process of enslaving. However, 802.3ad verified if it's the first slave by looking at the if (bond_first_slave(bond) == new_slave) which, because we've moved the linking to the end, became broken - on the first slave bond_first_slave(bond) returns NULL. Fix this by verifying if the prev_slave, that equals bond_last_slave(), is actually populated - if it is - then it's not the first slave, and vice versa. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index d5c3153226b7..0367f8095390 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1557,7 +1557,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) */ bond_set_slave_inactive_flags(new_slave); /* if this is the first slave */ - if (bond_first_slave(bond) == new_slave) { + if (!prev_slave) { SLAVE_AD_INFO(new_slave).id = 1; /* Initialize AD with the number of times that the AD timer is called in 1 second * can be called only after the mac address of the bond is set -- cgit v1.2.3 From 746844931ed400eef32edaa069b996eb622bc39a Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 15:10:58 +0200 Subject: bonding: verify if we still have slaves in bond_3ad_unbind_slave() After commit 1f718f0f4f97145f4072d2d72dcf85069ca7226d ("bonding: populate neighbour's private on enslave"), we've moved the unlinking of the slave to the earliest position possible - so that nobody will see an half-uninited slave. However, bond_3ad_unbind_slave() relied that, even while removing the last slave, it is still accessible - via __get_first_agg() (and, eventually, bond_first_slave()). Fix that by verifying if the aggregator return is an actual aggregator, but not NULL. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 1337eafe4311..2715ea87d64c 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -2056,7 +2056,9 @@ void bond_3ad_unbind_slave(struct slave *slave) pr_info("%s: Removing an active aggregator\n", slave->bond->dev->name); // select new active aggregator - ad_agg_selection_logic(__get_first_agg(port)); + temp_aggregator = __get_first_agg(port); + if (temp_aggregator) + ad_agg_selection_logic(temp_aggregator); } } } -- cgit v1.2.3 From 3c4c88a138f0857b9e77266e09ad147d17629401 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 16:11:57 +0200 Subject: bonding: remove __get_next_port() Currently this function is only used in constructs like for (port = __get_first_port(bond); port; port = __get_next_port(port)) which is basicly the same as bond_for_each_slave(bond, slave, iter) { port = &(SLAVE_AD_INFO(slave).port); but a more time consuming. Remove the function and convert the users to bond_for_each_slave(). CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 2715ea87d64c..c1535f8762d1 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -148,28 +148,6 @@ static inline struct port *__get_first_port(struct bonding *bond) return first_slave ? &(SLAVE_AD_INFO(first_slave).port) : NULL; } -/** - * __get_next_port - get the next port in the bond - * @port: the port we're looking at - * - * Return the port of the slave that is next in line of @port's slave in the - * bond, or %NULL if it can't be found. - */ -static inline struct port *__get_next_port(struct port *port) -{ - struct bonding *bond = __get_bond_by_port(port); - struct slave *slave = port->slave, *slave_next; - - // If there's no bond for this port, or this is the last slave - if (bond == NULL) - return NULL; - slave_next = bond_next_slave(bond, slave); - if (!slave_next || bond_is_first_slave(bond, slave_next)) - return NULL; - - return &(SLAVE_AD_INFO(slave_next).port); -} - /** * __get_first_agg - get the first aggregator in the bond * @bond: the bond we're looking at @@ -2113,8 +2091,10 @@ void bond_3ad_state_machine_handler(struct work_struct *work) { struct bonding *bond = container_of(work, struct bonding, ad_work.work); - struct port *port; struct aggregator *aggregator; + struct list_head *iter; + struct slave *slave; + struct port *port; read_lock(&bond->lock); @@ -2139,7 +2119,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work) } // for each port run the state machines - for (port = __get_first_port(bond); port; port = __get_next_port(port)) { + bond_for_each_slave(bond, slave, iter) { + port = &(SLAVE_AD_INFO(slave).port); if (!port->slave) { pr_warning("%s: Warning: Found an uninitialized port\n", bond->dev->name); @@ -2384,9 +2365,12 @@ int __bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info) { struct aggregator *aggregator = NULL; + struct list_head *iter; + struct slave *slave; struct port *port; - for (port = __get_first_port(bond); port; port = __get_next_port(port)) { + bond_for_each_slave(bond, slave, iter) { + port = &(SLAVE_AD_INFO(slave).port); if (port->aggregator && port->aggregator->is_active) { aggregator = port->aggregator; break; -- cgit v1.2.3 From fe9323dae5a05eb3d582d5546219c601862dd082 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 16:11:58 +0200 Subject: bonding: remove __get_first_port() Currently we have only one user of it, so it's kind of useless and just obfusicates things. Remove it and move the logic to the only user - bond_3ad_state_machine_handler(). CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index c1535f8762d1..0f86d2bd54b8 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -135,19 +135,6 @@ static inline struct bonding *__get_bond_by_port(struct port *port) return bond_get_bond_by_slave(port->slave); } -/** - * __get_first_port - get the first port in the bond - * @bond: the bond we're looking at - * - * Return the port of the first slave in @bond, or %NULL if it can't be found. - */ -static inline struct port *__get_first_port(struct bonding *bond) -{ - struct slave *first_slave = bond_first_slave(bond); - - return first_slave ? &(SLAVE_AD_INFO(first_slave).port) : NULL; -} - /** * __get_first_agg - get the first aggregator in the bond * @bond: the bond we're looking at @@ -2104,8 +2091,11 @@ void bond_3ad_state_machine_handler(struct work_struct *work) // 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); + port = slave ? &(SLAVE_AD_INFO(slave).port) : NULL; + // select the active aggregator for the bond - if ((port = __get_first_port(bond))) { + if (port) { if (!port->slave) { pr_warning("%s: Warning: bond's first port is uninitialized\n", bond->dev->name); -- cgit v1.2.3 From 3e36bb75ce32d0c14a736ca48480fe964491c7fc Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 16:11:59 +0200 Subject: bonding: make ad_port_selection_logic() use bond_for_each_slave() Currently, ad_port_selection_logic() uses for (aggregator = __get_first_agg(port); aggregator; aggregator = __get_next_agg(aggregator)) { construct, however it's suboptimal, difficult to read and understand. Change it to a standard bond_for_each_slave(), so that we won't need __get_first/next_agg() and have it more readable. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 0f86d2bd54b8..8b64ed4722cb 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -1239,12 +1239,17 @@ static void ad_port_selection_logic(struct port *port) { struct aggregator *aggregator, *free_aggregator = NULL, *temp_aggregator; struct port *last_port = NULL, *curr_port; + struct list_head *iter; + struct bonding *bond; + struct slave *slave; int found = 0; // if the port is already Selected, do nothing if (port->sm_vars & AD_PORT_SELECTED) return; + bond = __get_bond_by_port(port); + // if the port is connected to other aggregator, detach it if (port->aggregator) { // detach the port from its former aggregator @@ -1285,8 +1290,8 @@ static void ad_port_selection_logic(struct port *port) } } // search on all aggregators for a suitable aggregator for this port - for (aggregator = __get_first_agg(port); aggregator; - aggregator = __get_next_agg(aggregator)) { + bond_for_each_slave(bond, slave, iter) { + aggregator = &(SLAVE_AD_INFO(slave).aggregator); // keep a free aggregator for later use(if needed) if (!aggregator->lag_ports) { -- cgit v1.2.3 From 19177e7d554eba7e73d5eadc2c742d602bbec5ae Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 16:12:00 +0200 Subject: bonding: make __get_active_agg() use bond_for_each_slave() Currently we're relying on suboptimal construct for (; aggregator; aggregator = __get_next_agg(aggregator)) { where aggregator is an argument of __get_active_agg() which is _always_ the first slave's aggregator - judging by all the callers, comments in the ad_agg_selection_logic() and by logic. Convert it to use the standard bond_for_each_slave(). CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 8b64ed4722cb..1109d82d1929 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -720,16 +720,15 @@ static u32 __get_agg_bandwidth(struct aggregator *aggregator) */ static struct aggregator *__get_active_agg(struct aggregator *aggregator) { - struct aggregator *retval = NULL; + struct bonding *bond = aggregator->slave->bond; + struct list_head *iter; + struct slave *slave; - for (; aggregator; aggregator = __get_next_agg(aggregator)) { - if (aggregator->is_active) { - retval = aggregator; - break; - } - } + bond_for_each_slave(bond, slave, iter) + if (SLAVE_AD_INFO(slave).aggregator.is_active) + return &(SLAVE_AD_INFO(slave).aggregator); - return retval; + return NULL; } /** -- cgit v1.2.3 From bef1fcce41a317a068e6edd2232112afa7e05ab1 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 16:12:01 +0200 Subject: bonding: make ad_agg_selection_logic() use bond_for_each_slave() Convert all instances of for (agg = __get_first_agg(); agg; agg = __get_next_port) to the standard bond_for_each_slave(). Also, remove the useless checks before calling bond_3ad_set_carrier() - if we have something NULL - it would fire long ago, in __get_first/next_port(), per example. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 1109d82d1929..6cd1444e1de2 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -1484,19 +1484,23 @@ static int agg_device_up(const struct aggregator *agg) static void ad_agg_selection_logic(struct aggregator *agg) { struct aggregator *best, *active, *origin; + struct bonding *bond = agg->slave->bond; + struct list_head *iter; + struct slave *slave; struct port *port; origin = agg; active = __get_active_agg(agg); best = (active && agg_device_up(active)) ? active : NULL; - do { + bond_for_each_slave(bond, slave, iter) { + agg = &(SLAVE_AD_INFO(slave).aggregator); + agg->is_active = 0; if (agg->num_of_ports && agg_device_up(agg)) best = ad_agg_selection_test(best, agg); - - } while ((agg = __get_next_agg(agg))); + } if (best && __get_agg_selection_mode(best->lag_ports) == BOND_AD_STABLE) { @@ -1534,8 +1538,8 @@ static void ad_agg_selection_logic(struct aggregator *agg) best->lag_ports, best->slave, best->slave ? best->slave->dev->name : "NULL"); - for (agg = __get_first_agg(best->lag_ports); agg; - agg = __get_next_agg(agg)) { + bond_for_each_slave(bond, slave, iter) { + agg = &(SLAVE_AD_INFO(slave).aggregator); pr_debug("Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n", agg->aggregator_identifier, agg->num_of_ports, @@ -1583,13 +1587,7 @@ static void ad_agg_selection_logic(struct aggregator *agg) } } - if (origin->slave) { - struct bonding *bond; - - bond = bond_get_bond_by_slave(origin->slave); - if (bond) - bond_3ad_set_carrier(bond); - } + bond_3ad_set_carrier(bond); } /** -- cgit v1.2.3 From 0b08826478cde26b00e37e59b390371c7c7a7c7a Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 16:12:02 +0200 Subject: bonding: make bond_3ad_unbind_slave() use bond_for_each_slave() Convert all instances of for (agg = __get_first_agg(); agg; agg = __get_next_port) to the standard bond_for_each_slave(). CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 6cd1444e1de2..6dbb84d8a725 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -1936,6 +1936,9 @@ void bond_3ad_unbind_slave(struct slave *slave) struct port *port, *prev_port, *temp_port; struct aggregator *aggregator, *new_aggregator, *temp_aggregator; int select_new_active_agg = 0; + struct bonding *bond = slave->bond; + struct slave *slave_iter; + struct list_head *iter; // find the aggregator related to this slave aggregator = &(SLAVE_AD_INFO(slave).aggregator); @@ -1965,14 +1968,16 @@ void bond_3ad_unbind_slave(struct slave *slave) // reason to search for new aggregator, and that we will find one if ((aggregator->lag_ports != port) || (aggregator->lag_ports->next_port_in_aggregator)) { // find new aggregator for the related port(s) - new_aggregator = __get_first_agg(port); - for (; new_aggregator; new_aggregator = __get_next_agg(new_aggregator)) { + bond_for_each_slave(bond, slave_iter, iter) { + new_aggregator = &(SLAVE_AD_INFO(slave_iter).aggregator); // if the new aggregator is empty, or it is connected to our port only if (!new_aggregator->lag_ports || ((new_aggregator->lag_ports == port) && !new_aggregator->lag_ports->next_port_in_aggregator)) break; } + if (!slave_iter) + new_aggregator = NULL; // if new aggregator found, copy the aggregator's parameters // and connect the related lag_ports to the new aggregator if ((new_aggregator) && ((!new_aggregator->lag_ports) || ((new_aggregator->lag_ports == port) && !new_aggregator->lag_ports->next_port_in_aggregator))) { @@ -2032,8 +2037,8 @@ void bond_3ad_unbind_slave(struct slave *slave) pr_debug("Unbinding port %d\n", port->actor_port_number); // find the aggregator that this port is connected to - temp_aggregator = __get_first_agg(port); - for (; temp_aggregator; temp_aggregator = __get_next_agg(temp_aggregator)) { + bond_for_each_slave(bond, slave_iter, iter) { + temp_aggregator = &(SLAVE_AD_INFO(slave_iter).aggregator); prev_port = NULL; // search the port in the aggregator's related ports for (temp_port = temp_aggregator->lag_ports; temp_port; -- cgit v1.2.3 From da8f0919adcedb7a7b1d343e20c5d044928726d8 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 16:12:03 +0200 Subject: bonding: remove unused __get_next_agg() It has no users, so it's safe to remove it completely. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 22 ---------------------- 1 file changed, 22 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 6dbb84d8a725..c62606a67f6a 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -155,28 +155,6 @@ static inline struct aggregator *__get_first_agg(struct port *port) return first_slave ? &(SLAVE_AD_INFO(first_slave).aggregator) : NULL; } -/** - * __get_next_agg - get the next aggregator in the bond - * @aggregator: the aggregator we're looking at - * - * Return the aggregator of the slave that is next in line of @aggregator's - * slave in the bond, or %NULL if it can't be found. - */ -static inline struct aggregator *__get_next_agg(struct aggregator *aggregator) -{ - struct slave *slave = aggregator->slave, *slave_next; - struct bonding *bond = bond_get_bond_by_slave(slave); - - // If there's no bond for this aggregator, or this is the last slave - if (bond == NULL) - return NULL; - slave_next = bond_next_slave(bond, slave); - if (!slave_next || bond_is_first_slave(bond, slave_next)) - return NULL; - - return &(SLAVE_AD_INFO(slave_next).aggregator); -} - /* * __agg_has_partner * -- cgit v1.2.3 From f965084535713d952c5bb702806fd6052641b9e9 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 16:12:04 +0200 Subject: bonding: don't use bond_next_slave() in bond_info_seq_next() We don't need the circular loop there and it's the only current user of bond_next_slave() - so just use the standard bond_for_each_slave(). CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_procfs.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c index 7af5646e4410..fb868d6c22da 100644 --- a/drivers/net/bonding/bond_procfs.c +++ b/drivers/net/bonding/bond_procfs.c @@ -31,17 +31,25 @@ static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos) static void *bond_info_seq_next(struct seq_file *seq, void *v, loff_t *pos) { struct bonding *bond = seq->private; - struct slave *slave = v; + struct list_head *iter; + struct slave *slave; + bool found = false; ++*pos; if (v == SEQ_START_TOKEN) return bond_first_slave(bond); - if (bond_is_last_slave(bond, slave)) + if (bond_is_last_slave(bond, v)) return NULL; - slave = bond_next_slave(bond, slave); - return slave; + bond_for_each_slave(bond, slave, iter) { + if (found) + return slave; + if (slave == v) + found = true; + } + + return NULL; } static void bond_info_seq_stop(struct seq_file *seq, void *v) -- cgit v1.2.3 From 4aa0a03f519812f48ac48d046bc451e97649ec82 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 16:12:05 +0200 Subject: bonding: remove bond_next_slave() There are no users left, so it's safe to remove. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bonding.h | 31 ------------------------------- 1 file changed, 31 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 5b71601666cd..713b6af555c7 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -89,9 +89,6 @@ #define bond_is_first_slave(bond, pos) (pos == bond_first_slave(bond)) #define bond_is_last_slave(bond, pos) (pos == bond_last_slave(bond)) -/* Since bond_first/last_slave can return NULL, these can return NULL too */ -#define bond_next_slave(bond, pos) __bond_next_slave(bond, pos) - /** * bond_for_each_slave - iterate over all slaves * @bond: the bond holding this list @@ -243,34 +240,6 @@ struct bonding { #define bond_slave_get_rtnl(dev) \ ((struct slave *) rtnl_dereference(dev->rx_handler_data)) -/** - * __bond_next_slave - get the next slave after the one provided - * @bond - bonding struct - * @slave - the slave provided - * - * Returns the next slave after the slave provided, first slave if the - * slave provided is the last slave and NULL if slave is not found - */ -static inline struct slave *__bond_next_slave(struct bonding *bond, - struct slave *slave) -{ - struct slave *slave_iter; - struct list_head *iter; - bool found = false; - - netdev_for_each_lower_private(bond->dev, slave_iter, iter) { - if (found) - return slave_iter; - if (slave_iter == slave) - found = true; - } - - if (found) - return bond_first_slave(bond); - - return NULL; -} - /** * Returns NULL if the net_device does not belong to any of the bond's slaves * -- cgit v1.2.3 From 90de527d7ec77e509a22596d8f9eae9b90fd28ca Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Fri, 27 Sep 2013 01:22:01 +0200 Subject: bonding: trivial: remove forgotten bond_next_vlan() It's a forgotten function declaration, which was removed some time ago already. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bonding.h | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 713b6af555c7..9a26fbd82645 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -398,7 +398,6 @@ static inline bool slave_can_tx(struct slave *slave) struct bond_net; int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, struct slave *slave); -struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr); int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb, struct net_device *slave_dev); void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id); int bond_create(struct net *net, const char *name); -- cgit v1.2.3 From b32418705107265dfca5edfe2b547643e53a732e Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Sat, 28 Sep 2013 21:18:56 +0200 Subject: bonding: RCUify bond_set_rx_mode() Currently we rely on rtnl locking in bond_set_rx_mode(), however it's not always the case: RTNL: assertion failed at drivers/net/bonding/bond_main.c (3391) ... [] dump_stack+0x54/0x74 [] bond_set_rx_mode+0xc7/0xd0 [bonding] [] __dev_set_rx_mode+0x57/0xa0 [] __dev_mc_add+0x58/0x70 [] dev_mc_add+0x10/0x20 [] igmp6_group_added+0x18e/0x1d0 [] ? kmem_cache_alloc_trace+0x236/0x260 [] ipv6_dev_mc_inc+0x29f/0x320 [] ipv6_sock_mc_join+0x157/0x260 ... Fix this by using RCU primitives. Reported-by: Joe Lawrence Tested-by: Joe Lawrence CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 0367f8095390..894a7f34f88e 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -3393,20 +3393,21 @@ static void bond_set_rx_mode(struct net_device *bond_dev) struct list_head *iter; struct slave *slave; - ASSERT_RTNL(); + rcu_read_lock(); if (USES_PRIMARY(bond->params.mode)) { - slave = rtnl_dereference(bond->curr_active_slave); + slave = rcu_dereference(bond->curr_active_slave); if (slave) { dev_uc_sync(slave->dev, bond_dev); dev_mc_sync(slave->dev, bond_dev); } } else { - bond_for_each_slave(bond, slave, iter) { + bond_for_each_slave_rcu(bond, slave, iter) { dev_uc_sync_multiple(slave->dev, bond_dev); dev_mc_sync_multiple(slave->dev, bond_dev); } } + rcu_read_unlock(); } static int bond_neigh_init(struct neighbour *n) -- cgit v1.2.3 From 32819dc1834866cb9547cb75f81af9edd58d33cd Mon Sep 17 00:00:00 2001 From: Nikolay Aleksandrov Date: Wed, 2 Oct 2013 13:39:25 +0200 Subject: bonding: modify the old and add new xmit hash policies This patch adds two new hash policy modes which use skb_flow_dissect: 3 - Encapsulated layer 2+3 4 - Encapsulated layer 3+4 There should be a good improvement for tunnel users in those modes. It also changes the old hash functions to: hash ^= (__force u32)flow.dst ^ (__force u32)flow.src; hash ^= (hash >> 16); hash ^= (hash >> 8); Where hash will be initialized either to L2 hash, that is SRCMAC[5] XOR DSTMAC[5], or to flow->ports which should be extracted from the upper layer. Flow's dst and src are also extracted based on the xmit policy either directly from the buffer or by using skb_flow_dissect, but in both cases if the protocol is IPv6 then dst and src are obtained by ipv6_addr_hash() on the real addresses. In case of a non-dissectable packet, the algorithms fall back to L2 hashing. The bond_set_mode_ops() function is now obsolete and thus deleted because it was used only to set the proper hash policy. Also we trim a pointer from struct bonding because we no longer need to keep the hash function, now there's only a single hash function - bond_xmit_hash that works based on bond->params.xmit_policy. The hash function and skb_flow_dissect were suggested by Eric Dumazet. The layer names were suggested by Andy Gospodarek, because I suck at semantics. Signed-off-by: Nikolay Aleksandrov Acked-by: Eric Dumazet Acked-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 2 +- drivers/net/bonding/bond_main.c | 197 ++++++++++++++------------------------- drivers/net/bonding/bond_sysfs.c | 2 - drivers/net/bonding/bonding.h | 3 +- include/uapi/linux/if_bonding.h | 2 + 5 files changed, 72 insertions(+), 134 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index c62606a67f6a..ea3e64e22e22 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -2403,7 +2403,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev) goto out; } - slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg); + slave_agg_no = bond_xmit_hash(bond, skb, slaves_in_agg); first_ok_slave = NULL; bond_for_each_slave(bond, slave, iter) { diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index fe8a94f9d7db..dfb4f6dd5de0 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -78,6 +78,7 @@ #include #include #include +#include #include "bonding.h" #include "bond_3ad.h" #include "bond_alb.h" @@ -159,7 +160,8 @@ MODULE_PARM_DESC(min_links, "Minimum number of available links before turning on module_param(xmit_hash_policy, charp, 0); MODULE_PARM_DESC(xmit_hash_policy, "balance-xor and 802.3ad hashing method; " "0 for layer 2 (default), 1 for layer 3+4, " - "2 for layer 2+3"); + "2 for layer 2+3, 3 for encap layer 2+3, " + "4 for encap layer 3+4"); module_param(arp_interval, int, 0); MODULE_PARM_DESC(arp_interval, "arp interval in milliseconds"); module_param_array(arp_ip_target, charp, NULL, 0); @@ -217,6 +219,8 @@ const struct bond_parm_tbl xmit_hashtype_tbl[] = { { "layer2", BOND_XMIT_POLICY_LAYER2}, { "layer3+4", BOND_XMIT_POLICY_LAYER34}, { "layer2+3", BOND_XMIT_POLICY_LAYER23}, +{ "encap2+3", BOND_XMIT_POLICY_ENCAP23}, +{ "encap3+4", BOND_XMIT_POLICY_ENCAP34}, { NULL, -1}, }; @@ -3035,99 +3039,85 @@ static struct notifier_block bond_netdev_notifier = { /*---------------------------- Hashing Policies -----------------------------*/ -/* - * Hash for the output device based upon layer 2 data - */ -static int bond_xmit_hash_policy_l2(struct sk_buff *skb, int count) +/* L2 hash helper */ +static inline u32 bond_eth_hash(struct sk_buff *skb) { struct ethhdr *data = (struct ethhdr *)skb->data; if (skb_headlen(skb) >= offsetof(struct ethhdr, h_proto)) - return (data->h_dest[5] ^ data->h_source[5]) % count; + return data->h_dest[5] ^ data->h_source[5]; return 0; } -/* - * Hash for the output device based upon layer 2 and layer 3 data. If - * the packet is not IP, fall back on bond_xmit_hash_policy_l2() - */ -static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count) +/* Extract the appropriate headers based on bond's xmit policy */ +static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb, + struct flow_keys *fk) { - const struct ethhdr *data; + const struct ipv6hdr *iph6; const struct iphdr *iph; - const struct ipv6hdr *ipv6h; - u32 v6hash; - const __be32 *s, *d; + int noff, proto = -1; - if (skb->protocol == htons(ETH_P_IP) && - pskb_network_may_pull(skb, sizeof(*iph))) { + if (bond->params.xmit_policy > BOND_XMIT_POLICY_LAYER23) + return skb_flow_dissect(skb, fk); + + fk->ports = 0; + noff = skb_network_offset(skb); + if (skb->protocol == htons(ETH_P_IP)) { + if (!pskb_may_pull(skb, noff + sizeof(*iph))) + return false; iph = ip_hdr(skb); - data = (struct ethhdr *)skb->data; - return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^ - (data->h_dest[5] ^ data->h_source[5])) % count; - } else if (skb->protocol == htons(ETH_P_IPV6) && - pskb_network_may_pull(skb, sizeof(*ipv6h))) { - ipv6h = ipv6_hdr(skb); - data = (struct ethhdr *)skb->data; - s = &ipv6h->saddr.s6_addr32[0]; - d = &ipv6h->daddr.s6_addr32[0]; - v6hash = (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]); - v6hash ^= (v6hash >> 24) ^ (v6hash >> 16) ^ (v6hash >> 8); - return (v6hash ^ data->h_dest[5] ^ data->h_source[5]) % count; - } - - return bond_xmit_hash_policy_l2(skb, count); + fk->src = iph->saddr; + fk->dst = iph->daddr; + noff += iph->ihl << 2; + if (!ip_is_fragment(iph)) + proto = iph->protocol; + } else if (skb->protocol == htons(ETH_P_IPV6)) { + if (!pskb_may_pull(skb, noff + sizeof(*iph6))) + return false; + iph6 = ipv6_hdr(skb); + fk->src = (__force __be32)ipv6_addr_hash(&iph6->saddr); + fk->dst = (__force __be32)ipv6_addr_hash(&iph6->daddr); + noff += sizeof(*iph6); + proto = iph6->nexthdr; + } else { + return false; + } + if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER34 && proto >= 0) + fk->ports = skb_flow_get_ports(skb, noff, proto); + + return true; } -/* - * Hash for the output device based upon layer 3 and layer 4 data. If - * the packet is a frag or not TCP or UDP, just use layer 3 data. If it is - * altogether not IP, fall back on bond_xmit_hash_policy_l2() +/** + * bond_xmit_hash - generate a hash value based on the xmit policy + * @bond: bonding device + * @skb: buffer to use for headers + * @count: modulo value + * + * This function will extract the necessary headers from the skb buffer and use + * them to generate a hash based on the xmit_policy set in the bonding device + * which will be reduced modulo count before returning. */ -static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count) +int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count) { - u32 layer4_xor = 0; - const struct iphdr *iph; - const struct ipv6hdr *ipv6h; - const __be32 *s, *d; - const __be16 *l4 = NULL; - __be16 _l4[2]; - int noff = skb_network_offset(skb); - int poff; - - if (skb->protocol == htons(ETH_P_IP) && - pskb_may_pull(skb, noff + sizeof(*iph))) { - iph = ip_hdr(skb); - poff = proto_ports_offset(iph->protocol); + struct flow_keys flow; + u32 hash; - if (!ip_is_fragment(iph) && poff >= 0) { - l4 = skb_header_pointer(skb, noff + (iph->ihl << 2) + poff, - sizeof(_l4), &_l4); - if (l4) - layer4_xor = ntohs(l4[0] ^ l4[1]); - } - return (layer4_xor ^ - ((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count; - } else if (skb->protocol == htons(ETH_P_IPV6) && - pskb_may_pull(skb, noff + sizeof(*ipv6h))) { - ipv6h = ipv6_hdr(skb); - poff = proto_ports_offset(ipv6h->nexthdr); - if (poff >= 0) { - l4 = skb_header_pointer(skb, noff + sizeof(*ipv6h) + poff, - sizeof(_l4), &_l4); - if (l4) - layer4_xor = ntohs(l4[0] ^ l4[1]); - } - s = &ipv6h->saddr.s6_addr32[0]; - d = &ipv6h->daddr.s6_addr32[0]; - layer4_xor ^= (s[1] ^ d[1]) ^ (s[2] ^ d[2]) ^ (s[3] ^ d[3]); - layer4_xor ^= (layer4_xor >> 24) ^ (layer4_xor >> 16) ^ - (layer4_xor >> 8); - return layer4_xor % count; - } + if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER2 || + !bond_flow_dissect(bond, skb, &flow)) + return bond_eth_hash(skb) % count; + + if (bond->params.xmit_policy == BOND_XMIT_POLICY_LAYER23 || + bond->params.xmit_policy == BOND_XMIT_POLICY_ENCAP23) + hash = bond_eth_hash(skb); + else + hash = (__force u32)flow.ports; + hash ^= (__force u32)flow.dst ^ (__force u32)flow.src; + hash ^= (hash >> 16); + hash ^= (hash >> 8); - return bond_xmit_hash_policy_l2(skb, count); + return hash % count; } /*-------------------------- Device entry points ----------------------------*/ @@ -3721,8 +3711,7 @@ 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- +/* 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. */ @@ -3730,8 +3719,7 @@ static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); - bond_xmit_slave_id(bond, skb, - bond->xmit_hash_policy(skb, bond->slave_cnt)); + bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb, bond->slave_cnt)); return NETDEV_TX_OK; } @@ -3768,22 +3756,6 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev) /*------------------------- Device initialization ---------------------------*/ -static void bond_set_xmit_hash_policy(struct bonding *bond) -{ - switch (bond->params.xmit_policy) { - case BOND_XMIT_POLICY_LAYER23: - bond->xmit_hash_policy = bond_xmit_hash_policy_l23; - break; - case BOND_XMIT_POLICY_LAYER34: - bond->xmit_hash_policy = bond_xmit_hash_policy_l34; - break; - case BOND_XMIT_POLICY_LAYER2: - default: - bond->xmit_hash_policy = bond_xmit_hash_policy_l2; - break; - } -} - /* * Lookup the slave that corresponds to a qid */ @@ -3894,38 +3866,6 @@ static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev) return ret; } -/* - * set bond mode specific net device operations - */ -void bond_set_mode_ops(struct bonding *bond, int mode) -{ - struct net_device *bond_dev = bond->dev; - - switch (mode) { - case BOND_MODE_ROUNDROBIN: - break; - case BOND_MODE_ACTIVEBACKUP: - break; - case BOND_MODE_XOR: - bond_set_xmit_hash_policy(bond); - break; - case BOND_MODE_BROADCAST: - break; - case BOND_MODE_8023AD: - bond_set_xmit_hash_policy(bond); - break; - case BOND_MODE_ALB: - /* FALLTHRU */ - case BOND_MODE_TLB: - break; - default: - /* Should never happen, mode already checked */ - pr_err("%s: Error: Unknown bonding mode %d\n", - bond_dev->name, mode); - break; - } -} - static int bond_ethtool_get_settings(struct net_device *bond_dev, struct ethtool_cmd *ecmd) { @@ -4027,7 +3967,6 @@ static void bond_setup(struct net_device *bond_dev) ether_setup(bond_dev); bond_dev->netdev_ops = &bond_netdev_ops; bond_dev->ethtool_ops = &bond_ethtool_ops; - bond_set_mode_ops(bond, bond->params.mode); bond_dev->destructor = bond_destructor; diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index e06c644470b1..e9249527e7e7 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -318,7 +318,6 @@ static ssize_t bonding_store_mode(struct device *d, /* don't cache arp_validate between modes */ bond->params.arp_validate = BOND_ARP_VALIDATE_NONE; bond->params.mode = new_value; - bond_set_mode_ops(bond, bond->params.mode); pr_info("%s: setting mode to %s (%d).\n", bond->dev->name, bond_mode_tbl[new_value].modename, new_value); @@ -358,7 +357,6 @@ static ssize_t bonding_store_xmit_hash(struct device *d, ret = -EINVAL; } else { bond->params.xmit_policy = new_value; - bond_set_mode_ops(bond, bond->params.mode); pr_info("%s: setting xmit hash policy to %s (%d).\n", bond->dev->name, xmit_hashtype_tbl[new_value].modename, new_value); diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 9a26fbd82645..0bd04fbda8e9 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -217,7 +217,6 @@ struct bonding { char proc_file_name[IFNAMSIZ]; #endif /* CONFIG_PROC_FS */ struct list_head bond_list; - int (*xmit_hash_policy)(struct sk_buff *, int); u16 rr_tx_counter; struct ad_bond_info ad_info; struct alb_bond_info alb_info; @@ -409,7 +408,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev); void bond_mii_monitor(struct work_struct *); void bond_loadbalance_arp_mon(struct work_struct *); void bond_activebackup_arp_mon(struct work_struct *); -void bond_set_mode_ops(struct bonding *bond, int mode); +int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count); int bond_parse_parm(const char *mode_arg, const struct bond_parm_tbl *tbl); void bond_select_active_slave(struct bonding *bond); void bond_change_active_slave(struct bonding *bond, struct slave *new_active); diff --git a/include/uapi/linux/if_bonding.h b/include/uapi/linux/if_bonding.h index a17edda8a781..9635a62f6f89 100644 --- a/include/uapi/linux/if_bonding.h +++ b/include/uapi/linux/if_bonding.h @@ -91,6 +91,8 @@ #define BOND_XMIT_POLICY_LAYER2 0 /* layer 2 (MAC only), default */ #define BOND_XMIT_POLICY_LAYER34 1 /* layer 3+4 (IP ^ (TCP || UDP)) */ #define BOND_XMIT_POLICY_LAYER23 2 /* layer 2+3 (IP ^ MAC) */ +#define BOND_XMIT_POLICY_ENCAP23 3 /* encapsulated layer 2+3 */ +#define BOND_XMIT_POLICY_ENCAP34 4 /* encapsulated layer 3+4 */ typedef struct ifbond { __s32 bond_mode; -- cgit v1.2.3 From 4996b9098d5522f9d3233af6a7efd1fac5d43f00 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Mon, 7 Oct 2013 09:17:20 +0200 Subject: bonding: ensure that TLB mode's active slave has correct mac filter Currently, in TLB mode we change mac addresses only by memcpy-ing the to net_device->dev_addr, without actually setting them via dev_set_mac_address(). This permits us to receive all the traffic always on one mac address. However, in case the interface flips, some drivers might enforce the mac filtering for its FW/HW based on current ->dev_addr, and thus we won't be able to receive traffic on that interface, in case it will be selected as active in TLB mode. Fix it by setting the mac address forcefully on every new active slave that we select in TLB mode. CC: Jay Vosburgh CC: Andy Gospodarek CC: Yuval Mintz Reported-by: Yuval Mintz Tested-by: Yuval Mintz Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_alb.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index e96041816b5b..576cceae026a 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -1699,6 +1699,23 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave ASSERT_RTNL(); + /* in TLB mode, the slave might flip down/up with the old dev_addr, + * and thus filter bond->dev_addr's packets, so force bond's mac + */ + if (bond->params.mode == BOND_MODE_TLB) { + struct sockaddr sa; + u8 tmp_addr[ETH_ALEN]; + + memcpy(tmp_addr, new_slave->dev->dev_addr, ETH_ALEN); + + memcpy(sa.sa_data, bond->dev->dev_addr, bond->dev->addr_len); + sa.sa_family = bond->dev->type; + /* we don't care if it can't change its mac, best effort */ + dev_set_mac_address(new_slave->dev, &sa); + + memcpy(new_slave->dev->dev_addr, tmp_addr, ETH_ALEN); + } + /* curr_active_slave must be set before calling alb_swap_mac_addr */ if (swap_slave) { /* swap mac address */ -- cgit v1.2.3 From 47e91f56008b43e1365e8d1d4a6813fe8a33b6f6 Mon Sep 17 00:00:00 2001 From: dingtianhong Date: Tue, 15 Oct 2013 16:28:35 +0800 Subject: bonding: use RCU protection for 3ad xmit path The commit 278b20837511776dc9d5f6ee1c7fabd5479838bb (bonding: initial RCU conversion) has convert the roundrobin, active-backup, broadcast and xor xmit path to rcu protection, the performance will be better for these mode, so this time, convert xmit path for 3ad mode. Suggested-by: Nikolay Aleksandrov Suggested-by: Veaceslav Falico Signed-off-by: Ding Tianhong Signed-off-by: Wang Yufen Cc: Nikolay Aleksandrov Cc: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index ea3e64e22e22..187b1b7772ef 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -2344,7 +2344,7 @@ int __bond_3ad_get_active_agg_info(struct bonding *bond, struct slave *slave; struct port *port; - bond_for_each_slave(bond, slave, iter) { + bond_for_each_slave_rcu(bond, slave, iter) { port = &(SLAVE_AD_INFO(slave).port); if (port->aggregator && port->aggregator->is_active) { aggregator = port->aggregator; @@ -2369,9 +2369,9 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info) { int ret; - read_lock(&bond->lock); + rcu_read_lock(); ret = __bond_3ad_get_active_agg_info(bond, ad_info); - read_unlock(&bond->lock); + rcu_read_unlock(); return ret; } @@ -2388,7 +2388,6 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev) int res = 1; int agg_id; - read_lock(&bond->lock); if (__bond_3ad_get_active_agg_info(bond, &ad_info)) { pr_debug("%s: Error: __bond_3ad_get_active_agg_info failed\n", dev->name); @@ -2406,7 +2405,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev) slave_agg_no = bond_xmit_hash(bond, skb, slaves_in_agg); first_ok_slave = NULL; - bond_for_each_slave(bond, slave, iter) { + bond_for_each_slave_rcu(bond, slave, iter) { agg = SLAVE_AD_INFO(slave).port.aggregator; if (!agg || agg->aggregator_identifier != agg_id) continue; @@ -2436,7 +2435,6 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev) res = bond_dev_queue_xmit(bond, skb, first_ok_slave->dev); out: - read_unlock(&bond->lock); if (res) { /* no suitable interface, frame not sent */ kfree_skb(skb); -- cgit v1.2.3 From 28c719260da032c999ecb4b73ba56311c635ef4e Mon Sep 17 00:00:00 2001 From: dingtianhong Date: Tue, 15 Oct 2013 16:28:39 +0800 Subject: bonding: use RCU protection for alb xmit path The commit 278b20837511776dc9d5f6ee1c7fabd5479838bb (bonding: initial RCU conversion) has convert the roundrobin, active-backup, broadcast and xor xmit path to rcu protection, the performance will be better for these mode, so this time, convert xmit path for alb mode. Signed-off-by: Ding Tianhong Signed-off-by: Yang Yingliang Cc: Nikolay Aleksandrov Cc: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_alb.c | 58 +++++++++++++++++++++++++++++++----------- drivers/net/bonding/bonding.h | 14 ++++++++++ 2 files changed, 57 insertions(+), 15 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index 576cceae026a..02872405d35d 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -230,7 +230,7 @@ static struct slave *tlb_get_least_loaded_slave(struct bonding *bond) max_gap = LLONG_MIN; /* Find the slave with the largest gap */ - bond_for_each_slave(bond, slave, iter) { + bond_for_each_slave_rcu(bond, slave, iter) { if (SLAVE_IS_OK(slave)) { long long gap = compute_gap(slave); @@ -412,6 +412,39 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond) return rx_slave; } +/* Caller must hold rcu_read_lock() for read */ +static struct slave *__rlb_next_rx_slave(struct bonding *bond) +{ + struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); + struct slave *before = NULL, *rx_slave = NULL, *slave; + struct list_head *iter; + bool found = false; + + bond_for_each_slave_rcu(bond, slave, iter) { + if (!SLAVE_IS_OK(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) + bond_info->rx_slave = rx_slave; + + return rx_slave; +} + /* teach the switch the mac of a disabled slave * on the primary for fault tolerance * @@ -628,12 +661,14 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon { struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); struct arp_pkt *arp = arp_pkt(skb); - struct slave *assigned_slave; + struct slave *assigned_slave, *curr_active_slave; struct rlb_client_info *client_info; u32 hash_index = 0; _lock_rx_hashtbl(bond); + curr_active_slave = rcu_dereference(bond->curr_active_slave); + hash_index = _simple_hash((u8 *)&arp->ip_dst, sizeof(arp->ip_dst)); client_info = &(bond_info->rx_hashtbl[hash_index]); @@ -658,14 +693,14 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon * that the new client can be assigned to this entry. */ if (bond->curr_active_slave && - client_info->slave != bond->curr_active_slave) { - client_info->slave = bond->curr_active_slave; + client_info->slave != curr_active_slave) { + client_info->slave = curr_active_slave; rlb_update_client(client_info); } } } /* assign a new slave */ - assigned_slave = rlb_next_rx_slave(bond); + assigned_slave = __rlb_next_rx_slave(bond); if (assigned_slave) { if (!(client_info->assigned && @@ -728,7 +763,7 @@ static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond) /* Don't modify or load balance ARPs that do not originate locally * (e.g.,arrive via a bridge). */ - if (!bond_slave_has_mac(bond, arp->mac_src)) + if (!bond_slave_has_mac_rcu(bond, arp->mac_src)) return NULL; if (arp->op_code == htons(ARPOP_REPLY)) { @@ -1343,11 +1378,6 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev) skb_reset_mac_header(skb); eth_data = eth_hdr(skb); - /* make sure that the curr_active_slave do not change during tx - */ - read_lock(&bond->lock); - read_lock(&bond->curr_slave_lock); - switch (ntohs(skb->protocol)) { case ETH_P_IP: { const struct iphdr *iph = ip_hdr(skb); @@ -1429,12 +1459,12 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev) if (!tx_slave) { /* unbalanced or unassigned, send through primary */ - tx_slave = bond->curr_active_slave; + tx_slave = rcu_dereference(bond->curr_active_slave); bond_info->unbalanced_load += skb->len; } if (tx_slave && SLAVE_IS_OK(tx_slave)) { - if (tx_slave != bond->curr_active_slave) { + if (tx_slave != rcu_dereference(bond->curr_active_slave)) { memcpy(eth_data->h_source, tx_slave->dev->dev_addr, ETH_ALEN); @@ -1449,8 +1479,6 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev) } } - read_unlock(&bond->curr_slave_lock); - read_unlock(&bond->lock); if (res) { /* no suitable interface, frame not sent */ kfree_skb(skb); diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 0bd04fbda8e9..bb5c731e2560 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -464,6 +464,20 @@ static inline struct slave *bond_slave_has_mac(struct bonding *bond, return NULL; } +/* Caller must hold rcu_read_lock() for read */ +static inline struct slave *bond_slave_has_mac_rcu(struct bonding *bond, + const u8 *mac) +{ + struct list_head *iter; + struct slave *tmp; + + bond_for_each_slave_rcu(bond, tmp, iter) + if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr)) + return tmp; + + return NULL; +} + /* Check if the ip is present in arp ip list, or first free slot if ip == 0 * Returns -1 if not found, index if found */ -- cgit v1.2.3 From 4d1ae5fb752b2504cf2c3d79abdfb410a09ad928 Mon Sep 17 00:00:00 2001 From: dingtianhong Date: Tue, 15 Oct 2013 16:28:42 +0800 Subject: bonding: add rtnl lock and remove read lock for bond sysfs The bond_for_each_slave() will not be protected by read_lock(), only protected by rtnl_lock(), so need to replace read_lock() with rtnl_lock(). Signed-off-by: Ding Tianhong Signed-off-by: David S. Miller --- drivers/net/bonding/bond_sysfs.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index e9249527e7e7..03bed0ca935e 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -179,7 +179,9 @@ static ssize_t bonding_show_slaves(struct device *d, struct slave *slave; int res = 0; - read_lock(&bond->lock); + if (!rtnl_trylock()) + return restart_syscall(); + bond_for_each_slave(bond, slave, iter) { if (res > (PAGE_SIZE - IFNAMSIZ)) { /* not enough space for another interface name */ @@ -190,7 +192,9 @@ static ssize_t bonding_show_slaves(struct device *d, } res += sprintf(buf + res, "%s ", slave->dev->name); } - read_unlock(&bond->lock); + + rtnl_unlock(); + if (res) buf[res-1] = '\n'; /* eat the leftover space */ @@ -626,6 +630,9 @@ static ssize_t bonding_store_arp_targets(struct device *d, unsigned long *targets_rx; int ind, i, j, ret = -EINVAL; + if (!rtnl_trylock()) + return restart_syscall(); + targets = bond->params.arp_targets; newtarget = in_aton(buf + 1); /* look for adds */ @@ -699,6 +706,7 @@ static ssize_t bonding_store_arp_targets(struct device *d, ret = count; out: + rtnl_unlock(); return ret; } static DEVICE_ATTR(arp_ip_target, S_IRUGO | S_IWUSR , bonding_show_arp_targets, bonding_store_arp_targets); @@ -1467,7 +1475,6 @@ static ssize_t bonding_show_queue_id(struct device *d, if (!rtnl_trylock()) return restart_syscall(); - read_lock(&bond->lock); bond_for_each_slave(bond, slave, iter) { if (res > (PAGE_SIZE - IFNAMSIZ - 6)) { /* not enough space for another interface_name:queue_id pair */ @@ -1479,9 +1486,9 @@ static ssize_t bonding_show_queue_id(struct device *d, res += sprintf(buf + res, "%s:%d ", slave->dev->name, slave->queue_id); } - read_unlock(&bond->lock); if (res) buf[res-1] = '\n'; /* eat the leftover space */ + rtnl_unlock(); return res; @@ -1530,8 +1537,6 @@ static ssize_t bonding_store_queue_id(struct device *d, if (!sdev) goto err_no_cmd; - read_lock(&bond->lock); - /* Search for thes slave and check for duplicate qids */ update_slave = NULL; bond_for_each_slave(bond, slave, iter) { @@ -1542,23 +1547,20 @@ static ssize_t bonding_store_queue_id(struct device *d, */ update_slave = slave; else if (qid && qid == slave->queue_id) { - goto err_no_cmd_unlock; + goto err_no_cmd; } } if (!update_slave) - goto err_no_cmd_unlock; + goto err_no_cmd; /* Actually set the qids for the slave */ update_slave->queue_id = qid; - read_unlock(&bond->lock); out: rtnl_unlock(); return ret; -err_no_cmd_unlock: - read_unlock(&bond->lock); err_no_cmd: pr_info("invalid input for queue_id set for %s.\n", bond->dev->name); @@ -1591,6 +1593,9 @@ static ssize_t bonding_store_slaves_active(struct device *d, struct list_head *iter; struct slave *slave; + if (!rtnl_trylock()) + return restart_syscall(); + if (sscanf(buf, "%d", &new_value) != 1) { pr_err("%s: no all_slaves_active value specified.\n", bond->dev->name); @@ -1610,7 +1615,6 @@ static ssize_t bonding_store_slaves_active(struct device *d, goto out; } - read_lock(&bond->lock); bond_for_each_slave(bond, slave, iter) { if (!bond_is_active_slave(slave)) { if (new_value) @@ -1619,8 +1623,8 @@ static ssize_t bonding_store_slaves_active(struct device *d, slave->inactive = 1; } } - read_unlock(&bond->lock); out: + rtnl_unlock(); return ret; } static DEVICE_ATTR(all_slaves_active, S_IRUGO | S_IWUSR, -- cgit v1.2.3 From 0a2a78c4a95240e658272bd7cd7422a529e4eb4a Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 18 Oct 2013 17:43:33 +0200 Subject: bonding: push Netlink bits into separate file Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/bonding/Makefile | 2 +- drivers/net/bonding/bond_main.c | 32 ++++------------------ drivers/net/bonding/bond_netlink.c | 54 ++++++++++++++++++++++++++++++++++++++ drivers/net/bonding/bonding.h | 7 +++++ 4 files changed, 67 insertions(+), 28 deletions(-) create mode 100644 drivers/net/bonding/bond_netlink.c (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/Makefile b/drivers/net/bonding/Makefile index 4c21bf6b8b2f..09e8b2c83afe 100644 --- a/drivers/net/bonding/Makefile +++ b/drivers/net/bonding/Makefile @@ -4,7 +4,7 @@ obj-$(CONFIG_BONDING) += bonding.o -bonding-objs := bond_main.o bond_3ad.o bond_alb.o bond_sysfs.o bond_debugfs.o +bonding-objs := bond_main.o bond_3ad.o bond_alb.o bond_sysfs.o bond_debugfs.o bond_netlink.o proc-$(CONFIG_PROC_FS) += bond_procfs.o bonding-objs += $(proc-y) diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index dfb4f6dd5de0..a113e4212486 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -3951,7 +3951,7 @@ static void bond_destructor(struct net_device *bond_dev) free_netdev(bond_dev); } -static void bond_setup(struct net_device *bond_dev) +void bond_setup(struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); @@ -4451,32 +4451,11 @@ static int bond_init(struct net_device *bond_dev) return 0; } -static int bond_validate(struct nlattr *tb[], struct nlattr *data[]) -{ - if (tb[IFLA_ADDRESS]) { - if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN) - return -EINVAL; - if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS]))) - return -EADDRNOTAVAIL; - } - return 0; -} - -static unsigned int bond_get_num_tx_queues(void) +unsigned int bond_get_num_tx_queues(void) { return tx_queues; } -static struct rtnl_link_ops bond_link_ops __read_mostly = { - .kind = "bond", - .priv_size = sizeof(struct bonding), - .setup = bond_setup, - .validate = bond_validate, - .get_num_tx_queues = bond_get_num_tx_queues, - .get_num_rx_queues = bond_get_num_tx_queues, /* Use the same number - as for TX queues */ -}; - /* Create a new bond based on the specified name and bonding parameters. * If name is NULL, obtain a suitable "bond%d" name for us. * Caller must NOT hold rtnl_lock; we need to release it here before we @@ -4563,7 +4542,7 @@ static int __init bonding_init(void) if (res) goto out; - res = rtnl_link_register(&bond_link_ops); + res = bond_netlink_init(); if (res) goto err_link; @@ -4579,7 +4558,7 @@ static int __init bonding_init(void) out: return res; err: - rtnl_link_unregister(&bond_link_ops); + bond_netlink_fini(); err_link: unregister_pernet_subsys(&bond_net_ops); goto out; @@ -4592,7 +4571,7 @@ static void __exit bonding_exit(void) bond_destroy_debugfs(); - rtnl_link_unregister(&bond_link_ops); + bond_netlink_fini(); unregister_pernet_subsys(&bond_net_ops); #ifdef CONFIG_NET_POLL_CONTROLLER @@ -4609,4 +4588,3 @@ MODULE_LICENSE("GPL"); MODULE_VERSION(DRV_VERSION); MODULE_DESCRIPTION(DRV_DESCRIPTION ", v" DRV_VERSION); MODULE_AUTHOR("Thomas Davis, tadavis@lbl.gov and many others"); -MODULE_ALIAS_RTNL_LINK("bond"); diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c new file mode 100644 index 000000000000..3e5c5f80c320 --- /dev/null +++ b/drivers/net/bonding/bond_netlink.c @@ -0,0 +1,54 @@ +/* + * drivers/net/bond/bond_netlink.c - Netlink interface for bonding + * Copyright (c) 2013 Jiri Pirko + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include +#include +#include +#include +#include +#include "bonding.h" + +static int bond_validate(struct nlattr *tb[], struct nlattr *data[]) +{ + if (tb[IFLA_ADDRESS]) { + if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN) + return -EINVAL; + if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS]))) + return -EADDRNOTAVAIL; + } + return 0; +} + +struct rtnl_link_ops bond_link_ops __read_mostly = { + .kind = "bond", + .priv_size = sizeof(struct bonding), + .setup = bond_setup, + .validate = bond_validate, + .get_num_tx_queues = bond_get_num_tx_queues, + .get_num_rx_queues = bond_get_num_tx_queues, /* Use the same number + as for TX queues */ +}; + +int __init bond_netlink_init(void) +{ + return rtnl_link_register(&bond_link_ops); +} + +void __exit bond_netlink_fini(void) +{ + rtnl_link_unregister(&bond_link_ops); +} + +MODULE_ALIAS_RTNL_LINK("bond"); diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index bb5c731e2560..a2a353bf9ea6 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -418,6 +418,10 @@ void bond_debug_register(struct bonding *bond); void bond_debug_unregister(struct bonding *bond); void bond_debug_reregister(struct bonding *bond); const char *bond_mode_name(int mode); +void bond_setup(struct net_device *bond_dev); +unsigned int bond_get_num_tx_queues(void); +int bond_netlink_init(void); +void bond_netlink_fini(void); struct bond_net { struct net * net; /* Associated network namespace */ @@ -505,4 +509,7 @@ extern const struct bond_parm_tbl fail_over_mac_tbl[]; extern const struct bond_parm_tbl pri_reselect_tbl[]; extern struct bond_parm_tbl ad_select_tbl[]; +/* exported from bond_netlink.c */ +extern struct rtnl_link_ops bond_link_ops; + #endif /* _LINUX_BONDING_H */ -- cgit v1.2.3 From 72be35fee6eda2fad7122f7f0c959effa3b2b791 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 18 Oct 2013 17:43:34 +0200 Subject: bonding: move mode setting into separate function Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/bonding/Makefile | 2 +- drivers/net/bonding/bond_options.c | 55 ++++++++++++++++++++++++++++++++++++++ drivers/net/bonding/bond_sysfs.c | 45 ++++++++----------------------- drivers/net/bonding/bonding.h | 9 +++++-- 4 files changed, 74 insertions(+), 37 deletions(-) create mode 100644 drivers/net/bonding/bond_options.c (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/Makefile b/drivers/net/bonding/Makefile index 09e8b2c83afe..5a5d720da929 100644 --- a/drivers/net/bonding/Makefile +++ b/drivers/net/bonding/Makefile @@ -4,7 +4,7 @@ obj-$(CONFIG_BONDING) += bonding.o -bonding-objs := bond_main.o bond_3ad.o bond_alb.o bond_sysfs.o bond_debugfs.o bond_netlink.o +bonding-objs := bond_main.o bond_3ad.o bond_alb.o bond_sysfs.o bond_debugfs.o bond_netlink.o bond_options.o proc-$(CONFIG_PROC_FS) += bond_procfs.o bonding-objs += $(proc-y) diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c new file mode 100644 index 000000000000..294b7660b054 --- /dev/null +++ b/drivers/net/bonding/bond_options.c @@ -0,0 +1,55 @@ +/* + * drivers/net/bond/bond_options.c - bonding options + * Copyright (c) 2013 Jiri Pirko + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include "bonding.h" + +static bool bond_mode_is_valid(int mode) +{ + int i; + + for (i = 0; bond_mode_tbl[i].modename; i++); + + return mode >= 0 && mode < i; +} + +int bond_option_mode_set(struct bonding *bond, int mode) +{ + if (!bond_mode_is_valid(mode)) { + pr_err("invalid mode value %d.\n", mode); + return -EINVAL; + } + + if (bond->dev->flags & IFF_UP) { + pr_err("%s: unable to update mode because interface is up.\n", + bond->dev->name); + return -EPERM; + } + + if (bond_has_slaves(bond)) { + pr_err("%s: unable to update mode because bond has slaves.\n", + bond->dev->name); + return -EPERM; + } + + if (BOND_MODE_IS_LB(mode) && bond->params.arp_interval) { + pr_err("%s: %s mode is incompatible with arp monitoring.\n", + bond->dev->name, bond_mode_tbl[mode].modename); + return -EINVAL; + } + + /* don't cache arp_validate between modes */ + bond->params.arp_validate = BOND_ARP_VALIDATE_NONE; + bond->params.mode = mode; + return 0; +} diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index 03bed0ca935e..c234cec10e05 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -283,49 +283,26 @@ static ssize_t bonding_store_mode(struct device *d, struct device_attribute *attr, const char *buf, size_t count) { - int new_value, ret = count; + int new_value, ret; struct bonding *bond = to_bond(d); - if (!rtnl_trylock()) - return restart_syscall(); - - if (bond->dev->flags & IFF_UP) { - pr_err("unable to update mode of %s because interface is up.\n", - bond->dev->name); - ret = -EPERM; - goto out; - } - - if (bond_has_slaves(bond)) { - pr_err("unable to update mode of %s because it has slaves.\n", - bond->dev->name); - ret = -EPERM; - goto out; - } - new_value = bond_parse_parm(buf, bond_mode_tbl); if (new_value < 0) { pr_err("%s: Ignoring invalid mode value %.*s.\n", bond->dev->name, (int)strlen(buf) - 1, buf); - ret = -EINVAL; - goto out; + return -EINVAL; } - if ((new_value == BOND_MODE_ALB || - new_value == BOND_MODE_TLB) && - bond->params.arp_interval) { - pr_err("%s: %s mode is incompatible with arp monitoring.\n", - bond->dev->name, bond_mode_tbl[new_value].modename); - ret = -EINVAL; - goto out; + if (!rtnl_trylock()) + return restart_syscall(); + + ret = bond_option_mode_set(bond, new_value); + if (!ret) { + pr_info("%s: setting mode to %s (%d).\n", + bond->dev->name, bond_mode_tbl[new_value].modename, + new_value); + ret = count; } - /* don't cache arp_validate between modes */ - bond->params.arp_validate = BOND_ARP_VALIDATE_NONE; - bond->params.mode = new_value; - pr_info("%s: setting mode to %s (%d).\n", - bond->dev->name, bond_mode_tbl[new_value].modename, - new_value); -out: rtnl_unlock(); return ret; } diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index a2a353bf9ea6..7446849a20c4 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -58,6 +58,11 @@ #define TX_QUEUE_OVERRIDE(mode) \ (((mode) == BOND_MODE_ACTIVEBACKUP) || \ ((mode) == BOND_MODE_ROUNDROBIN)) + +#define BOND_MODE_IS_LB(mode) \ + (((mode) == BOND_MODE_TLB) || \ + ((mode) == BOND_MODE_ALB)) + /* * Less bad way to call ioctl from within the kernel; this needs to be * done some other way to get the call out of interrupt context. @@ -259,8 +264,7 @@ static inline struct bonding *bond_get_bond_by_slave(struct slave *slave) static inline bool bond_is_lb(const struct bonding *bond) { - return (bond->params.mode == BOND_MODE_TLB || - bond->params.mode == BOND_MODE_ALB); + return BOND_MODE_IS_LB(bond->params.mode); } static inline void bond_set_active_slave(struct slave *slave) @@ -422,6 +426,7 @@ void bond_setup(struct net_device *bond_dev); unsigned int bond_get_num_tx_queues(void); int bond_netlink_init(void); void bond_netlink_fini(void); +int bond_option_mode_set(struct bonding *bond, int mode); struct bond_net { struct net * net; /* Associated network namespace */ -- cgit v1.2.3 From d9e32b21cb394c0816f206539b8c7e9c023db332 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 18 Oct 2013 17:43:35 +0200 Subject: bonding: move active_slave setting into separate function Do a bit of refactoring on the way. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/bonding/bond_options.c | 69 +++++++++++++++++++++++++++++++++++ drivers/net/bonding/bond_sysfs.c | 74 +++++++------------------------------- drivers/net/bonding/bonding.h | 1 + 3 files changed, 83 insertions(+), 61 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c index 294b7660b054..09af5d10d43a 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c @@ -12,6 +12,9 @@ #include #include +#include +#include +#include #include "bonding.h" static bool bond_mode_is_valid(int mode) @@ -53,3 +56,69 @@ int bond_option_mode_set(struct bonding *bond, int mode) bond->params.mode = mode; return 0; } + +int bond_option_active_slave_set(struct bonding *bond, + struct net_device *slave_dev) +{ + int ret = 0; + + if (slave_dev) { + if (!netif_is_bond_slave(slave_dev)) { + pr_err("Device %s is not bonding slave.\n", + slave_dev->name); + return -EINVAL; + } + + if (bond->dev != netdev_master_upper_dev_get(slave_dev)) { + pr_err("%s: Device %s is not our slave.\n", + bond->dev->name, slave_dev->name); + return -EINVAL; + } + } + + if (!USES_PRIMARY(bond->params.mode)) { + pr_err("%s: Unable to change active slave; %s is in mode %d\n", + bond->dev->name, bond->dev->name, bond->params.mode); + return -EINVAL; + } + + block_netpoll_tx(); + read_lock(&bond->lock); + write_lock_bh(&bond->curr_slave_lock); + + /* check to see if we are clearing active */ + if (!slave_dev) { + pr_info("%s: Clearing current active slave.\n", + bond->dev->name); + rcu_assign_pointer(bond->curr_active_slave, NULL); + bond_select_active_slave(bond); + } else { + struct slave *old_active = bond->curr_active_slave; + struct slave *new_active = bond_slave_get_rtnl(slave_dev); + + BUG_ON(!new_active); + + if (new_active == old_active) { + /* do nothing */ + pr_info("%s: %s is already the current active slave.\n", + bond->dev->name, new_active->dev->name); + } else { + if (old_active && (new_active->link == BOND_LINK_UP) && + IS_UP(new_active->dev)) { + pr_info("%s: Setting %s as active slave.\n", + bond->dev->name, new_active->dev->name); + bond_change_active_slave(bond, new_active); + } else { + pr_err("%s: Could not set %s as active slave; either %s is down or the link is down.\n", + bond->dev->name, new_active->dev->name, + new_active->dev->name); + ret = -EINVAL; + } + } + } + + write_unlock_bh(&bond->curr_slave_lock); + read_unlock(&bond->lock); + unblock_netpoll_tx(); + return ret; +} diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index c234cec10e05..abd260047103 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -1235,81 +1235,33 @@ static ssize_t bonding_store_active_slave(struct device *d, struct device_attribute *attr, const char *buf, size_t count) { - struct slave *slave, *old_active, *new_active; + int ret; struct bonding *bond = to_bond(d); - struct list_head *iter; char ifname[IFNAMSIZ]; + struct net_device *dev; if (!rtnl_trylock()) return restart_syscall(); - old_active = new_active = NULL; - block_netpoll_tx(); - read_lock(&bond->lock); - write_lock_bh(&bond->curr_slave_lock); - - if (!USES_PRIMARY(bond->params.mode)) { - pr_info("%s: Unable to change active slave; %s is in mode %d\n", - bond->dev->name, bond->dev->name, bond->params.mode); - goto out; - } - sscanf(buf, "%15s", ifname); /* IFNAMSIZ */ - - /* check to see if we are clearing active */ if (!strlen(ifname) || buf[0] == '\n') { - pr_info("%s: Clearing current active slave.\n", - bond->dev->name); - rcu_assign_pointer(bond->curr_active_slave, NULL); - bond_select_active_slave(bond); - goto out; - } - - bond_for_each_slave(bond, slave, iter) { - if (strncmp(slave->dev->name, ifname, IFNAMSIZ) == 0) { - old_active = bond->curr_active_slave; - new_active = slave; - if (new_active == old_active) { - /* do nothing */ - pr_info("%s: %s is already the current" - " active slave.\n", - bond->dev->name, - slave->dev->name); - goto out; - } else { - if ((new_active) && - (old_active) && - (new_active->link == BOND_LINK_UP) && - IS_UP(new_active->dev)) { - pr_info("%s: Setting %s as active" - " slave.\n", - bond->dev->name, - slave->dev->name); - bond_change_active_slave(bond, - new_active); - } else { - pr_info("%s: Could not set %s as" - " active slave; either %s is" - " down or the link is down.\n", - bond->dev->name, - slave->dev->name, - slave->dev->name); - } - goto out; - } + dev = NULL; + } else { + dev = __dev_get_by_name(dev_net(bond->dev), ifname); + if (!dev) { + ret = -ENODEV; + goto out; } } - pr_info("%s: Unable to set %.*s as active slave.\n", - bond->dev->name, (int)strlen(buf) - 1, buf); - out: - write_unlock_bh(&bond->curr_slave_lock); - read_unlock(&bond->lock); - unblock_netpoll_tx(); + ret = bond_option_active_slave_set(bond, dev); + if (!ret) + ret = count; + out: rtnl_unlock(); - return count; + return ret; } static DEVICE_ATTR(active_slave, S_IRUGO | S_IWUSR, diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 7446849a20c4..686759dce4c6 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -427,6 +427,7 @@ unsigned int bond_get_num_tx_queues(void); int bond_netlink_init(void); void bond_netlink_fini(void); int bond_option_mode_set(struct bonding *bond, int mode); +int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev); struct bond_net { struct net * net; /* Associated network namespace */ -- cgit v1.2.3 From 080a06e1a9a5d2c55884d5fba8755d0af838cd5c Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 18 Oct 2013 17:43:36 +0200 Subject: bonding: remove bond_ioctl_change_active() no longer needed since bond_option_active_slave_set() can be used instead. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 59 ++--------------------------------------- 1 file changed, 2 insertions(+), 57 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index a113e4212486..d90734fca918 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1910,61 +1910,6 @@ static int bond_release_and_destroy(struct net_device *bond_dev, return ret; } -/* - * This function changes the active slave to slave . - * It returns -EINVAL in the following cases. - * - is not found in the list. - * - There is not active slave now. - * - is already active. - * - The link state of is not BOND_LINK_UP. - * - is not running. - * In these cases, this function does nothing. - * In the other cases, current_slave pointer is changed and 0 is returned. - */ -static int bond_ioctl_change_active(struct net_device *bond_dev, struct net_device *slave_dev) -{ - struct bonding *bond = netdev_priv(bond_dev); - struct slave *old_active = NULL; - struct slave *new_active = NULL; - int res = 0; - - if (!USES_PRIMARY(bond->params.mode)) - return -EINVAL; - - /* Verify that bond_dev is indeed the master of slave_dev */ - if (!(slave_dev->flags & IFF_SLAVE) || - !netdev_has_upper_dev(slave_dev, bond_dev)) - return -EINVAL; - - read_lock(&bond->lock); - - old_active = bond->curr_active_slave; - new_active = bond_get_slave_by_dev(bond, slave_dev); - /* - * Changing to the current active: do nothing; return success. - */ - if (new_active && new_active == old_active) { - read_unlock(&bond->lock); - return 0; - } - - if (new_active && - old_active && - new_active->link == BOND_LINK_UP && - IS_UP(new_active->dev)) { - block_netpoll_tx(); - write_lock_bh(&bond->curr_slave_lock); - bond_change_active_slave(bond, new_active); - write_unlock_bh(&bond->curr_slave_lock); - unblock_netpoll_tx(); - } else - res = -EINVAL; - - read_unlock(&bond->lock); - - return res; -} - static int bond_info_query(struct net_device *bond_dev, struct ifbond *info) { struct bonding *bond = netdev_priv(bond_dev); @@ -3257,6 +3202,7 @@ static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev, static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd) { + struct bonding *bond = netdev_priv(bond_dev); struct net_device *slave_dev = NULL; struct ifbond k_binfo; struct ifbond __user *u_binfo = NULL; @@ -3287,7 +3233,6 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd if (mii->reg_num == 1) { - struct bonding *bond = netdev_priv(bond_dev); mii->val_out = 0; read_lock(&bond->lock); read_lock(&bond->curr_slave_lock); @@ -3359,7 +3304,7 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd break; case BOND_CHANGE_ACTIVE_OLD: case SIOCBONDCHANGEACTIVE: - res = bond_ioctl_change_active(bond_dev, slave_dev); + res = bond_option_active_slave_set(bond, slave_dev); break; default: res = -EOPNOTSUPP; -- cgit v1.2.3 From 752d48b52ec929c6fd6ccd7ea9728571830fdd49 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 18 Oct 2013 17:43:37 +0200 Subject: bonding: move active_slave getting into separate function Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/bonding/bond_options.c | 18 ++++++++++++++++++ drivers/net/bonding/bond_sysfs.c | 8 ++++---- drivers/net/bonding/bonding.h | 2 ++ 3 files changed, 24 insertions(+), 4 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c index 09af5d10d43a..9a5223c7b4d1 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c @@ -57,6 +57,24 @@ int bond_option_mode_set(struct bonding *bond, int mode) return 0; } +static struct net_device *__bond_option_active_slave_get(struct bonding *bond, + struct slave *slave) +{ + return USES_PRIMARY(bond->params.mode) && slave ? slave->dev : NULL; +} + +struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond) +{ + struct slave *slave = rcu_dereference(bond->curr_active_slave); + + return __bond_option_active_slave_get(bond, slave); +} + +struct net_device *bond_option_active_slave_get(struct bonding *bond) +{ + return __bond_option_active_slave_get(bond, bond->curr_active_slave); +} + int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev) { diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index abd260047103..47749c970a01 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -1219,13 +1219,13 @@ static ssize_t bonding_show_active_slave(struct device *d, char *buf) { struct bonding *bond = to_bond(d); - struct slave *curr; + struct net_device *slave_dev; int count = 0; rcu_read_lock(); - curr = rcu_dereference(bond->curr_active_slave); - if (USES_PRIMARY(bond->params.mode) && curr) - count = sprintf(buf, "%s\n", curr->dev->name); + slave_dev = bond_option_active_slave_get_rcu(bond); + if (slave_dev) + count = sprintf(buf, "%s\n", slave_dev->name); rcu_read_unlock(); return count; diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 686759dce4c6..046a60535e04 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -428,6 +428,8 @@ int bond_netlink_init(void); void bond_netlink_fini(void); int bond_option_mode_set(struct bonding *bond, int mode); int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev); +struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond); +struct net_device *bond_option_active_slave_get(struct bonding *bond); struct bond_net { struct net * net; /* Associated network namespace */ -- cgit v1.2.3 From 90af231106c0b8d223c27d35464af95cb3d9cacf Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 18 Oct 2013 17:43:38 +0200 Subject: bonding: add Netlink support mode option Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/bonding/bond_netlink.c | 56 ++++++++++++++++++++++++++++++++++++++ include/uapi/linux/if_link.h | 10 +++++++ 2 files changed, 66 insertions(+) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c index 3e5c5f80c320..a94f870a6b60 100644 --- a/drivers/net/bonding/bond_netlink.c +++ b/drivers/net/bonding/bond_netlink.c @@ -20,6 +20,10 @@ #include #include "bonding.h" +static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = { + [IFLA_BOND_MODE] = { .type = NLA_U8 }, +}; + static int bond_validate(struct nlattr *tb[], struct nlattr *data[]) { if (tb[IFLA_ADDRESS]) { @@ -31,11 +35,63 @@ static int bond_validate(struct nlattr *tb[], struct nlattr *data[]) return 0; } +static int bond_changelink(struct net_device *bond_dev, + struct nlattr *tb[], struct nlattr *data[]) +{ + struct bonding *bond = netdev_priv(bond_dev); + int err; + + if (data && data[IFLA_BOND_MODE]) { + int mode = nla_get_u8(data[IFLA_BOND_MODE]); + + err = bond_option_mode_set(bond, mode); + if (err) + return err; + } + return 0; +} + +static int bond_newlink(struct net *src_net, struct net_device *bond_dev, + struct nlattr *tb[], struct nlattr *data[]) +{ + int err; + + err = bond_changelink(bond_dev, tb, data); + if (err < 0) + return err; + + return register_netdevice(bond_dev); +} + +static size_t bond_get_size(const struct net_device *bond_dev) +{ + return nla_total_size(sizeof(u8)); /* IFLA_BOND_MODE */ +} + +static int bond_fill_info(struct sk_buff *skb, + const struct net_device *bond_dev) +{ + struct bonding *bond = netdev_priv(bond_dev); + + if (nla_put_u8(skb, IFLA_BOND_MODE, bond->params.mode)) + goto nla_put_failure; + return 0; + +nla_put_failure: + return -EMSGSIZE; +} + struct rtnl_link_ops bond_link_ops __read_mostly = { .kind = "bond", .priv_size = sizeof(struct bonding), .setup = bond_setup, + .maxtype = IFLA_BOND_MAX, + .policy = bond_policy, .validate = bond_validate, + .newlink = bond_newlink, + .changelink = bond_changelink, + .get_size = bond_get_size, + .fill_info = bond_fill_info, .get_num_tx_queues = bond_get_num_tx_queues, .get_num_rx_queues = bond_get_num_tx_queues, /* Use the same number as for TX queues */ diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 80394e8dc3a3..06fd3fe10f3b 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -325,6 +325,16 @@ struct ifla_vxlan_port_range { __be16 high; }; +/* Bonding section */ + +enum { + IFLA_BOND_UNSPEC, + IFLA_BOND_MODE, + __IFLA_BOND_MAX, +}; + +#define IFLA_BOND_MAX (__IFLA_BOND_MAX - 1) + /* SR-IOV virtual function management section */ enum { -- cgit v1.2.3 From ec76aa49855f6d6fea5e01de179fb57dd47c619d Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 18 Oct 2013 17:43:39 +0200 Subject: bonding: add Netlink support active_slave option Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/bonding/bond_netlink.c | 23 ++++++++++++++++++++++- include/uapi/linux/if_link.h | 1 + 2 files changed, 23 insertions(+), 1 deletion(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c index a94f870a6b60..fe3500bb34e4 100644 --- a/drivers/net/bonding/bond_netlink.c +++ b/drivers/net/bonding/bond_netlink.c @@ -22,6 +22,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = { [IFLA_BOND_MODE] = { .type = NLA_U8 }, + [IFLA_BOND_ACTIVE_SLAVE] = { .type = NLA_U32 }, }; static int bond_validate(struct nlattr *tb[], struct nlattr *data[]) @@ -48,6 +49,22 @@ static int bond_changelink(struct net_device *bond_dev, if (err) return err; } + if (data && data[IFLA_BOND_ACTIVE_SLAVE]) { + int ifindex = nla_get_u32(data[IFLA_BOND_ACTIVE_SLAVE]); + struct net_device *slave_dev; + + if (ifindex == 0) { + slave_dev = NULL; + } else { + slave_dev = __dev_get_by_index(dev_net(bond_dev), + ifindex); + if (!slave_dev) + return -ENODEV; + } + err = bond_option_active_slave_set(bond, slave_dev); + if (err) + return err; + } return 0; } @@ -66,14 +83,18 @@ static int bond_newlink(struct net *src_net, struct net_device *bond_dev, static size_t bond_get_size(const struct net_device *bond_dev) { return nla_total_size(sizeof(u8)); /* IFLA_BOND_MODE */ + + nla_total_size(sizeof(u32)); /* IFLA_BOND_ACTIVE_SLAVE */ } static int bond_fill_info(struct sk_buff *skb, const struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); + struct net_device *slave_dev = bond_option_active_slave_get(bond); - if (nla_put_u8(skb, IFLA_BOND_MODE, bond->params.mode)) + if (nla_put_u8(skb, IFLA_BOND_MODE, bond->params.mode) || + (slave_dev && + nla_put_u32(skb, IFLA_BOND_ACTIVE_SLAVE, slave_dev->ifindex))) goto nla_put_failure; return 0; diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 06fd3fe10f3b..8a1e346243b7 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -330,6 +330,7 @@ struct ifla_vxlan_port_range { enum { IFLA_BOND_UNSPEC, IFLA_BOND_MODE, + IFLA_BOND_ACTIVE_SLAVE, __IFLA_BOND_MAX, }; -- cgit v1.2.3 From a729e83ad6f6de2cf11c0da4a5a7c0b3924d8335 Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Sat, 19 Oct 2013 19:09:18 -0400 Subject: bonding: Remove __exit tag from bond_netlink_fini(). It can be called from the module init function, so it cannot be in the exit section. Signed-off-by: David S. Miller --- drivers/net/bonding/bond_netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c index fe3500bb34e4..7661261de2f0 100644 --- a/drivers/net/bonding/bond_netlink.c +++ b/drivers/net/bonding/bond_netlink.c @@ -123,7 +123,7 @@ int __init bond_netlink_init(void) return rtnl_link_register(&bond_link_ops); } -void __exit bond_netlink_fini(void) +void bond_netlink_fini(void) { rtnl_link_unregister(&bond_link_ops); } -- cgit v1.2.3 From 5378c2e6ea236de847a39bdb6f3aa83137120d26 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Mon, 21 Oct 2013 11:48:30 +0200 Subject: bonding: move bond-specific init after enslave happens As Jiri noted, currently we first do all bonding-specific initialization (specifically - bond_select_active_slave(bond)) before we actually attach the slave (so that it becomes visible through bond_for_each_slave() and friends). This might result in bond_select_active_slave() not seeing the first/new slave and, thus, not actually selecting an active slave. Fix this by moving all the bond-related init part after we've actually completely initialized and linked (via bond_master_upper_dev_link()) the new slave. Also, remove the bond_(de/a)ttach_slave(), it's useless to have functions to ++/-- one int. After this we have all the initialization of the new slave *before* linking, and all the stuff that needs to be done on bonding *after* it. It has also a bonus effect - we can remove the locking on the new slave init completely, and only use it for bond_select_active_slave(). Reported-by: Jiri Pirko CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Acked-by: Ding Tianhong@huawei.com Reviewed-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 65 +++++++++-------------------------------- 1 file changed, 14 insertions(+), 51 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index d90734fca918..2daa066c6cdd 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -967,33 +967,6 @@ void bond_select_active_slave(struct bonding *bond) } } -/*--------------------------- slave list handling ---------------------------*/ - -/* - * This function attaches the slave to the end of list. - * - * bond->lock held for writing by caller. - */ -static void bond_attach_slave(struct bonding *bond, struct slave *new_slave) -{ - bond->slave_cnt++; -} - -/* - * This function detaches the slave from the list. - * WARNING: no check is made to verify if the slave effectively - * belongs to . - * Nothing is freed on return, structures are just unchained. - * If any slave pointer in bond was pointing to , - * it should be changed by the calling function. - * - * bond->lock held for writing by caller. - */ -static void bond_detach_slave(struct bonding *bond, struct slave *slave) -{ - bond->slave_cnt--; -} - #ifdef CONFIG_NET_POLL_CONTROLLER static inline int slave_enable_netpoll(struct slave *slave) { @@ -1471,22 +1444,13 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) goto err_close; } - write_lock_bh(&bond->lock); - prev_slave = bond_last_slave(bond); - bond_attach_slave(bond, new_slave); new_slave->delay = 0; new_slave->link_failure_count = 0; - write_unlock_bh(&bond->lock); - - bond_compute_features(bond); - bond_update_speed_duplex(new_slave); - read_lock(&bond->lock); - new_slave->last_arp_rx = jiffies - (msecs_to_jiffies(bond->params.arp_interval) + 1); for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) @@ -1547,12 +1511,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) } } - write_lock_bh(&bond->curr_slave_lock); - switch (bond->params.mode) { case BOND_MODE_ACTIVEBACKUP: bond_set_slave_inactive_flags(new_slave); - bond_select_active_slave(bond); break; case BOND_MODE_8023AD: /* in 802.3ad mode, the internal mechanism @@ -1578,7 +1539,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) case BOND_MODE_ALB: bond_set_active_slave(new_slave); bond_set_slave_inactive_flags(new_slave); - bond_select_active_slave(bond); break; default: pr_debug("This slave is always active in trunk mode\n"); @@ -1596,10 +1556,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) break; } /* switch(bond_mode) */ - write_unlock_bh(&bond->curr_slave_lock); - - bond_set_carrier(bond); - #ifdef CONFIG_NET_POLL_CONTROLLER slave_dev->npinfo = bond->dev->npinfo; if (slave_dev->npinfo) { @@ -1614,8 +1570,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) } #endif - read_unlock(&bond->lock); - res = netdev_rx_handler_register(slave_dev, bond_handle_frame, new_slave); if (res) { @@ -1629,6 +1583,17 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) goto err_unregister; } + bond->slave_cnt++; + bond_compute_features(bond); + 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", bond_dev->name, slave_dev->name, @@ -1648,7 +1613,6 @@ err_detach: vlan_vids_del_by_dev(slave_dev, bond_dev); write_lock_bh(&bond->lock); - bond_detach_slave(bond, new_slave); if (bond->primary_slave == new_slave) bond->primary_slave = NULL; if (bond->curr_active_slave == new_slave) { @@ -1686,7 +1650,6 @@ err_free: kfree(new_slave); err_undo_flags: - bond_compute_features(bond); /* Enslave of first slave has failed and we need to fix master's mac */ if (!bond_has_slaves(bond) && ether_addr_equal(bond_dev->dev_addr, slave_dev->dev_addr)) @@ -1740,6 +1703,9 @@ static int __bond_release_one(struct net_device *bond_dev, write_unlock_bh(&bond->lock); + /* release the slave from its bond */ + bond->slave_cnt--; + bond_upper_dev_unlink(bond_dev, slave_dev); /* unregister rx_handler early so bond_handle_frame wouldn't be called * for this slave anymore. @@ -1764,9 +1730,6 @@ static int __bond_release_one(struct net_device *bond_dev, bond->current_arp_slave = NULL; - /* release the slave from its bond */ - bond_detach_slave(bond, slave); - if (!all && !bond->params.fail_over_mac) { if (ether_addr_equal(bond_dev->dev_addr, slave->perm_hwaddr) && bond_has_slaves(bond)) -- cgit v1.2.3 From 7f29405403d7c17f539c099987972b862e7e5255 Mon Sep 17 00:00:00 2001 From: Alexei Starovoitov Date: Wed, 23 Oct 2013 16:02:42 -0700 Subject: net: fix rtnl notification in atomic context commit 991fb3f74c "dev: always advertise rx_flags changes via netlink" introduced rtnl notification from __dev_set_promiscuity(), which can be called in atomic context. Steps to reproduce: ip tuntap add dev tap1 mode tap ifconfig tap1 up tcpdump -nei tap1 & ip tuntap del dev tap1 mode tap [ 271.627994] device tap1 left promiscuous mode [ 271.639897] BUG: sleeping function called from invalid context at mm/slub.c:940 [ 271.664491] in_atomic(): 1, irqs_disabled(): 0, pid: 3394, name: ip [ 271.677525] INFO: lockdep is turned off. [ 271.690503] CPU: 0 PID: 3394 Comm: ip Tainted: G W 3.12.0-rc3+ #73 [ 271.703996] Hardware name: System manufacturer System Product Name/P8Z77 WS, BIOS 3007 07/26/2012 [ 271.731254] ffffffff81a58506 ffff8807f0d57a58 ffffffff817544e5 ffff88082fa0f428 [ 271.760261] ffff8808071f5f40 ffff8807f0d57a88 ffffffff8108bad1 ffffffff81110ff8 [ 271.790683] 0000000000000010 00000000000000d0 00000000000000d0 ffff8807f0d57af8 [ 271.822332] Call Trace: [ 271.838234] [] dump_stack+0x55/0x76 [ 271.854446] [] __might_sleep+0x181/0x240 [ 271.870836] [] ? rcu_irq_exit+0x68/0xb0 [ 271.887076] [] kmem_cache_alloc_node+0x4e/0x2a0 [ 271.903368] [] ? vprintk_emit+0x1dc/0x5a0 [ 271.919716] [] ? __alloc_skb+0x57/0x2a0 [ 271.936088] [] ? vprintk_emit+0x1e0/0x5a0 [ 271.952504] [] __alloc_skb+0x57/0x2a0 [ 271.968902] [] rtmsg_ifinfo+0x52/0x100 [ 271.985302] [] __dev_notify_flags+0xad/0xc0 [ 272.001642] [] __dev_set_promiscuity+0x8c/0x1c0 [ 272.017917] [] ? packet_notifier+0x5/0x380 [ 272.033961] [] dev_set_promiscuity+0x29/0x50 [ 272.049855] [] packet_dev_mc+0x87/0xc0 [ 272.065494] [] packet_notifier+0x1b2/0x380 [ 272.080915] [] ? packet_notifier+0x5/0x380 [ 272.096009] [] notifier_call_chain+0x66/0x150 [ 272.110803] [] __raw_notifier_call_chain+0xe/0x10 [ 272.125468] [] raw_notifier_call_chain+0x16/0x20 [ 272.139984] [] call_netdevice_notifiers_info+0x40/0x70 [ 272.154523] [] call_netdevice_notifiers+0x16/0x20 [ 272.168552] [] rollback_registered_many+0x145/0x240 [ 272.182263] [] rollback_registered+0x31/0x40 [ 272.195369] [] unregister_netdevice_queue+0x58/0x90 [ 272.208230] [] __tun_detach+0x140/0x340 [ 272.220686] [] tun_chr_close+0x36/0x60 Signed-off-by: Alexei Starovoitov Acked-by: Nicolas Dichtel Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 4 ++-- include/linux/rtnetlink.h | 2 +- net/core/dev.c | 16 ++++++++-------- net/core/rtnetlink.c | 9 +++++---- 4 files changed, 16 insertions(+), 15 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 2daa066c6cdd..a141f406cb98 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1213,7 +1213,7 @@ static int bond_master_upper_dev_link(struct net_device *bond_dev, if (err) return err; slave_dev->flags |= IFF_SLAVE; - rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE); + rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL); return 0; } @@ -1222,7 +1222,7 @@ static void bond_upper_dev_unlink(struct net_device *bond_dev, { netdev_upper_dev_unlink(slave_dev, bond_dev); slave_dev->flags &= ~IFF_SLAVE; - rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE); + rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL); } /* enslave device to bond device */ diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h index f28544b2f9af..939428ad25ac 100644 --- a/include/linux/rtnetlink.h +++ b/include/linux/rtnetlink.h @@ -15,7 +15,7 @@ extern int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics); extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst, u32 id, long expires, u32 error); -extern void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change); +void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change, gfp_t flags); /* RTNL is used as a global lock for all changes to network configuration */ extern void rtnl_lock(void); diff --git a/net/core/dev.c b/net/core/dev.c index bdffd654edc4..0054c8c75f50 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1203,7 +1203,7 @@ void netdev_state_change(struct net_device *dev) { if (dev->flags & IFF_UP) { call_netdevice_notifiers(NETDEV_CHANGE, dev); - rtmsg_ifinfo(RTM_NEWLINK, dev, 0); + rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL); } } EXPORT_SYMBOL(netdev_state_change); @@ -1293,7 +1293,7 @@ int dev_open(struct net_device *dev) if (ret < 0) return ret; - rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING); + rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING, GFP_KERNEL); call_netdevice_notifiers(NETDEV_UP, dev); return ret; @@ -1371,7 +1371,7 @@ static int dev_close_many(struct list_head *head) __dev_close_many(head); list_for_each_entry_safe(dev, tmp, head, close_list) { - rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING); + rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING, GFP_KERNEL); call_netdevice_notifiers(NETDEV_DOWN, dev); list_del_init(&dev->close_list); } @@ -5258,7 +5258,7 @@ void __dev_notify_flags(struct net_device *dev, unsigned int old_flags, unsigned int changes = dev->flags ^ old_flags; if (gchanges) - rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges); + rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges, GFP_ATOMIC); if (changes & IFF_UP) { if (dev->flags & IFF_UP) @@ -5490,7 +5490,7 @@ static void rollback_registered_many(struct list_head *head) if (!dev->rtnl_link_ops || dev->rtnl_link_state == RTNL_LINK_INITIALIZED) - rtmsg_ifinfo(RTM_DELLINK, dev, ~0U); + rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL); /* * Flush the unicast and multicast chains @@ -5889,7 +5889,7 @@ int register_netdevice(struct net_device *dev) */ if (!dev->rtnl_link_ops || dev->rtnl_link_state == RTNL_LINK_INITIALIZED) - rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U); + rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL); out: return ret; @@ -6501,7 +6501,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char call_netdevice_notifiers(NETDEV_UNREGISTER, dev); rcu_barrier(); call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev); - rtmsg_ifinfo(RTM_DELLINK, dev, ~0U); + rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL); /* * Flush the unicast and multicast chains @@ -6540,7 +6540,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char * Prevent userspace races by waiting until the network * device is fully setup before sending notifications. */ - rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U); + rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL); synchronize_net(); err = 0; diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 4aedf03da052..cf67144d3e3c 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -1984,14 +1984,15 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb) return skb->len; } -void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change) +void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change, + gfp_t flags) { struct net *net = dev_net(dev); struct sk_buff *skb; int err = -ENOBUFS; size_t if_info_size; - skb = nlmsg_new((if_info_size = if_nlmsg_size(dev, 0)), GFP_KERNEL); + skb = nlmsg_new((if_info_size = if_nlmsg_size(dev, 0)), flags); if (skb == NULL) goto errout; @@ -2002,7 +2003,7 @@ void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change) kfree_skb(skb); goto errout; } - rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, GFP_KERNEL); + rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags); return; errout: if (err < 0) @@ -2716,7 +2717,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi case NETDEV_JOIN: break; default: - rtmsg_ifinfo(RTM_NEWLINK, dev, 0); + rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL); break; } return NOTIFY_DONE; -- cgit v1.2.3 From 6b6c526147bb00b5788a2f48463481dd30c29b71 Mon Sep 17 00:00:00 2001 From: dingtianhong Date: Thu, 24 Oct 2013 11:09:03 +0800 Subject: bonding: remove bond read lock for bond_mii_monitor() The bond slave list may change when the monitor is running, the slave list is no longer protected by bond->lock, only protected by rtnl lock(), so we have 3 ways to modify it: 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 existing rtnl lock(). 3.use rcu_read_lock() to protect it, of course, it will transform bond_for_each_slave to bond_for_each_slave_rcu() and performance is better, but in slow path, it is ignored. so I remove the bond->lock and move the rtnl lock to protect the whole monitor function. Signed-off-by: Ding Tianhong Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 44 +++++++++++------------------------------ 1 file changed, 12 insertions(+), 32 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index a141f406cb98..0a7e32578540 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2118,49 +2118,29 @@ void bond_mii_monitor(struct work_struct *work) struct bonding *bond = container_of(work, struct bonding, mii_work.work); bool should_notify_peers = false; - unsigned long delay; - read_lock(&bond->lock); - - delay = msecs_to_jiffies(bond->params.miimon); + if (!rtnl_trylock()) + goto re_arm; - if (!bond_has_slaves(bond)) + if (!bond_has_slaves(bond)) { + rtnl_unlock(); goto re_arm; + } should_notify_peers = bond_should_notify_peers(bond); - if (bond_miimon_inspect(bond)) { - read_unlock(&bond->lock); - - /* 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); - + if (bond_miimon_inspect(bond)) bond_miimon_commit(bond); - read_unlock(&bond->lock); - rtnl_unlock(); /* might sleep, hold no other locks */ - read_lock(&bond->lock); - } + if (should_notify_peers) + 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(); - } + queue_delayed_work(bond->wq, &bond->mii_work, + msecs_to_jiffies(bond->params.miimon)); } static bool bond_has_this_ip(struct bonding *bond, __be32 ip) -- cgit v1.2.3 From 2d0dafb0152a6ac61cd31d38c3ef3d49463b6a57 Mon Sep 17 00:00:00 2001 From: dingtianhong Date: Thu, 24 Oct 2013 11:09:12 +0800 Subject: bonding: remove bond read lock for bond_alb_monitor() The bond slave list may change when the monitor is running, the slave list is no longer protected by bond->lock, only protected by rtnl lock(), so we have 3 ways to modify it: 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 existing rtnl lock(). 3.use rcu_read_lock() to protect it, of course, it will transform bond_for_each_slave to bond_for_each_slave_rcu() and performance is better, but in slow path, it is ignored. so I remove the bond->lock and move the rtnl lock to protect the whole monitor function. Signed-off-by: Ding Tianhong Signed-off-by: David S. Miller --- drivers/net/bonding/bond_alb.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index 02872405d35d..5d79f5e529e0 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -1495,11 +1495,13 @@ void bond_alb_monitor(struct work_struct *work) struct list_head *iter; struct slave *slave; - read_lock(&bond->lock); + if (!rtnl_trylock()) + goto re_arm; if (!bond_has_slaves(bond)) { bond_info->tx_rebalance_counter = 0; bond_info->lp_counter = 0; + rtnl_unlock(); goto re_arm; } @@ -1548,16 +1550,6 @@ void bond_alb_monitor(struct work_struct *work) if (bond_info->primary_is_promisc && (++bond_info->rlb_promisc_timeout_counter >= RLB_PROMISC_TIMEOUT)) { - /* - * dev_set_promiscuity requires rtnl and - * nothing else. Avoid race with bond_close. - */ - read_unlock(&bond->lock); - if (!rtnl_trylock()) { - read_lock(&bond->lock); - goto re_arm; - } - bond_info->rlb_promisc_timeout_counter = 0; /* If the primary was set to promiscuous mode @@ -1566,9 +1558,6 @@ void bond_alb_monitor(struct work_struct *work) */ dev_set_promiscuity(bond->curr_active_slave->dev, -1); bond_info->primary_is_promisc = 0; - - rtnl_unlock(); - read_lock(&bond->lock); } if (bond_info->rlb_rebalance) { @@ -1591,10 +1580,9 @@ void bond_alb_monitor(struct work_struct *work) } } + rtnl_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 -- cgit v1.2.3 From 7f1bb571b753ac75b6548f0b7c932dfc0bb1f970 Mon Sep 17 00:00:00 2001 From: dingtianhong Date: Thu, 24 Oct 2013 11:09:17 +0800 Subject: bonding: remove bond read lock for bond_loadbalance_arp_mon() The bond slave list may change when the monitor is running, the slave list is no longer protected by bond->lock, only protected by rtnl lock(), so we have 3 ways to modify it: 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 existing rtnl lock(). 3.use rcu_read_lock() to protect it, of course, it will transform bond_for_each_slave to bond_for_each_slave_rcu() and performance is better, but in slow path, it is ignored. so I remove the bond->lock and add the rtnl lock to protect the whole monitor function. Signed-off-by: Ding Tianhong Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 0a7e32578540..a620dfae1c82 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2396,10 +2396,13 @@ void bond_loadbalance_arp_mon(struct work_struct *work) struct list_head *iter; int do_failover = 0; - read_lock(&bond->lock); + if (!rtnl_trylock()) + goto re_arm; - if (!bond_has_slaves(bond)) + if (!bond_has_slaves(bond)) { + rtnl_unlock(); goto re_arm; + } oldcurrent = bond->curr_active_slave; /* see if any of the previous devices are up now (i.e. they have @@ -2481,13 +2484,12 @@ 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); } /* -- cgit v1.2.3 From 80b9d236ec56ecc18da4a43bd79e8ec9ac5036ff Mon Sep 17 00:00:00 2001 From: dingtianhong Date: Thu, 24 Oct 2013 11:09:25 +0800 Subject: bonding: remove bond read lock for bond_activebackup_arp_mon() The bond slave list may change when the monitor is running, the slave list is no longer protected by bond->lock, only protected by rtnl lock(), so we have 3 ways to modify it: 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 existing rtnl lock(). 3.use rcu_read_lock() to protect it, of course, it will transform bond_for_each_slave to bond_for_each_slave_rcu() and performance is better, but in slow path, it is ignored. so I remove the bond->lock and move the rtnl lock to protect the whole monitor function. Signed-off-by: Ding Tianhong Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 46 ++++++++++++----------------------------- 1 file changed, 13 insertions(+), 33 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index a620dfae1c82..535570ea8bbc 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2726,51 +2726,31 @@ void bond_activebackup_arp_mon(struct work_struct *work) struct bonding *bond = container_of(work, struct bonding, arp_work.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 (!rtnl_trylock()) + goto re_arm; - if (!bond_has_slaves(bond)) + if (!bond_has_slaves(bond)) { + rtnl_unlock(); goto re_arm; + } should_notify_peers = bond_should_notify_peers(bond); - if (bond_ab_arp_inspect(bond)) { - read_unlock(&bond->lock); - - /* 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); - + if (bond_ab_arp_inspect(bond)) bond_ab_arp_commit(bond); - read_unlock(&bond->lock); - rtnl_unlock(); - read_lock(&bond->lock); - } - bond_ab_arp_probe(bond); -re_arm: - if (bond->params.arp_interval) - queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks); + if (should_notify_peers) + call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev); - read_unlock(&bond->lock); + rtnl_unlock(); - if (should_notify_peers) { - if (!rtnl_trylock()) - return; - call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev); - rtnl_unlock(); - } +re_arm: + if (bond->params.arp_interval) + queue_delayed_work(bond->wq, &bond->arp_work, + msecs_to_jiffies(bond->params.arp_interval)); } /*-------------------------- netdev event handling --------------------------*/ -- cgit v1.2.3 From 5cc172c6de80cd9bf2a1228cb928b1fb42e30deb Mon Sep 17 00:00:00 2001 From: dingtianhong Date: Thu, 24 Oct 2013 11:09:31 +0800 Subject: bonding: remove bond read lock for bond_3ad_state_machine_handler() The bond slave list may change when the monitor is running, the slave list is no longer protected by bond->lock, only protected by rtnl lock(), so we have 3 ways to modify it: 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 existing rtnl lock(). 3.use rcu_read_lock() to protect it, of course, it will transform bond_for_each_slave to bond_for_each_slave_rcu() and performance is better, but in slow path, it is ignored. so I remove the bond->lock and move the rtnl lock to protect the whole monitor function. Signed-off-by: Ding Tianhong Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index 187b1b7772ef..d6fe00bf4858 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -2068,8 +2068,10 @@ void bond_3ad_state_machine_handler(struct work_struct *work) struct slave *slave; struct port *port; - read_lock(&bond->lock); - + if (!rtnl_trylock()) { + queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks); + return; + } //check if there are any slaves if (!bond_has_slaves(bond)) goto re_arm; @@ -2122,9 +2124,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work) } re_arm: + rtnl_unlock(); queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks); - - read_unlock(&bond->lock); } /** -- cgit v1.2.3 From 1f2cd845d3827412e82bf26dde0abca332ede402 Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Mon, 28 Oct 2013 00:11:22 -0400 Subject: Revert "Merge branch 'bonding_monitor_locking'" This reverts commit 4d961a101e032b4bf223b279b4b35bc77576f5a8, reversing changes made to a00f6fcc7d0c62a91768d9c4ccba4c7d64fbbce3. Revert bond locking changes, they cause regressions and Veaceslav Falico doesn't like how the commit messages were done at all. Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 9 ++-- drivers/net/bonding/bond_alb.c | 20 ++++++-- drivers/net/bonding/bond_main.c | 100 +++++++++++++++++++++++++++------------- 3 files changed, 89 insertions(+), 40 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index d6fe00bf4858..187b1b7772ef 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -2068,10 +2068,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work) struct slave *slave; struct port *port; - if (!rtnl_trylock()) { - queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks); - return; - } + read_lock(&bond->lock); + //check if there are any slaves if (!bond_has_slaves(bond)) goto re_arm; @@ -2124,8 +2122,9 @@ void bond_3ad_state_machine_handler(struct work_struct *work) } re_arm: - rtnl_unlock(); queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks); + + read_unlock(&bond->lock); } /** diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index 5d79f5e529e0..02872405d35d 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -1495,13 +1495,11 @@ void bond_alb_monitor(struct work_struct *work) struct list_head *iter; struct slave *slave; - if (!rtnl_trylock()) - goto re_arm; + read_lock(&bond->lock); if (!bond_has_slaves(bond)) { bond_info->tx_rebalance_counter = 0; bond_info->lp_counter = 0; - rtnl_unlock(); goto re_arm; } @@ -1550,6 +1548,16 @@ void bond_alb_monitor(struct work_struct *work) if (bond_info->primary_is_promisc && (++bond_info->rlb_promisc_timeout_counter >= RLB_PROMISC_TIMEOUT)) { + /* + * dev_set_promiscuity requires rtnl and + * nothing else. Avoid race with bond_close. + */ + read_unlock(&bond->lock); + if (!rtnl_trylock()) { + read_lock(&bond->lock); + goto re_arm; + } + bond_info->rlb_promisc_timeout_counter = 0; /* If the primary was set to promiscuous mode @@ -1558,6 +1566,9 @@ void bond_alb_monitor(struct work_struct *work) */ dev_set_promiscuity(bond->curr_active_slave->dev, -1); bond_info->primary_is_promisc = 0; + + rtnl_unlock(); + read_lock(&bond->lock); } if (bond_info->rlb_rebalance) { @@ -1580,9 +1591,10 @@ void bond_alb_monitor(struct work_struct *work) } } - rtnl_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 diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 535570ea8bbc..a141f406cb98 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2118,29 +2118,49 @@ void bond_mii_monitor(struct work_struct *work) struct bonding *bond = container_of(work, struct bonding, mii_work.work); bool should_notify_peers = false; + unsigned long delay; - if (!rtnl_trylock()) - goto re_arm; + read_lock(&bond->lock); - if (!bond_has_slaves(bond)) { - rtnl_unlock(); + delay = msecs_to_jiffies(bond->params.miimon); + + if (!bond_has_slaves(bond)) goto re_arm; - } should_notify_peers = bond_should_notify_peers(bond); - if (bond_miimon_inspect(bond)) - bond_miimon_commit(bond); + if (bond_miimon_inspect(bond)) { + read_unlock(&bond->lock); - if (should_notify_peers) - call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev); + /* Race avoidance with bond_close cancel of workqueue */ + if (!rtnl_trylock()) { + read_lock(&bond->lock); + delay = 1; + should_notify_peers = false; + goto re_arm; + } - rtnl_unlock(); + read_lock(&bond->lock); + + bond_miimon_commit(bond); + + read_unlock(&bond->lock); + rtnl_unlock(); /* might sleep, hold no other locks */ + read_lock(&bond->lock); + } re_arm: if (bond->params.miimon) - queue_delayed_work(bond->wq, &bond->mii_work, - msecs_to_jiffies(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) @@ -2396,13 +2416,10 @@ void bond_loadbalance_arp_mon(struct work_struct *work) struct list_head *iter; int do_failover = 0; - if (!rtnl_trylock()) - goto re_arm; + read_lock(&bond->lock); - if (!bond_has_slaves(bond)) { - rtnl_unlock(); + if (!bond_has_slaves(bond)) goto re_arm; - } oldcurrent = bond->curr_active_slave; /* see if any of the previous devices are up now (i.e. they have @@ -2484,12 +2501,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); } /* @@ -2726,31 +2744,51 @@ void bond_activebackup_arp_mon(struct work_struct *work) struct bonding *bond = container_of(work, struct bonding, arp_work.work); bool should_notify_peers = false; + int delta_in_ticks; - if (!rtnl_trylock()) - goto re_arm; + read_lock(&bond->lock); - if (!bond_has_slaves(bond)) { - rtnl_unlock(); + delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval); + + if (!bond_has_slaves(bond)) goto re_arm; - } should_notify_peers = bond_should_notify_peers(bond); - if (bond_ab_arp_inspect(bond)) - bond_ab_arp_commit(bond); + if (bond_ab_arp_inspect(bond)) { + read_unlock(&bond->lock); - bond_ab_arp_probe(bond); + /* 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; + } - if (should_notify_peers) - call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev); + read_lock(&bond->lock); - rtnl_unlock(); + bond_ab_arp_commit(bond); + + read_unlock(&bond->lock); + rtnl_unlock(); + read_lock(&bond->lock); + } + + bond_ab_arp_probe(bond); re_arm: if (bond->params.arp_interval) - queue_delayed_work(bond->wq, &bond->arp_work, - msecs_to_jiffies(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; + call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev); + rtnl_unlock(); + } } /*-------------------------- netdev event handling --------------------------*/ -- cgit v1.2.3 From e139862eeec985d7139b11b09deeb9a32e3f3af2 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 1 Nov 2013 13:18:44 +0300 Subject: bonding: bond_get_size() returns wrong size There is an extra semi-colon so bond_get_size() doesn't return the correct value. Fixes: ec76aa49855f ('bonding: add Netlink support active_slave option') Signed-off-by: Dan Carpenter Acked-by: Veaceslav Falico Reviewed-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/bonding/bond_netlink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c index 7661261de2f0..40e7b1cb4aea 100644 --- a/drivers/net/bonding/bond_netlink.c +++ b/drivers/net/bonding/bond_netlink.c @@ -82,8 +82,8 @@ static int bond_newlink(struct net *src_net, struct net_device *bond_dev, static size_t bond_get_size(const struct net_device *bond_dev) { - return nla_total_size(sizeof(u8)); /* IFLA_BOND_MODE */ - + nla_total_size(sizeof(u32)); /* IFLA_BOND_ACTIVE_SLAVE */ + return nla_total_size(sizeof(u8)) + /* IFLA_BOND_MODE */ + nla_total_size(sizeof(u32)); /* IFLA_BOND_ACTIVE_SLAVE */ } static int bond_fill_info(struct sk_buff *skb, -- cgit v1.2.3 From 73958329ea1fe0dc149b51e5d8703015f65a03e0 Mon Sep 17 00:00:00 2001 From: Nikolay Aleksandrov Date: Tue, 5 Nov 2013 13:51:41 +0100 Subject: bonding: extend round-robin mode with packets_per_slave This patch aims to extend round-robin mode with a new option called packets_per_slave which can have the following values and effects: 0 - choose a random slave 1 (default) - standard round-robin, 1 packet per slave >1 - round-robin when >1 packets have been transmitted per slave The allowed values are between 0 and 65535. This patch also fixes the comment style in bond_xmit_roundrobin(). Signed-off-by: Nikolay Aleksandrov Acked-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 55 ++++++++++++++++++++++++++++++++++++---- drivers/net/bonding/bond_sysfs.c | 49 +++++++++++++++++++++++++++++++++++ drivers/net/bonding/bonding.h | 3 ++- 3 files changed, 101 insertions(+), 6 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index a141f406cb98..4dd5ee2a34cc 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -79,6 +79,7 @@ #include #include #include +#include #include "bonding.h" #include "bond_3ad.h" #include "bond_alb.h" @@ -111,6 +112,7 @@ static char *fail_over_mac; static int all_slaves_active; static struct bond_params bonding_defaults; static int resend_igmp = BOND_DEFAULT_RESEND_IGMP; +static int packets_per_slave = 1; module_param(max_bonds, int, 0); MODULE_PARM_DESC(max_bonds, "Max number of bonded devices"); @@ -183,6 +185,10 @@ MODULE_PARM_DESC(all_slaves_active, "Keep all frames received on an interface" module_param(resend_igmp, int, 0); MODULE_PARM_DESC(resend_igmp, "Number of IGMP membership reports to send on " "link failure"); +module_param(packets_per_slave, int, 0); +MODULE_PARM_DESC(packets_per_slave, "Packets to send per slave in balance-rr " + "mode; 0 for a random slave, 1 packet per " + "slave (default), >1 packets per slave."); /*----------------------------- Global variables ----------------------------*/ @@ -3574,14 +3580,44 @@ void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id) kfree_skb(skb); } +/** + * bond_rr_gen_slave_id - generate slave id based on packets_per_slave + * @bond: bonding device to use + * + * Based on the value of the bonding device's packets_per_slave parameter + * this function generates a slave id, which is usually used as the next + * slave to transmit through. + */ +static u32 bond_rr_gen_slave_id(struct bonding *bond) +{ + int packets_per_slave = bond->params.packets_per_slave; + u32 slave_id; + + switch (packets_per_slave) { + case 0: + slave_id = prandom_u32(); + break; + case 1: + slave_id = bond->rr_tx_counter; + break; + default: + slave_id = reciprocal_divide(bond->rr_tx_counter, + packets_per_slave); + break; + } + bond->rr_tx_counter++; + + return slave_id; +} + static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); struct iphdr *iph = ip_hdr(skb); struct slave *slave; + u32 slave_id; - /* - * Start with the curr_active_slave that joined the bond as the + /* Start with the curr_active_slave that joined the bond as the * default for sending IGMP traffic. For failover purposes one * needs to maintain some consistency for the interface that will * send the join/membership reports. The curr_active_slave found @@ -3594,8 +3630,8 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev else bond_xmit_slave_id(bond, skb, 0); } else { - bond_xmit_slave_id(bond, skb, - bond->rr_tx_counter++ % bond->slave_cnt); + slave_id = bond_rr_gen_slave_id(bond); + bond_xmit_slave_id(bond, skb, slave_id % bond->slave_cnt); } return NETDEV_TX_OK; @@ -4099,6 +4135,12 @@ static int bond_check_params(struct bond_params *params) resend_igmp = BOND_DEFAULT_RESEND_IGMP; } + if (packets_per_slave < 0 || packets_per_slave > USHRT_MAX) { + pr_warn("Warning: packets_per_slave (%d) should be between 0 and %u resetting to 1\n", + packets_per_slave, USHRT_MAX); + packets_per_slave = 1; + } + /* reset values for TLB/ALB */ if ((bond_mode == BOND_MODE_TLB) || (bond_mode == BOND_MODE_ALB)) { @@ -4288,7 +4330,10 @@ static int bond_check_params(struct bond_params *params) params->resend_igmp = resend_igmp; params->min_links = min_links; params->lp_interval = BOND_ALB_DEFAULT_LP_INTERVAL; - + if (packets_per_slave > 1) + params->packets_per_slave = reciprocal_value(packets_per_slave); + else + params->packets_per_slave = packets_per_slave; if (primary) { strncpy(params->primary, primary, IFNAMSIZ); params->primary[IFNAMSIZ - 1] = 0; diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index 47749c970a01..75dc4d0efb34 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -40,6 +40,7 @@ #include #include #include +#include #include "bonding.h" @@ -1640,6 +1641,53 @@ out: static DEVICE_ATTR(lp_interval, S_IRUGO | S_IWUSR, bonding_show_lp_interval, bonding_store_lp_interval); +static ssize_t bonding_show_packets_per_slave(struct device *d, + struct device_attribute *attr, + char *buf) +{ + struct bonding *bond = to_bond(d); + int packets_per_slave = bond->params.packets_per_slave; + + if (packets_per_slave > 1) + packets_per_slave = reciprocal_value(packets_per_slave); + + return sprintf(buf, "%d\n", packets_per_slave); +} + +static ssize_t bonding_store_packets_per_slave(struct device *d, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct bonding *bond = to_bond(d); + int new_value, ret = count; + + if (sscanf(buf, "%d", &new_value) != 1) { + pr_err("%s: no packets_per_slave value specified.\n", + bond->dev->name); + ret = -EINVAL; + goto out; + } + if (new_value < 0 || new_value > USHRT_MAX) { + pr_err("%s: packets_per_slave must be between 0 and %u\n", + bond->dev->name, USHRT_MAX); + ret = -EINVAL; + goto out; + } + if (bond->params.mode != BOND_MODE_ROUNDROBIN) + pr_warn("%s: Warning: packets_per_slave has effect only in balance-rr mode\n", + bond->dev->name); + if (new_value > 1) + bond->params.packets_per_slave = reciprocal_value(new_value); + else + bond->params.packets_per_slave = new_value; +out: + return ret; +} + +static DEVICE_ATTR(packets_per_slave, S_IRUGO | S_IWUSR, + bonding_show_packets_per_slave, + bonding_store_packets_per_slave); + static struct attribute *per_bond_attrs[] = { &dev_attr_slaves.attr, &dev_attr_mode.attr, @@ -1671,6 +1719,7 @@ static struct attribute *per_bond_attrs[] = { &dev_attr_resend_igmp.attr, &dev_attr_min_links.attr, &dev_attr_lp_interval.attr, + &dev_attr_packets_per_slave.attr, NULL, }; diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 046a60535e04..77a07a12e77f 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -156,6 +156,7 @@ struct bond_params { int all_slaves_active; int resend_igmp; int lp_interval; + int packets_per_slave; }; struct bond_parm_tbl { @@ -222,7 +223,7 @@ struct bonding { char proc_file_name[IFNAMSIZ]; #endif /* CONFIG_PROC_FS */ struct list_head bond_list; - u16 rr_tx_counter; + u32 rr_tx_counter; struct ad_bond_info ad_info; struct alb_bond_info alb_info; struct bond_params params; -- cgit v1.2.3 From ec9f1d15db8185f63a2c3143dc1e90ba18541b08 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Tue, 12 Nov 2013 15:37:40 +0100 Subject: bonding: don't permit to use ARP monitoring in 802.3ad mode Currently the ARP monitoring is not supported with 802.3ad, and it's prohibited to use it via the module params. However we still can set it afterwards via sysfs, cause we only check for *LB modes there. To fix this - add a check for 802.3ad mode in bonding_store_arp_interval. CC: Jay Vosburgh CC: Andy Gospodarek Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_sysfs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index bc8fd362a5aa..6245d92b7a0c 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -524,8 +524,9 @@ static ssize_t bonding_store_arp_interval(struct device *d, goto out; } if (bond->params.mode == BOND_MODE_ALB || - bond->params.mode == BOND_MODE_TLB) { - pr_info("%s: ARP monitoring cannot be used with ALB/TLB. Only MII monitoring is supported on %s.\n", + bond->params.mode == BOND_MODE_TLB || + bond->params.mode == BOND_MODE_8023AD) { + pr_info("%s: ARP monitoring cannot be used with ALB/TLB/802.3ad. Only MII monitoring is supported on %s.\n", bond->dev->name, bond->dev->name); ret = -EINVAL; goto out; -- cgit v1.2.3 From b869ccfab1e324507fa3596e3e1308444fb68227 Mon Sep 17 00:00:00 2001 From: Nikolay Aleksandrov Date: Wed, 13 Nov 2013 17:07:46 +0100 Subject: bonding: fix two race conditions in bond_store_updelay/downdelay This patch fixes two race conditions between bond_store_updelay/downdelay and bond_store_miimon which could lead to division by zero as miimon can be set to 0 while either updelay/downdelay are being set and thus miss the zero check in the beginning, the zero div happens because updelay/downdelay are stored as new_value / bond->params.miimon. Use rtnl to synchronize with miimon setting. CC: Jay Vosburgh CC: Andy Gospodarek CC: Veaceslav Falico Signed-off-by: Nikolay Aleksandrov Acked-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_sysfs.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index 6245d92b7a0c..9f32e2304004 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -702,6 +702,8 @@ static ssize_t bonding_store_downdelay(struct device *d, int new_value, ret = count; struct bonding *bond = to_bond(d); + if (!rtnl_trylock()) + return restart_syscall(); if (!(bond->params.miimon)) { pr_err("%s: Unable to set down delay as MII monitoring is disabled\n", bond->dev->name); @@ -735,6 +737,7 @@ static ssize_t bonding_store_downdelay(struct device *d, } out: + rtnl_unlock(); return ret; } static DEVICE_ATTR(downdelay, S_IRUGO | S_IWUSR, @@ -757,6 +760,8 @@ static ssize_t bonding_store_updelay(struct device *d, int new_value, ret = count; struct bonding *bond = to_bond(d); + if (!rtnl_trylock()) + return restart_syscall(); if (!(bond->params.miimon)) { pr_err("%s: Unable to set up delay as MII monitoring is disabled\n", bond->dev->name); @@ -790,6 +795,7 @@ static ssize_t bonding_store_updelay(struct device *d, } out: + rtnl_unlock(); return ret; } static DEVICE_ATTR(updelay, S_IRUGO | S_IWUSR, -- cgit v1.2.3 From f9de11a165943a55e0fbda714caf60eaeb276a42 Mon Sep 17 00:00:00 2001 From: Wang Weidong Date: Fri, 15 Nov 2013 10:34:30 -0500 Subject: bonding: add ip checks when store ip target I met a Bug when I add ip target with the wrong ip address: echo +500.500.500.500 > /sys/class/net/bond0/bonding/arp_ip_target the wrong ip address will transfor to 245.245.245.244 and add to the ip target success, it is uncorrect, so I add checks to avoid adding wrong address. The in4_pton() will set wrong ip address to 0.0.0.0, it will return by the next check and will not add to ip target. v2 According Veaceslav's opinion, simplify the code. v3 According Veaceslav's opinion, add broadcast check and make a micro definition to package it. v4 Solve the problem of the format which David point out. Suggested-by: Veaceslav Falico Suggested-by: David S. Miller Signed-off-by: Ding Tianhong Signed-off-by: David S. Miller --- drivers/net/bonding/bond_sysfs.c | 19 ++++++------------- drivers/net/bonding/bonding.h | 3 +++ 2 files changed, 9 insertions(+), 13 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index 9f32e2304004..0ec2a7e8c8a9 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -604,15 +604,14 @@ static ssize_t bonding_store_arp_targets(struct device *d, return restart_syscall(); targets = bond->params.arp_targets; - newtarget = in_aton(buf + 1); + if (!in4_pton(buf + 1, -1, (u8 *)&newtarget, -1, NULL) || + IS_IP_TARGET_UNUSABLE_ADDRESS(newtarget)) { + pr_err("%s: invalid ARP target %pI4 specified for addition\n", + bond->dev->name, &newtarget); + goto out; + } /* look for adds */ if (buf[0] == '+') { - if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) { - pr_err("%s: invalid ARP target %pI4 specified for addition\n", - bond->dev->name, &newtarget); - goto out; - } - if (bond_get_targets_ip(targets, newtarget) != -1) { /* dup */ pr_err("%s: ARP target %pI4 is already present\n", bond->dev->name, &newtarget); @@ -635,12 +634,6 @@ static ssize_t bonding_store_arp_targets(struct device *d, targets[ind] = newtarget; write_unlock_bh(&bond->lock); } else if (buf[0] == '-') { - if ((newtarget == 0) || (newtarget == htonl(INADDR_BROADCAST))) { - pr_err("%s: invalid ARP target %pI4 specified for removal\n", - bond->dev->name, &newtarget); - goto out; - } - ind = bond_get_targets_ip(targets, newtarget); if (ind == -1) { pr_err("%s: unable to remove nonexistent ARP target %pI4.\n", diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index 77a07a12e77f..ca31286aa028 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -63,6 +63,9 @@ (((mode) == BOND_MODE_TLB) || \ ((mode) == BOND_MODE_ALB)) +#define IS_IP_TARGET_UNUSABLE_ADDRESS(a) \ + ((htonl(INADDR_BROADCAST) == a) || \ + ipv4_is_zeronet(a)) /* * Less bad way to call ioctl from within the kernel; this needs to be * done some other way to get the call out of interrupt context. -- cgit v1.2.3 From fe9d04afe9bee0ec37a9724937443b2c0e39ce4b Mon Sep 17 00:00:00 2001 From: dingtianhong Date: Fri, 22 Nov 2013 22:28:43 +0800 Subject: bonding: disable arp and enable mii monitoring when bond change to no uses arp mode Because the ARP monitoring is not support for 802.3ad, but I still could change the mode to 802.3ad from ab mode while ARP monitoring is running, it is incorrect. So add a check for 802.3ad in bonding_store_mode to fix the problem, and make a new macro BOND_NO_USES_ARP() to simplify the code. v2: according to the Dan Williams's suggestion, bond mode is the most important bond option, it should override any of the other sub-options. So when the mode is changed, the conficting values should be cleared or reset, otherwise the user has to duplicate more operations to modify the logic. I disable the arp and enable mii monitoring when the bond mode is changed to AB, TB and 8023AD if the arp interval is true. v3: according to the Nik's suggestion, the default value of miimon should need a name, there is several place to use it, and the bond_store_arp_interval() could use micro BOND_NO_USES_ARP to make the code more simpify. Suggested-by: Dan Williams Suggested-by: Nikolay Aleksandrov Signed-off-by: Ding Tianhong Reviewed-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 4 ++-- drivers/net/bonding/bond_options.c | 13 +++++++++---- drivers/net/bonding/bond_sysfs.c | 4 +--- drivers/net/bonding/bonding.h | 7 +++++++ 4 files changed, 19 insertions(+), 9 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 4dd5ee2a34cc..36eab0c4fb33 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4110,7 +4110,7 @@ static int bond_check_params(struct bond_params *params) if (!miimon) { pr_warning("Warning: miimon must be specified, otherwise bonding will not detect link failure, speed and duplex which are essential for 802.3ad operation\n"); pr_warning("Forcing miimon to 100msec\n"); - miimon = 100; + miimon = BOND_DEFAULT_MIIMON; } } @@ -4147,7 +4147,7 @@ static int bond_check_params(struct bond_params *params) if (!miimon) { pr_warning("Warning: miimon must be specified, otherwise bonding will not detect link failure and link speed which are essential for TLB/ALB load balancing\n"); pr_warning("Forcing miimon to 100msec\n"); - miimon = 100; + miimon = BOND_DEFAULT_MIIMON; } } diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c index 9a5223c7b4d1..ea6f640782b7 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c @@ -45,10 +45,15 @@ int bond_option_mode_set(struct bonding *bond, int mode) return -EPERM; } - if (BOND_MODE_IS_LB(mode) && bond->params.arp_interval) { - pr_err("%s: %s mode is incompatible with arp monitoring.\n", - bond->dev->name, bond_mode_tbl[mode].modename); - return -EINVAL; + if (BOND_NO_USES_ARP(mode) && bond->params.arp_interval) { + pr_info("%s: %s mode is incompatible with arp monitoring, start mii monitoring\n", + bond->dev->name, bond_mode_tbl[mode].modename); + /* disable arp monitoring */ + bond->params.arp_interval = 0; + /* set miimon to default value */ + bond->params.miimon = BOND_DEFAULT_MIIMON; + pr_info("%s: Setting MII monitoring interval to %d.\n", + bond->dev->name, bond->params.miimon); } /* don't cache arp_validate between modes */ diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index 0ec2a7e8c8a9..abf5e106edc5 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -523,9 +523,7 @@ static ssize_t bonding_store_arp_interval(struct device *d, ret = -EINVAL; goto out; } - if (bond->params.mode == BOND_MODE_ALB || - bond->params.mode == BOND_MODE_TLB || - bond->params.mode == BOND_MODE_8023AD) { + if (BOND_NO_USES_ARP(bond->params.mode)) { pr_info("%s: ARP monitoring cannot be used with ALB/TLB/802.3ad. Only MII monitoring is supported on %s.\n", bond->dev->name, bond->dev->name); ret = -EINVAL; diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h index ca31286aa028..a9f4f9f4d8ce 100644 --- a/drivers/net/bonding/bonding.h +++ b/drivers/net/bonding/bonding.h @@ -35,6 +35,8 @@ #define BOND_MAX_ARP_TARGETS 16 +#define BOND_DEFAULT_MIIMON 100 + #define IS_UP(dev) \ ((((dev)->flags & IFF_UP) == IFF_UP) && \ netif_running(dev) && \ @@ -55,6 +57,11 @@ ((mode) == BOND_MODE_TLB) || \ ((mode) == BOND_MODE_ALB)) +#define BOND_NO_USES_ARP(mode) \ + (((mode) == BOND_MODE_8023AD) || \ + ((mode) == BOND_MODE_TLB) || \ + ((mode) == BOND_MODE_ALB)) + #define TX_QUEUE_OVERRIDE(mode) \ (((mode) == BOND_MODE_ACTIVEBACKUP) || \ ((mode) == BOND_MODE_ROUNDROBIN)) -- cgit v1.2.3 From 89015c18ff34a39e4679ddd9b058e74d7dde28e7 Mon Sep 17 00:00:00 2001 From: dingtianhong Date: Wed, 4 Dec 2013 18:59:31 +0800 Subject: bonding: add arp_ip_target checks when install the module When I install the bonding with the wrong arp_ip_target, just like arp_ip_target=500.500.500.500, the arp_ip_target was transfored to 245.245.245.244 and stored in the ip target success, it is uncorrect, so I add checks to avoid adding wrong address. The in4_pton() will set wrong ip address to 0.0.0.0 and return 0, also use the micro IS_IP_TARGET_UNUSABLE_ADDRESS to simplify the code. Signed-off-by: Ding Tianhong Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 36eab0c4fb33..398e299ee1bd 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4199,9 +4199,9 @@ static int bond_check_params(struct bond_params *params) (arp_ip_count < BOND_MAX_ARP_TARGETS) && arp_ip_target[i]; i++) { /* not complete check, but should be good enough to catch mistakes */ - __be32 ip = in_aton(arp_ip_target[i]); - if (!isdigit(arp_ip_target[i][0]) || ip == 0 || - ip == htonl(INADDR_BROADCAST)) { + __be32 ip; + if (!in4_pton(arp_ip_target[i], -1, (u8 *)&ip, -1, NULL) || + IS_IP_TARGET_UNUSABLE_ADDRESS(ip)) { pr_warning("Warning: bad arp_ip_target module parameter (%s), ARP monitoring will not be performed\n", arp_ip_target[i]); arp_interval = 0; -- cgit v1.2.3 From a752a8b94da4865d9c361c16ccf7ccb2994291dd Mon Sep 17 00:00:00 2001 From: Nikolay Aleksandrov Date: Thu, 5 Dec 2013 11:36:58 +0100 Subject: bonding: fix packets_per_slave showing There's an issue when showing the value of packets_per_slave due to using signed integer. The value may be < 0 and thus not put through reciprocal_value() before showing. This patch makes it use unsigned integer when showing it. CC: Andy Gospodarek CC: Jay Vosburgh CC: Veaceslav Falico CC: David S. Miller Signed-off-by: Nikolay Aleksandrov Acked-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index abf5e106edc5..0ae580bbc5db 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -1635,12 +1635,12 @@ static ssize_t bonding_show_packets_per_slave(struct device *d, char *buf) { struct bonding *bond = to_bond(d); - int packets_per_slave = bond->params.packets_per_slave; + unsigned int packets_per_slave = bond->params.packets_per_slave; if (packets_per_slave > 1) packets_per_slave = reciprocal_value(packets_per_slave); - return sprintf(buf, "%d\n", packets_per_slave); + return sprintf(buf, "%u\n", packets_per_slave); } static ssize_t bonding_store_packets_per_slave(struct device *d, -- cgit v1.2.3