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

[aclorch] if vendor does not implement ACL action capability quieries… #1106

Merged
merged 3 commits into from
Oct 28, 2019

Conversation

stepanblyschak
Copy link
Contributor

… - use default capabilities (do not fail)

Signed-off-by: Stepan Blyschak stepanb@mellanox.com

What I did

If ACL max action count query failed - use defaultAclActionsSupported for both INGRESS and EGRESS
If ACL actions query on INGRESS failed use defaultAclActionsSupported for INGRESS
If ACL actions query on EGRESS failed use defaultAclActionsSupported for EGRESS

Warn in any scenario that orchagent will assume defaults.

#ifdef SAI_SUPPORTS_OBJECT_API /* does not compile right now */
if enum values supported for ACL action query failed - assume all are supported
(e.g. action - PACKET_ACTION, values - DROP,FORWARD etc.)
#else
assume all values are supported
#end

Why I did it

Seems to crash on many vendors because of lack of capability query support

How I verified it

Verify on system which supports attributes,
Simulate if system does not support and verify defaults are pushed to db

Details if related

… - use default capabilities (do not fail)

Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
@stepanblyschak stepanblyschak marked this pull request as ready for review October 25, 2019 08:14
}
else if (status == SAI_STATUS_NOT_IMPLEMENTED)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please have just the else case to populate the default table

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it. But that means SAI does not return valid status code

}

for (size_t i = 0; i < values.count; i++)
else if (status == SAI_STATUS_NOT_IMPLEMENTED)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above comment

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you address this as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Collaborator

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

As comments

Stepan Blyschak added 2 commits October 28, 2019 00:01
Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
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.

2 participants