From 44e6af9415e0273eeb2859cb124f1aea94caa98d Mon Sep 17 00:00:00 2001 From: Rajath V Date: Fri, 10 Jul 2026 06:52:03 +0000 Subject: [PATCH 1/2] [pfcwd] Don't abort orchagent when the egress ACL table create fails 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 --- orchagent/pfcactionhandler.cpp | 136 +++++++++++++++++++++++++-------- orchagent/pfcactionhandler.h | 11 ++- orchagent/pfcwdorch.cpp | 78 ++++++++++++++++--- orchagent/pfcwdorch.h | 3 + 4 files changed, 185 insertions(+), 43 deletions(-) diff --git a/orchagent/pfcactionhandler.cpp b/orchagent/pfcactionhandler.cpp index 32f984cea13..566c4b515ad 100644 --- a/orchagent/pfcactionhandler.cpp +++ b/orchagent/pfcactionhandler.cpp @@ -324,9 +324,21 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, if (found == m_aclTables.end()) { // First time of handling PFC for this queue, create ACL table, and bind - createPfcAclTable(port, m_strIngressTable, true); - shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strIngressTable); - createPfcAclRule(newRule, queueId, m_strIngressTable, port); + if (createPfcAclTable(port, m_strIngressTable, true)) + { + shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strIngressTable); + if (!createPfcAclRule(newRule, queueId, m_strIngressTable, port)) + { + SWSS_LOG_ERROR("Failed to create ingress PFCWD drop rule %s", m_strRule.c_str()); + m_rolledBack = true; + return; + } + } + else + { + m_rolledBack = true; + return; + } } else { @@ -334,9 +346,14 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, if (rule == nullptr) { shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strIngressTable); - createPfcAclRule(newRule, queueId, m_strIngressTable, port); - } - else + if (!createPfcAclRule(newRule, queueId, m_strIngressTable, port)) + { + SWSS_LOG_ERROR("Failed to create ingress PFCWD drop rule %s", m_strRule.c_str()); + m_rolledBack = true; + return; + } + } + else { gAclOrch->updateAclRule(m_strIngressTable, m_strRule, MATCH_IN_PORTS, &port, RULE_OPER_ADD); } @@ -365,9 +382,21 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, if (found == m_aclTables.end()) { // First time of handling PFC, create ACL table and also ACL rule. - createPfcAclTable(port, m_strEgressTable, false); - shared_ptr newRule = make_shared(gAclOrch, m_strEgressRule, m_strEgressTable); - createPfcAclRule(newRule, queueId, m_strEgressTable, port); + if (createPfcAclTable(port, m_strEgressTable, false)) + { + shared_ptr newRule = make_shared(gAclOrch, m_strEgressRule, m_strEgressTable); + if (!createPfcAclRule(newRule, queueId, m_strEgressTable, port)) + { + SWSS_LOG_ERROR("Failed to create egress PFCWD drop rule %s", m_strEgressRule.c_str()); + removeIngressBinding(port); + m_rolledBack = true; + } + } + else + { + removeIngressBinding(port); + m_rolledBack = true; + } } else { @@ -376,7 +405,12 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, if (rule == nullptr) { shared_ptr newRule = make_shared(gAclOrch, m_strEgressRule, m_strEgressTable); - createPfcAclRule(newRule, queueId, m_strEgressTable, port); + if (!createPfcAclRule(newRule, queueId, m_strEgressTable, port)) + { + SWSS_LOG_ERROR("Failed to create egress PFCWD drop rule %s", m_strEgressRule.c_str()); + removeIngressBinding(port); + m_rolledBack = true; + } } } } @@ -387,9 +421,21 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, if (found == m_aclTables.end()) { // First time of handling PFC for this queue, create ACL table, and bind - createPfcAclTable(port, m_strEgressTable, false); - shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strEgressTable); - createPfcAclRule(newRule, queueId, m_strEgressTable, port); + if (createPfcAclTable(port, m_strEgressTable, false)) + { + shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strEgressTable); + if (!createPfcAclRule(newRule, queueId, m_strEgressTable, port)) + { + SWSS_LOG_ERROR("Failed to create egress PFCWD drop rule %s", m_strRule.c_str()); + removeIngressBinding(port); + m_rolledBack = true; + } + } + else + { + removeIngressBinding(port); + m_rolledBack = true; + } } else { @@ -399,41 +445,60 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, } } -PfcWdAclHandler::~PfcWdAclHandler(void) +void PfcWdAclHandler::removeIngressBinding(sai_object_id_t port) { SWSS_LOG_ENTER(); + // The rule may be absent if creation was skipped (egress PMF full) - clean up + // gracefully; this must not throw when called from the destructor. AclRule* rule = gAclOrch->getAclRule(m_strIngressTable, m_strRule); if (rule == nullptr) { - SWSS_LOG_THROW("ACL Rule does not exist for rule %s", m_strRule.c_str()); + SWSS_LOG_NOTICE("ACL Rule %s does not exist for table %s", m_strRule.c_str(), m_strIngressTable.c_str()); + return; } vector port_set = rule->getInPorts(); - sai_object_id_t port = getPort(); - if ((port_set.size() == 1) && (port_set[0] == port)) { gAclOrch->removeAclRule(m_strIngressTable, m_strRule); } - else + else { gAclOrch->updateAclRule(m_strIngressTable, m_strRule, MATCH_IN_PORTS, &port, RULE_OPER_DELETE); - } + } +} + +PfcWdAclHandler::~PfcWdAclHandler(void) +{ + SWSS_LOG_ENTER(); + + sai_object_id_t port = getPort(); + + if (!m_rolledBack) + { + removeIngressBinding(port); + } if (shared_egress_acl_table) { - rule = gAclOrch->getAclRule(m_strEgressTable, m_strEgressRule); + AclRule* rule = gAclOrch->getAclRule(m_strEgressTable, m_strEgressRule); if (rule == nullptr) { - SWSS_LOG_THROW("Egress ACL Rule does not exist for rule %s", m_strEgressRule.c_str()); + SWSS_LOG_NOTICE("Egress ACL Rule %s does not exist for table %s", m_strEgressRule.c_str(), m_strEgressTable.c_str()); + } + else + { + gAclOrch->removeAclRule(m_strEgressTable, m_strEgressRule); } - gAclOrch->removeAclRule(m_strEgressTable, m_strEgressRule); } else { auto found = m_aclTables.find(m_strEgressTable); - found->second.unbind(port); + if (found != m_aclTables.end()) + { + found->second.unbind(port); + } } } @@ -448,7 +513,7 @@ void PfcWdAclHandler::clear() } } -void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, bool ingress) +bool PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, bool ingress) { SWSS_LOG_ENTER(); @@ -466,7 +531,7 @@ void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, b // DROP ACL table is already created SWSS_LOG_NOTICE("ACL table %s exists, reuse the same", strTable.c_str()); aclTable = *(gAclOrch->getTableByOid(table_oid)); - return; + return true; } // Link port only for ingress ACL table or unshared egress ACL table. @@ -489,11 +554,20 @@ void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, b aclTable.validateAddType(*pfcwdType); aclTable.stage = ACL_STAGE_EGRESS; } - - gAclOrch->addAclTable(aclTable); + + if (!gAclOrch->addAclTable(aclTable)) + { + // ACL table create failed (e.g. egress PMF full). Drop the half-built + // entry so the caller skips rule creation instead of throwing later. + SWSS_LOG_ERROR("Failed to create PFCWD ACL table %s", strTable.c_str()); + m_aclTables.erase(strTable); + return false; + } + + return true; } -void PfcWdAclHandler::createPfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, sai_object_id_t portOid) +bool PfcWdAclHandler::createPfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, sai_object_id_t portOid) { SWSS_LOG_ENTER(); @@ -522,9 +596,9 @@ void PfcWdAclHandler::createPfcAclRule(shared_ptr rule, uint8_t q if (!gPortsOrch->getPort(portOid, p)) { SWSS_LOG_ERROR("Failed to get port structure from port oid 0x%" PRIx64, portOid); - return; + return false; } - + attr_value = p.m_alias; rule->validateAddMatch(attr_name, attr_value); } @@ -533,7 +607,7 @@ void PfcWdAclHandler::createPfcAclRule(shared_ptr rule, uint8_t q attr_value = PACKET_ACTION_DROP; rule->validateAddAction(attr_name, attr_value); - gAclOrch->addAclRule(rule, strTable); + return gAclOrch->addAclRule(rule, strTable); } std::map PfcWdAclHandler::m_aclTables; diff --git a/orchagent/pfcactionhandler.h b/orchagent/pfcactionhandler.h index 9fa409af306..98557b542af 100644 --- a/orchagent/pfcactionhandler.h +++ b/orchagent/pfcactionhandler.h @@ -46,6 +46,8 @@ class PfcWdActionHandler return m_queueId; } + virtual bool isValid(void) const { return true; } + static void initWdCounters(shared_ptr countersTable, const string &queueIdStr); void initCounters(void); void commitCounters(bool periodic = false); @@ -103,19 +105,24 @@ class PfcWdAclHandler: public PfcWdLossyHandler // class shared cleanup static void clear(); + + bool isValid(void) const override { return !m_rolledBack; } private: // class shared dict: ACL table name -> ACL table static std::map m_aclTables; bool shared_egress_acl_table = false; + bool m_rolledBack = false; + string m_strIngressTable; string m_strEgressTable; string m_strRule; string m_strEgressRule; - void createPfcAclTable(sai_object_id_t port, string strTable, bool ingress); - void createPfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, sai_object_id_t port); + bool createPfcAclTable(sai_object_id_t port, string strTable, bool ingress); + bool createPfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, sai_object_id_t port); void updatePfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, vector port); + void removeIngressBinding(sai_object_id_t port); }; class PfcWdDlrHandler: public PfcWdLossyHandler diff --git a/orchagent/pfcwdorch.cpp b/orchagent/pfcwdorch.cpp index 8aefd244f3d..123436dd56b 100644 --- a/orchagent/pfcwdorch.cpp +++ b/orchagent/pfcwdorch.cpp @@ -18,6 +18,7 @@ #define PFC_STAT_HISTORY "pfc_stat_history" #define BIG_RED_SWITCH_FIELD "BIG_RED_SWITCH" #define PFC_WD_IN_STORM "storm" +#define PFC_WD_SW_STATE_TABLE "PFC_WD_SW_STATE_TABLE" #define PFC_WD_DETECTION_TIME_MAX (5 * 1000) #define PFC_WD_DETECTION_TIME_MIN 100 @@ -538,6 +539,13 @@ void PfcWdSwOrch::enableBigRedSwitchMode() } } +template +void PfcWdSwOrch::setSwWdState(const string& portAlias, uint8_t queueIdx, const char* status) +{ + vector fvs = { { "status", status } }; + m_pfcWdSwStateTable->set(portAlias + ":" + to_string(queueIdx), fvs); +} + template bool PfcWdSwOrch::registerInWdDb(const Port& port, uint32_t detectionTime, uint32_t restorationTime, PfcWdAction action, string pfcStatHistory) @@ -614,6 +622,8 @@ bool PfcWdSwOrch::registerInWdDb(const Port& port, PfcWdActionHandler::initWdCounters( this->getCountersTable(), sai_serialize_object_id(queueId)); + + setSwWdState(port.m_alias, i, "configured"); } // We do NOT need to create ACL table group here. It will be @@ -688,6 +698,8 @@ void PfcWdSwOrch::unregisterFromWdDb(const Port& po + m_applTable->getTableNameSeparator() + port.m_alias; m_applDb->hdel(instormKey, to_string(i)); + + m_pfcWdSwStateTable->del(port.m_alias + ":" + to_string(i)); } } @@ -706,7 +718,9 @@ PfcWdSwOrch::PfcWdSwOrch( c_queueAttrIds(queueAttrIds), m_pollInterval(pollInterval), m_applDb(make_shared("APPL_DB", 0)), - m_applTable(make_shared
(m_applDb.get(), APP_PFC_WD_TABLE_NAME "_INSTORM")) + m_applTable(make_shared
(m_applDb.get(), APP_PFC_WD_TABLE_NAME "_INSTORM")), + m_stateDb(make_shared("STATE_DB", 0)), + m_pfcWdSwStateTable(make_shared
(m_stateDb.get(), PFC_WD_SW_STATE_TABLE)) { SWSS_LOG_ENTER(); @@ -998,6 +1012,10 @@ bool PfcWdSwOrch::startWdActionOnQueue(const string SWSS_LOG_NOTICE("Receive notification, %s", event.c_str()); + // Contain handler-construction failures (e.g. ACL table create on a full + // egress PMF) so no PFC event path aborts orchagent. + try + { if (m_bigRedSwitchFlag) { SWSS_LOG_NOTICE("Big_RED_SWITCH mode is on, ignore syncd pfc watchdog notification"); @@ -1016,9 +1034,20 @@ bool PfcWdSwOrch::startWdActionOnQueue(const string entry->second.index, this->getCountersTable()); entry->second.handler->initCounters(); - // Log storm event to APPL_DB for warm-reboot purpose - string key = m_applTable->getTableName() + m_applTable->getTableNameSeparator() + entry->second.portAlias; - m_applDb->hset(key, to_string(entry->second.index), PFC_WD_IN_STORM); + if (entry->second.handler->isValid()) + { + // Log storm event to APPL_DB for warm-reboot purpose + string key = m_applTable->getTableName() + m_applTable->getTableNameSeparator() + entry->second.portAlias; + m_applDb->hset(key, to_string(entry->second.index), PFC_WD_IN_STORM); + setSwWdState(entry->second.portAlias, entry->second.index, "configured"); + } + else + { + // No INSTORM: a storm we can't mitigate must not replay on warm reboot. + SWSS_LOG_WARN("PFC storm on port %s queue %d detected but drop action could not be installed (ACL create failed); queue is NOT being mitigated", + entry->second.portAlias.c_str(), entry->second.index); + setSwWdState(entry->second.portAlias, entry->second.index, "failed"); + } } } else if (entry->second.action == PfcWdAction::PFC_WD_ACTION_DROP) @@ -1033,9 +1062,20 @@ bool PfcWdSwOrch::startWdActionOnQueue(const string entry->second.index, this->getCountersTable()); entry->second.handler->initCounters(); - // Log storm event to APPL_DB for warm-reboot purpose - string key = m_applTable->getTableName() + m_applTable->getTableNameSeparator() + entry->second.portAlias; - m_applDb->hset(key, to_string(entry->second.index), PFC_WD_IN_STORM); + if (entry->second.handler->isValid()) + { + // Log storm event to APPL_DB for warm-reboot purpose + string key = m_applTable->getTableName() + m_applTable->getTableNameSeparator() + entry->second.portAlias; + m_applDb->hset(key, to_string(entry->second.index), PFC_WD_IN_STORM); + setSwWdState(entry->second.portAlias, entry->second.index, "configured"); + } + else + { + // No INSTORM: a storm we can't mitigate must not replay on warm reboot. + SWSS_LOG_WARN("PFC storm on port %s queue %d detected but drop action could not be installed (ACL create failed); queue is NOT being mitigated", + entry->second.portAlias.c_str(), entry->second.index); + setSwWdState(entry->second.portAlias, entry->second.index, "failed"); + } } } else if (entry->second.action == PfcWdAction::PFC_WD_ACTION_FORWARD) @@ -1050,9 +1090,20 @@ bool PfcWdSwOrch::startWdActionOnQueue(const string entry->second.index, this->getCountersTable()); entry->second.handler->initCounters(); - // Log storm event to APPL_DB for warm-reboot purpose - string key = m_applTable->getTableName() + m_applTable->getTableNameSeparator() + entry->second.portAlias; - m_applDb->hset(key, to_string(entry->second.index), PFC_WD_IN_STORM); + if (entry->second.handler->isValid()) + { + // Log storm event to APPL_DB for warm-reboot purpose + string key = m_applTable->getTableName() + m_applTable->getTableNameSeparator() + entry->second.portAlias; + m_applDb->hset(key, to_string(entry->second.index), PFC_WD_IN_STORM); + setSwWdState(entry->second.portAlias, entry->second.index, "configured"); + } + else + { + // No INSTORM: a storm we can't mitigate must not replay on warm reboot. + SWSS_LOG_WARN("PFC storm on port %s queue %d detected but drop action could not be installed (ACL create failed); queue is NOT being mitigated", + entry->second.portAlias.c_str(), entry->second.index); + setSwWdState(entry->second.portAlias, entry->second.index, "failed"); + } } } else @@ -1077,6 +1128,7 @@ bool PfcWdSwOrch::startWdActionOnQueue(const string // Remove storm status in APPL_DB for warm-reboot purpose string key = m_applTable->getTableName() + m_applTable->getTableNameSeparator() + entry->second.portAlias; m_applDb->hdel(key, to_string(entry->second.index)); + setSwWdState(entry->second.portAlias, entry->second.index, "configured"); } } else @@ -1084,6 +1136,12 @@ bool PfcWdSwOrch::startWdActionOnQueue(const string SWSS_LOG_ERROR("Received unknown event from plugin, %s", event.c_str()); return false; } + } + catch (const std::exception &e) + { + SWSS_LOG_ERROR("PFC watchdog %s action failed on queue 0x%" PRIx64 ": %s", event.c_str(), queueId, e.what()); + return false; + } return true; } diff --git a/orchagent/pfcwdorch.h b/orchagent/pfcwdorch.h index 69ed9ecfcc7..0afa4385306 100644 --- a/orchagent/pfcwdorch.h +++ b/orchagent/pfcwdorch.h @@ -123,6 +123,7 @@ class PfcWdSwOrch: public PfcWdOrch bool registerInWdDb(const Port& port, uint32_t detectionTime, uint32_t restorationTime, PfcWdAction action, string pfcStatHistory); void unregisterFromWdDb(const Port& port); + void setSwWdState(const string& portAlias, uint8_t queueIdx, const char* status); void doTask(swss::NotificationConsumer &wdNotification); unordered_set filterPfcCounters(const unordered_set &counters, set& losslessTc); @@ -147,6 +148,8 @@ class PfcWdSwOrch: public PfcWdOrch shared_ptr m_applDb = nullptr; // Track queues in storm shared_ptr
m_applTable = nullptr; + shared_ptr m_stateDb = nullptr; + shared_ptr
m_pfcWdSwStateTable = nullptr; }; #endif From d72be06959f1ec33d6b92a70b619c0cd71a50fad Mon Sep 17 00:00:00 2001 From: Rajath V Date: Tue, 14 Jul 2026 05:05:02 +0000 Subject: [PATCH 2/2] [pfcwd] Address review: failure logs with SAI status, egress table rollback - 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 --- orchagent/pfcactionhandler.cpp | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/orchagent/pfcactionhandler.cpp b/orchagent/pfcactionhandler.cpp index 566c4b515ad..7025ef155cc 100644 --- a/orchagent/pfcactionhandler.cpp +++ b/orchagent/pfcactionhandler.cpp @@ -329,13 +329,16 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strIngressTable); if (!createPfcAclRule(newRule, queueId, m_strIngressTable, port)) { - SWSS_LOG_ERROR("Failed to create ingress PFCWD drop rule %s", m_strRule.c_str()); + SWSS_LOG_ERROR("Failed to create ingress PFCWD drop rule %s, last SAI status %s", + m_strRule.c_str(), sai_serialize_status(newRule->getLastSaiStatus()).c_str()); m_rolledBack = true; return; } } else { + SWSS_LOG_ERROR("Failed to create ingress PFCWD ACL table %s; PFCWD drop action is not installed", + m_strIngressTable.c_str()); m_rolledBack = true; return; } @@ -348,7 +351,8 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strIngressTable); if (!createPfcAclRule(newRule, queueId, m_strIngressTable, port)) { - SWSS_LOG_ERROR("Failed to create ingress PFCWD drop rule %s", m_strRule.c_str()); + SWSS_LOG_ERROR("Failed to create ingress PFCWD drop rule %s, last SAI status %s", + m_strRule.c_str(), sai_serialize_status(newRule->getLastSaiStatus()).c_str()); m_rolledBack = true; return; } @@ -387,13 +391,20 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, shared_ptr newRule = make_shared(gAclOrch, m_strEgressRule, m_strEgressTable); if (!createPfcAclRule(newRule, queueId, m_strEgressTable, port)) { - SWSS_LOG_ERROR("Failed to create egress PFCWD drop rule %s", m_strEgressRule.c_str()); + SWSS_LOG_ERROR("Failed to create egress PFCWD drop rule %s, last SAI status %s", + m_strEgressRule.c_str(), sai_serialize_status(newRule->getLastSaiStatus()).c_str()); + // Roll back the egress table created just above (no rule could be + // installed in it) along with the ingress drop rule. + gAclOrch->removeAclTable(m_strEgressTable); + m_aclTables.erase(m_strEgressTable); removeIngressBinding(port); m_rolledBack = true; } } else { + SWSS_LOG_ERROR("Failed to create egress PFCWD ACL table %s; rolling back ingress PFCWD drop rule %s", + m_strEgressTable.c_str(), m_strRule.c_str()); removeIngressBinding(port); m_rolledBack = true; } @@ -407,7 +418,9 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, shared_ptr newRule = make_shared(gAclOrch, m_strEgressRule, m_strEgressTable); if (!createPfcAclRule(newRule, queueId, m_strEgressTable, port)) { - SWSS_LOG_ERROR("Failed to create egress PFCWD drop rule %s", m_strEgressRule.c_str()); + // The egress table is shared with other queues/ports - leave it in place. + SWSS_LOG_ERROR("Failed to create egress PFCWD drop rule %s, last SAI status %s", + m_strEgressRule.c_str(), sai_serialize_status(newRule->getLastSaiStatus()).c_str()); removeIngressBinding(port); m_rolledBack = true; } @@ -426,13 +439,20 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strEgressTable); if (!createPfcAclRule(newRule, queueId, m_strEgressTable, port)) { - SWSS_LOG_ERROR("Failed to create egress PFCWD drop rule %s", m_strRule.c_str()); + SWSS_LOG_ERROR("Failed to create egress PFCWD drop rule %s, last SAI status %s", + m_strRule.c_str(), sai_serialize_status(newRule->getLastSaiStatus()).c_str()); + // Roll back the egress table created just above (no rule could be + // installed in it) along with the ingress drop rule. + gAclOrch->removeAclTable(m_strEgressTable); + m_aclTables.erase(m_strEgressTable); removeIngressBinding(port); m_rolledBack = true; } } else { + SWSS_LOG_ERROR("Failed to create egress PFCWD ACL table %s; rolling back ingress PFCWD drop rule %s", + m_strEgressTable.c_str(), m_strRule.c_str()); removeIngressBinding(port); m_rolledBack = true; }