Skip to content

Commit

Permalink
Configure attributes on create
Browse files Browse the repository at this point in the history
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
  • Loading branch information
stepanblyschak committed Feb 14, 2025
1 parent c44f630 commit 39f1342
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 13 deletions.
1 change: 0 additions & 1 deletion orchagent/port.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
59 changes: 54 additions & 5 deletions orchagent/portsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1056,13 +1056,51 @@ bool PortsOrch::addPortBulk(const std::vector<PortConfig> &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)
{
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<uint8_t>(0xff);
attrList.push_back(attr);
}

p.m_pfc_asym = cit.pfc_asym.value;
}

if (m_cmisModuleAsicSyncSupported)
Expand Down Expand Up @@ -3241,6 +3279,11 @@ task_process_status PortsOrch::setPortAutoNeg(Port &port, bool autoneg)
{
SWSS_LOG_ENTER();

if (port.m_autoneg == autoneg)
{
return task_success;
}

sai_attribute_t attr;
attr.id = SAI_PORT_ATTR_AUTO_NEG_MODE;
attr.value.booldata = autoneg;
Expand All @@ -3251,7 +3294,9 @@ task_process_status PortsOrch::setPortAutoNeg(Port &port, bool autoneg)
SWSS_LOG_ERROR("Failed to set AutoNeg %u to port %s", attr.value.booldata, port.m_alias.c_str());
return handleSaiSetStatus(SAI_API_PORT, status);
}
SWSS_LOG_INFO("Set AutoNeg %u to port %s", attr.value.booldata, port.m_alias.c_str());

SWSS_LOG_INFO("Set AutoNeg %u to port %s", autoneg, port.m_alias.c_str());

return task_success;
}

Expand Down Expand Up @@ -4625,7 +4670,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)
{
Expand Down Expand Up @@ -4678,14 +4723,19 @@ 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(
"Set port %s FEC mode to %s",
p.m_alias.c_str(), m_portHlpr.getFecStr(pCfg).c_str()
);
}

if (!p.m_fec_cfg)
{
setGearboxPortsAttr(p, SAI_PORT_ATTR_FEC_MODE, &pCfg.fec.value, pCfg.fec.override_fec);
p.m_fec_cfg = true;
}
}

if (pCfg.learn_mode.is_set)
Expand Down Expand Up @@ -4715,7 +4765,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())
{
Expand All @@ -4730,7 +4780,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(
Expand Down
117 changes: 110 additions & 7 deletions tests/mock_tests/portsorch_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<sai_port_fec_mode_t> mock_port_fec_modes = {SAI_PORT_FEC_MODE_RS, SAI_PORT_FEC_MODE_FC};

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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++;
Expand Down Expand Up @@ -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<FieldValueTuple> 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<Orch*>(gPortsOrch)->doTask();

// Dump pending tasks
std::vector<std::string> 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);
Expand Down Expand Up @@ -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<Orch*>(gPortsOrch)->doTask();
Expand All @@ -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<KeyOpFieldsValuesTuple> kfvMtu = {{
Expand All @@ -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<Orch*>(gPortsOrch)->doTask();
Expand All @@ -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<std::string> taskList;
Expand Down Expand Up @@ -2471,7 +2574,7 @@ namespace portsorch_test

entries.push_back({"Ethernet0", "SET",
{
{ "pfc_asym", "off"}
{ "pfc_asym", "on"}
}});
auto consumer = dynamic_cast<Consumer *>(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME));
consumer->addToSync(entries);
Expand Down

0 comments on commit 39f1342

Please sign in to comment.