[pfcwd] Don't abort orchagent when the egress ACL table create fails#4743
[pfcwd] Don't abort orchagent when the egress ACL table create fails#4743rajath-nexthop wants to merge 2 commits into
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
|
An issue tagging this is raised -#4748 |
|
@vmittal-msft kindly review. |
|
@rajath-nexthop @pinky-nexthop can you please add test case for this ? |
|
@vmittal-msft yes, I am working on a |
| } | ||
| } | ||
| else | ||
| { |
There was a problem hiding this comment.
add error logs for Create ACL table failures with the error code. Will be good to get the resource exhaustion and other failures in the log
There was a problem hiding this comment.
Added relevant info (the rule itself and sai status for the rule)
| if (!createPfcAclRule(newRule, queueId, m_strEgressTable, port)) | ||
| { | ||
| SWSS_LOG_ERROR("Failed to create egress PFCWD drop rule %s", m_strRule.c_str()); | ||
| removeIngressBinding(port); |
There was a problem hiding this comment.
Remove egress binding when egress ACL rule create fails?
There was a problem hiding this comment.
Could you also check for any ACL CRMs
There was a problem hiding this comment.
Added code to remove the egress binding. ACL CRMs look okay - CRM_ACL_TABLE is incremented only after create() succeeds and is decremented when removeAclTable is invoked - so there should be no inconsistencies from there.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Hi @vmittal-msft , PR raised for the test as well - sonic-net/sonic-mgmt#26413 |
What I did ========== PfcWdAclHandler builds an ingress (and, on some platforms, egress) ACL table + rule when PFC watchdog first acts on a stormed queue. If the ACL table creation failed (e.g. the egress PMF banks are exhausted by a configured egress ACL), the failed AclOrch::addAclTable() was swallowed and an AclRule was still constructed against the missing table; its ctor SWSS_LOG_THROWs, which is uncaught and calls std::terminate -> SIGABRT, taking down the whole swss container. - createPfcAclTable()/createPfcAclRule() now return bool; on failure the half-built m_aclTables entry is erased and the call sites skip rule construction and roll back what was already installed. - ~PfcWdAclHandler() no longer SWSS_LOG_THROWs when a rule/table is absent (a destructor is implicitly noexcept; throwing -> terminate). - startWdActionOnQueue() only publishes the queue's INSTORM state when the handler installed successfully (isValid()); an un-installable action is surfaced to STATE_DB PFC_WD_SW_STATE_TABLE as "failed" and is not replayed on warm reboot. Handler construction is wrapped in a try/catch backstop so no PFC event path can abort orchagent. How to verify it ================ On a platform whose egress PFCWD ACL shares the egress PMF with standard egress ACLs: fill the egress ACL banks, then trigger PFCWD drop action on a queue (storm or debug storm). Before: orchagent SIGABRT + core, swss down. After: orchagent logs "Failed to create PFCWD ACL table ..." and stays up; the queue is reported as "failed" in STATE_DB. Signed-off-by: Rajath V <rajath@nexthop.ai>
…llback - Log ACL table-create failures at each call site (ingress: drop action not installed; egress: rolling back the ingress drop rule). The exact SAI failure status of the table create is logged by the SAI layer adjacent to these messages. - Include the rule's last SAI status (AclRule::getLastSaiStatus()) in all rule-create failure logs, so resource exhaustion (e.g. SAI_STATUS_TABLE_FULL / INSUFFICIENT_RESOURCES) is visible directly. - When the egress ACL rule create fails right after this handler created the egress table, remove that just-created (empty) egress table as part of the rollback instead of leaving it behind. A pre-existing/shared egress table is intentionally left in place. Signed-off-by: Rajath V <rajath@nexthop.ai>
372745d to
d72be06
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
5 participants
Why I did it
On platforms where the egress PFC-watchdog ACL table draws from the same
egress PMF banks as configured standard egress ACLs, those banks can be
exhausted. When
create_acl_table()then fails,PfcWdAclHandlerswallowed the failed
AclOrch::addAclTable()and still built anAclRuleagainst the missing table; the rule ctor
SWSS_LOG_THROWs, which isuncaught →
std::terminate→ SIGABRT, taking down the entire swsscontainer. A single un-creatable ACL table should degrade gracefully, not
crash orchagent.
Work item tracking
How I did it
createPfcAclTable()/createPfcAclRule()returnbool; onaddAclTable()failure the half-builtm_aclTablesentry is erased andthe call sites skip
AclRuleconstruction and roll back what wasalready installed for the queue.
~PfcWdAclHandler()no longerSWSS_LOG_THROWs when a rule/table isabsent (a destructor is implicitly
noexcept; throwing →terminate) —it logs and cleans up what exists.
startWdActionOnQueue()publishes the queue'sINSTORMstate only whenthe handler installed successfully (
isValid()); an action that couldnot be installed is surfaced to
STATE_DBPFC_WD_SW_STATE_TABLEasfailedand is not replayed on warm reboot. Handler construction iswrapped in a try/catch backstop so no PFC event path can abort orchagent.
How to verify it
On a platform whose egress PFCWD ACL shares the egress PMF with standard
egress ACLs: fill the egress ACL banks with egress ACLs, then trigger the
PFCWD drop action on a queue (real storm or debug storm).
Failed to create PFCWD ACL table ...and staysup; the un-mitigated queue is reported as
failedinSTATE_DB|PFC_WD_SW_STATE_TABLE.Sonic-mgmt test added for the same - sonic-net/sonic-mgmt#26413
Which release branch to backport (provide reason below if selected)
Description for the changelog
pfcwd: handle egress ACL table creation failure gracefully instead of
aborting orchagent.