Fix floating ips with external gateway

If dnsmasq is configured to use an external gateway, then floating
ips on other interfaces do not work properly. This is because
outgoing traffic is no longer snatted to the floating ip.

This patch fixes it by adding an ebtables rule to force traffic
from ips that have a floating ip associated to route instead
of bridge.

Fixes bug 1096985

Change-Id: I8e4904660d42fe51c44b66686bed9f5d622693bd

9 files changed, 115 insertions, 77 deletions

diff --git a/nova/db/api.py b/nova/db/api.py
index d8a16c52d..d409ec5f7 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -305,7 +305,7 @@ def floating_ip_destroy(context, address):
 def floating_ip_disassociate(context, address):
     """Disassociate a floating ip from a fixed ip by address.
 
-    :returns: the address of the previous fixed ip or None
+    :returns: the fixed ip record joined to network record or None
               if the ip was not associated to an ip.
 
     """
@@ -316,7 +316,7 @@ def floating_ip_fixed_ip_associate(context, floating_address,
                                    fixed_address, host):
     """Associate a floating ip to a fixed_ip by address.
 
-    :returns: the address of the new fixed ip (fixed_address) or None
+    :returns: the fixed ip record joined to network record or None
               if the ip was already associated to the fixed ip.
     """
 
@@ -477,9 +477,12 @@ def fixed_ip_disassociate_all_by_timeout(context, host, time):
     return IMPL.fixed_ip_disassociate_all_by_timeout(context, host, time)
 
 
-def fixed_ip_get(context, id):
-    """Get fixed ip by id or raise if it does not exist."""
-    return IMPL.fixed_ip_get(context, id)
+def fixed_ip_get(context, id, get_network=False):
+    """Get fixed ip by id or raise if it does not exist.
+
+    If get_network is true, also return the assocated network.
+    """
+    return IMPL.fixed_ip_get(context, id, get_network)
 
 
 def fixed_ip_get_all(context):
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index cc83ec4f5..895f3f03e 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -775,15 +775,16 @@ def floating_ip_fixed_ip_associate(context, floating_address,
         floating_ip_ref = _floating_ip_get_by_address(context,
                                                       floating_address,
                                                       session=session)
-        fixed_ip_ref = fixed_ip_get_by_address(context,
-                                               fixed_address,
-                                               session=session)
+        fixed_ip_ref = model_query(context, models.FixedIp, session=session).\
+                         filter_by(address=fixed_address).\
+                         options(joinedload('network')).\
+                         first()
         if floating_ip_ref.fixed_ip_id == fixed_ip_ref["id"]:
             return None
         floating_ip_ref.fixed_ip_id = fixed_ip_ref["id"]
         floating_ip_ref.host = host
         floating_ip_ref.save(session=session)
-        return fixed_address
+        return fixed_ip_ref
 
 
 @require_context
@@ -816,15 +817,12 @@ def floating_ip_disassociate(context, address):
 
         fixed_ip_ref = model_query(context, models.FixedIp, session=session).\
                             filter_by(id=floating_ip_ref['fixed_ip_id']).\
+                            options(joinedload('network')).\
                             first()
-        if fixed_ip_ref:
-            fixed_ip_address = fixed_ip_ref['address']
-        else:
-            fixed_ip_address = None
         floating_ip_ref.fixed_ip_id = None
         floating_ip_ref.host = None
         floating_ip_ref.save(session=session)
-    return fixed_ip_address
+    return fixed_ip_ref
 
 
 @require_context
@@ -1124,10 +1122,11 @@ def fixed_ip_disassociate_all_by_timeout(context, host, time):
 
 
 @require_context
-def fixed_ip_get(context, id):
-    result = model_query(context, models.FixedIp).\
-                     filter_by(id=id).\
-                     first()
+def fixed_ip_get(context, id, get_network=False):
+    query = model_query(context, models.FixedIp).filter_by(id=id)
+    if get_network:
+        query = query.options(joinedload('network'))
+    result = query.first()
     if not result:
         raise exception.FixedIpNotFound(id=id)
 
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 5050cb77e..406863f18 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -717,6 +717,12 @@ class FixedIp(BASE, NovaBase):
     leased = Column(Boolean, default=False)
     reserved = Column(Boolean, default=False)
     host = Column(String(255))
+    network = relationship(Network,
+                           backref=backref('fixed_ips'),
+                           foreign_keys=network_id,
+                           primaryjoin='and_('
+                                'FixedIp.network_id == Network.id,'
+                                'FixedIp.deleted == False)')
 
 
 class FloatingIp(BASE, NovaBase):
diff --git a/nova/network/l3.py b/nova/network/l3.py
index baf77c112..14abf41eb 100644
--- a/nova/network/l3.py
+++ b/nova/network/l3.py
@@ -48,13 +48,16 @@ class L3Driver(object):
         """:returns: True/False (whether the driver is initialized)."""
         raise NotImplementedError()
 
-    def add_floating_ip(self, floating_ip, fixed_ip, l3_interface_id):
+    def add_floating_ip(self, floating_ip, fixed_ip, l3_interface_id,
+                        network=None):
         """Add a floating IP bound to the fixed IP with an optional
            l3_interface_id.  Some drivers won't care about the
-           l3_interface_id so just pass None in that case"""
+           l3_interface_id so just pass None in that case. Network
+           is also an optional parameter."""
         raise NotImplementedError()
 
-    def remove_floating_ip(self, floating_ip, fixed_ip, l3_interface_id):
+    def remove_floating_ip(self, floating_ip, fixed_ip, l3_interface_id,
+                           network=None):
         raise NotImplementedError()
 
     def add_vpn(self, public_ip, port, private_ip):
@@ -96,15 +99,17 @@ class LinuxNetL3(L3Driver):
     def remove_gateway(self, network_ref):
         linux_net.unplug(network_ref)
 
-    def add_floating_ip(self, floating_ip, fixed_ip, l3_interface_id):
+    def add_floating_ip(self, floating_ip, fixed_ip, l3_interface_id,
+                        network=None):
         linux_net.bind_floating_ip(floating_ip, l3_interface_id)
         linux_net.ensure_floating_forward(floating_ip, fixed_ip,
-                                          l3_interface_id)
+                                          l3_interface_id, network)
 
-    def remove_floating_ip(self, floating_ip, fixed_ip, l3_interface_id):
+    def remove_floating_ip(self, floating_ip, fixed_ip, l3_interface_id,
+                           network=None):
         linux_net.unbind_floating_ip(floating_ip, l3_interface_id)
         linux_net.remove_floating_forward(floating_ip, fixed_ip,
-                                          l3_interface_id)
+                                          l3_interface_id, network)
 
     def add_vpn(self, public_ip, port, private_ip):
         linux_net.ensure_vpn_forward(public_ip, port, private_ip)
@@ -140,10 +145,12 @@ class NullL3(L3Driver):
     def remove_gateway(self, network_ref):
         pass
 
-    def add_floating_ip(self, floating_ip, fixed_ip, l3_interface_id):
+    def add_floating_ip(self, floating_ip, fixed_ip, l3_interface_id,
+                        network=None):
         pass
 
-    def remove_floating_ip(self, floating_ip, fixed_ip, l3_interface_id):
+    def remove_floating_ip(self, floating_ip, fixed_ip, l3_interface_id,
+                           network=None):
         pass
 
     def add_vpn(self, public_ip, port, private_ip):
diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 1be06f66a..b25008cf8 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -645,18 +645,29 @@ def ensure_vpn_forward(public_ip, port, private_ip):
     iptables_manager.apply()
 
 
-def ensure_floating_forward(floating_ip, fixed_ip, device):
+def ensure_floating_forward(floating_ip, fixed_ip, device, network):
     """Ensure floating ip forwarding rule."""
     for chain, rule in floating_forward_rules(floating_ip, fixed_ip, device):
         iptables_manager.ipv4['nat'].add_rule(chain, rule)
     iptables_manager.apply()
+    if device != network['bridge']:
+        ensure_ebtables_rules(*floating_ebtables_rules(fixed_ip, network))
 
 
-def remove_floating_forward(floating_ip, fixed_ip, device):
+def remove_floating_forward(floating_ip, fixed_ip, device, network):
     """Remove forwarding for floating ip."""
     for chain, rule in floating_forward_rules(floating_ip, fixed_ip, device):
         iptables_manager.ipv4['nat'].remove_rule(chain, rule)
     iptables_manager.apply()
+    if device != network['bridge']:
+        remove_ebtables_rules(*floating_ebtables_rules(fixed_ip, network))
+
+
+def floating_ebtables_rules(fixed_ip, network):
+    """Makes sure only in-network traffic is bridged."""
+    return (['PREROUTING --logical-in %s -p ipv4 --ip-src %s '
+            '! --ip-dst %s -j redirect --redirect-target ACCEPT' %
+            (network['bridge'], fixed_ip, network['cidr'])], 'nat')
 
 
 def floating_forward_rules(floating_ip, fixed_ip, device):
@@ -1387,18 +1398,18 @@ class LinuxBridgeInterfaceDriver(LinuxNetInterfaceDriver):
 
 
 @lockutils.synchronized('ebtables', 'nova-', external=True)
-def ensure_ebtables_rules(rules):
+def ensure_ebtables_rules(rules, table='filter'):
     for rule in rules:
-        cmd = ['ebtables', '-D'] + rule.split()
+        cmd = ['ebtables', '-t', table, '-D'] + rule.split()
         _execute(*cmd, check_exit_code=False, run_as_root=True)
-        cmd[1] = '-I'
+        cmd[3] = '-I'
         _execute(*cmd, run_as_root=True)
 
 
 @lockutils.synchronized('ebtables', 'nova-', external=True)
-def remove_ebtables_rules(rules):
+def remove_ebtables_rules(rules, table='filter'):
     for rule in rules:
-        cmd = ['ebtables', '-D'] + rule.split()
+        cmd = ['ebtables', '-t', table, '-D'] + rule.split()
         _execute(*cmd, check_exit_code=False, run_as_root=True)
 
 
diff --git a/nova/network/manager.py b/nova/network/manager.py
index e4a97f162..222364a65 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -317,17 +317,19 @@ class FloatingIP(object):
             fixed_ip_id = floating_ip.get('fixed_ip_id')
             if fixed_ip_id:
                 try:
-                    fixed_ip_ref = self.db.fixed_ip_get(admin_context,
-                                                        fixed_ip_id)
+                    fixed_ip = self.db.fixed_ip_get(admin_context,
+                                                    fixed_ip_id,
+                                                    get_network=True)
                 except exception.FixedIpNotFound:
                     msg = _('Fixed ip %(fixed_ip_id)s not found') % locals()
                     LOG.debug(msg)
                     continue
-                fixed_address = fixed_ip_ref['address']
                 interface = CONF.public_interface or floating_ip['interface']
                 try:
                     self.l3driver.add_floating_ip(floating_ip['address'],
-                            fixed_address, interface)
+                                                  fixed_ip['address'],
+                                                  interface,
+                                                  fixed_ip['network'])
                 except exception.ProcessExecutionError:
                     LOG.debug(_('Interface %(interface)s not found'), locals())
                     raise exception.NoFloatingIpInterface(interface=interface)
@@ -587,17 +589,17 @@ class FloatingIP(object):
         @lockutils.synchronized(unicode(floating_address), 'nova-')
         def do_associate():
             # associate floating ip
-            res = self.db.floating_ip_fixed_ip_associate(context,
-                                                         floating_address,
-                                                         fixed_address,
-                                                         self.host)
-            if not res:
+            fixed = self.db.floating_ip_fixed_ip_associate(context,
+                                                           floating_address,
+                                                           fixed_address,
+                                                           self.host)
+            if not fixed:
                 # NOTE(vish): ip was already associated
                 return
             try:
                 # gogo driver time
                 self.l3driver.add_floating_ip(floating_address, fixed_address,
-                        interface)
+                        interface, fixed['network'])
             except exception.ProcessExecutionError as e:
                 self.db.floating_ip_disassociate(context, floating_address)
                 if "Cannot find device" in str(e):
@@ -681,15 +683,15 @@ class FloatingIP(object):
             #             don't worry about this case because the miniscule
             #             window where the ip is on both hosts shouldn't cause
             #             any problems.
-            fixed_address = self.db.floating_ip_disassociate(context, address)
+            fixed = self.db.floating_ip_disassociate(context, address)
 
-            if not fixed_address:
+            if not fixed:
                 # NOTE(vish): ip was already disassociated
                 return
             if interface:
                 # go go driver time
-                self.l3driver.remove_floating_ip(address, fixed_address,
-                                                 interface)
+                self.l3driver.remove_floating_ip(address, fixed['address'],
+                                                 interface, fixed['network'])
             payload = dict(project_id=context.project_id,
                            instance_id=instance_uuid,
                            floating_ip=address)
@@ -762,10 +764,12 @@ class FloatingIP(object):
 
             interface = CONF.public_interface or floating_ip['interface']
             fixed_ip = self.db.fixed_ip_get(context,
-                                            floating_ip['fixed_ip_id'])
+                                            floating_ip['fixed_ip_id'],
+                                            get_network=True)
             self.l3driver.remove_floating_ip(floating_ip['address'],
                                              fixed_ip['address'],
-                                             interface)
+                                             interface,
+                                             fixed_ip['network'])
 
             # NOTE(wenjianhn): Make this address will not be bound to public
             # interface when restarts nova-network on dest compute node
@@ -804,10 +808,12 @@ class FloatingIP(object):
 
             interface = CONF.public_interface or floating_ip['interface']
             fixed_ip = self.db.fixed_ip_get(context,
-                                            floating_ip['fixed_ip_id'])
+                                            floating_ip['fixed_ip_id'],
+                                            get_network=True)
             self.l3driver.add_floating_ip(floating_ip['address'],
                                           fixed_ip['address'],
-                                          interface)
+                                          interface,
+                                          fixed_ip['network'])
 
     def _prepare_domain_entry(self, context, domain):
         domainref = self.db.dnsdomain_get(context, domain)
diff --git a/nova/tests/network/test_linux_net.py b/nova/tests/network/test_linux_net.py
index 8a7865b83..3c219f5f4 100644
--- a/nova/tests/network/test_linux_net.py
+++ b/nova/tests/network/test_linux_net.py
@@ -461,14 +461,14 @@ class LinuxNetworkTestCase(test.TestCase):
                    'bridge_interface': iface}
         driver.plug(network, 'fakemac')
         expected = [
-            ('ebtables', '-D', 'INPUT', '-p', 'ARP', '-i', iface,
-             '--arp-ip-dst', dhcp, '-j', 'DROP'),
-            ('ebtables', '-I', 'INPUT', '-p', 'ARP', '-i', iface,
-             '--arp-ip-dst', dhcp, '-j', 'DROP'),
-            ('ebtables', '-D', 'OUTPUT', '-p', 'ARP', '-o', iface,
-             '--arp-ip-src', dhcp, '-j', 'DROP'),
-            ('ebtables', '-I', 'OUTPUT', '-p', 'ARP', '-o', iface,
-             '--arp-ip-src', dhcp, '-j', 'DROP'),
+            ('ebtables', '-t', 'filter', '-D', 'INPUT', '-p', 'ARP', '-i',
+             iface, '--arp-ip-dst', dhcp, '-j', 'DROP'),
+            ('ebtables', '-t', 'filter', '-I', 'INPUT', '-p', 'ARP', '-i',
+             iface, '--arp-ip-dst', dhcp, '-j', 'DROP'),
+            ('ebtables', '-t', 'filter', '-D', 'OUTPUT', '-p', 'ARP', '-o',
+             iface, '--arp-ip-src', dhcp, '-j', 'DROP'),
+            ('ebtables', '-t', 'filter', '-I', 'OUTPUT', '-p', 'ARP', '-o',
+             iface, '--arp-ip-src', dhcp, '-j', 'DROP'),
             ('iptables-save', '-c'),
             ('iptables-restore', '-c'),
             ('ip6tables-save', '-c'),
@@ -500,10 +500,10 @@ class LinuxNetworkTestCase(test.TestCase):
 
         driver.unplug(network)
         expected = [
-            ('ebtables', '-D', 'INPUT', '-p', 'ARP', '-i', iface,
-             '--arp-ip-dst', dhcp, '-j', 'DROP'),
-            ('ebtables', '-D', 'OUTPUT', '-p', 'ARP', '-o', iface,
-             '--arp-ip-src', dhcp, '-j', 'DROP'),
+            ('ebtables', '-t', 'filter', '-D', 'INPUT', '-p', 'ARP', '-i',
+             iface, '--arp-ip-dst', dhcp, '-j', 'DROP'),
+            ('ebtables', '-t', 'filter', '-D', 'OUTPUT', '-p', 'ARP', '-o',
+             iface, '--arp-ip-src', dhcp, '-j', 'DROP'),
             ('iptables-save', '-c'),
             ('iptables-restore', '-c'),
             ('ip6tables-save', '-c'),
diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py
index 1552630fb..d4201612a 100644
--- a/nova/tests/network/test_manager.py
+++ b/nova/tests/network/test_manager.py
@@ -673,7 +673,7 @@ class VlanNetworkTestCase(test.TestCase):
                                       is_admin=False)
 
         def fake1(*args, **kwargs):
-            return '10.0.0.1'
+            return {'address': '10.0.0.1', 'network': 'fakenet'}
 
         # floating ip that's already associated
         def fake2(*args, **kwargs):
@@ -793,9 +793,9 @@ class VlanNetworkTestCase(test.TestCase):
         self.stubs.Set(self.network.db, 'floating_ip_get_all_by_host',
                        get_all_by_host)
 
-        def fixed_ip_get(_context, fixed_ip_id):
+        def fixed_ip_get(_context, fixed_ip_id, get_network):
             if fixed_ip_id == 1:
-                return {'address': 'fakefixed'}
+                return {'address': 'fakefixed', 'network': 'fakenet'}
             raise exception.FixedIpNotFound(id=fixed_ip_id)
         self.stubs.Set(self.network.db, 'fixed_ip_get', fixed_ip_get)
 
@@ -803,7 +803,8 @@ class VlanNetworkTestCase(test.TestCase):
         self.flags(public_interface=False)
         self.network.l3driver.add_floating_ip('fakefloat',
                                               'fakefixed',
-                                              'fakeiface')
+                                              'fakeiface',
+                                              'fakenet')
         self.mox.ReplayAll()
         self.network.init_host_floating_ips()
         self.mox.UnsetStubs()
@@ -813,7 +814,8 @@ class VlanNetworkTestCase(test.TestCase):
         self.flags(public_interface='fooiface')
         self.network.l3driver.add_floating_ip('fakefloat',
                                               'fakefixed',
-                                              'fooiface')
+                                              'fooiface',
+                                              'fakenet')
         self.mox.ReplayAll()
         self.network.init_host_floating_ips()
         self.mox.UnsetStubs()
@@ -1812,11 +1814,13 @@ class FloatingIPTestCase(test.TestCase):
         def fake_is_stale_floating_ip_address(context, floating_ip):
             return floating_ip['address'] == '172.24.4.23'
 
-        def fake_fixed_ip_get(context, fixed_ip_id):
+        def fake_fixed_ip_get(context, fixed_ip_id, get_network):
             return {'instance_uuid': 'fake_uuid',
-                    'address': '10.0.0.2'}
+                    'address': '10.0.0.2',
+                    'network': 'fakenet'}
 
-        def fake_remove_floating_ip(floating_addr, fixed_addr, interface):
+        def fake_remove_floating_ip(floating_addr, fixed_addr, interface,
+                                    network):
             called['count'] += 1
 
         def fake_floating_ip_update(context, address, args):
@@ -1853,11 +1857,13 @@ class FloatingIPTestCase(test.TestCase):
         def fake_is_stale_floating_ip_address(context, floating_ip):
             return floating_ip['address'] == '172.24.4.23'
 
-        def fake_fixed_ip_get(context, fixed_ip_id):
+        def fake_fixed_ip_get(context, fixed_ip_id, get_network):
             return {'instance_uuid': 'fake_uuid',
-                    'address': '10.0.0.2'}
+                    'address': '10.0.0.2',
+                    'network': 'fakenet'}
 
-        def fake_add_floating_ip(floating_addr, fixed_addr, interface):
+        def fake_add_floating_ip(floating_addr, fixed_addr, interface,
+                                 network):
             called['count'] += 1
 
         def fake_floating_ip_update(context, address, args):
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index c70e96cdc..e7a4ce512 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -253,11 +253,11 @@ class DbApiTestCase(test.TestCase):
         values = {'address': 'fixed'}
         fixed = db.fixed_ip_create(ctxt, values)
         res = db.floating_ip_fixed_ip_associate(ctxt, floating, fixed, 'foo')
-        self.assertEqual(res, fixed)
+        self.assertEqual(res['address'], fixed)
         res = db.floating_ip_fixed_ip_associate(ctxt, floating, fixed, 'foo')
         self.assertEqual(res, None)
         res = db.floating_ip_disassociate(ctxt, floating)
-        self.assertEqual(res, fixed)
+        self.assertEqual(res['address'], fixed)
         res = db.floating_ip_disassociate(ctxt, floating)
         self.assertEqual(res, None)