From e7111bd5a76ff2113abb54ebe1a30c9966697af1 Mon Sep 17 00:00:00 2001
From: Adit Sarfaty <asarfaty@vmware.com>
Date: Thu, 22 Feb 2018 14:40:22 +0200
Subject: [PATCH] NSX-V3 Add NO NAT rules for router interfaces

For each router interface subnets, we need to add NO DNAT rule whenever
NAT rules are added, or else internal traffic will be blocked.

Change-Id: I34d72b12289d6f6527bc114a32dac88281dd2cc4
---
 vmware_nsx/plugins/common/plugin.py         |  3 +-
 vmware_nsx/plugins/nsx_v3/plugin.py         | 43 +++++++++++++++++++--
 vmware_nsx/tests/unit/nsx_v3/test_plugin.py | 41 ++++++++++++++++++++
 3 files changed, 82 insertions(+), 5 deletions(-)

diff --git a/vmware_nsx/plugins/common/plugin.py b/vmware_nsx/plugins/common/plugin.py
index 9d3422dd2b..81a9c072ac 100644
--- a/vmware_nsx/plugins/common/plugin.py
+++ b/vmware_nsx/plugins/common/plugin.py
@@ -191,7 +191,8 @@ class NsxPluginBase(db_base_plugin_v2.NeutronDbPluginV2,
                 subnet = subnet_qry.filter_by(id=ip.subnet_id).one()
                 subnets.append({'id': subnet.id, 'cidr': subnet.cidr,
                                 'subnetpool_id': subnet.subnetpool_id,
-                                'ip_version': subnet.ip_version})
+                                'ip_version': subnet.ip_version,
+                                'network_id': subnet.network_id})
         return subnets
 
     def _find_router_gw_subnets(self, context, router):
diff --git a/vmware_nsx/plugins/nsx_v3/plugin.py b/vmware_nsx/plugins/nsx_v3/plugin.py
index b345fcb7eb..476e6c0eac 100644
--- a/vmware_nsx/plugins/nsx_v3/plugin.py
+++ b/vmware_nsx/plugins/nsx_v3/plugin.py
@@ -117,6 +117,7 @@ from vmware_nsx.services.trunk.nsx_v3 import driver as trunk_driver
 from vmware_nsxlib.v3 import core_resources as nsx_resources
 from vmware_nsxlib.v3 import exceptions as nsx_lib_exc
 from vmware_nsxlib.v3 import nsx_constants as nsxlib_consts
+from vmware_nsxlib.v3 import router as nsxlib_router
 from vmware_nsxlib.v3 import security
 from vmware_nsxlib.v3 import utils as nsxlib_utils
 
@@ -3207,6 +3208,9 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
                              (newaddr != orgaddr or
                               not new_enable_snat))
 
+        # Remove No-DNAT rules if GW was removed
+        remove_no_dnat_rules = (orgaddr and not newaddr)
+
         # Revocate bgp announce for nonat subnets if tier0 router link is
         # changed or enable_snat is updated from False to True
         revocate_bgp_announce = (not org_enable_snat and org_tier0_uuid and
@@ -3225,6 +3229,9 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
                           (newaddr != orgaddr or
                            not org_enable_snat))
 
+        # Add No-DNAT rules if GW was added
+        add_no_dnat_rules = (newaddr and not orgaddr)
+
         # Bgp announce for nonat subnets if tier0 router link is changed or
         # enable_snat is updated from True to False
         bgp_announce = (not new_enable_snat and new_tier0_uuid and
@@ -3236,11 +3243,18 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
         advertise_route_nat_flag = True if new_enable_snat else False
         advertise_route_connected_flag = True if not new_enable_snat else False
 
+        if add_no_dnat_rules or remove_no_dnat_rules or add_snat_rules:
+            subnets = self._find_router_subnets(context.elevated(),
+                                                router_id)
+
         if revocate_bgp_announce:
             # TODO(berlin): revocate bgp announce on org tier0 router
             pass
         if remove_snat_rules:
             self.nsxlib.router.delete_gw_snat_rules(nsx_router_id, orgaddr)
+        if remove_no_dnat_rules:
+            for subnet in subnets:
+                self._del_subnet_no_dnat_rule(context, nsx_router_id, subnet)
         if remove_router_link_port:
             self.nsxlib.router.remove_router_link_port(
                 nsx_router_id, org_tier0_uuid)
@@ -3260,11 +3274,13 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
             # than the gw
             gw_address_scope = self._get_network_address_scope(
                 context, router.gw_port.network_id)
-            subnets = self._find_router_subnets(context.elevated(),
-                                                router_id)
             for subnet in subnets:
                 self._add_subnet_snat_rule(context, router_id, nsx_router_id,
                                            subnet, gw_address_scope, newaddr)
+        if add_no_dnat_rules:
+            for subnet in subnets:
+                self._add_subnet_no_dnat_rule(context, nsx_router_id, subnet)
+
         if bgp_announce:
             # TODO(berlin): bgp announce on new tier0 router
             pass
@@ -3293,6 +3309,21 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
                                             source_net=subnet['cidr'],
                                             bypass_firewall=False)
 
+    def _add_subnet_no_dnat_rule(self, context, nsx_router_id, subnet):
+        # Add NO-DNAT rule to allow internal traffic between VMs, even if
+        # they have floating ips
+        self.nsxlib.logical_router.add_nat_rule(
+            nsx_router_id, "NO_DNAT", None,
+            dest_net=subnet['cidr'],
+            rule_priority=nsxlib_router.GW_NAT_PRI)
+
+    def _del_subnet_no_dnat_rule(self, context, nsx_router_id, subnet):
+        # Delete the previously created NO-DNAT rules
+        self.nsxlib.logical_router.delete_nat_rule_by_values(
+            nsx_router_id,
+            action="NO_DNAT",
+            match_destination_network=subnet['cidr'])
+
     def _process_extra_attr_router_create(self, context, router_db, r):
         for extra_attr in l3_attrs_db.get_attr_info().keys():
             if (extra_attr in r and
@@ -3851,7 +3882,7 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
                 nsx_rpc.handle_router_metadata_access(self, context, router_id,
                                                       interface=info)
 
-            # add the SNAT rule for this interface
+            # add the SNAT/NO_DNAT rules for this interface
             if (router_db.enable_snat and gw_network_id and
                 router_db.gw_port.get('fixed_ips')):
                 gw_ip = router_db.gw_port['fixed_ips'][0]['ip_address']
@@ -3859,6 +3890,8 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
                     context, gw_network_id)
                 self._add_subnet_snat_rule(context, router_id, nsx_router_id,
                                            subnet, gw_address_scope, gw_ip)
+            if gw_network_id:
+                self._add_subnet_no_dnat_rule(context, nsx_router_id, subnet)
             # update firewall rules
             self.update_router_firewall(context, router_id)
         except Exception:
@@ -3931,13 +3964,15 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
             else:
                 self.nsxlib.logical_router_port.delete_by_lswitch_id(
                     nsx_net_id)
-            # try to delete the SNAT rule of this subnet
+            # try to delete the SNAT/NO_DNAT rules of this subnet
             if (router_db.gw_port and router_db.enable_snat and
                 router_db.gw_port.get('fixed_ips')):
                 gw_ip = router_db.gw_port['fixed_ips'][0]['ip_address']
                 self.nsxlib.router.delete_gw_snat_rule_by_source(
                     nsx_router_id, gw_ip, subnet['cidr'],
                     skip_not_found=True)
+            if router_db.gw_port:
+                self._del_subnet_no_dnat_rule(context, nsx_router_id, subnet)
 
         except nsx_lib_exc.ResourceNotFound:
             LOG.error("router port on router %(router_id)s for net "
diff --git a/vmware_nsx/tests/unit/nsx_v3/test_plugin.py b/vmware_nsx/tests/unit/nsx_v3/test_plugin.py
index bd017f14a1..cd474b8419 100644
--- a/vmware_nsx/tests/unit/nsx_v3/test_plugin.py
+++ b/vmware_nsx/tests/unit/nsx_v3/test_plugin.py
@@ -1266,6 +1266,47 @@ class TestL3NatTestCase(L3NatTest,
                 gw_info = body['router']['external_gateway_info']
                 self.assertIsNone(gw_info)
 
+    def test_router_on_vlan_net(self):
+        providernet_args = {pnet.NETWORK_TYPE: 'vlan',
+                            pnet.SEGMENTATION_ID: 10}
+        with mock.patch('vmware_nsxlib.v3.core_resources.NsxLibTransportZone.'
+                       'get_transport_type', return_value='VLAN'):
+            result = self._create_network(fmt='json', name='badvlan_net',
+                                          admin_state_up=True,
+                                          providernet_args=providernet_args,
+                                          arg_list=(
+                                              pnet.NETWORK_TYPE,
+                                              pnet.SEGMENTATION_ID))
+            vlan_network = self.deserialize('json', result)
+            with self.router() as r1,\
+                self.subnet() as ext_subnet,\
+                self.subnet(cidr='11.0.0.0/24', network=vlan_network) as s1:
+                self._set_net_external(ext_subnet['subnet']['network_id'])
+                # adding a vlan interface with no GW should fail
+                self._router_interface_action(
+                    'add', r1['router']['id'],
+                    s1['subnet']['id'], None,
+                    expected_code=400)
+                # adding GW
+                self._add_external_gateway_to_router(
+                    r1['router']['id'],
+                    ext_subnet['subnet']['network_id'])
+                # adding the vlan interface
+                self._router_interface_action(
+                    'add', r1['router']['id'],
+                    s1['subnet']['id'], None)
+
+                # adding a floating ip
+                with self.port(subnet=s1) as p:
+                    fip_res = self._create_floatingip(
+                        self.fmt,
+                        ext_subnet['subnet']['network_id'],
+                        subnet_id=ext_subnet['subnet']['id'],
+                        port_id=p['port']['id'])
+                    fip = self.deserialize(self.fmt, fip_res)
+                    self.assertEqual(p['port']['id'],
+                                     fip['floatingip']['port_id'])
+
     def test_create_router_gateway_fails(self):
         self.skipTest('not supported')