openstack/os-vif
Code Issues Proposed changes

Move mtu update request into ovsdb transaction

Browse Source 
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
This commit is contained in:
Rajesh Tailor 2022-09-15 16:23:44 +05:30
parent 35705ba59f
commit 2f55e9ea06
3 changed files with 60 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
vif_plug_ovs/ovsdb/ovsdb_lib.py
View File

@ -44,10 +44,14 @@ class BaseOVS(object):
def _ovs_supports_mtu_requests(self): def _ovs_supports_mtu_requests(self):
return self.ovsdb.has_table_column('Interface', 'mtu_request') return self.ovsdb.has_table_column('Interface', 'mtu_request')
def _set_mtu_request(self, dev, mtu): def _set_mtu_request(self, txn, dev, mtu):
self.ovsdb.db_set('Interface', dev, ('mtu_request', mtu)).execute() 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: if not mtu:
return return
if interface_type not in [ if interface_type not in [
@ -60,7 +64,7 @@ class BaseOVS(object):
# programming the MTU and fallback to DHCP advertisement. # programming the MTU and fallback to DHCP advertisement.
linux_net.set_device_mtu(dev, mtu) linux_net.set_device_mtu(dev, mtu)
elif self._ovs_supports_mtu_requests(): elif self._ovs_supports_mtu_requests():
self._set_mtu_request(dev, mtu) self._set_mtu_request(txn, dev, mtu)
else: else:
LOG.debug("MTU not set on %(interface_name)s interface " LOG.debug("MTU not set on %(interface_name)s interface "
"of type %(interface_type)s.", "of type %(interface_type)s.",
@ -186,10 +190,15 @@ class BaseOVS(object):
txn.add(self.ovsdb.db_set('Port', dev, ('tag', tag))) txn.add(self.ovsdb.db_set('Port', dev, ('tag', tag)))
if col_values: if col_values:
txn.add(self.ovsdb.db_set('Interface', dev, *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): 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): def delete_ovs_vif_port(self, bridge, dev, delete_netdev=True):
self.ovsdb.del_port(dev, bridge=bridge, if_exists=True).execute() self.ovsdb.del_port(dev, bridge=bridge, if_exists=True).execute()

6
vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py
View File

@ -87,9 +87,11 @@ class TestOVSDBLib(testscenarios.WithScenarios,
self.addCleanup(self._del_bridge, self.brname) self.addCleanup(self._del_bridge, self.brname)
self._add_port(self.brname, port_name) self._add_port(self.brname, port_name)
if self.ovs._ovs_supports_mtu_requests(): 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._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) self._check_parameter('Interface', port_name, 'mtu', 1500)
else: else:
self.skipTest('Current version of Open vSwitch does not support ' self.skipTest('Current version of Open vSwitch does not support '

56
vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py
View File

@ -49,7 +49,7 @@ class BaseOVSTest(testtools.TestCase):
'transaction').start() 'transaction').start()
def test__set_mtu_request(self): 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))] calls = [mock.call('Interface', 'device', ('mtu_request', 1500))]
self.mock_db_set.assert_has_calls(calls) self.mock_db_set.assert_has_calls(calls)
@ -57,14 +57,18 @@ class BaseOVSTest(testtools.TestCase):
@mock.patch.object(linux_net, 'set_device_mtu') @mock.patch.object(linux_net, 'set_device_mtu')
def test__update_device_mtu_interface_not_vhostuser_linux(self, def test__update_device_mtu_interface_not_vhostuser_linux(self,
mock_set_device_mtu): 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_set_device_mtu.assert_has_calls([mock.call('device', 1500)])
@mock.patch('sys.platform', constants.PLATFORM_WIN32) @mock.patch('sys.platform', constants.PLATFORM_WIN32)
@mock.patch.object(linux_net, 'set_device_mtu') @mock.patch.object(linux_net, 'set_device_mtu')
def test__update_device_mtu_interface_not_vhostuser_windows(self, def test__update_device_mtu_interface_not_vhostuser_windows(self,
mock_set_device_mtu): 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() mock_set_device_mtu.assert_not_called()
def test__update_device_mtu_interface_vhostuser_supports_mtu_req(self): def test__update_device_mtu_interface_vhostuser_supports_mtu_req(self):
@ -72,17 +76,27 @@ class BaseOVSTest(testtools.TestCase):
return_value=True), \ return_value=True), \
mock.patch.object(self.br, '_set_mtu_request') as \ mock.patch.object(self.br, '_set_mtu_request') as \
mock_set_mtu_request: mock_set_mtu_request:
self.br.update_device_mtu('device', 1500, self.br.update_device_mtu(
constants.OVS_VHOSTUSER_INTERFACE_TYPE) self.mock_transaction, 'device', 1500,
mock_set_mtu_request.assert_has_calls([mock.call('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): def test__update_device_mtu_interface_vhostuser_not_supports_mtu_req(self):
with mock.patch.object(self.br, '_ovs_supports_mtu_requests', with mock.patch.object(self.br, '_ovs_supports_mtu_requests',
return_value=False), \ return_value=False), \
mock.patch.object(self.br, '_set_mtu_request') as \ mock.patch.object(self.br, '_set_mtu_request') as \
mock_set_mtu_request: mock_set_mtu_request:
self.br.update_device_mtu('device', 1500, self.br.update_device_mtu(
constants.OVS_VHOSTUSER_INTERFACE_TYPE) self.mock_transaction, 'device', 1500,
constants.OVS_VHOSTUSER_INTERFACE_TYPE
)
mock_set_mtu_request.assert_not_called() mock_set_mtu_request.assert_not_called()
def test_create_ovs_vif_port(self): def test_create_ovs_vif_port(self):
@ -115,8 +129,14 @@ class BaseOVSTest(testtools.TestCase):
self.mock_db_set.assert_has_calls( self.mock_db_set.assert_has_calls(
[mock.call('Port', device, ('tag', 4000)), [mock.call('Port', device, ('tag', 4000)),
mock.call('Interface', device, *values)]) mock.call('Interface', device, *values)])
mock_update_device_mtu.assert_has_calls( with self.br._ovsdb.transaction() as txn:
[mock.call(device, mtu, interface_type=interface_type)]) 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): def test_create_ovs_vif_port_type_dpdk(self):
iface_id = 'iface_id' 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_add_port.assert_has_calls([mock.call(bridge, device)])
self.mock_db_set.assert_has_calls( self.mock_db_set.assert_has_calls(
[mock.call('Interface', device, *values)]) [mock.call('Interface', device, *values)])
mock_update_device_mtu.assert_has_calls( with self.br._ovsdb.transaction() as txn:
[mock.call(device, mtu, interface_type=interface_type)]) mock_update_device_mtu.assert_has_calls(
[
mock.call(
txn, device, mtu, interface_type=interface_type
)
]
)
def test_update_ovs_vif_port(self): def test_update_ovs_vif_port(self):
with mock.patch.object(self.br, 'update_device_mtu') as \ with mock.patch.object(self.br, 'update_device_mtu') as \
mock_update_device_mtu: mock_update_device_mtu:
self.br.update_ovs_vif_port('device', mtu=1500, self.br.update_ovs_vif_port('device', mtu=1500,
interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE)
mock_update_device_mtu.assert_has_calls( with self.br._ovsdb.transaction() as txn:
[mock.call( mock_update_device_mtu.assert_has_calls([mock.call(
'device', 1500, txn, 'device', 1500,
interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE)]) interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE)])
@mock.patch.object(linux_net, 'delete_net_dev') @mock.patch.object(linux_net, 'delete_net_dev')