[orchagent]: Skip installing ACL counter when ACL mirror rule is inactive #3223
Conversation
orchagent/aclorch.cpp
Outdated
| @@ -2055,6 +2055,11 @@ bool AclRuleMirror::activate() | |||
|
|
|||
| if (!state) | |||
| { | |||
| // if the mirror session is inactive, do not create the counter yet. FlexCounter registration will be skipped as well. | |||
There was a problem hiding this comment.
In this situation, where the mirror session is inactive, was the ACL rule created and then removed?
There was a problem hiding this comment.
Hi, thank you for reviewing. In this case the ACL rule, was not created at all by SAI.
ACL entry creation happens at the bottom of AclRuleMirror::activate() function via AclRule::createRule(), but code execution never reaches that point since function returns earlier when state == false, i.e., for inactive session.
There was a problem hiding this comment.
If the ACL rule was never created, would it be possible not to create the counter at the beginning?
(Instead of it being created and then removed)
There was a problem hiding this comment.
Looking into it. Thanks for your suggestion.
There was a problem hiding this comment.
PR has been updated based on your comment. Thank you!
* Don't install counters when session rule is inactive. Mirror session is not activated and thus, the ACL rule is not created in sairedis/SAI (there is no next hop for the mirror destination IP). However, orchagent creates and registers the ACL counter with the flexcounter (FC) and the counter is being polled every polling interval. Since the ACL counter is not attached to any ACL rule, the BCM SAI introduces print errors in syslog. Signed-off-by: fountzou <ioannis.fountzoulas@nokia.com>
* Don't install counters when session rule is inactive. Mirror session is not activated and thus, the ACL rule is not created in sairedis/SAI (there is no next hop for the mirror destination IP). However, orchagent creates and registers the ACL counter with the flexcounter (FC) and the counter is being polled every polling interval. Since the ACL counter is not attached to any ACL rule, the BCM SAI introduces print errors in syslog. Signed-off-by: fountzou <ioannis.fountzoulas@nokia.com>
|
@bingwang-ms can you please help review this? |
Signed-off-by: fountzou <ioannis.fountzoulas@nokia.com>
|
@prsunny @bingwang-ms @rlhui Please review this PR. |
|
LGTM now. Thanks for the fix. |
|
@prsunny, please help merge this PR. |
|
Restarting the VS tests |
|
/azp run Azure.sonic-swss |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@fountzou , could you check the test failure?, also please fix the title of PR |
fountzou
changed the title
|
Hi @prsunny. Title has been fixed -- thanks. Looking into this but not sure yet if it is related to my code changes; Noticed in the pipelines that there are several PRs that fail in the same testcase. Thank you |
|
@fountzou Do we need this PR for 202405 branch? |
@mlok-nokia Yes, we do -- thanks for bringing it up. Appropriate label should be added. |
|
@prsunny Hi Prince, This PR is required by branch 202405. Please help to merge to it. Thanks. |
Mirror session is not activated and thus, the ACL rule is not created in sairedis/SAI (there is no next hop for the mirror destination IP). However, orchagent creates and registers the ACL counter with the flexcounter (FC) and the counter is being polled every polling interval. Since the ACL counter is not attached to any ACL rule, the BCM SAI introduces print errors in syslog.
What I did
Removed the ACL counter when ACL mirror rule / session is inactive.
Why I did it
To solve #18844 issue.
How I verified it
Details if related