From c471c7330c6c7a642e11ceae5fd604177059a3e8 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Wed, 22 Jan 2020 15:00:32 +0000 Subject: [PATCH] [OVN] Remove VLAN check when setting external_mac This patch reverts [0]. The code wasn't accounting for VLAN provider networks, as stated in the bug #1842988, DVR won't work if the provider network (where the FIP is created) is VLAN. There was also an incosistency in how the external_mac was set for the VLAN networks. Upon creating the FIP the code was checking for the network type and not setting the external_mac attribute in case the network was VLAN type. But, if the port went down and up again (e.g if you reboot the VM) the event handler that set/unset the external_mac [1] wasn't check for the type. This is how people worked around the DVR problem (as stated in bug #1842988). For more information see bug #1842988. [0] https://github.com/openstack/networking-ovn/commit/c5aef51edc9843db605303ec8bd8610b6c55e9c2 [1] https://github.com/openstack/networking-ovn/blob/eda5d7f80d877601170631c5f5485370ea701f42/networking_ovn/ml2/mech_driver.py#L794-L800 Change-Id: Ifb795626dc9c2ac4f0104f491dd38c9b4cc902c9 Closes-Bug: #1842988 Signed-off-by: Lucas Alvares Gomes --- .../ovn/mech_driver/ovsdb/ovn_client.py | 5 +--- .../tests/unit/services/ovn_l3/test_plugin.py | 27 +++++-------------- 2 files changed, 8 insertions(+), 24 deletions(-) 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 e168c9608f5..4edea85696e 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 @@ -834,10 +834,7 @@ class OVNClient(object): columns['logical_port'] = floatingip['port_id'] ext_ids[ovn_const.OVN_FIP_EXT_MAC_KEY] = port['mac_address'] if self._nb_idl.lsp_get_up(floatingip['port_id']).execute(): - port_net = self._plugin.get_network(admin_context, - port['network_id']) - if port_net.get(pnet.NETWORK_TYPE) != const.TYPE_VLAN: - columns['external_mac'] = port['mac_address'] + columns['external_mac'] = port['mac_address'] # TODO(dalvarez): remove this check once the minimum OVS required # version contains the column (when OVS 2.8.2 is released). diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index ee7fbbf0296..eae1519a5ac 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -1088,8 +1088,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin._get_floatingip') @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' 'update_floatingip') - def _test_update_floatingip_associate_distributed(self, network_type, - uf, gf, gp, gn): + def test_update_floatingip_associate_distributed(self, uf, gf, gp, gn): self.get_a_ctx_mock_p.stop() self.l3_inst._ovn.is_col_present.return_value = True self.fake_floating_ip.update({'fixed_port_id': None}) @@ -1099,7 +1098,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): uf.return_value = self.fake_floating_ip_new fake_network_vlan = self.fake_network - fake_network_vlan[pnet.NETWORK_TYPE] = network_type + fake_network_vlan[pnet.NETWORK_TYPE] = constants.TYPE_FLAT gn.return_value = fake_network_vlan config.cfg.CONF.set_override( @@ -1114,23 +1113,11 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( self.fake_floating_ip_new['router_id']), ovn_const.OVN_FIP_EXT_MAC_KEY: '00:01:02:03:04:05'} - if network_type == constants.TYPE_VLAN: - self.l3_inst._ovn.add_nat_rule_in_lrouter.assert_called_once_with( - 'neutron-new-router-id', type='dnat_and_snat', - logical_ip='10.10.10.10', external_ip='192.168.0.10', - logical_port='new-port_id', external_ids=expected_ext_ids) - else: - self.l3_inst._ovn.add_nat_rule_in_lrouter.assert_called_once_with( - 'neutron-new-router-id', type='dnat_and_snat', - logical_ip='10.10.10.10', external_ip='192.168.0.10', - external_mac='00:01:02:03:04:05', logical_port='new-port_id', - external_ids=expected_ext_ids) - - def test_update_floatingip_associate_distributed_flat(self): - self._test_update_floatingip_associate_distributed(constants.TYPE_FLAT) - - def test_update_floatingip_associate_distributed_vlan(self): - self._test_update_floatingip_associate_distributed(constants.TYPE_VLAN) + self.l3_inst._ovn.add_nat_rule_in_lrouter.assert_called_once_with( + 'neutron-new-router-id', type='dnat_and_snat', + logical_ip='10.10.10.10', external_ip='192.168.0.10', + external_mac='00:01:02:03:04:05', logical_port='new-port_id', + external_ids=expected_ext_ids) @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin._get_floatingip') @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.'