bonding: send IPv6 neighbor advertisement on failover

This patch adds better IPv6 failover support for bonding devices, especially when in active-backup mode and there are only IPv6 addresses configured, as reported by Alex Sidorenko.

- Creates a new file, net/drivers/bonding/bond_ipv6.c, for the IPv6-specific routines. Both regular bonds and VLANs over bonds are supported.

- Adds a new tunable, num_unsol_na, to limit the number of unsolicited IPv6 Neighbor Advertisements that are sent on a failover event. Default is 1.

- Creates two new IPv6 neighbor discovery functions:

ndisc_build_skb()
ndisc_send_skb()

These were required to support VLANs since we have to be able to add the VLAN id to the skb since ndisc_send_na() and friends shouldn't be asked to do this. These two routines are basically __ndisc_send() split into two pieces, in a slightly different order.

- Updates Documentation/networking/bonding.txt and bumps the rev of bond support to 3.4.0.

On failover, this new code will generate one packet:

- An unsolicited IPv6 Neighbor Advertisement, which helps the switch learn that the address has moved to the new slave.

Testing has shown that sending just the NA results in pretty good behavior when in active-back mode, I saw no lost ping packets for example.

-Brian

Signed-off-by Brian Haley <[email protected]>

Brian,
I'll make the same comment I did in the earlier version,
which is I don't think we need a new control for the count. Instead, it should use the count for unsolicited NA's we send when adding an address. It is the same case, really, and I, FWIW, would rather we didn't have any more sysctl's than we need.

                                                +-DLS

Brian,
I'll make the same comment I did in the earlier version,
which is I don't think we need a new control for the count. Instead, it should use the count for unsolicited NA's we send when adding an address. It is the same case, really, and I, FWIW, would rather we didn't have any more sysctl's than we need.

                                                +-DLS

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Well, actually, it looks like I'm suggesting you to re-use something that doesn't exist. :-)

MLD (and IGMP) has such a thing where unsolicited advertisements are sent multiple times, with delays in between, to account for lossy networks possibly dropping the first one. There are configurable counts associated with probes and retransmit intervals for solicits, but I don't see the equivalent yet for unsolicited NA's.

So, instead, what I suggest is that you add (or find!) THAT knob, instead of a bonding-specific one. Because adding an address that wasn't there before has identical issues with unsolicited NA's as bonding has with activating a new address. The default should probably be 1, but if you ever need to send multiple unsolicited NA's for bonding, you probably also need it for adding a normal address on the same network. dad_transmits is similar, but not really the same thing.

Quoted 4 lines · David Stevens

Well, actually, it looks like I'm suggesting you to re-use something that doesn't
exist. :-)

Quoted 8 lines

MLD (and IGMP) has such a thing where unsolicited advertisements are sent multiple times, with delays in between, to account for lossy networks possibly
dropping the first one. There are configurable counts associated with probes
and retransmit intervals for solicits, but I don't see the equivalent yet for
unsolicited NA's.

I don't see an equivalent either, since the only unsolicited NA the kernel sends is for DAD, which uses dad_transmits.

Quoted 4 lines · Brian Haley

Quoted 3 lines · David Stevens

Well, actually, it looks like I'm suggesting you to re-use something that doesn't exist. :-)

Quoted 6 lines

Quoted 5 lines

MLD (and IGMP) has such a thing where unsolicited advertisements are sent multiple times, with delays in between, to account for lossy networks possibly dropping the first one. There are configurable counts associated with probes and retransmit intervals for solicits, but I don't see the equivalent yet for unsolicited NA's.

I don't see an equivalent either, since the only unsolicited NA the kernel sends is for DAD, which uses dad_transmits.

Doesn't DAD use neighbor solicitation rather than unsolicited NA?

Can we use NS in the bonding failover scenario too?

Well, actually, it looks like I'm suggesting you to re-use something that doesn't
exist. :-)

MLD (and IGMP) has such a thing where unsolicited advertisements are sent multiple times, with delays in between, to account for lossy networks possibly
dropping the first one. There are configurable counts associated with probes
and retransmit intervals for solicits, but I don't see the equivalent yet for
unsolicited NA's.

So, instead, what I suggest is that you add (or find!) THAT knob, instead of a
bonding-specific one. Because adding an address that wasn't there before has identical issues with unsolicited NA's as bonding has with activating a
new address.

The default should probably be 1, but if you ever need to send multiple unsolicited NA's for bonding, you probably also need it for adding a normal address on the same network. dad_transmits is similar, but not really the same thing.

                                                        +-DLS

Well, I think the reason to send mulitple of them is identical. If one is dropped due to network load, it won't happen; sending multiple increases the odds of success.

DAD itself should update caches for neighboring nodes, so I guess it makes sense that it isn't sending unsolicited NA's, But that makes me think that the DAD retransmit counter is the one you want. At least, the part of the DAD retransmit counter that is for updating other nodes' caches. :-)

For MLD and IGMP, they were explicit SHOULD's-- I need to have a look at ND RFC's to again to see what it says about it.

I don't think that alone is a reason to block the patch, but I also don't think that updating neighbor caches with a new MAC address is a unique requirement of bonding.

Moving an address manually
ought to be identical in needs and behavior, as well as very-quick reboots where the hardware changed. Thus, I don't think the knob ought to be specific to bonding. I guess that leads to the suggestion that you re-use the DAD counter for that.

References to MLD now and before are just me looking for an analog to what ND should be doing. No new knob is definitely required for them, since they already have this support for unsolicited reports.

index 688dfe1..4025565 100644

Documentation/networking/bonding.txt +10 -0

@@ -551,6 +551,16 @@ num_grat_arp
 	affects only the active-backup mode.  This option was added for
 	bonding version 3.3.0.
 
+num_unsol_na
+
+	Specifies the number of unsolicited IPv6 Neighbor Advertisements
+	to be issued after a failover event.  One unsolicited NA is issued
+	immediately after the failover.
+
+	The valid range is 0 - 255; the default value is 1.  This option
+	affects only the active-backup mode.  This option was added for
+	bonding version 3.4.0.
+
 primary
 
 	A string (eth0, eth2, etc) specifying which slave is the

drivers/net/Kconfig +1 -0

index 2d6a060..37f55e1 100644
@@ -61,6 +61,7 @@ config DUMMY
 config BONDING
 	tristate "Bonding driver support"
 	depends on INET
+	depends on IPV6 || IPV6=n
 	---help---
 	  Say 'Y' or 'M' if you wish to be able to 'bond' multiple Ethernet
 	  Channels together. This is called 'Etherchannel' by Cisco,

drivers/net/bonding/Makefile +3 -0

index 5cdae2b..6f9c6fa 100644
@@ -6,3 +6,6 @@ obj-$(CONFIG_BONDING) += bonding.o
 
 bonding-objs := bond_main.o bond_3ad.o bond_alb.o bond_sysfs.o
 
+ipv6-$(subst m,y,$(CONFIG_IPV6)) += bond_ipv6.o
+bonding-objs += $(ipv6-y)
+

drivers/net/bonding/bond_ipv6.c new file +153 -0

new file mode 100644
index 0000000..b6b0351
@@ -0,0 +1,208 @@
+/*
+ * Copyright(c) 2008 Hewlett-Packard Development Company, L.P.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+ *
+ * The full GNU General Public License is included in this distribution in the
+ * file called LICENSE.
+ *
+ */
+
+//#define BONDING_DEBUG 1
+
+#include <linux/types.h>
+#include <linux/if_vlan.h>
+#include <net/ipv6.h>
+#include <net/ndisc.h>
+#include <net/addrconf.h>
+#include "bonding.h"
+
+/*
+ * Assign bond->master_ipv6 to the next IPv6 address in the list, or
+ * zero it out if there are none.
+ */
+static void bond_glean_dev_ipv6(struct net_device *dev, struct in6_addr *addr)
+{
+	struct inet6_dev *idev;
+	struct inet6_ifaddr *ifa;
+
+	if (!dev)
+		return;
+
+	idev = in6_dev_get(dev);
+	if (!idev)
+		return;
+
+	ifa = idev->addr_list;
+	if (ifa)
+		ipv6_addr_copy(addr, &ifa->addr);
+	else
+		ipv6_addr_set(addr, 0, 0, 0, 0);
+
+	in6_dev_put(idev);
+}
+
+static void bond_na_send(struct net_device *slave_dev,
+			 struct in6_addr *daddr,
+			 int router,
+			 unsigned short vlan_id)
+{
+	struct in6_addr mcaddr;
+	struct icmp6hdr icmp6h = {
+		.icmp6_type = NDISC_NEIGHBOUR_ADVERTISEMENT,
+	};
+	struct sk_buff *skb;
+
+	icmp6h.icmp6_router = router;
+	icmp6h.icmp6_solicited = 0;
+	icmp6h.icmp6_override = 1;
+
+	addrconf_addr_solict_mult(daddr, &mcaddr);
+
+	dprintk("ipv6 na on slave %s: dest %s, src %s\n" NIP6_FMT NIP6_FMT,
+	       slave->name, NIP6(&mcaddr), NIP6(daddr));
+
+	skb = ndisc_build_skb(slave_dev, &mcaddr, daddr, &icmp6h, daddr,
+			      ND_OPT_TARGET_LL_ADDR);
+
+	if (!skb) {
+		printk(KERN_ERR DRV_NAME ": NA packet allocation failed\n");
+		return;
+	}
+
+	if (vlan_id) {
+		skb = vlan_put_tag(skb, vlan_id);
+		if (!skb) {
+			printk(KERN_ERR DRV_NAME ": failed to insert VLAN tag\n");
+			return;
+		}
+	}
+
+	ndisc_send_skb(skb, slave_dev, NULL, &mcaddr, daddr, &icmp6h);
+}
+
+/*
+ * Kick out an unsolicited Neighbor Advertisement for an IPv6 address on
+ * the bonding master.  This will help the switch learn our address
+ * if in active-backup mode.
+ *
+ * Caller must hold curr_slave_lock for read or better
+ */
+void bond_send_unsolicited_na(struct bonding *bond)
+{
+	struct slave *slave = bond->curr_active_slave;
+	struct vlan_entry *vlan;
+	struct inet6_dev *idev;
+	int is_router;
+
+	dprintk("bond_send_unsol_na: bond %s slave %s\n", bond->dev->name,
+				slave ? slave->dev->name : "NULL");
+
+	if (!slave || !bond->send_unsol_na ||
+	    test_bit(__LINK_STATE_LINKWATCH_PENDING, &slave->dev->state))
+		return;
+
+	bond->send_unsol_na--;
+
+	idev = in6_dev_get(bond->dev);
+	if (!idev)
+		return;
+
+	is_router = !!idev->cnf.forwarding;
+
+	in6_dev_put(idev);
+
+	if (!ipv6_addr_any(&bond->master_ipv6))
+		bond_na_send(slave->dev, &bond->master_ipv6, is_router, 0);
+
+	list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
+		if (!ipv6_addr_any(&vlan->vlan_ipv6)) {
+			bond_na_send(slave->dev, &vlan->vlan_ipv6, is_router,
+				     vlan->vlan_id);
+		}
+	}
+}
+
+/*
+ * bond_inet6addr_event: handle inet6addr notifier chain events.
+ *
+ * We keep track of device IPv6 addresses primarily to use as source
+ * addresses in NS probes.
+ *
+ * We track one IPv6 for the main device (if it has one).
+ */
+static int bond_inet6addr_event(struct notifier_block *this,
+				unsigned long event,
+				void *ptr)
+{
+	struct inet6_ifaddr *ifa = ptr;
+	struct net_device *vlan_dev, *event_dev = ifa->idev->dev;
+	struct bonding *bond;
+	struct vlan_entry *vlan;

drivers/net/bonding/bond_ipv6.c new file +4 -0

+			default:
+				return NOTIFY_DONE;
+			}
+		}

drivers/net/bonding/bond_ipv6.c new file +13 -0

+				default:
+					return NOTIFY_DONE;
+				}
+			}
+		}
+	}
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block bond_inet6addr_notifier = {
+	.notifier_call = bond_inet6addr_event,
+};
+

Quoted 5 lines

drivers/net/bonding/bond_ipv6.c new file +5 -0

+
+	list_for_each_entry(bond, &bond_dev_list, bond_list) {
+		if (bond->dev == event_dev) {
+			switch (event) {
+			case NETDEV_UP:

Review

Quoted 2 lines

(quoted diff) +2 -0

+				ipv6_addr_copy(&bond->master_ipv6, &ifa->addr);
+				return NOTIFY_OK;

I think you want to store the first address configured on the device (most likely link-local), and not overwrite it every time a new address is configured. Since new addresses can be configured rather often (think temporary, new RAs, etc) we really want the most stable address we can have. Also, since ND is a link protocol, link-local is sufficient.

Quoted 4 lines · Vlad Yasevich

Quoted 4 lines

drivers/net/bonding/bond_ipv6.c new file +4 -0

+
+	list_for_each_entry(bond, &bond_dev_list, bond_list) {
+		if (bond->dev == event_dev) {
+			switch (event) {

Quoted 8 lines

drivers/net/bonding/bond_ipv6.c line 163

Quoted 3 lines

drivers/net/bonding/bond_ipv6.c new file +3 -0

+			case NETDEV_UP:
+				ipv6_addr_copy(&bond->master_ipv6, &ifa->addr);
+				return NOTIFY_OK;

I think you want to store the first address configured on the device (most likely link-local), and not overwrite it every time a new address is configured. Since new addresses can be configured rather often (think temporary, new RAs, etc) we really want the most stable address we can have. Also, since ND is a link protocol, link-local is sufficient.

That depends upon how the IPv6 unsolicited NAs are handled by
the switch. For IPv4, we issue a gratuitous ARP for one of the IP addresses on the interface to update the switch's MAC table; for this case, it doesn't matter which IP address is used.

If IPv6-smart switches snoop the same way, then it again doesn't
matter which IPv6 address is used; this is just to update the MAC table. I'll agree that it's logically sensible to use a link-local, though. If, on the other hand, IPv6 needs an update for each configured address, then storing just one IPv6 address is insufficient (as we'd need an NA for each address).

Quoted 4 lines · Vlad Yasevich

Quoted 4 lines

drivers/net/bonding/bond_ipv6.c new file +4 -0

+
+	list_for_each_entry(bond, &bond_dev_list, bond_list) {
+		if (bond->dev == event_dev) {
+			switch (event) {

Quoted 8 lines

drivers/net/bonding/bond_ipv6.c line 163

Quoted 3 lines

drivers/net/bonding/bond_ipv6.c new file +3 -0

+			case NETDEV_UP:
+				ipv6_addr_copy(&bond->master_ipv6, &ifa->addr);
+				return NOTIFY_OK;

I think you want to store the first address configured on the device (most likely link-local), and not overwrite it every time a new address is configured. Since new addresses can be configured rather often (think temporary, new RAs, etc) we really want the most stable address we can have. Also, since ND is a link protocol, link-local is sufficient.

That depends upon how the IPv6 unsolicited NAs are handled by
the switch. For IPv4, we issue a gratuitous ARP for one of the IP addresses on the interface to update the switch's MAC table; for this case, it doesn't matter which IP address is used.

If IPv6-smart switches snoop the same way, then it again doesn't
matter which IPv6 address is used; this is just to update the MAC table. I'll agree that it's logically sensible to use a link-local, though. If, on the other hand, IPv6 needs an update for each configured address, then storing just one IPv6 address is insufficient (as we'd need an NA for each address).

	-J

---
-Jay Vosburgh, IBM Linux Technology Center, [email protected]

Quoted 20 lines · Jay Vosburgh

Quoted 4 lines · Vlad Yasevich

Quoted 4 lines

drivers/net/bonding/bond_ipv6.c new file +4 -0

+
+	list_for_each_entry(bond, &bond_dev_list, bond_list) {
+		if (bond->dev == event_dev) {
+			switch (event) {

Quoted 8 lines

drivers/net/bonding/bond_ipv6.c line 163

Quoted 3 lines

drivers/net/bonding/bond_ipv6.c new file +3 -0

+			case NETDEV_UP:
+				ipv6_addr_copy(&bond->master_ipv6, &ifa->addr);
+				return NOTIFY_OK;

I think you want to store the first address configured on the device (most likely link-local), and not overwrite it every time a new address is configured. Since new addresses can be configured rather often (think temporary, new RAs, etc) we really want the most stable address we can have. Also, since ND is a link protocol, link-local is sufficient.

That depends upon how the IPv6 unsolicited NAs are handled by
the switch. For IPv4, we issue a gratuitous ARP for one of the IP addresses on the interface to update the switch's MAC table; for this case, it doesn't matter which IP address is used.

Quoted 7 lines

If IPv6-smart switches snoop the same way, then it again doesn't
matter which IPv6 address is used; this is just to update the MAC table. I'll agree that it's logically sensible to use a link-local, though. If, on the other hand, IPv6 needs an update for each configured address, then storing just one IPv6 address is insufficient (as we'd need an NA for each address).

Yes, but the unsolicited NA for the global address just looks rather strange when the link local one is provide. Also, with temporaries that can come and go, it's better to use a stable address.

We are simply using it to refresh the MAC tables and for a while I thought it would be sufficient to do just one ARP or ND, but then I realized that in an environment where 2 systems are connected back-to-back, you would potentially need to do both. Need to play with this config...