From e4c92f377dbeb981680750cd7a523c26635f91f4 Mon Sep 17 00:00:00 2001 From: "shine.chen" Date: Thu, 7 Mar 2019 22:45:57 -0800 Subject: [PATCH 1/6] add support to set mac-address learning attribute on bridge-port Signed-off-by: shine.chen --- orchagent/portsorch.cpp | 158 +++++++++++++++++++++++++++++++++++++++- orchagent/portsorch.h | 5 +- 2 files changed, 160 insertions(+), 3 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index e479092b80..df8fe4920b 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1482,6 +1482,7 @@ void PortsOrch::doPortTask(Consumer &consumer) string pfc_asym; uint32_t mtu = 0; uint32_t speed = 0; + string learn_mode; int an = -1; for (auto i : kfvFieldsValues(t)) @@ -1524,6 +1525,12 @@ void PortsOrch::doPortTask(Consumer &consumer) fec_mode = fvValue(i); } + /* Get port fdb learn mode*/ + if (fvField(i) == "learn_mode") + { + learn_mode = fvValue(i); + } + /* Set port asymmetric PFC */ if (fvField(i) == "pfc_asym") pfc_asym = fvValue(i); @@ -1813,7 +1820,48 @@ void PortsOrch::doPortTask(Consumer &consumer) SWSS_LOG_ERROR("Unknown fec mode %s", fec_mode.c_str()); } } + + if (learn_mode != "") + { + if (learn_mode == "enabled" || learn_mode == "disabled") + { + int32_t mode; + if (learn_mode == "enabled") + { + mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW; + } + else + { + mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE; + } + + if (p.m_bridge_port_id != SAI_NULL_OBJECT_ID) + { + if(setBridgePortLearnMode(p, mode)) + { + SWSS_LOG_NOTICE("Set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); + } + else + { + SWSS_LOG_ERROR("Failed to set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); + it++; + continue; + } + } + else + { + m_portSetLearnModeList[alias] = learn_mode; + + SWSS_LOG_NOTICE("Saved to set port %s learn mode %s", alias.c_str(), learn_mode.c_str()); + } + } + else + { + SWSS_LOG_ERROR("Unknown learn mode %s", learn_mode.c_str()); + } + } + if (pfc_asym != "") { if (setPortPfcAsym(p, pfc_asym)) @@ -2013,7 +2061,38 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer) } if (addBridgePort(port) && addVlanMember(vlan, port, tagging_mode)) + { + int32_t mode; + string learning_mode; + + if(m_portSetLearnModeList.find(port_alias) != m_portSetLearnModeList.end()) + { + learning_mode = m_portSetLearnModeList[port_alias]; + + SWSS_LOG_NOTICE("Set port %s learn mode %s when add port to vlan %s", port_alias.c_str(), learning_mode.c_str(), vlan_alias.c_str()); + + if (learning_mode == "enabled") + { + mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW; + } + else if (learning_mode == "disabled") + { + mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE; + } + else + { + SWSS_LOG_ERROR("Wrong learning_mode '%s' ", learning_mode.c_str()); + it = consumer.m_toSync.erase(it); + continue; + } + + setBridgePortLearnMode(port, mode); + + m_portSetLearnModeList.erase(port_alias); + } + it = consumer.m_toSync.erase(it); + } else it++; } @@ -2097,6 +2176,57 @@ void PortsOrch::doLagTask(Consumer &consumer) gIntfsOrch->setRouterIntfsMtu(l); } } + + string learn_mode; + for (auto i : kfvFieldsValues(t)) + { + /* Get port fdb learn mode*/ + if (fvField(i) == "learn_mode") + { + learn_mode = fvValue(i); + } + } + + if (learn_mode != "") + { + if (learn_mode == "enabled" || learn_mode == "disabled") + { + int32_t mode; + + if (learn_mode == "enabled") + { + mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW; + } + else + { + mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE; + } + + if (l.m_bridge_port_id != SAI_NULL_OBJECT_ID) + { + if(setBridgePortLearnMode(l, mode)) + { + SWSS_LOG_NOTICE("Set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); + } + else + { + SWSS_LOG_ERROR("Failed to set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); + it++; + continue; + } + } + else + { + m_portSetLearnModeList[alias] = learn_mode; + + SWSS_LOG_NOTICE("Saved to set port %s learn mode %s", alias.c_str(), learn_mode.c_str()); + } + } + else + { + SWSS_LOG_ERROR("Unknown learn mode %s", learn_mode.c_str()); + } + } } it = consumer.m_toSync.erase(it); @@ -2568,6 +2698,32 @@ bool PortsOrch::removeBridgePort(Port &port) return true; } +bool PortsOrch::setBridgePortLearnMode(Port &port, int32_t mode) +{ + SWSS_LOG_ENTER(); + + if (port.m_bridge_port_id == SAI_NULL_OBJECT_ID) + { + return true; + } + /* Set bridge port learning mode */ + sai_attribute_t attr; + attr.id = SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE; + attr.value.s32 = mode; + + sai_status_t status = sai_bridge_api->set_bridge_port_attribute(port.m_bridge_port_id, &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set bridge port %s learning mode, rv:%d", + port.m_alias.c_str(), status); + return false; + } + + SWSS_LOG_NOTICE("Set bridge port %s learning mode", port.m_alias.c_str()); + + return true; +} + bool PortsOrch::addVlan(string vlan_alias) { SWSS_LOG_ENTER(); @@ -2767,7 +2923,7 @@ bool PortsOrch::addLag(string lag_alias) PortUpdate update = { lag, true }; notify(SUBJECT_TYPE_PORT_CHANGE, static_cast(&update)); - + return true; } diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index fd2d763636..7756ed40dc 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -117,9 +117,9 @@ class PortsOrch : public Orch, public Subject map, sai_object_id_t> m_portListLaneMap; map, tuple> m_lanesAliasSpeedMap; map m_portList; - unordered_set m_pendingPortSet; - + map m_portSetLearnModeList; + NotificationConsumer* m_portStatusNotificationConsumer; void doTask(Consumer &consumer); @@ -143,6 +143,7 @@ class PortsOrch : public Orch, public Subject bool addBridgePort(Port &port); bool removeBridgePort(Port &port); + bool setBridgePortLearnMode(Port &port, int32_t mode); bool addVlan(string vlan); bool removeVlan(Port vlan); From 8bf19056323890ce0f14d87d69581847889c9bee Mon Sep 17 00:00:00 2001 From: "leo.li" Date: Wed, 12 Jun 2019 04:18:37 -0700 Subject: [PATCH 2/6] add the corresponding vs-test Signed-off-by: leo.li --- tests/test_port_mac_learn.py | 172 +++++++++++++++++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 tests/test_port_mac_learn.py diff --git a/tests/test_port_mac_learn.py b/tests/test_port_mac_learn.py new file mode 100644 index 0000000000..833f37d6ef --- /dev/null +++ b/tests/test_port_mac_learn.py @@ -0,0 +1,172 @@ +from swsscommon import swsscommon + +import time +import os + +class TestPortMacLearn(object): + def setup_db(self, dvs): + self.pdb = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0) + self.adb = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0) + self.cdb = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) + self.cntdb = swsscommon.DBConnector(swsscommon.COUNTERS_DB, dvs.redis_sock, 0) + + def get_port_oid(self, port_name): + port_map_tbl = swsscommon.Table(self.cntdb, 'COUNTERS_PORT_NAME_MAP') + for k in port_map_tbl.get('')[1]: + if k[0] == port_name: + return k[1] + return None + + def get_bridge_port_oid(self, port_oid): + tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT") + for key in tbl.getKeys(): + status, data = tbl.get(key) + assert status + values = dict(data) + if port_oid == values["SAI_BRIDGE_PORT_ATTR_PORT_ID"]: + return key + return None + + def check_learn_mode_in_appdb(self, table, interface, learn_mode): + (status, fvs) = table.get(interface) + assert status == True + for fv in fvs: + if fv[0] == "learn_mode": + if fv[1] == learn_mode: + return True + return False + + def check_learn_mode_in_asicdb(self, interface_oid, learn_mode): + # Get bridge port oid + bridge_port_oid = self.get_bridge_port_oid(interface_oid) + assert bridge_port_oid is not None + + tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT") + (status, fvs) = tbl.get(bridge_port_oid) + assert status == True + values = dict(fvs) + if values["SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE"] == learn_mode: + return True + else: + return False + + def test_PortMacLearnMode(self, dvs, testlog): + self.setup_db(dvs) + + # create vlan + tbl = swsscommon.Table(self.cdb, "VLAN") + fvs = swsscommon.FieldValuePairs([("vlanid", "2")]) + tbl.set("Vlan2", fvs) + time.sleep(1) + + # create vlan member entry in application db + tbl = swsscommon.Table(self.cdb, "VLAN_MEMBER") + fvs = swsscommon.FieldValuePairs([("tagging_mode", "untagged")]) + tbl.set("Vlan2|Ethernet8", fvs) + time.sleep(1) + + # get port oid + port_oid = self.get_port_oid("Ethernet8") + assert port_oid is not None + + # check appdb before setting mac learn mode; There is no "learn_mode" attribute by default. + tbl = swsscommon.Table(self.pdb, "PORT_TABLE") + status = self.check_learn_mode_in_appdb(tbl, "Ethernet8", "disabled") + assert status == False + + # check asicdb before setting mac learn mode; The default learn_mode value is SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW. + status = self.check_learn_mode_in_asicdb(port_oid, "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW") + assert status == True + + # set MAC learn mode disable to port + tbl = swsscommon.Table(self.cdb, "PORT") + fvs = swsscommon.FieldValuePairs([("learn_mode", "disabled")]) + tbl.set("Ethernet8", fvs) + time.sleep(1) + + # check application database + tbl = swsscommon.Table(self.pdb, "PORT_TABLE") + status = self.check_learn_mode_in_appdb(tbl, "Ethernet8", "disabled") + assert status == True + + # check ASIC bridge port database + status = self.check_learn_mode_in_asicdb(port_oid, "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE") + assert status == True + + # remove vlan member + tbl = swsscommon.Table(self.cdb, "VLAN_MEMBER") + tbl._del("Vlan2|Ethernet8") + time.sleep(1) + + # remove vlan + tbl = swsscommon.Table(self.cdb, "VLAN") + tbl._del("Vlan2") + time.sleep(1) + + def test_PortchannelMacLearnMode(self, dvs, testlog): + self.setup_db(dvs) + + #create portchannel + tbl = swsscommon.Table(self.cdb, "PORTCHANNEL") + fvs = swsscommon.FieldValuePairs([("admin_status", "up"), + ("mtu", "9100")]) + tbl.set("PortChannel001", fvs) + time.sleep(1) + + # create vlan + tbl = swsscommon.Table(self.cdb, "VLAN") + fvs = swsscommon.FieldValuePairs([("vlanid", "3")]) + tbl.set("Vlan3", fvs) + time.sleep(1) + + # create vlan member entry in application db + tbl = swsscommon.Table(self.cdb, "VLAN_MEMBER") + fvs = swsscommon.FieldValuePairs([("tagging_mode", "untagged")]) + tbl.set("Vlan3|PortChannel001", fvs) + time.sleep(1) + + # get PortChannel oid; When sonic-swss pr885 is complete, you can get oid directly from COUNTERS_LAG_NAME_MAP, which would be better. + lag_tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_LAG") + lag_entries = lag_tbl.getKeys() + # At this point there should be only one lag in the system, which is PortChannel001. + assert len(lag_entries) == 1 + lag_oid = lag_entries[0] + + # check appdb before setting mac learn mode; There is no "learn_mode" attribute by default. + tbl = swsscommon.Table(self.pdb, "LAG_TABLE") + status = self.check_learn_mode_in_appdb(tbl, "PortChannel001", "disabled") + assert status == False + + # check asicdb before setting mac learn mode; The default learn_mode value is SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW. + status = self.check_learn_mode_in_asicdb(lag_oid, "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW") + assert status == True + + # set mac learn mode disable to PortChannel + tbl = swsscommon.Table(self.cdb, "PORTCHANNEL") + fvs = swsscommon.FieldValuePairs([("learn_mode", "disabled")]) + tbl.set("PortChannel001", fvs) + time.sleep(1) + + # check application database + tbl = swsscommon.Table(self.pdb, "LAG_TABLE") + status = self.check_learn_mode_in_appdb(tbl, "PortChannel001", "disabled") + assert status == True + + # check ASIC bridge port database + status = self.check_learn_mode_in_asicdb(lag_oid, "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE") + assert status == True + + # remove vlan member + tbl = swsscommon.Table(self.cdb, "VLAN_MEMBER") + tbl._del("Vlan3|PortChannel001") + time.sleep(1) + + # create vlan + tbl = swsscommon.Table(self.cdb, "VLAN") + tbl._del("Vlan3") + time.sleep(1) + + # remove PortChannel + tbl = swsscommon.Table(self.cdb, "PORTCHANNEL") + tbl._del("PortChannel001") + time.sleep(1) From 15d745142228da8b85d9196040c3485f50b20e69 Mon Sep 17 00:00:00 2001 From: "shine.chen" Date: Mon, 17 Jun 2019 00:51:49 -0700 Subject: [PATCH 3/6] refine code according to community review Signed-off-by: shine.chen --- cfgmgr/portmgr.cpp | 23 +++++- cfgmgr/portmgr.h | 1 + cfgmgr/teammgr.cpp | 23 ++++++ cfgmgr/teammgr.h | 1 + orchagent/port.h | 1 + orchagent/portsorch.cpp | 159 ++++++++++++++++------------------------ orchagent/portsorch.h | 4 +- 7 files changed, 112 insertions(+), 100 deletions(-) diff --git a/cfgmgr/portmgr.cpp b/cfgmgr/portmgr.cpp index 03b2e61eb6..976c524f7d 100644 --- a/cfgmgr/portmgr.cpp +++ b/cfgmgr/portmgr.cpp @@ -55,6 +55,17 @@ bool PortMgr::setPortAdminStatus(const string &alias, const bool up) return true; } +bool PortMgr::setPortLearnMode(const string &alias, const string &learn_mode) +{ + // Set the port MAC learn mode in application database + vector fvs; + FieldValueTuple fv("learn_mode", learn_mode); + fvs.push_back(fv); + m_appPortTable.set(alias, fvs); + + return true; +} + bool PortMgr::isPortStateOk(const string &alias) { vector temp; @@ -91,7 +102,7 @@ void PortMgr::doTask(Consumer &consumer) continue; } - string admin_status, mtu; + string admin_status, mtu, learn_mode; bool configured = (m_portList.find(alias) != m_portList.end()); @@ -116,6 +127,10 @@ void PortMgr::doTask(Consumer &consumer) { admin_status = fvValue(i); } + else if (fvField(i) == "learn_mode") + { + learn_mode = fvValue(i); + } } if (!mtu.empty()) @@ -129,6 +144,12 @@ void PortMgr::doTask(Consumer &consumer) setPortAdminStatus(alias, admin_status == "up"); SWSS_LOG_NOTICE("Configure %s admin status to %s", alias.c_str(), admin_status.c_str()); } + + if (!learn_mode.empty()) + { + setPortLearnMode(alias, learn_mode); + SWSS_LOG_NOTICE("Configure %s MAC learn mode to %s", alias.c_str(), learn_mode.c_str()); + } } it = consumer.m_toSync.erase(it); diff --git a/cfgmgr/portmgr.h b/cfgmgr/portmgr.h index eae407d13c..d7b365922f 100644 --- a/cfgmgr/portmgr.h +++ b/cfgmgr/portmgr.h @@ -31,6 +31,7 @@ class PortMgr : public Orch void doTask(Consumer &consumer); bool setPortMtu(const string &alias, const string &mtu); bool setPortAdminStatus(const string &alias, const bool up); + bool setPortLearnMode(const string &alias, const string &learn_mode); bool isPortStateOk(const string &alias); }; diff --git a/cfgmgr/teammgr.cpp b/cfgmgr/teammgr.cpp index aa70127641..b22b10af03 100644 --- a/cfgmgr/teammgr.cpp +++ b/cfgmgr/teammgr.cpp @@ -126,6 +126,7 @@ void TeamMgr::doLagTask(Consumer &consumer) bool fallback = false; string admin_status = DEFAULT_ADMIN_STATUS_STR; string mtu = DEFAULT_MTU_STR; + string learn_mode; for (auto i : kfvFieldsValues(t)) { @@ -153,6 +154,12 @@ void TeamMgr::doLagTask(Consumer &consumer) mtu = fvValue(i); SWSS_LOG_INFO("Get MTU %s", mtu.c_str()); } + else if (fvField(i) == "learn_mode") + { + learn_mode = fvValue(i); + SWSS_LOG_INFO("Get learn_mode %s", + learn_mode.c_str()); + } } if (m_lagList.find(alias) == m_lagList.end()) @@ -168,6 +175,11 @@ void TeamMgr::doLagTask(Consumer &consumer) setLagAdminStatus(alias, admin_status); setLagMtu(alias, mtu); + if (!learn_mode.empty()) + { + setLagLearnMode(alias, learn_mode); + SWSS_LOG_NOTICE("Configure %s MAC learn mode to %s", alias.c_str(), learn_mode.c_str()); + } } else if (op == DEL_COMMAND) { @@ -365,6 +377,17 @@ bool TeamMgr::setLagMtu(const string &alias, const string &mtu) return true; } +bool TeamMgr::setLagLearnMode(const string &alias, const string &learn_mode) +{ + // Set the port MAC learn mode in application database + vector fvs; + FieldValueTuple fv("learn_mode", learn_mode); + fvs.push_back(fv); + m_appLagTable.set(alias, fvs); + + return true; +} + task_process_status TeamMgr::addLag(const string &alias, int min_links, bool fallback) { SWSS_LOG_ENTER(); diff --git a/cfgmgr/teammgr.h b/cfgmgr/teammgr.h index ce3864150f..753f27d632 100644 --- a/cfgmgr/teammgr.h +++ b/cfgmgr/teammgr.h @@ -44,6 +44,7 @@ class TeamMgr : public Orch bool setLagAdminStatus(const string &alias, const string &admin_status); bool setLagMtu(const string &alias, const string &mtu); + bool setLagLearnMode(const string &alias, const string &learn_mode); bool isPortEnslaved(const string &); bool findPortMaster(string &, const string &); diff --git a/orchagent/port.h b/orchagent/port.h index 454ecd68af..2c9dcd4c9e 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -72,6 +72,7 @@ class Port uint32_t m_mtu = DEFAULT_MTU; uint32_t m_speed = 0; // Mbps bool m_autoneg = 0; + std::string m_learn_mode = "hardware"; sai_object_id_t m_port_id = 0; sai_port_fec_mode_t m_fec_mode = SAI_PORT_FEC_MODE_NONE; VlanInfo m_vlan_info; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index df8fe4920b..617e329a72 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -58,6 +58,16 @@ static map pfc_asym_map = { "off", SAI_PORT_PRIORITY_FLOW_CONTROL_MODE_COMBINED } }; +static map learn_mode_map = +{ + { "drop", SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DROP }, + { "disable", SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE }, + { "hardware", SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW }, + { "cpu_trap", SAI_BRIDGE_PORT_FDB_LEARNING_MODE_CPU_TRAP}, + { "cpu_log", SAI_BRIDGE_PORT_FDB_LEARNING_MODE_CPU_LOG}, + { "notification", SAI_BRIDGE_PORT_FDB_LEARNING_MODE_FDB_NOTIFICATION} +}; + const vector portStatIds = { SAI_PORT_STAT_IF_IN_OCTETS, @@ -1530,7 +1540,7 @@ void PortsOrch::doPortTask(Consumer &consumer) { learn_mode = fvValue(i); } - + /* Set port asymmetric PFC */ if (fvField(i) == "pfc_asym") pfc_asym = fvValue(i); @@ -1820,48 +1830,33 @@ void PortsOrch::doPortTask(Consumer &consumer) SWSS_LOG_ERROR("Unknown fec mode %s", fec_mode.c_str()); } } - - if (learn_mode != "") + + if (!learn_mode.empty() && (p.m_learn_mode != learn_mode)) { - if (learn_mode == "enabled" || learn_mode == "disabled") + if (p.m_bridge_port_id != SAI_NULL_OBJECT_ID) { - int32_t mode; - - if (learn_mode == "enabled") + if(setBridgePortLearnMode(p, learn_mode)) { - mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW; + p.m_learn_mode = learn_mode; + m_portList[alias] = p; + SWSS_LOG_NOTICE("Set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); } else { - mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE; - } - - if (p.m_bridge_port_id != SAI_NULL_OBJECT_ID) - { - if(setBridgePortLearnMode(p, mode)) - { - SWSS_LOG_NOTICE("Set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); - } - else - { - SWSS_LOG_ERROR("Failed to set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); - it++; - continue; - } - } - else - { - m_portSetLearnModeList[alias] = learn_mode; - - SWSS_LOG_NOTICE("Saved to set port %s learn mode %s", alias.c_str(), learn_mode.c_str()); + SWSS_LOG_ERROR("Failed to set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); + it++; + continue; } } else { - SWSS_LOG_ERROR("Unknown learn mode %s", learn_mode.c_str()); + p.m_learn_mode = learn_mode; + m_portList[alias] = p; + + SWSS_LOG_NOTICE("Saved to set port %s learn mode %s", alias.c_str(), learn_mode.c_str()); } } - + if (pfc_asym != "") { if (setPortPfcAsym(p, pfc_asym)) @@ -2061,38 +2056,7 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer) } if (addBridgePort(port) && addVlanMember(vlan, port, tagging_mode)) - { - int32_t mode; - string learning_mode; - - if(m_portSetLearnModeList.find(port_alias) != m_portSetLearnModeList.end()) - { - learning_mode = m_portSetLearnModeList[port_alias]; - - SWSS_LOG_NOTICE("Set port %s learn mode %s when add port to vlan %s", port_alias.c_str(), learning_mode.c_str(), vlan_alias.c_str()); - - if (learning_mode == "enabled") - { - mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW; - } - else if (learning_mode == "disabled") - { - mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE; - } - else - { - SWSS_LOG_ERROR("Wrong learning_mode '%s' ", learning_mode.c_str()); - it = consumer.m_toSync.erase(it); - continue; - } - - setBridgePortLearnMode(port, mode); - - m_portSetLearnModeList.erase(port_alias); - } - it = consumer.m_toSync.erase(it); - } else it++; } @@ -2176,7 +2140,7 @@ void PortsOrch::doLagTask(Consumer &consumer) gIntfsOrch->setRouterIntfsMtu(l); } } - + string learn_mode; for (auto i : kfvFieldsValues(t)) { @@ -2186,45 +2150,30 @@ void PortsOrch::doLagTask(Consumer &consumer) learn_mode = fvValue(i); } } - - if (learn_mode != "") + + if (!learn_mode.empty() && (l.m_learn_mode != learn_mode)) { - if (learn_mode == "enabled" || learn_mode == "disabled") + if (l.m_bridge_port_id != SAI_NULL_OBJECT_ID) { - int32_t mode; - - if (learn_mode == "enabled") - { - mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW; - } - else + if(setBridgePortLearnMode(l, learn_mode)) { - mode = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE; - } - - if (l.m_bridge_port_id != SAI_NULL_OBJECT_ID) - { - if(setBridgePortLearnMode(l, mode)) - { - SWSS_LOG_NOTICE("Set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); - } - else - { - SWSS_LOG_ERROR("Failed to set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); - it++; - continue; - } + l.m_learn_mode = learn_mode; + m_portList[alias] = l; + SWSS_LOG_NOTICE("Set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); } else { - m_portSetLearnModeList[alias] = learn_mode; - - SWSS_LOG_NOTICE("Saved to set port %s learn mode %s", alias.c_str(), learn_mode.c_str()); + SWSS_LOG_ERROR("Failed to set port %s learn mode to %s", alias.c_str(), learn_mode.c_str()); + it++; + continue; } } else { - SWSS_LOG_ERROR("Unknown learn mode %s", learn_mode.c_str()); + l.m_learn_mode = learn_mode; + m_portList[alias] = l; + + SWSS_LOG_NOTICE("Saved to set port %s learn mode %s", alias.c_str(), learn_mode.c_str()); } } } @@ -2627,7 +2576,15 @@ bool PortsOrch::addBridgePort(Port &port) /* 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; + auto found = learn_mode_map.find(port.m_learn_mode); + if (found == learn_mode_map.end()) + { + attr.value.s32 = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW; + } + else + { + attr.value.s32 = found->second; + } 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()); @@ -2698,7 +2655,7 @@ bool PortsOrch::removeBridgePort(Port &port) return true; } -bool PortsOrch::setBridgePortLearnMode(Port &port, int32_t mode) +bool PortsOrch::setBridgePortLearnMode(Port &port, string learn_mode) { SWSS_LOG_ENTER(); @@ -2706,10 +2663,18 @@ bool PortsOrch::setBridgePortLearnMode(Port &port, int32_t mode) { return true; } + + auto found = learn_mode_map.find(learn_mode); + if (found == learn_mode_map.end()) + { + SWSS_LOG_ERROR("Incorrect MAC learn mode: %s", learn_mode.c_str()); + return false; + } + /* Set bridge port learning mode */ sai_attribute_t attr; attr.id = SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE; - attr.value.s32 = mode; + attr.value.s32 = found->second; sai_status_t status = sai_bridge_api->set_bridge_port_attribute(port.m_bridge_port_id, &attr); if (status != SAI_STATUS_SUCCESS) @@ -2719,7 +2684,7 @@ bool PortsOrch::setBridgePortLearnMode(Port &port, int32_t mode) return false; } - SWSS_LOG_NOTICE("Set bridge port %s learning mode", port.m_alias.c_str()); + SWSS_LOG_NOTICE("Set bridge port %s learning mode %s", port.m_alias.c_str(), learn_mode.c_str()); return true; } @@ -2923,7 +2888,7 @@ bool PortsOrch::addLag(string lag_alias) PortUpdate update = { lag, true }; notify(SUBJECT_TYPE_PORT_CHANGE, static_cast(&update)); - + return true; } diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 7756ed40dc..f70b2e0dd9 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -118,7 +118,7 @@ class PortsOrch : public Orch, public Subject map, tuple> m_lanesAliasSpeedMap; map m_portList; unordered_set m_pendingPortSet; - map m_portSetLearnModeList; + NotificationConsumer* m_portStatusNotificationConsumer; @@ -143,7 +143,7 @@ class PortsOrch : public Orch, public Subject bool addBridgePort(Port &port); bool removeBridgePort(Port &port); - bool setBridgePortLearnMode(Port &port, int32_t mode); + bool setBridgePortLearnMode(Port &port, string learn_mode); bool addVlan(string vlan); bool removeVlan(Port vlan); From 2f9c789bf507bd9b377a952dbce87b6e3e6394ba Mon Sep 17 00:00:00 2001 From: "leo.li" Date: Thu, 20 Jun 2019 22:14:20 -0700 Subject: [PATCH 4/6] [vstest] update the corresponding vstest for pr#809 Signed-off-by: leo.li --- tests/test_port_mac_learn.py | 79 ++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 35 deletions(-) diff --git a/tests/test_port_mac_learn.py b/tests/test_port_mac_learn.py index 833f37d6ef..2abf4291ac 100644 --- a/tests/test_port_mac_learn.py +++ b/tests/test_port_mac_learn.py @@ -10,6 +10,15 @@ def setup_db(self, dvs): self.cdb = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) self.cntdb = swsscommon.DBConnector(swsscommon.COUNTERS_DB, dvs.redis_sock, 0) + def get_learn_mode_map(self): + learn_mode_map = { "drop": "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DROP", + "disable": "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE", + "hardware": "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW", + "cpu_trap": "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_CPU_TRAP", + "cpu_log": "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_CPU_LOG", + "notification": "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_FDB_NOTIFICATION"} + return learn_mode_map + def get_port_oid(self, port_name): port_map_tbl = swsscommon.Table(self.cntdb, 'COUNTERS_PORT_NAME_MAP') for k in port_map_tbl.get('')[1]: @@ -69,30 +78,33 @@ def test_PortMacLearnMode(self, dvs, testlog): port_oid = self.get_port_oid("Ethernet8") assert port_oid is not None - # check appdb before setting mac learn mode; There is no "learn_mode" attribute by default. - tbl = swsscommon.Table(self.pdb, "PORT_TABLE") - status = self.check_learn_mode_in_appdb(tbl, "Ethernet8", "disabled") - assert status == False - # check asicdb before setting mac learn mode; The default learn_mode value is SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW. status = self.check_learn_mode_in_asicdb(port_oid, "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW") assert status == True - # set MAC learn mode disable to port + learn_mode_map = self.get_learn_mode_map() + for key, value in learn_mode_map.items(): + # set MAC learn mode to port + tbl = swsscommon.Table(self.cdb, "PORT") + fvs = swsscommon.FieldValuePairs([("learn_mode", key)]) + tbl.set("Ethernet8", fvs) + time.sleep(1) + + # check application database + tbl = swsscommon.Table(self.pdb, "PORT_TABLE") + status = self.check_learn_mode_in_appdb(tbl, "Ethernet8", key) + assert status == True + + # check ASIC bridge port database + status = self.check_learn_mode_in_asicdb(port_oid, value) + assert status == True + + # set default learn mode for Ethernet8 tbl = swsscommon.Table(self.cdb, "PORT") - fvs = swsscommon.FieldValuePairs([("learn_mode", "disabled")]) + fvs = swsscommon.FieldValuePairs([("learn_mode", "hardware")]) tbl.set("Ethernet8", fvs) time.sleep(1) - # check application database - tbl = swsscommon.Table(self.pdb, "PORT_TABLE") - status = self.check_learn_mode_in_appdb(tbl, "Ethernet8", "disabled") - assert status == True - - # check ASIC bridge port database - status = self.check_learn_mode_in_asicdb(port_oid, "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE") - assert status == True - # remove vlan member tbl = swsscommon.Table(self.cdb, "VLAN_MEMBER") tbl._del("Vlan2|Ethernet8") @@ -132,29 +144,26 @@ def test_PortchannelMacLearnMode(self, dvs, testlog): assert len(lag_entries) == 1 lag_oid = lag_entries[0] - # check appdb before setting mac learn mode; There is no "learn_mode" attribute by default. - tbl = swsscommon.Table(self.pdb, "LAG_TABLE") - status = self.check_learn_mode_in_appdb(tbl, "PortChannel001", "disabled") - assert status == False - # check asicdb before setting mac learn mode; The default learn_mode value is SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW. status = self.check_learn_mode_in_asicdb(lag_oid, "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW") assert status == True - # set mac learn mode disable to PortChannel - tbl = swsscommon.Table(self.cdb, "PORTCHANNEL") - fvs = swsscommon.FieldValuePairs([("learn_mode", "disabled")]) - tbl.set("PortChannel001", fvs) - time.sleep(1) - - # check application database - tbl = swsscommon.Table(self.pdb, "LAG_TABLE") - status = self.check_learn_mode_in_appdb(tbl, "PortChannel001", "disabled") - assert status == True - - # check ASIC bridge port database - status = self.check_learn_mode_in_asicdb(lag_oid, "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE") - assert status == True + learn_mode_map = self.get_learn_mode_map() + for key, value in learn_mode_map.items(): + # set mac learn mode to PortChannel + tbl = swsscommon.Table(self.cdb, "PORTCHANNEL") + fvs = swsscommon.FieldValuePairs([("learn_mode", key)]) + tbl.set("PortChannel001", fvs) + time.sleep(1) + + # check application database + tbl = swsscommon.Table(self.pdb, "LAG_TABLE") + status = self.check_learn_mode_in_appdb(tbl, "PortChannel001", key) + assert status == True + + # check ASIC bridge port database + status = self.check_learn_mode_in_asicdb(lag_oid, value) + assert status == True # remove vlan member tbl = swsscommon.Table(self.cdb, "VLAN_MEMBER") From 2deeaf13ce54a64e17622af6cd4bc95821af8b63 Mon Sep 17 00:00:00 2001 From: "shine.chen" Date: Thu, 29 Aug 2019 19:32:38 -0700 Subject: [PATCH 5/6] optimize the process of fetching learn mode attribute Signed-off-by: shine.chen --- orchagent/portsorch.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 668862f6a1..fe3ef08d4d 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2229,12 +2229,20 @@ void PortsOrch::doLagTask(Consumer &consumer) { // Retrieve attributes uint32_t mtu = 0; + string learn_mode; + for (auto i : kfvFieldsValues(t)) { if (fvField(i) == "mtu") { mtu = (uint32_t)stoul(fvValue(i)); } + + /* Get port fdb learn mode*/ + if (fvField(i) == "learn_mode") + { + learn_mode = fvValue(i); + } } // Create a new LAG when the new alias comes @@ -2265,16 +2273,6 @@ void PortsOrch::doLagTask(Consumer &consumer) } } - string learn_mode; - for (auto i : kfvFieldsValues(t)) - { - /* Get port fdb learn mode*/ - if (fvField(i) == "learn_mode") - { - learn_mode = fvValue(i); - } - } - if (!learn_mode.empty() && (l.m_learn_mode != learn_mode)) { if (l.m_bridge_port_id != SAI_NULL_OBJECT_ID) From c757eceaec4eb2a949718ea5ba085933d36e96f2 Mon Sep 17 00:00:00 2001 From: shine4chen <37530989+shine4chen@users.noreply.github.com> Date: Wed, 2 Oct 2019 20:00:17 +0800 Subject: [PATCH 6/6] fix compiler error --- orchagent/portsorch.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 4350b9e128..063dde01f7 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2219,13 +2219,11 @@ void PortsOrch::doLagTask(Consumer &consumer) { mtu = (uint32_t)stoul(fvValue(i)); } - - /* Get port fdb learn mode*/ - if (fvField(i) == "learn_mode") + else if (fvField(i) == "learn_mode") { learn_mode = fvValue(i); - - if (fvField(i) == "oper_status") + } + else if (fvField(i) == "oper_status") { if (fvValue(i) == "down") {