Skip to content

[portsorch] Portsorch simple improvements #718

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

Merged
qiluo-msft merged 4 commits into from
Dec 3, 2018
Merged

[portsorch] Portsorch simple improvements #718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
56abd0f
[portsorch] throw runtime_error object instead of const char*
Nov 22, 2018
fc46ea0
[portsorch] simple improvements for portsorch
Nov 22, 2018
fffc884
[portsorch]] address comments
Dec 3, 2018
8430def
[portsorch] make setHostIntfsOperStatus/updateDbPortOperStatus const
Dec 3, 2018
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
2 changes: 1 addition & 1 deletion orchagent/neighorch.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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