Skip to content

Commit

Permalink
[portinit] Do not call GET on SAI_PORT_ATTR_SPEED when AUTONEG is ena…
Browse files Browse the repository at this point in the history
…bled (#2484)

What I did
During port init if AutoNeg is enabled do not do GET oper on SAI_PORT_ATTR_SPEED (do not call getPortSpeed)

Why I did it
This PR fixes an issue where in some platforms syncd crashes when AutoNeg is enabled and port is oper down.
The crash happens in the warmboot recovery path:

In the target image as part of portinit, GET operation on SAI_PORT_ATTR_SPEED returns a random value.
This random value does not match the speed in the base image.
Diff in speed causes syncd comparison logic to attempt to set newly detected speed.
Comparison logic crashes in APPLY_VIEW when doing a SET speed operation on port with the new speed.
Why SAI returns random value in GET oper on SAI_PORT_ATTR_SPEED:
SAI returns random value as when autoneg is enabled attribute SAI_PORT_ATTR_SPEED is not set in the first place.
When autoneg is enabled a list of speeds is set using attribute SAI_PORT_ATTR_ADVERTISED_SPEED (instead of SAI_PORT_ATTR_SPEED when autoneg is not enabled.

In autoneg enabled case, get oper on SAI_PORT_ATTR_SPEED is not allowed as the speed on the port is not certain, and instead depends on advertised/negotiated speed.
  • Loading branch information
vaibhavhd committed Dec 7, 2022
1 parent 6afefe1 commit 872f7bf
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
5 changes: 5 additions & 0 deletions orchagent/p4orch/tests/fake_portorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,11 @@ bool PortsOrch::getQueueTypeAndIndex(sai_object_id_t queue_id, string &type, uin
return true;
}

bool PortsOrch::isAutoNegEnabled(sai_object_id_t id)
{
return true;
}

task_process_status PortsOrch::setPortAutoNeg(sai_object_id_t id, int an)
{
return task_success;
Expand Down
19 changes: 18 additions & 1 deletion orchagent/portsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2484,6 +2484,23 @@ bool PortsOrch::getQueueTypeAndIndex(sai_object_id_t queue_id, string &type, uin
return true;
}

bool PortsOrch::isAutoNegEnabled(sai_object_id_t id)
{
SWSS_LOG_ENTER();

sai_attribute_t attr;
attr.id = SAI_PORT_ATTR_AUTO_NEG_MODE;

sai_status_t status = sai_port_api->get_port_attribute(id, 1, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get port AutoNeg status for port pid:%" PRIx64, id);
return false;
}

return attr.value.booldata;
}

task_process_status PortsOrch::setPortAutoNeg(sai_object_id_t id, int an)
{
SWSS_LOG_ENTER();
Expand Down Expand Up @@ -4758,7 +4775,7 @@ bool PortsOrch::initializePort(Port &port)
}

/* initialize port admin speed */
if (!getPortSpeed(port.m_port_id, port.m_speed))
if (!isAutoNegEnabled(port.m_port_id) && !getPortSpeed(port.m_port_id, port.m_speed))
{
SWSS_LOG_ERROR("Failed to get initial port admin speed %d", port.m_speed);
return false;
Expand Down
1 change: 1 addition & 0 deletions orchagent/portsorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ class PortsOrch : public Orch, public Subject
bool m_isPortCounterMapGenerated = false;
bool m_isPortBufferDropCounterMapGenerated = false;

bool isAutoNegEnabled(sai_object_id_t id);
task_process_status setPortAutoNeg(sai_object_id_t id, int an);
bool setPortFecMode(sai_object_id_t id, int fec);
task_process_status setPortInterfaceType(sai_object_id_t id, sai_port_interface_type_t interface_type);
Expand Down

0 comments on commit 872f7bf

Please sign in to comment.