Configure IPv6 LLADDR only on master L3 HA instance

HA standby routers must never transmit traffic from
any of their ports. This is because we allocate the same
port on all agents. For example, for a given external interface,
we place the same port with the same IP/MAC on every agent
the HA router is scheduled on. Thus, if a standby router
transmits data out of that interface, the physical switches
in the datacenter will re-learn the MAC address of the external
port, and place it on a port that's looking at a standby and
not at the master. This causes 100% packet loss for any incoming
traffic that should be going through the master instance of the
router.

Keepalived manages addresses on the router interfaces, and makes
sure that these addresses only live on the master. However, we
forgot about IPv6 link local addresses. They are generated
from the MAC address of the interface, and thus are identical on
all agents.

This patch tries to treat IPv6 link local addresses the same
as IPv4 addresses - define them as VIPs and let keepalived
move them around.

Closes-Bug: #1403860
Change-Id: Ia5071552239c9444c5105a150b268fb0437e4b85

diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py
index 7335ebcb016c7a0f686373e89ff42cc0d36a279a..c73481e52d6a5b00b4fbc223fecfea0ff51769ca 100644 (file)
--- a/neutron/agent/l3/agent.py
+++ b/neutron/agent/l3/agent.py
@@ -805,6 +805,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
 
         if ri.is_ha:
             self._ha_external_gateway_added(ri, ex_gw_port, interface_name)
+            self._ha_disable_addressing_on_interface(ri, interface_name)
 
     def external_gateway_updated(self, ri, ex_gw_port, interface_name):
         preserve_ips = []
@@ -924,6 +925,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
                                      ri.is_ha)
 
         if ri.is_ha:
+            self._ha_disable_addressing_on_interface(ri, interface_name)
             self._add_vip(ri, internal_cidr, interface_name)
 
         ex_gw_port = self._get_ex_gw_port(ri)

diff --git a/neutron/agent/l3/ha.py b/neutron/agent/l3/ha.py
index 4e006fa50976c5270d3370f3eed68c7425c141ab..e3433ebb2e48558e8bd6cbac67131f7975e5da8d 100644 (file)
--- a/neutron/agent/l3/ha.py
+++ b/neutron/agent/l3/ha.py
@@ -16,8 +16,10 @@
 import os
 import signal
 
+import netaddr
 from oslo.config import cfg
 
+from neutron.agent.linux import ip_lib
 from neutron.agent.linux import keepalived
 from neutron.agent.metadata import driver as metadata_driver
 from neutron.common import constants as l3_constants
@@ -56,14 +58,17 @@ class AgentMixin(object):
         if not os.path.isdir(ha_full_path):
             os.makedirs(ha_full_path, 0o755)
 
-    def _init_keepalived_manager(self, ri):
-        ri.keepalived_manager = keepalived.KeepalivedManager(
+    def get_keepalived_manager(self, ri):
+        return keepalived.KeepalivedManager(
             ri.router['id'],
             keepalived.KeepalivedConf(),
             conf_path=self.conf.ha_confs_path,
             namespace=ri.ns_name,
             root_helper=self.root_helper)
 
+    def _init_keepalived_manager(self, ri):
+        ri.keepalived_manager = self.get_keepalived_manager(ri)
+
         config = ri.keepalived_manager.config
 
         interface_name = self.get_ha_device_name(ri.ha_port['id'])
@@ -122,9 +127,9 @@ class AgentMixin(object):
                            prefix=HA_DEV_PREFIX)
         ri.ha_port = None
 
-    def _add_vip(self, ri, ip_cidr, interface):
+    def _add_vip(self, ri, ip_cidr, interface, scope=None):
         instance = ri.keepalived_manager.config.get_instance(ri.ha_vr_id)
-        instance.add_vip(ip_cidr, interface)
+        instance.add_vip(ip_cidr, interface, scope)
 
     def _remove_vip(self, ri, ip_cidr):
         instance = ri.keepalived_manager.config.get_instance(ri.ha_vr_id)
@@ -171,6 +176,43 @@ class AgentMixin(object):
         self._add_vip(ri, ex_gw_port['ip_cidr'], interface_name)
         self._add_default_gw_virtual_route(ri, ex_gw_port, interface_name)
 
+    def _should_delete_ipv6_lladdr(self, ri, ipv6_lladdr):
+        """Only the master should have any IP addresses configured.
+        Let keepalived manage IPv6 link local addresses, the same way we let
+        it manage IPv4 addresses. In order to do that, we must delete
+        the address first as it is autoconfigured by the kernel.
+        """
+        process = keepalived.KeepalivedManager.get_process(
+            self.conf,
+            ri.router_id,
+            self.root_helper,
+            ri.ns_name,
+            self.conf.ha_confs_path)
+        if process.active:
+            manager = self.get_keepalived_manager(ri)
+            conf = manager.get_conf_on_disk()
+            managed_by_keepalived = conf and ipv6_lladdr in conf
+            if managed_by_keepalived:
+                return False
+        return True
+
+    def _ha_disable_addressing_on_interface(self, ri, interface_name):
+        """Disable IPv6 link local addressing on the device and add it as
+        a VIP to keepalived. This means that the IPv6 link local address
+        will only be present on the master.
+        """
+        device = ip_lib.IPDevice(interface_name, self.root_helper, ri.ns_name)
+        ipv6_lladdr = self._get_ipv6_lladdr(device.link.address)
+
+        if self._should_delete_ipv6_lladdr(ri, ipv6_lladdr):
+            device.addr.flush()
+
+        self._remove_vip(ri, ipv6_lladdr)
+        self._add_vip(ri, ipv6_lladdr, interface_name, scope='link')
+
+    def _get_ipv6_lladdr(self, mac_addr):
+        return '%s/64' % netaddr.EUI(mac_addr).ipv6_link_local()
+
     def _ha_external_gateway_removed(self, ri, interface_name):
         self._clear_vips(ri, interface_name)
 

diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py
index 44bbcd4970b74ae46fd1c0a45987520dde1bc1eb..d8700f9f99ae12eed57a84157f578131247836d9 100644 (file)
--- a/neutron/agent/linux/keepalived.py
+++ b/neutron/agent/linux/keepalived.py
@@ -12,6 +12,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import errno
 import itertools
 import os
 import stat
@@ -79,12 +80,16 @@ class InvalidAuthenticationTypeExecption(exceptions.NeutronException):
 class KeepalivedVipAddress(object):
     """A virtual address entry of a keepalived configuration."""
 
-    def __init__(self, ip_address, interface_name):
+    def __init__(self, ip_address, interface_name, scope=None):
         self.ip_address = ip_address
         self.interface_name = interface_name
+        self.scope = scope
 
     def build_config(self):
-        return '%s dev %s' % (self.ip_address, self.interface_name)
+        result = '%s dev %s' % (self.ip_address, self.interface_name)
+        if self.scope:
+            result += ' scope %s' % self.scope
+        return result
 
 
 class KeepalivedVirtualRoute(object):
@@ -165,8 +170,8 @@ class KeepalivedInstance(object):
 
         self.authentication = (auth_type, password)
 
-    def add_vip(self, ip_cidr, interface_name):
-        self.vips.append(KeepalivedVipAddress(ip_cidr, interface_name))
+    def add_vip(self, ip_cidr, interface_name, scope):
+        self.vips.append(KeepalivedVipAddress(ip_cidr, interface_name, scope))
 
     def remove_vips_vroutes_by_interface(self, interface_name):
         self.vips = [vip for vip in self.vips
@@ -385,15 +390,23 @@ class KeepalivedManager(KeepalivedNotifierMixin):
 
         return config_path
 
+    def get_conf_on_disk(self):
+        config_path = self._get_full_config_file_path('keepalived.conf')
+        try:
+            with open(config_path) as conf:
+                return conf.read()
+        except (OSError, IOError) as e:
+            if e.errno != errno.ENOENT:
+                raise
+
     def spawn(self):
         config_path = self._output_config_file()
 
-        self.process = external_process.ProcessManager(
-            self.conf,
-            self.resource_id,
-            self.root_helper,
-            self.namespace,
-            pids_path=self.conf_path)
+        self.process = self.get_process(self.conf,
+                                        self.resource_id,
+                                        self.root_helper,
+                                        self.namespace,
+                                        self.conf_path)
 
         def callback(pid_file):
             cmd = ['keepalived', '-P',
@@ -432,3 +445,12 @@ class KeepalivedManager(KeepalivedNotifierMixin):
     def revive(self):
         if self.spawned and not self.process.active:
             self.restart()
+
+    @classmethod
+    def get_process(cls, conf, resource_id, root_helper, namespace, conf_path):
+        return external_process.ProcessManager(
+            conf,
+            resource_id,
+            root_helper,
+            namespace,
+            pids_path=conf_path)

diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py
index 9bc806d1c751936f2fb1b36bc36fac88ac52a2cf..7ce844a2131b8cc20bf471844468d0228f2c7df6 100755 (executable)
--- a/neutron/tests/functional/agent/test_l3_agent.py
+++ b/neutron/tests/functional/agent/test_l3_agent.py
@@ -145,10 +145,14 @@ class L3AgentTestFramework(base.BaseOVSLinuxTestCase):
         ha_device_name = self.agent.get_ha_device_name(router.ha_port['id'])
         ha_device_cidr = router.ha_port['ip_cidr']
         external_port = self.agent._get_ex_gw_port(router)
+        ex_port_ipv6 = self.agent._get_ipv6_lladdr(
+            external_port['mac_address'])
         external_device_name = self.agent.get_external_device_name(
             external_port['id'])
         external_device_cidr = external_port['ip_cidr']
         internal_port = router.router[l3_constants.INTERFACE_KEY][0]
+        int_port_ipv6 = self.agent._get_ipv6_lladdr(
+            internal_port['mac_address'])
         internal_device_name = self.agent.get_internal_device_name(
             internal_port['id'])
         internal_device_cidr = internal_port['ip_cidr']
@@ -181,6 +185,8 @@ vrrp_instance VR_1 {
         %(floating_ip_cidr)s dev %(external_device_name)s
         %(external_device_cidr)s dev %(external_device_name)s
         %(internal_device_cidr)s dev %(internal_device_name)s
+        %(ex_port_ipv6)s dev %(external_device_name)s scope link
+        %(int_port_ipv6)s dev %(internal_device_name)s scope link
     }
     virtual_routes {
         0.0.0.0/0 via %(default_gateway_ip)s dev %(external_device_name)s
@@ -195,7 +201,9 @@ vrrp_instance VR_1 {
             'internal_device_name': internal_device_name,
             'internal_device_cidr': internal_device_cidr,
             'floating_ip_cidr': floating_ip_cidr,
-            'default_gateway_ip': default_gateway_ip
+            'default_gateway_ip': default_gateway_ip,
+            'int_port_ipv6': int_port_ipv6,
+            'ex_port_ipv6': ex_port_ipv6
         }
 
     def _get_rule(self, iptables_manager, table, chain, predicate):
@@ -309,6 +317,9 @@ class L3AgentTestCase(L3AgentTestFramework):
         router = self.manage_router(self.agent, router_info)
 
         if enable_ha:
+            port = self.agent._get_ex_gw_port(router)
+            interface_name = self.agent.get_external_device_name(port['id'])
+            self._assert_no_ip_addresses_on_interface(router, interface_name)
             helpers.wait_until_true(lambda: router.ha_state == 'master')
 
             # Keepalived notifies of a state transition when it starts,
@@ -377,6 +388,10 @@ class L3AgentTestCase(L3AgentTestFramework):
             router.router[l3_constants.HA_INTERFACE_KEY],
             self.agent.get_ha_device_name, router.ns_name))
 
+    def _assert_no_ip_addresses_on_interface(self, router, interface):
+        device = ip_lib.IPDevice(interface, self.root_helper, router.ns_name)
+        self.assertEqual([], device.addr.list())
+
 
 class L3HATestFramework(L3AgentTestFramework):
     def setUp(self):

diff --git a/neutron/tests/unit/agent/linux/test_keepalived.py b/neutron/tests/unit/agent/linux/test_keepalived.py
index 5dfe0dc0fdff0bff59d7ec330db24759db212737..7ef58de4976f287650b816f05abb15b899337840 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_keepalived.py
+++ b/neutron/tests/unit/agent/linux/test_keepalived.py
@@ -294,6 +294,15 @@ vrrp_instance VR_2 {
         self.assertEqual(expected, '\n'.join(instance.build_config()))
 
 
+class KeepalivedVipAddressTestCase(base.BaseTestCase):
+    def test_vip_with_scope(self):
+        vip = keepalived.KeepalivedVipAddress('fe80::3e97:eff:fe26:3bfa/64',
+                                              'eth1',
+                                              'link')
+        self.assertEqual('fe80::3e97:eff:fe26:3bfa/64 dev eth1 scope link',
+                         vip.build_config())
+
+
 class KeepalivedVirtualRouteTestCase(base.BaseTestCase):
     def test_virtual_route_with_dev(self):
         route = keepalived.KeepalivedVirtualRoute('0.0.0.0/0', '1.2.3.4',

diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index ae09ba5c4ef048c801ace7bfd6ef10506c0bdefa..bab6af0deacbcbeba117b2fa69d9f19a46564804 100644 (file)
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/neutron/tests/unit/test_l3_agent.py
@@ -75,6 +75,8 @@ def router_append_interface(router, count=1, ip_version=4, ra_mode=None,
         [netaddr.IPNetwork(p['subnet']['cidr']).version == ip_version
          for p in interfaces])
 
+    mac_address = netaddr.EUI('ca:fe:de:ad:be:ef')
+    mac_address.dialect = netaddr.mac_unix
     for i in range(current, current + count):
         interfaces.append(
             {'id': _uuid(),
@@ -82,11 +84,12 @@ def router_append_interface(router, count=1, ip_version=4, ra_mode=None,
              'admin_state_up': True,
              'fixed_ips': [{'ip_address': ip_pool % i,
                             'subnet_id': _uuid()}],
-             'mac_address': 'ca:fe:de:ad:be:ef',
+             'mac_address': str(mac_address),
              'subnet': {'cidr': cidr_pool % i,
                         'gateway_ip': gw_pool % i,
                         'ipv6_ra_mode': ra_mode,
                         'ipv6_address_mode': addr_mode}})
+        mac_address.value += 1
 
 
 def prepare_router_data(ip_version=4, enable_snat=None, num_internal_ports=1,
@@ -104,7 +107,7 @@ def prepare_router_data(ip_version=4, enable_snat=None, num_internal_ports=1,
 
     router_id = _uuid()
     ex_gw_port = {'id': _uuid(),
-                  'mac_address': 'ca:fe:de:ad:be:ef',
+                  'mac_address': 'ca:fe:de:ad:be:ee',
                   'network_id': _uuid(),
                   'fixed_ips': [{'ip_address': ip_addr,
                                  'subnet_id': _uuid()}],