Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 34 additions & 31 deletions orchagent/portsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4723,44 +4723,47 @@ void PortsOrch::doPortTask(Consumer &consumer)
}
if (p.m_cap_an < 1)
{
SWSS_LOG_ERROR("%s: autoneg is not supported (cap=%d)", p.m_alias.c_str(), p.m_cap_an);
// autoneg is not supported, don't retry
it = taskMap.erase(it);
continue;
/* autoneg not supported: log and continue applying remaining config (disable is a no-op; enable cannot be honored) */
SWSS_LOG_WARN("%s: autoneg is not supported (cap=%d), requested=%s",
p.m_alias.c_str(), p.m_cap_an,
pCfg.autoneg.value ? "on" : "off");
}
if (p.m_admin_state_up)
else
{
/* Bring port down before applying speed */
if (!setPortAdminStatus(p, false))
if (p.m_admin_state_up)
{
SWSS_LOG_ERROR(
"Failed to set port %s admin status DOWN to set port autoneg mode",
p.m_alias.c_str()
);
it++;
continue;
}

p.m_admin_state_up = false;
m_portList[p.m_alias] = p;
}
/* Bring port down before changing autoneg mode */
if (!setPortAdminStatus(p, false))
{
SWSS_LOG_ERROR(
"Failed to set port %s admin status DOWN to set port autoneg mode",
p.m_alias.c_str()
);
it++;
continue;
}

auto status = setPortAutoNeg(p, pCfg.autoneg.value);
if (status != task_success)
{
SWSS_LOG_ERROR(
"Failed to set port %s AN from %d to %d",
p.m_alias.c_str(), p.m_autoneg, pCfg.autoneg.value
);
if (status == task_need_retry)
{
it++;
p.m_admin_state_up = false;
m_portList[p.m_alias] = p;
}
else

auto status = setPortAutoNeg(p, pCfg.autoneg.value);
if (status != task_success)
{
it = taskMap.erase(it);
SWSS_LOG_ERROR(
"Failed to set port %s AN from %d to %d",
p.m_alias.c_str(), p.m_autoneg, pCfg.autoneg.value
);
if (status == task_need_retry)
{
it++;
}
else
{
it = taskMap.erase(it);
}
continue;
}
continue;
}

p.m_autoneg = pCfg.autoneg.value;
Expand Down
154 changes: 154 additions & 0 deletions tests/mock_tests/portsorch_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ namespace portsorch_test
bool not_support_fetching_fec;
uint32_t _sai_set_port_fec_count;
uint32_t _sai_set_port_auto_neg_count;
bool mock_autoneg_cap_supported = true;
bool set_autoneg_need_retry = false;
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 @@ -110,6 +112,11 @@ namespace portsorch_test
status = SAI_STATUS_SUCCESS;
}
}
else if (attr_count == 1 && attr_list[0].id == SAI_PORT_ATTR_SUPPORTED_AUTO_NEG_MODE)
{
attr_list[0].value.booldata = mock_autoneg_cap_supported;
status = SAI_STATUS_SUCCESS;
}
else
{
status = pold_sai_port_api->get_port_attribute(port_id, attr_count, attr_list);
Expand Down Expand Up @@ -153,6 +160,10 @@ namespace portsorch_test
else if (attr[0].id == SAI_PORT_ATTR_AUTO_NEG_MODE)
{
_sai_set_port_auto_neg_count++;
if (set_autoneg_need_retry)
{
return SAI_STATUS_INSUFFICIENT_RESOURCES;
}
/* Simulating failure case */
return SAI_STATUS_FAILURE;
}
Expand Down Expand Up @@ -3511,6 +3522,149 @@ namespace portsorch_test
_unhook_sai_switch_api();
}

TEST_F(PortsOrchTest, AutonegOnUnsupportedPort)
{
// Configuring autoneg on a port that doesn't support it (either on or off)
// must skip the ASIC call, mark the port configured so we don't retry, and
// not drop the rest of the task's config. The requested value is mirrored
// in cache.
_hook_sai_port_api();
mock_autoneg_cap_supported = false;

Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);
auto ports = ut_helper::getInitialSaiPorts();

for (const auto &it : ports)
{
portTable.set(it.first, it.second);
}
portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();

auto consumer = dynamic_cast<Consumer *>(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME));

for (const auto &requested : {std::string("off"), std::string("on")})
{
const auto an_call_count = _sai_set_port_auto_neg_count;

// Reset the port's AN-configured flag so each iteration re-enters the AN block
Port pre;
ASSERT_TRUE(gPortsOrch->getPort("Ethernet0", pre));
pre.m_an_cfg = false;
gPortsOrch->m_portList["Ethernet0"] = pre;

std::deque<KeyOpFieldsValuesTuple> entries;
entries.push_back({"Ethernet0", SET_COMMAND, { { "autoneg", requested } }});
consumer->addToSync(entries);
static_cast<Orch *>(gPortsOrch)->doTask();

// No SAI set call should have been made
EXPECT_EQ(_sai_set_port_auto_neg_count, an_call_count) << "requested=" << requested;

// Port marked configured; requested value reflected in cache
Port p;
ASSERT_TRUE(gPortsOrch->getPort("Ethernet0", p));
EXPECT_TRUE(p.m_an_cfg) << "requested=" << requested;
EXPECT_EQ(p.m_autoneg, requested == "on") << "requested=" << requested;

// Task consumed (no pending) — sibling config in the same task is no longer dropped
vector<string> ts;
gPortsOrch->dumpPendingTasks(ts);
EXPECT_TRUE(ts.empty()) << "requested=" << requested;
}

mock_autoneg_cap_supported = true;
_unhook_sai_port_api();
}

TEST_F(PortsOrchTest, AutonegAdminDownFailure)
{
// When autoneg is configured on a port that is admin UP and bringing the
// port DOWN fails, the task must be retried (it++, not erased).
_hook_sai_port_api();

Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);
auto ports = ut_helper::getInitialSaiPorts();

for (const auto &it : ports)
{
portTable.set(it.first, it.second);
}
portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();

// Force Ethernet0 into admin UP state
Port p;
ASSERT_TRUE(gPortsOrch->getPort("Ethernet0", p));
p.m_admin_state_up = true;
gPortsOrch->m_portList["Ethernet0"] = p;

set_admin_status_fail = true;

std::deque<KeyOpFieldsValuesTuple> entries;
entries.push_back({"Ethernet0", SET_COMMAND,
{
{ "autoneg", "on" }
}});
auto consumer = dynamic_cast<Consumer *>(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME));
consumer->addToSync(entries);
static_cast<Orch *>(gPortsOrch)->doTask();

EXPECT_EQ(set_admin_status_failures, 1);

// Task should remain queued for retry
vector<string> ts;
gPortsOrch->dumpPendingTasks(ts);
EXPECT_FALSE(ts.empty());

set_admin_status_fail = false;
set_admin_status_failures = 0;
_unhook_sai_port_api();
}

TEST_F(PortsOrchTest, AutonegSetNeedsRetry)
{
// When setPortAutoNeg returns task_need_retry (SAI_STATUS_INSUFFICIENT_RESOURCES),
// the task must stay queued (it++) rather than being dropped.
_hook_sai_port_api();
set_autoneg_need_retry = true;

Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);
auto ports = ut_helper::getInitialSaiPorts();

for (const auto &it : ports)
{
portTable.set(it.first, it.second);
}
portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } });
gPortsOrch->addExistingData(&portTable);
static_cast<Orch *>(gPortsOrch)->doTask();

const auto an_call_count = _sai_set_port_auto_neg_count;

std::deque<KeyOpFieldsValuesTuple> entries;
entries.push_back({"Ethernet0", SET_COMMAND,
{
{ "autoneg", "on" }
}});
auto consumer = dynamic_cast<Consumer *>(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME));
consumer->addToSync(entries);
static_cast<Orch *>(gPortsOrch)->doTask();

// SAI set was attempted
EXPECT_GT(_sai_set_port_auto_neg_count, an_call_count);

// Task should remain queued for retry
vector<string> ts;
gPortsOrch->dumpPendingTasks(ts);
EXPECT_FALSE(ts.empty());

set_autoneg_need_retry = false;
_unhook_sai_port_api();
}

TEST_F(PortsOrchTest, PortReadinessColdBoot)
{
Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);
Expand Down
Loading