Skip to content

Port serdes changes #1611

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
daall merged 9 commits into from
Feb 9, 2021
Merged

Port serdes changes #1611

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
830409a
Port serdes changes
Jan 19, 2021
ce6b2d9
Addressing code review comments
dgsudharsan Jan 27, 2021
1ff5675
Fixing build due to errors in logs
dgsudharsan Jan 27, 2021
6d9c8f4
Addressing code review comments
dgsudharsan Jan 27, 2021
db00b00
Merge branch 'master' of https://github.com/Azure/sonic-swss into sixtap
dgsudharsan Jan 28, 2021
bfb04c1
Optimizing the parsing of fields
dgsudharsan Jan 30, 2021
accc403
Added comments
dgsudharsan Feb 1, 2021
6cf96c5
Removing the error log as per comments
dgsudharsan Feb 2, 2021
d14d9e0
Merge branch 'master' of https://github.com/Azure/sonic-swss into sixtap
dgsudharsan Feb 5, 2021
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
227 changes: 156 additions & 71 deletions orchagent/portsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1944,6 +1944,9 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id)
/* remove port name map from counter table */
m_counter_db->hdel(COUNTERS_PORT_NAME_MAP, alias);

/* Remove the associated port serdes attribute */
removePortSerdesAttribute(p.m_port_id);

m_portList[alias].m_init = false;
SWSS_LOG_NOTICE("De-Initialized port %s", alias.c_str());
}
Expand Down Expand Up @@ -2102,9 +2105,9 @@ void PortsOrch::doPortTask(Consumer &consumer)
if (op == SET_COMMAND)
{
set<int> lane_set;
vector<uint32_t> pre_emphasis;
vector<uint32_t> idriver;
vector<uint32_t> ipredriver;
vector<uint32_t> attr_val;
map<sai_port_serdes_attr_t, vector<uint32_t>> serdes_attr;
typedef pair<sai_port_serdes_attr_t, vector<uint32_t>> serdes_attr_pair;
string admin_status;
string fec_mode;
string pfc_asym;
Expand All @@ -2116,14 +2119,14 @@ void PortsOrch::doPortTask(Consumer &consumer)

for (auto i : kfvFieldsValues(t))
{
attr_val.clear();
/* Set interface index */
if (fvField(i) == "index")
{
index = (int)stoul(fvValue(i));
}

/* Get lane information of a physical port and initialize the port */
if (fvField(i) == "lanes")
else if (fvField(i) == "lanes")
{
string lane_str;
istringstream iss(fvValue(i));
Expand All @@ -2133,65 +2136,111 @@ void PortsOrch::doPortTask(Consumer &consumer)
int lane = stoi(lane_str);
lane_set.insert(lane);
}

}

/* Set port admin status */
if (fvField(i) == "admin_status")
else if (fvField(i) == "admin_status")
{
admin_status = fvValue(i);
}

/* Set port MTU */
if (fvField(i) == "mtu")
else if (fvField(i) == "mtu")
{
mtu = (uint32_t)stoul(fvValue(i));
}

/* Set port speed */
if (fvField(i) == "speed")
else if (fvField(i) == "speed")
{
speed = (uint32_t)stoul(fvValue(i));
}

/* Set port fec */
if (fvField(i) == "fec")
else if (fvField(i) == "fec")
{
fec_mode = fvValue(i);
}

/* Get port fdb learn mode*/
if (fvField(i) == "learn_mode")
else if (fvField(i) == "learn_mode")
{
learn_mode = fvValue(i);
}

/* Set port asymmetric PFC */
if (fvField(i) == "pfc_asym")
else if (fvField(i) == "pfc_asym")
{
pfc_asym = fvValue(i);

}
/* Set autoneg and ignore the port speed setting */
if (fvField(i) == "autoneg")
else if (fvField(i) == "autoneg")
{
an = (int)stoul(fvValue(i));
}

/* Set port serdes Pre-emphasis */
if (fvField(i) == "preemphasis")
else if (fvField(i) == "preemphasis")
{
getPortSerdesVal(fvValue(i), pre_emphasis);
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_PREEMPHASIS, attr_val));
}

/* Set port serdes idriver */
if (fvField(i) == "idriver")
else if (fvField(i) == "idriver")
{
getPortSerdesVal(fvValue(i), idriver);
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_IDRIVER, attr_val));
}

/* Set port serdes ipredriver */
if (fvField(i) == "ipredriver")
else if (fvField(i) == "ipredriver")
{
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_IPREDRIVER, attr_val));
}
/* Set port serdes pre1 */
else if (fvField(i) == "pre1")
{
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_TX_FIR_PRE1, attr_val));
}
/* Set port serdes pre2 */
else if (fvField(i) == "pre2")
{
getPortSerdesVal(fvValue(i), ipredriver);
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_TX_FIR_PRE2, attr_val));
}
/* Set port serdes pre3 */
else if (fvField(i) == "pre3")
{
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_TX_FIR_PRE3, attr_val));
}
/* Set port serdes main */
else if (fvField(i) == "main")
{
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_TX_FIR_MAIN, attr_val));
}
/* Set port serdes post1 */
else if (fvField(i) == "post1")
{
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_TX_FIR_POST1, attr_val));
}
/* Set port serdes post2 */
else if (fvField(i) == "post2")
{
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_TX_FIR_POST2, attr_val));
}
/* Set port serdes post3 */
else if (fvField(i) == "post3")
{
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_TX_FIR_POST3, attr_val));
}
/* Set port serdes attn */
else if (fvField(i) == "attn")
{
getPortSerdesVal(fvValue(i), attr_val);
serdes_attr.insert(serdes_attr_pair(SAI_PORT_SERDES_ATTR_TX_FIR_ATTN, attr_val));
}
else
{
SWSS_LOG_ERROR("Unknown port field: %s", fvField(i).c_str());
Copy link
Copy Markdown
Contributor

@daall daall Feb 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CRM tests are failing b/c the syslog is getting throttled due to this message:

Feb  2 23:46:10.029687 9a649eaefbe1 ERR #orchagent: message repeated 4793 times: [ :- doPortTask: Unknown port field: alias]

Do we need to add/skip "alias" here?

Copy link
Copy Markdown
Collaborator Author

@dgsudharsan dgsudharsan Feb 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove this as there might be other fields that could be added in the future which may not be significant for the SAI conversion.

}
}

Expand Down Expand Up @@ -2512,9 +2561,9 @@ void PortsOrch::doPortTask(Consumer &consumer)
}
}

if (pre_emphasis.size() != 0)
if (serdes_attr.size() != 0)
{
if (setPortSerdesAttribute(p.m_port_id, SAI_PORT_ATTR_SERDES_PREEMPHASIS, pre_emphasis))
if (setPortSerdesAttribute(p.m_port_id, serdes_attr))
{
SWSS_LOG_NOTICE("Set port %s preemphasis is success", alias.c_str());
}
Expand All @@ -2527,36 +2576,6 @@ void PortsOrch::doPortTask(Consumer &consumer)

}

if (idriver.size() != 0)
{
if (setPortSerdesAttribute(p.m_port_id, SAI_PORT_ATTR_SERDES_IDRIVER, idriver))
{
SWSS_LOG_NOTICE("Set port %s idriver is success", alias.c_str());
}
else
{
SWSS_LOG_ERROR("Failed to set port %s idriver", alias.c_str());
it++;
continue;
}

}

if (ipredriver.size() != 0)
{
if (setPortSerdesAttribute(p.m_port_id, SAI_PORT_ATTR_SERDES_IPREDRIVER, ipredriver))
{
SWSS_LOG_NOTICE("Set port %s ipredriver is success", alias.c_str());
}
else
{
SWSS_LOG_ERROR("Failed to set port %s ipredriver", alias.c_str());
it++;
continue;
}

}

/* Last step set port admin status */
if (!admin_status.empty() && (p.m_admin_state_up != (admin_status == "up")))
{
Expand Down Expand Up @@ -4387,29 +4406,95 @@ bool PortsOrch::removeAclTableGroup(const Port &p)
return true;
}

bool PortsOrch::setPortSerdesAttribute(sai_object_id_t port_id, sai_attr_id_t attr_id,
vector<uint32_t> &serdes_val)
bool PortsOrch::setPortSerdesAttribute(sai_object_id_t port_id,
map<sai_port_serdes_attr_t, vector<uint32_t>> &serdes_attr)
{
SWSS_LOG_ENTER();

sai_attribute_t attr;
vector<sai_attribute_t> attr_list;
sai_attribute_t port_attr;
sai_attribute_t port_serdes_attr;
sai_status_t status;
sai_object_id_t port_serdes_id = SAI_NULL_OBJECT_ID;

port_attr.id = SAI_PORT_ATTR_PORT_SERDES_ID;
status = sai_port_api->get_port_attribute(port_id, 1, &port_attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get port attr serdes id %d to port pid:0x%" PRIx64,
port_attr.id, port_id);
return false;
}

memset(&attr, 0, sizeof(attr));
attr.id = attr_id;
if (port_attr.value.oid != SAI_NULL_OBJECT_ID)
{
status = sai_port_api->remove_port_serdes(port_attr.value.oid);
Copy link
Copy Markdown
Collaborator

@prsunny prsunny Jan 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can port_serdes be removed on the fly? Or do you require the port to be shutdown etc?

Copy link
Copy Markdown
Collaborator Author

@dgsudharsan dgsudharsan Jan 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be removed on the fly. It just removes the software object. The logic is introduced because the port serdes object has all attributes as create only and since multiple attributes need to be set which the current set attribute doesn't support, we have this logic of deleting the existing object and recreating it.

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to remove existing port serdes attr 0x%" PRIx64 " port 0x%" PRIx64,
port_attr.value.oid, port_id);
return false;
}
}

attr.value.u32list.count = (uint32_t)serdes_val.size();
attr.value.u32list.list = serdes_val.data();

sai_status_t status = sai_port_api->set_port_attribute(port_id, &attr);
port_serdes_attr.id = SAI_PORT_SERDES_ATTR_PORT_ID;
port_serdes_attr.value.oid = port_id;
attr_list.emplace_back(port_serdes_attr);
SWSS_LOG_INFO("Creating serdes for port 0x%" PRIx64, port_id);

for (auto it = serdes_attr.begin(); it != serdes_attr.end(); it++)
{
port_serdes_attr.id = it->first;
port_serdes_attr.value.u32list.count = (uint32_t)it->second.size();
port_serdes_attr.value.u32list.list = it->second.data();
attr_list.emplace_back(port_serdes_attr);
}
status = sai_port_api->create_port_serdes(&port_serdes_id, gSwitchId,
static_cast<uint32_t>(serdes_attr.size()+1),
attr_list.data());

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set serdes attribute %d to port pid:%" PRIx64,
attr_id, port_id);
SWSS_LOG_ERROR("Failed to create port serdes for port 0x%" PRIx64,
port_id);
return false;
}
SWSS_LOG_NOTICE("Created port serdes object 0x%" PRIx64 " for port 0x%" PRIx64, port_serdes_id, port_id);
return true;
}

void PortsOrch::removePortSerdesAttribute(sai_object_id_t port_id)
{
SWSS_LOG_ENTER();

sai_attribute_t port_attr;
sai_status_t status;
sai_object_id_t port_serdes_id = SAI_NULL_OBJECT_ID;

port_attr.id = SAI_PORT_ATTR_PORT_SERDES_ID;
status = sai_port_api->get_port_attribute(port_id, 1, &port_attr);

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_DEBUG("Failed to get port attr serdes id %d to port pid:0x%" PRIx64,
port_attr.id, port_id);
return;
}

if (port_attr.value.oid != SAI_NULL_OBJECT_ID)
{
status = sai_port_api->remove_port_serdes(port_attr.value.oid);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to remove existing port serdes attr 0x%" PRIx64 " port 0x%" PRIx64,
port_attr.value.oid, port_id);
return;
}
}
SWSS_LOG_NOTICE("Removed port serdes object 0x%" PRIx64 " for port 0x%" PRIx64, port_serdes_id, port_id);
}

void PortsOrch::getPortSerdesVal(const std::string& val_str,
std::vector<uint32_t> &lane_values)
{
Expand Down
8 changes: 6 additions & 2 deletions orchagent/portsorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,12 @@ class PortsOrch : public Orch, public Subject

void getPortSerdesVal(const std::string& s, std::vector<uint32_t> &lane_values);

bool setPortSerdesAttribute(sai_object_id_t port_id, sai_attr_id_t attr_id,
vector<uint32_t> &serdes_val);
bool setPortSerdesAttribute(sai_object_id_t port_id,
std::map<sai_port_serdes_attr_t, std::vector<uint32_t>> &serdes_attr);


void removePortSerdesAttribute(sai_object_id_t port_id);

bool getSaiAclBindPointType(Port::Type type,
sai_acl_bind_point_type_t &sai_acl_bind_type);
void initGearbox();
Expand Down