From 53d6a1d1342c4e2a4ada262c2fd2ca3f21087b5c Mon Sep 17 00:00:00 2001 From: Volodymyr Samotiy Date: Wed, 4 Oct 2017 01:17:13 +0300 Subject: [PATCH 1/4] [portsorch]: Add support of cable breakout feature (#320) Signed-off-by: Volodymyr Samotiy --- orchagent/portsorch.cpp | 186 +++++++++++++++++++++++++++++++++------- orchagent/portsorch.h | 6 ++ portsyncd/portsyncd.cpp | 57 +++++++++--- 3 files changed, 206 insertions(+), 43 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index e446bb76ac..40f9a8219c 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -5,12 +5,14 @@ #include #include #include +#include #include #include "net/if.h" #include "logger.h" #include "schema.h" +#include "converter.h" extern sai_switch_api_t *sai_switch_api; extern sai_bridge_api_t *sai_bridge_api; @@ -470,6 +472,107 @@ void PortsOrch::updateDbPortOperStatus(sai_object_id_t id, sai_port_oper_status_ } } +bool PortsOrch::addPort(const set &lane_set, uint32_t speed) +{ + SWSS_LOG_ENTER(); + + vector lanes(lane_set.begin(), lane_set.end()); + + sai_attribute_t attr; + vector attrs; + + attr.id = SAI_PORT_ATTR_SPEED; + attr.value.u32 = speed; + attrs.push_back(attr); + + attr.id = SAI_PORT_ATTR_HW_LANE_LIST; + attr.value.u32list.list = lanes.data(); + attr.value.u32list.count = static_cast(lanes.size()); + attrs.push_back(attr); + + sai_object_id_t port_id; + sai_status_t status = sai_port_api->create_port(&port_id, gSwitchId, static_cast(attrs.size()), attrs.data()); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create port with the speed %u, rv:%d", speed, status); + return false; + } + + m_portListLaneMap[lane_set] = port_id; + + SWSS_LOG_NOTICE("Create port %lx with the speed %u", port_id, speed); + + return true; +} + +bool PortsOrch::removePort(sai_object_id_t port_id) +{ + SWSS_LOG_ENTER(); + + sai_status_t status = sai_port_api->remove_port(port_id); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to remove port %lx, rv:%d", port_id, status); + return false; + } + + SWSS_LOG_NOTICE("Remove port %lx", port_id); + + return true; +} + +bool PortsOrch::initPort(const string &alias, const set &lane_set) +{ + SWSS_LOG_ENTER(); + + /* Determine if the lane combination exists in switch */ + if (m_portListLaneMap.find(lane_set) != m_portListLaneMap.end()) + { + sai_object_id_t id = m_portListLaneMap[lane_set]; + + /* Determine if the port has already been initialized before */ + if (m_portList.find(alias) != m_portList.end() && m_portList[alias].m_port_id == id) + { + SWSS_LOG_INFO("Port has already been initialized before alias:%s", alias.c_str()); + } + else + { + Port p(alias, Port::PHY); + + p.m_index = static_cast(m_portList.size()); // TODO: Assume no deletion of physical port + p.m_port_id = id; + + /* Initialize the port and create router interface and host interface */ + if (initializePort(p)) + { + /* Add port to port list */ + m_portList[alias] = p; + /* Add port name map to counter table */ + std::stringstream ss; + ss << hex << p.m_port_id; + FieldValueTuple tuple(p.m_alias, ss.str()); + vector vector; + vector.push_back(tuple); + m_counterTable->set("", vector); + + SWSS_LOG_NOTICE("Initialized port %s", alias.c_str()); + } + else + { + SWSS_LOG_ERROR("Failed to initialize port %s", alias.c_str()); + return false; + } + } + } + else + { + SWSS_LOG_ERROR("Failed to locate port lane combination alias:%s", alias.c_str()); + return false; + } + + return true; +} + void PortsOrch::doPortTask(Consumer &consumer) { SWSS_LOG_ENTER(); @@ -482,13 +585,26 @@ void PortsOrch::doPortTask(Consumer &consumer) string alias = kfvKey(t); string op = kfvOp(t); + if (alias == "PortConfigDone") + { + m_portConfigDone = true; + + for (auto i : kfvFieldsValues(t)) + { + if (fvField(i) == "count") + { + m_portCount = to_uint(fvValue(i)); + } + } + } + /* Get notification from application */ /* portsyncd application: - * When portsorch receives 'ConfigDone' message, it indicates port initialization + * When portsorch receives 'PortInitDone' message, it indicates port initialization * procedure is done. Before port initialization procedure, none of other tasks * are executed. */ - if (alias == "ConfigDone") + if (alias == "PortInitDone") { /* portsyncd restarting case: * When portsyncd restarts, duplicate notifications may be received. @@ -496,7 +612,7 @@ void PortsOrch::doPortTask(Consumer &consumer) if (!m_initDone) { m_initDone = true; - SWSS_LOG_INFO("Get ConfigDone notification from portsyncd."); + SWSS_LOG_INFO("Get PortInitDone notification from portsyncd."); } it = consumer.m_toSync.erase(it); @@ -523,7 +639,6 @@ void PortsOrch::doPortTask(Consumer &consumer) int lane = stoi(lane_str); lane_set.insert(lane); } - } /* Set port admin status */ @@ -539,47 +654,52 @@ void PortsOrch::doPortTask(Consumer &consumer) speed = (uint32_t)stoul(fvValue(i)); } + /* Collect information about all received ports */ if (lane_set.size()) { - /* Determine if the lane combination exists in switch */ - if (m_portListLaneMap.find(lane_set) != - m_portListLaneMap.end()) - { - sai_object_id_t id = m_portListLaneMap[lane_set]; + m_lanesAliasSpeedMap[lane_set] = make_tuple(alias, speed); + } - /* Determin if the port has already been initialized before */ - if (m_portList.find(alias) != m_portList.end() && m_portList[alias].m_port_id == id) + /* Once all ports received, go through the each port and perform appropriate actions: + * 1. Remove ports which don't exist anymore + * 2. Create new ports + * 3. Initialize all ports + */ + if (m_portConfigDone && (m_lanesAliasSpeedMap.size() == m_portCount)) + { + for (auto it = m_portListLaneMap.begin(); it != m_portListLaneMap.end();) + { + if (m_lanesAliasSpeedMap.find(it->first) == m_lanesAliasSpeedMap.end()) { - SWSS_LOG_INFO("Port has already been initialized before alias:%s", alias.c_str()); + if (!removePort(it->second)) + { + throw runtime_error("PortsOrch initialization failure."); + } + it = m_portListLaneMap.erase(it); } else { - Port p(alias, Port::PHY); - - p.m_index = (uint32_t)m_portList.size(); // TODO: Assume no deletion of physical port - p.m_port_id = id; + ++it; + } + } - /* Initialize the port and create router interface and host interface */ - if (initializePort(p)) + for (auto it = m_lanesAliasSpeedMap.begin(); it != m_lanesAliasSpeedMap.end();) + { + if (m_portListLaneMap.find(it->first) == m_portListLaneMap.end()) + { + if (!addPort(it->first, get<1>(it->second))) { - /* Add port to port list */ - m_portList[alias] = p; - /* Add port name map to counter table */ - std::stringstream ss; - ss << hex << p.m_port_id; - FieldValueTuple tuple(p.m_alias, ss.str()); - vector vector; - vector.push_back(tuple); - m_counterTable->set("", vector); - - SWSS_LOG_NOTICE("Initialized port %s", alias.c_str()); + throw runtime_error("PortsOrch initialization failure."); } - else - SWSS_LOG_ERROR("Failed to initialize port %s", alias.c_str()); } + + if (!initPort(get<0>(it->second), it->first)) + { + throw runtime_error("PortsOrch initialization failure."); + } + + it = m_lanesAliasSpeedMap.erase(it); } - else - SWSS_LOG_ERROR("Failed to locate port lane combination alias:%s", alias.c_str()); } Port p; diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index b5e78297e2..959f9bf77c 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -67,8 +67,10 @@ class PortsOrch : public Orch, public Subject sai_object_id_t m_default1QBridge; sai_object_id_t m_defaultVlan; + bool m_portConfigDone = false; sai_uint32_t m_portCount; map, sai_object_id_t> m_portListLaneMap; + map, tuple> m_lanesAliasSpeedMap; map m_portList; void doTask(Consumer &consumer); @@ -100,6 +102,10 @@ class PortsOrch : public Orch, public Subject bool addLagMember(Port lag, Port port); bool removeLagMember(Port lag, Port port); + bool addPort(const set &lane_set, uint32_t speed); + bool removePort(sai_object_id_t port_id); + bool initPort(const string &alias, const set &lane_set); + bool setPortAdminStatus(sai_object_id_t id, bool up); bool setPortMtu(sai_object_id_t id, sai_uint32_t mtu); diff --git a/portsyncd/portsyncd.cpp b/portsyncd/portsyncd.cpp index f10c03ba6b..c62550fd7f 100644 --- a/portsyncd/portsyncd.cpp +++ b/portsyncd/portsyncd.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include "dbconnector.h" #include "select.h" #include "netdispatcher.h" @@ -24,7 +25,7 @@ using namespace swss; * interfaces are already created and remove them from this set. We will * remove the rest of the ports in the set when receiving the first netlink * message indicating that the host interfaces are created. After the set - * is empty, we send out the signal ConfigDone. g_init is used to limit the + * is empty, we send out the signal PortInitDone. g_init is used to limit the * command to be run only once. */ set g_portSet; @@ -107,7 +108,7 @@ int main(int argc, char **argv) */ FieldValueTuple finish_notice("lanes", "0"); vector attrs = { finish_notice }; - p.set("ConfigDone", attrs); + p.set("PortInitDone", attrs); g_init = true; } @@ -134,34 +135,70 @@ void handlePortConfigFile(ProducerStateTable &p, string file) throw "Port configuration file not found!"; } + list header = {"name", "lanes", "alias", "speed"}; string line; while (getline(infile, line)) { if (line.at(0) == '#') { + /* Find out what info is specified in the configuration file */ + for (auto it = header.begin(); it != header.end();) + { + if (line.find(*it) == string::npos) + { + it = header.erase(it); + } + else + { + ++it; + } + } + continue; } istringstream iss(line); - string name, lanes, alias; - iss >> name >> lanes >> alias; + map entry; - /* If port has no alias, then use its' name as alias */ - if (alias == "") + /* Read port configuration entry */ + for (auto column : header) { - alias = name; + iss >> entry[column]; } - FieldValueTuple lanes_attr("lanes", lanes); + + /* If port has no alias, then use its name as alias */ + string alias; + if ((entry.find("alias") != entry.end()) && (entry["alias"] != "")) + { + alias = entry["alias"]; + } + else + { + alias = entry["name"]; + } + + FieldValueTuple lanes_attr("lanes", entry["lanes"]); FieldValueTuple alias_attr("alias", alias); vector attrs; attrs.push_back(lanes_attr); attrs.push_back(alias_attr); - p.set(name, attrs); + if ((entry.find("speed") != entry.end()) && (entry["speed"] != "")) + { + FieldValueTuple speed_attr("speed", entry["speed"]); + attrs.push_back(speed_attr); + } - g_portSet.insert(name); + p.set(entry["name"], attrs); + + g_portSet.insert(entry["name"]); } infile.close(); + + /* Notify that all ports added */ + FieldValueTuple finish_notice("count", to_string(g_portSet.size())); + vector attrs = { finish_notice }; + p.set("PortConfigDone", attrs); } From 70a28660c71b318a861c60f5cb937d67c6f09f39 Mon Sep 17 00:00:00 2001 From: Volodymyr Samotiy Date: Wed, 4 Oct 2017 01:35:05 +0300 Subject: [PATCH 2/4] [switchorch]: Add support of ECMP and LAG hash seed attribute (#324) Signed-off-by: Volodymyr Samotiy --- orchagent/switchorch.cpp | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/orchagent/switchorch.cpp b/orchagent/switchorch.cpp index e48dcae431..ab74bb8ad1 100644 --- a/orchagent/switchorch.cpp +++ b/orchagent/switchorch.cpp @@ -1,6 +1,7 @@ #include #include "switchorch.h" +#include "converter.h" using namespace std; using namespace swss; @@ -12,7 +13,9 @@ const map switch_attribute_map = { {"fdb_unicast_miss_packet_action", SAI_SWITCH_ATTR_FDB_UNICAST_MISS_PACKET_ACTION}, {"fdb_broadcast_miss_packet_action", SAI_SWITCH_ATTR_FDB_BROADCAST_MISS_PACKET_ACTION}, - {"fdb_multicast_miss_packet_action", SAI_SWITCH_ATTR_FDB_MULTICAST_MISS_PACKET_ACTION} + {"fdb_multicast_miss_packet_action", SAI_SWITCH_ATTR_FDB_MULTICAST_MISS_PACKET_ACTION}, + {"ecmp_hash_seed", SAI_SWITCH_ATTR_ECMP_DEFAULT_HASH_SEED}, + {"lag_hash_seed", SAI_SWITCH_ATTR_LAG_DEFAULT_HASH_SEED} }; const map packet_action_map = @@ -52,16 +55,32 @@ void SwitchOrch::doTask(Consumer &consumer) } auto value = fvValue(i); - if (packet_action_map.find(value) == packet_action_map.end()) - { - SWSS_LOG_ERROR("Unsupported packet action %s", value.c_str()); - it = consumer.m_toSync.erase(it); - continue; - } sai_attribute_t attr; attr.id = switch_attribute_map.at(attribute); - attr.value.s32 = packet_action_map.at(value); + + switch (attr.id) + { + case SAI_SWITCH_ATTR_FDB_UNICAST_MISS_PACKET_ACTION: + case SAI_SWITCH_ATTR_FDB_BROADCAST_MISS_PACKET_ACTION: + case SAI_SWITCH_ATTR_FDB_MULTICAST_MISS_PACKET_ACTION: + if (packet_action_map.find(value) == packet_action_map.end()) + { + SWSS_LOG_ERROR("Unsupported packet action %s", value.c_str()); + it = consumer.m_toSync.erase(it); + continue; + } + attr.value.s32 = packet_action_map.at(value); + break; + + case SAI_SWITCH_ATTR_ECMP_DEFAULT_HASH_SEED: + case SAI_SWITCH_ATTR_LAG_DEFAULT_HASH_SEED: + attr.value.u32 = to_uint(value); + break; + + default: + break; + } sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr); if (status != SAI_STATUS_SUCCESS) @@ -83,3 +102,4 @@ void SwitchOrch::doTask(Consumer &consumer) } } } + From f208eb7f94c53c39607ae494f9cc459c0057f349 Mon Sep 17 00:00:00 2001 From: Shuotian Cheng Date: Tue, 3 Oct 2017 21:15:55 -0700 Subject: [PATCH 3/4] [portsorch]: Update comments (#333) Signed-off-by: Shu0T1an ChenG --- orchagent/portsorch.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 40f9a8219c..3d9a7deba8 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -359,6 +359,7 @@ bool PortsOrch::validatePortSpeed(sai_object_id_t port_id, sai_uint32_t speed) attr.id = SAI_PORT_ATTR_SUPPORTED_SPEED; attr.value.u32list.count = 0; attr.value.u32list.list = NULL; + status = sai_port_api->get_port_attribute(port_id, 1, &attr); if (status == SAI_STATUS_BUFFER_OVERFLOW) { @@ -542,7 +543,7 @@ bool PortsOrch::initPort(const string &alias, const set &lane_set) p.m_index = static_cast(m_portList.size()); // TODO: Assume no deletion of physical port p.m_port_id = id; - /* Initialize the port and create router interface and host interface */ + /* Initialize the port and create corresponding host interface */ if (initializePort(p)) { /* Add port to port list */ @@ -679,7 +680,7 @@ void PortsOrch::doPortTask(Consumer &consumer) } else { - ++it; + it++; } } @@ -709,6 +710,12 @@ void PortsOrch::doPortTask(Consumer &consumer) } else { + /* Set port speed + * 1. Get supported speed list and validate if the target speed is within the list + * 2. Get the current port speed and check if it is the same as the target speed + * 3. Set port admin status to DOWN before changing the speed + * 4. Set port speed + */ if (speed != 0) { sai_uint32_t current_speed; @@ -724,7 +731,7 @@ void PortsOrch::doPortTask(Consumer &consumer) { if (speed != current_speed) { - if(setPortAdminStatus(p.m_port_id, false)) + if (setPortAdminStatus(p.m_port_id, false)) { if (setPortSpeed(p.m_port_id, speed)) { @@ -747,13 +754,14 @@ void PortsOrch::doPortTask(Consumer &consumer) { SWSS_LOG_ERROR("Failed to get current speed for port %s", alias.c_str()); } - } if (admin_status != "") { if (setPortAdminStatus(p.m_port_id, admin_status == "up")) + { SWSS_LOG_NOTICE("Set port %s admin status to %s", alias.c_str(), admin_status.c_str()); + } else { SWSS_LOG_ERROR("Failed to set port %s admin status to %s", alias.c_str(), admin_status.c_str()); @@ -765,7 +773,9 @@ void PortsOrch::doPortTask(Consumer &consumer) if (mtu != 0) { if (setPortMtu(p.m_port_id, mtu)) + { SWSS_LOG_NOTICE("Set port %s MTU to %u", alias.c_str(), mtu); + } else { SWSS_LOG_ERROR("Failed to set port %s MTU to %u", alias.c_str(), mtu); From 5dd5e36d4855efc1591d63d3c5e2f4841fe21f14 Mon Sep 17 00:00:00 2001 From: Marian Pritsak Date: Wed, 4 Oct 2017 21:59:41 +0300 Subject: [PATCH 4/4] [portsorch]: Use sai_serialize api to write to DB (#331) Keep object IDs serializeable in database, so that they can be reused later by client code --- orchagent/portsorch.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 3d9a7deba8..8f34bb8f93 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -13,6 +13,7 @@ #include "logger.h" #include "schema.h" #include "converter.h" +#include "saiserialize.h" extern sai_switch_api_t *sai_switch_api; extern sai_bridge_api_t *sai_bridge_api; @@ -549,9 +550,7 @@ bool PortsOrch::initPort(const string &alias, const set &lane_set) /* Add port to port list */ m_portList[alias] = p; /* Add port name map to counter table */ - std::stringstream ss; - ss << hex << p.m_port_id; - FieldValueTuple tuple(p.m_alias, ss.str()); + FieldValueTuple tuple(p.m_alias, sai_serialize_object_id(p.m_port_id)); vector vector; vector.push_back(tuple); m_counterTable->set("", vector);