From 351638e7deeed2ec8ce451b53d33921b3da68f83 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Tue, 28 May 2013 01:30:21 +0000 Subject: net: pass info struct via netdevice notifier So far, only net_device * could be passed along with netdevice notifier event. This patch provides a possibility to pass custom structure able to provide info that event listener needs to know. Signed-off-by: Jiri Pirko v2->v3: fix typo on simeth shortened dev_getter shortened notifier_info struct name v1->v2: fix notifier_call parameter in call_netdevice_notifier() 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/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 29b846cbfb48..f4489d65bf33 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -3277,7 +3277,7 @@ static int bond_slave_netdev_event(unsigned long event, static int bond_netdev_event(struct notifier_block *this, unsigned long event, void *ptr) { - struct net_device *event_dev = (struct net_device *)ptr; + struct net_device *event_dev = netdev_notifier_info_to_dev(ptr); pr_debug("event_dev: %s, event: %lx\n", event_dev ? event_dev->name : "None", -- cgit v1.2.3 From 303d1cbf610eef31e039efc2e8da30cf94cf5ebc Mon Sep 17 00:00:00 2001 From: Jay Vosburgh Date: Fri, 31 May 2013 11:57:30 +0000 Subject: bonding: Convert hw addr handling to sync/unsync, support ucast addresses This patch converts bonding to use the dev_uc/mc_sync and dev_uc/mc_sync_multiple functions for updating the hardware addresses of bonding slaves. The existing functions to add or remove addresses are removed, and their functionality is replaced with calls to dev_mc_sync or dev_mc_sync_multiple, depending upon the bonding mode. Calls to dev_uc_sync and dev_uc_sync_multiple are also added, so that unicast addresses added to a bond will be properly synced with its slaves. Various functions are renamed to better reflect the new situation, and relevant comments are updated. Signed-off-by: Jay Vosburgh Cc: Vlad Yasevich Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 168 ++++++++++------------------------------ 1 file changed, 43 insertions(+), 125 deletions(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index f4489d65bf33..4953f66a29a4 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -706,45 +706,6 @@ static int bond_set_allmulti(struct bonding *bond, int inc) return err; } -/* - * Add a Multicast address to slaves - * according to mode - */ -static void bond_mc_add(struct bonding *bond, void *addr) -{ - if (USES_PRIMARY(bond->params.mode)) { - /* write lock already acquired */ - if (bond->curr_active_slave) - dev_mc_add(bond->curr_active_slave->dev, addr); - } else { - struct slave *slave; - int i; - - bond_for_each_slave(bond, slave, i) - dev_mc_add(slave->dev, addr); - } -} - -/* - * Remove a multicast address from slave - * according to mode - */ -static void bond_mc_del(struct bonding *bond, void *addr) -{ - if (USES_PRIMARY(bond->params.mode)) { - /* write lock already acquired */ - if (bond->curr_active_slave) - dev_mc_del(bond->curr_active_slave->dev, addr); - } else { - struct slave *slave; - int i; - bond_for_each_slave(bond, slave, i) { - dev_mc_del(slave->dev, addr); - } - } -} - - static void __bond_resend_igmp_join_requests(struct net_device *dev) { struct in_device *in_dev; @@ -803,17 +764,15 @@ static void bond_resend_igmp_join_requests_delayed(struct work_struct *work) bond_resend_igmp_join_requests(bond); } -/* - * flush all members of flush->mc_list from device dev->mc_list +/* Flush bond's hardware addresses from slave */ -static void bond_mc_list_flush(struct net_device *bond_dev, +static void bond_hw_addr_flush(struct net_device *bond_dev, struct net_device *slave_dev) { struct bonding *bond = netdev_priv(bond_dev); - struct netdev_hw_addr *ha; - netdev_for_each_mc_addr(ha, bond_dev) - dev_mc_del(slave_dev, ha->addr); + dev_uc_unsync(slave_dev, bond_dev); + dev_mc_unsync(slave_dev, bond_dev); if (bond->params.mode == BOND_MODE_8023AD) { /* del lacpdu mc addr from mc list */ @@ -825,22 +784,14 @@ static void bond_mc_list_flush(struct net_device *bond_dev, /*--------------------------- Active slave change ---------------------------*/ -/* - * Update the mc list and multicast-related flags for the new and - * old active slaves (if any) according to the multicast mode, and - * promiscuous flags unconditionally. +/* Update the hardware address list and promisc/allmulti for the new and + * old active slaves (if any). Modes that are !USES_PRIMARY keep all + * slaves up date at all times; only the USES_PRIMARY modes need to call + * this function to swap these settings during a failover. */ -static void bond_mc_swap(struct bonding *bond, struct slave *new_active, - struct slave *old_active) +static void bond_hw_addr_swap(struct bonding *bond, struct slave *new_active, + struct slave *old_active) { - struct netdev_hw_addr *ha; - - if (!USES_PRIMARY(bond->params.mode)) - /* nothing to do - mc list is already up-to-date on - * all slaves - */ - return; - if (old_active) { if (bond->dev->flags & IFF_PROMISC) dev_set_promiscuity(old_active->dev, -1); @@ -848,10 +799,7 @@ static void bond_mc_swap(struct bonding *bond, struct slave *new_active, if (bond->dev->flags & IFF_ALLMULTI) dev_set_allmulti(old_active->dev, -1); - netif_addr_lock_bh(bond->dev); - netdev_for_each_mc_addr(ha, bond->dev) - dev_mc_del(old_active->dev, ha->addr); - netif_addr_unlock_bh(bond->dev); + bond_hw_addr_flush(bond->dev, old_active->dev); } if (new_active) { @@ -863,8 +811,8 @@ static void bond_mc_swap(struct bonding *bond, struct slave *new_active, dev_set_allmulti(new_active->dev, 1); netif_addr_lock_bh(bond->dev); - netdev_for_each_mc_addr(ha, bond->dev) - dev_mc_add(new_active->dev, ha->addr); + dev_uc_sync(new_active->dev, bond->dev); + dev_mc_sync(new_active->dev, bond->dev); netif_addr_unlock_bh(bond->dev); } } @@ -1083,7 +1031,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active) } if (USES_PRIMARY(bond->params.mode)) - bond_mc_swap(bond, new_active, old_active); + bond_hw_addr_swap(bond, new_active, old_active); if (bond_is_lb(bond)) { bond_alb_handle_active_change(bond, new_active); @@ -1526,7 +1474,6 @@ 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 netdev_hw_addr *ha; struct sockaddr addr; int link_reporting; int res = 0; @@ -1706,10 +1653,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) goto err_close; } - /* If the mode USES_PRIMARY, then the new slave gets the - * master's promisc (and mc) settings only if it becomes the - * curr_active_slave, and that is taken care of later when calling - * bond_change_active() + /* If the mode USES_PRIMARY, then the following is handled by + * bond_change_active_slave(). */ if (!USES_PRIMARY(bond->params.mode)) { /* set promiscuity level to new slave */ @@ -1727,9 +1672,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) } netif_addr_lock_bh(bond_dev); - /* upload master's mc_list to new slave */ - netdev_for_each_mc_addr(ha, bond_dev) - dev_mc_add(slave_dev, ha->addr); + + dev_mc_sync_multiple(slave_dev, bond_dev); + dev_uc_sync_multiple(slave_dev, bond_dev); + netif_addr_unlock_bh(bond_dev); } @@ -1908,11 +1854,9 @@ err_dest_symlinks: bond_destroy_slave_symlinks(bond_dev, slave_dev); err_detach: - if (!USES_PRIMARY(bond->params.mode)) { - netif_addr_lock_bh(bond_dev); - bond_mc_list_flush(bond_dev, slave_dev); - netif_addr_unlock_bh(bond_dev); - } + if (!USES_PRIMARY(bond->params.mode)) + bond_hw_addr_flush(bond_dev, slave_dev); + bond_del_vlans_from_slave(bond, slave_dev); write_lock_bh(&bond->lock); bond_detach_slave(bond, new_slave); @@ -2107,9 +2051,8 @@ static int __bond_release_one(struct net_device *bond_dev, bond_del_vlans_from_slave(bond, slave_dev); - /* If the mode USES_PRIMARY, then we should only remove its - * promisc and mc settings if it was the curr_active_slave, but that was - * already taken care of above when we detached the slave + /* If the mode USES_PRIMARY, then this cases was handled above by + * bond_change_active_slave(..., NULL) */ if (!USES_PRIMARY(bond->params.mode)) { /* unset promiscuity level from slave */ @@ -2120,10 +2063,7 @@ static int __bond_release_one(struct net_device *bond_dev, if (bond_dev->flags & IFF_ALLMULTI) dev_set_allmulti(slave_dev, -1); - /* flush master's mc_list from slave */ - netif_addr_lock_bh(bond_dev); - bond_mc_list_flush(bond_dev, slave_dev); - netif_addr_unlock_bh(bond_dev); + bond_hw_addr_flush(bond_dev, slave_dev); } bond_upper_dev_unlink(bond_dev, slave_dev); @@ -3660,19 +3600,6 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd return res; } -static bool bond_addr_in_mc_list(unsigned char *addr, - struct netdev_hw_addr_list *list, - int addrlen) -{ - struct netdev_hw_addr *ha; - - netdev_hw_addr_list_for_each(ha, list) - if (!memcmp(ha->addr, addr, addrlen)) - return true; - - return false; -} - static void bond_change_rx_flags(struct net_device *bond_dev, int change) { struct bonding *bond = netdev_priv(bond_dev); @@ -3686,35 +3613,29 @@ static void bond_change_rx_flags(struct net_device *bond_dev, int change) bond_dev->flags & IFF_ALLMULTI ? 1 : -1); } -static void bond_set_multicast_list(struct net_device *bond_dev) +static void bond_set_rx_mode(struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); - struct netdev_hw_addr *ha; - bool found; + struct slave *slave; + int i; read_lock(&bond->lock); - /* looking for addresses to add to slaves' mc list */ - netdev_for_each_mc_addr(ha, bond_dev) { - found = bond_addr_in_mc_list(ha->addr, &bond->mc_list, - bond_dev->addr_len); - if (!found) - bond_mc_add(bond, ha->addr); - } - - /* looking for addresses to delete from slaves' list */ - netdev_hw_addr_list_for_each(ha, &bond->mc_list) { - found = bond_addr_in_mc_list(ha->addr, &bond_dev->mc, - bond_dev->addr_len); - if (!found) - bond_mc_del(bond, ha->addr); + if (USES_PRIMARY(bond->params.mode)) { + read_lock(&bond->curr_slave_lock); + slave = bond->curr_active_slave; + if (slave) { + dev_uc_sync(slave->dev, bond_dev); + dev_mc_sync(slave->dev, bond_dev); + } + read_unlock(&bond->curr_slave_lock); + } else { + bond_for_each_slave(bond, slave, i) { + dev_uc_sync_multiple(slave->dev, bond_dev); + dev_mc_sync_multiple(slave->dev, bond_dev); + } } - /* save master's multicast list */ - __hw_addr_flush(&bond->mc_list); - __hw_addr_add_multiple(&bond->mc_list, &bond_dev->mc, - bond_dev->addr_len, NETDEV_HW_ADDR_T_MULTICAST); - read_unlock(&bond->lock); } @@ -4321,7 +4242,7 @@ static const struct net_device_ops bond_netdev_ops = { .ndo_get_stats64 = bond_get_stats, .ndo_do_ioctl = bond_do_ioctl, .ndo_change_rx_flags = bond_change_rx_flags, - .ndo_set_rx_mode = bond_set_multicast_list, + .ndo_set_rx_mode = bond_set_rx_mode, .ndo_change_mtu = bond_change_mtu, .ndo_set_mac_address = bond_set_mac_address, .ndo_neigh_setup = bond_neigh_setup, @@ -4426,8 +4347,6 @@ static void bond_uninit(struct net_device *bond_dev) bond_debug_unregister(bond); - __hw_addr_flush(&bond->mc_list); - list_for_each_entry_safe(vlan, tmp, &bond->vlan_list, vlan_list) { list_del(&vlan->vlan_list); kfree(vlan); @@ -4838,7 +4757,6 @@ static int bond_init(struct net_device *bond_dev) bond->dev_addr_from_first = true; } - __hw_addr_init(&bond->mc_list); return 0; } -- cgit v1.2.3 From 1b5acd292336da029535de010af568533df9b665 Mon Sep 17 00:00:00 2001 From: Jay Vosburgh Date: Fri, 31 May 2013 11:57:31 +0000 Subject: bonding: disallow change of MAC if fail_over_mac enabled Currently, if fail_over_mac is set to active, then attempts to change the MAC of the bond itself silently fail. However, if fail_over_mac is set to follow, changes are permitted. Permitting the bond's MAC to change with fail_over_mac=follow will disrupt the follow functionality, which normally controls the assignment of MAC address to the bond and its slaves, and can cause multiple ports to be assigned the same MAC address. which will interfere with the functioning of the device (where the device here is a virtualization-aware card for s390, qeth). Signed-off-by: Jay Vosburgh Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 4953f66a29a4..bc1246f6f86a 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -3780,11 +3780,10 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr) pr_debug("bond=%p, name=%s\n", bond, bond_dev ? bond_dev->name : "None"); - /* - * If fail_over_mac is set to active, do nothing and return - * success. Returning an error causes ifenslave to fail. + /* If fail_over_mac is enabled, do nothing and return success. + * Returning an error causes ifenslave to fail. */ - if (bond->params.fail_over_mac == BOND_FOM_ACTIVE) + if (bond->params.fail_over_mac) return 0; if (!is_valid_ether_addr(sa->sa_data)) -- cgit v1.2.3 From 87a7b84b588c2ddbde890890855aef18ec34174e Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Mon, 24 Jun 2013 11:49:29 +0200 Subject: bonding: add helper function bond_get_targets_ip(targets, ip) Add function bond_get_targets_ip(targets, ip) which searches through targets array of ips (arp_targets) and returns the position of first match. If ip == 0, returns the first free slot. On failure to find the ip or free slot, return -1. Use it to verify if the arp we've received is valid and in sysfs. v1->v2: Fix "[2/6] bonding: add helper function bond_get_targets_ip(targets, ip)", per Nikolay's advice, to verify if source ip != 0.0.0.0, otherwise we might update 'null' arp_ip_targets' last_rx. Also, address style. Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 3b31c19972d4..976d28e3498a 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2610,19 +2610,16 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave) static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32 sip, __be32 tip) { - int i; - __be32 *targets = bond->params.arp_targets; + if (!sip || !bond_has_this_ip(bond, tip)) { + pr_debug("bva: sip %pI4 tip %pI4 not found\n", &sip, &tip); + return; + } - for (i = 0; (i < BOND_MAX_ARP_TARGETS) && targets[i]; i++) { - pr_debug("bva: sip %pI4 tip %pI4 t[%d] %pI4 bhti(tip) %d\n", - &sip, &tip, i, &targets[i], - bond_has_this_ip(bond, tip)); - if (sip == targets[i]) { - if (bond_has_this_ip(bond, tip)) - slave->last_arp_rx = jiffies; - return; - } + if (bond_get_targets_ip(bond->params.arp_targets, sip) == -1) { + pr_debug("bva: sip %pI4 not found in targets\n", &sip); + return; } + slave->last_arp_rx = jiffies; } static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, @@ -4839,7 +4836,7 @@ static int __net_init bond_net_init(struct net *net) bond_create_proc_dir(bn); bond_create_sysfs(bn); - + return 0; } -- cgit v1.2.3 From 0afee4e8b9fe4b5f58734b2f28e980dd58d3e3cb Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Mon, 24 Jun 2013 11:49:30 +0200 Subject: bonding: don't add duplicate targets to arp_ip_target Print a warning and skip them. Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 976d28e3498a..1645130c0a60 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4588,7 +4588,11 @@ static int bond_check_params(struct bond_params *params) arp_ip_target[i]); arp_interval = 0; } else { - arp_target[arp_ip_count++] = ip; + if (bond_get_targets_ip(arp_target, ip) == -1) + arp_target[arp_ip_count++] = ip; + else + pr_warning("Warning: duplicate address %pI4 in arp_ip_target, skipping\n", + &ip); } } -- cgit v1.2.3 From 2c14610210978512271dd6fe21d6f55b789d9a80 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Mon, 24 Jun 2013 11:49:31 +0200 Subject: bonding: don't validate arp if we don't have to Currently, we validate all the incoming arps if arp_validate not 0. However, we don't have to validate backup slaves if arp_validate == active and vice versa, so return early in bond_arp_rcv() in these cases. It works correctly now because we verify arp_validate in slave_last_rx(), however we're just doing useless work in bond_arp_rcv(). Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 1645130c0a60..7fe9802c9eec 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2634,6 +2634,10 @@ static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, return RX_HANDLER_ANOTHER; read_lock(&bond->lock); + + if (!slave_do_arp_validate(bond, slave)) + goto out_unlock; + alen = arp_hdr_len(bond->dev); pr_debug("bond_arp_rcv: bond %s skb->dev %s\n", -- cgit v1.2.3 From aeea64ac717a920ea655b061e37b14fbc872f7db Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Mon, 24 Jun 2013 11:49:32 +0200 Subject: bonding: don't trust arp requests unless active slave really works Currently, if we receive any arp packet on a backup slave in active-backup mode and arp_validate enabled, we suppose that it's an arp request, swap source/target ip and try to validate it. This optimization gives us virtually no downtime in the most common situation (active and backup slaves are in the same broadcast domain and the active slave failed). However, if we can't reach the arp_ip_target(s), we end up in an endless loop of reselecting slaves, because we receive our arp requests, sent by the active slave, and think that backup slaves are up, thus selecting them as active and, again, sending arp requests, which fool our backup slaves. Fix this by not validating the swapped arp packets if the current active slave didn't receive any arp reply after it was selected as active. This way we will only accept arp requests if we know that the current active slave can actually reach arp_ip_target. v3->v4: Obey 80 lines and make checkpatch.pl happy, per Sergei's suggestion. v1->v3: No change. Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 7fe9802c9eec..d3a70c0d0edd 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2677,10 +2677,17 @@ static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, * configuration, the ARP probe will (hopefully) travel from * the active, through one switch, the router, then the other * switch before reaching the backup. + * + * We 'trust' the arp requests if there is an active slave and + * it received valid arp reply(s) after it became active. This + * is done to avoid endless looping when we can't reach the + * arp_ip_target and fool ourselves with our own arp requests. */ if (bond_is_active_slave(slave)) bond_validate_arp(bond, slave, sip, tip); - else + else if (bond->curr_active_slave && + time_after(slave_last_rx(bond, bond->curr_active_slave), + bond->curr_active_slave->jiffies)) bond_validate_arp(bond, slave, tip, sip); out_unlock: -- cgit v1.2.3 From 8599b52e14a1611dcb563289421bee76751f1d53 Mon Sep 17 00:00:00 2001 From: Veaceslav Falico Date: Mon, 24 Jun 2013 11:49:34 +0200 Subject: bonding: add an option to fail when any of arp_ip_target is inaccessible Currently, we fail only when all of the ips in arp_ip_target are gone. However, in some situations we might need to fail if even one host from arp_ip_target becomes unavailable. All situations, obviously, rely on the idea that we need *completely* functional network, with all interfaces/addresses working correctly. One real world example might be: vlans on top on bond (hybrid port). If bond and vlans have ips assigned and we have their peers monitored via arp_ip_target - in case of switch misconfiguration (trunk/access port), slave driver malfunction or tagged/untagged traffic dropped on the way - we will be able to switch to another slave. Though any other configuration needs that if we need to have access to all arp_ip_targets. This patch adds this possibility by adding a new parameter - arp_all_targets (both as a module parameter and as a sysfs knob). It can be set to: 0 or any (the default) - which works exactly as it's working now - the slave is up if any of the arp_ip_targets are up. 1 or all - the slave is up if all of the arp_ip_targets are up. This parameter can be changed on the fly (via sysfs), and requires the mode to be active-backup and arp_validate to be enabled (it obeys the arp_validate config on which slaves to validate). Internally it's done through: 1) Add target_last_arp_rx[BOND_MAX_ARP_TARGETS] array to slave struct. It's an array of jiffies, meaning that slave->target_last_arp_rx[i] is the last time we've received arp from bond->params.arp_targets[i] on this slave. 2) If we successfully validate an arp from bond->params.arp_targets[i] in bond_validate_arp() - update the slave->target_last_arp_rx[i] with the current jiffies value. 3) When getting slave's last_rx via slave_last_rx(), we return the oldest time when we've received an arp from any address in bond->params.arp_targets[]. If the value of arp_all_targets == 0 - we still work the same way as before. Also, update the documentation to reflect the new parameter. v3->v4: Kill the forgotten rtnl_unlock(), rephrase the documentation part to be more clear, don't fail setting arp_all_targets if arp_validate is not set - it has no effect anyway but can be easier to set up. Also, print a warning if the last arp_ip_target is removed while the arp_interval is on, but not the arp_validate. v2->v3: Use _bh spinlock, remove useless rtnl_lock() and use jiffies for new arp_ip_target last arp, instead of slave_last_rx(). On bond_enslave(), use the same initialization value for target_last_arp_rx[] as is used for the default last_arp_rx, to avoid useless interface flaps. Also, instead of failing to remove the last arp_ip_target just print a warning - otherwise it might break existing scripts. v1->v2: Correctly handle adding/removing hosts in arp_ip_target - we need to shift/initialize all slave's target_last_arp_rx. Also, don't fail module loading on arp_all_targets misconfiguration, just disable it, and some minor style fixes. Signed-off-by: Veaceslav Falico Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index d3a70c0d0edd..142d55dc526e 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -104,6 +104,7 @@ static char *xmit_hash_policy; static int arp_interval = BOND_LINK_ARP_INTERV; static char *arp_ip_target[BOND_MAX_ARP_TARGETS]; static char *arp_validate; +static char *arp_all_targets; static char *fail_over_mac; static int all_slaves_active = 0; static struct bond_params bonding_defaults; @@ -166,6 +167,8 @@ module_param(arp_validate, charp, 0); MODULE_PARM_DESC(arp_validate, "validate src/dst of ARP probes; " "0 for none (default), 1 for active, " "2 for backup, 3 for all"); +module_param(arp_all_targets, charp, 0); +MODULE_PARM_DESC(arp_all_targets, "fail on any/all arp targets timeout; 0 for any (default), 1 for all"); module_param(fail_over_mac, charp, 0); MODULE_PARM_DESC(fail_over_mac, "For active-backup, do not set all slaves to " "the same MAC; 0 for none (default), " @@ -216,6 +219,12 @@ const struct bond_parm_tbl xmit_hashtype_tbl[] = { { NULL, -1}, }; +const struct bond_parm_tbl arp_all_targets_tbl[] = { +{ "any", BOND_ARP_TARGETS_ANY}, +{ "all", BOND_ARP_TARGETS_ALL}, +{ NULL, -1}, +}; + const struct bond_parm_tbl arp_validate_tbl[] = { { "none", BOND_ARP_VALIDATE_NONE}, { "active", BOND_ARP_VALIDATE_ACTIVE}, @@ -1483,7 +1492,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) struct slave *new_slave = NULL; struct sockaddr addr; int link_reporting; - int res = 0; + int res = 0, i; if (!bond->params.use_carrier && slave_dev->ethtool_ops->get_link == NULL && @@ -1712,6 +1721,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) new_slave->last_arp_rx = jiffies - (msecs_to_jiffies(bond->params.arp_interval) + 1); + for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) + new_slave->target_last_arp_rx[i] = new_slave->last_arp_rx; if (bond->params.miimon && !bond->params.use_carrier) { link_reporting = bond_check_dev_link(bond, slave_dev, 1); @@ -2610,16 +2621,20 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave) static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32 sip, __be32 tip) { + int i; + if (!sip || !bond_has_this_ip(bond, tip)) { pr_debug("bva: sip %pI4 tip %pI4 not found\n", &sip, &tip); return; } - if (bond_get_targets_ip(bond->params.arp_targets, sip) == -1) { + i = bond_get_targets_ip(bond->params.arp_targets, sip); + if (i == -1) { pr_debug("bva: sip %pI4 not found in targets\n", &sip); return; } slave->last_arp_rx = jiffies; + slave->target_last_arp_rx[i] = jiffies; } static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, @@ -4409,6 +4424,7 @@ int bond_parse_parm(const char *buf, const struct bond_parm_tbl *tbl) static int bond_check_params(struct bond_params *params) { int arp_validate_value, fail_over_mac_value, primary_reselect_value, i; + int arp_all_targets_value; /* * Convert string parameters. @@ -4634,6 +4650,18 @@ static int bond_check_params(struct bond_params *params) } else arp_validate_value = 0; + arp_all_targets_value = 0; + if (arp_all_targets) { + arp_all_targets_value = bond_parse_parm(arp_all_targets, + arp_all_targets_tbl); + + if (arp_all_targets_value == -1) { + pr_err("Error: invalid arp_all_targets_value \"%s\"\n", + arp_all_targets); + arp_all_targets_value = 0; + } + } + if (miimon) { pr_info("MII link monitoring set to %d ms\n", miimon); } else if (arp_interval) { @@ -4698,6 +4726,7 @@ static int bond_check_params(struct bond_params *params) params->num_peer_notif = num_peer_notif; params->arp_interval = arp_interval; params->arp_validate = arp_validate_value; + params->arp_all_targets = arp_all_targets_value; params->updelay = updelay; params->downdelay = downdelay; params->use_carrier = use_carrier; -- cgit v1.2.3 From 8d2ada77f8a7f8f65fcbf71b23cbac54b64151a6 Mon Sep 17 00:00:00 2001 From: "nikolay@redhat.com" Date: Wed, 26 Jun 2013 17:13:37 +0200 Subject: bonding: remove unnecessary setup_by_slave member We have a member called setup_by_slave in struct bonding to denote if the bond dev has different type than ARPHRD_ETHER, but that is already denoted in bond's netdev type variable if it was setup by the slave, so use that instead of the member. Signed-off-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 142d55dc526e..2e8b9f1e2747 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1382,8 +1382,6 @@ done: static void bond_setup_by_slave(struct net_device *bond_dev, struct net_device *slave_dev) { - struct bonding *bond = netdev_priv(bond_dev); - bond_dev->header_ops = slave_dev->header_ops; bond_dev->type = slave_dev->type; @@ -1392,7 +1390,6 @@ static void bond_setup_by_slave(struct net_device *bond_dev, memcpy(bond_dev->broadcast, slave_dev->broadcast, slave_dev->addr_len); - bond->setup_by_slave = 1; } /* On bonding slaves other than the currently active slave, suppress @@ -3187,7 +3184,7 @@ static int bond_slave_netdev_event(unsigned long event, switch (event) { case NETDEV_UNREGISTER: - if (bond->setup_by_slave) + if (bond_dev->type != ARPHRD_ETHER) bond_release_and_destroy(bond_dev, slave_dev); else bond_release(bond_dev, slave_dev); -- cgit v1.2.3 From 97a1e6396b07581249506952a4c417dc6d2a4f9c Mon Sep 17 00:00:00 2001 From: "nikolay@redhat.com" Date: Wed, 26 Jun 2013 17:13:38 +0200 Subject: bonding: remove unnecessary dev_addr_from_first member In struct bonding there's a member called dev_addr_from_first which is used to denote when the bond dev should clone the first slave's MAC address but since we have netdev's addr_assign_type variable that is not necessary. We clone the first slave's MAC each time we have a random MAC set to the bond device. This has the nice side-effect of also fixing an inconsistency - when the MAC address of the bond dev is set after its creation, but prior to having slaves, it's not kept and the first slave's MAC is cloned. The only way to keep the MAC was to create the bond device with the MAC address set (e.g. through ip link). In all cases if the bond device is left without any slaves - its MAC gets reset to a random one as before. Signed-off-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 2e8b9f1e2747..0da3c126c7c4 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1596,7 +1596,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 (bond->slave_cnt == 0 && bond->dev_addr_from_first) + if (!bond->slave_cnt && bond->dev->addr_assign_type == NET_ADDR_RANDOM) bond_set_dev_addr(bond->dev, slave_dev); new_slave = kzalloc(sizeof(struct slave), GFP_KERNEL); @@ -2041,7 +2041,6 @@ static int __bond_release_one(struct net_device *bond_dev, if (bond->slave_cnt == 0) { bond_set_carrier(bond); eth_hw_addr_random(bond_dev); - bond->dev_addr_from_first = true; if (bond_vlan_used(bond)) { pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n", @@ -4800,10 +4799,8 @@ static int bond_init(struct net_device *bond_dev) /* Ensure valid dev_addr */ if (is_zero_ether_addr(bond_dev->dev_addr) && - bond_dev->addr_assign_type == NET_ADDR_PERM) { + bond_dev->addr_assign_type == NET_ADDR_PERM) eth_hw_addr_random(bond_dev); - bond->dev_addr_from_first = true; - } return 0; } -- cgit v1.2.3 From ae0d67505ca30c635f7763564622c9710913f293 Mon Sep 17 00:00:00 2001 From: "nikolay@redhat.com" Date: Wed, 26 Jun 2013 17:13:39 +0200 Subject: bonding: when cloning a MAC use NET_ADDR_STOLEN A simple semantic change, when a slave's MAC is cloned by the bond master then set addr_assign_type to NET_ADDR_STOLEN instead of NET_ADDR_SET. Also use bond_set_dev_addr() in BOND_FOM_ACTIVE mode to change the bond's MAC address because the assign_type has to be set properly. Signed-off-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 0da3c126c7c4..ec44580b076f 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -833,6 +833,24 @@ static void bond_hw_addr_swap(struct bonding *bond, struct slave *new_active, } } +/** + * bond_set_dev_addr - clone slave's address to bond + * @bond_dev: bond net device + * @slave_dev: slave net device + * + * Should be called with RTNL held. + */ +static void bond_set_dev_addr(struct net_device *bond_dev, + struct net_device *slave_dev) +{ + pr_debug("bond_dev=%p\n", bond_dev); + pr_debug("slave_dev=%p\n", slave_dev); + pr_debug("slave_dev->addr_len=%d\n", slave_dev->addr_len); + memcpy(bond_dev->dev_addr, slave_dev->dev_addr, slave_dev->addr_len); + bond_dev->addr_assign_type = NET_ADDR_STOLEN; + call_netdevice_notifiers(NETDEV_CHANGEADDR, bond_dev); +} + /* * bond_do_fail_over_mac * @@ -855,11 +873,9 @@ static void bond_do_fail_over_mac(struct bonding *bond, switch (bond->params.fail_over_mac) { case BOND_FOM_ACTIVE: if (new_active) { - memcpy(bond->dev->dev_addr, new_active->dev->dev_addr, - new_active->dev->addr_len); write_unlock_bh(&bond->curr_slave_lock); read_unlock(&bond->lock); - call_netdevice_notifiers(NETDEV_CHANGEADDR, bond->dev); + bond_set_dev_addr(bond->dev, new_active->dev); read_lock(&bond->lock); write_lock_bh(&bond->curr_slave_lock); } @@ -1290,17 +1306,6 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev) /*---------------------------------- IOCTL ----------------------------------*/ -static void bond_set_dev_addr(struct net_device *bond_dev, - struct net_device *slave_dev) -{ - pr_debug("bond_dev=%p\n", bond_dev); - pr_debug("slave_dev=%p\n", slave_dev); - pr_debug("slave_dev->addr_len=%d\n", slave_dev->addr_len); - memcpy(bond_dev->dev_addr, slave_dev->dev_addr, slave_dev->addr_len); - bond_dev->addr_assign_type = NET_ADDR_SET; - call_netdevice_notifiers(NETDEV_CHANGEADDR, bond_dev); -} - static netdev_features_t bond_fix_features(struct net_device *dev, netdev_features_t features) { -- cgit v1.2.3 From 008aebde9be37e7e1248332b1983976e354327ea Mon Sep 17 00:00:00 2001 From: Nikolay Aleksandrov Date: Sat, 29 Jun 2013 13:16:59 +0200 Subject: bonding: combine pr_debugs in bond_set_dev_addr into one Combine the multiple pr_debugs in bond_set_dev_addr into one pr_debug. Signed-off-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/net/bonding/bond_main.c') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index ec44580b076f..742c193881fa 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -843,9 +843,8 @@ static void bond_hw_addr_swap(struct bonding *bond, struct slave *new_active, static void bond_set_dev_addr(struct net_device *bond_dev, struct net_device *slave_dev) { - pr_debug("bond_dev=%p\n", bond_dev); - pr_debug("slave_dev=%p\n", slave_dev); - pr_debug("slave_dev->addr_len=%d\n", slave_dev->addr_len); + pr_debug("bond_dev=%p slave_dev=%p slave_dev->addr_len=%d\n", + bond_dev, slave_dev, slave_dev->addr_len); memcpy(bond_dev->dev_addr, slave_dev->dev_addr, slave_dev->addr_len); bond_dev->addr_assign_type = NET_ADDR_STOLEN; call_netdevice_notifiers(NETDEV_CHANGEADDR, bond_dev); -- cgit v1.2.3