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.

Sign up for GitHub

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

Jump to bottom

[aclorch] egress mirror action support and action ASIC support check #963

Merged
merged 19 commits into from
Sep 16, 2019
Merged

[aclorch] egress mirror action support and action ASIC support check #963

merged 19 commits into from
Sep 16, 2019

Conversation

stepanblyschak
Copy link
Contributor

@stepanblyschak stepanblyschak commented Jul 1, 2019 •
edited
Loading

What I did

  • add support for egress mirror action
  • move redirect out from PACKET_ACTION to its own REDIRECT_ACTION key preserving backwards compatibility with old schema to be aligned with SAI data types
  • query ACL action list supported by ASIC per stage and put this information in STATE DB SWITCH_CAPABILITY table
  • perform secondary query for ACL action attributes which parameters are enum values (e.g. for PACKET_ACTION - DROP,FORWARD)
  • implement VS test cases for 1 and 2
  • fix restart of portsyncd (required for one of the test cases which restart SwSS daemons)

Why I did it

  • To add egress mirroring support
  • Ability to check if specific action is supported before applying the configuration

How I verified it

Example output on Mellanox platform:

admin@arc-switch1025:~$ redis-cli -n 6 hgetall 'SWITCH_CAPABILITY|switch'
 1) "MIRROR"
 2) "true"
 3) "MIRRORV6"
 4) "true"
 5) "ACL_ACTIONS|INGRESS"
 6) "PACKET_ACTION,REDIRECT_ACTION,MIRROR_INGRESS_ACTION"
 7) "ACL_ACTIONS|EGRESS"
 8) "PACKET_ACTION,MIRROR_EGRESS_ACTION"
 9) "ACL_ACTION|PACKET_ACTION"
10) "DROP,FORWARD"

Example output on VS platform

root@8ec027155dfe:/# redis-cli -n 6 hgetall 'SWITCH_CAPABILITY|switch'
 1) "ACL_ACTIONS|EGRESS"
 2) "PACKET_ACTION,REDIRECT_ACTION,MIRROR_EGRESS_ACTION,MIRROR_INGRESS_ACTION,DROP_REPORT_ENABLE,FLOW_OP,FLOW_SAMPLE_PERCENT,INT_SESSION,REPORT_ALL_PACKETS,TAIL_DROP_REPORT_ENABLE"
 3) "MIRROR"
 4) "true"
 5) "MIRRORV6"
 6) "false"
 7) "ACL_ACTIONS|INGRESS"
 8) "PACKET_ACTION,REDIRECT_ACTION,MIRROR_EGRESS_ACTION,MIRROR_INGRESS_ACTION,DROP_REPORT_ENABLE,FLOW_OP,FLOW_SAMPLE_PERCENT,INT_SESSION,REPORT_ALL_PACKETS,TAIL_DROP_REPORT_ENABLE"
 9) "ACL_ACTION|FLOW_OP"
10) "INT,IOAM,NOP,POSTCARD"
11) "ACL_ACTION|PACKET_ACTION"
12) "DROP,FORWARD"

Details if related

DEPENDS sonic-net/sonic-sairedis#481

@stepanblyschak stepanblyschak force-pushed the acl_cap branch 2 times, most recently from ae6eeac to f851635 Compare July 4, 2019 14:12
@stepanblyschak stepanblyschak mentioned this pull request Jul 5, 2019
Merged
stcheng
stcheng suggested changes Aug 6, 2019
View reviewed changes
Copy link
Contributor

@stcheng stcheng left a comment

Choose a reason for hiding this comment

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

thanks! please check the comments.

orchagent/aclorch.h Show resolved Hide resolved
portsyncd/portsyncd.cpp Outdated Show resolved Hide resolved
orchagent/acltable.h
#define TABLE_EGRESS "EGRESS"
#define STAGE_INGRESS "INGRESS"
#define STAGE_EGRESS "EGRESS"
#define TABLE_INGRESS STAGE_INGRESS
Copy link
Contributor

Choose a reason for hiding this comment

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

mark these two definitions as backward compatibility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I don't quite understand why this is for backward compatibility. TABLE_INGRESS and TABLE_EGRESS is still used in the code not for backward compatibility
STAGE_INGRESS, STAGE_EGRESS were added for ACL mirror rule code just to not confuse reader with TABLE_* defines

portsyncd/linksync.cpp Outdated Show resolved Hide resolved
orchagent/aclorch.h Show resolved Hide resolved
orchagent/aclorch.cpp Show resolved Hide resolved
tests/test_acl.py Show resolved Hide resolved
tests/test_acl.py Show resolved Hide resolved
tests/test_acl.py Outdated Show resolved Hide resolved
tests/test_mirror.py Show resolved Hide resolved
@stepanblyschak stepanblyschak mentioned this pull request Aug 8, 2019
Merged
Stepan Blyschak added 19 commits August 8, 2019 20:42
[aclorch] support egress mirror action
174fdd5 
Signed-off-by: Stepan Blyschak <[email protected]>

[test_mirror] add integration test for egress mirror rule creation

Signed-off-by: Stepan Blyschak <[email protected]>
[aclorch] query acl action capabilities per stage
2f241b5 
Signed-off-by: Stepan Blyschak <[email protected]>

[aclorch] add getAclActionTypeFromAttr in AclOrch

Signed-off-by: Stepan Blyschak <[email protected]>

[aclorch] add isAclActionSupported in AclOrch

Signed-off-by: Stepan Blyschak <[email protected]>

[aclorch] add generic AclRule::validateAddAction implementation

Each derivative of AclRule now has to call AclRule::validateAddAction
to validate if action is supported.

Signed-off-by: Stepan Blyschak <[email protected]>

[aclorch] move action support check in different method

Signed-off-by: Stepan Blyschak <[email protected]>

[aclorch] fix acl action support check

Signed-off-by: Stepan Blyschak <[email protected]>
[aclorch] put acl actions capabilities in state db
0da7d7c 
Signed-off-by: Stepan Blyschak <[email protected]>
[test_acl] add acl rule validation test cases
de02759 
Signed-off-by: Stepan Blyschak <[email protected]>
[doc] update schema for mirror rule
dce9fa6 
Signed-off-by: Stepan Blyschak <[email protected]>
[aclorch] replace throw with SWSS_LOG_THROW and limit scope for strin…
758735e 
…gstream in queryAclActionCapabilities

Signed-off-by: Stepan Blyschak <[email protected]>
[aclorch] new schema
d203963 
Signed-off-by: Stepan Blyschak <[email protected]>
[aclorch] fix indent
489e22e 
Signed-off-by: Stepan Blyschak <[email protected]>
[aclorch] fix ACL_ACTION -> ACL_ACTIONS
6253887 
Signed-off-by: Stepan Blyschak <[email protected]>
[test_acl] fix ACL_ACTION -> ACL_ACTIONS
1d10f15
[aclorch] update according to design
808629d 
Signed-off-by: Stepan Blyschak <[email protected]>
[aclorch] perform secondary query for supported enum values
b3a598f 
Right now ACL actions that have enum values are - PACKET_ACTION and DTEL_FLOW_OP.
Once SAI object API is supported by libsairedis we will query which values are supported
by calling "sai_query_attribute_enum_values_capability"; untill then we will assume all
values are supported and populate this in DB

Signed-off-by: Stepan Blyschak <[email protected]>
[swss-schema.md] update schema according to design
b247827
[aclorch] 'if 0' -> 'ifdef SAIREDIS_SUPPORT_OBJECT_API'
0c806bc
[test_acl] fix vs test
9da8e9c 
Signed-off-by: Stepan Blyschak <[email protected]>
[aclorch] fix mirror rule creation
747501a 
Signed-off-by: Stepan Blyschak <[email protected]>
[aclorch] fix typos and remove reduntant spaces, new lines
f122cee 
Signed-off-by: Stepan Blyschak <[email protected]>
[test_acl] remove rule in validation test
8d6492e 
Signed-off-by: Stepan Blyschak <[email protected]>
[test_acl] restart syncd explicitely
fdc3c24 
Signed-off-by: Stepan Blyschak <[email protected]>
@stepanblyschak
Copy link
Contributor Author

thanks! please check the comments.

@stcheng comments were fixed or resolved, could you please review?

@stcheng stcheng merged commit d98d1e9 into sonic-net:master Sep 16, 2019
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
@kcudnik
[az] Set diff coverage threshold to 50% (sonic-net#963)
04793b1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stcheng stcheng stcheng approved these changes

@lguohan lguohan lguohan approved these changes

Assignees

@stcheng stcheng

Labels
Enhancement ➕
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stepanblyschak @lguohan @stcheng