diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index c20ab295769..d359221dce7 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -2730,7 +2730,8 @@ def add_txns_to_remove_port_dns_records(self, txn, port): txn.add(self._nb_idl.dns_remove_record( ls_dns_record.uuid, ptr_record, if_exists=True)) - def create_ovn_fair_meter(self, meter_name, from_reload=False, txn=None): + def _create_ovn_fair_meter(self, meter_name, from_reload=False, txn=None, + stateless=False): """Create row in Meter table with fair attribute set to True. Create a row in OVN's NB Meter table based on well-known name. This @@ -2745,11 +2746,26 @@ def create_ovn_fair_meter(self, meter_name, from_reload=False, txn=None): """ meter = self._nb_idl.db_find_rows( "Meter", ("name", "=", meter_name)).execute(check_error=True) - # The meter is created when a log object is created, not by default. - # This condition avoids creating the meter if it wasn't there already + # The meters are created when a log object is created, not by default. + # This condition avoids creating the meter if it wasn't there already. commands = [] if from_reload and not meter: return + + burst_limit = cfg.CONF.network_log.burst_limit + rate_limit = cfg.CONF.network_log.rate_limit + if stateless: + meter_name = meter_name + "_stateless" + burst_limit = int(burst_limit / 2) + rate_limit = int(rate_limit / 2) + # The stateless meter is only created once the stateful meter was + # successfully created. + # The treatment of limits is not equal for stateful and stateless + # traffic at a kernel level according to: + # https://bugzilla.redhat.com/show_bug.cgi?id=2212952 + # The stateless meter is created to adjust this issue. + meter = self._nb_idl.db_find_rows( + "Meter", ("name", "=", meter_name)).execute(check_error=True) if meter: meter = meter[0] meter_band = self._nb_idl.lookup("Meter_Band", @@ -2757,9 +2773,8 @@ def create_ovn_fair_meter(self, meter_name, from_reload=False, txn=None): if meter_band: if all((meter.unit == "pktps", meter.fair[0], - meter_band.rate == cfg.CONF.network_log.rate_limit, - meter_band.burst_size == - cfg.CONF.network_log.burst_limit)): + meter_band.rate == rate_limit, + meter_band.burst_size == burst_limit)): # Meter (and its meter-band) unchanged: noop. return # Re-create meter (and its meter-band) with the new attributes. @@ -2773,10 +2788,15 @@ def create_ovn_fair_meter(self, meter_name, from_reload=False, txn=None): commands.append(self._nb_idl.meter_add( name=meter_name, unit="pktps", - rate=cfg.CONF.network_log.rate_limit, + rate=rate_limit, fair=True, - burst_size=cfg.CONF.network_log.burst_limit, + burst_size=burst_limit, may_exist=False, external_ids={ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY: log_const.LOGGING_PLUGIN})) self._transaction(commands, txn=txn) + + def create_ovn_fair_meter(self, meter_name, from_reload=False, txn=None): + self._create_ovn_fair_meter(meter_name, from_reload, txn) + self._create_ovn_fair_meter(meter_name, from_reload, txn, + stateless=True) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py index e46a105a045..fda944e61d6 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py @@ -424,11 +424,13 @@ def __init__(self, driver): self.l3_plugin = directory.get_plugin(constants.L3) table = 'Port_Binding' events = (self.ROW_UPDATE,) - super(PortBindingChassisEvent, self).__init__( - events, table, (('type', '=', ovn_const.OVN_CHASSIS_REDIRECT),)) + super().__init__(events, table, None) self.event_name = 'PortBindingChassisEvent' def match_fn(self, event, row, old): + if row.type != ovn_const.OVN_CHASSIS_REDIRECT: + return False + if len(old._data) == 1 and 'external_ids' in old._data: # NOTE: since [1], the NB logical_router_port.external_ids are # copied into the SB port_binding.external_ids. If only the diff --git a/neutron/services/logapi/drivers/ovn/driver.py b/neutron/services/logapi/drivers/ovn/driver.py index 5882b11300f..56b02d38fa2 100644 --- a/neutron/services/logapi/drivers/ovn/driver.py +++ b/neutron/services/logapi/drivers/ovn/driver.py @@ -28,6 +28,7 @@ from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils from neutron.conf.services import logging as log_cfg +from neutron.objects import securitygroup as sg_obj from neutron.services.logapi.common import db_api from neutron.services.logapi.common import sg_callback from neutron.services.logapi.drivers import base @@ -152,9 +153,21 @@ def _remove_acls_log(self, pgs, ovn_txn, log_name=None): msg += " for network log {}".format(log_name) LOG.info(msg, acl_changes, acl_absents, acl_visits) - def _set_acls_log(self, pgs, ovn_txn, actions_enabled, log_name): + def _set_acls_log(self, pgs, context, ovn_txn, actions_enabled, log_name): acl_changes, acl_visits = 0, 0 for pg in pgs: + meter_name = self.meter_name + if pg["name"] != ovn_const.OVN_DROP_PORT_GROUP_NAME: + sg = sg_obj.SecurityGroup.get_sg_by_id( + context, + pg["external_ids"][ovn_const.OVN_SG_EXT_ID_KEY]) + if not sg: + LOG.warning("Port Group %s is missing a corresponding " + "security group, skipping its network log " + "setting...", pg["name"]) + continue + if not sg.stateful: + meter_name = meter_name + ("_stateless") for acl_uuid in pg["acls"]: acl_visits += 1 acl = self.ovn_nb.lookup("ACL", acl_uuid) @@ -163,7 +176,7 @@ def _set_acls_log(self, pgs, ovn_txn, actions_enabled, log_name): continue columns = { 'log': acl.action in actions_enabled, - 'meter': self.meter_name, + 'meter': meter_name, 'name': log_name, 'severity': "info" } @@ -183,12 +196,13 @@ def _update_log_objs(self, context, ovn_txn, log_objs): for log_obj in log_objs: pgs = self._pgs_from_log_obj(context, log_obj) actions_enabled = self._acl_actions_enabled(log_obj) - self._set_acls_log(pgs, ovn_txn, actions_enabled, + self._set_acls_log(pgs, context, ovn_txn, actions_enabled, utils.ovn_name(log_obj.id)) def _pgs_all(self): return self.ovn_nb.db_list( - "Port_Group", columns=["name", "acls"]).execute(check_error=True) + "Port_Group", + columns=["name", "external_ids", "acls"]).execute(check_error=True) def _pgs_from_log_obj(self, context, log_obj): """Map Neutron log_obj into affected port groups in OVN. @@ -207,11 +221,13 @@ def _pgs_from_log_obj(self, context, log_obj): # No sg, no port, DROP: return DROP pg if log_obj.event == log_const.DROP_EVENT: return [{"name": pg_drop.name, - "acls": [r.uuid for r in pg_drop.acls]}] + "external_ids": pg_drop.external_ids, + "acls": [r.uuid for r in pg_drop.acls]}] # No sg, no port, ACCEPT: return all except DROP pg pgs = self._pgs_all() pgs.remove({"name": pg_drop.name, - "acls": [r.uuid for r in pg_drop.acls]}) + "external_ids": pg_drop.external_ids, + "acls": [r.uuid for r in pg_drop.acls]}) return pgs except idlutils.RowNotFound: pass @@ -223,6 +239,7 @@ def _pgs_from_log_obj(self, context, log_obj): pg = self.ovn_nb.lookup("Port_Group", ovn_const.OVN_DROP_PORT_GROUP_NAME) pgs.append({"name": pg.name, + "external_ids": pg.external_ids, "acls": [r.uuid for r in pg.acls]}) except idlutils.RowNotFound: pass @@ -235,6 +252,7 @@ def _pgs_from_log_obj(self, context, log_obj): utils.ovn_port_group_name( log_obj.resource_id)) pgs.append({"name": pg.name, + "external_ids": pg.external_ids, "acls": [r.uuid for r in pg.acls]}) except idlutils.RowNotFound: pass @@ -248,6 +266,7 @@ def _pgs_from_log_obj(self, context, log_obj): pg = self.ovn_nb.lookup("Port_Group", utils.ovn_port_group_name(sg_id)) pgs.append({"name": pg.name, + "external_ids": pg.external_ids, "acls": [r.uuid for r in pg.acls]}) except idlutils.RowNotFound: pass @@ -266,7 +285,7 @@ def create_log(self, context, log_obj): with self.ovn_nb.transaction(check_error=True) as ovn_txn: self._ovn_client.create_ovn_fair_meter(self.meter_name, txn=ovn_txn) - self._set_acls_log(pgs, ovn_txn, actions_enabled, + self._set_acls_log(pgs, context, ovn_txn, actions_enabled, utils.ovn_name(log_obj.id)) def create_log_precommit(self, context, log_obj): @@ -334,7 +353,7 @@ def update_log(self, context, log_obj): if not self._unset_disabled_acls(context, log_obj, ovn_txn): pgs = self._pgs_from_log_obj(context, log_obj) actions_enabled = self._acl_actions_enabled(log_obj) - self._set_acls_log(pgs, ovn_txn, actions_enabled, + self._set_acls_log(pgs, context, ovn_txn, actions_enabled, utils.ovn_name(log_obj.id)) def delete_log(self, context, log_obj): @@ -356,6 +375,8 @@ def delete_log(self, context, log_obj): self._remove_acls_log(pgs, ovn_txn) ovn_txn.add(self.ovn_nb.meter_del(self.meter_name, if_exists=True)) + ovn_txn.add(self.ovn_nb.meter_del( + self.meter_name + "_stateless", if_exists=True)) LOG.info("All ACL logs cleared after deletion of log_obj %s", log_obj.id) return diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 26afa1fbf40..529295f07e9 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -1125,10 +1125,9 @@ def test_check_for_logging_conf_change(self): # Check a meter and fair meter exist self.assertTrue(self.nb_api._tables['Meter'].rows) self.assertTrue(self.nb_api._tables['Meter_Band'].rows) - self.assertEqual(cfg.CONF.network_log.burst_limit, - [*self.nb_api._tables['Meter_Band'].rows.values()][0].burst_size) - self.assertEqual(cfg.CONF.network_log.rate_limit, - [*self.nb_api._tables['Meter_Band'].rows.values()][0].rate) + self.assertEqual(len([*self.nb_api._tables['Meter'].rows.values()]), + len([*self.nb_api._tables['Meter_Band'].rows.values()])) + self._check_meters_consistency() # Update burst and rate limit values on the configuration ovn_config.cfg.CONF.set_override('burst_limit', CFG_NEW_BURST, group='network_log') @@ -1138,7 +1137,16 @@ def test_check_for_logging_conf_change(self): self.assertRaises(periodics.NeverAgain, self.maint.check_fair_meter_consistency) # Check meter band was effectively changed after the maintenance call - self.assertEqual(CFG_NEW_BURST, - [*self.nb_api._tables['Meter_Band'].rows.values()][0].burst_size) - self.assertEqual(CFG_NEW_RATE, - [*self.nb_api._tables['Meter_Band'].rows.values()][0].rate) + self._check_meters_consistency(CFG_NEW_BURST, CFG_NEW_RATE) + + def _check_meters_consistency(self, new_burst=None, new_rate=None): + burst, rate = (new_burst, new_rate) if new_burst else ( + cfg.CONF.network_log.burst_limit, cfg.CONF.network_log.rate_limit) + for meter in [*self.nb_api._tables['Meter'].rows.values()]: + meter_band = self.nb_api.lookup('Meter_Band', meter.bands[0].uuid) + if "_stateless" in meter.name: + self.assertEqual(int(burst / 2), meter_band.burst_size) + self.assertEqual(int(rate / 2), meter_band.rate) + else: + self.assertEqual(burst, meter_band.burst_size) + self.assertEqual(rate, meter_band.rate) diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index a0d46ca2b99..71bdf69c0a9 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -577,3 +577,45 @@ def test_ovsdb_probe_interval(self): interval = ovn_conf.get_ovn_ovsdb_probe_interval() for idl in idls: self.assertEqual(interval, idl._session.reconnect.probe_interval) + + +class TestPortBindingChassisEvent(base.TestOVNFunctionalBase, + test_l3.L3NatTestCaseMixin): + + def setUp(self, **kwargs): + super().setUp(**kwargs) + self.chassis = self.add_fake_chassis('ovs-host1') + self.l3_plugin = directory.get_plugin(plugin_constants.L3) + kwargs = {'arg_list': (external_net.EXTERNAL,), + external_net.EXTERNAL: True} + self.net = self._make_network( + self.fmt, 'ext_net', True, as_admin=True, **kwargs) + self._make_subnet(self.fmt, self.net, '20.0.10.1', '20.0.10.0/24') + port_res = self._create_port(self.fmt, self.net['network']['id']) + self.port = self.deserialize(self.fmt, port_res)['port'] + + self.ext_api = test_extensions.setup_extensions_middleware( + test_l3.L3TestExtensionManager()) + self.pb_event_match = mock.patch.object( + self.sb_api.idl._portbinding_event, 'match_fn').start() + + def _check_pb_type(self, _type): + def check_pb_type(_type): + if len(self.pb_event_match.call_args_list) < 1: + return False + + pb_row = self.pb_event_match.call_args_list[0].args[1] + return _type == pb_row.type + + n_utils.wait_until_true(lambda: check_pb_type(_type), timeout=5) + + def test_pb_type_patch(self): + router = self._make_router(self.fmt, self._tenant_id) + self._add_external_gateway_to_router(router['router']['id'], + self.net['network']['id']) + self._check_pb_type('patch') + + def test_pb_type_empty(self): + self.sb_api.lsp_bind(self.port['id'], self.chassis, + may_exist=True).execute(check_error=True) + self._check_pb_type('') diff --git a/neutron/tests/functional/services/logapi/drivers/ovn/test_driver.py b/neutron/tests/functional/services/logapi/drivers/ovn/test_driver.py index 57c7978b8fc..91fa727a809 100644 --- a/neutron/tests/functional/services/logapi/drivers/ovn/test_driver.py +++ b/neutron/tests/functional/services/logapi/drivers/ovn/test_driver.py @@ -30,6 +30,12 @@ def setUp(self): self._check_is_supported() self.ctxt = context.Context('admin', 'fake_tenant') + # Since these tests use the _create_network() from the unit test suite + # but _create_security_group() is from the functional tests, two + # different tenant_ids will be used unless we specify the following + # line in the code: + self._tenant_id = self.ctxt.project_id + def _check_is_supported(self): if not self.log_driver.network_logging_supported(self.nb_api): self.skipTest("The current OVN version does not offer support " diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py index b91460246f2..1ec8ffbc60d 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py @@ -247,7 +247,16 @@ def test_create_ovn_fair_meter(self): self.ovn_client.create_ovn_fair_meter(self._log_driver.meter_name) self.assertFalse(self.nb_idl.meter_del.called) self.assertTrue(self.nb_idl.meter_add.called) - self.nb_idl.meter_add.assert_called_once_with( + self.nb_idl.meter_add.assert_any_call( + name=self._log_driver.meter_name + "_stateless", + unit="pktps", + rate=int(self.fake_cfg_network_log.rate_limit / 2), + fair=True, + burst_size=int(self.fake_cfg_network_log.burst_limit / 2), + may_exist=False, + external_ids={constants.OVN_DEVICE_OWNER_EXT_ID_KEY: + log_const.LOGGING_PLUGIN}) + self.nb_idl.meter_add.assert_any_call( name=self._log_driver.meter_name, unit="pktps", rate=self.fake_cfg_network_log.rate_limit, @@ -259,10 +268,17 @@ def test_create_ovn_fair_meter(self): def test_create_ovn_fair_meter_unchanged(self): mock_find_rows = mock.Mock() - mock_find_rows.execute.return_value = [self._fake_meter()] + fake_meter1 = [self._fake_meter()] + fake_meter2 = [self._fake_meter( + name=self._log_driver.meter_name + "_stateless", + bands=[mock.Mock(uuid='tb_stateless')])] + mock_find_rows.execute.side_effect = [fake_meter1, fake_meter1, + fake_meter2, fake_meter2] self.nb_idl.db_find_rows.return_value = mock_find_rows self.nb_idl.lookup.side_effect = lambda table, key, default: ( - self._fake_meter_band() if key == "test_band" else default) + self._fake_meter_band() if key == "test_band" else + self._fake_meter_band_stateless() if key == "tb_stateless" else + default) self.ovn_client.create_ovn_fair_meter(self._log_driver.meter_name) self.assertFalse(self.nb_idl.meter_del.called) self.assertFalse(self.nb_idl.meter_add.called) diff --git a/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py b/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py index db8e2c17731..8399f244e09 100644 --- a/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py +++ b/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py @@ -21,6 +21,7 @@ from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils as ovn_utils +from neutron.objects import securitygroup as sg_obj from neutron.services.logapi.drivers.ovn import driver as ovn_driver from neutron.tests import base from neutron.tests.unit import fake_resources @@ -92,6 +93,15 @@ def _fake_meter_band(self, **kwargs): meter_band_obj_dict = {**meter_band_defaults_dict, **kwargs} return mock.Mock(**meter_band_obj_dict) + def _fake_meter_band_stateless(self, **kwargs): + meter_band_defaults_dict = { + 'uuid': 'tb_stateless', + 'rate': int(self.fake_cfg_network_log.rate_limit / 2), + 'burst_size': int(self.fake_cfg_network_log.burst_limit / 2), + } + meter_band_obj_dict = {**meter_band_defaults_dict, **kwargs} + return mock.Mock(**meter_band_obj_dict) + class TestOVNDriver(TestOVNDriverBase): def test_create(self): @@ -119,18 +129,16 @@ def __init__(self, name=None, **acl_dict): self.__dict__ = {**acl_defaults_dict, **acl_dict} def _fake_pg_dict(self, **kwargs): + uuid = uuidutils.generate_uuid() pg_defaults_dict = { - "name": ovn_utils.ovn_port_group_name(uuidutils.generate_uuid()), + "name": ovn_utils.ovn_port_group_name(uuid), + "external_ids": {ovn_const.OVN_SG_EXT_ID_KEY: uuid}, "acls": [] } return {**pg_defaults_dict, **kwargs} def _fake_pg(self, **kwargs): - pg_defaults_dict = { - "name": ovn_utils.ovn_port_group_name(uuidutils.generate_uuid()), - "acls": [] - } - pg_dict = {**pg_defaults_dict, **kwargs} + pg_dict = self._fake_pg_dict(**kwargs) return mock.Mock(**pg_dict) def _fake_log_obj(self, **kwargs): @@ -180,7 +188,9 @@ def _mock_lookup(_pg_table, pg_name): pgs = self._log_driver._pgs_from_log_obj(self.context, log_obj) mock_pgs_all.assert_not_called() self.assertEqual(2, self._nb_ovn.lookup.call_count) - self.assertEqual([{'acls': [], 'name': pg.name}], pgs) + self.assertEqual([{'acls': [], + 'external_ids': pg.external_ids, + 'name': pg.name}], pgs) def test__pgs_from_log_obj_pg(self): with mock.patch.object(self._log_driver, '_pgs_all', @@ -194,7 +204,9 @@ def test__pgs_from_log_obj_pg(self): mock_pgs_all.assert_not_called() self._nb_ovn.lookup.assert_called_once_with( "Port_Group", ovn_utils.ovn_port_group_name('resource_id')) - self.assertEqual([{'acls': [], 'name': pg.name}], pgs) + self.assertEqual([{'acls': [], + 'external_ids': pg.external_ids, + 'name': pg.name}], pgs) def test__pgs_from_log_obj_port(self): with mock.patch.object(self._log_driver, '_pgs_all', @@ -211,7 +223,9 @@ def test__pgs_from_log_obj_port(self): self._nb_ovn.lookup.assert_called_once_with("Port_Group", pg_name) self.fake_get_sgs_attached_to_port.assert_called_once_with( self.context, 'target_id') - self.assertEqual([{'acls': [], 'name': pg.name}], pgs) + self.assertEqual([{'acls': [], + 'external_ids': pg.external_ids, + 'name': pg.name}], pgs) @mock.patch.object(ovn_driver.LOG, 'info') def test__remove_acls_log(self, m_info): @@ -287,7 +301,8 @@ def _mock_lookup(_pg_table, acl_uuid, default): self._nb_ovn.db_set.call_count) @mock.patch.object(ovn_driver.LOG, 'info') - def test__set_acls_log(self, m_info): + @mock.patch.object(sg_obj.SecurityGroup, 'get_sg_by_id') + def test__set_acls_log(self, get_sg, m_info): pg_dict = self._fake_pg_dict(acls=['acl1', 'acl2', 'acl3', 'acl4']) log_name = 'test_obj_name' used_name = 'test_used_name' @@ -297,10 +312,14 @@ def _mock_lookup(_pg_table, acl_uuid): return self._fake_acl() return self._fake_acl(name=used_name) + sg = fake_resources.FakeSecurityGroup.create_one_security_group( + attrs={'stateful': True}) + get_sg.return_value = sg self._nb_ovn.lookup.side_effect = _mock_lookup actions_enabled = self._log_driver._acl_actions_enabled( self._fake_log_obj(event=log_const.ALL_EVENT)) - self._log_driver._set_acls_log([pg_dict], self._nb_ovn.transaction, + self._log_driver._set_acls_log([pg_dict], self.context, + self._nb_ovn.transaction, actions_enabled, log_name) info_args, _info_kwargs = m_info.call_args_list[0] self.assertIn('Set %d (out of %d visited) ACLs for network log %s',