From 1844af3f251bc31e898816007086ff45855be53b Mon Sep 17 00:00:00 2001 From: fountzou Date: Fri, 5 Jul 2024 10:52:55 -0400 Subject: [PATCH 1/3] [orchagent]: Resolving issue #18844 * 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 --- orchagent/aclorch.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index f8bf775868..eade3a5d97 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -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. + if (hasCounter()) + { + removeCounter(); + } return true; } @@ -2152,7 +2157,11 @@ void AclRuleMirror::onUpdate(SubjectType type, void *cntx) if (update->active) { SWSS_LOG_INFO("Activating mirroring ACL %s for session %s", m_id.c_str(), m_sessionName.c_str()); - activate(); + // During mirror session activation, the newly created counter needs to be registered to the FC. + if(activate() && hasCounter()) + { + m_pAclOrch->registerFlexCounter(*this); + } } else { From 347c9283a89a956ce38cef2639fa21fbfb56957f Mon Sep 17 00:00:00 2001 From: fountzou Date: Wed, 24 Jul 2024 04:01:09 -0400 Subject: [PATCH 2/3] [orchagent]: Resolving issue #18844 * 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 --- orchagent/aclorch.cpp | 17 ++++++++++++----- orchagent/aclorch.h | 1 + 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index eade3a5d97..a8de70f556 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -2023,6 +2023,18 @@ bool AclRuleMirror::validate() return true; } +bool AclRuleMirror::createCounter() +{ + SWSS_LOG_ENTER(); + + // If the mirror session is inactive, do not create the ACL counter from the first place + bool state = false; + + m_pMirrorOrch->getSessionStatus(m_sessionName, state); + + return !state ? true : AclRule::createCounter(); +} + bool AclRuleMirror::createRule() { SWSS_LOG_ENTER(); @@ -2055,11 +2067,6 @@ bool AclRuleMirror::activate() if (!state) { - // if the mirror session is inactive, do not create the counter yet. FlexCounter registration will be skipped as well. - if (hasCounter()) - { - removeCounter(); - } return true; } diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 5458e970be..71ff7cd592 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -343,6 +343,7 @@ class AclRuleMirror: public AclRule AclRuleMirror(AclOrch *m_pAclOrch, MirrorOrch *m_pMirrorOrch, string rule, string table); bool validateAddAction(string attr_name, string attr_value); bool validate(); + bool createCounter(); bool createRule(); bool removeRule(); void onUpdate(SubjectType, void *) override; From a1164afbe478396598fe2b4a88e25730194fb0ad Mon Sep 17 00:00:00 2001 From: fountzou Date: Thu, 25 Jul 2024 09:05:13 -0400 Subject: [PATCH 3/3] Code refactoring to improve clarity. Signed-off-by: fountzou --- orchagent/aclorch.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index a8de70f556..e9636ed6c8 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -2027,12 +2027,17 @@ bool AclRuleMirror::createCounter() { SWSS_LOG_ENTER(); - // If the mirror session is inactive, do not create the ACL counter from the first place bool state = false; m_pMirrorOrch->getSessionStatus(m_sessionName, state); - return !state ? true : AclRule::createCounter(); + // If the mirror session is active, create the ACL counter + if(state) + { + return AclRule::createCounter(); + } + + return true; } bool AclRuleMirror::createRule()