From d163bf7e4a1ca34eb9cbcd7ee408026fb4aca127 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Fri, 14 Oct 2022 11:58:02 +0200 Subject: [PATCH] Disable in-band management for bridges before setting up controllers Disabling in-band management for bridge will effectively disable it for all controllers which are or will be set for the bridge. This will prevent us from having short time between configuring controller and setting connection_mode of the controller to "out-of-band" when controller works in the default "in-band" connection mode and adds some hidden flows to the bridge. Conflicts: neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py neutron/tests/functional/agent/test_ovs_lib.py neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge_test_base.py Closes-Bug: #1992953 Change-Id: Ibca81eb59fbfad71f223832228f408fb248c5dfa (cherry picked from commit 8fcf00a36dfcec80bba73b63896d806f48835faf) (cherry picked from commit 9d826bc77aab36dd194cc471397af68bd4ad39e5) (cherry picked from commit 32a8b2d3388b0e1ed4b19a5f3b68a99762f9f70b) (cherry picked from commit b662fd25773647259f0e8c197beafa6a5ca385d4) --- neutron/agent/common/ovs_lib.py | 19 ++++++---- .../agent/openflow/native/ovs_bridge.py | 38 +++++++++++-------- .../tests/functional/agent/test_ovs_lib.py | 37 +++++------------- .../openflow/native/ovs_bridge_test_base.py | 7 ++-- 4 files changed, 47 insertions(+), 54 deletions(-) diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index 4e989043516..a0fb64e5ba9 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -260,6 +260,18 @@ class OVSBridge(BaseOVS): def set_agent_uuid_stamp(self, val): self._default_cookie = val + def disable_in_band(self): + """Disable in-band remote management for the bridge. + + That configuration will apply to all controllers configured for the + bridge. + """ + other_config = { + 'disable-in-band': 'true'} + self.ovsdb.db_set( + 'Bridge', self.br_name, + ('other_config', other_config)).execute(check_error=True) + def set_controller(self, controllers): self.ovsdb.set_controller(self.br_name, controllers).execute(check_error=True) @@ -748,13 +760,6 @@ class OVSBridge(BaseOVS): msg = _('Unable to determine mac address for %s') % self.br_name raise Exception(msg) - def set_controllers_connection_mode(self, connection_mode): - """Set bridge controllers connection mode. - - :param connection_mode: "out-of-band" or "in-band" - """ - self.set_controller_field('connection_mode', connection_mode) - def set_controllers_inactivity_probe(self, interval): """Set bridge controllers inactivity probe interval. diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py index 355c5f8fddc..d6ed2c9bc31 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py @@ -70,6 +70,29 @@ class OVSAgentBridge(ofswitch.OpenFlowSwitchMixin, self._cached_dpid = new_dpid def setup_controllers(self, conf): + # NOTE(slaweq): Disable remote in-band management for all controllers + # in the bridge + # + # By default openvswitch uses "in-band" controller connection mode + # which adds hidden OpenFlow rules (only visible by issuing ovs-appctl + # bridge/dump-flows
) and leads to a network loop on br-tun. As of + # now the OF controller is hosted locally with OVS which fits the + # "out-of-band" mode. If the remote OF controller is ever to be + # supported by openvswitch agent in the future, "In-Band Control" [1] + # should be taken into consideration for physical bridge only, but + # br-int and br-tun must be configured with the "out-of-band" + # controller connection mode. + # + # Setting connection_mode for controllers should be done in single + # transaction together with controllers setup but it will be easier to + # disable in-band remote management for bridge which + # effectively means that this configurations will applied to all + # controllers in the bridge + # + # [1] https://github.com/openvswitch/ovs/blob/master/DESIGN.md + # [2] https://bugzilla.redhat.com/show_bug.cgi?id=2134772 + self.disable_in_band() + controller = ( "tcp:%(address)s:%(port)s" % { "address": conf.OVS.of_listen_address, @@ -83,21 +106,6 @@ class OVSAgentBridge(ofswitch.OpenFlowSwitchMixin, self.set_controller([controller]) self.add_protocols(ovs_consts.OPENFLOW13) - # NOTE(ivc): Force "out-of-band" controller connection mode (see - # "In-Band Control" [1]). - # - # By default openvswitch uses "in-band" controller connection mode - # which adds hidden OpenFlow rules (only visible by issuing ovs-appctl - # bridge/dump-flows
) and leads to a network loop on br-tun. As of - # now the OF controller is hosted locally with OVS which fits the - # "out-of-band" mode. If the remote OF controller is ever to be - # supported by openvswitch agent in the future, "In-Band Control" [1] - # should be taken into consideration for physical bridge only, but - # br-int and br-tun must be configured with the "out-of-band" - # controller connection mode. - # - # [1] https://github.com/openvswitch/ovs/blob/master/DESIGN.md - self.set_controllers_connection_mode("out-of-band") self.set_controllers_inactivity_probe(conf.OVS.of_inactivity_probe) def drop_port(self, in_port): diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index bfed9b93201..2de4befbb9d 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -14,7 +14,6 @@ # under the License. import collections -import uuid import mock from neutron_lib import constants as const @@ -136,6 +135,15 @@ class OVSBridgeTestCase(OVSBridgeTestBase): self.br.del_controller() self.assertEqual([], self.br.get_controller()) + def test_disable_in_band(self): + self.br.disable_in_band() + br_other_config = self.ovs.ovsdb.db_find( + 'Bridge', ('name', '=', self.br.br_name), columns=['other_config'] + ).execute()[0]['other_config'] + self.assertEqual( + 'true', + br_other_config.get('disable-in-band', '').lower()) + def test_non_index_queries(self): controllers = ['tcp:127.0.0.1:6633'] self.br.set_controller(controllers) @@ -391,33 +399,6 @@ class OVSBridgeTestCase(OVSBridgeTestBase): self.br.delete_ports(all_ports=True) self.assertEqual(len(self.br.get_port_name_list()), 0) - def test_set_controller_connection_mode(self): - controllers = ['tcp:192.0.2.0:6633'] - self._set_controllers_connection_mode(controllers) - - def test_set_multi_controllers_connection_mode(self): - controllers = ['tcp:192.0.2.0:6633', 'tcp:192.0.2.1:55'] - self._set_controllers_connection_mode(controllers) - - def _set_controllers_connection_mode(self, controllers): - self.br.set_controller(controllers) - self.assertEqual(sorted(controllers), sorted(self.br.get_controller())) - self.br.set_controllers_connection_mode('out-of-band') - self._assert_controllers_connection_mode('out-of-band') - self.br.del_controller() - self.assertEqual([], self.br.get_controller()) - - def _assert_controllers_connection_mode(self, connection_mode): - controllers = self.br.db_get_val('Bridge', self.br.br_name, - 'controller') - controllers = [controllers] if isinstance( - controllers, uuid.UUID) else controllers - for controller in controllers: - self.assertEqual(connection_mode, - self.br.db_get_val('Controller', - controller, - 'connection_mode')) - def test_db_create_references(self): with self.ovs.ovsdb.transaction(check_error=True) as txn: queue = txn.add(self.ovs.ovsdb.db_create("Queue", diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge_test_base.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge_test_base.py index 7773f1fce2a..f7bedb32522 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge_test_base.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge_test_base.py @@ -146,10 +146,9 @@ class OVSBridgeTestBase(ovs_test_base.OVSOSKenTestBase): m_set_controller = mock.patch.object(self.br, 'set_controller') m_set_probe = mock.patch.object(self.br, 'set_controllers_inactivity_probe') - m_set_ccm = mock.patch.object(self.br, - 'set_controllers_connection_mode') + m_disable_in_band = mock.patch.object(self.br, 'disable_in_band') - with m_set_ccm as set_ccm, \ + with m_disable_in_band as disable_in_band, \ m_add_protocols as add_protocols, \ m_set_controller as set_controller, \ m_get_controller as get_controller, \ @@ -162,7 +161,7 @@ class OVSBridgeTestBase(ovs_test_base.OVSOSKenTestBase): set_controller.assert_not_called() else: set_controller.assert_called_once_with(["tcp:127.0.0.1:6633"]) - set_ccm.assert_called_once_with("out-of-band") + disable_in_band.assert_called_once_with() add_protocols.assert_called_once_with(constants.OPENFLOW13) def test_setup_controllers(self):