Skip to content

Commit

Permalink
[portsorch] Portsorch simple improvements (sonic-net#718)
Browse files Browse the repository at this point in the history 
* [portsorch] throw runtime_error object instead of const char*
* [portsorch] simple improvements for portsorch
 - Change 'p' varaible name to 'port'
 - Pass 'Port' struct object to set/update methods instead of SAI OID to avoid
   unncessary 'for' loops that search port object in m_portList
 - minor simple changes
* [portsorch]] address comments
* [portsorch] make setHostIntfsOperStatus/updateDbPortOperStatus const
  • Loading branch information
@stepanblyschak @qiluo-msft
stepanblyschak authored and qiluo-msft committed Dec 3, 2018
1 parent 95c3739 commit 3d60b3e
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 95 deletions.
2 changes: 1 addition & 1 deletion orchagent/neighorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ bool NeighOrch::addNextHop(IpAddress ipAddress, string alias)
// flag Should be set on it.
// This scenario may happen under race condition where buffered neighbor event
// is processed after incoming port is down.
if (p.m_oper_status == SAI_PORT_OPER_STATUS_DOWN)
if (p.m_oper_status != SAI_PORT_OPER_STATUS_UP)
{
if (setNextHopFlag(ipAddress, NHFLAGS_IFDOWN) == false)
{
Expand Down
2 changes: 1 addition & 1 deletion orchagent/port.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class Port
sai_object_id_t m_ingress_acl_table_group_id = 0;
sai_object_id_t m_egress_acl_table_group_id = 0;
vlan_members_t m_vlan_members;
sai_port_oper_status_t m_oper_status;
sai_port_oper_status_t m_oper_status = SAI_PORT_OPER_STATUS_UNKNOWN;
std::set<std::string> m_members;
std::vector<sai_object_id_t> m_queue_ids;
std::vector<sai_object_id_t> m_priority_group_ids;
Expand Down
197 changes: 106 additions & 91 deletions orchagent/portsorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ PortsOrch::PortsOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames)
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get CPU port, rv:%d", status);
throw "PortsOrch initialization failure";
throw runtime_error("PortsOrch initialization failure");
}

m_cpuPort = Port("CPU", Port::CPU);
Expand All @@ -228,7 +228,7 @@ PortsOrch::PortsOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames)
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get port number, rv:%d", status);
throw "PortsOrch initialization failure";
throw runtime_error("PortsOrch initialization failure");
}

m_portCount = attr.value.u32;
Expand All @@ -246,7 +246,7 @@ PortsOrch::PortsOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames)
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get port list, rv:%d", status);
throw "PortsOrch initialization failure";
throw runtime_error("PortsOrch initialization failure");
}

/* Get port hardware lane info */
Expand All @@ -261,7 +261,7 @@ PortsOrch::PortsOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames)
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get hardware lane list pid:%lx", port_list[i]);
throw "PortsOrch initialization failure";
throw runtime_error("PortsOrch initialization failure");
}

set<int> tmp_lane_set;
Expand Down Expand Up @@ -290,7 +290,7 @@ PortsOrch::PortsOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames)
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get default 1Q bridge and/or default VLAN, rv:%d", status);
throw "PortsOrch initialization failure";
throw runtime_error("PortsOrch initialization failure");
}

m_default1QBridge = attrs[0].value.oid;
Expand Down Expand Up @@ -320,7 +320,7 @@ void PortsOrch::removeDefaultVlanMembers()
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get VLAN member list in default VLAN, rv:%d", status);
throw "PortsOrch initialization failure";
throw runtime_error("PortsOrch initialization failure");
}

/* Remove VLAN members in default VLAN */
Expand All @@ -330,7 +330,7 @@ void PortsOrch::removeDefaultVlanMembers()
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to remove VLAN member, rv:%d", status);
throw "PortsOrch initialization failure";
throw runtime_error("PortsOrch initialization failure");
}
}

Expand All @@ -354,7 +354,7 @@ void PortsOrch::removeDefaultBridgePorts()
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get bridge port list in default 1Q bridge, rv:%d", status);
throw "PortsOrch initialization failure";
throw runtime_error("PortsOrch initialization failure");
}

auto bridge_port_count = attr.value.objlist.count;
Expand All @@ -369,15 +369,15 @@ void PortsOrch::removeDefaultBridgePorts()
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get bridge port type, rv:%d", status);
throw "PortsOrch initialization failure";
throw runtime_error("PortsOrch initialization failure");
}
if (attr.value.s32 == SAI_BRIDGE_PORT_TYPE_PORT)
{
status = sai_bridge_api->remove_bridge_port(bridge_port_list[i]);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to remove bridge port, rv:%d", status);
throw "PortsOrch initialization failure";
throw runtime_error("PortsOrch initialization failure");
}
}
}
Expand Down Expand Up @@ -1168,55 +1168,36 @@ bool PortsOrch::setPortAutoNeg(sai_object_id_t id, int an)
return true;
}

bool PortsOrch::setHostIntfsOperStatus(sai_object_id_t port_id, bool up)
bool PortsOrch::setHostIntfsOperStatus(const Port& port, bool isUp) const
{
SWSS_LOG_ENTER();

for (auto it = m_portList.begin(); it != m_portList.end(); it++)
sai_attribute_t attr;
attr.id = SAI_HOSTIF_ATTR_OPER_STATUS;
attr.value.booldata = isUp;

sai_status_t status = sai_hostif_api->set_hostif_attribute(port.m_hif_id, &attr);
if (status != SAI_STATUS_SUCCESS)
{
if (it->second.m_port_id != port_id)
{
continue;
}
SWSS_LOG_WARN("Failed to set operation status %s to host interface %s",
isUp ? "UP" : "DOWN", port.m_alias.c_str());
return false;
}

sai_attribute_t attr;
attr.id = SAI_HOSTIF_ATTR_OPER_STATUS;
attr.value.booldata = up;
SWSS_LOG_NOTICE("Set operation status %s to host interface %s",
isUp ? "UP" : "DOWN", port.m_alias.c_str());

sai_status_t status = sai_hostif_api->set_hostif_attribute(it->second.m_hif_id, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_WARN("Failed to set operation status %s to host interface %s",
up ? "UP" : "DOWN", it->second.m_alias.c_str());
return false;
}
SWSS_LOG_NOTICE("Set operation status %s to host interface %s",
up ? "UP" : "DOWN", it->second.m_alias.c_str());
if (gNeighOrch->ifChangeInformNextHop(it->second.m_alias, up) == false)
{
SWSS_LOG_WARN("Inform nexthop operation failed for interface %s",
it->second.m_alias.c_str());
}
return true;
}
return false;
return true;
}

void PortsOrch::updateDbPortOperStatus(sai_object_id_t id, sai_port_oper_status_t status)
void PortsOrch::updateDbPortOperStatus(const Port& port, sai_port_oper_status_t status) const
{
SWSS_LOG_ENTER();

for (auto it = m_portList.begin(); it != m_portList.end(); it++)
{
if (it->second.m_port_id == id)
{
vector<FieldValueTuple> tuples;
FieldValueTuple tuple("oper_status", oper_status_strings.at(status));
tuples.push_back(tuple);
m_portTable->set(it->first, tuples);
it->second.m_oper_status = status;
}
}
vector<FieldValueTuple> tuples;
FieldValueTuple tuple("oper_status", oper_status_strings.at(status));
tuples.push_back(tuple);
m_portTable->set(port.m_alias, tuples);
}

bool PortsOrch::addPort(const set<int> &lane_set, uint32_t speed, int an, string fec_mode)
Expand Down Expand Up @@ -1481,6 +1462,7 @@ void PortsOrch::doPortTask(Consumer &consumer)

it = consumer.m_toSync.erase(it);
return;

}

if (op == SET_COMMAND)
Expand Down Expand Up @@ -2344,69 +2326,64 @@ void PortsOrch::initializePriorityGroups(Port &port)
SWSS_LOG_INFO("Get priority groups for port %s", port.m_alias.c_str());
}

bool PortsOrch::initializePort(Port &p)
bool PortsOrch::initializePort(Port &port)
{
SWSS_LOG_ENTER();

SWSS_LOG_NOTICE("Initializing port alias:%s pid:%lx", p.m_alias.c_str(), p.m_port_id);
SWSS_LOG_NOTICE("Initializing port alias:%s pid:%lx", port.m_alias.c_str(), port.m_port_id);

initializePriorityGroups(p);
initializeQueues(p);
initializePriorityGroups(port);
initializeQueues(port);

/* Create host interface */
addHostIntfs(p, p.m_alias, p.m_hif_id);
if (!addHostIntfs(port, port.m_alias, port.m_hif_id))
{
SWSS_LOG_ERROR("Failed to create host interface for port %s", port.m_alias.c_str());
return false;
}

/* Check warm start states */
vector<FieldValueTuple> tuples;
bool exist = m_portTable->get(p.m_alias, tuples);
string adminStatus, operStatus;
bool exist = m_portTable->get(port.m_alias, tuples);
string operStatus;
if (exist)
{
for (auto i : tuples)
{
if (fvField(i) == "admin_status")
{
adminStatus = fvValue(i);
}
else if (fvField(i) == "oper_status")
if (fvField(i) == "oper_status")
{
operStatus = fvValue(i);
}
}
}
SWSS_LOG_DEBUG("initializePort %s with admin %s and oper %s", p.m_alias.c_str(), adminStatus.c_str(), operStatus.c_str());
SWSS_LOG_DEBUG("initializePort %s with oper %s", port.m_alias.c_str(), operStatus.c_str());

/**
* Create database port oper status as DOWN if attr missing
* This status will be updated upon receiving port_oper_status_notification.
*/
if (operStatus == "up")
{
p.m_oper_status = SAI_PORT_OPER_STATUS_UP;
port.m_oper_status = SAI_PORT_OPER_STATUS_UP;
}
else if (operStatus.empty())
{
p.m_oper_status = SAI_PORT_OPER_STATUS_DOWN;
port.m_oper_status = SAI_PORT_OPER_STATUS_DOWN;
/* Fill oper_status in db with default value "down" */
m_portTable->hset(p.m_alias, "oper_status", "down");
m_portTable->hset(port.m_alias, "oper_status", "down");
}
else
{
p.m_oper_status = SAI_PORT_OPER_STATUS_DOWN;
port.m_oper_status = SAI_PORT_OPER_STATUS_DOWN;
}

/*
* always initialize Port SAI_HOSTIF_ATTR_OPER_STATUS based on oper_status value in appDB.
*/
sai_attribute_t attr;
attr.id = SAI_HOSTIF_ATTR_OPER_STATUS;
attr.value.booldata = (p.m_oper_status == SAI_PORT_OPER_STATUS_UP);

sai_status_t status = sai_hostif_api->set_hostif_attribute(p.m_hif_id, &attr);
if (status != SAI_STATUS_SUCCESS)
if (!setHostIntfsOperStatus(port, port.m_oper_status))
{
SWSS_LOG_WARN("Failed to set operation status %s to host interface %s",
operStatus.c_str(), p.m_alias.c_str());
operStatus.c_str(), port.m_alias.c_str());
return false;
}

Expand Down Expand Up @@ -3100,12 +3077,17 @@ void PortsOrch::doTask(NotificationConsumer &consumer)
SWSS_LOG_NOTICE("Get port state change notification id:%lx status:%d", id, status);

Port port;

if (!getPort(id, port))
{
SWSS_LOG_ERROR("Failed to get port object for port id 0x%lx", id);
continue;
}

updatePortOperStatus(port, status);

/* update m_portList */
m_portList[port.m_alias] = port;
}

sai_deserialize_free_port_oper_status_ntf(count, portoperstatus);
Expand All @@ -3117,13 +3099,23 @@ void PortsOrch::updatePortOperStatus(Port &port, sai_port_oper_status_t status)
SWSS_LOG_NOTICE("Port %s oper state set from %s to %s",
port.m_alias.c_str(), oper_status_strings.at(port.m_oper_status).c_str(),
oper_status_strings.at(status).c_str());
if (status != port.m_oper_status)
if (status == port.m_oper_status)
{
this->updateDbPortOperStatus(port.m_port_id, status);
if (status == SAI_PORT_OPER_STATUS_UP || port.m_oper_status == SAI_PORT_OPER_STATUS_UP)
{
this->setHostIntfsOperStatus(port.m_port_id, status == SAI_PORT_OPER_STATUS_UP);
}
return ;
}

updateDbPortOperStatus(port, status);
port.m_oper_status = status;

bool isUp = status == SAI_PORT_OPER_STATUS_UP;
if (!setHostIntfsOperStatus(port, isUp))
{
SWSS_LOG_ERROR("Failed to set host interface %s operational status %s", port.m_alias.c_str(),
isUp ? "up" : "down");
}
if (!gNeighOrch->ifChangeInformNextHop(port.m_alias, isUp))
{
SWSS_LOG_WARN("Inform nexthop operation failed for interface %s", port.m_alias.c_str());
}
}

Expand All @@ -3144,21 +3136,44 @@ void PortsOrch::refreshPortStatus()

for (auto &it: m_portList)
{
auto &p = it.second;
if (p.m_type == Port::PHY)
auto &port = it.second;
if (port.m_type != Port::PHY)
{
sai_attribute_t attr;
attr.id = SAI_PORT_ATTR_OPER_STATUS;
continue;
}

sai_status_t ret = sai_port_api->get_port_attribute(p.m_port_id, 1, &attr);
if (ret != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get oper status for %s", p.m_alias.c_str());
throw "PortsOrch get port oper status failure";
}
sai_port_oper_status_t status = (sai_port_oper_status_t)attr.value.u32;
SWSS_LOG_INFO("%s oper status is %s", p.m_alias.c_str(), oper_status_strings.at(status).c_str());
updatePortOperStatus(p, status);
sai_port_oper_status_t status;
if (!getPortOperStatus(port, status))
{
throw runtime_error("PortsOrch get port oper status failure");
}

SWSS_LOG_INFO("%s oper status is %s", port.m_alias.c_str(), oper_status_strings.at(status).c_str());
updatePortOperStatus(port, status);
}
}

bool PortsOrch::getPortOperStatus(const Port& port, sai_port_oper_status_t& status) const
{
SWSS_LOG_ENTER();

if (port.m_type != Port::PHY)
{
return false;
}

sai_attribute_t attr;
attr.id = SAI_PORT_ATTR_OPER_STATUS;

sai_status_t ret = sai_port_api->get_port_attribute(port.m_port_id, 1, &attr);
if (ret != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get oper_status for %s", port.m_alias.c_str());
return false;
}

status = static_cast<sai_port_oper_status_t>(attr.value.u32);

return true;
}

Loading

0 comments on commit 3d60b3e

Please sign in to comment.