Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CRM][DASH] Add the possibility of querying availability for OIDs. #1245

Merged
merged 7 commits into from
Sep 13, 2023

Conversation

oleksandrivantsiv
Copy link
Collaborator

  • Add support for DASH resource availability to VirtualSwitchSaiInterface.

- Add support for DASH resource availability to VirtualSwitchSaiInterface.
@oleksandrivantsiv
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@oleksandrivantsiv could you please track failures and rerun if needed?

@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-sairedis

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 1245 in repo sonic-net/sonic-sairedis

@kcudnik
Copy link
Collaborator

kcudnik commented Jul 14, 2023

Seems like there is build error nit related to this change

@kcudnik
Copy link
Collaborator

kcudnik commented Jul 14, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@oleksandrivantsiv oleksandrivantsiv marked this pull request as draft August 4, 2023 14:59
@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-sairedis

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 1245 in repo sonic-net/sonic-sairedis

Comment on lines +850 to +858
else if ((objectType == (sai_object_type_t)SAI_OBJECT_TYPE_VNET) ||
(objectType == (sai_object_type_t)SAI_OBJECT_TYPE_ENI) ||
(objectType == (sai_object_type_t)SAI_OBJECT_TYPE_ENI_ETHER_ADDRESS_MAP_ENTRY) ||
(objectType == (sai_object_type_t)SAI_OBJECT_TYPE_INBOUND_ROUTING_ENTRY) ||
(objectType == (sai_object_type_t)SAI_OBJECT_TYPE_OUTBOUND_ROUTING_ENTRY) ||
(objectType == (sai_object_type_t)SAI_OBJECT_TYPE_PA_VALIDATION_ENTRY) ||
(objectType == (sai_object_type_t)SAI_OBJECT_TYPE_OUTBOUND_CA_TO_PA_ENTRY) ||
(objectType == (sai_object_type_t)SAI_OBJECT_TYPE_DASH_ACL_GROUP) ||
(objectType == (sai_object_type_t)SAI_OBJECT_TYPE_DASH_ACL_RULE))
Copy link
Collaborator

@kcudnik kcudnik Aug 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why those specific types ? do they need to be listed explicitly, seems like all of t hem are experimental objects, maybe you could use compare ?
you could get object_info from sai_metadata_get_object_type_info(ot) (check for null) on top of the function and then use info->isexperimental flag,
or if objectType >= SAI_OBJECT_TYPE_MAX

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those are the DASH objects that we use and want to test in CRM DASH VS tests. Not all experimental objects are resources, so info->isexperimental flag and objectType >= SAI_OBJECT_TYPE_MAX are redundant.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then use flag md->isresourcetype ?

@kcudnik
Copy link
Collaborator

kcudnik commented Aug 27, 2023

please update branch and resolve conflits

@oleksandrivantsiv oleksandrivantsiv marked this pull request as ready for review August 28, 2023 15:09
@oleksandrivantsiv
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Contributor

prsunny commented Sep 7, 2023

@kcudnik , @oleksandrivantsiv , can we merge this?

@oleksandrivantsiv
Copy link
Collaborator Author

oleksandrivantsiv commented Sep 7, 2023

@prsunny from my pov the PR is ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants