From 2f55e9ea06f975240e3ff5aff21195ec6af76193 Mon Sep 17 00:00:00 2001 From: Rajesh Tailor Date: Thu, 15 Sep 2022 16:23:44 +0530 Subject: [PATCH] Move mtu update request into ovsdb transaction os-vif currently set the requested mtu as part of separate request to the port add command. As a result the port is initially created with mbufer pool for the jumboframe size, which results in an error in openvswitch-vswitch logs. This change moves the mtu update request into ovsdb transaction. Closes-Bug: #1959586 Change-Id: I1acd74efc100c6f949b5c72525c455aebfa2c50e --- vif_plug_ovs/ovsdb/ovsdb_lib.py | 21 +++++-- .../tests/functional/ovsdb/test_ovsdb_lib.py | 6 +- .../tests/unit/ovsdb/test_ovsdb_lib.py | 56 ++++++++++++++----- 3 files changed, 60 insertions(+), 23 deletions(-) diff --git a/vif_plug_ovs/ovsdb/ovsdb_lib.py b/vif_plug_ovs/ovsdb/ovsdb_lib.py index 98cba802..f2348cd1 100644 --- a/vif_plug_ovs/ovsdb/ovsdb_lib.py +++ b/vif_plug_ovs/ovsdb/ovsdb_lib.py @@ -44,10 +44,14 @@ class BaseOVS(object): def _ovs_supports_mtu_requests(self): return self.ovsdb.has_table_column('Interface', 'mtu_request') - def _set_mtu_request(self, dev, mtu): - self.ovsdb.db_set('Interface', dev, ('mtu_request', mtu)).execute() + def _set_mtu_request(self, txn, dev, mtu): + txn.add( + self.ovsdb.db_set( + 'Interface', dev, ('mtu_request', mtu) + ) + ) - def update_device_mtu(self, dev, mtu, interface_type=None): + def update_device_mtu(self, txn, dev, mtu, interface_type=None): if not mtu: return if interface_type not in [ @@ -60,7 +64,7 @@ class BaseOVS(object): # programming the MTU and fallback to DHCP advertisement. linux_net.set_device_mtu(dev, mtu) elif self._ovs_supports_mtu_requests(): - self._set_mtu_request(dev, mtu) + self._set_mtu_request(txn, dev, mtu) else: LOG.debug("MTU not set on %(interface_name)s interface " "of type %(interface_type)s.", @@ -186,10 +190,15 @@ class BaseOVS(object): txn.add(self.ovsdb.db_set('Port', dev, ('tag', tag))) if col_values: txn.add(self.ovsdb.db_set('Interface', dev, *col_values)) - self.update_device_mtu(dev, mtu, interface_type=interface_type) + self.update_device_mtu( + txn, dev, mtu, interface_type=interface_type + ) def update_ovs_vif_port(self, dev, mtu=None, interface_type=None): - self.update_device_mtu(dev, mtu, interface_type=interface_type) + with self.ovsdb.transaction() as txn: + self.update_device_mtu( + txn, dev, mtu, interface_type=interface_type + ) def delete_ovs_vif_port(self, bridge, dev, delete_netdev=True): self.ovsdb.del_port(dev, bridge=bridge, if_exists=True).execute() diff --git a/vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py b/vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py index 1f600a4d..444af7e5 100644 --- a/vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py +++ b/vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py @@ -87,9 +87,11 @@ class TestOVSDBLib(testscenarios.WithScenarios, self.addCleanup(self._del_bridge, self.brname) self._add_port(self.brname, port_name) if self.ovs._ovs_supports_mtu_requests(): - self.ovs._set_mtu_request(port_name, 1000) + with self._ovsdb.transaction() as txn: + self.ovs._set_mtu_request(txn, port_name, 1000) self._check_parameter('Interface', port_name, 'mtu', 1000) - self.ovs._set_mtu_request(port_name, 1500) + with self._ovsdb.transaction() as txn: + self.ovs._set_mtu_request(txn, port_name, 1500) self._check_parameter('Interface', port_name, 'mtu', 1500) else: self.skipTest('Current version of Open vSwitch does not support ' diff --git a/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py b/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py index 899f7cd8..7251154a 100644 --- a/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py +++ b/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py @@ -49,7 +49,7 @@ class BaseOVSTest(testtools.TestCase): 'transaction').start() def test__set_mtu_request(self): - self.br._set_mtu_request('device', 1500) + self.br._set_mtu_request(self.mock_transaction, 'device', 1500) calls = [mock.call('Interface', 'device', ('mtu_request', 1500))] self.mock_db_set.assert_has_calls(calls) @@ -57,14 +57,18 @@ class BaseOVSTest(testtools.TestCase): @mock.patch.object(linux_net, 'set_device_mtu') def test__update_device_mtu_interface_not_vhostuser_linux(self, mock_set_device_mtu): - self.br.update_device_mtu('device', 1500, 'not_vhost') + self.br.update_device_mtu( + self.mock_transaction, 'device', 1500, 'not_vhost' + ) mock_set_device_mtu.assert_has_calls([mock.call('device', 1500)]) @mock.patch('sys.platform', constants.PLATFORM_WIN32) @mock.patch.object(linux_net, 'set_device_mtu') def test__update_device_mtu_interface_not_vhostuser_windows(self, mock_set_device_mtu): - self.br.update_device_mtu('device', 1500, 'not_vhost') + self.br.update_device_mtu( + self.mock_transaction, 'device', 1500, 'not_vhost' + ) mock_set_device_mtu.assert_not_called() def test__update_device_mtu_interface_vhostuser_supports_mtu_req(self): @@ -72,17 +76,27 @@ class BaseOVSTest(testtools.TestCase): return_value=True), \ mock.patch.object(self.br, '_set_mtu_request') as \ mock_set_mtu_request: - self.br.update_device_mtu('device', 1500, - constants.OVS_VHOSTUSER_INTERFACE_TYPE) - mock_set_mtu_request.assert_has_calls([mock.call('device', 1500)]) + self.br.update_device_mtu( + self.mock_transaction, 'device', 1500, + constants.OVS_VHOSTUSER_INTERFACE_TYPE + ) + mock_set_mtu_request.assert_has_calls( + [ + mock.call( + self.mock_transaction, 'device', 1500 + ) + ] + ) def test__update_device_mtu_interface_vhostuser_not_supports_mtu_req(self): with mock.patch.object(self.br, '_ovs_supports_mtu_requests', return_value=False), \ mock.patch.object(self.br, '_set_mtu_request') as \ mock_set_mtu_request: - self.br.update_device_mtu('device', 1500, - constants.OVS_VHOSTUSER_INTERFACE_TYPE) + self.br.update_device_mtu( + self.mock_transaction, 'device', 1500, + constants.OVS_VHOSTUSER_INTERFACE_TYPE + ) mock_set_mtu_request.assert_not_called() def test_create_ovs_vif_port(self): @@ -115,8 +129,14 @@ class BaseOVSTest(testtools.TestCase): self.mock_db_set.assert_has_calls( [mock.call('Port', device, ('tag', 4000)), mock.call('Interface', device, *values)]) - mock_update_device_mtu.assert_has_calls( - [mock.call(device, mtu, interface_type=interface_type)]) + with self.br._ovsdb.transaction() as txn: + mock_update_device_mtu.assert_has_calls( + [ + mock.call( + txn, device, mtu, interface_type=interface_type + ) + ] + ) def test_create_ovs_vif_port_type_dpdk(self): iface_id = 'iface_id' @@ -147,17 +167,23 @@ class BaseOVSTest(testtools.TestCase): self.mock_add_port.assert_has_calls([mock.call(bridge, device)]) self.mock_db_set.assert_has_calls( [mock.call('Interface', device, *values)]) - mock_update_device_mtu.assert_has_calls( - [mock.call(device, mtu, interface_type=interface_type)]) + with self.br._ovsdb.transaction() as txn: + mock_update_device_mtu.assert_has_calls( + [ + mock.call( + txn, device, mtu, interface_type=interface_type + ) + ] + ) def test_update_ovs_vif_port(self): with mock.patch.object(self.br, 'update_device_mtu') as \ mock_update_device_mtu: self.br.update_ovs_vif_port('device', mtu=1500, interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) - mock_update_device_mtu.assert_has_calls( - [mock.call( - 'device', 1500, + with self.br._ovsdb.transaction() as txn: + mock_update_device_mtu.assert_has_calls([mock.call( + txn, 'device', 1500, interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE)]) @mock.patch.object(linux_net, 'delete_net_dev')