[syncd] Add workaround for snoop SAI_PORT_ATTR_SPEED #1132
Conversation
This is workaround. Skip port speed because of broadcom bug, after warm reboot SAI returns different speed then before warm reboot, which causes to generate SET operation on comparison logic and this SET operation fails. There is no SET operation for speed generated by orchagent, since all values are snooped here.
| @@ -3044,6 +3044,24 @@ void Syncd::snoopGetResponse( | |||
| continue; | |||
| } | |||
|
|
|||
| if (meta->objecttype == SAI_OBJECT_TYPE_PORT && | |||
There was a problem hiding this comment.
Should there be a check that this is part of the warmreboot to skip this or we can skip this completely for other boot causes (cold/fastreboot) and not causing issue for those boot cases?
There was a problem hiding this comment.
I believe that the issue was that SAI_PORT_ATTR_SPEED was getting queried on a port with the port in auto negotiation enabled. I suggest that SONIC check the port's configured auto negotiation mode and query the right attribute accordingly i.e., SAI_PORT_ATTR_SPEED or SAI_PORT_ATTR_ADVERTISED_SPEED.
There was a problem hiding this comment.
at this point check is not that easy, there should be no issue with other modes then warmboot
There was a problem hiding this comment.
@prsunny can you please review this change too? If this is a blanket ignore on SPEED GET for PORT ATTR:
- Do you think this should be better handled at portorch level? I mean that's where the call initiates from, so ignoring from the source might be better?
- Do you see any concerns on ignoring this GET call for all the flows, and not just warmboot?
There was a problem hiding this comment.
as mentioned in PR, it a workaround anyways for bcm platform. So fine with placing it here.
There was a problem hiding this comment.
discussed offline with Vaibhav and think this should preferably be done from orchagent side as suggested in above comments.
This is workaround. Skip port speed because of broadcom bug, after warm reboot SAI returns different speed then before warm reboot, which causes to generate SET operation on comparison logic and this SET operation fails. There is no SET operation for speed generated by orchagent, since all values are snooped here.