From 0f069108215ec5cb456e87554309ee99febb302d Mon Sep 17 00:00:00 2001 From: Nazarii Hnydyn Date: Wed, 30 Mar 2022 15:17:45 +0300 Subject: [PATCH] [PBH] Implement Edit Flows (#2169) PBH Edit Flows is a second phase of PBH feature implementation. It allows user to modify the already existing objects. PBH Edit Flows offer a full entity update which assumes Config DB field ADD/UPDATE/REMOVE processing. PBH OA interprets series of Redis HSET/HDEL as SONiC SET operation and internally does the diff of existing/supplied configuration to deploy the new changes. HLD: Azure/SONiC#909 - What I did Implemented Edit Flows in scope of PBH enhancement - Why I did it Implementation is done according to the PBH HLD Signed-off-by: Nazarii Hnydyn --- orchagent/Makefile.am | 1 + orchagent/pbh/pbhcap.cpp | 688 +++++++++++++++++++++++++++++++++++ orchagent/pbh/pbhcap.h | 186 ++++++++++ orchagent/pbh/pbhcnt.h | 9 +- orchagent/pbh/pbhmgr.cpp | 43 +-- orchagent/pbh/pbhrule.cpp | 47 +++ orchagent/pbh/pbhrule.h | 1 + orchagent/pbh/pbhschema.h | 37 ++ orchagent/pbhorch.cpp | 437 +++++++++++++++++++--- orchagent/pbhorch.h | 13 +- tests/dvslib/dvs_acl.py | 47 +++ tests/dvslib/dvs_database.py | 18 + tests/dvslib/dvs_pbh.py | 30 ++ tests/mock_tests/Makefile.am | 1 + tests/test_pbh.py | 105 ++++++ 15 files changed, 1567 insertions(+), 96 deletions(-) create mode 100644 orchagent/pbh/pbhcap.cpp create mode 100644 orchagent/pbh/pbhcap.h create mode 100644 orchagent/pbh/pbhschema.h diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index f6026d613f57..fedd7ef4aefc 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -62,6 +62,7 @@ orchagent_SOURCES = \ mirrororch.cpp \ fdborch.cpp \ aclorch.cpp \ + pbh/pbhcap.cpp \ pbh/pbhcnt.cpp \ pbh/pbhmgr.cpp \ pbh/pbhrule.cpp \ diff --git a/orchagent/pbh/pbhcap.cpp b/orchagent/pbh/pbhcap.cpp new file mode 100644 index 000000000000..46a3a49e19f8 --- /dev/null +++ b/orchagent/pbh/pbhcap.cpp @@ -0,0 +1,688 @@ +// includes ----------------------------------------------------------------------------------------------------------- + +#include +#include +#include +#include +#include +#include + +#include "pbhschema.h" +#include "schema.h" +#include "logger.h" + +#include "pbhcap.h" + +using namespace swss; + +// defines ------------------------------------------------------------------------------------------------------------ + +#define PBH_PLATFORM_ENV_VAR "ASIC_VENDOR" +#define PBH_PLATFORM_GENERIC "generic" +#define PBH_PLATFORM_MELLANOX "mellanox" +#define PBH_PLATFORM_UNKN "unknown" + +#define PBH_TABLE_CAPABILITIES_KEY "table" +#define PBH_RULE_CAPABILITIES_KEY "rule" +#define PBH_HASH_CAPABILITIES_KEY "hash" +#define PBH_HASH_FIELD_CAPABILITIES_KEY "hash-field" + +#define PBH_FIELD_CAPABILITY_ADD "ADD" +#define PBH_FIELD_CAPABILITY_UPDATE "UPDATE" +#define PBH_FIELD_CAPABILITY_REMOVE "REMOVE" +#define PBH_FIELD_CAPABILITY_UNKN "UNKNOWN" + +#define PBH_STATE_DB_NAME "STATE_DB" +#define PBH_STATE_DB_TIMEOUT 0 + +// constants ---------------------------------------------------------------------------------------------------------- + +static const std::map pbhAsicVendorMap = +{ + { PbhAsicVendor::GENERIC, PBH_PLATFORM_GENERIC }, + { PbhAsicVendor::MELLANOX, PBH_PLATFORM_MELLANOX } +}; + +static const std::map pbhFieldCapabilityMap = +{ + { PbhFieldCapability::ADD, PBH_FIELD_CAPABILITY_ADD }, + { PbhFieldCapability::UPDATE, PBH_FIELD_CAPABILITY_UPDATE }, + { PbhFieldCapability::REMOVE, PBH_FIELD_CAPABILITY_REMOVE } +}; + +// functions ---------------------------------------------------------------------------------------------------------- + +static std::string toStr(PbhAsicVendor value) noexcept +{ + const auto &cit = pbhAsicVendorMap.find(value); + if (cit != pbhAsicVendorMap.cend()) + { + return cit->second; + } + + return PBH_PLATFORM_UNKN; +} + +static std::string toStr(PbhFieldCapability value) noexcept +{ + const auto &cit = pbhFieldCapabilityMap.find(value); + if (cit != pbhFieldCapabilityMap.cend()) + { + return cit->second; + } + + return PBH_FIELD_CAPABILITY_UNKN; +} + +static std::string toStr(const std::set &value) noexcept +{ + std::stringstream ss; + bool separator = false; + + for (const auto &cit : value) + { + if (!separator) + { + ss << toStr(cit); + separator = true; + } + else + { + ss << "," << toStr(cit); + } + } + + return ss.str(); +} + +// PBH field capabilities --------------------------------------------------------------------------------------------- + +void PbhVendorFieldCapabilities::setPbhDefaults(std::set &fieldCap) noexcept +{ + fieldCap.insert(PbhFieldCapability::ADD); + fieldCap.insert(PbhFieldCapability::UPDATE); + fieldCap.insert(PbhFieldCapability::REMOVE); +} + +PbhGenericFieldCapabilities::PbhGenericFieldCapabilities() noexcept +{ + this->table.interface_list.insert(PbhFieldCapability::UPDATE); + this->table.description.insert(PbhFieldCapability::UPDATE); + + this->rule.priority.insert(PbhFieldCapability::UPDATE); + this->setPbhDefaults(this->rule.gre_key); + this->setPbhDefaults(this->rule.ether_type); + this->setPbhDefaults(this->rule.ip_protocol); + this->setPbhDefaults(this->rule.ipv6_next_header); + this->setPbhDefaults(this->rule.l4_dst_port); + this->setPbhDefaults(this->rule.inner_ether_type); + this->rule.hash.insert(PbhFieldCapability::UPDATE); + this->setPbhDefaults(this->rule.packet_action); + this->setPbhDefaults(this->rule.flow_counter); + + this->hash.hash_field_list.insert(PbhFieldCapability::UPDATE); +} + +PbhMellanoxFieldCapabilities::PbhMellanoxFieldCapabilities() noexcept +{ + this->table.interface_list.insert(PbhFieldCapability::UPDATE); + this->table.description.insert(PbhFieldCapability::UPDATE); + + this->rule.priority.insert(PbhFieldCapability::UPDATE); + this->setPbhDefaults(this->rule.gre_key); + this->setPbhDefaults(this->rule.ether_type); + this->setPbhDefaults(this->rule.ip_protocol); + this->setPbhDefaults(this->rule.ipv6_next_header); + this->setPbhDefaults(this->rule.l4_dst_port); + this->setPbhDefaults(this->rule.inner_ether_type); + this->rule.hash.insert(PbhFieldCapability::UPDATE); + this->setPbhDefaults(this->rule.packet_action); + this->setPbhDefaults(this->rule.flow_counter); +} + +// PBH entity capabilities -------------------------------------------------------------------------------------------- + +PbhEntityCapabilities::PbhEntityCapabilities(const std::shared_ptr &fieldCap) noexcept : + fieldCap(fieldCap) +{ + +} + +bool PbhEntityCapabilities::validate(const std::set &fieldCap, PbhFieldCapability value) const +{ + const auto &cit = fieldCap.find(value); + if (cit == fieldCap.cend()) + { + return false; + } + + return true; +} + +PbhTableCapabilities::PbhTableCapabilities(const std::shared_ptr &fieldCap) noexcept : + PbhEntityCapabilities(fieldCap) +{ + +} + +bool PbhTableCapabilities::validatePbhInterfaceList(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().interface_list, value); +} + +bool PbhTableCapabilities::validatePbhDescription(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().description, value); +} + +auto PbhTableCapabilities::getPbhCap() const -> const decltype(PbhVendorFieldCapabilities::table) & +{ + return this->fieldCap->table; +} + +PbhRuleCapabilities::PbhRuleCapabilities(const std::shared_ptr &fieldCap) noexcept : + PbhEntityCapabilities(fieldCap) +{ + +} + +bool PbhRuleCapabilities::validatePbhPriority(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().priority, value); +} + +bool PbhRuleCapabilities::validatePbhGreKey(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().gre_key, value); +} + +bool PbhRuleCapabilities::validatePbhEtherType(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().ether_type, value); +} + +bool PbhRuleCapabilities::validatePbhIpProtocol(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().ip_protocol, value); +} + +bool PbhRuleCapabilities::validatePbhIpv6NextHeader(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().ipv6_next_header, value); +} + +bool PbhRuleCapabilities::validatePbhL4DstPort(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().l4_dst_port, value); +} + +bool PbhRuleCapabilities::validatePbhInnerEtherType(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().inner_ether_type, value); +} + +bool PbhRuleCapabilities::validatePbhHash(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().hash, value); +} + +bool PbhRuleCapabilities::validatePbhPacketAction(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().packet_action, value); +} + +bool PbhRuleCapabilities::validatePbhFlowCounter(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().flow_counter, value); +} + +auto PbhRuleCapabilities::getPbhCap() const -> const decltype(PbhVendorFieldCapabilities::rule) & +{ + return this->fieldCap->rule; +} + +PbhHashCapabilities::PbhHashCapabilities(const std::shared_ptr &fieldCap) noexcept : + PbhEntityCapabilities(fieldCap) +{ + +} + +bool PbhHashCapabilities::validatePbhHashFieldList(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().hash_field_list, value); +} + +auto PbhHashCapabilities::getPbhCap() const -> const decltype(PbhVendorFieldCapabilities::hash) & +{ + return this->fieldCap->hash; +} + +PbhHashFieldCapabilities::PbhHashFieldCapabilities(const std::shared_ptr &fieldCap) noexcept : + PbhEntityCapabilities(fieldCap) +{ + +} + +bool PbhHashFieldCapabilities::validatePbhHashField(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().hash_field, value); +} + +bool PbhHashFieldCapabilities::validatePbhIpMask(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().ip_mask, value); +} + +bool PbhHashFieldCapabilities::validatePbhSequenceId(PbhFieldCapability value) const +{ + return this->validate(this->getPbhCap().sequence_id, value); +} + +auto PbhHashFieldCapabilities::getPbhCap() const -> const decltype(PbhVendorFieldCapabilities::hashField) & +{ + return this->fieldCap->hashField; +} + +// PBH capabilities --------------------------------------------------------------------------------------------------- + +DBConnector PbhCapabilities::stateDb(PBH_STATE_DB_NAME, PBH_STATE_DB_TIMEOUT); +Table PbhCapabilities::capTable(&stateDb, STATE_PBH_CAPABILITIES_TABLE_NAME); + +PbhCapabilities::PbhCapabilities() noexcept +{ + SWSS_LOG_ENTER(); + + if (!this->parsePbhAsicVendor()) + { + SWSS_LOG_WARN("Failed to parse ASIC vendor: fallback to %s platform", PBH_PLATFORM_GENERIC); + this->asicVendor = PbhAsicVendor::GENERIC; + } + + this->initPbhVendorCapabilities(); + this->writePbhVendorCapabilitiesToDb(); +} + +PbhAsicVendor PbhCapabilities::getAsicVendor() const noexcept +{ + return this->asicVendor; +} + +bool PbhCapabilities::parsePbhAsicVendor() +{ + SWSS_LOG_ENTER(); + + const auto *envVar = std::getenv(PBH_PLATFORM_ENV_VAR); + if (envVar == nullptr) + { + SWSS_LOG_WARN("Failed to detect ASIC vendor: environmental variable(%s) is not found", PBH_PLATFORM_ENV_VAR); + return false; + } + + std::string platform(envVar); + + if (platform == PBH_PLATFORM_MELLANOX) + { + this->asicVendor = PbhAsicVendor::MELLANOX; + } + else + { + this->asicVendor = PbhAsicVendor::GENERIC; + } + + SWSS_LOG_NOTICE("Parsed PBH ASIC vendor: %s", toStr(this->asicVendor).c_str()); + + return true; +} + +void PbhCapabilities::initPbhVendorCapabilities() +{ + std::shared_ptr fieldCap; + + switch (this->asicVendor) + { + case PbhAsicVendor::GENERIC: + fieldCap = std::make_shared(); + break; + + case PbhAsicVendor::MELLANOX: + fieldCap = std::make_shared(); + break; + + default: + SWSS_LOG_WARN("Unknown ASIC vendor: skipping ..."); + break; + } + + if (!fieldCap) + { + SWSS_LOG_ERROR("Failed to initialize PBH capabilities: unknown ASIC vendor"); + return; + } + + this->table = std::make_shared(fieldCap); + this->rule = std::make_shared(fieldCap); + this->hash = std::make_shared(fieldCap); + this->hashField = std::make_shared(fieldCap); + + SWSS_LOG_NOTICE("Initialized PBH capabilities: %s platform", toStr(this->asicVendor).c_str()); +} + +template<> +void PbhCapabilities::writePbhEntityCapabilitiesToDb(const std::shared_ptr &entityCap) +{ + SWSS_LOG_ENTER(); + + auto key = PbhCapabilities::capTable.getKeyName(PBH_TABLE_CAPABILITIES_KEY); + std::vector fvList; + + fvList.push_back(FieldValueTuple(PBH_TABLE_INTERFACE_LIST, toStr(entityCap->getPbhCap().interface_list))); + fvList.push_back(FieldValueTuple(PBH_TABLE_DESCRIPTION, toStr(entityCap->getPbhCap().description))); + + PbhCapabilities::capTable.set(PBH_TABLE_CAPABILITIES_KEY, fvList); + + SWSS_LOG_NOTICE("Wrote PBH table capabilities to State DB: %s key", key.c_str()); +} + +template<> +void PbhCapabilities::writePbhEntityCapabilitiesToDb(const std::shared_ptr &entityCap) +{ + SWSS_LOG_ENTER(); + + auto key = PbhCapabilities::capTable.getKeyName(PBH_RULE_CAPABILITIES_KEY); + std::vector fvList; + + fvList.push_back(FieldValueTuple(PBH_RULE_PRIORITY, toStr(entityCap->getPbhCap().priority))); + fvList.push_back(FieldValueTuple(PBH_RULE_GRE_KEY, toStr(entityCap->getPbhCap().gre_key))); + fvList.push_back(FieldValueTuple(PBH_RULE_ETHER_TYPE, toStr(entityCap->getPbhCap().ether_type))); + fvList.push_back(FieldValueTuple(PBH_RULE_IP_PROTOCOL, toStr(entityCap->getPbhCap().ip_protocol))); + fvList.push_back(FieldValueTuple(PBH_RULE_IPV6_NEXT_HEADER, toStr(entityCap->getPbhCap().ipv6_next_header))); + fvList.push_back(FieldValueTuple(PBH_RULE_L4_DST_PORT, toStr(entityCap->getPbhCap().l4_dst_port))); + fvList.push_back(FieldValueTuple(PBH_RULE_INNER_ETHER_TYPE, toStr(entityCap->getPbhCap().inner_ether_type))); + fvList.push_back(FieldValueTuple(PBH_RULE_HASH, toStr(entityCap->getPbhCap().hash))); + fvList.push_back(FieldValueTuple(PBH_RULE_PACKET_ACTION, toStr(entityCap->getPbhCap().packet_action))); + fvList.push_back(FieldValueTuple(PBH_RULE_FLOW_COUNTER, toStr(entityCap->getPbhCap().flow_counter))); + + PbhCapabilities::capTable.set(PBH_RULE_CAPABILITIES_KEY, fvList); + + SWSS_LOG_NOTICE("Wrote PBH rule capabilities to State DB: %s key", key.c_str()); +} + +template<> +void PbhCapabilities::writePbhEntityCapabilitiesToDb(const std::shared_ptr &entityCap) +{ + SWSS_LOG_ENTER(); + + auto key = PbhCapabilities::capTable.getKeyName(PBH_HASH_CAPABILITIES_KEY); + std::vector fvList; + + fvList.push_back(FieldValueTuple(PBH_HASH_HASH_FIELD_LIST, toStr(entityCap->getPbhCap().hash_field_list))); + + PbhCapabilities::capTable.set(PBH_HASH_CAPABILITIES_KEY, fvList); + + SWSS_LOG_NOTICE("Wrote PBH hash capabilities to State DB: %s key", key.c_str()); +} + +template<> +void PbhCapabilities::writePbhEntityCapabilitiesToDb(const std::shared_ptr &entityCap) +{ + SWSS_LOG_ENTER(); + + auto key = PbhCapabilities::capTable.getKeyName(PBH_HASH_FIELD_CAPABILITIES_KEY); + std::vector fvList; + + fvList.push_back(FieldValueTuple(PBH_HASH_FIELD_HASH_FIELD, toStr(entityCap->getPbhCap().hash_field))); + fvList.push_back(FieldValueTuple(PBH_HASH_FIELD_IP_MASK, toStr(entityCap->getPbhCap().ip_mask))); + fvList.push_back(FieldValueTuple(PBH_HASH_FIELD_SEQUENCE_ID, toStr(entityCap->getPbhCap().sequence_id))); + + PbhCapabilities::capTable.set(PBH_HASH_FIELD_CAPABILITIES_KEY, fvList); + + SWSS_LOG_NOTICE("Wrote PBH hash field capabilities to State DB: %s key", key.c_str()); +} + +void PbhCapabilities::writePbhVendorCapabilitiesToDb() +{ + SWSS_LOG_ENTER(); + + this->writePbhEntityCapabilitiesToDb(this->table); + this->writePbhEntityCapabilitiesToDb(this->rule); + this->writePbhEntityCapabilitiesToDb(this->hash); + this->writePbhEntityCapabilitiesToDb(this->hashField); + + SWSS_LOG_NOTICE("Wrote PBH capabilities to State DB: %s table", STATE_PBH_CAPABILITIES_TABLE_NAME); +} + +bool PbhCapabilities::validatePbhTableCap(const std::vector &fieldList, PbhFieldCapability value) const +{ + SWSS_LOG_ENTER(); + + for (const auto &cit : fieldList) + { + if (cit == PBH_TABLE_INTERFACE_LIST) + { + if (!this->table->validatePbhInterfaceList(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_TABLE_DESCRIPTION) + { + if (!this->table->validatePbhDescription(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else + { + SWSS_LOG_WARN("Unknown field(%s): skipping ...", cit.c_str()); + } + } + + return true; +} + +bool PbhCapabilities::validatePbhRuleCap(const std::vector &fieldList, PbhFieldCapability value) const +{ + SWSS_LOG_ENTER(); + + for (const auto &cit : fieldList) + { + if (cit == PBH_RULE_PRIORITY) + { + if (!this->rule->validatePbhPriority(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_RULE_GRE_KEY) + { + if (!this->rule->validatePbhGreKey(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_RULE_ETHER_TYPE) + { + if (!this->rule->validatePbhEtherType(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_RULE_IP_PROTOCOL) + { + if (!this->rule->validatePbhIpProtocol(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_RULE_IPV6_NEXT_HEADER) + { + if (!this->rule->validatePbhIpv6NextHeader(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_RULE_L4_DST_PORT) + { + if (!this->rule->validatePbhL4DstPort(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_RULE_INNER_ETHER_TYPE) + { + if (!this->rule->validatePbhInnerEtherType(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_RULE_HASH) + { + if (!this->rule->validatePbhHash(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_RULE_PACKET_ACTION) + { + if (!this->rule->validatePbhPacketAction(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_RULE_FLOW_COUNTER) + { + if (!this->rule->validatePbhFlowCounter(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else + { + SWSS_LOG_WARN("Unknown field(%s): skipping ...", cit.c_str()); + } + } + + return true; +} + +bool PbhCapabilities::validatePbhHashCap(const std::vector &fieldList, PbhFieldCapability value) const +{ + SWSS_LOG_ENTER(); + + for (const auto &cit : fieldList) + { + if (cit == PBH_HASH_HASH_FIELD_LIST) + { + if (!this->hash->validatePbhHashFieldList(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else + { + SWSS_LOG_WARN("Unknown field(%s): skipping ...", cit.c_str()); + } + } + + return true; +} + +bool PbhCapabilities::validatePbhHashFieldCap(const std::vector &fieldList, PbhFieldCapability value) const +{ + SWSS_LOG_ENTER(); + + for (const auto &cit : fieldList) + { + if (cit == PBH_HASH_FIELD_HASH_FIELD) + { + if (!this->hashField->validatePbhHashField(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_HASH_FIELD_IP_MASK) + { + if (!this->hashField->validatePbhIpMask(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else if (cit == PBH_HASH_FIELD_SEQUENCE_ID) + { + if (!this->hashField->validatePbhSequenceId(value)) + { + SWSS_LOG_ERROR("Failed to validate field(%s): capability(%s) is not supported", + cit.c_str(), + toStr(value).c_str() + ); + return false; + } + } + else + { + SWSS_LOG_WARN("Unknown field(%s): skipping ...", cit.c_str()); + } + } + + return true; +} diff --git a/orchagent/pbh/pbhcap.h b/orchagent/pbh/pbhcap.h new file mode 100644 index 000000000000..adc2a4c9e665 --- /dev/null +++ b/orchagent/pbh/pbhcap.h @@ -0,0 +1,186 @@ +#pragma once + +#include +#include +#include +#include +#include + +#include "dbconnector.h" +#include "table.h" + +enum class PbhAsicVendor : std::int32_t +{ + GENERIC, + MELLANOX +}; + +enum class PbhFieldCapability : std::int32_t +{ + ADD, + UPDATE, + REMOVE +}; + +class PbhVendorFieldCapabilities +{ +public: + PbhVendorFieldCapabilities() = default; + virtual ~PbhVendorFieldCapabilities() = default; + +protected: + void setPbhDefaults(std::set &fieldCap) noexcept; + +public: + struct { + std::set interface_list; + std::set description; + } table; + + struct { + std::set priority; + std::set gre_key; + std::set ether_type; + std::set ip_protocol; + std::set ipv6_next_header; + std::set l4_dst_port; + std::set inner_ether_type; + std::set hash; + std::set packet_action; + std::set flow_counter; + } rule; + + struct { + std::set hash_field_list; + } hash; + + struct { + std::set hash_field; + std::set ip_mask; + std::set sequence_id; + } hashField; +}; + +class PbhGenericFieldCapabilities final : public PbhVendorFieldCapabilities +{ +public: + PbhGenericFieldCapabilities() noexcept; + ~PbhGenericFieldCapabilities() = default; +}; + +class PbhMellanoxFieldCapabilities final : public PbhVendorFieldCapabilities +{ +public: + PbhMellanoxFieldCapabilities() noexcept; + ~PbhMellanoxFieldCapabilities() = default; +}; + +class PbhEntityCapabilities +{ +public: + PbhEntityCapabilities() = delete; + virtual ~PbhEntityCapabilities() = default; + + PbhEntityCapabilities(const std::shared_ptr &fieldCap) noexcept; + +protected: + bool validate(const std::set &fieldCap, PbhFieldCapability value) const; + + std::shared_ptr fieldCap; +}; + +class PbhTableCapabilities final : public PbhEntityCapabilities +{ +public: + PbhTableCapabilities() = delete; + ~PbhTableCapabilities() = default; + + PbhTableCapabilities(const std::shared_ptr &fieldCap) noexcept; + + bool validatePbhInterfaceList(PbhFieldCapability value) const; + bool validatePbhDescription(PbhFieldCapability value) const; + + auto getPbhCap() const -> const decltype(PbhVendorFieldCapabilities::table) &; +}; + +class PbhRuleCapabilities final : public PbhEntityCapabilities +{ +public: + PbhRuleCapabilities() = delete; + ~PbhRuleCapabilities() = default; + + PbhRuleCapabilities(const std::shared_ptr &fieldCap) noexcept; + + bool validatePbhPriority(PbhFieldCapability value) const; + bool validatePbhGreKey(PbhFieldCapability value) const; + bool validatePbhEtherType(PbhFieldCapability value) const; + bool validatePbhIpProtocol(PbhFieldCapability value) const; + bool validatePbhIpv6NextHeader(PbhFieldCapability value) const; + bool validatePbhL4DstPort(PbhFieldCapability value) const; + bool validatePbhInnerEtherType(PbhFieldCapability value) const; + bool validatePbhHash(PbhFieldCapability value) const; + bool validatePbhPacketAction(PbhFieldCapability value) const; + bool validatePbhFlowCounter(PbhFieldCapability value) const; + + auto getPbhCap() const -> const decltype(PbhVendorFieldCapabilities::rule) &; +}; + +class PbhHashCapabilities final : public PbhEntityCapabilities +{ +public: + PbhHashCapabilities() = delete; + ~PbhHashCapabilities() = default; + + PbhHashCapabilities(const std::shared_ptr &fieldCap) noexcept; + + bool validatePbhHashFieldList(PbhFieldCapability value) const; + + auto getPbhCap() const -> const decltype(PbhVendorFieldCapabilities::hash) &; +}; + +class PbhHashFieldCapabilities final : public PbhEntityCapabilities +{ +public: + PbhHashFieldCapabilities() = delete; + ~PbhHashFieldCapabilities() = default; + + PbhHashFieldCapabilities(const std::shared_ptr &fieldCap) noexcept; + + bool validatePbhHashField(PbhFieldCapability value) const; + bool validatePbhIpMask(PbhFieldCapability value) const; + bool validatePbhSequenceId(PbhFieldCapability value) const; + + auto getPbhCap() const -> const decltype(PbhVendorFieldCapabilities::hashField) &; +}; + +class PbhCapabilities final +{ +public: + PbhCapabilities() noexcept; + ~PbhCapabilities() = default; + + bool validatePbhTableCap(const std::vector &fieldList, PbhFieldCapability value) const; + bool validatePbhRuleCap(const std::vector &fieldList, PbhFieldCapability value) const; + bool validatePbhHashCap(const std::vector &fieldList, PbhFieldCapability value) const; + bool validatePbhHashFieldCap(const std::vector &fieldList, PbhFieldCapability value) const; + + PbhAsicVendor getAsicVendor() const noexcept; + +private: + template + void writePbhEntityCapabilitiesToDb(const std::shared_ptr &entityCap); + + bool parsePbhAsicVendor(); + void initPbhVendorCapabilities(); + void writePbhVendorCapabilitiesToDb(); + + PbhAsicVendor asicVendor; + + std::shared_ptr table; + std::shared_ptr rule; + std::shared_ptr hash; + std::shared_ptr hashField; + + static swss::DBConnector stateDb; + static swss::Table capTable; +}; diff --git a/orchagent/pbh/pbhcnt.h b/orchagent/pbh/pbhcnt.h index 787d91b63c7d..90c40bb681a2 100644 --- a/orchagent/pbh/pbhcnt.h +++ b/orchagent/pbh/pbhcnt.h @@ -105,19 +105,22 @@ class PbhRule final : public PbhContainer } inner_ether_type; struct { + struct { + std::string name; + } meta; std::string value; bool is_set = false; } hash; struct { + struct { + std::string name; + } meta; sai_acl_entry_attr_t value; bool is_set = false; } packet_action; struct { - struct { - std::string name; - } meta; bool value; bool is_set = false; } flow_counter; diff --git a/orchagent/pbh/pbhmgr.cpp b/orchagent/pbh/pbhmgr.cpp index ed10ff756cd6..8dfb8e09f810 100644 --- a/orchagent/pbh/pbhmgr.cpp +++ b/orchagent/pbh/pbhmgr.cpp @@ -7,6 +7,7 @@ #include #include +#include "pbhschema.h" #include "ipaddress.h" #include "converter.h" #include "tokenize.h" @@ -16,42 +17,6 @@ using namespace swss; -// defines ------------------------------------------------------------------------------------------------------------ - -#define PBH_TABLE_INTERFACE_LIST "interface_list" -#define PBH_TABLE_DESCRIPTION "description" - -#define PBH_RULE_PACKET_ACTION_SET_ECMP_HASH "SET_ECMP_HASH" -#define PBH_RULE_PACKET_ACTION_SET_LAG_HASH "SET_LAG_HASH" - -#define PBH_RULE_FLOW_COUNTER_ENABLED "ENABLED" -#define PBH_RULE_FLOW_COUNTER_DISABLED "DISABLED" - -#define PBH_RULE_PRIORITY "priority" -#define PBH_RULE_GRE_KEY "gre_key" -#define PBH_RULE_ETHER_TYPE "ether_type" -#define PBH_RULE_IP_PROTOCOL "ip_protocol" -#define PBH_RULE_IPV6_NEXT_HEADER "ipv6_next_header" -#define PBH_RULE_L4_DST_PORT "l4_dst_port" -#define PBH_RULE_INNER_ETHER_TYPE "inner_ether_type" -#define PBH_RULE_HASH "hash" -#define PBH_RULE_PACKET_ACTION "packet_action" -#define PBH_RULE_FLOW_COUNTER "flow_counter" - -#define PBH_HASH_HASH_FIELD_LIST "hash_field_list" - -#define PBH_HASH_FIELD_HASH_FIELD_INNER_IP_PROTOCOL "INNER_IP_PROTOCOL" -#define PBH_HASH_FIELD_HASH_FIELD_INNER_L4_DST_PORT "INNER_L4_DST_PORT" -#define PBH_HASH_FIELD_HASH_FIELD_INNER_L4_SRC_PORT "INNER_L4_SRC_PORT" -#define PBH_HASH_FIELD_HASH_FIELD_INNER_DST_IPV4 "INNER_DST_IPV4" -#define PBH_HASH_FIELD_HASH_FIELD_INNER_SRC_IPV4 "INNER_SRC_IPV4" -#define PBH_HASH_FIELD_HASH_FIELD_INNER_DST_IPV6 "INNER_DST_IPV6" -#define PBH_HASH_FIELD_HASH_FIELD_INNER_SRC_IPV6 "INNER_SRC_IPV6" - -#define PBH_HASH_FIELD_HASH_FIELD "hash_field" -#define PBH_HASH_FIELD_IP_MASK "ip_mask" -#define PBH_HASH_FIELD_SEQUENCE_ID "sequence_id" - // constants ---------------------------------------------------------------------------------------------------------- static const std::unordered_map pbhRulePacketActionMap = @@ -712,6 +677,7 @@ bool PbhHelper::parsePbhRuleHash(PbhRule &rule, const std::string &field, const return false; } + rule.hash.meta.name = field; rule.hash.value = value; rule.hash.is_set = true; @@ -729,6 +695,7 @@ bool PbhHelper::parsePbhRulePacketAction(PbhRule &rule, const std::string &field return false; } + rule.packet_action.meta.name = field; rule.packet_action.value = cit->second; rule.packet_action.is_set = true; @@ -746,7 +713,6 @@ bool PbhHelper::parsePbhRuleFlowCounter(PbhRule &rule, const std::string &field, return false; } - rule.flow_counter.meta.name = field; rule.flow_counter.value = cit->second; rule.flow_counter.is_set = true; @@ -1036,6 +1002,7 @@ bool PbhHelper::validatePbhRule(PbhRule &rule) const PBH_RULE_PACKET_ACTION_SET_ECMP_HASH ); + rule.packet_action.meta.name = PBH_RULE_PACKET_ACTION; rule.packet_action.value = SAI_ACL_ENTRY_ATTR_ACTION_SET_ECMP_HASH_ID; rule.packet_action.is_set = true; @@ -1049,7 +1016,7 @@ bool PbhHelper::validatePbhRule(PbhRule &rule) const PBH_RULE_FLOW_COUNTER, PBH_RULE_FLOW_COUNTER_DISABLED ); - rule.flow_counter.meta.name = PBH_RULE_FLOW_COUNTER; + rule.flow_counter.value = false; rule.flow_counter.is_set = true; diff --git a/orchagent/pbh/pbhrule.cpp b/orchagent/pbh/pbhrule.cpp index 7d35f4bb8fdf..0b00e40e4426 100644 --- a/orchagent/pbh/pbhrule.cpp +++ b/orchagent/pbh/pbhrule.cpp @@ -98,3 +98,50 @@ bool AclRulePbh::validateAddAction(string attr_name, string attr_value) { SWSS_LOG_THROW("This API should not be used on PbhRule"); } + +bool AclRulePbh::disableAction() +{ + const auto &cit1 = m_actions.find(SAI_ACL_ENTRY_ATTR_ACTION_SET_ECMP_HASH_ID); + if (cit1 != m_actions.cend()) + { + const auto &attr1 = cit1->second.getSaiAttr(); + if (attr1.value.aclaction.enable) + { + sai_attribute_t attr; + + attr.id = attr1.id; + attr.value.aclaction.enable = false; + attr.value.aclaction.parameter.oid = SAI_NULL_OBJECT_ID; + + if (!setAttribute(attr)) + { + return false; + } + + m_actions.erase(cit1); + } + } + + const auto &cit2 = m_actions.find(SAI_ACL_ENTRY_ATTR_ACTION_SET_LAG_HASH_ID); + if (cit2 != m_actions.cend()) + { + const auto &attr2 = cit2->second.getSaiAttr(); + if (attr2.value.aclaction.enable) + { + sai_attribute_t attr; + + attr.id = attr2.id; + attr.value.aclaction.enable = false; + attr.value.aclaction.parameter.oid = SAI_NULL_OBJECT_ID; + + if (!setAttribute(attr)) + { + return false; + } + + m_actions.erase(cit2); + } + } + + return true; +} diff --git a/orchagent/pbh/pbhrule.h b/orchagent/pbh/pbhrule.h index 9e661761c406..5fa5ddf1fc42 100644 --- a/orchagent/pbh/pbhrule.h +++ b/orchagent/pbh/pbhrule.h @@ -13,4 +13,5 @@ class AclRulePbh: public AclRule bool validate() override; void onUpdate(SubjectType, void *) override; bool validateAddAction(string attr_name, string attr_value) override; + bool disableAction(); }; diff --git a/orchagent/pbh/pbhschema.h b/orchagent/pbh/pbhschema.h new file mode 100644 index 000000000000..3ea280f76952 --- /dev/null +++ b/orchagent/pbh/pbhschema.h @@ -0,0 +1,37 @@ +#pragma once + +// defines ------------------------------------------------------------------------------------------------------------ + +#define PBH_TABLE_INTERFACE_LIST "interface_list" +#define PBH_TABLE_DESCRIPTION "description" + +#define PBH_RULE_PACKET_ACTION_SET_ECMP_HASH "SET_ECMP_HASH" +#define PBH_RULE_PACKET_ACTION_SET_LAG_HASH "SET_LAG_HASH" + +#define PBH_RULE_FLOW_COUNTER_ENABLED "ENABLED" +#define PBH_RULE_FLOW_COUNTER_DISABLED "DISABLED" + +#define PBH_RULE_PRIORITY "priority" +#define PBH_RULE_GRE_KEY "gre_key" +#define PBH_RULE_ETHER_TYPE "ether_type" +#define PBH_RULE_IP_PROTOCOL "ip_protocol" +#define PBH_RULE_IPV6_NEXT_HEADER "ipv6_next_header" +#define PBH_RULE_L4_DST_PORT "l4_dst_port" +#define PBH_RULE_INNER_ETHER_TYPE "inner_ether_type" +#define PBH_RULE_HASH "hash" +#define PBH_RULE_PACKET_ACTION "packet_action" +#define PBH_RULE_FLOW_COUNTER "flow_counter" + +#define PBH_HASH_HASH_FIELD_LIST "hash_field_list" + +#define PBH_HASH_FIELD_HASH_FIELD_INNER_IP_PROTOCOL "INNER_IP_PROTOCOL" +#define PBH_HASH_FIELD_HASH_FIELD_INNER_L4_DST_PORT "INNER_L4_DST_PORT" +#define PBH_HASH_FIELD_HASH_FIELD_INNER_L4_SRC_PORT "INNER_L4_SRC_PORT" +#define PBH_HASH_FIELD_HASH_FIELD_INNER_DST_IPV4 "INNER_DST_IPV4" +#define PBH_HASH_FIELD_HASH_FIELD_INNER_SRC_IPV4 "INNER_SRC_IPV4" +#define PBH_HASH_FIELD_HASH_FIELD_INNER_DST_IPV6 "INNER_DST_IPV6" +#define PBH_HASH_FIELD_HASH_FIELD_INNER_SRC_IPV6 "INNER_SRC_IPV6" + +#define PBH_HASH_FIELD_HASH_FIELD "hash_field" +#define PBH_HASH_FIELD_IP_MASK "ip_mask" +#define PBH_HASH_FIELD_SEQUENCE_ID "sequence_id" diff --git a/orchagent/pbhorch.cpp b/orchagent/pbhorch.cpp index 83a1e80bd043..e2146cb3621e 100644 --- a/orchagent/pbhorch.cpp +++ b/orchagent/pbhorch.cpp @@ -53,7 +53,26 @@ static inline std::vector uMapDiffByKey(const umap_t &uMap1, const umap const auto &s1 = uMapToKeySet(uMap1); const auto &s2 = uMapToKeySet(uMap2); - std::set_symmetric_difference( + std::set_difference( + s1.cbegin(), + s1.cend(), + s2.cbegin(), + s2.cend(), + std::back_inserter(v) + ); + + return v; +} + +template +static inline std::vector uMapIntersectByKey(const umap_t &uMap1, const umap_t &uMap2) +{ + std::vector v; + + const auto &s1 = uMapToKeySet(uMap1); + const auto &s2 = uMapToKeySet(uMap2); + + std::set_intersection( s1.cbegin(), s1.cend(), s2.cbegin(), @@ -76,11 +95,52 @@ PbhOrch::PbhOrch( this->portsOrch = portsOrch; } -PbhOrch::~PbhOrch() +template +std::vector PbhOrch::getPbhAddedFields(const T &obj, const T &nObj) const +{ + return uMapDiffByKey(nObj.fieldValueMap, obj.fieldValueMap); +} + +template std::vector PbhOrch::getPbhAddedFields(const PbhTable &obj, const PbhTable &nObj) const; +template std::vector PbhOrch::getPbhAddedFields(const PbhRule &obj, const PbhRule &nObj) const; +template std::vector PbhOrch::getPbhAddedFields(const PbhHash &obj, const PbhHash &nObj) const; +template std::vector PbhOrch::getPbhAddedFields(const PbhHashField &obj, const PbhHashField &nObj) const; + +template +std::vector PbhOrch::getPbhUpdatedFields(const T &obj, const T &nObj) const { + std::vector v; + + const auto &iv = uMapIntersectByKey(obj.fieldValueMap, nObj.fieldValueMap); + std::copy_if( + iv.cbegin(), + iv.cend(), + std::back_inserter(v), + [&obj, &nObj](const auto &f) { + return obj.fieldValueMap.at(f) != nObj.fieldValueMap.at(f); + } + ); + + return v; } +template std::vector PbhOrch::getPbhUpdatedFields(const PbhTable &obj, const PbhTable &nObj) const; +template std::vector PbhOrch::getPbhUpdatedFields(const PbhRule &obj, const PbhRule &nObj) const; +template std::vector PbhOrch::getPbhUpdatedFields(const PbhHash &obj, const PbhHash &nObj) const; +template std::vector PbhOrch::getPbhUpdatedFields(const PbhHashField &obj, const PbhHashField &nObj) const; + +template +std::vector PbhOrch::getPbhRemovedFields(const T &obj, const T &nObj) const +{ + return uMapDiffByKey(obj.fieldValueMap, nObj.fieldValueMap); +} + +template std::vector PbhOrch::getPbhRemovedFields(const PbhTable &obj, const PbhTable &nObj) const; +template std::vector PbhOrch::getPbhRemovedFields(const PbhRule &obj, const PbhRule &nObj) const; +template std::vector PbhOrch::getPbhRemovedFields(const PbhHash &obj, const PbhHash &nObj) const; +template std::vector PbhOrch::getPbhRemovedFields(const PbhHashField &obj, const PbhHashField &nObj) const; + template<> auto PbhOrch::getPbhSetupTaskMap() const -> const std::unordered_map& { @@ -252,6 +312,34 @@ bool PbhOrch::updatePbhTable(const PbhTable &table) return false; } + const auto &aFields = this->getPbhAddedFields(tObj, table); + const auto &uFields = this->getPbhUpdatedFields(tObj, table); + const auto &rFields = this->getPbhRemovedFields(tObj, table); + + if (aFields.empty() && uFields.empty() && rFields.empty()) + { + SWSS_LOG_NOTICE("PBH table(%s) in SAI is up-to-date", table.key.c_str()); + return true; + } + + if (!this->pbhCap.validatePbhTableCap(aFields, PbhFieldCapability::ADD)) + { + SWSS_LOG_ERROR("Failed to validate PBH table(%s) added fields: unsupported capabilities", table.key.c_str()); + return false; + } + + if (!this->pbhCap.validatePbhTableCap(uFields, PbhFieldCapability::UPDATE)) + { + SWSS_LOG_ERROR("Failed to validate PBH table(%s) updated fields: unsupported capabilities", table.key.c_str()); + return false; + } + + if (!this->pbhCap.validatePbhTableCap(rFields, PbhFieldCapability::REMOVE)) + { + SWSS_LOG_ERROR("Failed to validate PBH table(%s) removed fields: unsupported capabilities", table.key.c_str()); + return false; + } + AclTable pbhTable(this->aclOrch, table.name); if (table.interface_list.is_set) @@ -577,57 +665,227 @@ bool PbhOrch::updatePbhRule(const PbhRule &rule) return false; } - if (!uMapDiffByKey(rObj.fieldValueMap, rule.fieldValueMap).empty()) + const auto &aFields = this->getPbhAddedFields(rObj, rule); + const auto &uFields = this->getPbhUpdatedFields(rObj, rule); + const auto &rFields = this->getPbhRemovedFields(rObj, rule); + + if (aFields.empty() && uFields.empty() && rFields.empty()) + { + SWSS_LOG_NOTICE("PBH rule(%s) in SAI is up-to-date", rule.key.c_str()); + return true; + } + + if (!this->pbhCap.validatePbhRuleCap(aFields, PbhFieldCapability::ADD)) { - SWSS_LOG_ERROR("Failed to update PBH rule(%s) in SAI: fields add/remove is prohibited", rule.key.c_str()); + SWSS_LOG_ERROR("Failed to validate PBH rule(%s) added fields: unsupported capabilities", rule.key.c_str()); return false; } - bool flowCounterUpdate = false; + if (!this->pbhCap.validatePbhRuleCap(uFields, PbhFieldCapability::UPDATE)) + { + SWSS_LOG_ERROR("Failed to validate PBH rule(%s) updated fields: unsupported capabilities", rule.key.c_str()); + return false; + } - for (const auto &oCit : rObj.fieldValueMap) + if (!this->pbhCap.validatePbhRuleCap(rFields, PbhFieldCapability::REMOVE)) { - const auto &field = oCit.first; + SWSS_LOG_ERROR("Failed to validate PBH rule(%s) removed fields: unsupported capabilities", rule.key.c_str()); + return false; + } - const auto &oValue = oCit.second; - const auto &nValue = rule.fieldValueMap.at(field); + std::shared_ptr pbhRule; + + if (rule.flow_counter.is_set) + { + pbhRule = std::make_shared(this->aclOrch, rule.name, rule.table, rule.flow_counter.value); + } + else + { + pbhRule = std::make_shared(this->aclOrch, rule.name, rule.table); + } - if (oValue == nValue) + if (rule.priority.is_set) + { + if (!pbhRule->validateAddPriority(rule.priority.value)) { - continue; + SWSS_LOG_ERROR("Failed to configure PBH rule(%s) priority", rule.key.c_str()); + return false; } + } + + if (rule.gre_key.is_set) + { + sai_attribute_t attr; - if (field != rule.flow_counter.meta.name) + attr.id = SAI_ACL_ENTRY_ATTR_FIELD_GRE_KEY; + attr.value.aclfield.enable = true; + attr.value.aclfield.data.u32 = rule.gre_key.value; + attr.value.aclfield.mask.u32 = rule.gre_key.mask; + + if (!pbhRule->validateAddMatch(attr)) { - SWSS_LOG_ERROR( - "Failed to update PBH rule(%s) in SAI: field(%s) update is prohibited", - rule.key.c_str(), - field.c_str() - ); + SWSS_LOG_ERROR("Failed to configure PBH rule(%s) match: GRE_KEY", rule.key.c_str()); return false; } + } - flowCounterUpdate = true; + if (rule.ether_type.is_set) + { + sai_attribute_t attr; + + attr.id = SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE; + attr.value.aclfield.enable = true; + attr.value.aclfield.data.u16 = rule.ether_type.value; + attr.value.aclfield.mask.u16 = rule.ether_type.mask; + + if (!pbhRule->validateAddMatch(attr)) + { + SWSS_LOG_ERROR("Failed to configure PBH rule(%s) match: ETHER_TYPE", rule.key.c_str()); + return false; + } } - if (!flowCounterUpdate) + if (rule.ip_protocol.is_set) { - SWSS_LOG_NOTICE("PBH rule(%s) in SAI is up-to-date", rule.key.c_str()); - return true; + sai_attribute_t attr; + + attr.id = SAI_ACL_ENTRY_ATTR_FIELD_IP_PROTOCOL; + attr.value.aclfield.enable = true; + attr.value.aclfield.data.u8 = rule.ip_protocol.value; + attr.value.aclfield.mask.u8 = rule.ip_protocol.mask; + + if (!pbhRule->validateAddMatch(attr)) + { + SWSS_LOG_ERROR("Failed to configure PBH rule(%s) match: IP_PROTOCOL", rule.key.c_str()); + return false; + } + } + + if (rule.ipv6_next_header.is_set) + { + sai_attribute_t attr; + + attr.id = SAI_ACL_ENTRY_ATTR_FIELD_IPV6_NEXT_HEADER; + attr.value.aclfield.enable = true; + attr.value.aclfield.data.u8 = rule.ipv6_next_header.value; + attr.value.aclfield.mask.u8 = rule.ipv6_next_header.mask; + + if (!pbhRule->validateAddMatch(attr)) + { + SWSS_LOG_ERROR("Failed to configure PBH rule(%s) match: IPV6_NEXT_HEADER", rule.key.c_str()); + return false; + } + } + + if (rule.l4_dst_port.is_set) + { + sai_attribute_t attr; + + attr.id = SAI_ACL_ENTRY_ATTR_FIELD_L4_DST_PORT; + attr.value.aclfield.enable = true; + attr.value.aclfield.data.u16 = rule.l4_dst_port.value; + attr.value.aclfield.mask.u16 = rule.l4_dst_port.mask; + + if (!pbhRule->validateAddMatch(attr)) + { + SWSS_LOG_ERROR("Failed to configure PBH rule(%s) match: L4_DST_PORT", rule.key.c_str()); + return false; + } + } + + if (rule.inner_ether_type.is_set) + { + sai_attribute_t attr; + + attr.id = SAI_ACL_ENTRY_ATTR_FIELD_INNER_ETHER_TYPE; + attr.value.aclfield.enable = true; + attr.value.aclfield.data.u16 = rule.inner_ether_type.value; + attr.value.aclfield.mask.u16 = rule.inner_ether_type.mask; + + if (!pbhRule->validateAddMatch(attr)) + { + SWSS_LOG_ERROR("Failed to configure PBH rule(%s) match: INNER_ETHER_TYPE", rule.key.c_str()); + return false; + } } - if (!this->aclOrch->updateAclRule(rule.table, rule.name, rule.flow_counter.value)) + if (rule.hash.is_set && rule.packet_action.is_set) + { + PbhHash hObj; + + if (this->pbhHlpr.getPbhHash(hObj, rule.hash.value)) + { + sai_attribute_t attr; + + attr.id = rule.packet_action.value; + attr.value.aclaction.enable = true; + attr.value.aclaction.parameter.oid = hObj.getOid(); + + if (!pbhRule->validateAddAction(attr)) + { + SWSS_LOG_ERROR("Failed to configure PBH rule(%s) action", rule.key.c_str()); + return false; + } + } + } + + if (!pbhRule->validate()) + { + SWSS_LOG_ERROR("Failed to validate PBH rule(%s)", rule.key.c_str()); + return false; + } + + // Mellanox W/A + if (this->pbhCap.getAsicVendor() == PbhAsicVendor::MELLANOX) + { + const auto &hMeta = rule.hash.meta; + const auto &paMeta = rule.packet_action.meta; + + auto cond1 = std::find(uFields.cbegin(), uFields.cend(), hMeta.name) != uFields.cend(); + auto cond2 = std::find(uFields.cbegin(), uFields.cend(), paMeta.name) != uFields.cend(); + + if (cond1 || cond2) + { + auto pbhRulePtr = dynamic_cast(this->aclOrch->getAclRule(rule.table, rule.name)); + + if (pbhRulePtr == nullptr) + { + SWSS_LOG_ERROR("Failed to update PBH rule(%s) in SAI: invalid object type", rule.key.c_str()); + return false; + } + + if (!pbhRulePtr->disableAction()) + { + SWSS_LOG_ERROR("Failed to disable PBH rule(%s) action", rule.key.c_str()); + return false; + } + } + } + + if (!this->aclOrch->updateAclRule(pbhRule)) { SWSS_LOG_ERROR("Failed to update PBH rule(%s) in SAI", rule.key.c_str()); return false; } + if (!this->pbhHlpr.decRefCount(rObj)) + { + SWSS_LOG_ERROR("Failed to remove PBH rule(%s) dependencies", rObj.key.c_str()); + return false; + } + if (!this->pbhHlpr.updatePbhRule(rule)) { SWSS_LOG_ERROR("Failed to update PBH rule(%s) in internal cache", rule.key.c_str()); return false; } + if (!this->pbhHlpr.incRefCount(rule)) + { + SWSS_LOG_ERROR("Failed to add PBH rule(%s) dependencies", rule.key.c_str()); + return false; + } + SWSS_LOG_NOTICE("Updated PBH rule(%s) in SAI", rule.key.c_str()); return true; @@ -832,31 +1090,98 @@ bool PbhOrch::updatePbhHash(const PbhHash &hash) return false; } - if (!uMapDiffByKey(hObj.fieldValueMap, hash.fieldValueMap).empty()) + const auto &aFields = this->getPbhAddedFields(hObj, hash); + const auto &uFields = this->getPbhUpdatedFields(hObj, hash); + const auto &rFields = this->getPbhRemovedFields(hObj, hash); + + if (aFields.empty() && uFields.empty() && rFields.empty()) { - SWSS_LOG_ERROR("Failed to update PBH hash(%s) in SAI: fields add/remove is prohibited", hash.key.c_str()); + SWSS_LOG_NOTICE("PBH hash(%s) in SAI is up-to-date", hash.key.c_str()); + return true; + } + + if (!this->pbhCap.validatePbhHashCap(aFields, PbhFieldCapability::ADD)) + { + SWSS_LOG_ERROR("Failed to validate PBH hash(%s) added fields: unsupported capabilities", hash.key.c_str()); + return false; + } + + if (!this->pbhCap.validatePbhHashCap(uFields, PbhFieldCapability::UPDATE)) + { + SWSS_LOG_ERROR("Failed to validate PBH hash(%s) updated fields: unsupported capabilities", hash.key.c_str()); return false; } - for (const auto &oCit : hObj.fieldValueMap) + if (!this->pbhCap.validatePbhHashCap(rFields, PbhFieldCapability::REMOVE)) { - const auto &field = oCit.first; + SWSS_LOG_ERROR("Failed to validate PBH hash(%s) removed fields: unsupported capabilities", hash.key.c_str()); + return false; + } - const auto &oValue = oCit.second; - const auto &nValue = hash.fieldValueMap.at(field); + std::vector hashFieldOidList; - if (oValue != nValue) + if (hash.hash_field_list.is_set) + { + for (const auto &cit : hash.hash_field_list.value) { - SWSS_LOG_ERROR( - "Failed to update PBH hash(%s) in SAI: field(%s) update is prohibited", - hash.key.c_str(), - field.c_str() - ); - return false; + PbhHashField hfObj; + + if (!this->pbhHlpr.getPbhHashField(hfObj, cit)) + { + SWSS_LOG_ERROR( + "Failed to update PBH hash(%s) in SAI: missing hash field(%s)", + hash.key.c_str(), + cit.c_str() + ); + return false; + } + + hashFieldOidList.push_back(hfObj.getOid()); } } - SWSS_LOG_NOTICE("PBH hash(%s) in SAI is up-to-date", hash.key.c_str()); + if (hashFieldOidList.empty()) + { + SWSS_LOG_ERROR("Failed to update PBH hash(%s) in SAI: missing hash fields", hash.key.c_str()); + return false; + } + + sai_attribute_t attr; + + attr.id = SAI_HASH_ATTR_FINE_GRAINED_HASH_FIELD_LIST; + attr.value.objlist.count = static_cast(hashFieldOidList.size()); + attr.value.objlist.list = hashFieldOidList.data(); + + sai_status_t status; + + status = sai_hash_api->set_hash_attribute(hObj.getOid(), &attr); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to update PBH hash(%s) in SAI", hash.key.c_str()); + return false; + } + + if (!this->pbhHlpr.decRefCount(hObj)) + { + SWSS_LOG_ERROR("Failed to remove PBH hash(%s) dependencies", hObj.key.c_str()); + return false; + } + + hObj.hash_field_list = hash.hash_field_list; + + if (!this->pbhHlpr.updatePbhHash(hObj)) + { + SWSS_LOG_ERROR("Failed to update PBH hash(%s) in internal cache", hObj.key.c_str()); + return false; + } + + if (!this->pbhHlpr.incRefCount(hObj)) + { + SWSS_LOG_ERROR("Failed to add PBH hash(%s) dependencies", hObj.key.c_str()); + return false; + } + + SWSS_LOG_NOTICE("Updated PBH hash(%s) in SAI", hObj.key.c_str()); return true; } @@ -1072,33 +1397,37 @@ bool PbhOrch::updatePbhHashField(const PbhHashField &hashField) return false; } - if (!uMapDiffByKey(hfObj.fieldValueMap, hashField.fieldValueMap).empty()) + const auto &aFields = this->getPbhAddedFields(hfObj, hashField); + const auto &uFields = this->getPbhUpdatedFields(hfObj, hashField); + const auto &rFields = this->getPbhRemovedFields(hfObj, hashField); + + if (aFields.empty() && uFields.empty() && rFields.empty()) { - SWSS_LOG_ERROR("Failed to update PBH hash field(%s) in SAI: fields add/remove is prohibited", hashField.key.c_str()); - return false; + SWSS_LOG_NOTICE("PBH hash field(%s) in SAI is up-to-date", hashField.key.c_str()); + return true; } - for (const auto &oCit : hfObj.fieldValueMap) + if (!this->pbhCap.validatePbhHashFieldCap(aFields, PbhFieldCapability::ADD)) { - const auto &field = oCit.first; + SWSS_LOG_ERROR("Failed to validate PBH hash field(%s) added fields: unsupported capabilities", hashField.key.c_str()); + return false; + } - const auto &oValue = oCit.second; - const auto &nValue = hashField.fieldValueMap.at(field); + if (!this->pbhCap.validatePbhHashFieldCap(uFields, PbhFieldCapability::UPDATE)) + { + SWSS_LOG_ERROR("Failed to validate PBH hash field(%s) updated fields: unsupported capabilities", hashField.key.c_str()); + return false; + } - if (oValue != nValue) - { - SWSS_LOG_ERROR( - "Failed to update PBH hash field(%s) in SAI: field(%s) update is prohibited", - hashField.key.c_str(), - field.c_str() - ); - return false; - } + if (!this->pbhCap.validatePbhHashFieldCap(rFields, PbhFieldCapability::REMOVE)) + { + SWSS_LOG_ERROR("Failed to validate PBH hash field(%s) removed fields: unsupported capabilities", hashField.key.c_str()); + return false; } - SWSS_LOG_NOTICE("PBH hash field(%s) in SAI is up-to-date", hashField.key.c_str()); + SWSS_LOG_ERROR("Failed to update PBH hash field(%s) in SAI: update is prohibited", hfObj.key.c_str()); - return true; + return false; } bool PbhOrch::removePbhHashField(const PbhHashField &hashField) diff --git a/orchagent/pbhorch.h b/orchagent/pbhorch.h index 1aa49e1d26bb..250963f54a14 100644 --- a/orchagent/pbhorch.h +++ b/orchagent/pbhorch.h @@ -8,20 +8,30 @@ #include "pbh/pbhrule.h" #include "pbh/pbhmgr.h" +#include "pbh/pbhcap.h" class PbhOrch final : public Orch { public: + PbhOrch() = delete; + ~PbhOrch() = default; + PbhOrch( std::vector &connectorList, AclOrch *aclOrch, PortsOrch *portsOrch ); - ~PbhOrch(); using Orch::doTask; // Allow access to the basic doTask private: + template + std::vector getPbhAddedFields(const T &obj, const T &nObj) const; + template + std::vector getPbhUpdatedFields(const T &obj, const T &nObj) const; + template + std::vector getPbhRemovedFields(const T &obj, const T &nObj) const; + template auto getPbhSetupTaskMap() const -> const std::unordered_map&; template @@ -75,4 +85,5 @@ class PbhOrch final : public Orch PortsOrch *portsOrch; PbhHelper pbhHlpr; + PbhCapabilities pbhCap; }; diff --git a/tests/dvslib/dvs_acl.py b/tests/dvslib/dvs_acl.py index 9111de7a8e04..266761c5685d 100644 --- a/tests/dvslib/dvs_acl.py +++ b/tests/dvslib/dvs_acl.py @@ -16,6 +16,7 @@ class DVSAcl: ADB_ACL_TABLE_NAME = "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE" ADB_ACL_GROUP_TABLE_NAME = "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP" ADB_ACL_GROUP_MEMBER_TABLE_NAME = "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP_MEMBER" + ADB_ACL_COUNTER_TABLE_NAME = "ASIC_STATE:SAI_OBJECT_TYPE_ACL_COUNTER" ADB_ACL_STAGE_LOOKUP = { "ingress": "SAI_ACL_STAGE_INGRESS", @@ -140,6 +141,19 @@ def remove_acl_table_type(self, name: str) -> None: """ self.config_db.delete_entry(self.CDB_ACL_TABLE_TYPE_NAME, name) + def get_acl_counter_ids(self, expected: int) -> List[str]: + """Get all of the ACL counter IDs in ASIC DB. + + This method will wait for the expected number of counters to exist, or fail. + + Args: + expected: The number of counters that are expected to be present in ASIC DB. + + Returns: + The list of ACL counter IDs in ASIC DB. + """ + return self.asic_db.wait_for_n_keys(self.ADB_ACL_COUNTER_TABLE_NAME, expected) + def get_acl_table_ids(self, expected: int) -> List[str]: """Get all of the ACL table IDs in ASIC DB. @@ -530,6 +544,39 @@ def verify_mirror_acl_rule( self._check_acl_entry_mirror_action(fvs, session_oid, stage) self._check_acl_entry_counters_map(acl_rule_id) + def verify_acl_rule_generic( + self, + sai_qualifiers: Dict[str, str], + acl_table_id: str = None, + acl_rule_id: str = None + ) -> None: + """Verify that an ACL rule has the correct ASIC DB representation. + + Args: + sai_qualifiers: The expected set of SAI qualifiers to be found in ASIC DB. + acl_table_id: A specific OID to check in ASIC DB. If left empty, this method + assumes that only one table exists in ASIC DB. + acl_rule_id: A specific OID to check in ASIC DB. If left empty, this method + assumes that only one rule exists in ASIC DB. + """ + if not acl_table_id: + acl_table_id = self.get_acl_table_ids(1)[0] + + if not acl_rule_id: + acl_rule_id = self._get_acl_rule_id() + + entry = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", acl_rule_id) + + for k, v in entry.items(): + if k == "SAI_ACL_ENTRY_ATTR_TABLE_ID": + assert v == acl_table_id + elif k == "SAI_ACL_ENTRY_ATTR_ADMIN_STATE": + assert v == "true" + elif k in sai_qualifiers: + assert sai_qualifiers[k](v) + else: + assert False, "Unknown SAI qualifier: key={}, value={}".format(k, v) + def verify_acl_rule_set( self, priorities: List[str], diff --git a/tests/dvslib/dvs_database.py b/tests/dvslib/dvs_database.py index f2657f75161b..4256b5802d29 100644 --- a/tests/dvslib/dvs_database.py +++ b/tests/dvslib/dvs_database.py @@ -34,6 +34,24 @@ def create_entry(self, table_name: str, key: str, entry: Dict[str, str]) -> None formatted_entry = swsscommon.FieldValuePairs(list(entry.items())) table.set(key, formatted_entry) + def set_entry(self, table_name: str, key: str, entry: Dict[str, str]) -> None: + """Set entry of an existing key in the specified table. + + Args: + table_name: The name of the table. + key: The key that needs to be updated. + entry: A set of key-value pairs to be updated. + """ + table = swsscommon.Table(self.db_connection, table_name) + (status, fv_pairs) = table.get(key) + + formatted_entry = swsscommon.FieldValuePairs(list(entry.items())) + table.set(key, formatted_entry) + + if status: + for f in [ k for k, v in dict(fv_pairs).items() if k not in entry.keys() ]: + table.hdel(key, f) + def update_entry(self, table_name: str, key: str, entry: Dict[str, str]) -> None: """Update entry of an existing key in the specified table. diff --git a/tests/dvslib/dvs_pbh.py b/tests/dvslib/dvs_pbh.py index 79a58681a9cd..df612638ead1 100644 --- a/tests/dvslib/dvs_pbh.py +++ b/tests/dvslib/dvs_pbh.py @@ -10,6 +10,8 @@ class DVSPbh: CDB_PBH_HASH = "PBH_HASH" CDB_PBH_HASH_FIELD = "PBH_HASH_FIELD" + ADB_PBH_HASH = "ASIC_STATE:SAI_OBJECT_TYPE_HASH" + def __init__(self, asic_db, config_db): """Create a new DVS PBH Manager.""" self.asic_db = asic_db @@ -60,6 +62,27 @@ def create_pbh_rule( self.config_db.create_entry(self.CDB_PBH_RULE, "{}|{}".format(table_name, rule_name), attr_dict) + def update_pbh_rule( + self, + table_name: str, + rule_name: str, + priority: str, + qualifiers: Dict[str, str], + hash_name: str, + packet_action: str = "SET_ECMP_HASH", + flow_counter: str = "DISABLED" + ) -> None: + """Update PBH rule in Config DB.""" + attr_dict = { + "priority": priority, + "hash": hash_name, + "packet_action": packet_action, + "flow_counter": flow_counter, + **qualifiers + } + + self.config_db.set_entry(self.CDB_PBH_RULE, "{}|{}".format(table_name, rule_name), attr_dict) + def remove_pbh_rule( self, table_name: str, @@ -125,3 +148,10 @@ def verify_pbh_hash_field_count( ) -> None: """Verify that there are N hash field objects in ASIC DB.""" self.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_FINE_GRAINED_HASH_FIELD", expected) + + def get_pbh_hash_ids( + self, + expected: int + ) -> List[str]: + """Get all of the PBH hash IDs in ASIC DB.""" + return self.asic_db.wait_for_n_keys(self.ADB_PBH_HASH, expected) diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 2489bef6d289..15bc47bd700d 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -59,6 +59,7 @@ tests_SOURCES = aclorch_ut.cpp \ $(top_srcdir)/orchagent/mirrororch.cpp \ $(top_srcdir)/orchagent/fdborch.cpp \ $(top_srcdir)/orchagent/aclorch.cpp \ + $(top_srcdir)/orchagent/pbh/pbhcap.cpp \ $(top_srcdir)/orchagent/pbh/pbhcnt.cpp \ $(top_srcdir)/orchagent/pbh/pbhmgr.cpp \ $(top_srcdir)/orchagent/pbh/pbhrule.cpp \ diff --git a/tests/test_pbh.py b/tests/test_pbh.py index 328e8231bcfe..65155ba2e95c 100644 --- a/tests/test_pbh.py +++ b/tests/test_pbh.py @@ -253,6 +253,111 @@ def test_PbhRuleCreationDeletion(self, testlog): self.dvs_pbh.verify_pbh_hash_field_count(0) +class TestPbhBasicEditFlows: + def test_PbhRuleUpdate(self, testlog): + try: + # PBH hash field + pbhlogger.info("Create PBH hash field: {}".format(PBH_HASH_FIELD_NAME)) + self.dvs_pbh.create_pbh_hash_field( + hash_field_name=PBH_HASH_FIELD_NAME, + hash_field=PBH_HASH_FIELD_HASH_FIELD, + sequence_id=PBH_HASH_FIELD_SEQUENCE_ID + ) + self.dvs_pbh.verify_pbh_hash_field_count(1) + + # PBH hash + pbhlogger.info("Create PBH hash: {}".format(PBH_HASH_NAME)) + self.dvs_pbh.create_pbh_hash( + hash_name=PBH_HASH_NAME, + hash_field_list=PBH_HASH_HASH_FIELD_LIST + ) + self.dvs_pbh.verify_pbh_hash_count(1) + + # PBH table + pbhlogger.info("Create PBH table: {}".format(PBH_TABLE_NAME)) + self.dvs_pbh.create_pbh_table( + table_name=PBH_TABLE_NAME, + interface_list=PBH_TABLE_INTERFACE_LIST, + description=PBH_TABLE_DESCRIPTION + ) + self.dvs_acl.verify_acl_table_count(1) + + # PBH rule + attr_dict = { + "ether_type": PBH_RULE_ETHER_TYPE, + "ip_protocol": PBH_RULE_IP_PROTOCOL, + "gre_key": PBH_RULE_GRE_KEY, + "inner_ether_type": PBH_RULE_INNER_ETHER_TYPE + } + + pbhlogger.info("Create PBH rule: {}".format(PBH_RULE_NAME)) + self.dvs_pbh.create_pbh_rule( + table_name=PBH_TABLE_NAME, + rule_name=PBH_RULE_NAME, + priority=PBH_RULE_PRIORITY, + qualifiers=attr_dict, + hash_name=PBH_RULE_HASH + ) + self.dvs_acl.verify_acl_rule_count(1) + + attr_dict = { + "ether_type": "0x86dd", + "ipv6_next_header": "0x2f", + "inner_ether_type": "0x0800" + } + + pbhlogger.info("Update PBH rule: {}".format(PBH_RULE_NAME)) + self.dvs_pbh.update_pbh_rule( + table_name=PBH_TABLE_NAME, + rule_name=PBH_RULE_NAME, + priority="100", + qualifiers=attr_dict, + hash_name=PBH_RULE_HASH, + packet_action="SET_LAG_HASH", + flow_counter="ENABLED" + ) + + hash_id = self.dvs_pbh.get_pbh_hash_ids(1)[0] + counter_id = self.dvs_acl.get_acl_counter_ids(1)[0] + + sai_attr_dict = { + "SAI_ACL_ENTRY_ATTR_PRIORITY": self.dvs_acl.get_simple_qualifier_comparator("100"), + "SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE": self.dvs_acl.get_simple_qualifier_comparator("34525&mask:0xffff"), + "SAI_ACL_ENTRY_ATTR_FIELD_IP_PROTOCOL": self.dvs_acl.get_simple_qualifier_comparator("disabled"), + "SAI_ACL_ENTRY_ATTR_FIELD_IPV6_NEXT_HEADER": self.dvs_acl.get_simple_qualifier_comparator("47&mask:0xff"), + "SAI_ACL_ENTRY_ATTR_FIELD_GRE_KEY": self.dvs_acl.get_simple_qualifier_comparator("disabled"), + "SAI_ACL_ENTRY_ATTR_FIELD_INNER_ETHER_TYPE": self.dvs_acl.get_simple_qualifier_comparator("2048&mask:0xffff"), + "SAI_ACL_ENTRY_ATTR_ACTION_SET_ECMP_HASH_ID": self.dvs_acl.get_simple_qualifier_comparator("disabled"), + "SAI_ACL_ENTRY_ATTR_ACTION_SET_LAG_HASH_ID": self.dvs_acl.get_simple_qualifier_comparator(hash_id), + "SAI_ACL_ENTRY_ATTR_ACTION_COUNTER": self.dvs_acl.get_simple_qualifier_comparator(counter_id) + } + + self.dvs_acl.verify_acl_rule_generic( + sai_qualifiers=sai_attr_dict + ) + + finally: + # PBH rule + pbhlogger.info("Remove PBH rule: {}".format(PBH_RULE_NAME)) + self.dvs_pbh.remove_pbh_rule(PBH_TABLE_NAME, PBH_RULE_NAME) + self.dvs_acl.verify_acl_rule_count(0) + + # PBH table + pbhlogger.info("Remove PBH table: {}".format(PBH_TABLE_NAME)) + self.dvs_pbh.remove_pbh_table(PBH_TABLE_NAME) + self.dvs_acl.verify_acl_table_count(0) + + # PBH hash + pbhlogger.info("Remove PBH hash: {}".format(PBH_HASH_NAME)) + self.dvs_pbh.remove_pbh_hash(PBH_HASH_NAME) + self.dvs_pbh.verify_pbh_hash_count(0) + + # PBH hash field + pbhlogger.info("Remove PBH hash field: {}".format(PBH_HASH_FIELD_NAME)) + self.dvs_pbh.remove_pbh_hash_field(PBH_HASH_FIELD_NAME) + self.dvs_pbh.verify_pbh_hash_field_count(0) + + @pytest.mark.usefixtures("dvs_lag_manager") class TestPbhExtendedFlows: class PbhRefCountHelper(object):