From 93060ae09950e8a84f8708d177e7e22a7da8e047 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 13 Sep 2018 01:39:44 +0000 Subject: [PATCH 1/7] Disable FDB learning on all bridge ports before freezing orchagent Signed-off-by: Qi Luo --- orchagent/orchdaemon.cpp | 7 +++++++ orchagent/portsorch.cpp | 21 +++++++++++++++++++++ orchagent/portsorch.h | 1 + 3 files changed, 29 insertions(+) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 0606996dfa..9b2f3a1aca 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -363,6 +363,13 @@ void OrchDaemon::start() // Should sleep here or continue handling timers and etc.?? if (!gSwitchOrch->checkRestartNoFreeze()) { + // Disable FDB learning on all bridge ports + for (auto& pair: gPortsOrch->getAllPorts()) + { + auto& port = pair.second; + gPortsOrch->setBridgePortLearningFDB(port, false); + } + SWSS_LOG_WARN("Orchagent is frozen for warm restart!"); sleep(UINT_MAX); } diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 864f28f52c..e074f1a41b 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2312,6 +2312,27 @@ bool PortsOrch::addHostIntfs(Port &port, string alias, sai_object_id_t &host_int return true; } +bool PortsOrch::setBridgePortLearningFDB(Port &port, bool enable) +{ + // TODO: how to support 1D bridge? + if (port.m_type != Port::PHY) return false; + + auto bridge_port_id = port.m_bridge_port_id; + if (bridge_port_id == SAI_NULL_OBJECT_ID) return false; + + sai_attribute_t bport_attr; + bport_attr.id = SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE; + bport_attr.value.s32 = enable ? SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW : SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE; + auto status = sai_bridge_api->set_bridge_port_attribute(bridge_port_id, &bport_attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set bridge port %lx learning_mode attribute: %d", bridge_port_id, status); + return false; + } + SWSS_LOG_NOTICE("Disable FDB learning on bridge port %s(%lx)", port.m_alias.c_str(), bridge_port_id); + return true; +} + bool PortsOrch::addBridgePort(Port &port) { SWSS_LOG_ENTER(); diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 1a012f8cba..408f459be9 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -59,6 +59,7 @@ class PortsOrch : public Orch, public Subject bool bake() override; void cleanPortTable(const vector& keys); bool getBridgePort(sai_object_id_t id, Port &port); + bool setBridgePortLearningFDB(Port &port, bool enable); bool getPort(string alias, Port &port); bool getPort(sai_object_id_t id, Port &port); bool getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port); From 7ba0ffc864d5a17a658b6050669e152e5357fc01 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Mon, 24 Sep 2018 21:04:16 +0000 Subject: [PATCH 2/7] Refine create_bridge_port attr --- orchagent/portsorch.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index e074f1a41b..4806fa8ffe 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2371,6 +2371,11 @@ bool PortsOrch::addBridgePort(Port &port) attr.value.booldata = true; attrs.push_back(attr); + /* And with hardware FDB learning mode set to HW (explicit default value) */ + attr.id = SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE; + attr.value.s32 = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW; + attrs.push_back(attr); + sai_status_t status = sai_bridge_api->create_bridge_port(&port.m_bridge_port_id, gSwitchId, (uint32_t)attrs.size(), attrs.data()); if (status != SAI_STATUS_SUCCESS) { From d1d9595ae5976384bb49e12ab2ea2e64390218b3 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Tue, 25 Sep 2018 05:07:17 +0000 Subject: [PATCH 3/7] Flush before freeze Signed-off-by: Qi Luo --- orchagent/orchdaemon.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 9b2f3a1aca..75ba9fa4a9 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -369,6 +369,7 @@ void OrchDaemon::start() auto& port = pair.second; gPortsOrch->setBridgePortLearningFDB(port, false); } + flush(); SWSS_LOG_WARN("Orchagent is frozen for warm restart!"); sleep(UINT_MAX); From 8957d794b2efad2f4e7034a06b9081ce1cb06830 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Mon, 17 Sep 2018 23:18:26 +0000 Subject: [PATCH 4/7] Add vs test Signed-off-by: Qi Luo --- tests/conftest.py | 32 ++++++++++------- tests/test_fdb_cold.py | 17 +-------- tests/test_fdb_warm.py | 82 +++++++++++++++++++++++------------------- 3 files changed, 66 insertions(+), 65 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 2fae380ca7..2b39e528fb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -269,15 +269,28 @@ def copy_file(self, path, filename): self.ctn.put_archive(path, tarstr.getvalue()) tarstr.close() + def get_map_iface_bridge_port_id(self, asic_db): + port_id_2_iface = self.asicdb.portoidmap + tbl = swsscommon.Table(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT") + iface_2_bridge_port_id = {} + for key in tbl.getKeys(): + status, data = tbl.get(key) + assert status + values = dict(data) + iface_id = values["SAI_BRIDGE_PORT_ATTR_PORT_ID"] + iface_name = port_id_2_iface[iface_id] + iface_2_bridge_port_id[iface_name] = key + + return iface_2_bridge_port_id + def is_table_entry_exists(self, db, table, keyregex, attributes): tbl = swsscommon.Table(db, table) keys = tbl.getKeys() - exists = False extra_info = [] - key_found = False for key in keys: - key_found = re.match(keyregex, key) + if re.match(keyregex, key) is None: + continue status, fvs = tbl.get(key) assert status, "Error reading from table %s" % table @@ -288,17 +301,12 @@ def is_table_entry_exists(self, db, table, keyregex, attributes): del d_attributes[k] if len(d_attributes) != 0: - exists = False extra_info.append("Desired attributes %s was not found for key %s" % (str(d_attributes), key)) else: - exists = True - break - - if not key_found: - exists = False - extra_info.append("Desired key with parameters %s was not found" % str(key_values)) - - return exists, extra_info + return True, extra_info + else: + extra_info.append("Desired key regex %s was not found" % str(keyregex)) + return False, extra_info def is_fdb_entry_exists(self, db, table, key_values, attributes): tbl = swsscommon.Table(db, table) diff --git a/tests/test_fdb_cold.py b/tests/test_fdb_cold.py index 680dd65afc..d0556a2e85 100644 --- a/tests/test_fdb_cold.py +++ b/tests/test_fdb_cold.py @@ -20,21 +20,6 @@ def create_entry_pst(db, table, key, pairs): tbl = swsscommon.ProducerStateTable(db, table) create_entry(tbl, key, pairs) - -def get_map_iface_bridge_port_id(asic_db, dvs): - port_id_2_iface = dvs.asicdb.portoidmap - tbl = swsscommon.Table(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT") - iface_2_bridge_port_id = {} - for key in tbl.getKeys(): - status, data = tbl.get(key) - assert status - values = dict(data) - iface_id = values["SAI_BRIDGE_PORT_ATTR_PORT_ID"] - iface_name = port_id_2_iface[iface_id] - iface_2_bridge_port_id[iface_name] = key - - return iface_2_bridge_port_id - def how_many_entries_exist(db, table): tbl = swsscommon.Table(db, table) return len(tbl.getKeys()) @@ -90,7 +75,7 @@ def test_FDBAddedAfterMemberCreated(dvs): assert vm_after - vm_before == 1, "The vlan member wasn't added" # Get mapping between interface name and its bridge port_id - iface_2_bridge_port_id = get_map_iface_bridge_port_id(dvs.adb, dvs) + iface_2_bridge_port_id = dvs.get_map_iface_bridge_port_id(dvs.adb) # check that the FDB entry was inserted into ASIC DB assert how_many_entries_exist(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY") == 1, "The fdb entry wasn't inserted to ASIC" diff --git a/tests/test_fdb_warm.py b/tests/test_fdb_warm.py index 2e4810671e..3a6050434f 100644 --- a/tests/test_fdb_warm.py +++ b/tests/test_fdb_warm.py @@ -20,21 +20,6 @@ def create_entry_pst(db, table, key, pairs): tbl = swsscommon.ProducerStateTable(db, table) create_entry(tbl, key, pairs) - -def get_map_iface_bridge_port_id(asic_db, dvs): - port_id_2_iface = dvs.asicdb.portoidmap - tbl = swsscommon.Table(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT") - iface_2_bridge_port_id = {} - for key in tbl.getKeys(): - status, data = tbl.get(key) - assert status - values = dict(data) - iface_id = values["SAI_BRIDGE_PORT_ATTR_PORT_ID"] - iface_name = port_id_2_iface[iface_id] - iface_2_bridge_port_id[iface_name] = key - - return iface_2_bridge_port_id - def how_many_entries_exist(db, table): tbl = swsscommon.Table(db, table) return len(tbl.getKeys()) @@ -42,7 +27,7 @@ def how_many_entries_exist(db, table): def test_fdb_notifications(dvs): dvs.setup_db() - #dvs.runcmd("sonic-clear fdb all") + dvs.runcmd("sonic-clear fdb all") dvs.runcmd("crm config polling interval 1") dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'SAI_SWITCH_ATTR_AVAILABLE_FDB_ENTRY', '1000') @@ -55,6 +40,20 @@ def test_fdb_notifications(dvs): dvs.create_vlan_member("6", "Ethernet64") dvs.create_vlan_member("6", "Ethernet68") + # Get mapping between interface name and its bridge port_id + time.sleep(2) + iface_2_bridge_port_id = dvs.get_map_iface_bridge_port_id(dvs.adb) + + # check FDB learning mode + ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT', + iface_2_bridge_port_id["Ethernet64"], + [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW")]) + assert ok, str(extra) + ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT', + iface_2_bridge_port_id["Ethernet68"], + [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW")]) + assert ok, str(extra) + # bring up vlan and member dvs.set_interface_status("Vlan6", "up") dvs.add_ip_address("Vlan6", "6.6.6.1/24") @@ -72,9 +71,6 @@ def test_fdb_notifications(dvs): rc = dvs.servers[17].runcmd("ping -c 1 6.6.6.6") assert rc == 0 - # Get mapping between interface name and its bridge port_id - time.sleep(2) - iface_2_bridge_port_id = get_map_iface_bridge_port_id(dvs.adb, dvs) # check that the FDB entries were inserted into ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", @@ -113,24 +109,33 @@ def test_fdb_notifications(dvs): assert ok, str(extra) # enable warm restart - # TODO: use cfg command to config it - create_entry_tbl( - dvs.cdb, - swsscommon.CFG_WARM_RESTART_TABLE_NAME, "swss", - [ - ("enable", "true"), - ] - ) + dvs.runcmd("config warm_restart enable swss") + + # freeze orchagent for warm restart + (exitcode, result) = dvs.runcmd("/usr/bin/orchagent_restart_check") + assert result == "RESTARTCHECK succeeded\n" + time.sleep(2) try: # restart orchagent dvs.stop_swss() + + # check FDB learning mode + ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT', + iface_2_bridge_port_id["Ethernet64"], + [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE")]) + assert ok, str(extra) + ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT', + iface_2_bridge_port_id["Ethernet68"], + [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE")]) + assert ok, str(extra) + dvs.start_swss() time.sleep(2) # Get mapping between interface name and its bridge port_id # Note: they are changed - iface_2_bridge_port_id = get_map_iface_bridge_port_id(dvs.adb, dvs) + iface_2_bridge_port_id = dvs.get_map_iface_bridge_port_id(dvs.adb) # check that the FDB entries were inserted into ASIC DB ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY", @@ -148,17 +153,20 @@ def test_fdb_notifications(dvs): ) assert ok, str(extra) + # check FDB learning mode + ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT', + iface_2_bridge_port_id["Ethernet64"], + [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW")]) + assert ok, str(extra) + ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT', + iface_2_bridge_port_id["Ethernet68"], + [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW")]) + assert ok, str(extra) + time.sleep(2) counter_restarted = dvs.getCrmCounterValue('STATS', 'crm_stats_fdb_entry_used') assert counter_inserted == counter_restarted finally: - # disable warm restart - # TODO: use cfg command to config it - create_entry_tbl( - dvs.cdb, - swsscommon.CFG_WARM_RESTART_TABLE_NAME, "swss", - [ - ("enable", "false"), - ] - ) + # enable warm restart + dvs.runcmd("config warm_restart enable swss") From 758c4acd60d360976314377e00bc59aa895674ab Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Tue, 25 Sep 2018 18:56:14 +0000 Subject: [PATCH 5/7] Fix error message Signed-off-by: Qi Luo --- tests/conftest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 2b39e528fb..82e63cdb63 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -305,7 +305,8 @@ def is_table_entry_exists(self, db, table, keyregex, attributes): else: return True, extra_info else: - extra_info.append("Desired key regex %s was not found" % str(keyregex)) + if not extra_info: + extra_info.append("Desired key regex %s was not found" % str(keyregex)) return False, extra_info def is_fdb_entry_exists(self, db, table, key_values, attributes): From f484a2bef2a7e68746987537129bc7645a6e2ec7 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Tue, 25 Sep 2018 19:18:32 +0000 Subject: [PATCH 6/7] Generalize setBridgePortLearningFDB() Signed-off-by: Qi Luo --- orchagent/orchdaemon.cpp | 2 +- orchagent/portsorch.cpp | 4 ++-- orchagent/portsorch.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 75ba9fa4a9..181f480adc 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -367,7 +367,7 @@ void OrchDaemon::start() for (auto& pair: gPortsOrch->getAllPorts()) { auto& port = pair.second; - gPortsOrch->setBridgePortLearningFDB(port, false); + gPortsOrch->setBridgePortLearningFDB(port, SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE); } flush(); diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 4806fa8ffe..9e5a5b9272 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2312,7 +2312,7 @@ bool PortsOrch::addHostIntfs(Port &port, string alias, sai_object_id_t &host_int return true; } -bool PortsOrch::setBridgePortLearningFDB(Port &port, bool enable) +bool PortsOrch::setBridgePortLearningFDB(Port &port, sai_bridge_port_fdb_learning_mode_t mode) { // TODO: how to support 1D bridge? if (port.m_type != Port::PHY) return false; @@ -2322,7 +2322,7 @@ bool PortsOrch::setBridgePortLearningFDB(Port &port, bool enable) sai_attribute_t bport_attr; bport_attr.id = SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE; - bport_attr.value.s32 = enable ? SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW : SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE; + bport_attr.value.s32 = mode; auto status = sai_bridge_api->set_bridge_port_attribute(bridge_port_id, &bport_attr); if (status != SAI_STATUS_SUCCESS) { diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 408f459be9..93a92752ae 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -59,7 +59,7 @@ class PortsOrch : public Orch, public Subject bool bake() override; void cleanPortTable(const vector& keys); bool getBridgePort(sai_object_id_t id, Port &port); - bool setBridgePortLearningFDB(Port &port, bool enable); + bool setBridgePortLearningFDB(Port &port, sai_bridge_port_fdb_learning_mode_t mode); bool getPort(string alias, Port &port); bool getPort(sai_object_id_t id, Port &port); bool getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port); From d6e9f994f53547c26a56ff3bfaa1c9921d3c12d7 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Tue, 25 Sep 2018 19:20:19 +0000 Subject: [PATCH 7/7] Tune test Signed-off-by: Qi Luo --- tests/test_fdb_warm.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_fdb_warm.py b/tests/test_fdb_warm.py index 3a6050434f..d56bc9954e 100644 --- a/tests/test_fdb_warm.py +++ b/tests/test_fdb_warm.py @@ -109,10 +109,11 @@ def test_fdb_notifications(dvs): assert ok, str(extra) # enable warm restart - dvs.runcmd("config warm_restart enable swss") + (exitcode, result) = dvs.runcmd("config warm_restart enable swss") + assert exitcode == 0 # freeze orchagent for warm restart - (exitcode, result) = dvs.runcmd("/usr/bin/orchagent_restart_check") + (exitcode, result) = dvs.runcmd("/usr/bin/orchagent_restart_check") assert result == "RESTARTCHECK succeeded\n" time.sleep(2) @@ -170,3 +171,5 @@ def test_fdb_notifications(dvs): finally: # enable warm restart dvs.runcmd("config warm_restart enable swss") + # slow down crm polling + dvs.runcmd("crm config polling interval 10000")