From 2578041011c43c38f3742e161cdb99b710d5a9f8 Mon Sep 17 00:00:00 2001 From: Debabrata Banerjee Date: Mon, 14 May 2018 14:48:07 -0400 Subject: [PATCH 1/4] bonding: don't queue up extraneous rlb updates arps for incomplete entries can't be sent anyway. Signed-off-by: Debabrata Banerjee Signed-off-by: David S. Miller --- drivers/net/bonding/bond_alb.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index a8f60982d483..9f7031ec56d5 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -421,7 +421,8 @@ static void rlb_clear_slave(struct bonding *bond, struct slave *slave) if (assigned_slave) { rx_hash_table[index].slave = assigned_slave; if (!ether_addr_equal_64bits(rx_hash_table[index].mac_dst, - mac_bcast)) { + mac_bcast) && + !is_zero_ether_addr(rx_hash_table[index].mac_dst)) { bond_info->rx_hashtbl[index].ntt = 1; bond_info->rx_ntt = 1; /* A slave has been removed from the @@ -524,7 +525,8 @@ static void rlb_req_update_slave_clients(struct bonding *bond, struct slave *sla client_info = &(bond_info->rx_hashtbl[hash_index]); if ((client_info->slave == slave) && - !ether_addr_equal_64bits(client_info->mac_dst, mac_bcast)) { + !ether_addr_equal_64bits(client_info->mac_dst, mac_bcast) && + !is_zero_ether_addr(client_info->mac_dst)) { client_info->ntt = 1; ntt = 1; } @@ -565,7 +567,8 @@ static void rlb_req_update_subnet_clients(struct bonding *bond, __be32 src_ip) if ((client_info->ip_src == src_ip) && !ether_addr_equal_64bits(client_info->slave->dev->dev_addr, bond->dev->dev_addr) && - !ether_addr_equal_64bits(client_info->mac_dst, mac_bcast)) { + !ether_addr_equal_64bits(client_info->mac_dst, mac_bcast) && + !is_zero_ether_addr(client_info->mac_dst)) { client_info->ntt = 1; bond_info->rx_ntt = 1; } @@ -641,7 +644,8 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon ether_addr_copy(client_info->mac_src, arp->mac_src); client_info->slave = assigned_slave; - if (!ether_addr_equal_64bits(client_info->mac_dst, mac_bcast)) { + if (!ether_addr_equal_64bits(client_info->mac_dst, mac_bcast) && + !is_zero_ether_addr(client_info->mac_dst)) { client_info->ntt = 1; bond->alb_info.rx_ntt = 1; } else { @@ -733,8 +737,10 @@ static void rlb_rebalance(struct bonding *bond) assigned_slave = __rlb_next_rx_slave(bond); if (assigned_slave && (client_info->slave != assigned_slave)) { client_info->slave = assigned_slave; - client_info->ntt = 1; - ntt = 1; + if (!is_zero_ether_addr(client_info->mac_dst)) { + client_info->ntt = 1; + ntt = 1; + } } } From cbeeea70de457ce4de89197d72943ab455b4172c Mon Sep 17 00:00:00 2001 From: Debabrata Banerjee Date: Mon, 14 May 2018 14:48:08 -0400 Subject: [PATCH 2/4] bonding: use common mac addr checks Replace homegrown mac addr checks with faster defs from etherdevice.h Note that this will also prevent any rlb arp updates for multicast addresses, however this should have been forbidden anyway. Signed-off-by: Debabrata Banerjee Signed-off-by: David S. Miller --- drivers/net/bonding/bond_alb.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index 9f7031ec56d5..67fd1af1d1de 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -40,11 +40,6 @@ #include #include - - -static const u8 mac_bcast[ETH_ALEN + 2] __long_aligned = { - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff -}; static const u8 mac_v6_allmcast[ETH_ALEN + 2] __long_aligned = { 0x33, 0x33, 0x00, 0x00, 0x00, 0x01 }; @@ -420,9 +415,7 @@ static void rlb_clear_slave(struct bonding *bond, struct slave *slave) if (assigned_slave) { rx_hash_table[index].slave = assigned_slave; - if (!ether_addr_equal_64bits(rx_hash_table[index].mac_dst, - mac_bcast) && - !is_zero_ether_addr(rx_hash_table[index].mac_dst)) { + if (is_valid_ether_addr(rx_hash_table[index].mac_dst)) { bond_info->rx_hashtbl[index].ntt = 1; bond_info->rx_ntt = 1; /* A slave has been removed from the @@ -525,8 +518,7 @@ static void rlb_req_update_slave_clients(struct bonding *bond, struct slave *sla client_info = &(bond_info->rx_hashtbl[hash_index]); if ((client_info->slave == slave) && - !ether_addr_equal_64bits(client_info->mac_dst, mac_bcast) && - !is_zero_ether_addr(client_info->mac_dst)) { + is_valid_ether_addr(client_info->mac_dst)) { client_info->ntt = 1; ntt = 1; } @@ -567,8 +559,7 @@ static void rlb_req_update_subnet_clients(struct bonding *bond, __be32 src_ip) if ((client_info->ip_src == src_ip) && !ether_addr_equal_64bits(client_info->slave->dev->dev_addr, bond->dev->dev_addr) && - !ether_addr_equal_64bits(client_info->mac_dst, mac_bcast) && - !is_zero_ether_addr(client_info->mac_dst)) { + is_valid_ether_addr(client_info->mac_dst)) { client_info->ntt = 1; bond_info->rx_ntt = 1; } @@ -596,7 +587,7 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon if ((client_info->ip_src == arp->ip_src) && (client_info->ip_dst == arp->ip_dst)) { /* the entry is already assigned to this client */ - if (!ether_addr_equal_64bits(arp->mac_dst, mac_bcast)) { + if (!is_broadcast_ether_addr(arp->mac_dst)) { /* update mac address from arp */ ether_addr_copy(client_info->mac_dst, arp->mac_dst); } @@ -644,8 +635,7 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon ether_addr_copy(client_info->mac_src, arp->mac_src); client_info->slave = assigned_slave; - if (!ether_addr_equal_64bits(client_info->mac_dst, mac_bcast) && - !is_zero_ether_addr(client_info->mac_dst)) { + if (is_valid_ether_addr(client_info->mac_dst)) { client_info->ntt = 1; bond->alb_info.rx_ntt = 1; } else { @@ -1418,9 +1408,9 @@ netdev_tx_t bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev) case ETH_P_IP: { const struct iphdr *iph = ip_hdr(skb); - if (ether_addr_equal_64bits(eth_data->h_dest, mac_bcast) || - (iph->daddr == ip_bcast) || - (iph->protocol == IPPROTO_IGMP)) { + if (is_broadcast_ether_addr(eth_data->h_dest) || + iph->daddr == ip_bcast || + iph->protocol == IPPROTO_IGMP) { do_tx_balance = false; break; } @@ -1432,7 +1422,7 @@ netdev_tx_t bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev) /* IPv6 doesn't really use broadcast mac address, but leave * that here just in case. */ - if (ether_addr_equal_64bits(eth_data->h_dest, mac_bcast)) { + if (is_broadcast_ether_addr(eth_data->h_dest)) { do_tx_balance = false; break; } From e79c1055749e3183a2beee04a24da378623329c5 Mon Sep 17 00:00:00 2001 From: Debabrata Banerjee Date: Mon, 14 May 2018 14:48:09 -0400 Subject: [PATCH 3/4] bonding: allow use of tx hashing in balance-alb The rx load balancing provided by balance-alb is not mutually exclusive with using hashing for tx selection, and should provide a decent speed increase because this eliminates spinlocks and cache contention. Signed-off-by: Debabrata Banerjee Signed-off-by: David S. Miller --- drivers/net/bonding/bond_alb.c | 20 ++++++++++++++++++-- drivers/net/bonding/bond_main.c | 25 +++++++++++++++---------- drivers/net/bonding/bond_options.c | 2 +- include/net/bonding.h | 11 +++++++++-- 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c index 67fd1af1d1de..e82108c917a6 100644 --- a/drivers/net/bonding/bond_alb.c +++ b/drivers/net/bonding/bond_alb.c @@ -1478,8 +1478,24 @@ netdev_tx_t bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev) } if (do_tx_balance) { - hash_index = _simple_hash(hash_start, hash_size); - tx_slave = tlb_choose_channel(bond, hash_index, skb->len); + if (bond->params.tlb_dynamic_lb) { + hash_index = _simple_hash(hash_start, hash_size); + tx_slave = tlb_choose_channel(bond, hash_index, skb->len); + } else { + /* + * do_tx_balance means we are free to select the tx_slave + * So we do exactly what tlb would do for hash selection + */ + + struct bond_up_slave *slaves; + unsigned int count; + + slaves = rcu_dereference(bond->slave_arr); + count = slaves ? READ_ONCE(slaves->count) : 0; + if (likely(count)) + tx_slave = slaves->arr[bond_xmit_hash(bond, skb) % + count]; + } } return bond_do_alb_xmit(skb, bond, tx_slave); diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 4176e1d95f47..a4cd7f6bfd4d 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -159,7 +159,7 @@ module_param(min_links, int, 0); MODULE_PARM_DESC(min_links, "Minimum number of available links before turning on carrier"); module_param(xmit_hash_policy, charp, 0); -MODULE_PARM_DESC(xmit_hash_policy, "balance-xor and 802.3ad hashing method; " +MODULE_PARM_DESC(xmit_hash_policy, "balance-alb, balance-tlb, balance-xor, 802.3ad hashing method; " "0 for layer 2 (default), 1 for layer 3+4, " "2 for layer 2+3, 3 for encap layer 2+3, " "4 for encap layer 3+4"); @@ -1735,7 +1735,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev, unblock_netpoll_tx(); } - if (bond_mode_uses_xmit_hash(bond)) + if (bond_mode_can_use_xmit_hash(bond)) bond_update_slave_arr(bond, NULL); bond->nest_level = dev_get_nest_level(bond_dev); @@ -1870,7 +1870,7 @@ static int __bond_release_one(struct net_device *bond_dev, if (BOND_MODE(bond) == BOND_MODE_8023AD) bond_3ad_unbind_slave(slave); - if (bond_mode_uses_xmit_hash(bond)) + if (bond_mode_can_use_xmit_hash(bond)) bond_update_slave_arr(bond, slave); netdev_info(bond_dev, "Releasing %s interface %s\n", @@ -3102,7 +3102,7 @@ static int bond_slave_netdev_event(unsigned long event, * events. If these (miimon/arpmon) parameters are configured * then array gets refreshed twice and that should be fine! */ - if (bond_mode_uses_xmit_hash(bond)) + if (bond_mode_can_use_xmit_hash(bond)) bond_update_slave_arr(bond, NULL); break; case NETDEV_CHANGEMTU: @@ -3322,7 +3322,7 @@ static int bond_open(struct net_device *bond_dev) */ if (bond_alb_initialize(bond, (BOND_MODE(bond) == BOND_MODE_ALB))) return -ENOMEM; - if (bond->params.tlb_dynamic_lb) + if (bond->params.tlb_dynamic_lb || BOND_MODE(bond) == BOND_MODE_ALB) queue_delayed_work(bond->wq, &bond->alb_work, 0); } @@ -3341,7 +3341,7 @@ static int bond_open(struct net_device *bond_dev) bond_3ad_initiate_agg_selection(bond, 1); } - if (bond_mode_uses_xmit_hash(bond)) + if (bond_mode_can_use_xmit_hash(bond)) bond_update_slave_arr(bond, NULL); return 0; @@ -3894,7 +3894,7 @@ static void bond_slave_arr_handler(struct work_struct *work) * to determine the slave interface - * (a) BOND_MODE_8023AD * (b) BOND_MODE_XOR - * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0 + * (c) (BOND_MODE_TLB || BOND_MODE_ALB) && tlb_dynamic_lb == 0 * * The caller is expected to hold RTNL only and NO other lock! */ @@ -3947,6 +3947,11 @@ int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave) continue; if (skipslave == slave) continue; + + netdev_dbg(bond->dev, + "Adding slave dev %s to tx hash array[%d]\n", + slave->dev->name, new_arr->count); + new_arr->arr[new_arr->count++] = slave; } @@ -4324,9 +4329,9 @@ static int bond_check_params(struct bond_params *params) } if (xmit_hash_policy) { - if ((bond_mode != BOND_MODE_XOR) && - (bond_mode != BOND_MODE_8023AD) && - (bond_mode != BOND_MODE_TLB)) { + if (bond_mode == BOND_MODE_ROUNDROBIN || + bond_mode == BOND_MODE_ACTIVEBACKUP || + bond_mode == BOND_MODE_BROADCAST) { pr_info("xmit_hash_policy param is irrelevant in mode %s\n", bond_mode_name(bond_mode)); } else { diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c index 58c705f24f96..8a945c9341d6 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c @@ -395,7 +395,7 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = { .id = BOND_OPT_TLB_DYNAMIC_LB, .name = "tlb_dynamic_lb", .desc = "Enable dynamic flow shuffling", - .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_TLB)), + .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_TLB) | BIT(BOND_MODE_ALB)), .values = bond_tlb_dynamic_lb_tbl, .flags = BOND_OPTFLAG_IFDOWN, .set = bond_option_tlb_dynamic_lb_set, diff --git a/include/net/bonding.h b/include/net/bonding.h index b52235158836..808f1d167349 100644 --- a/include/net/bonding.h +++ b/include/net/bonding.h @@ -285,8 +285,15 @@ static inline bool bond_needs_speed_duplex(const struct bonding *bond) static inline bool bond_is_nondyn_tlb(const struct bonding *bond) { - return (BOND_MODE(bond) == BOND_MODE_TLB) && - (bond->params.tlb_dynamic_lb == 0); + return (bond_is_lb(bond) && bond->params.tlb_dynamic_lb == 0); +} + +static inline bool bond_mode_can_use_xmit_hash(const struct bonding *bond) +{ + return (BOND_MODE(bond) == BOND_MODE_8023AD || + BOND_MODE(bond) == BOND_MODE_XOR || + BOND_MODE(bond) == BOND_MODE_TLB || + BOND_MODE(bond) == BOND_MODE_ALB); } static inline bool bond_mode_uses_xmit_hash(const struct bonding *bond) From 1386c36b30388f46a95100924bfcae75160db715 Mon Sep 17 00:00:00 2001 From: Debabrata Banerjee Date: Mon, 14 May 2018 14:48:10 -0400 Subject: [PATCH 4/4] bonding: allow carrier and link status to determine link state In a mixed environment it may be difficult to tell if your hardware support carrier, if it does not it can always report true. With a new use_carrier option of 2, we can check both carrier and link status sequentially, instead of one or the other Signed-off-by: Debabrata Banerjee Signed-off-by: David S. Miller --- Documentation/networking/bonding.txt | 4 ++-- drivers/net/bonding/bond_main.c | 12 ++++++++---- drivers/net/bonding/bond_options.c | 7 ++++--- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt index c13214d073a4..86d07fbb592d 100644 --- a/Documentation/networking/bonding.txt +++ b/Documentation/networking/bonding.txt @@ -828,8 +828,8 @@ use_carrier MII / ETHTOOL ioctl method to determine the link state. A value of 1 enables the use of netif_carrier_ok(), a value of - 0 will use the deprecated MII / ETHTOOL ioctls. The default - value is 1. + 0 will use the deprecated MII / ETHTOOL ioctls. A value of 2 + will check both. The default value is 1. xmit_hash_policy diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index a4cd7f6bfd4d..e4c253dc7dfb 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -132,7 +132,7 @@ MODULE_PARM_DESC(downdelay, "Delay before considering link down, " "in milliseconds"); module_param(use_carrier, int, 0); MODULE_PARM_DESC(use_carrier, "Use netif_carrier_ok (vs MII ioctls) in miimon; " - "0 for off, 1 for on (default)"); + "0 for off, 1 for on (default), 2 for carrier then legacy checks"); module_param(mode, charp, 0); MODULE_PARM_DESC(mode, "Mode of operation; 0 for balance-rr, " "1 for active-backup, 2 for balance-xor, " @@ -434,12 +434,16 @@ static int bond_check_dev_link(struct bonding *bond, int (*ioctl)(struct net_device *, struct ifreq *, int); struct ifreq ifr; struct mii_ioctl_data *mii; + bool carrier = true; if (!reporting && !netif_running(slave_dev)) return 0; if (bond->params.use_carrier) - return netif_carrier_ok(slave_dev) ? BMSR_LSTATUS : 0; + carrier = netif_carrier_ok(slave_dev) ? BMSR_LSTATUS : 0; + + if (!carrier) + return carrier; /* Try to get link status using Ethtool first. */ if (slave_dev->ethtool_ops->get_link) @@ -4403,8 +4407,8 @@ static int bond_check_params(struct bond_params *params) downdelay = 0; } - if ((use_carrier != 0) && (use_carrier != 1)) { - pr_warn("Warning: use_carrier module parameter (%d), not of valid value (0/1), so it was set to 1\n", + if (use_carrier < 0 || use_carrier > 2) { + pr_warn("Warning: use_carrier module parameter (%d), not of valid value (0-2), so it was set to 1\n", use_carrier); use_carrier = 1; } diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c index 8a945c9341d6..dba6cef05134 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c @@ -164,9 +164,10 @@ static const struct bond_opt_value bond_primary_reselect_tbl[] = { }; static const struct bond_opt_value bond_use_carrier_tbl[] = { - { "off", 0, 0}, - { "on", 1, BOND_VALFLAG_DEFAULT}, - { NULL, -1, 0} + { "off", 0, 0}, + { "on", 1, BOND_VALFLAG_DEFAULT}, + { "both", 2, 0}, + { NULL, -1, 0} }; static const struct bond_opt_value bond_all_slaves_active_tbl[] = {