[FEC] Adding support of override based on attribute query of SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE #2874
Conversation
dgsudharsan
changed the title
|
@prgeor Can you please review? The checkers now pass |
orchagent/portsorch.cpp
Outdated
| m_gearboxTable->hset(key, speed_attr, to_string(speed)); | ||
| SWSS_LOG_NOTICE("BOX: Updated APPL_DB key:%s %s %d", key.c_str(), speed_attr.c_str(), speed); | ||
| } | ||
| else if (id == SAI_PORT_ATTR_FEC_MODE && fec_override_sup) |
There was a problem hiding this comment.
@dgsudharsan why are we setting this inside gerabox ?
There was a problem hiding this comment.
The FEC attribute setting API after setting for port will call gearbox API if gearbox is present. Hence this needs to be set to be in sync with Port FEC setting.
orchagent/portsorch.cpp
Outdated
| attr.id = SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE; | ||
| attr.value.booldata = true; | ||
|
|
||
| status = sai_port_api->set_port_attribute(dest_port_id, &attr); |
There was a problem hiding this comment.
@dgsudharsan i think we should do fec override if AN is configured.
There was a problem hiding this comment.
@prgeor Both are independent attributes. SAI headers doesn't mandate that FEC override should need to be set only on AN. To avoid complexity in orchagent, both are set independently and vendor SAI will handle them in any order
prgeor
left a comment
prgeor
left a comment
There was a problem hiding this comment.
@dgsudharsan can we have a one API setPortFecOverride() and call this instead of repeating the code?
Done |
…_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE (sonic-net#2874)
4 participants
Depends on sonic-net/sonic-sairedis#1271
What I did
Providing support of FEC override functionality existing today through attribute query of SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE
Why I did it
Today when FEC is enabled by user in SONiC it should explicitly override any auto negotiated values. However when the support SAI_PORT_ATTR_AUTO_NEG_FEC_MODE_OVERRIDE exists in vendor SAI, it should be queried and used to get this functionality.
How I verified it
Adding UT to verify it.
Details if related