From 829b39b1ebdb96d87323852e2e099a955e402b63 Mon Sep 17 00:00:00 2001 From: IWAMOTO Toshihiro Date: Wed, 30 Nov 2016 15:26:39 +0900 Subject: [PATCH] ovsfw: Refresh OFPort when necessary Events like server reboots change ofport numbers. In such cases, cached ofports need to be refreshed. Change-Id: If4acf61736b8f1e9707efc409509e1f557d5f886 Closes-Bug: #1645655 --- .../linux/openvswitch_firewall/firewall.py | 31 +++++++++++++------ .../openvswitch_firewall/test_firewall.py | 23 +++++++++++++- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index 502c69fd6d0..5590f659d39 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -243,19 +243,31 @@ class OVSFirewallDriver(firewall.FirewallDriver): for table in ovs_consts.OVS_FIREWALL_TABLES: self.int_br.br.add_flow(table=table, priority=0, actions='drop') - def get_or_create_ofport(self, port): + def get_ofport(self, port): port_id = port['device'] + return self.sg_port_map.ports.get(port_id) + + def get_or_create_ofport(self, port): + """Get ofport specified by port['device'], checking and reflecting + ofport changes. + If ofport is nonexistent, create and return one. + """ + port_id = port['device'] + ovs_port = self.int_br.br.get_vif_port_by_id(port_id) + if not ovs_port: + raise exceptions.OVSFWPortNotFound(port_id=port_id) + try: of_port = self.sg_port_map.ports[port_id] except KeyError: - ovs_port = self.int_br.br.get_vif_port_by_id(port_id) - if not ovs_port: - raise exceptions.OVSFWPortNotFound(port_id=port_id) port_vlan_id = get_tag_from_other_config( self.int_br.br, ovs_port.port_name) of_port = OFPort(port, ovs_port, port_vlan_id) self.sg_port_map.create_port(of_port, port) else: + if of_port.ofport != ovs_port.ofport: + self.sg_port_map.remove_port(of_port) + of_port = OFPort(port, ovs_port, of_port.vlan_tag) self.sg_port_map.update_port(of_port, port) return of_port @@ -266,13 +278,13 @@ class OVSFirewallDriver(firewall.FirewallDriver): def prepare_port_filter(self, port): if not firewall.port_sec_enabled(port): return - port_exists = self.is_port_managed(port) + old_of_port = self.get_ofport(port) of_port = self.get_or_create_ofport(port) - if port_exists: + if old_of_port: LOG.error(_LE("Initializing port %s that was already " "initialized."), port['device']) - self.delete_all_port_flows(of_port) + self.delete_all_port_flows(old_of_port) self.initialize_port_flows(of_port) self.add_flows_from_rules(of_port) @@ -289,9 +301,10 @@ class OVSFirewallDriver(firewall.FirewallDriver): elif not self.is_port_managed(port): self.prepare_port_filter(port) return + old_of_port = self.get_ofport(port) of_port = self.get_or_create_ofport(port) # TODO(jlibosva): Handle firewall blink - self.delete_all_port_flows(of_port) + self.delete_all_port_flows(old_of_port) self.initialize_port_flows(of_port) self.add_flows_from_rules(of_port) @@ -303,7 +316,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): """ if self.is_port_managed(port): - of_port = self.get_or_create_ofport(port) + of_port = self.get_ofport(port) self.delete_all_port_flows(of_port) self.sg_port_map.remove_port(of_port) 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 b9160ebb3c7..c59457f7685 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -30,7 +30,8 @@ TESTING_VLAN_TAG = 1 def create_ofport(port_dict): - ovs_port = mock.Mock(vif_mac='00:00:00:00:00:00', port_name="port-name") + ovs_port = mock.Mock(vif_mac='00:00:00:00:00:00', ofport=1, + port_name="port-name") return ovsfw.OFPort(port_dict, ovs_port, vlan_tag=TESTING_VLAN_TAG) @@ -327,6 +328,18 @@ class TestOVSFirewallDriver(base.BaseTestCase): self.assertIn(port, sg1.ports) self.assertIn(port, sg2.ports) + def test_get_or_create_ofport_changed(self): + port_dict = { + 'device': 'port-id', + 'security_groups': [123, 456]} + of_port = create_ofport(port_dict) + self.firewall.sg_port_map.ports[of_port.id] = of_port + fake_ovs_port = FakeOVSPort('port', 2, '00:00:00:00:00:00') + self.mock_bridge.br.get_vif_port_by_id.return_value = \ + fake_ovs_port + port = self.firewall.get_or_create_ofport(port_dict) + self.assertEqual(port.ofport, 2) + def test_get_or_create_ofport_missing(self): port_dict = { 'device': 'port-id', @@ -335,6 +348,14 @@ class TestOVSFirewallDriver(base.BaseTestCase): with testtools.ExpectedException(exceptions.OVSFWPortNotFound): self.firewall.get_or_create_ofport(port_dict) + def test_get_or_create_ofport_missing_nocreate(self): + port_dict = { + 'device': 'port-id', + 'security_groups': [123, 456]} + self.mock_bridge.br.get_vif_port_by_id.return_value = None + self.assertIsNone(self.firewall.get_ofport(port_dict)) + self.assertFalse(self.mock_bridge.br.get_vif_port_by_id.called) + def test_is_port_managed_managed_port(self): port_dict = {'device': 'port-id'} self.firewall.sg_port_map.ports[port_dict['device']] = object()