[ACL] Add default action_list for default ACL table type #2298
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: bingwang <wang.bing@microsoft.com>
|
@stepanblyschak @ysmanman Could you help take a look? Thanks |
orchagent/aclorch.cpp
Outdated
| AclActionCapabilities | ||
| { | ||
| { | ||
| SAI_ACL_ACTION_TYPE_PACKET_ACTION |
There was a problem hiding this comment.
There's a redirect action used for L3, L3V6 tables
There was a problem hiding this comment.
Fixed. I added SAI_ACL_ACTION_TYPE_REDIRECT for L3 and L3V6. Thanks
There was a problem hiding this comment.
There's a redirect action used for L3, L3V6 tables
Redirect action should only be added for ingress and not egress. This is breaking other platforms.
orchagent/aclorch.cpp
Outdated
| { | ||
| SAI_ACL_ACTION_TYPE_PACKET_ACTION | ||
| }, | ||
| false |
There was a problem hiding this comment.
I guess this boolean isn't used. Maybe you could create another type instead of using acl_capabilities_t
There was a problem hiding this comment.
Good idea. I use the set<sai_acl_action_type_t> directly. Thanks
Signed-off-by: bingwang <wang.bing@microsoft.com>
bingwang-ms
force-pushed
the
add_default_action_list
branch
from
17ba13c to
02e4439
Compare
yxieca
approved these changes
May 27, 2022
ysmanman
approved these changes
May 27, 2022
|
@yxieca , @bingwang-ms , can you please check on the coverage? Can this be fixed? |
This is needed in SONiC 202111 as well. Can this be ported there? |
rck-innovium
reviewed
Jun 7, 2022
| }, | ||
| false | ||
| SAI_ACL_ACTION_TYPE_PACKET_ACTION, | ||
| SAI_ACL_ACTION_TYPE_REDIRECT |
There was a problem hiding this comment.
redirect action should be only for the ingress and not egress.
preetham-singh
pushed a commit
to preetham-singh/sonic-swss
that referenced
this pull request
Aug 6, 2022
) What I did This PR is derived from sonic-net#2205 Fix sonic-net/sonic-buildimage#10425 We were seeing ACL table creation failure on some platform because action_list is mandatory, while the action_list is not provided by aclorch. Apr 1 01:24:11.702608 str2-7050cx3-acs-03 ERR swss#orchagent: :- validate: Action list for table DATAACL is mandatory Apr 1 01:24:11.702608 str2-7050cx3-acs-03 ERR swss#orchagent: :- doAclTableTask: Failed to create ACL table DATAACL, invalid configuration Apr 1 01:24:11.702741 str2-7050cx3-acs-03 ERR swss#orchagent: :- validate: Action list for table EVERFLOW is mandatory Apr 1 01:24:11.702741 str2-7050cx3-acs-03 ERR swss#orchagent: :- doAclTableTask: Failed to create ACL table EVERFLOW, invalid configuration Apr 1 01:24:11.702926 str2-7050cx3-acs-03 ERR swss#orchagent: :- validate: Action list for table EVERFLOWV6 is mandatory Apr 1 01:24:11.702926 str2-7050cx3-acs-03 ERR swss#orchagent: :- doAclTableTask: Failed to create ACL table EVERFLOWV6, invalid configuration This PR fixed the issue by adding default action_list to the default ACL table type if not present. Why I did it Fix the ACL table creation issue. How I verified it Verified by running test_acl and test_everflow on Broadcom TD3 platform Signed-off-by: bingwang <wang.bing@microsoft.com> Co-authored-by: syuan <syuan@arista.com>
judyjoseph
pushed a commit
to judyjoseph/sonic-swss
that referenced
this pull request
Oct 27, 2022
) What I did This PR is derived from sonic-net#2205 Fix sonic-net/sonic-buildimage#10425 We were seeing ACL table creation failure on some platform because action_list is mandatory, while the action_list is not provided by aclorch. Apr 1 01:24:11.702608 str2-7050cx3-acs-03 ERR swss#orchagent: :- validate: Action list for table DATAACL is mandatory Apr 1 01:24:11.702608 str2-7050cx3-acs-03 ERR swss#orchagent: :- doAclTableTask: Failed to create ACL table DATAACL, invalid configuration Apr 1 01:24:11.702741 str2-7050cx3-acs-03 ERR swss#orchagent: :- validate: Action list for table EVERFLOW is mandatory Apr 1 01:24:11.702741 str2-7050cx3-acs-03 ERR swss#orchagent: :- doAclTableTask: Failed to create ACL table EVERFLOW, invalid configuration Apr 1 01:24:11.702926 str2-7050cx3-acs-03 ERR swss#orchagent: :- validate: Action list for table EVERFLOWV6 is mandatory Apr 1 01:24:11.702926 str2-7050cx3-acs-03 ERR swss#orchagent: :- doAclTableTask: Failed to create ACL table EVERFLOWV6, invalid configuration This PR fixed the issue by adding default action_list to the default ACL table type if not present. Why I did it Fix the ACL table creation issue. How I verified it Verified by running test_acl and test_everflow on Broadcom TD3 platform Signed-off-by: bingwang <wang.bing@microsoft.com> Co-authored-by: syuan <syuan@arista.com>
judyjoseph
added a commit
that referenced
this pull request
Oct 27, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What I did
This PR is derived from #2205
Fix sonic-net/sonic-buildimage#10425
We were seeing ACL table creation failure on some platform because
action_listis mandatory, while theaction_listis not provided byaclorch.This PR fixed the issue by adding default
action_listto the default ACL table type if not present.Why I did it
Fix the ACL table creation issue.
How I verified it
Verified by running
test_aclandtest_everflowon Broadcom TD3 platformDetails if related