[syncd] Support ACL action data object in remove dep tree #1065
Conversation
This is required if some of the attributes on ACL action data object are OID's and we need to catch their refrence when removing object and it's dependency tree.
| @@ -2089,7 +2089,8 @@ void ComparisonLogic::removeCurrentObjectDependencyTree( | |||
| continue; | |||
| } | |||
|
|
|||
| if (revgraph->attrmetadata->attrvaluetype != SAI_ATTR_VALUE_TYPE_OBJECT_ID) | |||
| if (revgraph->attrmetadata->attrvaluetype != SAI_ATTR_VALUE_TYPE_OBJECT_ID && | |||
| revgraph->attrmetadata->attrvaluetype != SAI_ATTR_VALUE_TYPE_ACL_ACTION_DATA_OBJECT_ID) | |||
There was a problem hiding this comment.
Can this issue be fixed in a more generic fashion? In this approach we always need to hit the issue before fixing it. In future there may be any other OID attr on any other obj, and this issue will be seen again.
What are downsides of logging this as an error/warning and continuing, instead of failing out using THROW.
There was a problem hiding this comment.
We need to update the comment now? // currently we only support reference on OID, not list does not hold true anymore?
There was a problem hiding this comment.
What happens when this object and its dependency tree is removed? Will we lose this object in DB after warmreboot? Will that be a concern?
There was a problem hiding this comment.
this is how this is solved currently in master branch, and there are currently only 2 types of objects taht are used in attr value, attr and attr action data oid, and those 2 oids are supported in SaiAttr::getOid() function which is executed after that. We can't just log this instead of throw since this is in recursive function removing current object, since the object is removed it needs to remove all it dependencies first, and this needs to be done recursively. cant skip it since it would leave wrong number of references and cause to crash later on
for oid list, this scenario is not supported yet, it would require to remove more complicated logic, we don't have that scenario yet here, let's figure this out when we hit that scenario
if object and dependency tree is removed, it's put on the list to remove when execute it on asic, and those operations will be executed and state from one view will be brought to state in temp view, you will not loos that object in DB, since at the end DB will be looking like OA created it after warm boot. remove is caused here because there are differences in current and temp view which CL tries to fix, and in need to do this by removing some objects and then maybe create them, depends on how complicated views are
|
issue was addressed previously on master here: #1025 |
Update sonic-sairedis submodule to include cross-branch warmboot fix: sonic-net/sonic-sairedis#1065: Support ACL action data object in remove dep tree: This is required if some of the attributes on ACL action data object are OID's and we need to catch their refrence when removing object and it's dependency tree.
This is required if some of the attributes on ACL action data
object are OID's and we need to catch their refrence when
removing object and it's dependency tree.