Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[portsorch] remove port OID from saiOidToAlias map on port deletion #2483

Merged
merged 1 commit into from
Oct 25, 2022
Merged

[portsorch] remove port OID from saiOidToAlias map on port deletion #2483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 20 additions & 16 deletions orchagent/portsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,10 @@ bool PortsOrch::getPort(sai_object_id_t id, Port &port)
}
else
{
getPort(itr->second, port);
if (!getPort(itr->second, port))
{
SWSS_LOG_THROW("Inconsistent saiOidToAlias map and m_portList map: oid=%" PRIx64, id);
}
return true;
}

Expand Down Expand Up @@ -1066,9 +1069,9 @@ void PortsOrch::getCpuPort(Port &port)
port = m_cpuPort;
}

/*
* Create host_tx_ready field in PORT_TABLE of STATE-DB
* and set the field to false by default for the
/*
* Create host_tx_ready field in PORT_TABLE of STATE-DB
* and set the field to false by default for the
* front<Ethernet> port.
*/
void PortsOrch::initHostTxReadyState(Port &port)
Expand All @@ -1093,7 +1096,7 @@ void PortsOrch::initHostTxReadyState(Port &port)
if (hostTxReady.empty())
{
m_portStateTable.hset(port.m_alias, "host_tx_ready", "false");
SWSS_LOG_INFO("initalize hostTxReady %s with status %s",
SWSS_LOG_INFO("initalize hostTxReady %s with status %s",
port.m_alias.c_str(), hostTxReady.c_str());
}
}
Expand All @@ -1107,13 +1110,13 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state)
attr.value.booldata = state;

/* Update the host_tx_ready to false before setting admin_state, when admin state is false */
if (!state)
if (!state)
{
m_portStateTable.hset(port.m_alias, "host_tx_ready", "false");
SWSS_LOG_INFO("Set admin status DOWN host_tx_ready to false to port pid:%" PRIx64,
port.m_port_id);
}

sai_status_t status = sai_port_api->set_port_attribute(port.m_port_id, &attr);
if (status != SAI_STATUS_SUCCESS)
{
Expand All @@ -1132,14 +1135,14 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state)
{
m_portStateTable.hset(port.m_alias, "host_tx_ready", "false");
}

/* Update the state table for host_tx_ready*/
if (state && (gbstatus == true) && (status == SAI_STATUS_SUCCESS) )
{
m_portStateTable.hset(port.m_alias, "host_tx_ready", "true");
SWSS_LOG_INFO("Set admin status UP host_tx_ready to true to port pid:%" PRIx64,
port.m_port_id);
}
}

return true;
}
Expand Down Expand Up @@ -1373,9 +1376,9 @@ bool PortsOrch::setPortPfcWatchdogStatus(sai_object_id_t portId, uint8_t pfcwd_b
SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, portId);
return false;
}

p.m_pfcwd_sw_bitmask = pfcwd_bitmask;

m_portList[p.m_alias] = p;

SWSS_LOG_INFO("Set PFC watchdog port id=0x%" PRIx64 ", bitmast=0x%x", portId, pfcwd_bitmask);
Expand All @@ -1393,9 +1396,9 @@ bool PortsOrch::getPortPfcWatchdogStatus(sai_object_id_t portId, uint8_t *pfcwd_
SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, portId);
return false;
}

*pfcwd_bitmask = p.m_pfcwd_sw_bitmask;

return true;
}

Expand Down Expand Up @@ -3772,10 +3775,10 @@ void PortsOrch::doPortTask(Consumer &consumer)
continue;
}
}

/* create host_tx_ready field in state-db */
initHostTxReadyState(p);

/* Last step set port admin status */
if (!admin_status.empty() && (p.m_admin_state_up != (admin_status == "up")))
{
Expand Down Expand Up @@ -3853,6 +3856,7 @@ void PortsOrch::doPortTask(Consumer &consumer)

/* Delete port from port list */
m_portList.erase(alias);
saiOidToAlias.erase(port_id);
}
else
{
Expand Down Expand Up @@ -6277,7 +6281,7 @@ void PortsOrch::doTask(NotificationConsumer &consumer)

if (!getPort(id, port))
{
SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, id);
SWSS_LOG_NOTICE("Got port state change for port id 0x%" PRIx64 " which does not exist, possibly outdated event", id);
continue;
}

Expand Down
56 changes: 48 additions & 8 deletions tests/mock_tests/portsorch_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,16 @@ namespace portsorch_test
{
sai_port_api = pold_sai_port_api;
}

sai_queue_api_t ut_sai_queue_api;
sai_queue_api_t *pold_sai_queue_api;
int _sai_set_queue_attr_count = 0;

sai_status_t _ut_stub_sai_set_queue_attribute(sai_object_id_t queue_id, const sai_attribute_t *attr)
{
if(attr->id == SAI_QUEUE_ATTR_PFC_DLR_INIT)
{
if(attr->value.booldata == true)
if(attr->value.booldata == true)
{
_sai_set_queue_attr_count++;
}
Expand Down Expand Up @@ -269,6 +269,46 @@ namespace portsorch_test

};

/**
* Test that verifies PortsOrch::getPort() on a port that has been deleted
*/
TEST_F(PortsOrchTest, GetPortTest)
{
Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);
std::deque<KeyOpFieldsValuesTuple> entries;

// Get SAI default ports to populate DB
auto ports = ut_helper::getInitialSaiPorts();

for (const auto &it : ports)
{
portTable.set(it.first, it.second);
}

// Set PortConfigDone
portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } });

// refill consumer
gPortsOrch->addExistingData(&portTable);

// Apply configuration :
// create ports
static_cast<Orch *>(gPortsOrch)->doTask();

Port port;
ASSERT_TRUE(gPortsOrch->getPort("Ethernet0", port));
ASSERT_NE(port.m_port_id, SAI_NULL_OBJECT_ID);

// Delete port
entries.push_back({"Ethernet0", "DEL", {}});
auto consumer = dynamic_cast<Consumer *>(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME));
consumer->addToSync(entries);
static_cast<Orch *>(gPortsOrch)->doTask();
entries.clear();

ASSERT_FALSE(gPortsOrch->getPort(port.m_port_id, port));
}

TEST_F(PortsOrchTest, PortSupportedFecModes)
{
_hook_sai_port_api();
Expand Down Expand Up @@ -892,7 +932,7 @@ namespace portsorch_test
* updated to DB.
*/
TEST_F(PortsOrchTest, PortOperStatusIsUpAndOperSpeedIsZero)
{
{
Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);

// Get SAI default ports to populate DB
Expand All @@ -917,7 +957,7 @@ namespace portsorch_test
Port port;
gPortsOrch->getPort("Ethernet0", port);
ASSERT_TRUE(port.m_oper_status != SAI_PORT_OPER_STATUS_UP);

// save original api since we will spy
auto orig_port_api = sai_port_api;
sai_port_api = new sai_port_api_t();
Expand All @@ -935,14 +975,14 @@ namespace portsorch_test
// Return 0 for port operational speed
attrs[0].value.u32 = 0;
}

return (sai_status_t)SAI_STATUS_SUCCESS;
}
);

auto exec = static_cast<Notifier *>(gPortsOrch->getExecutor("PORT_STATUS_NOTIFICATIONS"));
auto consumer = exec->getNotificationConsumer();

// mock a redis reply for notification, it notifies that Ehernet0 is going to up
mockReply = (redisReply *)calloc(sizeof(redisReply), 1);
mockReply->type = REDIS_REPLY_ARRAY;
Expand All @@ -964,7 +1004,7 @@ namespace portsorch_test
// trigger the notification
consumer->readData();
gPortsOrch->doTask(*consumer);
mockReply = nullptr;
mockReply = nullptr;

gPortsOrch->getPort("Ethernet0", port);
ASSERT_TRUE(port.m_oper_status == SAI_PORT_OPER_STATUS_UP);
Expand Down