From b02c09c33260635bdd05006799c87c4d5bd63ad0 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Thu, 26 Mar 2020 15:26:03 -0400 Subject: [PATCH] Remove some native openflow driver 'deferred' code From the comments, this code existed to have API compatibility between the native openflow and ovs-ofctl of_interface drivers, but since the latter was removed, this code is no longer necessary. Remove the tunnel bridge code now, the integration bridge code needs further work. Change-Id: I692789e35a4be8872ec72ffb10bc5488cab05f2b --- .../agent/openflow/native/br_tun.py | 23 --------- .../openvswitch/agent/ovs_neutron_agent.py | 24 +++------ .../agent/test_ovs_neutron_agent.py | 50 +++++++------------ 3 files changed, 24 insertions(+), 73 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_tun.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_tun.py index f361d082dc2..2d1232d44f0 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_tun.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_tun.py @@ -269,26 +269,3 @@ class OVSTunnelBridge(ovs_bridge.OVSAgentBridge, # REVISIT(yamamoto): match in_port as well? self.uninstall_flows(table_id=constants.DVR_NOT_LEARN, eth_src=mac) - - def deferred(self): - # REVISIT(yamamoto): This is for API compat with "ovs-ofctl" - # interface. Consider removing this mechanism when obsoleting - # "ovs-ofctl" interface. - # For "ovs-ofctl" interface, "deferred" mechanism would improve - # performance by batching flow-mods with a single ovs-ofctl command - # invocation. - # On the other hand, for this "native" interface, the overheads of - # each flow-mods are already minimum and batching doesn't make much - # sense. Thus this method is left as no-op. - # It might be possible to send multiple flow-mods with a single - # barrier. But it's unclear that level of performance optimization - # is desirable while it would certainly complicate error handling. - return self - - def __enter__(self): - # REVISIT(yamamoto): See the comment on deferred(). - return self - - def __exit__(self, exc_type, exc_value, traceback): - # REVISIT(yamamoto): See the comment on deferred(). - pass diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index c53dbe6d5a1..9145b5c83f3 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -770,27 +770,17 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, for lvm, agent_ports in self.get_agent_ports(fdb_entries): agent_ports.pop(self.local_ip, None) if len(agent_ports): - if not self.enable_distributed_routing: - with self.tun_br.deferred() as deferred_br: - self.fdb_add_tun(context, deferred_br, lvm, - agent_ports, self._tunnel_port_lookup) - else: - self.fdb_add_tun(context, self.tun_br, lvm, - agent_ports, self._tunnel_port_lookup) + self.fdb_add_tun(context, self.tun_br, lvm, + agent_ports, self._tunnel_port_lookup) def fdb_remove(self, context, fdb_entries): LOG.debug("fdb_remove received") for lvm, agent_ports in self.get_agent_ports(fdb_entries): agent_ports.pop(self.local_ip, None) if len(agent_ports): - if not self.enable_distributed_routing: - with self.tun_br.deferred() as deferred_br: - self.fdb_remove_tun(context, deferred_br, lvm, - agent_ports, - self._tunnel_port_lookup) - else: - self.fdb_remove_tun(context, self.tun_br, lvm, - agent_ports, self._tunnel_port_lookup) + self.fdb_remove_tun(context, self.tun_br, lvm, + agent_ports, + self._tunnel_port_lookup) def add_fdb_flow(self, br, port_info, remote_ip, lvm, ofport): if port_info == n_const.FLOODING_ENTRY: @@ -826,9 +816,7 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, def _fdb_chg_ip(self, context, fdb_entries): LOG.debug("update chg_ip received") - with self.tun_br.deferred() as deferred_br: - self.fdb_chg_ip_tun(context, deferred_br, fdb_entries, - self.local_ip) + self.fdb_chg_ip_tun(context, self.tun_br, fdb_entries, self.local_ip) def setup_entry_for_arp_reply(self, br, action, local_vid, mac_address, ip_address): diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 9745a7d8d79..a4184daaa5d 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -1768,13 +1768,12 @@ class TestOvsNeutronAgent(object): {'agent_ip': [l2pop_rpc.PortInfo(FAKE_MAC, FAKE_IP1), n_const.FLOODING_ENTRY]}}} - with mock.patch.object(self.agent.tun_br, - "deferred") as defer_fn: + with mock.patch.object(self.agent, 'tun_br', autospec=True) as tun_br: self.agent.fdb_add(None, fdb_entry) - defer_fn.assert_not_called() + tun_br.add_port.assert_not_called() self.agent.fdb_remove(None, fdb_entry) - defer_fn.assert_not_called() + tun_br.delete_port.assert_not_called() def test_fdb_add_flows(self): self._prepare_l2_pop_ofports() @@ -1792,14 +1791,12 @@ class TestOvsNeutronAgent(object): autospec=True) as add_tun_fn: self.agent.fdb_add(None, fdb_entry) add_tun_fn.assert_not_called() - deferred_br_call = mock.call.deferred().__enter__() expected_calls = [ - deferred_br_call.install_arp_responder('vlan1', FAKE_IP1, - FAKE_MAC), - deferred_br_call.install_unicast_to_tun('vlan1', 'seg1', '2', - FAKE_MAC), - deferred_br_call.install_flood_to_tun('vlan1', 'seg1', - set(['1', '2'])), + mock.call.install_arp_responder('vlan1', FAKE_IP1, FAKE_MAC), + mock.call.install_unicast_to_tun('vlan1', 'seg1', '2', + FAKE_MAC), + mock.call.install_flood_to_tun('vlan1', 'seg1', + set(['1', '2'])), ] tun_br.assert_has_calls(expected_calls) @@ -1814,17 +1811,12 @@ class TestOvsNeutronAgent(object): n_const.FLOODING_ENTRY]}}} with mock.patch.object(self.agent, 'tun_br', autospec=True) as br_tun: self.agent.fdb_remove(None, fdb_entry) - deferred_br_call = mock.call.deferred().__enter__() expected_calls = [ - mock.call.deferred(), - mock.call.deferred().__enter__(), - deferred_br_call.delete_arp_responder('vlan2', FAKE_IP1), - deferred_br_call.delete_unicast_to_tun('vlan2', FAKE_MAC), - deferred_br_call.install_flood_to_tun('vlan2', 'seg2', - set(['1'])), - deferred_br_call.delete_port('gre-02020202'), - deferred_br_call.cleanup_tunnel_port('2'), - mock.call.deferred().__exit__(None, None, None), + mock.call.delete_arp_responder('vlan2', FAKE_IP1), + mock.call.delete_unicast_to_tun('vlan2', FAKE_MAC), + mock.call.install_flood_to_tun('vlan2', 'seg2', set(['1'])), + mock.call.delete_port('gre-02020202'), + mock.call.cleanup_tunnel_port('2'), ] br_tun.assert_has_calls(expected_calls) @@ -1843,9 +1835,8 @@ class TestOvsNeutronAgent(object): fdb_entry['net1']['ports']['10.10.10.10'] = [ l2pop_rpc.PortInfo(FAKE_MAC, FAKE_IP1)] self.agent.fdb_add(None, fdb_entry) - deferred_br = tun_br.deferred().__enter__() add_tun_fn.assert_called_with( - deferred_br, 'gre-0a0a0a0a', '10.10.10.10', 'gre') + tun_br, 'gre-0a0a0a0a', '10.10.10.10', 'gre') def test_fdb_del_port(self): self._prepare_l2_pop_ofports() @@ -1853,13 +1844,9 @@ class TestOvsNeutronAgent(object): {'network_type': 'gre', 'segment_id': 'tun2', 'ports': {'2.2.2.2': [n_const.FLOODING_ENTRY]}}} - with mock.patch.object(self.agent.tun_br, 'deferred') as defer_fn,\ - mock.patch.object(self.agent.tun_br, - 'delete_port') as delete_port_fn: + with mock.patch.object(self.agent, 'tun_br', autospec=True) as tun_br: self.agent.fdb_remove(None, fdb_entry) - deferred_br = defer_fn().__enter__() - deferred_br.delete_port.assert_called_once_with('gre-02020202') - delete_port_fn.assert_not_called() + tun_br.delete_port.assert_called_once_with('gre-02020202') def test_fdb_update_chg_ip(self): self._prepare_l2_pop_ofports() @@ -1868,10 +1855,9 @@ class TestOvsNeutronAgent(object): {'agent_ip': {'before': [l2pop_rpc.PortInfo(FAKE_MAC, FAKE_IP1)], 'after': [l2pop_rpc.PortInfo(FAKE_MAC, FAKE_IP2)]}}}} - with mock.patch.object(self.agent.tun_br, 'deferred') as deferred_fn: + with mock.patch.object(self.agent, 'tun_br', autospec=True) as tun_br: self.agent.fdb_update(None, fdb_entries) - deferred_br = deferred_fn().__enter__() - deferred_br.assert_has_calls([ + tun_br.assert_has_calls([ mock.call.install_arp_responder('vlan1', FAKE_IP2, FAKE_MAC), mock.call.delete_arp_responder('vlan1', FAKE_IP1) ])