From 4da2726593ef50a2e1b6d47b0c0c6275421bf93f Mon Sep 17 00:00:00 2001 From: Anna Khmelnitsky Date: Thu, 17 Nov 2016 14:55:18 -0800 Subject: [PATCH] By default, add floating IP NAT rules to each vnic on router There are use cases where instances on the subnet need to access one another via floating IP's. For this to work, we need to create the NAT rules on the internal interfaces of the edge. Before this change, floating IP NAT rule was added to external interface only, and hosts residing on same network were not able to connect via floating IPs. This behavior is still available by setting config variable bind_floatingip_to_all_interfaces to false. New behavior is dependant on driver version: before 6.2.4, a NAT rule is created explicitly for each vnic from 6.2.4 onwards, single NAT rule without vnic is created, which is interpreted by platform as 'bind to all interfaces'. Change-Id: Iacc858f726f58d05fa54a723304193f3dc1fb66d --- vmware_nsx/common/config.py | 6 ++ .../nsx_v/drivers/shared_router_driver.py | 5 +- .../nsx_v/vshield/edge_appliance_driver.py | 59 ++++++++---- .../plugins/nsx_v/vshield/edge_utils.py | 24 ++++- .../unit/nsx_v/vshield/test_vcns_driver.py | 92 +++++++++++++++++++ 5 files changed, 168 insertions(+), 18 deletions(-) diff --git a/vmware_nsx/common/config.py b/vmware_nsx/common/config.py index fae9f82e70..1c75194a62 100644 --- a/vmware_nsx/common/config.py +++ b/vmware_nsx/common/config.py @@ -631,6 +631,12 @@ nsxv_opts = [ help=_("(Optional) Sets the network address for distributed " "router TLR-PLR connectivity, with " "/ syntax")), + cfg.BoolOpt('bind_floatingip_to_all_interfaces', default=True, + help=_("If set to False, router will associate floating ip " + "with external interface of only, thus denying " + "connectivity between hosts on same network via " + "their floating ips. If True, floating ip will " + "be associated with all router interfaces.")), ] # Register the configuration options diff --git a/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py b/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py index 7ace788e05..8b64f7d243 100644 --- a/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py +++ b/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py @@ -213,9 +213,12 @@ class RouterSharedDriver(router_driver.RouterBaseDriver): snat, dnat = self.plugin._get_nat_rules(context, router) snats.extend(snat) dnats.extend(dnat) - if len(dnat) > 0: + if (not cfg.CONF.nsxv.bind_floatingip_to_all_interfaces and + len(dnat) > 0): # Copy each DNAT rule to all vnics of the other routers, # to allow NAT-ed traffic between routers + # no need for that if bind_floatingip_to_all_interfaces + # is on (default) other_vnics = [] for other_router_id in router_ids: if other_router_id != router_id: diff --git a/vmware_nsx/plugins/nsx_v/vshield/edge_appliance_driver.py b/vmware_nsx/plugins/nsx_v/vshield/edge_appliance_driver.py index 460939284d..a8dd543bfa 100644 --- a/vmware_nsx/plugins/nsx_v/vshield/edge_appliance_driver.py +++ b/vmware_nsx/plugins/nsx_v/vshield/edge_appliance_driver.py @@ -539,14 +539,15 @@ class EdgeApplianceDriver(object): def _assemble_nat_rule(self, action, original_address, translated_address, - vnic_index=constants.EXTERNAL_VNIC_INDEX, + vnic_index=None, enabled=True, protocol='any', original_port='any', translated_port='any'): nat_rule = {} nat_rule['action'] = action - nat_rule['vnic'] = vnic_index + if vnic_index is not None: + nat_rule['vnic'] = vnic_index nat_rule['originalAddress'] = original_address nat_rule['translatedAddress'] = translated_address nat_rule['enabled'] = enabled @@ -564,31 +565,57 @@ class EdgeApplianceDriver(object): e.response) raise e - def update_nat_rules(self, edge_id, snats, dnats): + def update_nat_rules(self, edge_id, snats, dnats, indices=None): LOG.debug("VCNS: update nat rule\n" "SNAT:%(snat)s\n" - "DNAT:%(dnat)s\n", { - 'snat': snats, 'dnat': dnats}) + "DNAT:%(dnat)s\n" + "INDICES: %(index)s\n", { + 'snat': snats, 'dnat': dnats, 'index': indices}) nat_rules = [] for dnat in dnats: - vnic_index = constants.EXTERNAL_VNIC_INDEX + vnic_index = None if 'vnic_index' in dnat: vnic_index = dnat['vnic_index'] - nat_rules.append(self._assemble_nat_rule( - 'dnat', dnat['dst'], dnat['translated'], vnic_index=vnic_index - )) - nat_rules.append(self._assemble_nat_rule( - 'snat', dnat['translated'], dnat['dst'], vnic_index=vnic_index - )) + if vnic_index or not indices: + # we are adding a predefined index or + # adding to all interfaces + nat_rules.append(self._assemble_nat_rule( + 'dnat', dnat['dst'], dnat['translated'], + vnic_index=vnic_index + )) + nat_rules.append(self._assemble_nat_rule( + 'snat', dnat['translated'], dnat['dst'], + vnic_index=vnic_index + )) + else: + for index in indices: + nat_rules.append(self._assemble_nat_rule( + 'dnat', dnat['dst'], dnat['translated'], + vnic_index=index + )) + nat_rules.append(self._assemble_nat_rule( + 'snat', dnat['translated'], dnat['dst'], + vnic_index=index + )) for snat in snats: - vnic_index = constants.EXTERNAL_VNIC_INDEX + vnic_index = None if 'vnic_index' in snat: vnic_index = snat['vnic_index'] - nat_rules.append(self._assemble_nat_rule( - 'snat', snat['src'], snat['translated'], vnic_index=vnic_index - )) + if vnic_index or not indices: + # we are adding a predefined index + # or adding to all interfaces + nat_rules.append(self._assemble_nat_rule( + 'snat', snat['src'], snat['translated'], + vnic_index=vnic_index + )) + else: + for index in indices: + nat_rules.append(self._assemble_nat_rule( + 'snat', snat['src'], snat['translated'], + vnic_index=index + )) nat = { 'featureType': 'nat', diff --git a/vmware_nsx/plugins/nsx_v/vshield/edge_utils.py b/vmware_nsx/plugins/nsx_v/vshield/edge_utils.py index 3f4fef372b..8ebabf139a 100644 --- a/vmware_nsx/plugins/nsx_v/vshield/edge_utils.py +++ b/vmware_nsx/plugins/nsx_v/vshield/edge_utils.py @@ -2299,7 +2299,29 @@ def _delete_interface(nsxv_manager, context, router_id, network_id, def update_nat_rules(nsxv_manager, context, router_id, snat, dnat): binding = nsxv_db.get_nsxv_router_binding(context.session, router_id) if binding: - nsxv_manager.update_nat_rules(binding['edge_id'], snat, dnat) + bind_to_all = cfg.CONF.nsxv.bind_floatingip_to_all_interfaces + + indices = None + if bind_to_all: + # from 6.2.4 onwards, unspecified vnic will result + # in binding the rule to all interfaces + ver = nsxv_manager.vcns.get_version() + if version.LooseVersion(ver) < version.LooseVersion('6.2.4'): + LOG.debug("NSX version %s requires explicit nat rule " + "for each interface", ver) + edge_id = binding['edge_id'] + vnic_bindings = nsxv_db.get_edge_vnic_bindings_by_edge( + context.session, edge_id) + indices = [vnic_binding.vnic_index + for vnic_binding in vnic_bindings] + + indices.append(vcns_const.EXTERNAL_VNIC_INDEX) + else: + LOG.debug("Configuring nat rules on external " + "interface only for %s", router_id) + indices = [vcns_const.EXTERNAL_VNIC_INDEX] + + nsxv_manager.update_nat_rules(binding['edge_id'], snat, dnat, indices) else: LOG.warning(_LW("Bindings do not exists for %s"), router_id) diff --git a/vmware_nsx/tests/unit/nsx_v/vshield/test_vcns_driver.py b/vmware_nsx/tests/unit/nsx_v/vshield/test_vcns_driver.py index 527bd0ae20..9b980ac10f 100644 --- a/vmware_nsx/tests/unit/nsx_v/vshield/test_vcns_driver.py +++ b/vmware_nsx/tests/unit/nsx_v/vshield/test_vcns_driver.py @@ -411,6 +411,7 @@ class VcnsDriverTestCase(base.BaseTestCase): 'translated': '192.168.2.1' } ] + result = self.vcns_driver.update_nat_rules(self.edge_id, snats, dnats) self.assertTrue(result) @@ -425,6 +426,97 @@ class VcnsDriverTestCase(base.BaseTestCase): self.natEquals(rules[5], snats[1]) self.natEquals(rules[6], snats[2]) + def test_update_nat_rules_for_all_vnics(self): + self._deploy_edge() + snats = [{ + 'src': '192.168.1.0/24', + 'translated': '10.0.0.1' + }, { + 'src': '192.168.2.0/24', + 'translated': '10.0.0.2' + }, { + 'src': '192.168.3.0/24', + 'translated': '10.0.0.3' + } + ] + dnats = [{ + 'dst': '100.0.0.4', + 'translated': '192.168.1.1' + }, { + 'dst': '100.0.0.5', + 'translated': '192.168.2.1' + } + ] + + indices = [0, 1, 2, 3] + result = self.vcns_driver.update_nat_rules(self.edge_id, + snats, dnats, indices) + self.assertTrue(result) + + natcfg = self.vcns_driver.get_nat_config(self.edge_id) + rules = natcfg['rules']['natRulesDtos'] + + self.assertEqual(len(rules), 2 * len(indices) * len(dnats) + + len(indices) * len(snats)) + + sorted_rules = sorted(rules, key=lambda k: k['vnic']) + for i in range(0, len(sorted_rules), 7): + self.natEquals(sorted_rules[i], dnats[0]) + self.natEquals(sorted_rules[i + 1], self.snat_for_dnat(dnats[0])) + self.natEquals(sorted_rules[i + 2], dnats[1]) + self.natEquals(sorted_rules[i + 3], self.snat_for_dnat(dnats[1])) + self.natEquals(sorted_rules[i + 4], snats[0]) + self.natEquals(sorted_rules[i + 5], snats[1]) + self.natEquals(sorted_rules[i + 6], snats[2]) + + def test_update_nat_rules_for_specific_vnics(self): + self._deploy_edge() + snats = [{ + 'src': '192.168.1.0/24', + 'translated': '10.0.0.1', + 'vnic_index': 5 + }, { + 'src': '192.168.2.0/24', + 'translated': '10.0.0.2' + }, { + 'src': '192.168.3.0/24', + 'translated': '10.0.0.3' + } + ] + dnats = [{ + 'dst': '100.0.0.4', + 'translated': '192.168.1.1', + 'vnic_index': 2 + }, { + 'dst': '100.0.0.5', + 'translated': '192.168.2.1' + } + ] + + result = self.vcns_driver.update_nat_rules(self.edge_id, snats, dnats) + self.assertTrue(result) + + natcfg = self.vcns_driver.get_nat_config(self.edge_id) + + rules = natcfg['rules']['natRulesDtos'] + + self.assertEqual(len(rules), 2 * len(dnats) + len(snats)) + + self.natEquals(rules[0], dnats[0]) + self.assertEqual(rules[0]['vnic'], 2) + self.natEquals(rules[1], self.snat_for_dnat(dnats[0])) + self.assertEqual(rules[1]['vnic'], 2) + self.natEquals(rules[2], dnats[1]) + self.assertNotIn('vnic', rules[2]) + self.natEquals(rules[3], self.snat_for_dnat(dnats[1])) + self.assertNotIn('vnic', rules[3]) + self.natEquals(rules[4], snats[0]) + self.assertEqual(rules[4]['vnic'], 5) + self.natEquals(rules[5], snats[1]) + self.assertNotIn('vnic', rules[5]) + self.natEquals(rules[6], snats[2]) + self.assertNotIn('vnic', rules[6]) + def snat_for_dnat(self, dnat): return { 'src': dnat['translated'],