diff --git a/orchagent/port.h b/orchagent/port.h index 2c27e17590..eb18b8d0b7 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -252,7 +252,6 @@ class Port bool m_adv_intf_cfg = false; // Advertised interface type bool m_fec_cfg = false; // Forward Error Correction (FEC) bool m_override_fec = false; // Enable Override FEC - bool m_pfc_asym_cfg = false; // Asymmetric Priority Flow Control (PFC) bool m_lm_cfg = false; // Forwarding Database (FDB) Learning Mode (LM) bool m_lt_cfg = false; // Link Training (LT) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 9475ec16f4..aba75147e1 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1056,6 +1056,8 @@ bool PortsOrch::addPortBulk(const std::vector &portList, std::vector attr.value.booldata = cit.autoneg.value; attrList.push_back(attr); p.m_autoneg = cit.autoneg.value; + // If port is successfully created then autoneg was set and is supported + p.m_cap_an = 1; } if (cit.fec.is_set) @@ -1063,6 +1065,42 @@ bool PortsOrch::addPortBulk(const std::vector &portList, std::vector attr.id = SAI_PORT_ATTR_FEC_MODE; attr.value.s32 = cit.fec.value; attrList.push_back(attr); + + if (fec_override_sup) + { + attr.id = SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE; + attr.value.booldata = cit.fec.override_fec; + attrList.push_back(attr); + } + + p.m_fec_mode = cit.fec.value; + p.m_override_fec = cit.fec.override_fec; + } + + if (cit.tpid.is_set) + { + sai_attribute_t attr; + attr.id = SAI_PORT_ATTR_TPID; + attr.value.u16 = cit.tpid.value; + attrList.push_back(attr); + p.m_tpid = cit.tpid.value; + } + + if (cit.pfc_asym.is_set) + { + sai_attribute_t attr; + attr.id = SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL; + attr.value.s32 = cit.pfc_asym.value; + attrList.push_back(attr); + + if (cit.pfc_asym.value == SAI_PORT_PRIORITY_FLOW_CONTROL_MODE_SEPARATE) + { + attr.id = SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL_RX; + attr.value.u8 = static_cast(0xff); + attrList.push_back(attr); + } + + p.m_pfc_asym = cit.pfc_asym.value; } if (m_cmisModuleAsicSyncSupported) @@ -4152,7 +4190,7 @@ void PortsOrch::doPortTask(Consumer &consumer) if (pCfg.autoneg.is_set) { - if (!p.m_an_cfg || p.m_autoneg != pCfg.autoneg.value) + if (p.m_autoneg != pCfg.autoneg.value) { if (p.m_cap_an < 0) { @@ -4200,7 +4238,11 @@ void PortsOrch::doPortTask(Consumer &consumer) } continue; } + } + // First time config or when AN changes + if (!p.m_an_cfg || p.m_autoneg != pCfg.autoneg.value) + { p.m_autoneg = pCfg.autoneg.value; p.m_an_cfg = true; m_portList[p.m_alias] = p; @@ -4625,7 +4667,7 @@ void PortsOrch::doPortTask(Consumer &consumer) if (pCfg.fec.is_set) { /* reset fec mode upon mode change */ - if (!p.m_fec_cfg || p.m_fec_mode != pCfg.fec.value || p.m_override_fec != pCfg.fec.override_fec) + if (p.m_fec_mode != pCfg.fec.value || p.m_override_fec != pCfg.fec.override_fec) { if (!pCfg.fec.override_fec && !fec_override_sup) { @@ -4678,7 +4720,6 @@ void PortsOrch::doPortTask(Consumer &consumer) p.m_fec_mode = pCfg.fec.value; p.m_override_fec = pCfg.fec.override_fec; - p.m_fec_cfg = true; m_portList[p.m_alias] = p; SWSS_LOG_NOTICE( @@ -4686,6 +4727,14 @@ void PortsOrch::doPortTask(Consumer &consumer) p.m_alias.c_str(), m_portHlpr.getFecStr(pCfg).c_str() ); } + + // Sync geabox FEC on first time config + if (!p.m_fec_cfg) + { + setGearboxPortsAttr(p, SAI_PORT_ATTR_FEC_MODE, &pCfg.fec.value, pCfg.fec.override_fec); + p.m_fec_cfg = true; + m_portList[p.m_alias] = p; + } } if (pCfg.learn_mode.is_set) @@ -4715,7 +4764,7 @@ void PortsOrch::doPortTask(Consumer &consumer) if (pCfg.pfc_asym.is_set) { - if (!p.m_pfc_asym_cfg || p.m_pfc_asym != pCfg.pfc_asym.value) + if (p.m_pfc_asym != pCfg.pfc_asym.value) { if (m_portCap.isPortPfcAsymSupported()) { @@ -4730,7 +4779,6 @@ void PortsOrch::doPortTask(Consumer &consumer) } p.m_pfc_asym = pCfg.pfc_asym.value; - p.m_pfc_asym_cfg = true; m_portList[p.m_alias] = p; SWSS_LOG_NOTICE( diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp index 2dfee93b2e..a992f92dd0 100644 --- a/tests/mock_tests/portsorch_ut.cpp +++ b/tests/mock_tests/portsorch_ut.cpp @@ -39,6 +39,8 @@ namespace portsorch_test bool not_support_fetching_fec; uint32_t _sai_set_port_fec_count; + uint32_t _sai_set_port_auto_neg_count; + uint32_t _sai_set_port_tpid_count; int32_t _sai_port_fec_mode; vector mock_port_fec_modes = {SAI_PORT_FEC_MODE_RS, SAI_PORT_FEC_MODE_FC}; @@ -113,6 +115,7 @@ namespace portsorch_test } else if (attr[0].id == SAI_PORT_ATTR_AUTO_NEG_MODE) { + _sai_set_port_auto_neg_count++; /* Simulating failure case */ return SAI_STATUS_FAILURE; } @@ -164,6 +167,10 @@ namespace portsorch_test return SAI_STATUS_INVALID_ATTR_VALUE_0; } } + else if (attr[0].id == SAI_PORT_ATTR_TPID) + { + _sai_set_port_tpid_count++; + } else if (attr[0].id == SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGORITHM) { _sai_set_link_event_damping_algorithm_count++; @@ -915,6 +922,101 @@ namespace portsorch_test ASSERT_TRUE(keys.empty()); } + // Verifies certain port attributes are set on port creation, ensures no set API calls are made. + TEST_F(PortsOrchTest, PortAttributeSetOnCreation) + { + auto portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); + + const auto alias = "Ethernet0"; + + // Get SAI default ports + auto &ports = defaultPortList; + ASSERT_TRUE(!ports.empty()); + + // default Ethernet0 has different lanes so create_ports() is triggered + std::vector fvList = { + { "alias", alias }, + { "index", "0" }, + { "lanes", "0,1,2,3" }, + { "speed", "100000" }, + { "autoneg", "on" }, + { "adv_speeds", "all" }, + { "interface_type", "none" }, + { "adv_interface_types", "all" }, + { "fec", "rs" }, + { "mtu", "9100" }, + { "tpid", "0x8101" }, + { "pfc_asym", "on" }, + { "admin_status", "up" }, + { "description", "FP port" } + }; + + portTable.set(alias, fvList); + + // Set PortConfigDone + portTable.set("PortConfigDone", { { "count", std::to_string(ports.size()) } }); + + // Refill consumer + gPortsOrch->addExistingData(&portTable); + + const auto set_port_fec_count = _sai_set_port_fec_count; + const auto set_port_auto_neg_count = _sai_set_port_auto_neg_count; + const auto set_port_tpid_count = _sai_set_port_tpid_count; + const auto sai_set_pfc_mode_count = _sai_set_pfc_mode_count; + + _hook_sai_port_api(); + + // Apply configuration + static_cast(gPortsOrch)->doTask(); + + // Dump pending tasks + std::vector taskList; + gPortsOrch->dumpPendingTasks(taskList); + EXPECT_TRUE(taskList.empty()); + + _unhook_sai_port_api(); + + Port p; + EXPECT_TRUE(gPortsOrch->getPort(alias, p)); + + // Validate SAI port configuration + + sai_attribute_t attr; + + attr.id = SAI_PORT_ATTR_FEC_MODE; + EXPECT_EQ(SAI_STATUS_SUCCESS, sai_port_api->get_port_attribute(p.m_port_id, 1, &attr)); + EXPECT_EQ(attr.value.s32, SAI_PORT_FEC_MODE_RS); + + if (gPortsOrch->fec_override_sup) + { + attr.id = SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE; + EXPECT_EQ(SAI_STATUS_SUCCESS, sai_port_api->get_port_attribute(p.m_port_id, 1, &attr)); + EXPECT_FALSE(attr.value.booldata); + } + + attr.id = SAI_PORT_ATTR_AUTO_NEG_MODE; + EXPECT_EQ(SAI_STATUS_SUCCESS, sai_port_api->get_port_attribute(p.m_port_id, 1, &attr)); + EXPECT_TRUE(attr.value.booldata); + + attr.id = SAI_PORT_ATTR_TPID; + EXPECT_EQ(SAI_STATUS_SUCCESS, sai_port_api->get_port_attribute(p.m_port_id, 1, &attr)); + EXPECT_EQ(attr.value.u16, 0x8101); + + attr.id = SAI_PORT_ATTR_PRIORITY_FLOW_CONTROL_RX; + EXPECT_EQ(SAI_STATUS_SUCCESS, sai_port_api->get_port_attribute(p.m_port_id, 1, &attr)); + EXPECT_EQ(attr.value.u8, 0xff); + + // Validate no set API calls performed for specified attributes + + EXPECT_EQ(set_port_fec_count, _sai_set_port_fec_count); + EXPECT_EQ(set_port_auto_neg_count, _sai_set_port_auto_neg_count); + EXPECT_EQ(set_port_tpid_count, _sai_set_port_tpid_count); + EXPECT_EQ(sai_set_pfc_mode_count, _sai_set_pfc_mode_count); + + // Cleanup ports + cleanupPorts(gPortsOrch); + } + TEST_F(PortsOrchTest, PortBasicConfig) { auto portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); @@ -1218,7 +1320,8 @@ namespace portsorch_test consumer->addToSync(kfvSerdes); _hook_sai_port_api(); - uint32_t current_sai_api_call_count = _sai_set_admin_state_down_count; + uint32_t down_call_count = _sai_set_admin_state_down_count; + uint32_t up_call_count = _sai_set_admin_state_up_count; // Apply configuration static_cast(gPortsOrch)->doTask(); @@ -1233,8 +1336,8 @@ namespace portsorch_test ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_IDRIVER), idriver); // Verify admin-disable then admin-enable - ASSERT_EQ(_sai_set_admin_state_down_count, ++current_sai_api_call_count); - ASSERT_EQ(_sai_set_admin_state_up_count, current_sai_api_call_count); + ASSERT_EQ(_sai_set_admin_state_down_count, ++down_call_count); + ASSERT_EQ(_sai_set_admin_state_up_count, ++up_call_count); // Configure non-serdes attribute that does not trigger admin state change std::deque kfvMtu = {{ @@ -1248,7 +1351,7 @@ namespace portsorch_test consumer->addToSync(kfvMtu); _hook_sai_port_api(); - current_sai_api_call_count = _sai_set_admin_state_down_count; + down_call_count = _sai_set_admin_state_down_count; // Apply configuration static_cast(gPortsOrch)->doTask(); @@ -1262,8 +1365,8 @@ namespace portsorch_test ASSERT_EQ(p.m_mtu, 1234); // Verify no admin-disable then admin-enable - ASSERT_EQ(_sai_set_admin_state_down_count, current_sai_api_call_count); - ASSERT_EQ(_sai_set_admin_state_up_count, current_sai_api_call_count); + ASSERT_EQ(_sai_set_admin_state_down_count, down_call_count); + ASSERT_EQ(_sai_set_admin_state_up_count, up_call_count); // Dump pending tasks std::vector taskList; @@ -2471,7 +2574,7 @@ namespace portsorch_test entries.push_back({"Ethernet0", "SET", { - { "pfc_asym", "off"} + { "pfc_asym", "on"} }}); auto consumer = dynamic_cast(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME)); consumer->addToSync(entries);