Skip to content

Commit 872f7bf

Browse files
authored
[portinit] Do not call GET on SAI_PORT_ATTR_SPEED when AUTONEG is enabled (sonic-net#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.
1 parent 6afefe1 commit 872f7bf

File tree

3 files changed

+24
-1
lines changed

3 files changed

+24
-1
lines changed

orchagent/p4orch/tests/fake_portorch.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,11 @@ bool PortsOrch::getQueueTypeAndIndex(sai_object_id_t queue_id, string &type, uin
606606
return true;
607607
}
608608

609+
bool PortsOrch::isAutoNegEnabled(sai_object_id_t id)
610+
{
611+
return true;
612+
}
613+
609614
task_process_status PortsOrch::setPortAutoNeg(sai_object_id_t id, int an)
610615
{
611616
return task_success;

orchagent/portsorch.cpp

+18-1
Original file line numberDiff line numberDiff line change
@@ -2484,6 +2484,23 @@ bool PortsOrch::getQueueTypeAndIndex(sai_object_id_t queue_id, string &type, uin
24842484
return true;
24852485
}
24862486

2487+
bool PortsOrch::isAutoNegEnabled(sai_object_id_t id)
2488+
{
2489+
SWSS_LOG_ENTER();
2490+
2491+
sai_attribute_t attr;
2492+
attr.id = SAI_PORT_ATTR_AUTO_NEG_MODE;
2493+
2494+
sai_status_t status = sai_port_api->get_port_attribute(id, 1, &attr);
2495+
if (status != SAI_STATUS_SUCCESS)
2496+
{
2497+
SWSS_LOG_ERROR("Failed to get port AutoNeg status for port pid:%" PRIx64, id);
2498+
return false;
2499+
}
2500+
2501+
return attr.value.booldata;
2502+
}
2503+
24872504
task_process_status PortsOrch::setPortAutoNeg(sai_object_id_t id, int an)
24882505
{
24892506
SWSS_LOG_ENTER();
@@ -4758,7 +4775,7 @@ bool PortsOrch::initializePort(Port &port)
47584775
}
47594776

47604777
/* initialize port admin speed */
4761-
if (!getPortSpeed(port.m_port_id, port.m_speed))
4778+
if (!isAutoNegEnabled(port.m_port_id) && !getPortSpeed(port.m_port_id, port.m_speed))
47624779
{
47634780
SWSS_LOG_ERROR("Failed to get initial port admin speed %d", port.m_speed);
47644781
return false;

orchagent/portsorch.h

+1
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ class PortsOrch : public Orch, public Subject
363363
bool m_isPortCounterMapGenerated = false;
364364
bool m_isPortBufferDropCounterMapGenerated = false;
365365

366+
bool isAutoNegEnabled(sai_object_id_t id);
366367
task_process_status setPortAutoNeg(sai_object_id_t id, int an);
367368
bool setPortFecMode(sai_object_id_t id, int fec);
368369
task_process_status setPortInterfaceType(sai_object_id_t id, sai_port_interface_type_t interface_type);

0 commit comments

Comments
 (0)