From 01d70223e897af607cbc8f767c09321ecb5359db Mon Sep 17 00:00:00 2001 From: Anna Khmelnitsky Date: Thu, 28 Feb 2019 15:24:03 -0800 Subject: [PATCH] NSX|V3+P: Enable dual stack fixed IPs Till now, only single fixed ip could be configured on port. This patch will allow maximum one fixed ip per ip version to enable dual stack ports. Change-Id: Ia3e06c10c7f420f7f89f805650214645eec02ee8 --- vmware_nsx/plugins/common_v3/plugin.py | 112 ++++++++++++++-- vmware_nsx/plugins/nsx_p/plugin.py | 19 +-- vmware_nsx/plugins/nsx_v3/plugin.py | 137 +++++++++++--------- vmware_nsx/tests/unit/nsx_p/test_plugin.py | 22 +++- vmware_nsx/tests/unit/nsx_v3/test_plugin.py | 7 +- 5 files changed, 210 insertions(+), 87 deletions(-) diff --git a/vmware_nsx/plugins/common_v3/plugin.py b/vmware_nsx/plugins/common_v3/plugin.py index 6d8966b2c8..59ce040df8 100644 --- a/vmware_nsx/plugins/common_v3/plugin.py +++ b/vmware_nsx/plugins/common_v3/plugin.py @@ -198,6 +198,26 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, return self.conn.consume_in_threads() + def _get_interface_subnet(self, context, interface_info): + is_port, is_sub = self._validate_interface_info(interface_info) + + subnet_id = None + if is_sub: + subnet_id = interface_info.get('subnet_id') + + if not subnet_id: + port_id = interface_info['port_id'] + port = self.get_port(context, port_id) + if 'fixed_ips' in port and port['fixed_ips']: + if len(port['fixed_ips'][0]) > 1: + # This should never happen since router interface is per + # IP version, and we allow single fixed ip per ip version + return + subnet_id = port['fixed_ips'][0]['subnet_id'] + + if subnet_id: + return self.get_subnet(context, subnet_id) + def _get_interface_network(self, context, interface_info): is_port, is_sub = self._validate_interface_info(interface_info) if is_port: @@ -462,8 +482,56 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, LOG.warning(err_msg) raise n_exc.InvalidInput(error_message=err_msg) + def _validate_max_ips_per_port(self, context, fixed_ip_list, device_owner): + """Validate the number of fixed ips on a port + + Do not allow multiple ip addresses on a port since the nsx backend + cannot add multiple static dhcp bindings with the same port + """ + if (device_owner and + nl_net_utils.is_port_trusted({'device_owner': device_owner})): + return + + if not validators.is_attr_set(fixed_ip_list): + return + + msg = _('Exceeded maximum amount of fixed ips per port and ip version') + if len(fixed_ip_list) > 2: + raise n_exc.InvalidInput(error_message=msg) + + if len(fixed_ip_list) < 2: + return + + def get_fixed_ip_version(i): + if 'ip_address' in fixed_ip_list[i]: + return netaddr.IPAddress( + fixed_ip_list[i]['ip_address']).version + if 'subnet_id' in fixed_ip_list[i]: + subnet = self.get_subnet(context.elevated(), + fixed_ip_list[i]['subnet_id']) + return subnet['ip_version'] + + ipver1 = get_fixed_ip_version(0) + ipver2 = get_fixed_ip_version(1) + if ipver1 and ipver2 and ipver1 != ipver2: + # One fixed IP is allowed for each IP version + return + + raise n_exc.InvalidInput(error_message=msg) + + def _get_subnets_for_fixed_ips_on_port(self, context, port_data): + # get the subnet id from the fixed ips of the port + if 'fixed_ips' in port_data and port_data['fixed_ips']: + subnet_ids = (fixed_ip['subnet_id'] + for fixed_ip in port_data['fixed_ips']) + + # check only dhcp enabled subnets + return (self.get_subnet(context.elevated(), subnet_id) + for subnet_id in subnet_ids) + def _validate_create_port(self, context, port_data): - self._validate_max_ips_per_port(port_data.get('fixed_ips', []), + self._validate_max_ips_per_port(context, + port_data.get('fixed_ips', []), port_data.get('device_owner')) is_external_net = self._network_is_external( @@ -582,8 +650,9 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, self._assert_on_device_owner_change(port_data, orig_dev_owner) self._assert_on_port_admin_state(port_data, device_owner) self._assert_on_port_sec_change(port_data, device_owner) - self._validate_max_ips_per_port( - port_data.get('fixed_ips', []), device_owner) + self._validate_max_ips_per_port(context, + port_data.get('fixed_ips', []), + device_owner) self._assert_on_vpn_port_change(original_port) self._assert_on_lb_port_fixed_ip_change(port_data, orig_dev_owner) self._validate_extra_dhcp_options(port_data.get(ext_edo.EXTRADHCPOPTS)) @@ -2369,8 +2438,16 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, LOG.error(msg) raise n_exc.InvalidInput(error_message=msg) + def _need_router_no_dnat_rules(self, subnet): + # NAT is not supported for IPv6 + return (subnet['ip_version'] == 4) + def _need_router_snat_rules(self, context, router_id, subnet, gw_address_scope): + # NAT is not supported for IPv6 + if subnet['ip_version'] != 4: + return False + # if the subnets address scope is the same as the gateways: # no need for SNAT if gw_address_scope: @@ -2426,7 +2503,8 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, """Should be implemented by each plugin""" pass - def _validate_multiple_subnets_routers(self, context, router_id, net_id): + def _validate_multiple_subnets_routers(self, context, router_id, + net_id, subnet): network = self.get_network(context, net_id) net_type = network.get(pnet.NETWORK_TYPE) if (net_type and @@ -2450,17 +2528,31 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, router_ids = [port['device_id'] for port in intf_ports if port['device_id']] if len(router_ids) > 0: - err_msg = _("Only one subnet of network %(net_id)s can be " - "attached to router, one subnet is already attached " - "to router %(router_id)s") % { + err_msg = _("Only one subnet of each IP version in a network " + "%(net_id)s can be attached to router, one subnet " + "is already attached to router %(router_id)s") % { 'net_id': net_id, 'router_id': router_ids[0]} - LOG.error(err_msg) if router_id in router_ids: - # attach to the same router again - raise n_exc.InvalidInput(error_message=err_msg) + # We support 2 subnets from same net only for dual stack case + if not subnet: + # No IP provided on connected port + LOG.error(err_msg) + raise n_exc.InvalidInput(error_message=err_msg) + for port in intf_ports: + if port['device_id'] != router_id: + continue + if 'fixed_ips' in port and port['fixed_ips']: + ex_subnet = self.get_subnet( + context.elevated(), + port['fixed_ips'][0]['subnet_id']) + if ex_subnet['ip_version'] == subnet['ip_version']: + # attach to the same router with same IP version + LOG.error(err_msg) + raise n_exc.InvalidInput(error_message=err_msg) else: # attach to multiple routers + LOG.error(err_msg) raise l3_exc.RouterInterfaceAttachmentConflict(reason=err_msg) def _router_has_edge_fw_rules(self, context, router): diff --git a/vmware_nsx/plugins/nsx_p/plugin.py b/vmware_nsx/plugins/nsx_p/plugin.py index a4e6419bc4..31ed41fc35 100644 --- a/vmware_nsx/plugins/nsx_p/plugin.py +++ b/vmware_nsx/plugins/nsx_p/plugin.py @@ -13,8 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -import netaddr - from oslo_config import cfg from oslo_db import exception as db_exc from oslo_log import log @@ -657,9 +655,6 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): address_bindings = [] for fixed_ip in port_data['fixed_ips']: - if netaddr.IPNetwork(fixed_ip['ip_address']).version != 4: - #TODO(annak): enable when IPv6 is supported - continue binding = self.nsxpolicy.segment_port.build_address_binding( fixed_ip['ip_address'], port_data['mac_address']) address_bindings.append(binding) @@ -968,8 +963,9 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): device_owner = (port_data['device_owner'] if 'device_owner' in port_data else original_port.get('device_owner')) - self._validate_max_ips_per_port( - port_data.get('fixed_ips', []), device_owner) + self._validate_max_ips_per_port(context, + port_data.get('fixed_ips', []), + device_owner) direct_vnic_type = self._validate_port_vnic_type( context, port_data, original_port['network_id']) @@ -1126,6 +1122,9 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): return 'ND-' + subnet['id'] def _add_subnet_no_dnat_rule(self, context, router_id, subnet): + if not self._need_router_no_dnat_rules(subnet): + return + # Add NO-DNAT rule to allow internal traffic between VMs, even if # they have floating ips (Only for routers with snat enabled) self.nsxpolicy.tier1_nat_rule.create_or_overwrite( @@ -1463,12 +1462,16 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): router_db = self._get_router(context, router_id) gw_network_id = (router_db.gw_port.network_id if router_db.gw_port else None) + # NOTE: In dual stack case, neutron would create a separate interface + # for each IP version + # We only allow one subnet per IP version + subnet = self._get_interface_subnet(context, interface_info) with locking.LockManager.get_lock(str(network_id)): # disallow more than one subnets belong to same network being # attached to routers self._validate_multiple_subnets_routers( - context, router_id, network_id) + context, router_id, network_id, subnet) # A router interface cannot be an external network if extern_net: diff --git a/vmware_nsx/plugins/nsx_v3/plugin.py b/vmware_nsx/plugins/nsx_v3/plugin.py index 2ab9203155..d5e9e1f8d3 100644 --- a/vmware_nsx/plugins/nsx_v3/plugin.py +++ b/vmware_nsx/plugins/nsx_v3/plugin.py @@ -1253,11 +1253,6 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, def _build_address_bindings(self, port): address_bindings = [] for fixed_ip in port['fixed_ips']: - # NOTE(arosen): nsx-v3 doesn't seem to handle ipv6 addresses - # currently so for now we remove them here and do not pass - # them to the backend which would raise an error. - if netaddr.IPNetwork(fixed_ip['ip_address']).version == 6: - continue address_bindings.append(nsx_resources.PacketAddressClassifier( fixed_ip['ip_address'], port['mac_address'], None)) @@ -1466,31 +1461,35 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, # get the subnet id from the fixed ips of the port if 'fixed_ips' in port_data and port_data['fixed_ips']: - subnet_id = port_data['fixed_ips'][0]['subnet_id'] + subnets = self._get_subnets_for_fixed_ips_on_port(context, + port_data) elif 'fixed_ips' in original_port and original_port['fixed_ips']: - subnet_id = original_port['fixed_ips'][0]['subnet_id'] + subnets = self._get_subnets_for_fixed_ips_on_port(context, + original_port) else: return # check only dhcp enabled subnets - subnet = self.get_subnet(context.elevated(), subnet_id) - if not subnet['enable_dhcp']: + subnets = (subnet for subnet in subnets if subnet['enable_dhcp']) + if not subnets: return + subnet_ids = (subnet['id'] for subnet in subnets) # check if the subnet is attached to a router port_filters = {'device_owner': [l3_db.DEVICE_OWNER_ROUTER_INTF], 'network_id': [original_port['network_id']]} interfaces = self.get_ports(context.elevated(), filters=port_filters) - router_found = False for interface in interfaces: - if interface['fixed_ips'][0]['subnet_id'] == subnet_id: - router_found = True - break - if not router_found: - err_msg = _("Neutron is configured with DHCP_Relay but no router " - "connected to the subnet") - LOG.warning(err_msg) - raise n_exc.InvalidInput(error_message=err_msg) + for subnet in subnets: + for fixed_ip in interface['fixed_ips']: + if fixed_ip['subnet_id'] in subnet_ids: + # Router exists - validation passed + return + + err_msg = _("Neutron is configured with DHCP_Relay but no router " + "connected to the subnet") + LOG.warning(err_msg) + raise n_exc.InvalidInput(error_message=err_msg) def _update_lport_with_security_groups(self, context, lport_id, original, updated): @@ -2248,6 +2247,8 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, bypass_firewall=False) def _add_subnet_no_dnat_rule(self, context, nsx_router_id, subnet): + if not self._need_router_no_dnat_rules(subnet): + return # Add NO-DNAT rule to allow internal traffic between VMs, even if # they have floating ips (Only for routers with snat enabled) if self.nsxlib.feature_supported( @@ -2630,20 +2631,29 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, exclude_sub_ids=None): exclude_sub_ids = [] if not exclude_sub_ids else exclude_sub_ids address_groups = [] - ports = self._get_router_interface_ports_by_network( + network_ports = self._get_router_interface_ports_by_network( context, router_id, network_id) - ports = [port for port in ports - if port['fixed_ips'] and - port['fixed_ips'][0]['subnet_id'] not in exclude_sub_ids] + ports = [] + for port in network_ports: + if port['fixed_ips']: + add_port = False + for fip in port['fixed_ips']: + if fip['subnet_id'] not in exclude_sub_ids: + add_port = True + + if add_port: + ports.append(port) + for port in ports: - address_group = {} - gateway_ip = port['fixed_ips'][0]['ip_address'] - subnet = self.get_subnet(context, - port['fixed_ips'][0]['subnet_id']) - prefixlen = str(netaddr.IPNetwork(subnet['cidr']).prefixlen) - address_group['ip_addresses'] = [gateway_ip] - address_group['prefix_length'] = prefixlen - address_groups.append(address_group) + for fip in port['fixed_ips']: + address_group = {} + gateway_ip = fip['ip_address'] + subnet = self.get_subnet(context, fip['subnet_id']) + prefixlen = str(netaddr.IPNetwork(subnet['cidr']).prefixlen) + address_group['ip_addresses'] = [gateway_ip] + address_group['prefix_length'] = prefixlen + address_groups.append(address_group) + return (ports, address_groups) def _add_router_interface_wrapper(self, context, router_id, @@ -2665,11 +2675,15 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, router_db = self._get_router(context, router_id) gw_network_id = (router_db.gw_port.network_id if router_db.gw_port else None) + # In case on dual stack, neutron creates a separate interface per + # IP version + subnet = self._get_interface_subnet(context, interface_info) + with locking.LockManager.get_lock(str(network_id)): # disallow more than one subnets belong to same network being # attached to routers self._validate_multiple_subnets_routers( - context, router_id, network_id) + context, router_id, network_id, subnet) # A router interface cannot be an external network if extern_net: @@ -2749,12 +2763,14 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, # add the SNAT/NO_DNAT rules for this interface if router_db.enable_snat and gw_network_id: if router_db.gw_port.get('fixed_ips'): - gw_ip = router_db.gw_port['fixed_ips'][0]['ip_address'] gw_address_scope = self._get_network_address_scope( context, gw_network_id) - self._add_subnet_snat_rule( - context, router_id, nsx_router_id, - subnet, gw_address_scope, gw_ip) + for fip in router_db.gw_port['fixed_ips']: + gw_ip = fip['ip_address'] + self._add_subnet_snat_rule( + context, router_id, nsx_router_id, + subnet, gw_address_scope, gw_ip) + self._add_subnet_no_dnat_rule(context, nsx_router_id, subnet) # update firewall rules @@ -2783,9 +2799,10 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, # find subnet_id - it is need for removing the SNAT rule port = self._get_port(context, port_id) if port.get('fixed_ips'): - subnet_id = port['fixed_ips'][0]['subnet_id'] - self._confirm_router_interface_not_in_use( - context, router_id, subnet_id) + for fip in port['fixed_ips']: + subnet_id = fip['subnet_id'] + self._confirm_router_interface_not_in_use( + context, router_id, subnet_id) if not (port['device_owner'] in const.ROUTER_INTERFACE_OWNERS and port['device_id'] == router_id): raise l3_exc.RouterInterfaceNotFound( @@ -2801,7 +2818,9 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, device_owner=l3_db.DEVICE_OWNER_ROUTER_INTF, network_id=subnet['network_id']) for p in ports: - if p['fixed_ips'][0]['subnet_id'] == subnet_id: + fip_subnet_ids = [fixed_ip['subnet_id'] + for fixed_ip in p['fixed_ips']] + if subnet_id in fip_subnet_ids: port_id = p['id'] break else: @@ -2837,10 +2856,11 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, # try to delete the SNAT/NO_DNAT rules of this subnet if router_db.gw_port and router_db.enable_snat: if 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) + for fixed_ip in router_db.gw_port['fixed_ips']: + gw_ip = fixed_ip['ip_address'] + self.nsxlib.router.delete_gw_snat_rule_by_source( + nsx_router_id, gw_ip, subnet['cidr'], + skip_not_found=True) self._del_subnet_no_dnat_rule(context, nsx_router_id, subnet) except nsx_lib_exc.ResourceNotFound: @@ -3366,8 +3386,8 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, return LOG.info("Recalculating snat rules for router %s", router['id']) - fip = router['external_gateway_info']['external_fixed_ips'][0] - ext_addr = fip['ip_address'] + fips = router['external_gateway_info']['external_fixed_ips'] + ext_addrs = [fip['ip_address'] for fip in fips] gw_address_scope = self._get_network_address_scope( context, router['external_gateway_info']['network_id']) @@ -3383,20 +3403,21 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, 'subnet': subnet['id']}) # Delete rule for this router/subnet pair if it exists - self.nsxlib.router.delete_gw_snat_rule_by_source( - nsx_router_id, ext_addr, subnet['cidr'], - skip_not_found=True) + for ext_addr in ext_addrs: + self.nsxlib.router.delete_gw_snat_rule_by_source( + nsx_router_id, ext_addr, subnet['cidr'], + skip_not_found=True) - if (gw_address_scope != subnet_address_scope): - # subnet is no longer under same address scope with GW - LOG.info("Adding SNAT rule for %(router)s " - "and subnet %(subnet)s", - {'router': router['id'], - 'subnet': subnet['id']}) - self.nsxlib.router.add_gw_snat_rule( - nsx_router_id, ext_addr, - source_net=subnet['cidr'], - bypass_firewall=False) + if (gw_address_scope != subnet_address_scope): + # subnet is no longer under same address scope with GW + LOG.info("Adding SNAT rule for %(router)s " + "and subnet %(subnet)s", + {'router': router['id'], + 'subnet': subnet['id']}) + self.nsxlib.router.add_gw_snat_rule( + nsx_router_id, ext_addr, + source_net=subnet['cidr'], + bypass_firewall=False) def _get_tier0_uplink_cidrs(self, tier0_id): # return a list of tier0 uplink ip/prefix addresses diff --git a/vmware_nsx/tests/unit/nsx_p/test_plugin.py b/vmware_nsx/tests/unit/nsx_p/test_plugin.py index f029d48e5f..463d7a91ff 100644 --- a/vmware_nsx/tests/unit/nsx_p/test_plugin.py +++ b/vmware_nsx/tests/unit/nsx_p/test_plugin.py @@ -631,8 +631,9 @@ class NsxPTestPorts(test_db_base_plugin_v2.TestPortsV2, def test_update_port_mac_v6_slaac(self): self.skipTest('Multiple fixed ips on a port are not supported') + @with_disable_dhcp def test_requested_subnet_id_v4_and_v6(self): - self.skipTest('Multiple fixed ips on a port are not supported') + return super(NsxPTestPorts, self).test_requested_subnet_id_v4_and_v6() def test_requested_invalid_fixed_ips(self): self.skipTest('Multiple fixed ips on a port are not supported') @@ -670,8 +671,11 @@ class NsxPTestPorts(test_db_base_plugin_v2.TestPortsV2, def test_create_router_port_ipv4_and_ipv6_slaac_no_fixed_ips(self): self.skipTest('No DHCP v6 Support yet') + @with_disable_dhcp def test_create_port_with_multiple_ipv4_and_ipv6_subnets(self): - self.skipTest('No DHCP v6 Support yet') + return super( + NsxPTestPorts, + self).test_create_port_with_multiple_ipv4_and_ipv6_subnets def test_ip_allocation_for_ipv6_2_subnet_slaac_mode(self): self.skipTest('No DHCP v6 Support yet') @@ -1565,8 +1569,11 @@ class NsxPTestL3NatTestCase(NsxPTestL3NatTest, def test_router_delete_dhcpv6_stateless_subnet_inuse_returns_409(self): self.skipTest('not supported') + @with_disable_dhcp + @common_v3.with_external_network def test_router_update_gateway_upon_subnet_create_ipv6(self): - self.skipTest('not supported') + super(NsxPTestL3NatTestCase, + self).test_router_update_gateway_upon_subnet_create_ipv6() def test_router_delete_ipv6_slaac_subnet_inuse_returns_409(self): self.skipTest('not supported') @@ -1575,10 +1582,13 @@ class NsxPTestL3NatTestCase(NsxPTestL3NatTest, self.skipTest('not supported') def test_router_add_interface_ipv6_subnet(self): - self.skipTest('not supported') + self.skipTest('slaac not supported') - def test_router_add_iface_ipv6_ext_ra_subnet_returns_400(self): - self.skipTest('not supported') + def test_router_add_interface_ipv6_single_subnet(self): + with self.router() as r, self.network() as n: + with self.subnet(network=n, cidr='fd00::1/64', + gateway_ip='fd00::1', ip_version=6) as s: + self._test_router_add_interface_subnet(r, s) @with_disable_dhcp def test_route_clear_routes_with_None(self): diff --git a/vmware_nsx/tests/unit/nsx_v3/test_plugin.py b/vmware_nsx/tests/unit/nsx_v3/test_plugin.py index 7c88f33ea6..63783ea10e 100644 --- a/vmware_nsx/tests/unit/nsx_v3/test_plugin.py +++ b/vmware_nsx/tests/unit/nsx_v3/test_plugin.py @@ -1727,9 +1727,6 @@ class TestPortsV2(test_plugin.TestPortsV2, NsxV3PluginTestCaseMixin, def test_update_port_mac_v6_slaac(self): self.skipTest('Multiple fixed ips on a port are not supported') - def test_requested_subnet_id_v4_and_v6(self): - self.skipTest('Multiple fixed ips on a port are not supported') - def test_requested_invalid_fixed_ips(self): self.skipTest('Multiple fixed ips on a port are not supported') @@ -2122,7 +2119,7 @@ class TestL3NatTestCase(L3NatTest, self.skipTest('not supported') def test_router_add_gateway_multiple_subnets_ipv6(self): - self.skipTest('not supported') + self.skipTest('multiple ipv6 subnets not supported') def test__notify_gateway_port_ip_changed(self): self.skipTest('not supported') @@ -2274,7 +2271,7 @@ class TestL3NatTestCase(L3NatTest, self.skipTest('not supported') def test_router_add_interface_ipv6_port_existing_network_returns_400(self): - self.skipTest('not supported') + self.skipTest('multiple ipv6 subnets not supported') def test_routes_update_for_multiple_routers(self): self.skipTest('not supported')