diff --git a/doc/source/contributor/internals/openvswitch_firewall.rst b/doc/source/contributor/internals/openvswitch_firewall.rst index 8db8ee0837e..5bcefcef002 100644 --- a/doc/source/contributor/internals/openvswitch_firewall.rst +++ b/doc/source/contributor/internals/openvswitch_firewall.rst @@ -525,6 +525,19 @@ will be: table=94, priority=10,reg6=0x284,dl_src=fa:16:3e:24:57:c7,dl_dst=00:00:00:00:00:00/01:00:00:00:00:00 actions=push_vlan:0x8100,set_field:0x1->vlan_vid,output:3 table=94, priority=1 actions=NORMAL +The OVS firewall will initialize a default goto table 94 flow +on TRANSIENT_TABLE |table_60|, if ``explicitly_egress_direct`` +is set to True, which is mainly for ports without security groups +and disabled port_security. For instance: + +:: + table=60, priority=2 actions=resubmit(,94) + +Then for packets from the outside to VM without security functionalities +(--disable-port-security --no-security-group) +will go to table 94 and do the same direct actions. + + OVS firewall integration points ------------------------------- diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index daf119735c3..3fa6419d000 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -644,6 +644,14 @@ def _initialize_common_flows(self): 'resubmit(,%d)' % ovs_consts.BASE_EGRESS_TABLE, ) + if cfg.CONF.AGENT.explicitly_egress_direct: + self._add_flow( + table=ovs_consts.TRANSIENT_TABLE, + priority=2, + actions='resubmit(,%d)' % ( + ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE) + ) + def _initialize_third_party_tables(self): self.int_br.br.add_flow( table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE, @@ -1253,6 +1261,7 @@ def install_accepted_egress_direct_flow(self, mac, vlan_tag, dst_port, return # Prevent flood for accepted egress traffic + # For packets from internal ports or VM ports. self._add_flow( flow_group_id=dst_port, table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE, @@ -1261,6 +1270,15 @@ def install_accepted_egress_direct_flow(self, mac, vlan_tag, dst_port, reg_net=vlan_tag, actions='output:{:d}'.format(dst_port) ) + # For packets from patch ports. + self._add_flow( + flow_group_id=dst_port, + table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE, + priority=12, + dl_dst=mac, + dl_vlan=vlan_tag, + actions='strip_vlan,output:{:d}'.format(dst_port) + ) # The former flow may not match, that means the destination port is # not in this host. So, we direct the packet to mapped bridge(s). @@ -1309,6 +1327,12 @@ def delete_accepted_egress_direct_flow(self, mac, vlan_tag): dl_src=mac, reg_net=vlan_tag) + self._delete_flows( + table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE, + dl_dst=mac, + dl_vlan=vlan_tag + ) + def _initialize_tracked_egress(self, port): # Drop invalid packets self._add_flow( diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index d90655da457..f49bda76d2d 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -1237,3 +1237,7 @@ def validate_port_forwarding_configuration(): if any(net_type in provider_network_types for net_type in cfg.CONF.ml2.tenant_network_types): raise ovn_exc.InvalidPortForwardingConfiguration() + + +def is_nat_gateway_port_supported(idl): + return idl.is_col_present('NAT', 'gateway_port') diff --git a/neutron/conf/plugins/ml2/drivers/ovs_conf.py b/neutron/conf/plugins/ml2/drivers/ovs_conf.py index f6bc332f509..6ffd67f5d2f 100644 --- a/neutron/conf/plugins/ml2/drivers/ovs_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovs_conf.py @@ -228,12 +228,16 @@ "outgoing IP packet carrying GRE/VXLAN tunnel.")), cfg.BoolOpt('baremetal_smartnic', default=False, help=_("Enable the agent to process Smart NIC ports.")), + # TODO(liuyulong): consider adding a new configuration + # item to control ingress behavior. cfg.BoolOpt('explicitly_egress_direct', default=False, help=_("When set to True, the accepted egress unicast " "traffic will not use action NORMAL. The accepted " "egress packets will be taken care of in the final " "egress tables direct output flows for unicast " - "traffic.")), + "traffic. This will aslo change the pipleline for " + "ingress traffic to ports without security, the final " + "output action will be hit in table 94. ")), ] dhcp_opts = [ diff --git a/neutron/db/models/tag.py b/neutron/db/models/tag.py index 14e680fccc7..55195340d03 100644 --- a/neutron/db/models/tag.py +++ b/neutron/db/models/tag.py @@ -26,6 +26,6 @@ class Tag(model_base.BASEV2): tag = sa.Column(sa.String(255), nullable=False, primary_key=True) standard_attr = orm.relationship( 'StandardAttribute', load_on_pending=True, - backref=orm.backref('tags', lazy='joined', viewonly=True), + backref=orm.backref('tags', lazy='subquery', viewonly=True), sync_backref=False) revises_on_change = ('standard_attr', ) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py index 25e4726cc3e..d39604be5a5 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py @@ -57,7 +57,7 @@ def setup_default_table(self, enable_openflow_dhcp=False, self.install_goto(dest_table_id=constants.PACKET_RATE_LIMIT) self.install_goto(dest_table_id=constants.TRANSIENT_TABLE, table_id=constants.PACKET_RATE_LIMIT) - self.install_normal(table_id=constants.TRANSIENT_TABLE, priority=3) + self.install_normal(table_id=constants.TRANSIENT_TABLE, priority=1) self.init_dhcp(enable_openflow_dhcp=enable_openflow_dhcp, enable_dhcpv6=enable_dhcpv6) self.install_drop(table_id=constants.ARP_SPOOF_TABLE) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index e15b666cc81..4c0f8bdc7bc 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -1053,8 +1053,24 @@ def update_virtual_port_host(self, port_id, chassis_id): 'Chassis', chassis_id, 'hostname').execute(check_error=True) else: hostname = '' - self._plugin.update_virtual_port_host(n_context.get_admin_context(), - port_id, hostname) + + # Updates neutron database with hostname for virtual port + context = n_context.get_admin_context() + self._plugin.update_virtual_port_host(context, port_id, hostname) + db_port = self._plugin.get_port(context, port_id) + check_rev_cmd = self.nb_ovn.check_revision_number( + port_id, db_port, ovn_const.TYPE_PORTS) + # Updates OVN NB database with hostname for lsp virtual port + with self.nb_ovn.transaction(check_error=True) as txn: + ext_ids = ('external_ids', + {ovn_const.OVN_HOST_ID_EXT_ID_KEY: hostname}) + txn.add( + self.nb_ovn.db_set( + 'Logical_Switch_Port', port_id, ext_ids)) + txn.add(check_rev_cmd) + if check_rev_cmd.result == ovn_const.TXN_COMMITTED: + ovn_revision_numbers_db.bump_revision(context, db_port, + ovn_const.TYPE_PORTS) def get_workers(self): """Get any worker instances that should have their own process diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py index fdd20fe2f10..094f3fbf1fd 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py @@ -361,6 +361,10 @@ def get_all_logical_routers_with_rports(self): columns['external_mac'] = nat.external_mac[0] if nat.logical_port: columns['logical_port'] = nat.logical_port[0] + columns['external_ids'] = nat.external_ids + columns['uuid'] = nat.uuid + if utils.is_nat_gateway_port_supported(self): + columns['gateway_port'] = nat.gateway_port dnat_and_snats.append(columns) elif nat.type == 'snat': snat.append(columns) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 0663940cd31..66634923efe 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -1160,6 +1160,52 @@ def cleanup_old_hash_ring_nodes(self): context = n_context.get_admin_context() hash_ring_db.cleanup_old_nodes(context, days=5) + @periodics.periodic(spacing=600, run_immediately=True) + def update_nat_floating_ip_with_gateway_port_reference(self): + """Set NAT rule gateway_port column to any floating IP without + router gateway port uuid reference - LP#2035281. + """ + + if not utils.is_nat_gateway_port_supported(self._nb_idl): + raise periodics.NeverAgain() + + context = n_context.get_admin_context() + fip_update = [] + lrouters = self._nb_idl.get_all_logical_routers_with_rports() + for router in lrouters: + ovn_fips = router['dnat_and_snats'] + for ovn_fip in ovn_fips: + # Skip FIPs that are already configured with gateway_port + if ovn_fip['gateway_port']: + continue + fip_id = ovn_fip['external_ids'].get( + ovn_const.OVN_FIP_EXT_ID_KEY) + if fip_id: + fip_update.append({'uuid': ovn_fip['uuid'], + 'router_id': router['name']}) + + # Simple caching mechanism to avoid unnecessary DB calls + gw_port_id_cache = {} + lrp_cache = {} + cmds = [] + for fip in fip_update: + lrouter = utils.ovn_name(fip['router_id']) + if lrouter not in gw_port_id_cache.keys(): + router_db = self._ovn_client._l3_plugin.get_router( + context, fip['router_id'], fields=['gw_port_id']) + gw_port_id_cache[lrouter] = router_db.get('gw_port_id') + lrp_cache[lrouter] = self._nb_idl.get_lrouter_port( + gw_port_id_cache[lrouter]) + columns = {'gateway_port': lrp_cache[lrouter].uuid} + cmds.append(self._nb_idl.set_nat_rule_in_lrouter( + lrouter, fip['uuid'], **columns)) + + if cmds: + with self._nb_idl.transaction(check_error=True) as txn: + for cmd in cmds: + txn.add(cmd) + raise periodics.NeverAgain() + class HashRingHealthCheckPeriodics(object): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 14776429ba3..f7235a033c2 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -850,6 +850,14 @@ def _create_or_update_floatingip(self, floatingip, txn=None): 'logical_port': floatingip['port_id'], 'external_ids': ext_ids} + # If OVN supports gateway_port column for NAT rules set gateway port + # uuid to any floating IP without gw port reference - LP#2035281. + if utils.is_nat_gateway_port_supported(self._nb_idl): + router_db = self._l3_plugin.get_router(admin_context, router_id) + gw_port_id = router_db.get('gw_port_id') + lrp = self._nb_idl.get_lrouter_port(gw_port_id) + columns['gateway_port'] = lrp.uuid + if ovn_conf.is_ovn_distributed_floating_ip(): if self._nb_idl.lsp_get_up(floatingip['port_id']).execute(): columns['external_mac'] = port_db['mac_address'] diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py index 39d51f3a867..32e4d60f7f8 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py @@ -566,6 +566,17 @@ def match_fn(self, event, row, old): virtual_parents = (row.options or {}).get( ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY) + + if getattr(old, 'chassis', None) is not None and virtual_parents: + # The port moved from chassis due to VIP failover or migration, + # which means we need to update the host_id information + return True + + if getattr(old, 'options', None) is None: + # The "old.options" dictionary is not being modified, + # thus the virtual parents didn't change. + return False + old_virtual_parents = getattr(old, 'options', {}).get( ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY) if virtual_parents != old_virtual_parents: diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 6a846959c96..74ceaa960ce 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -1039,6 +1039,65 @@ def test_check_for_ha_chassis_group(self): lsp = self.nb_api.lookup('Logical_Switch_Port', p1['id']) self.assertEqual(hcg_uuid, lsp.ha_chassis_group[0].uuid) + def test_floating_ip_with_gateway_port(self): + ext_net = self._create_network('ext_networktest', external=True) + ext_subnet = self._create_subnet( + 'ext_subnettest', + ext_net['id'], + **{'cidr': '100.0.0.0/24', + 'gateway_ip': '100.0.0.254', + 'allocation_pools': [ + {'start': '100.0.0.2', 'end': '100.0.0.253'}], + 'enable_dhcp': False}) + net1 = self._create_network('network1test', external=False) + subnet1 = self._create_subnet('subnet1test', net1['id']) + external_gateway_info = { + 'enable_snat': True, + 'network_id': ext_net['id'], + 'external_fixed_ips': [ + {'ip_address': '100.0.0.2', 'subnet_id': ext_subnet['id']}]} + router = self._create_router( + 'routertest', external_gateway_info=external_gateway_info) + self._add_router_interface(router['id'], subnet1['id']) + + p1 = self._create_port('testp1', net1['id']) + logical_ip = p1['fixed_ips'][0]['ip_address'] + fip_info = {'floatingip': { + 'tenant_id': self._tenant_id, + 'description': 'test_fip', + 'floating_network_id': ext_net['id'], + 'port_id': p1['id'], + 'fixed_ip_address': logical_ip}} + + # Create floating IP without gateway_port + with mock.patch.object(utils, + 'is_nat_gateway_port_supported', return_value=False): + fip = self.l3_plugin.create_floatingip(self.context, fip_info) + + self.assertEqual(router['id'], fip['router_id']) + self.assertEqual('testp1', fip['port_details']['name']) + self.assertIsNotNone(self.nb_api.get_lswitch_port(fip['port_id'])) + + rules = self.nb_api.get_all_logical_routers_with_rports()[0] + fip_rule = rules['dnat_and_snats'][0] + if utils.is_nat_gateway_port_supported(self.nb_api): + self.assertEqual([], fip_rule['gateway_port']) + else: + self.assertNotIn('gateway_port', fip_rule) + + # Call the maintenance task and check that the value has been + # updated in the NAT rule + self.assertRaises(periodics.NeverAgain, + self.maint.update_nat_floating_ip_with_gateway_port_reference) + + rules = self.nb_api.get_all_logical_routers_with_rports()[0] + fip_rule = rules['dnat_and_snats'][0] + + if utils.is_nat_gateway_port_supported(self.nb_api): + self.assertNotEqual([], fip_rule['gateway_port']) + else: + self.assertNotIn('gateway_port', fip_rule) + class TestLogMaintenance(_TestMaintenanceHelper, test_log_driver.LogApiTestCaseBase): diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index 5630a8c50eb..175f8fe200c 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -323,9 +323,11 @@ def _set_port_binding_virtual_parent(self, port_id, parent_port_id): pb_port_vip = self.sb_api.db_find_rows( 'Port_Binding', ('logical_port', '=', port_id)).execute( check_error=True)[0] + pb_virtual_parent = str(pb_port_parent.uuid) self.sb_api.db_set( 'Port_Binding', pb_port_vip.uuid, - ('virtual_parent', pb_port_parent.uuid)).execute(check_error=True) + ('chassis', pb_port_parent.chassis), + ('virtual_parent', pb_virtual_parent)).execute(check_error=True) def _check_port_binding_type(self, port_id, port_type): def is_port_binding_type(port_id, port_type): @@ -338,8 +340,19 @@ def is_port_binding_type(port_id, port_type): def _check_port_virtual_parents(self, port_id, vparents): def is_port_virtual_parents(port_id, vparents): bp = self._find_port_binding(port_id) - return (vparents == - bp.options.get(ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY)) + vp = bp.options.get(ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY) + + # If the given vparents is None, or if the current value is None + # Then just do a check on that, no need to split strings if it + # is not required + if None in (vp, vparents): + return vp == vparents + + # Since the virtual parents is a string, representing a list of + # ports, we should make a set() and compare sets + bp_set = {p for p in vp.split(',')} + vp_set = {p for p in vparents.split(',')} + return bp_set == vp_set check = functools.partial(is_port_virtual_parents, port_id, vparents) n_utils.wait_until_true(check, timeout=10) @@ -417,6 +430,106 @@ def test_non_virtual_port_no_host_update(self, mock_update_vip_host): self.assertRaises(n_utils.WaitTimeout, n_utils.wait_until_true, lambda: mock_update_vip_host.called, timeout=5) + def _check_port_host_set(self, port_id, host_id): + # This function checks if given host_id matches the values in the + # neutron DB as well as in the OVN DB for the port with given port_id + core_plugin = directory.get_plugin() + + # Get port from neutron DB + port = core_plugin.get_ports( + self.context, filters={'id': [port_id]})[0] + + # Get port from OVN DB + bp = self._find_port_binding(port_id) + ovn_host_id = bp.external_ids.get(ovn_const.OVN_HOST_ID_EXT_ID_KEY) + + # Check that both neutron and ovn are the same as given host_id + return port[portbindings.HOST_ID] == host_id == ovn_host_id + + def _check_port_and_port_binding_revision_number(self, port_id): + + def is_port_and_port_binding_same_revision_number(port_id): + # This function checks if given port matches the revision_number + # in the neutron DB as well as in the OVN DB for the port_binding + core_plugin = directory.get_plugin() + + # Get port from neutron DB + port = core_plugin.get_ports( + self.context, filters={'id': [port_id]})[0] + + # Get port binding from OVN DB + bp = self._find_port_binding(port_id) + ovn_port_binding_revision_number = bp.external_ids.get( + ovn_const.OVN_REV_NUM_EXT_ID_KEY, ovn_const.INITIAL_REV_NUM) + + # Check that both neutron and ovn are the same as given host_id + return port['revision_number'] == int( + ovn_port_binding_revision_number) + + check = functools.partial( + is_port_and_port_binding_same_revision_number, port_id) + n_utils.wait_until_true(check, timeout=10) + + def test_virtual_port_host_update_upon_failover(self): + # NOTE: we can't simulate traffic, but we can simulate the event that + # would've been triggered by OVN, which is what we do. + + # The test is based to test_virtual_port_host_update, though in this + # test we actually test the OVNMechanismDriver.update_virtual_port_host + # method, that updates the hostname in neutron and OVN + # We do not extensively check if the allowed-address-pair is being kept + # up-to-date, since test_virtual_port_host_update does this already. + + # 1) Setup a second chassis + second_chassis_name = 'ovs-host2' + second_chassis = self.add_fake_chassis(second_chassis_name) + + # 2) Create port with the VIP for allowed address pair setup + vip = self.create_port(device_owner='', host='') + vip_address = vip['fixed_ips'][0]['ip_address'] + allowed_address_pairs = [{'ip_address': vip_address}] + self._check_port_binding_type(vip['id'], '') + self._check_port_and_port_binding_revision_number(vip['id']) + + # 3) Create two ports with the allowed address pairs set. + hosts = ('ovs-host1', second_chassis_name) + ports = [] + for idx in range(len(hosts)): + ports.append(self.create_port(host=hosts[idx])) + data = {'port': {'allowed_address_pairs': allowed_address_pairs}} + req = self.new_update_request('ports', data, ports[idx]['id']) + req.get_response(self.api) + + port_ids = [p['id'] for p in ports] + + # 4) Check that the vip port has become virtual and that both parents + # have been assigned to the port binding + self._check_port_binding_type(vip['id'], ovn_const.LSP_TYPE_VIRTUAL) + self._check_port_virtual_parents(vip['id'], ','.join(port_ids)) + self._check_port_and_port_binding_revision_number(vip['id']) + + # 5) Bind the ports to a host, so a chassis is bound, which is + # required for the update_virtual_port_host method. Without this + # chassis set, it will not set a hostname in the DB's + self._test_port_binding_and_status(ports[0]['id'], 'bind', 'ACTIVE') + self.chassis = second_chassis + self._test_port_binding_and_status(ports[1]['id'], 'bind', 'ACTIVE') + self._check_port_and_port_binding_revision_number(vip['id']) + + # 6) For both ports, bind vip on parent and check hostname in DBs + for idx in range(len(ports)): + # Set port binding to the first port, and update the chassis + self._set_port_binding_virtual_parent(vip['id'], ports[idx]['id']) + self._check_port_and_port_binding_revision_number(vip['id']) + + # Check if the host_id has been updated in OVN and DB + # by the event that eventually calls for method + # OVNMechanismDriver.update_virtual_port_host + n_utils.wait_until_true( + lambda: self._check_port_host_set(vip['id'], hosts[idx]), + timeout=10) + self._check_port_and_port_binding_revision_number(vip['id']) + class TestNBDbMonitorOverTcp(TestNBDbMonitor): def get_ovsdb_server_protocol(self): diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 0ee1257acbd..66c5235e3e8 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -1249,3 +1249,103 @@ def test_agent_delete(self): self.plugin.delete_agent(self.context, metadata_id) self.assertRaises(agent_exc.AgentNotFound, self.plugin.get_agent, self.context, metadata_id) + + +class TestNATRuleGatewayPort(base.TestOVNFunctionalBase): + + def setUp(self): + super().setUp() + self._ovn_client = self.mech_driver._ovn_client + + def deserialize(self, content_type, response): + ctype = 'application/%s' % content_type + data = self._deserializers[ctype].deserialize(response.body)['body'] + return data + + def _create_router(self, name, external_gateway_info=None): + data = {'router': {'name': name, 'tenant_id': self._tenant_id, + 'external_gateway_info': external_gateway_info}} + req = self.new_create_request('routers', data, self.fmt) + res = req.get_response(self.api) + return self.deserialize(self.fmt, res)['router'] + + def _process_router_interface(self, action, router_id, subnet_id): + req = self.new_action_request( + 'routers', {'subnet_id': subnet_id}, router_id, + '%s_router_interface' % action) + res = req.get_response(self.api) + return self.deserialize(self.fmt, res) + + def _add_router_interface(self, router_id, subnet_id): + return self._process_router_interface('add', router_id, subnet_id) + + def _create_port(self, name, net_id, security_groups=None, + device_owner=None): + data = {'port': {'name': name, + 'tenant_id': self._tenant_id, + 'network_id': net_id}} + + if security_groups is not None: + data['port']['security_groups'] = security_groups + + if device_owner is not None: + data['port']['device_owner'] = device_owner + + req = self.new_create_request('ports', data, self.fmt) + res = req.get_response(self.api) + return self.deserialize(self.fmt, res)['port'] + + def test_create_floatingip(self): + ext_net = self._make_network( + self.fmt, 'ext_networktest', True, + arg_list=('router:external', + 'provider:network_type', + 'provider:physical_network'), + **{'router:external': True, + 'provider:network_type': 'flat', + 'provider:physical_network': 'public'})['network'] + res = self._create_subnet(self.fmt, ext_net['id'], + '100.0.0.0/24', gateway_ip='100.0.0.254', + allocation_pools=[{'start': '100.0.0.2', + 'end': '100.0.0.253'}], + enable_dhcp=False) + ext_subnet = self.deserialize(self.fmt, res)['subnet'] + net1 = self._make_network( + self.fmt, 'network1test', True)['network'] + res = self._create_subnet(self.fmt, net1['id'], + '192.168.0.0/24', gateway_ip='192.168.0.1', + allocation_pools=[{'start': '192.168.0.2', + 'end': '192.168.0.253'}], + enable_dhcp=False) + subnet1 = self.deserialize(self.fmt, res)['subnet'] + external_gateway_info = { + 'enable_snat': True, + 'network_id': ext_net['id'], + 'external_fixed_ips': [ + {'ip_address': '100.0.0.2', 'subnet_id': ext_subnet['id']}]} + router = self._create_router( + 'routertest', external_gateway_info=external_gateway_info) + self._add_router_interface(router['id'], subnet1['id']) + + p1 = self._create_port('testp1', net1['id']) + logical_ip = p1['fixed_ips'][0]['ip_address'] + fip_info = {'floatingip': { + 'tenant_id': self._tenant_id, + 'description': 'test_fip', + 'floating_network_id': ext_net['id'], + 'port_id': p1['id'], + 'fixed_ip_address': logical_ip}} + + fip = self.l3_plugin.create_floatingip(self.context, fip_info) + + self.assertEqual(router['id'], fip['router_id']) + self.assertEqual('testp1', fip['port_details']['name']) + self.assertIsNotNone(self.nb_api.get_lswitch_port(fip['port_id'])) + + rules = self.nb_api.get_all_logical_routers_with_rports()[0] + fip_rule = rules['dnat_and_snats'][0] + + if utils.is_nat_gateway_port_supported(self.nb_api): + self.assertNotEqual([], fip_rule['gateway_port']) + else: + self.assertNotIn('gateway_port', fip_rule) diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py index 8eee3a4bf3a..dd286ae5929 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -909,8 +909,13 @@ def test_delete_all_port_flows(self): "reg6": port.vlan_tag} flow7 = mock.call(**call_args7) + call_args8 = {"table": ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE, + "dl_dst": port.mac, + "dl_vlan": port.vlan_tag} + flow8 = mock.call(**call_args8) + self.mock_bridge.br.delete_flows.assert_has_calls( - [flow1, flow2, flow3, flow6, flow7, flow4, flow5]) + [flow1, flow2, flow3, flow6, flow7, flow8, flow4, flow5]) def test_prepare_port_filter_initialized_port(self): port_dict = {'device': 'port-id', diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index ebbb374d73c..37d76cbbc59 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -164,6 +164,7 @@ def __init__(self, **kwargs): self.ha_chassis_group_del_chassis = mock.Mock() self.lrp_get = mock.Mock() self.get_schema_version = mock.Mock(return_value='3.6.0') + self.get_lrouter_port = mock.Mock() class FakeOvsdbSbOvnIdl(object): diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py index 8ad863c7665..5d92167cdad 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py @@ -73,7 +73,7 @@ def test_setup_default_table(self): ]), ], match=ofpp.OFPMatch(), - priority=3, + priority=1, table_id=ovs_constants.TRANSIENT_TABLE), active_bundle=None), call._send_msg(ofpp.OFPFlowMod(dp, diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py index 209a41bf98c..584f13fda6c 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py @@ -15,6 +15,7 @@ import copy from unittest import mock +from oslo_utils import uuidutils from ovsdbapp.backend import ovs_idl from neutron.common.ovn import constants as ovn_const @@ -193,11 +194,15 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn): 'type': 'snat'}, {'external_ip': '20.0.2.4', 'logical_ip': '10.0.0.4', 'type': 'dnat_and_snat', 'external_mac': [], - 'logical_port': []}, + 'logical_port': [], + 'external_ids': {ovn_const.OVN_FIP_EXT_ID_KEY: 'fip_id_a'}, + 'gateway_port': uuidutils.generate_uuid()}, {'external_ip': '20.0.2.5', 'logical_ip': '10.0.0.5', 'type': 'dnat_and_snat', 'external_mac': ['00:01:02:03:04:05'], - 'logical_port': ['lsp-id-001']}], + 'logical_port': ['lsp-id-001'], + 'external_ids': {ovn_const.OVN_FIP_EXT_ID_KEY: 'fip_id_b'}, + 'gateway_port': []}], 'acls': [ {'unit_test_id': 1, 'action': 'allow-related', 'direction': 'from-lport', @@ -446,13 +451,39 @@ def test_get_all_logical_switches_with_ports(self): 'provnet_ports': []}] self.assertCountEqual(mapping, expected) - def test_get_all_logical_routers_with_rports(self): + def _test_get_all_logical_routers_with_rports(self, is_gw_port): # Test empty mapping = self.nb_ovn_idl.get_all_logical_switches_with_ports() self.assertCountEqual(mapping, {}) # Test loaded values self._load_nb_db() + + # Test with gateway_port_support enabled + utils.is_nat_gateway_port_supported = mock.Mock() + utils.is_nat_gateway_port_supported.return_value = is_gw_port mapping = self.nb_ovn_idl.get_all_logical_routers_with_rports() + lra_nat = self._find_ovsdb_fake_row(self.nat_table, + 'external_ip', '20.0.2.4') + lrb_nat = self._find_ovsdb_fake_row(self.nat_table, + 'external_ip', '20.0.2.5') + + lra_fip = {'external_ip': '20.0.2.4', + 'logical_ip': '10.0.0.4', + 'type': 'dnat_and_snat', + 'external_ids': {ovn_const.OVN_FIP_EXT_ID_KEY: 'fip_id_a'}, + 'uuid': lra_nat.uuid} + lrb_fip = {'external_ip': '20.0.2.5', + 'logical_ip': '10.0.0.5', + 'type': 'dnat_and_snat', + 'external_mac': '00:01:02:03:04:05', + 'logical_port': 'lsp-id-001', + 'external_ids': {ovn_const.OVN_FIP_EXT_ID_KEY: 'fip_id_b'}, + 'uuid': lrb_nat.uuid} + + if is_gw_port: + lra_fip['gateway_port'] = lra_nat.gateway_port + lrb_fip['gateway_port'] = lrb_nat.gateway_port + expected = [{'name': 'lr-id-a', 'ports': {'orp-id-a1': ['10.0.1.0/24'], 'orp-id-a2': ['10.0.2.0/24'], @@ -471,14 +502,7 @@ def test_get_all_logical_routers_with_rports(self): 'snats': [{'external_ip': '20.0.2.1', 'logical_ip': '10.0.0.0/24', 'type': 'snat'}], - 'dnat_and_snats': [{'external_ip': '20.0.2.4', - 'logical_ip': '10.0.0.4', - 'type': 'dnat_and_snat'}, - {'external_ip': '20.0.2.5', - 'logical_ip': '10.0.0.5', - 'type': 'dnat_and_snat', - 'external_mac': '00:01:02:03:04:05', - 'logical_port': 'lsp-id-001'}]}, + 'dnat_and_snats': [lra_fip, lrb_fip]}, {'name': 'lr-id-c', 'ports': {}, 'static_routes': [], 'snats': [], 'dnat_and_snats': []}, {'name': 'lr-id-d', 'ports': {}, 'static_routes': [], @@ -487,6 +511,12 @@ def test_get_all_logical_routers_with_rports(self): 'snats': [], 'dnat_and_snats': []}] self.assertCountEqual(mapping, expected) + def test_get_all_logical_routers_with_rports(self): + self._test_get_all_logical_routers_with_rports(True) + + def test_get_all_logical_routers_with_rports_without_nat_gw_port(self): + self._test_get_all_logical_routers_with_rports(False) + def test_get_acls_for_lswitches(self): self._load_nb_db() # Test neutron switches diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 84798506569..c4530a66ec1 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -979,7 +979,7 @@ def test_add_gw_port_info_to_logical_router_port_no_action_needed(self): internal_net_id = uuidutils.generate_uuid() routers_db = [{ 'id': uuidutils.generate_uuid(), - 'external_gateways': [{'network_id': ext_net_id}]}] + 'external_gateway_info': {'network_id': ext_net_id}}] ext_gw_lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'external_ids': { constants.OVN_NETWORK_NAME_EXT_ID_KEY: @@ -1055,3 +1055,66 @@ def test_add_vnic_type_and_pb_capabilities_to_lsp(self, mock_get_pb): external_ids=external_ids)] nb_idl.set_lswitch_port.assert_has_calls(expected_calls) self.assertEqual(2, nb_idl.set_lswitch_port.call_count) + + def test_update_nat_floating_ip_with_gateway_port(self): + _nb_idl = self.fake_ovn_client._nb_idl + utils.is_nat_gateway_port_supported.return_value = True + + lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={'options': {}}) + _nb_idl.get_lrouter_port.return_value = lrp + self.fake_external_fixed_ips = { + 'network_id': 'ext-network-id', + 'external_fixed_ips': [{'ip_address': '20.0.2.1', + 'subnet_id': 'ext-subnet-id'}]} + lrouter = { + 'id': 'lr-id-b', + 'name': utils.ovn_name('lr-id-b'), + 'admin_state_up': True, + 'external_gateway_info': self.fake_external_fixed_ips, + 'gw_port_id': 'gw-port-id' + } + _nb_idl._l3_plugin.get_router.return_value = lrouter + + lra_nat = {'external_ip': '20.0.2.4', 'logical_ip': '10.0.0.4', + 'type': 'dnat_and_snat', 'external_mac': [], + 'logical_port': [], + 'external_ids': {constants.OVN_FIP_EXT_ID_KEY: 'fip_id_1'}, + 'gateway_port': uuidutils.generate_uuid(), + 'uuid': uuidutils.generate_uuid()} + + lrb_nat = {'external_ip': '20.0.2.5', 'logical_ip': '10.0.0.5', + 'type': 'dnat_and_snat', + 'external_mac': ['00:01:02:03:04:05'], + 'logical_port': ['lsp-id-001'], + 'external_ids': {constants.OVN_FIP_EXT_ID_KEY: 'fip_id_2'}, + 'gateway_port': [], + 'uuid': uuidutils.generate_uuid()} + + expected = [{'name': 'lr-id-a', + 'ports': {'orp-id-a1': ['10.0.1.0/24'], + 'orp-id-a2': ['10.0.2.0/24'], + 'orp-id-a3': ['10.0.3.0/24']}, + 'static_routes': [{'destination': '20.0.0.0/16', + 'nexthop': '10.0.3.253'}], + 'snats': [{'external_ip': '10.0.3.1', + 'logical_ip': '20.0.0.0/16', + 'type': 'snat'}], + 'dnat_and_snats': []}, + {'name': 'lr-id-b', + 'ports': {'xrp-id-b1': ['20.0.1.0/24'], + 'orp-id-b2': ['20.0.2.0/24']}, + 'static_routes': [{'destination': '10.0.0.0/16', + 'nexthop': '20.0.2.253'}], + 'snats': [{'external_ip': '20.0.2.1', + 'logical_ip': '10.0.0.0/24', + 'type': 'snat'}], + 'dnat_and_snats': [lra_nat, lrb_nat]}] + _nb_idl.get_all_logical_routers_with_rports.return_value = expected + + self.assertRaises(periodics.NeverAgain, + self.periodic.update_nat_floating_ip_with_gateway_port_reference) + + _nb_idl.set_nat_rule_in_lrouter.assert_called_once_with( + utils.ovn_name('lr-id-b'), + lrb_nat['uuid'], + gateway_port=lrp.uuid) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index 8121d51dd87..a0f74336e4a 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -392,6 +392,65 @@ def test_event_matches(self): 'type': '_fake_'})) +class TestPortBindingUpdateVirtualPortsEvent(base.BaseTestCase): + def setUp(self): + super().setUp() + self.event = ovsdb_monitor.PortBindingUpdateVirtualPortsEvent(None) + + self.pbtable = fakes.FakeOvsdbTable.create_one_ovsdb_table( + attrs={'name': 'Port_Binding'}) + self.ovsdb_row = fakes.FakeOvsdbRow.create_one_ovsdb_row + + self.row = self.ovsdb_row( + attrs={'_table': self.pbtable, + 'chassis': 'newchassis', + 'options': { + 'virtual-parents': 'uuid1,uuid2'}}) + + def test_delete_event_matches(self): + # Delete event (only type virtual). + self.assertFalse(self.event.match_fn( + self.event.ROW_DELETE, + self.ovsdb_row(attrs={'_table': self.pbtable, 'type': '_fake_'}), + None)) + self.assertTrue(self.event.match_fn( + self.event.ROW_DELETE, + self.ovsdb_row(attrs={'_table': self.pbtable, 'type': 'virtual'}), + None)) + + def test_event_no_match_no_options(self): + # Unrelated portbind change (no options in old, so no virtual parents) + self.assertFalse(self.event.match_fn( + self.event.ROW_UPDATE, self.row, + self.ovsdb_row(attrs={'_table': self.pbtable, + 'name': 'somename'}))) + + def test_event_no_match_other_options_change(self): + # Non-virtual parent change, no chassis has changed + old = self.ovsdb_row(attrs={'_table': self.pbtable, + 'options': { + 'virtual-parents': 'uuid1,uuid2', + 'other-opt': '_fake_'}}) + + self.assertFalse(self.event.match_fn(self.event.ROW_UPDATE, + self.row, old)) + + def test_event_match_chassis_change(self): + # Port binding change (chassis changed, and marked in old) + self.assertTrue(self.event.match_fn( + self.event.ROW_UPDATE, self.row, + self.ovsdb_row(attrs={'_table': self.pbtable, + 'chassis': 'fakechassis'}))) + + def test_event_match_virtual_parent_change(self): + # Virtual parent change + old = self.ovsdb_row(attrs={'_table': self.pbtable, + 'options': { + 'virtual-parents': 'uuid1,uuid3'}}) + self.assertTrue(self.event.match_fn(self.event.ROW_UPDATE, + self.row, old)) + + class TestOvnNbIdlNotifyHandler(test_mech_driver.OVNMechanismDriverTestCase): def setUp(self): diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index c795db28037..3f3767467e7 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -307,6 +307,9 @@ def setUp(self): 'neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.ovn_client.' 'OVNClient.delete_mac_binding_entries_by_mac', return_value=1) + self._start_mock( + 'neutron.common.ovn.utils.is_nat_gateway_port_supported', + return_value=False) def test__plugin_driver(self): # No valid mech drivers should raise an exception. @@ -1149,6 +1152,58 @@ def test_create_floatingip_lb_vip_fip(self): mock.call('NAT', self.fake_ovn_nat_rule.uuid, 'external_mac'), mock.call('NAT', self.fake_ovn_nat_rule.uuid, 'logical_port')]) + def _test_create_floatingip_gateway_port_option(self, is_gw_port): + _nb_ovn = self.l3_inst._nb_ovn + _nb_ovn.is_col_present.return_value = True + self._get_floatingip.return_value = {'floating_port_id': 'fip-port-id'} + _nb_ovn.get_lrouter_nat_rules.return_value = [ + {'external_ip': '192.168.0.10', 'logical_ip': '10.0.0.0/24', + 'type': 'snat', 'uuid': 'uuid1'}] + utils.is_nat_gateway_port_supported.return_value = is_gw_port + + lrp = fake_resources.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'options': {}}) + _nb_ovn.get_lrouter_port.return_value = lrp + self.l3_inst.get_router.return_value = self.fake_router_with_ext_gw + + self.l3_inst.create_floatingip(self.context, 'floatingip') + _nb_ovn.set_nat_rule_in_lrouter.assert_not_called() + + expected_ext_ids = { + ovn_const.OVN_FIP_EXT_ID_KEY: self.fake_floating_ip['id'], + ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1', + ovn_const.OVN_FIP_PORT_EXT_ID_KEY: + self.fake_floating_ip['port_id'], + ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( + self.fake_floating_ip['router_id']), + ovn_const.OVN_FIP_EXT_MAC_KEY: 'aa:aa:aa:aa:aa:aa', + ovn_const.OVN_FIP_NET_ID: + self.fake_floating_ip['floating_network_id']} + + if is_gw_port: + _nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with( + 'neutron-router-id', + type='dnat_and_snat', + logical_ip='10.0.0.10', + external_ip='192.168.0.10', + logical_port='port_id', + external_ids=expected_ext_ids, + gateway_port=lrp.uuid) + else: + _nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with( + 'neutron-router-id', + type='dnat_and_snat', + logical_ip='10.0.0.10', + external_ip='192.168.0.10', + logical_port='port_id', + external_ids=expected_ext_ids) + + def test_create_floatingip_with_gateway_port(self): + self._test_create_floatingip_gateway_port_option(True) + + def test_create_floatingip_without_gateway_port(self): + self._test_create_floatingip_gateway_port_option(False) + @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.delete_floatingip') def test_delete_floatingip(self, df): nb_ovn = self.l3_inst._nb_ovn diff --git a/releasenotes/notes/add-gw-port-support-for-FIP-fb97b85f5928740b.yaml b/releasenotes/notes/add-gw-port-support-for-FIP-fb97b85f5928740b.yaml new file mode 100644 index 00000000000..07544b85468 --- /dev/null +++ b/releasenotes/notes/add-gw-port-support-for-FIP-fb97b85f5928740b.yaml @@ -0,0 +1,15 @@ +--- +prelude: > + The OVN changed support for NAT rules including a new column and + auto-discovery logic to know about logical router gateway ports for NAT on + a Logical Router. +features: + - | + A new OVN driver Northbound DB column has been added to allow configuring + gateway port for NAT rule. If the OVN backend supports the `gateway_port` + column in the Northbound DB NAT table, the gateway port uuid will be + configured to any floating IP to prevent North/South traffic issues. + Previously created FIP rules will be updated only once during the + maintenance task to include the gateway_port reference (if OVN backend + supports it). In case all FIP entries are already configured no maintenance + action will be performed.