diff --git a/.buckconfig.d/external_cells/facebook/buck2-shims-meta/external_cell.buckconfig b/.buckconfig.d/external_cells/facebook/buck2-shims-meta/external_cell.buckconfig index 51665d718950a..c231baf41a8f2 100644 --- a/.buckconfig.d/external_cells/facebook/buck2-shims-meta/external_cell.buckconfig +++ b/.buckconfig.d/external_cells/facebook/buck2-shims-meta/external_cell.buckconfig @@ -6,4 +6,4 @@ gh_facebook_buck2_shims_meta = git [external_cell_gh_facebook_buck2_shims_meta] git_origin = https://github.com/facebook/buck2-shims-meta.git -commit_hash = 461a1b10af7eb8af2f6d0eb0129009d60048e6de +commit_hash = b79235f6447a07f78644ab08261e3acfe4e12604 diff --git a/build/deps/github_hashes/facebook/fb303-rev.txt b/build/deps/github_hashes/facebook/fb303-rev.txt index d57cd36a4163c..e3ab9e52b2b77 100644 --- a/build/deps/github_hashes/facebook/fb303-rev.txt +++ b/build/deps/github_hashes/facebook/fb303-rev.txt @@ -1 +1 @@ -Subproject commit 43a8ca1e89fc3ab45abf9eff78b6c0ec4510ebdb +Subproject commit d15028627fdd473945f04124f118e17142471f81 diff --git a/build/deps/github_hashes/facebook/fbthrift-rev.txt b/build/deps/github_hashes/facebook/fbthrift-rev.txt index 2e120f76285e3..334bb35fc08bd 100644 --- a/build/deps/github_hashes/facebook/fbthrift-rev.txt +++ b/build/deps/github_hashes/facebook/fbthrift-rev.txt @@ -1 +1 @@ -Subproject commit 6c55aa9762cffed619c25abf07d273b39985e979 +Subproject commit 300f4d5e79e763ef74243da9a1f3b9f649d5addd diff --git a/build/deps/github_hashes/facebook/wangle-rev.txt b/build/deps/github_hashes/facebook/wangle-rev.txt index 0c225b340a4c2..d94e75e4f27b3 100644 --- a/build/deps/github_hashes/facebook/wangle-rev.txt +++ b/build/deps/github_hashes/facebook/wangle-rev.txt @@ -1 +1 @@ -Subproject commit 46688946ef13435ad9e873a5ba5684677783945b +Subproject commit d54405ad3588791be7146a19fe4e914af0807c28 diff --git a/cmake/Agent.cmake b/cmake/Agent.cmake index 597d810c8b502..2c843db1acdac 100644 --- a/cmake/Agent.cmake +++ b/cmake/Agent.cmake @@ -457,6 +457,7 @@ target_link_libraries(switchid_scope_resolver fboss_error hwswitch_matcher state + switchinfo_utils ) add_library(hwagent diff --git a/fboss/agent/ApplyThriftConfig.cpp b/fboss/agent/ApplyThriftConfig.cpp index 0ecfa93b6c34b..9f4c387eae0ac 100644 --- a/fboss/agent/ApplyThriftConfig.cpp +++ b/fboss/agent/ApplyThriftConfig.cpp @@ -101,6 +101,7 @@ using folly::StringPiece; using std::make_shared; using std::shared_ptr; +using namespace facebook::fboss; namespace { const uint8_t kV6LinkLocalAddrMask{64}; @@ -148,6 +149,55 @@ std::shared_ptr toMultiSwitchMap( return multiMap; } +bool haveParallelLinksToInterfaceNodes( + const cfg::SwitchConfig* cfg, + const std::vector& localFabricSwitchIds, + const std::unordered_map>& + switchNameToSwitchIds, + SwitchIdScopeResolver& scopeResolver, + const PlatformMapping* platformMapping) { + for (const auto& fabricSwitchId : localFabricSwitchIds) { + // Determine parallel links on VD level - there are two VDs per R3 ASIC + std::unordered_map> vd2VoqNeighbors; + for (const auto& port : *cfg->ports()) { + // Only process ports belonging to one switchId + if (scopeResolver.scope(port).has(SwitchID(fabricSwitchId)) && + port.expectedNeighborReachability()->size() > 0) { + auto neighborRemoteSwitchId = + getRemoteSwitchID(cfg, port, switchNameToSwitchIds); + const auto& neighborDsfNodeIter = + cfg->dsfNodes()->find(neighborRemoteSwitchId); + CHECK(neighborDsfNodeIter != cfg->dsfNodes()->end()); + if (*neighborDsfNodeIter->second.type() == + cfg::DsfNodeType::INTERFACE_NODE) { + CHECK(port.name().has_value()); + auto localVirtualDeviceId = + platformMapping->getVirtualDeviceID(*port.name()); + if (!localVirtualDeviceId.has_value()) { + throw FbossError( + "Unable to find virtual device id for port: ", + *port.logicalID(), + " virtual device"); + } + + if (vd2VoqNeighbors.find(localVirtualDeviceId.value()) == + vd2VoqNeighbors.end()) { + vd2VoqNeighbors.insert( + {localVirtualDeviceId.value(), + std::unordered_set()}); + } + auto& voqNeighbors = vd2VoqNeighbors[localVirtualDeviceId.value()]; + const auto& [neighborName, _] = getExpectedNeighborAndPortName(port); + if (voqNeighbors.find(neighborName) != voqNeighbors.end()) { + return true; + } + voqNeighbors.insert(neighborName); + } + } + } + } + return false; +}; } // anonymous namespace namespace facebook::fboss { @@ -4532,6 +4582,28 @@ shared_ptr ThriftConfigApplier::updateSwitchSettings( switchSettingsChange = true; } + std::optional newLinkFlowControlCreditThreshold; + if (cfg_->switchSettings()->linkFlowControlCreditThreshold()) { + newLinkFlowControlCreditThreshold = + *cfg_->switchSettings()->linkFlowControlCreditThreshold(); + } + if (newLinkFlowControlCreditThreshold != + origSwitchSettings->getLinkFlowControlCreditThreshold()) { + newSwitchSettings->setLinkFlowControlCreditThreshold( + newLinkFlowControlCreditThreshold); + switchSettingsChange = true; + } + + std::optional newVoqDramBoundThreshold; + if (cfg_->switchSettings()->voqDramBoundThreshold()) { + newVoqDramBoundThreshold = *cfg_->switchSettings()->voqDramBoundThreshold(); + } + if (newVoqDramBoundThreshold != + origSwitchSettings->getVoqDramBoundThreshold()) { + newSwitchSettings->setVoqDramBoundThreshold(newVoqDramBoundThreshold); + switchSettingsChange = true; + } + if (origSwitchSettings->getSwitchDrainState() != *cfg_->switchSettings()->switchDrainState()) { auto numVoqSwtitches = diff --git a/fboss/agent/BUCK b/fboss/agent/BUCK index 6914bf80711cb..a1e6c3fd84cb8 100644 --- a/fboss/agent/BUCK +++ b/fboss/agent/BUCK @@ -616,6 +616,22 @@ cpp_library( ], ) +cpp_library( + name = "fsdb_adapted_sub_manager", + srcs = [ + "FsdbAdaptedSubManager.cpp", + ], + headers = [ + "FsdbAdaptedSubManager.h", + ], + exported_deps = [ + "//fboss/agent/state:state", + "//fboss/fsdb/client:fsdb_sub_manager", + "//fboss/fsdb/if:fsdb_model", + "//fboss/thrift_cow/storage:cow_storage", + ], +) + cpp_library( name = "core", srcs = [ @@ -633,7 +649,6 @@ cpp_library( "EncapIndexAllocator.cpp", "FabricConnectivityManager.cpp", "FibHelpers.cpp", - "FsdbAdaptedSubManager.cpp", "FsdbSyncer.cpp", "HwAsicTable.cpp", "HwSwitchConnectionStatusTable.cpp", @@ -721,6 +736,7 @@ cpp_library( ":fboss-error", ":fboss-event-base", ":fboss-types", + ":fsdb_adapted_sub_manager", ":hw_switch_handler", ":hwswitchcallback", ":l2learn_event_observer", @@ -1181,6 +1197,7 @@ cpp_library( ":fboss-error", ":fboss-types", ":hwswitch_matcher", + ":switchinfo_utils", "//fboss/agent/state:state", ], exported_external_deps = [ diff --git a/fboss/agent/DsfUpdateValidator.cpp b/fboss/agent/DsfUpdateValidator.cpp index e5f4a2ed5f5a4..c494deaf6065a 100644 --- a/fboss/agent/DsfUpdateValidator.cpp +++ b/fboss/agent/DsfUpdateValidator.cpp @@ -2,7 +2,7 @@ #include "fboss/agent/DsfUpdateValidator.h" #include "fboss/agent/DsfStateUpdaterUtil.h" -#include "fboss/agent/Utils.h" +#include "fboss/agent/SwitchInfoUtils.h" #include "fboss/agent/state/DeltaFunctions.h" #include "fboss/agent/state/InterfaceMap.h" #include "fboss/agent/state/StateDelta.h" diff --git a/fboss/agent/SwitchIdScopeResolver.cpp b/fboss/agent/SwitchIdScopeResolver.cpp index d122e99c127da..f90515682fb24 100644 --- a/fboss/agent/SwitchIdScopeResolver.cpp +++ b/fboss/agent/SwitchIdScopeResolver.cpp @@ -2,6 +2,7 @@ #include "fboss/agent/SwitchIdScopeResolver.h" #include "fboss/agent/FbossError.h" +#include "fboss/agent/SwitchInfoUtils.h" #include "fboss/agent/state/AclTableGroup.h" #include "fboss/agent/state/AggregatePort.h" #include "fboss/agent/state/ForwardingInformationBaseMap.h" @@ -137,17 +138,11 @@ HwSwitchMatcher SwitchIdScopeResolver::scope( } HwSwitchMatcher SwitchIdScopeResolver::scope(SystemPortID sysPortId) const { - auto sysPortInt = static_cast(sysPortId); for (const auto& [id, info] : switchIdToSwitchInfo_) { - if (!info.systemPortRange().has_value()) { - continue; - } - if (sysPortInt >= *info.systemPortRange()->minimum() && - sysPortInt <= *info.systemPortRange()->maximum()) { + if (withinRange(*info.systemPortRanges(), sysPortId)) { return HwSwitchMatcher(std::unordered_set({SwitchID(id)})); } } - // This is a non local sys port. So it maps to all local voq switchIds return voqSwitchMatcher(); } diff --git a/fboss/agent/SwitchInfoUtils.cpp b/fboss/agent/SwitchInfoUtils.cpp index 356d454a93b19..6bb3fc60bcd70 100644 --- a/fboss/agent/SwitchInfoUtils.cpp +++ b/fboss/agent/SwitchInfoUtils.cpp @@ -17,6 +17,20 @@ #include "fboss/agent/hw/switch_asics/HwAsic.h" namespace facebook::fboss { +namespace { + +bool withinRange(const cfg::SystemPortRanges& ranges, int64_t id) { + bool inRange{false}; + for (const auto& sysPortRange : *ranges.systemPortRanges()) { + if (id >= *sysPortRange.minimum() && id <= *sysPortRange.maximum()) { + inRange = true; + break; + } + } + return inRange; +} +} // namespace + const std::map getSwitchInfoFromConfigImpl( const cfg::SwitchConfig* config) { std::map switchInfoMap; @@ -32,16 +46,6 @@ const std::map getSwitchInfoFromConfigImpl( switchInfo.portIdRange()->maximum() = cfg::switch_config_constants::DEFAULT_PORT_ID_RANGE_MAX(); } - if (switchInfo.switchType() == cfg::SwitchType::VOQ && - !switchInfo.systemPortRange()) { - auto dsfItr = - config->dsfNodes()->find(static_cast(entry.first)); - if (dsfItr != config->dsfNodes()->end()) { - auto localNode = dsfItr->second; - CHECK(localNode.systemPortRange().has_value()); - switchInfo.systemPortRange() = *localNode.systemPortRange(); - } - } switchInfoMap.emplace(entry.first, switchInfo); } } @@ -106,4 +110,13 @@ const std::optional getSdkVersionFromConfig( auto& swConfig = config->thrift.sw().value(); return getSdkVersionFromConfigImpl(&swConfig); } + +bool withinRange(const cfg::SystemPortRanges& ranges, InterfaceID intfId) { + return withinRange(ranges, static_cast(intfId)); +} + +bool withinRange(const cfg::SystemPortRanges& ranges, SystemPortID sysPortId) { + return withinRange(ranges, static_cast(sysPortId)); +} + } // namespace facebook::fboss diff --git a/fboss/agent/SwitchInfoUtils.h b/fboss/agent/SwitchInfoUtils.h index 3c82e75004d68..f62b30f464331 100644 --- a/fboss/agent/SwitchInfoUtils.h +++ b/fboss/agent/SwitchInfoUtils.h @@ -40,4 +40,7 @@ const std::optional getSdkVersionFromConfig(); const std::optional getSdkVersionFromConfig( const AgentConfig* config); +bool withinRange(const cfg::SystemPortRanges& ranges, InterfaceID intfId); + +bool withinRange(const cfg::SystemPortRanges& ranges, SystemPortID sysPortId); } // namespace facebook::fboss diff --git a/fboss/agent/Utils.cpp b/fboss/agent/Utils.cpp index 789647e3fa1ab..ff58d442029ce 100644 --- a/fboss/agent/Utils.cpp +++ b/fboss/agent/Utils.cpp @@ -14,7 +14,6 @@ #include "fboss/agent/AsicUtils.h" #include "fboss/agent/FbossError.h" #include "fboss/agent/FsdbHelper.h" -#include "fboss/agent/SwitchIdScopeResolver.h" #include "fboss/agent/SysError.h" #include "fboss/agent/hw/HwSwitchFb303Stats.h" #include "fboss/agent/state/ArpEntry.h" @@ -139,16 +138,6 @@ getPlatformMappingForDsfNode(const facebook::fboss::PlatformType platformType) { return nullptr; } -bool withinRange(const cfg::SystemPortRanges& ranges, int64_t id) { - bool inRange{false}; - for (const auto& sysPortRange : *ranges.systemPortRanges()) { - if (id >= *sysPortRange.minimum() && id <= *sysPortRange.maximum()) { - inRange = true; - break; - } - } - return inRange; -} } // namespace // @@ -501,14 +490,6 @@ SystemPortID getInbandSystemPortID( *switchInfoItr->second.inbandPortId()); } -bool withinRange(const cfg::SystemPortRanges& ranges, InterfaceID intfId) { - return withinRange(ranges, static_cast(intfId)); -} - -bool withinRange(const cfg::SystemPortRanges& ranges, SystemPortID sysPortId) { - return withinRange(ranges, static_cast(sysPortId)); -} - cfg::Range64 getFirstSwitchSystemPortIdRange( const std::map& switchToSwitchInfo) { for (const auto& [switchId, switchInfo] : switchToSwitchInfo) { @@ -1062,56 +1043,6 @@ int getRemoteSwitchID( return remoteSwitchId; }; -bool haveParallelLinksToInterfaceNodes( - const cfg::SwitchConfig* cfg, - const std::vector& localFabricSwitchIds, - const std::unordered_map>& - switchNameToSwitchIds, - SwitchIdScopeResolver& scopeResolver, - const PlatformMapping* platformMapping) { - for (const auto& fabricSwitchId : localFabricSwitchIds) { - // Determine parallel links on VD level - there are two VDs per R3 ASIC - std::unordered_map> vd2VoqNeighbors; - for (const auto& port : *cfg->ports()) { - // Only process ports belonging to one switchId - if (scopeResolver.scope(port).has(SwitchID(fabricSwitchId)) && - port.expectedNeighborReachability()->size() > 0) { - auto neighborRemoteSwitchId = - getRemoteSwitchID(cfg, port, switchNameToSwitchIds); - const auto& neighborDsfNodeIter = - cfg->dsfNodes()->find(neighborRemoteSwitchId); - CHECK(neighborDsfNodeIter != cfg->dsfNodes()->end()); - if (*neighborDsfNodeIter->second.type() == - cfg::DsfNodeType::INTERFACE_NODE) { - CHECK(port.name().has_value()); - auto localVirtualDeviceId = - platformMapping->getVirtualDeviceID(*port.name()); - if (!localVirtualDeviceId.has_value()) { - throw FbossError( - "Unable to find virtual device id for port: ", - *port.logicalID(), - " virtual device"); - } - - if (vd2VoqNeighbors.find(localVirtualDeviceId.value()) == - vd2VoqNeighbors.end()) { - vd2VoqNeighbors.insert( - {localVirtualDeviceId.value(), - std::unordered_set()}); - } - auto& voqNeighbors = vd2VoqNeighbors[localVirtualDeviceId.value()]; - const auto& [neighborName, _] = getExpectedNeighborAndPortName(port); - if (voqNeighbors.find(neighborName) != voqNeighbors.end()) { - return true; - } - voqNeighbors.insert(neighborName); - } - } - } - } - return false; -}; - CpuCosQueueId hwQueueIdToCpuCosQueueId( uint8_t hwQueueId, const HwAsic* asic, diff --git a/fboss/agent/Utils.h b/fboss/agent/Utils.h index 6e6e6571d3a6b..614ac4c36ef91 100644 --- a/fboss/agent/Utils.h +++ b/fboss/agent/Utils.h @@ -85,7 +85,6 @@ inline const int kUdfAclRethDmaLenFieldSizeInBytes(2); class SwitchState; class Interface; class SwitchSettings; -class SwitchIdScopeResolver; class PlatformMapping; struct AgentConfig; class HwAsic; @@ -276,9 +275,6 @@ std::vector getPortsForInterface( InterfaceID intf, const std::shared_ptr& state); -bool withinRange(const cfg::SystemPortRanges& ranges, InterfaceID intfId); - -bool withinRange(const cfg::SystemPortRanges& ranges, SystemPortID sysPortId); /* * An NPU switch injects a broadcast message such as neighbor solicitation or * advertisement via pipeline lookup. ASIC forwards these messages to all the @@ -443,14 +439,6 @@ int getRemoteSwitchID( const std::unordered_map>& switchNameToSwitchIds); -bool haveParallelLinksToInterfaceNodes( - const cfg::SwitchConfig* cfg, - const std::vector& localFabricSwitchIds, - const std::unordered_map>& - switchNameToSwitchIds, - SwitchIdScopeResolver& scopeResolver, - const PlatformMapping* platformMapping); - CpuCosQueueId hwQueueIdToCpuCosQueueId( uint8_t hwQueueId, const HwAsic* asic, diff --git a/fboss/agent/hw/sai/store/SaiObject.cpp b/fboss/agent/hw/sai/store/SaiObject.cpp index 6abb8fab5b170..2b85f55935f6c 100644 --- a/fboss/agent/hw/sai/store/SaiObject.cpp +++ b/fboss/agent/hw/sai/store/SaiObject.cpp @@ -61,7 +61,7 @@ SaiObject::adapterHostKeyToFollyDynamic() { template <> typename SaiNextHopGroupTraits::AdapterHostKey SaiObject::follyDynamicToAdapterHostKey( - folly::dynamic json) { + const folly::dynamic& json) { SaiNextHopGroupTraits::AdapterHostKey key; for (auto object : json) { auto type = @@ -138,7 +138,8 @@ folly::dynamic SaiObject::adapterHostKeyToFollyDynamic() { template <> typename SaiLagTraits::AdapterHostKey -SaiObject::follyDynamicToAdapterHostKey(folly::dynamic json) { +SaiObject::follyDynamicToAdapterHostKey( + const folly::dynamic& json) { std::string label = json.asString(); SaiLagTraits::AdapterHostKey::ValueType key{}; std::copy(std::begin(label), std::end(label), std::begin(key)); @@ -182,7 +183,7 @@ folly::dynamic SaiObject::adapterHostKeyToFollyDynamic() { template void pupulateOptionalAttrtToKey( - folly::dynamic& array, + const folly::dynamic& array, SaiWredTraits::AdapterHostKey& adapterHostKey, int index) { if (!array[index].isString()) { @@ -192,7 +193,8 @@ void pupulateOptionalAttrtToKey( template <> typename SaiWredTraits::AdapterHostKey -SaiObject::follyDynamicToAdapterHostKey(folly::dynamic json) { +SaiObject::follyDynamicToAdapterHostKey( + const folly::dynamic& json) { SaiWredTraits::AdapterHostKey key; std::get(key) = json[0].asBool(); pupulateOptionalAttrtToKey( @@ -219,7 +221,7 @@ folly::dynamic SaiObject::adapterHostKeyToFollyDynamic() { template <> typename SaiAclTableTraits::AdapterHostKey SaiObject::follyDynamicToAdapterHostKey( - folly::dynamic json) { + const folly::dynamic& json) { return json.asString(); } @@ -231,7 +233,7 @@ folly::dynamic SaiObject::adapterHostKeyToFollyDynamic() { template <> typename SaiUdfGroupTraits::AdapterHostKey SaiObject::follyDynamicToAdapterHostKey( - folly::dynamic json) { + const folly::dynamic& json) { return json.asString(); } } // namespace fboss diff --git a/fboss/agent/hw/sai/store/SaiObject.h b/fboss/agent/hw/sai/store/SaiObject.h index 5e524db492d7d..330c8758f8cc1 100644 --- a/fboss/agent/hw/sai/store/SaiObject.h +++ b/fboss/agent/hw/sai/store/SaiObject.h @@ -372,7 +372,7 @@ class SaiObject { folly::dynamic adapterHostKeyToFollyDynamic(); static typename SaiObjectTraits::AdapterHostKey follyDynamicToAdapterHostKey( - folly::dynamic); + const folly::dynamic&); void setIgnoreMissingInHwOnDelete(bool ignore) { ignoreMissingInHwOnDelete_ = ignore; diff --git a/fboss/agent/hw/sai/switch/SaiAclTableGroupManager.cpp b/fboss/agent/hw/sai/switch/SaiAclTableGroupManager.cpp index 3682193292fba..5ad1582702ad4 100644 --- a/fboss/agent/hw/sai/switch/SaiAclTableGroupManager.cpp +++ b/fboss/agent/hw/sai/switch/SaiAclTableGroupManager.cpp @@ -33,6 +33,8 @@ sai_acl_stage_t SaiAclTableGroupManager::cfgAclStageToSaiAclStage( return SAI_ACL_STAGE_INGRESS_MACSEC; case cfg::AclStage::EGRESS_MACSEC: return SAI_ACL_STAGE_EGRESS_MACSEC; + case cfg::AclStage::EGRESS: + return SAI_ACL_STAGE_EGRESS; } // should return in one of the cases diff --git a/fboss/agent/hw/sai/switch/SaiAclTableManager.cpp b/fboss/agent/hw/sai/switch/SaiAclTableManager.cpp index 419f1ba9feae3..2659f5a85374a 100644 --- a/fboss/agent/hw/sai/switch/SaiAclTableManager.cpp +++ b/fboss/agent/hw/sai/switch/SaiAclTableManager.cpp @@ -1409,7 +1409,11 @@ std::set SaiAclTableManager::getSupportedQualifierSet( } std::set SaiAclTableManager::getSupportedQualifierSet( - sai_acl_stage_t /* aclStage */) const { + sai_acl_stage_t aclStage) const { + if (aclStage == SAI_ACL_STAGE_EGRESS && + platform_->getAsic()->isSupported(HwAsic::Feature::EGRESS_ACL_TABLE)) { + throw FbossError("egress acl table is not supported on switch asic"); + } /* * Not all the qualifiers are supported by every ASIC. * Moreover, different ASICs have different max key widths. @@ -1567,20 +1571,24 @@ std::set SaiAclTableManager::getSupportedQualifierSet( } } -void SaiAclTableManager::addDefaultAclTable(cfg::AclStage stage) { - if (handles_.find(kAclTable1) != handles_.end()) { - throw FbossError("default acl table already exists."); +void SaiAclTableManager::addDefaultAclTable( + cfg::AclStage stage, + const std::string& name) { + if (handles_.find(name) != handles_.end()) { + throw FbossError("default acl table ", name, " already exists."); } // TODO(saranicholas): set appropriate table priority state::AclTableFields aclTableFields{}; aclTableFields.priority() = 0; - aclTableFields.id() = kAclTable1; + aclTableFields.id() = name; auto table1 = std::make_shared(std::move(aclTableFields)); addAclTable(table1, stage); } -void SaiAclTableManager::removeDefaultAclTable(cfg::AclStage stage) { - if (handles_.find(kAclTable1) == handles_.end()) { +void SaiAclTableManager::removeDefaultAclTable( + cfg::AclStage stage, + const std::string& name) { + if (handles_.find(name) == handles_.end()) { return; } // remove from acl table group @@ -1588,7 +1596,7 @@ void SaiAclTableManager::removeDefaultAclTable(cfg::AclStage stage) { SaiAclTableGroupManager::cfgAclStageToSaiAclStage(stage); if (platform_->getAsic()->isSupported(HwAsic::Feature::ACL_TABLE_GROUP)) { managerTable_->aclTableGroupManager().removeAclTableGroupMember( - saiAclStage, kAclTable1); + saiAclStage, name); } handles_.erase(kAclTable1); } @@ -1878,4 +1886,11 @@ std::shared_ptr SaiAclTableManager::reconstructAclEntry( int /*priority*/) const { throw FbossError("reconstructAclEntry not implemented in SaiAclTableManager"); } + +void SaiAclTableManager::addDefaultIngressAclTable() { + addDefaultAclTable(cfg::AclStage::INGRESS, kAclTable1); +} +void SaiAclTableManager::removeDefaultIngressAclTable() { + removeDefaultAclTable(cfg::AclStage::INGRESS, kAclTable1); +} } // namespace facebook::fboss diff --git a/fboss/agent/hw/sai/switch/SaiAclTableManager.h b/fboss/agent/hw/sai/switch/SaiAclTableManager.h index f12d60aceece1..3e31f63e42ca2 100644 --- a/fboss/agent/hw/sai/switch/SaiAclTableManager.h +++ b/fboss/agent/hw/sai/switch/SaiAclTableManager.h @@ -191,8 +191,8 @@ class SaiAclTableManager { std::set getSupportedQualifierSet( cfg::AclStage stage) const; - void addDefaultAclTable(cfg::AclStage stage); - void removeDefaultAclTable(cfg::AclStage stage); + void addDefaultIngressAclTable(); + void removeDefaultIngressAclTable(); bool isQualifierSupported( const std::string& aclTableName, @@ -277,6 +277,9 @@ class SaiAclTableManager { std::set getSupportedQualifierSet( sai_acl_stage_t aclStage) const; + void addDefaultAclTable(cfg::AclStage stage, const std::string& name); + void removeDefaultAclTable(cfg::AclStage stage, const std::string& name); + SaiStore* saiStore_; SaiManagerTable* managerTable_; const SaiPlatform* platform_; diff --git a/fboss/agent/hw/sai/switch/SaiSwitch.cpp b/fboss/agent/hw/sai/switch/SaiSwitch.cpp index 312dfa7853b74..4bf7bbf158559 100644 --- a/fboss/agent/hw/sai/switch/SaiSwitch.cpp +++ b/fboss/agent/hw/sai/switch/SaiSwitch.cpp @@ -1107,10 +1107,8 @@ std::shared_ptr SaiSwitch::stateChangedImplLocked( // remove default acl table and add a new one. table removal should // clear acl entries too managerTable_->switchManager().resetIngressAcl(); - managerTable_->aclTableManager().removeDefaultAclTable( - cfg::AclStage::INGRESS); - managerTable_->aclTableManager().addDefaultAclTable( - cfg::AclStage::INGRESS); + managerTable_->aclTableManager().removeDefaultIngressAclTable(); + managerTable_->aclTableManager().addDefaultIngressAclTable(); managerTable_->switchManager().setIngressAcl(); } @@ -1359,6 +1357,26 @@ void SaiSwitch::processSwitchSettingsChangeSansDrainedEntryLocked( newSramGlobalXonTh.value_or(0)); } } + { + const auto oldLinkFlowControlCreditTh = + oldSwitchSettings->getLinkFlowControlCreditThreshold(); + const auto newLinkFlowControlCreditTh = + newSwitchSettings->getLinkFlowControlCreditThreshold(); + if (oldLinkFlowControlCreditTh != newLinkFlowControlCreditTh) { + managerTable_->switchManager().setLinkFlowControlCreditTh( + newLinkFlowControlCreditTh.value_or(0)); + } + } + { + const auto oldVoqDramBoundTh = + oldSwitchSettings->getVoqDramBoundThreshold(); + const auto newVoqDramBoundTh = + newSwitchSettings->getVoqDramBoundThreshold(); + if (oldVoqDramBoundTh != newVoqDramBoundTh) { + managerTable_->switchManager().setVoqDramBoundTh( + newVoqDramBoundTh.value_or(0)); + } + } } template @@ -2564,8 +2582,7 @@ void SaiSwitch::initStoreAndManagersLocked( std::make_shared(cfg::AclStage::INGRESS); managerTable_->aclTableGroupManager().addAclTableGroup(aclTableGroup); - managerTable_->aclTableManager().addDefaultAclTable( - cfg::AclStage::INGRESS); + managerTable_->aclTableManager().addDefaultIngressAclTable(); } } diff --git a/fboss/agent/hw/sai/switch/SaiSwitchManager.cpp b/fboss/agent/hw/sai/switch/SaiSwitchManager.cpp index 13639e35617ce..bb19587f1c8af 100644 --- a/fboss/agent/hw/sai/switch/SaiSwitchManager.cpp +++ b/fboss/agent/hw/sai/switch/SaiSwitchManager.cpp @@ -1026,4 +1026,26 @@ void SaiSwitchManager::setSramGlobalFreePercentXonTh( sramFreePercentXonThreshold}); #endif } + +void SaiSwitchManager::setLinkFlowControlCreditTh( + uint16_t linkFlowControlThreshold) { +#if defined(BRCM_SAI_SDK_DNX_GTE_11_0) && !defined(BRCM_SAI_SDK_DNX_GTE_12_0) + switch_->setOptionalAttribute( + SaiSwitchTraits::Attributes::FabricCllfcTxCreditTh{ + linkFlowControlThreshold}); +#endif +} + +void SaiSwitchManager::setVoqDramBoundTh(uint32_t dramBoundThreshold) { +#if defined(BRCM_SAI_SDK_DNX_GTE_11_0) && !defined(BRCM_SAI_SDK_DNX_GTE_12_0) + // There are 3 different types of rate classes available and + // dramBound, upper and lower limits are applied to each of + // those. However, in our case, we just need to set the same + // DRAM bounds value for all the rate classes. + std::vector dramBounds(6, dramBoundThreshold); + switch_->setOptionalAttribute( + SaiSwitchTraits::Attributes::VoqDramBoundTh{dramBounds}); +#endif +} + } // namespace facebook::fboss diff --git a/fboss/agent/hw/sai/switch/SaiSwitchManager.h b/fboss/agent/hw/sai/switch/SaiSwitchManager.h index 5981a8c9fd1d5..3c428fe48cdc4 100644 --- a/fboss/agent/hw/sai/switch/SaiSwitchManager.h +++ b/fboss/agent/hw/sai/switch/SaiSwitchManager.h @@ -105,6 +105,8 @@ class SaiSwitchManager { void setReachabilityGroupList(int reachabilityGroupListSize); void setSramGlobalFreePercentXoffTh(uint8_t sramFreePercentXoffThreshold); void setSramGlobalFreePercentXonTh(uint8_t sramFreePercentXonThreshold); + void setLinkFlowControlCreditTh(uint16_t linkFlowControlThreshold); + void setVoqDramBoundTh(uint32_t dramBoundThreshold); private: void programEcmpLoadBalancerParams( diff --git a/fboss/agent/hw/switch_asics/ChenabAsic.cpp b/fboss/agent/hw/switch_asics/ChenabAsic.cpp index 787458ffcd1d8..28a61777b86dd 100644 --- a/fboss/agent/hw/switch_asics/ChenabAsic.cpp +++ b/fboss/agent/hw/switch_asics/ChenabAsic.cpp @@ -76,6 +76,7 @@ bool ChenabAsic::isSupportedNonFabric(Feature feature) const { case HwAsic::Feature::SAI_TTL0_PACKET_FORWARD_ENABLE: case HwAsic::Feature::L3_INTF_MTU: case HwAsic::Feature::PORT_MTU_ERROR_TRAP: + case HwAsic::Feature::EGRESS_ACL_TABLE: return true; case HwAsic::Feature::EVENTOR_PORT_FOR_SFLOW: case HwAsic::Feature::CPU_VOQ_BUFFER_PROFILE: diff --git a/fboss/agent/hw/switch_asics/EbroAsic.cpp b/fboss/agent/hw/switch_asics/EbroAsic.cpp index 03e2d71bd6f1e..22d4df180c75a 100644 --- a/fboss/agent/hw/switch_asics/EbroAsic.cpp +++ b/fboss/agent/hw/switch_asics/EbroAsic.cpp @@ -190,6 +190,7 @@ bool EbroAsic::isSupportedNonFabric(Feature feature) const { case HwAsic::Feature::MULTIPLE_EGRESS_BUFFER_POOL: case HwAsic::Feature::PORT_MTU_ERROR_TRAP: case HwAsic::Feature::DEDICATED_CPU_BUFFER_POOL: + case HwAsic::Feature::EGRESS_ACL_TABLE: return false; case HwAsic::Feature::SAI_ACL_ENTRY_SRC_PORT_QUALIFIER: /* diff --git a/fboss/agent/hw/switch_asics/HwAsic.h b/fboss/agent/hw/switch_asics/HwAsic.h index 08b75f8b4e31e..bc26fdb565b7c 100644 --- a/fboss/agent/hw/switch_asics/HwAsic.h +++ b/fboss/agent/hw/switch_asics/HwAsic.h @@ -199,6 +199,7 @@ class HwAsic { PORT_MTU_ERROR_TRAP, L3_INTF_MTU, DEDICATED_CPU_BUFFER_POOL, + EGRESS_ACL_TABLE, }; enum class AsicMode { diff --git a/fboss/agent/hw/switch_asics/Jericho2Asic.cpp b/fboss/agent/hw/switch_asics/Jericho2Asic.cpp index 4ce365512a903..b0a6b828a5d44 100644 --- a/fboss/agent/hw/switch_asics/Jericho2Asic.cpp +++ b/fboss/agent/hw/switch_asics/Jericho2Asic.cpp @@ -194,6 +194,7 @@ bool Jericho2Asic::isSupported(Feature feature) const { case HwAsic::Feature::ENABLE_DELAY_DROP_CONGESTION_THRESHOLD: case HwAsic::Feature::PORT_MTU_ERROR_TRAP: case HwAsic::Feature::DEDICATED_CPU_BUFFER_POOL: + case HwAsic::Feature::EGRESS_ACL_TABLE: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/Jericho3Asic.cpp b/fboss/agent/hw/switch_asics/Jericho3Asic.cpp index 4867aa294b5db..24c9328247534 100644 --- a/fboss/agent/hw/switch_asics/Jericho3Asic.cpp +++ b/fboss/agent/hw/switch_asics/Jericho3Asic.cpp @@ -198,6 +198,7 @@ bool Jericho3Asic::isSupported(Feature feature) const { case HwAsic::Feature::L3_MTU_ERROR_TRAP: case HwAsic::Feature::L3_INTF_MTU: case HwAsic::Feature::DEDICATED_CPU_BUFFER_POOL: + case HwAsic::Feature::EGRESS_ACL_TABLE: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp index 334a6914be684..da695d4d2e55b 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk3Asic.cpp @@ -183,6 +183,7 @@ bool Tomahawk3Asic::isSupported(Feature feature) const { case HwAsic::Feature::ENABLE_DELAY_DROP_CONGESTION_THRESHOLD: case HwAsic::Feature::PORT_MTU_ERROR_TRAP: case HwAsic::Feature::DEDICATED_CPU_BUFFER_POOL: + case HwAsic::Feature::EGRESS_ACL_TABLE: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp index 43411ed5651b4..f56c555de5efc 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk4Asic.cpp @@ -202,6 +202,7 @@ bool Tomahawk4Asic::isSupported(Feature feature) const { case HwAsic::Feature::ENABLE_DELAY_DROP_CONGESTION_THRESHOLD: case HwAsic::Feature::PORT_MTU_ERROR_TRAP: case HwAsic::Feature::DEDICATED_CPU_BUFFER_POOL: + case HwAsic::Feature::EGRESS_ACL_TABLE: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp b/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp index 49325c85dad57..608bbe3e03e2c 100644 --- a/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp +++ b/fboss/agent/hw/switch_asics/Tomahawk5Asic.cpp @@ -185,6 +185,7 @@ bool Tomahawk5Asic::isSupported(Feature feature) const { case HwAsic::Feature::ENABLE_DELAY_DROP_CONGESTION_THRESHOLD: case HwAsic::Feature::PORT_MTU_ERROR_TRAP: case HwAsic::Feature::DEDICATED_CPU_BUFFER_POOL: + case HwAsic::Feature::EGRESS_ACL_TABLE: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/TomahawkAsic.cpp b/fboss/agent/hw/switch_asics/TomahawkAsic.cpp index ac461313d44f2..affb7d52db83d 100644 --- a/fboss/agent/hw/switch_asics/TomahawkAsic.cpp +++ b/fboss/agent/hw/switch_asics/TomahawkAsic.cpp @@ -183,6 +183,7 @@ bool TomahawkAsic::isSupported(Feature feature) const { case HwAsic::Feature::ENABLE_DELAY_DROP_CONGESTION_THRESHOLD: case HwAsic::Feature::PORT_MTU_ERROR_TRAP: case HwAsic::Feature::DEDICATED_CPU_BUFFER_POOL: + case HwAsic::Feature::EGRESS_ACL_TABLE: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/Trident2Asic.cpp b/fboss/agent/hw/switch_asics/Trident2Asic.cpp index a805d3a7ae540..2d743b39334f7 100644 --- a/fboss/agent/hw/switch_asics/Trident2Asic.cpp +++ b/fboss/agent/hw/switch_asics/Trident2Asic.cpp @@ -183,6 +183,7 @@ bool Trident2Asic::isSupported(Feature feature) const { case HwAsic::Feature::ENABLE_DELAY_DROP_CONGESTION_THRESHOLD: case HwAsic::Feature::PORT_MTU_ERROR_TRAP: case HwAsic::Feature::DEDICATED_CPU_BUFFER_POOL: + case HwAsic::Feature::EGRESS_ACL_TABLE: return false; } return false; diff --git a/fboss/agent/hw/switch_asics/YubaAsic.cpp b/fboss/agent/hw/switch_asics/YubaAsic.cpp index 1ab7b5e514189..471fbfab15d00 100644 --- a/fboss/agent/hw/switch_asics/YubaAsic.cpp +++ b/fboss/agent/hw/switch_asics/YubaAsic.cpp @@ -189,6 +189,7 @@ bool YubaAsic::isSupportedNonFabric(Feature feature) const { case HwAsic::Feature::EGRESS_GVOQ_WATERMARK_BYTES: case HwAsic::Feature::ENABLE_DELAY_DROP_CONGESTION_THRESHOLD: case HwAsic::Feature::PORT_MTU_ERROR_TRAP: + case HwAsic::Feature::EGRESS_ACL_TABLE: return false; } return false; diff --git a/fboss/agent/platforms/sai/SaiPlatform.cpp b/fboss/agent/platforms/sai/SaiPlatform.cpp index 8b90b960f752e..4127a96e71c5b 100644 --- a/fboss/agent/platforms/sai/SaiPlatform.cpp +++ b/fboss/agent/platforms/sai/SaiPlatform.cpp @@ -261,11 +261,11 @@ std::string SaiPlatform::getHwAsicConfig( } #if defined(BRCM_SAI_SDK_DNX_GTE_11_0) && !defined(BRCM_SAI_SDK_DNX_GTE_12_0) - if (getAsic()->isSupported(HwAsic::Feature::EVENTOR_PORT_FOR_SFLOW)) { - // Interim workaround for 11.7 GA as this SoC property is needed for - // J3AI 11.x but not for 12.x until 12.0.0.4. - // TODO: While integrating 12.0.0.4, these workarounds need to be removed - // and instead this SoC property would be added in config directly. + // Interim workaround for 11.7 GA as this SoC property is needed for + // J3AI 11.x but not for 12.x until 12.0.0.4. + // TODO: While integrating 12.0.0.4, these workarounds need to be removed + // and instead this SoC property would be added in config directly. + if (getAsic()->getAsicType() == cfg::AsicType::ASIC_TYPE_JERICHO3) { nameValStrs.push_back("custom_feature_shel_arm_enable=1"); } #endif diff --git a/fboss/agent/state/SwitchSettings.h b/fboss/agent/state/SwitchSettings.h index 0fe60d19eb6d6..46bd0ffa9aef8 100644 --- a/fboss/agent/state/SwitchSettings.h +++ b/fboss/agent/state/SwitchSettings.h @@ -611,6 +611,40 @@ class SwitchSettings } } + std::optional getLinkFlowControlCreditThreshold() const { + if (auto linkFlowControlCreditTh = + cref()) { + return linkFlowControlCreditTh->toThrift(); + } + return std::nullopt; + } + + void setLinkFlowControlCreditThreshold( + std::optional linkFlowControlCreditTh) { + if (!linkFlowControlCreditTh) { + ref().reset(); + } else { + set( + *linkFlowControlCreditTh); + } + } + + std::optional getVoqDramBoundThreshold() const { + if (auto voqDramBoundTh = + cref()) { + return voqDramBoundTh->toThrift(); + } + return std::nullopt; + } + + void setVoqDramBoundThreshold(std::optional voqDramBoundTh) { + if (!voqDramBoundTh) { + ref().reset(); + } else { + set(*voqDramBoundTh); + } + } + SwitchSettings* modify(std::shared_ptr* state); private: diff --git a/fboss/agent/switch_config.thrift b/fboss/agent/switch_config.thrift index fb59f71a58b37..1179cb14427ba 100644 --- a/fboss/agent/switch_config.thrift +++ b/fboss/agent/switch_config.thrift @@ -605,6 +605,7 @@ enum AclStage { INGRESS = 0, INGRESS_MACSEC = 1, EGRESS_MACSEC = 2, + EGRESS = 3, } // startdocs_AclTableGroup_struct @@ -1712,7 +1713,7 @@ struct SwitchSettings { 20: optional byte sramGlobalFreePercentXonThreshold; // Fabric side threshold tracking the minimum needed // fifo free space on the peer device fifo. - 21: optional i16 fabricCllfcTxCreditThreshold; + 21: optional i16 linkFlowControlCreditThreshold; // SRAM2DRAM threshold on VOQ. Single parameter as of now // controlling both bounds and recovery thresholds. 22: optional i32 voqDramBoundThreshold; diff --git a/fboss/agent/switch_state.thrift b/fboss/agent/switch_state.thrift index da9adb7c8631d..032c7c472c492 100644 --- a/fboss/agent/switch_state.thrift +++ b/fboss/agent/switch_state.thrift @@ -393,7 +393,7 @@ struct SwitchSettingsFields { // SRAM global thresholds to send PFC XOFF/XON 44: optional byte sramGlobalFreePercentXoffThreshold; 45: optional byte sramGlobalFreePercentXonThreshold; - 46: optional i16 fabricCllfcTxCreditThreshold; + 46: optional i16 linkFlowControlCreditThreshold; 47: optional i32 voqDramBoundThreshold; } diff --git a/fboss/agent/test/TestUtils.cpp b/fboss/agent/test/TestUtils.cpp index c65d6a1e6516f..cc65fce9e7a78 100644 --- a/fboss/agent/test/TestUtils.cpp +++ b/fboss/agent/test/TestUtils.cpp @@ -271,7 +271,10 @@ cfg::SwitchConfig testConfigAImpl(bool isMhnic, cfg::SwitchType switchType) { cfg::DsfNode myNode = makeDsfNodeCfg(kVoqSwitchIdBegin); cfg.dsfNodes()->insert({*myNode.switchId(), myNode}); cfg.interfaces()->resize(kPortCount); - CHECK(myNode.systemPortRange().has_value()); + CHECK(!myNode.systemPortRanges()->systemPortRanges()->empty()); + CHECK_EQ(myNode.systemPortRanges()->systemPortRanges()->size(), 1); + auto sysPortRange = + *myNode.systemPortRanges()->systemPortRanges()->begin(); cfg.switchSettings()->switchIdToSwitchInfo() = {std::make_pair( kVoqSwitchIdBegin, createSwitchInfo( @@ -282,13 +285,12 @@ cfg::SwitchConfig testConfigAImpl(bool isMhnic, cfg::SwitchType switchType) { cfg::switch_config_constants:: DEFAULT_PORT_ID_RANGE_MAX(), /* port id range max */ 0, /* switchIndex */ - *myNode.systemPortRange()->minimum(), - *myNode.systemPortRange()->maximum(), + *sysPortRange.minimum(), + *sysPortRange.maximum(), "02:00:00:00:0F:0B", /* switchMac */ "68:00" /* connection handle */))}; for (auto i = 0; i < kPortCount; ++i) { - auto intfId = - *cfg.ports()[i].logicalID() + *myNode.systemPortRange()->minimum(); + auto intfId = *cfg.ports()[i].logicalID() + *sysPortRange.minimum(); cfg.interfaces()[i].intfID() = intfId; cfg.interfaces()[i].routerID() = 0; cfg.interfaces()[i].type() = cfg::InterfaceType::SYSTEM_PORT; @@ -359,9 +361,10 @@ cfg::SwitchConfig testConfigBImpl() { auto switchId = switchIndex + 1; cfg::DsfNode myNode = makeDsfNodeCfg(switchId); cfg.dsfNodes()->insert({*myNode.switchId(), myNode}); - CHECK(myNode.systemPortRange().has_value()); auto minPort = (switchIndex == 0) ? 0 : 16; auto maxPort = (switchIndex == 0) ? 12 : 28; + CHECK(!myNode.systemPortRanges()->systemPortRanges()->empty()); + auto sysPortRange = *myNode.systemPortRanges()->systemPortRanges()->begin(); cfg.switchSettings()->switchIdToSwitchInfo()->emplace(std::make_pair( switchId, createSwitchInfo( @@ -370,8 +373,8 @@ cfg::SwitchConfig testConfigBImpl() { minPort, /* port id range min */ maxPort, /* port id range max */ switchIndex, /* switchIndex */ - *myNode.systemPortRange()->minimum(), - *myNode.systemPortRange()->maximum(), + *sysPortRange.minimum(), + *sysPortRange.maximum(), "02:00:00:00:0F:0B", /* switchMac */ "68:00" /* connection handle */))); } @@ -496,7 +499,6 @@ cfg::DsfNode makeDsfNodeCfg( cfg::Range64 sysPortRange; sysPortRange.minimum() = switchId * kBlockSize; sysPortRange.maximum() = switchId * kBlockSize + kBlockSize; - dsfNodeCfg.systemPortRange() = sysPortRange; dsfNodeCfg.loopbackIps() = getLoopbackIps(switchId); dsfNodeCfg.nodeMac() = "02:00:00:00:0F:0B"; dsfNodeCfg.localSystemPortOffset() = *sysPortRange.minimum(); diff --git a/fboss/agent/test/agent_hw_tests/AgentHwAclQualifierTest.cpp b/fboss/agent/test/agent_hw_tests/AgentHwAclQualifierTest.cpp index efc5223012389..8861002c9083c 100644 --- a/fboss/agent/test/agent_hw_tests/AgentHwAclQualifierTest.cpp +++ b/fboss/agent/test/agent_hw_tests/AgentHwAclQualifierTest.cpp @@ -13,6 +13,7 @@ enum class QualifierType : uint8_t { LOOKUPCLASS_L2, LOOKUPCLASS_NEIGHBOR, LOOKUPCLASS_ROUTE, + LOOKUPCLASS_IGNORE, }; template @@ -104,6 +105,8 @@ namespace facebook::fboss { class AgentHwAclQualifierTest : public AgentHwTest { public: + bool addQualifiers = false; + std::vector getProductionFeaturesVerified() const override { if (!FLAGS_enable_acl_table_group) { @@ -202,8 +205,24 @@ class AgentHwAclQualifierTest : public AgentHwTest { void aclSetupHelper( bool isIpV4, QualifierType lookupClassType, + bool addQualifiers = false, SwitchID switchID = SwitchID(0)) { + this->addQualifiers = addQualifiers; auto newCfg = initialConfig(*getAgentEnsemble()); + if (FLAGS_enable_acl_table_group) { + utility::addAclTableGroup( + &newCfg, cfg::AclStage::INGRESS, utility::getAclTableGroupName()); + std::vector actions = {}; + std::vector qualifiers = addQualifiers + ? utility::genAclQualifiersConfig(this->getAsicType()) + : std::vector(); + utility::addAclTable( + &newCfg, + utility::kDefaultAclTable(), + 0 /* priority */, + actions, + qualifiers); + } auto* acl = utility::addAcl(&newCfg, kAclName(), cfg::AclActionType::DENY); if (isIpV4) { @@ -233,6 +252,9 @@ class AgentHwAclQualifierTest : public AgentHwTest { isIpV4 ? cfg::AclLookupClass::DST_CLASS_L3_LOCAL_1 : cfg::AclLookupClass::DST_CLASS_L3_LOCAL_2); break; + case QualifierType::LOOKUPCLASS_IGNORE: + // This case is here exactly to not do anything + break; default: CHECK(false); } @@ -595,4 +617,35 @@ TEST_F(AgentHwAclQualifierTest, AclIp6LookupClassRoute) { this->verifyAcrossWarmBoots(setup, verify); } + +// canary on for qualifiers from default to coop +TEST_F(AgentHwAclQualifierTest, AclQualifiersCanaryOn) { + auto setup = [=, this]() { + this->aclSetupHelper(true, QualifierType::LOOKUPCLASS_IGNORE, false); + }; + + auto verify = [=, this]() { this->aclVerifyHelper(); }; + + auto setupPostWarmboot = [=, this]() { + this->aclSetupHelper(true, QualifierType::LOOKUPCLASS_IGNORE, true); + }; + + this->verifyAcrossWarmBoots(setup, verify, setupPostWarmboot, []() {}); +} + +// canary off for qualifiers from coop to default +TEST_F(AgentHwAclQualifierTest, AclQualifiersCanaryOff) { + auto setup = [=, this]() { + this->aclSetupHelper(true, QualifierType::LOOKUPCLASS_IGNORE, true); + }; + + auto verify = [=, this]() { this->aclVerifyHelper(); }; + + auto setupPostWarmboot = [=, this]() { + this->aclSetupHelper(true, QualifierType::LOOKUPCLASS_IGNORE, false); + }; + + this->verifyAcrossWarmBoots(setup, verify, setupPostWarmboot, []() {}); +} + } // namespace facebook::fboss diff --git a/fboss/agent/test/agent_hw_tests/AgentTunnelMgrTests.cpp b/fboss/agent/test/agent_hw_tests/AgentTunnelMgrTests.cpp index 20a08debdd0b6..c0f6702a3ab61 100644 --- a/fboss/agent/test/agent_hw_tests/AgentTunnelMgrTests.cpp +++ b/fboss/agent/test/agent_hw_tests/AgentTunnelMgrTests.cpp @@ -22,16 +22,46 @@ class AgentTunnelMgrTest : public AgentHwTest { } // Clear any stale kernel entries - void clearKernelEntries(std::string intfIp) { - // Delete the source route rule entries from the kernel - auto cmd = folly::to("ip rule delete table 1"); + void clearKernelEntries(const std::string& intfIp) { + auto cmd = folly::to("ip rule list | grep ", intfIp); + + auto output = runShellCmd(cmd); + + // There could be duplicate source route rule entries in the kernel. Clear + // all of them. + while (output.find(folly::to(intfIp)) != std::string::npos) { + // Break the output string into words + std::istringstream iss(output); + std::string word; + std::string lastWord; + + while (iss >> word) { + lastWord = folly::copy(word); + } - runShellCmd(cmd); + XLOG(DBG2) << "tableId: " << lastWord; + + // Delete the source route rule entries from the kernel + cmd = folly::to("ip rule delete table ", lastWord); + + runShellCmd(cmd); + + // Get the source route rule entries again + cmd = folly::to("ip rule list | grep ", intfIp); + + output = runShellCmd(cmd); + + XLOG(DBG2) << "clearKernelEntries Cmd: " << cmd; + XLOG(DBG2) << "clearKernelEntries Output: \n" << output; + } // Get the String cmd = folly::to("ip addr list | grep ", intfIp); - auto output = runShellCmd(cmd); + output = runShellCmd(cmd); + + XLOG(DBG2) << "clearKernelEntries Cmd: " << cmd; + XLOG(DBG2) << "clearKernelEntries Output: \n" << output; // Break the output string into words std::istringstream iss(output); @@ -43,6 +73,7 @@ class AgentTunnelMgrTest : public AgentHwTest { if (subs.find("fboss") != std::string::npos) { cmd = folly::to("ip link delete ", subs); runShellCmd(cmd); + XLOG(DBG2) << "clearKernelEntries Cmd: " << cmd; break; } } while (iss); @@ -59,6 +90,9 @@ class AgentTunnelMgrTest : public AgentHwTest { auto output = runShellCmd(cmd); + XLOG(DBG2) << "checkKernelEntriesRemoved Cmd: " << cmd; + XLOG(DBG2) << "checkKernelEntriesRemoved Output: \n" << output; + EXPECT_TRUE( output.find(folly::to(intfIp)) == std::string::npos); @@ -67,6 +101,9 @@ class AgentTunnelMgrTest : public AgentHwTest { output = runShellCmd(cmd); + XLOG(DBG2) << "checkKernelEntriesRemoved Cmd: " << cmd; + XLOG(DBG2) << "checkKernelEntriesRemoved Output: \n" << output; + EXPECT_TRUE( output.find(folly::to(intfIp)) == std::string::npos); @@ -75,6 +112,9 @@ class AgentTunnelMgrTest : public AgentHwTest { output = runShellCmd(cmd); + XLOG(DBG2) << "checkKernelEntriesRemoved Cmd: " << cmd; + XLOG(DBG2) << "checkKernelEntriesRemoved Output: \n" << output; + EXPECT_TRUE( output.find(folly::to(intfIp)) == std::string::npos); } @@ -90,8 +130,8 @@ class AgentTunnelMgrTest : public AgentHwTest { } }; -// Test that the tunnel manager is able to create the source route rule entries, -// tunnel address entries and default route entries in the kernel +// Test that the tunnel manager is able to create the source route rule +// entries, tunnel address entries and default route entries in the kernel TEST_F(AgentTunnelMgrTest, checkKernelEntries) { auto setup = [=]() {}; auto verify = [=, this]() { @@ -99,22 +139,27 @@ TEST_F(AgentTunnelMgrTest, checkKernelEntries) { auto intfIp = folly::IPAddress::createNetwork( config.interfaces()[0].ipAddresses()[0], -1, false) .first; + + // Apply the config + applyNewConfig(config); + waitForStateUpdates(getAgentEnsemble()->getSw()); + // Get TunManager pointer auto tunMgr_ = getAgentEnsemble()->getSw()->getTunManager(); auto status = tunMgr_->getIntfStatus( getProgrammedState(), (InterfaceID)config.interfaces()[0].get_intfID()); // There is a known limitation in the kernel that the source route rule - // entries are not created if the interface is not up. So, checking for the - // kernel entries if the interface is up + // entries are not created if the interface is not up. So, checking for + // the kernel entries if the interface is up if (status) { // Check that the source route rule entries are present in the kernel auto cmd = folly::to("ip rule list | grep ", intfIp); auto output = runShellCmd(cmd); - XLOG(DBG2) << "Cmd: " << cmd; - XLOG(DBG2) << "Output: \n" << output; + XLOG(DBG2) << "checkKernelEntries Cmd: " << cmd; + XLOG(DBG2) << "checkKernelEntries Output: \n" << output; EXPECT_TRUE( output.find(folly::to(intfIp)) != std::string::npos); @@ -124,6 +169,9 @@ TEST_F(AgentTunnelMgrTest, checkKernelEntries) { output = runShellCmd(cmd); + XLOG(DBG2) << "checkKernelEntries Cmd: " << cmd; + XLOG(DBG2) << "checkKernelEntries Output: \n" << output; + EXPECT_TRUE( output.find(folly::to(intfIp)) != std::string::npos); @@ -132,6 +180,9 @@ TEST_F(AgentTunnelMgrTest, checkKernelEntries) { output = runShellCmd(cmd); + XLOG(DBG2) << "checkKernelEntries Cmd: " << cmd; + XLOG(DBG2) << "checkKernelEntries Output:" << output; + EXPECT_TRUE( output.find(folly::to(intfIp)) != std::string::npos); } diff --git a/fboss/agent/test/utils/ConfigUtils.cpp b/fboss/agent/test/utils/ConfigUtils.cpp index 80fcba2ec31be..9ecd26d3c22d3 100644 --- a/fboss/agent/test/utils/ConfigUtils.cpp +++ b/fboss/agent/test/utils/ConfigUtils.cpp @@ -452,18 +452,17 @@ cfg::DsfNode dsfNodeConfig( dsfNode.switchId() = *otherAsic->getSwitchId(); dsfNode.name() = folly::sformat("hwTestSwitch{}", *dsfNode.switchId()); switch (otherAsic->getSwitchType()) { - case cfg::SwitchType::VOQ: + case cfg::SwitchType::VOQ: { dsfNode.type() = cfg::DsfNodeType::INTERFACE_NODE; CHECK(otherAsic->getSystemPortRange().has_value()); - dsfNode.systemPortRange() = *otherAsic->getSystemPortRange(); - dsfNode.systemPortRanges()->systemPortRanges()->push_back( - dsfNode.systemPortRange().value()); + auto sysPortRange = otherAsic->getSystemPortRange().value(); + dsfNode.systemPortRanges()->systemPortRanges()->push_back(sysPortRange); dsfNode.nodeMac() = kLocalCpuMac().toString(); dsfNode.loopbackIps() = getLoopbackIps(SwitchID(*dsfNode.switchId())); - dsfNode.localSystemPortOffset() = *dsfNode.systemPortRange()->minimum(); - dsfNode.globalSystemPortOffset() = *dsfNode.systemPortRange()->minimum(); + dsfNode.localSystemPortOffset() = *sysPortRange.minimum(); + dsfNode.globalSystemPortOffset() = *sysPortRange.minimum(); dsfNode.inbandPortId() = kSingleStageInbandPortId; - break; + } break; case cfg::SwitchType::FABRIC: dsfNode.type() = cfg::DsfNodeType::FABRIC_NODE; break; diff --git a/fboss/agent/test/utils/PfcTestUtils.cpp b/fboss/agent/test/utils/PfcTestUtils.cpp index e2efec6611e82..be449b4911921 100644 --- a/fboss/agent/test/utils/PfcTestUtils.cpp +++ b/fboss/agent/test/utils/PfcTestUtils.cpp @@ -208,6 +208,14 @@ void setupPfcBuffers( setupBufferPoolConfig( bufferPoolCfgMap, buffer.globalShared, buffer.globalHeadroom); cfg.bufferPoolConfigs() = std::move(bufferPoolCfgMap); + if (ensemble->getHwAsicTable() + ->getHwAsics() + .cbegin() + ->second->getAsicType() == cfg::AsicType::ASIC_TYPE_JERICHO3) { + // For J3, set the SRAM global PFC thresholds as well + cfg.switchSettings()->sramGlobalFreePercentXoffThreshold() = 10; + cfg.switchSettings()->sramGlobalFreePercentXonThreshold() = 20; + } } void addPuntPfcPacketAcl(cfg::SwitchConfig& cfg, uint16_t queueId) { diff --git a/fboss/bcm_wrapper/code_gen/BUCK b/fboss/bcm_wrapper/code_gen/BUCK deleted file mode 100644 index 36ce4bc37e992..0000000000000 --- a/fboss/bcm_wrapper/code_gen/BUCK +++ /dev/null @@ -1,34 +0,0 @@ -load("@fbcode_macros//build_defs:cpp_binary.bzl", "cpp_binary") -load("@fbcode_macros//build_defs:cpp_library.bzl", "cpp_library") - -cpp_library( - name = "libheader_to_thrift", - srcs = [ - "HeaderParser.cpp", - "ThriftIDL.cpp", - ], - exported_deps = [ - "//folly:format", - "//folly:string", - ], - exported_external_deps = [ - ("boost", None, "boost_variant"), - ("llvm-fb", None, "clangASTMatchers"), - ("llvm-fb", None, "clangFrontend"), - ("llvm-fb", None, "clangTooling"), - ], -) - -cpp_binary( - name = "header_to_thrift", - srcs = [ - "HeaderToThrift.cpp", - ], - deps = [ - ":libheader_to_thrift", - ], - external_deps = [ - ("llvm-fb", None, "LLVMSupport"), - ("llvm-fb", None, "clangTooling"), - ], -) diff --git a/fboss/bcm_wrapper/code_gen/HeaderParser.cpp b/fboss/bcm_wrapper/code_gen/HeaderParser.cpp deleted file mode 100644 index c326e7f0c02b4..0000000000000 --- a/fboss/bcm_wrapper/code_gen/HeaderParser.cpp +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Copyright (c) 2004-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - * - */ -#include "HeaderParser.h" - -#include - -namespace facebook { -namespace fboss { - -const clang::ast_matchers::DeclarationMatcher& -HeaderParser::recordDeclMatcher() { - static const clang::ast_matchers::DeclarationMatcher matcher = - clang::ast_matchers::recordDecl().bind("rd"); - return matcher; -} - -const clang::ast_matchers::DeclarationMatcher& HeaderParser::enumDeclMatcher() { - static const clang::ast_matchers::DeclarationMatcher matcher = - clang::ast_matchers::enumDecl().bind("ed"); - return matcher; -} - -const clang::ast_matchers::DeclarationMatcher& -HeaderParser::functionDeclMatcher() { - static const clang::ast_matchers::DeclarationMatcher matcher = - clang::ast_matchers::functionDecl().bind("fd"); - return matcher; -} - -HeaderParser::HeaderParser() - : file_(std::make_unique("BcmWrapper")) {} - -void HeaderParser::run( - const clang::ast_matchers::MatchFinder::MatchResult& result) { - // Parse structs, unions, and class declarations - const clang::RecordDecl* rd = result.Nodes.getNodeAs("rd"); - if (rd) { - try { - file_->addStruct(std::make_unique(*rd)); - } catch (const std::exception&) { - // TODO(borisb): this case should generate thrift comments calling for - // manual code that needs to be written. - } - return; - } - // Parse enum declarations - const clang::EnumDecl* ed = result.Nodes.getNodeAs("ed"); - if (ed) { - file_->addEnum(std::make_unique(*ed)); - return; - } - // Parse function declarations - const clang::FunctionDecl* fd = - result.Nodes.getNodeAs("fd"); - if (fd) { - try { - auto tm = std::make_unique(*fd); - auto ts = std::make_unique(*fd); - if (ts->hasFields()) { - tm->resultTypeName = ts->name; - file_->addStruct(std::move(ts)); - } - file_->addMethod(std::move(tm)); - } catch (const std::exception&) { - // TODO(borisb): this case should generate thrift comments calling for - // manual code that needs to be written. - } - return; - } -} - -std::string HeaderParser::getThrift() const { - return file_->getThrift(); -}; - -} // namespace fboss -} // namespace facebook diff --git a/fboss/bcm_wrapper/code_gen/HeaderParser.h b/fboss/bcm_wrapper/code_gen/HeaderParser.h deleted file mode 100644 index fe9675095d51d..0000000000000 --- a/fboss/bcm_wrapper/code_gen/HeaderParser.h +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright (c) 2004-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - * - */ - -#pragma once - -#include -#include - -#include -#include -#include -#include - -#include "ThriftIDL.h" - -namespace facebook { -namespace fboss { -/* - * This class implements the visitor function that we run on matching nodes in - * the clang AST. It also owns the matchers we use to determine whether to run - * on a node or not. Currently, we visit record (class, union, struct) - * declarations, enum declarations and function declarations. - * - * As we visit those declarations, we populate objects which represent ThrifIDL - * objects corresponding to those declarations which we can finally use to - * output thrift corresponding to the processed header file. - */ -class HeaderParser : public clang::ast_matchers::MatchFinder::MatchCallback { - public: - HeaderParser(); - void run( - const clang::ast_matchers::MatchFinder::MatchResult& result) override; - // matcher for structs, unions, and classes - static const clang::ast_matchers::DeclarationMatcher& recordDeclMatcher(); - // matcher for enums - static const clang::ast_matchers::DeclarationMatcher& enumDeclMatcher(); - // matcher for functions - static const clang::ast_matchers::DeclarationMatcher& functionDeclMatcher(); - // return the the thrift code we have generated while processing the header - std::string getThrift() const; - - private: - // These will store the generated Thrift IDL objects we parse as we go. - // We need to store because functions will result in methods (the function - // itself) and structs (the return type) so we have to write out the file - // at the end rather than as we go. - std::unique_ptr file_; -}; -} // namespace fboss -} // namespace facebook diff --git a/fboss/bcm_wrapper/code_gen/HeaderToThrift.cpp b/fboss/bcm_wrapper/code_gen/HeaderToThrift.cpp deleted file mode 100644 index ca646e85c344f..0000000000000 --- a/fboss/bcm_wrapper/code_gen/HeaderToThrift.cpp +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright (c) 2004-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - * - */ -#include "HeaderParser.h" - -#include -#include -#include - -static llvm::cl::extrahelp commonHelp( - clang::tooling::CommonOptionsParser::HelpMessage); -static llvm::cl::OptionCategory headerToThriftCategory( - "HeaderToThrift options"); - -int main(int argc, char** argv) { - clang::tooling::CommonOptionsParser optionsParser( - argc, const_cast(argv), headerToThriftCategory); - clang::tooling::ClangTool tool( - optionsParser.getCompilations(), optionsParser.getSourcePathList()); - facebook::fboss::HeaderParser hp; - clang::ast_matchers::MatchFinder mf; - mf.addMatcher(facebook::fboss::HeaderParser::recordDeclMatcher(), &hp); - mf.addMatcher(facebook::fboss::HeaderParser::enumDeclMatcher(), &hp); - mf.addMatcher(facebook::fboss::HeaderParser::functionDeclMatcher(), &hp); - tool.run(clang::tooling::newFrontendActionFactory(&mf).get()); - llvm::outs() << hp.getThrift() << "\n"; -} diff --git a/fboss/bcm_wrapper/code_gen/ThriftIDL.cpp b/fboss/bcm_wrapper/code_gen/ThriftIDL.cpp deleted file mode 100644 index ef86c55a00a9e..0000000000000 --- a/fboss/bcm_wrapper/code_gen/ThriftIDL.cpp +++ /dev/null @@ -1,358 +0,0 @@ -/* - * Copyright (c) 2004-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - * - */ -#include "ThriftIDL.h" - -#include -#include - -#include -#include - -namespace { -// struct_name -> structName -void snakeToCamel(std::string& s) { - int numSeen = 0; - bool shouldCapitalize = false; - for (int i = 0; i < s.size(); ++i) { - int target = i + numSeen; - while (target < s.size() && s.at(target) == '_') { - ++target; - ++numSeen; - shouldCapitalize = true; - } - if (target < s.size() && numSeen) { - std::swap(s.at(i), s.at(target)); - if (shouldCapitalize) { - auto upper = toupper(s.at(i)); - s.at(i) = upper; - shouldCapitalize = false; - } - } - } - s.resize(s.size() - numSeen); -} - -// fooBar -> FooBar -void capitalize(std::string& s) { - if (s.empty()) { - return; - } - auto upper = toupper(s.at(0)); - s.at(0) = upper; -} - -// These various "stripping" functions are a bit of a kludge. I think I can fix -// this by properly handling typedef declarations rather than relying on a -// naming scheme pattern. -void stripPrefix(std::string& s, const std::string& prefix) { - std::string::size_type structLoc = s.find(prefix); - if (structLoc != std::string::npos && structLoc == 0) { - s.erase(0, prefix.size()); - } -} - -// remove leading "struct " -void stripStructPrefix(std::string& s) { - stripPrefix(s, "struct "); -} - -// remove leading "enum " -void stripEnumPrefix(std::string& s) { - stripPrefix(s, "enum "); -} - -bool stripSuffix(std::string& s, const std::string& suffix) { - std::string::size_type loc = s.find_last_of(suffix); - if (loc == s.size() - suffix.size()) { - s.erase(loc, suffix.size()); - return true; - } - return false; -} - -// remove trailing "_t" or "_s" -void stripStructSuffix(std::string& s) { - if (stripSuffix(s, "_t")) { - return; - } - stripSuffix(s, "_s"); -} - -// remove trailing "_t" or "_e" -void stripEnumSuffix(std::string& s) { - if (stripSuffix(s, "_t")) { - return; - } - stripSuffix(s, "_e"); -} - -void normalizeEnumName(std::string& s) { - stripEnumPrefix(s); - stripEnumSuffix(s); - snakeToCamel(s); - capitalize(s); -} - -void normalizeStructName(std::string& s) { - stripStructPrefix(s); - stripStructSuffix(s); - snakeToCamel(s); - capitalize(s); -} -} // namespace - -namespace facebook { -namespace fboss { - -ThriftType::ThriftType(const clang::QualType& qt) { - if (qt->isStructureType()) { - std::string t = qt.getAsString(); - normalizeStructName(t); - type = t; - } else if (qt->isEnumeralType()) { - std::string t = qt.getAsString(); - normalizeEnumName(t); - type = t; - } else if (qt->isPointerType()) { - ThriftType elementType(qt->getPointeeType()); - type = List(elementType.type); - } else if (qt->isIntegerType()) { - std::string t = qt.getAsString(); - if (t == "bool") { - type = ThriftType::Primitive::Bool; - } else if (t == "int") { - type = ThriftType::Primitive::Int32; - } - } else if (qt->isVoidType()) { - type = "void"; - } else { - type = "UNMATCHED_TYPE"; - } - name = boost::apply_visitor(ThriftType::ThriftGenVisitor(), type); -} - -ThriftType::ThriftType() { - type = "void"; - name = boost::apply_visitor(ThriftType::ThriftGenVisitor(), type); -} - -std::string ThriftType::getThrift() const { - return name; -} - -ThriftType::List::List(const ThriftType::Type& elementType) { - this->elementType = elementType; -} - -std::string ThriftType::ThriftGenVisitor::operator()( - const std::string& name) const { - return name; -} - -std::string ThriftType::ThriftGenVisitor::operator()( - const ThriftType::Primitive& primitive) const { - switch (primitive) { - case ThriftType::Primitive::Bool: - return "bool"; - case ThriftType::Primitive::Byte: - return "byte"; - case ThriftType::Primitive::Int16: - return "i16"; - case ThriftType::Primitive::Int32: - return "i32"; - case ThriftType::Primitive::Int64: - return "i64"; - case ThriftType::Primitive::Float: - return "float"; - case ThriftType::Primitive::Double: - return "double"; - case ThriftType::Primitive::Binary: - return "binary"; - case ThriftType::Primitive::String: - return "string"; - default: - return "unknown primitive"; - } -} - -std::string ThriftType::ThriftGenVisitor::operator()( - const ThriftType::List& list) const { - return folly::sformat( - "list<{}>", - boost::apply_visitor(ThriftType::ThriftGenVisitor(), list.elementType)); -} - -ThriftEnumerator::ThriftEnumerator( - const clang::EnumConstantDecl& enumConstant) { - name = enumConstant.getName().str(); - value_ = enumConstant.getInitVal().getExtValue(); -} - -std::string ThriftEnumerator::getThrift() const { - return folly::sformat("{} = {}", name, value_); -} - -ThriftEnum::ThriftEnum(const clang::EnumDecl& en) { - name = en.getName().str(); - normalizeEnumName(name); - for (const clang::EnumConstantDecl* enumConstant : en.enumerators()) { - enumerators_.emplace_back( - std::make_unique(*enumConstant)); - } -} - -std::string ThriftEnum::getThrift() const { - std::stringstream ss; - ss << "enum " << name << "\n{\n"; - for (const auto& enumerator : enumerators_) { - ss << " " << enumerator->getThrift() << ",\n"; - } - ss << "}"; - return ss.str(); -} - -ThriftField::ThriftField(int index, const clang::ValueDecl& decl) - : ThriftField(decl.getName().str(), index, decl.getType()) {} - -ThriftField::ThriftField( - const std::string& name, - int index, - const clang::QualType& qualifiedType) - : index_(index) { - if (qualifiedType->isFunctionPointerType()) { - throw std::runtime_error("can't make a thrift field with fn ptr"); - } - this->name = name; - type_ = ThriftType(qualifiedType); -} - -std::string ThriftField::getThrift() const { - return folly::sformat("{}: {} {}", index_, type_.getThrift(), name); -} - -ThriftStruct::ThriftStruct(const clang::RecordDecl& record) { - name = record.getName().str(); - normalizeStructName(name); - int index = 0; - for (const clang::FieldDecl* field : record.fields()) { - fields_.emplace_back(std::make_unique(index, *field)); - ++index; - } -} - -ThriftStruct::ThriftStruct(const clang::FunctionDecl& function) { - name = folly::sformat( - "{}Result", ThriftMethod::methodName(function.getName().str())); - normalizeStructName(name); - int index = 0; - auto rt = function.getReturnType(); - if (!rt->isVoidType()) { - fields_.emplace_back(std::make_unique("retVal", index, rt)); - ++index; - } - for (const clang::ParmVarDecl* param : function.parameters()) { - if (param->getType()->isPointerType()) { - fields_.emplace_back(std::make_unique( - param->getName().str(), index, param->getType())); - ++index; - } - } -} - -std::string ThriftStruct::getThrift() const { - std::stringstream ss; - ss << "struct " << name << "\n{\n"; - for (const auto& field : fields_) { - ss << " " << field->getThrift() << "\n"; - } - ss << "}"; - return ss.str(); -} - -bool ThriftStruct::hasFields() const { - return not fields_.empty(); -} - -ThriftMethod::ThriftMethod(const clang::FunctionDecl& function) { - name = methodName(function.getName().str()); - resultTypeName = "void"; - int index = 0; - for (const clang::ParmVarDecl* param : function.parameters()) { - parameters_.emplace_back(std::make_unique(index, *param)); - ++index; - } -} - -std::string ThriftMethod::methodName(const std::string& functionName) { - std::string s = functionName; - snakeToCamel(s); - return s; -} - -std::string ThriftMethod::getThrift() const { - std::stringstream ss; - ss << resultTypeName << " "; - ss << name << "("; - int i = 0; - for (const auto& param : parameters_) { - ss << param->getThrift(); - if (++i < parameters_.size()) { - ss << ", "; - } - } - ss << ")"; - return ss.str(); -} - -void ThriftService::addMethod(std::unique_ptr method) { - methods_.push_back(std::move(method)); -} - -std::string ThriftService::getThrift() const { - std::stringstream ss; - ss << "service " << name << "\n{\n"; - for (const auto& method : methods_) { - ss << " " << method->getThrift() << "\n"; - } - ss << "}"; - return ss.str(); -} - -ThriftFile::ThriftFile(const std::string& name) { - this->name = name; - service_.name = folly::sformat("{}Service", name); -} - -void ThriftFile::addEnum(std::unique_ptr en) { - enums_.push_back(std::move(en)); -} - -void ThriftFile::addMethod(std::unique_ptr method) { - service_.addMethod(std::move(method)); -} - -void ThriftFile::addStruct(std::unique_ptr st) { - structs_.push_back(std::move(st)); -} - -std::string ThriftFile::getThrift() const { - std::stringstream ss; - for (const auto& en : enums_) { - ss << en->getThrift() << "\n"; - } - for (const auto& st : structs_) { - ss << st->getThrift() << "\n"; - } - ss << service_.getThrift() << "\n"; - return ss.str(); -} - -} // namespace fboss -} // namespace facebook diff --git a/fboss/bcm_wrapper/code_gen/ThriftIDL.h b/fboss/bcm_wrapper/code_gen/ThriftIDL.h deleted file mode 100644 index f5513cd7e11da..0000000000000 --- a/fboss/bcm_wrapper/code_gen/ThriftIDL.h +++ /dev/null @@ -1,194 +0,0 @@ -/* - * Copyright (c) 2004-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - * - */ -#pragma once - -#include -#include -#include -#include - -namespace facebook { -namespace fboss { - -/* - * Abstract base class for all the various Thrift IDL objects we support. - * The concrete classes roughly follow the Thrift IDL grammar defined at - * https://thrift.apache.org/docs/idl - */ -class ThriftIDLObject { - public: - virtual ~ThriftIDLObject() {} - virtual std::string getThrift() const = 0; - std::string name; -}; - -/* - * Represents a thrift type as used in objects which give something - * a type. Specifically this would be types in struct fields or in - * method parameters. - * ie: in a field line like: "0: i32 x", the "i32" portion is a ThriftType. - * - * Implementation Note: - * Since we output list types, I found internally representing things with - * boost::variant to be convenient. That way a type is either a name, a - * primitive, or a list of some other valid type. We can support list> - * rather naturally this way. - * (Lists are needed because all pointer types are treated as list) - */ -class ThriftType : public ThriftIDLObject { - public: - explicit ThriftType(const clang::QualType& qt); - ThriftType(); - std::string getThrift() const override; - enum class Primitive { - Bool = 0, - Byte = 1, - Int16 = 2, - Int32 = 3, - Int64 = 4, - Float = 5, - Double = 6, - Binary = 7, - String = 8, - }; - - class List; - using Type = boost::variant< - std::string, // thrift type name (eg a struct) - Primitive, - boost::recursive_wrapper>; - - class List { - public: - explicit List(const Type& elementType); - Type elementType; - }; - Type type; - - private: - class ThriftGenVisitor : public boost::static_visitor { - public: - std::string operator()(const std::string& name) const; - std::string operator()(const Primitive& primitive) const; - std::string operator()(const List& list) const; - }; -}; - -/* - * Represents an indexed, typed entry in a list of fields. In particular, - * this is used to represent fields in structs/unions and parameters of - * methods. - */ -class ThriftField : public ThriftIDLObject { - public: - ThriftField(int index, const clang::ValueDecl& field); - ThriftField( - const std::string& name, - int index, - const clang::QualType& qualifiedType); - std::string getThrift() const override; - - private: - int index_; - ThriftType type_; -}; - -/* - * Represents a struct. Can be constructed from a clang AST struct declaration - * or directly from a clang ast function declaration in the case of generating a - * struct for the return value of a method. - */ -class ThriftStruct : public ThriftIDLObject { - public: - explicit ThriftStruct(const clang::RecordDecl& record); - explicit ThriftStruct(const clang::FunctionDecl& function); - std::string getThrift() const override; - bool hasFields() const; - - private: - std::vector> fields_; -}; - -/* - * Represents one possible value for an enum - */ -class ThriftEnumerator : public ThriftIDLObject { - public: - explicit ThriftEnumerator(const clang::EnumConstantDecl& enumConstant); - std::string getThrift() const override; - - private: - int value_; -}; - -/* - * Represents an enum. Constructed from enum declarations in the clang AST - */ -class ThriftEnum : public ThriftIDLObject { - public: - explicit ThriftEnum(const clang::EnumDecl& en); - std::string getThrift() const override; - - private: - std::vector> enumerators_; -}; - -/* - * Represents a method inside a service. Constructed from function - * declarations in the clang AST. - */ -class ThriftMethod : public ThriftIDLObject { - public: - explicit ThriftMethod(const clang::FunctionDecl& function); - std::string getThrift() const override; - static std::string methodName(const std::string& functionName); - std::string resultTypeName; - - private: - std::vector> parameters_; -}; - -/* - * Represents a service. Has a list of methods and exposes the ability to - * extract all of their generated return types so that they can be included - * in the file. - */ -class ThriftService : public ThriftIDLObject { - public: - std::string getThrift() const override; - void addMethod(std::unique_ptr method); - - private: - std::vector> methods_; -}; - -/* - * Represents the entire thrift file. This is the top level object which - * will contain all the other IDL objects. - */ -class ThriftFile : public ThriftIDLObject { - public: - explicit ThriftFile(const std::string& name); - std::string getThrift() const override; - void addEnum(std::unique_ptr en); - // Adding a method also adds the struct representing the return value - void addMethod(std::unique_ptr method); - void addStruct(std::unique_ptr st); - - private: - std::vector> enums_; - // While thrift supports multiple services in a file, for now we generate - // just one, so don't bother with the extra complication - ThriftService service_; - std::vector> structs_; -}; - -} // namespace fboss -} // namespace facebook diff --git a/fboss/cli/fboss2/commands/show/dsfnodes/CmdShowDsfNodes.h b/fboss/cli/fboss2/commands/show/dsfnodes/CmdShowDsfNodes.h index ff28eb57c8501..8fc9d2408221e 100644 --- a/fboss/cli/fboss2/commands/show/dsfnodes/CmdShowDsfNodes.h +++ b/fboss/cli/fboss2/commands/show/dsfnodes/CmdShowDsfNodes.h @@ -52,7 +52,7 @@ class CmdShowDsfNodes "Name", "Switch Id", "Type", - "System port range", + "System port ranges", }); for (auto const& entry : model.get_dsfNodes()) { @@ -77,11 +77,13 @@ class CmdShowDsfNodes entry.type() = (node.type() == cfg::DsfNodeType::INTERFACE_NODE ? "Intf Node" : "Fabric Node"); - if (node.systemPortRange()) { - entry.systemPortRange() = folly::sformat( - "({}, {})", - *node.systemPortRange()->minimum(), - *node.systemPortRange()->maximum()); + std::vector ranges; + if (node.systemPortRanges()->systemPortRanges()->size()) { + for (const auto& range : *node.systemPortRanges()->systemPortRanges()) { + ranges.push_back( + folly::sformat("({}, {})", *range.minimum(), *range.maximum())); + } + entry.systemPortRange() = folly::join(", ", ranges); } else { entry.systemPortRange() = "--"; } diff --git a/fboss/cli/fboss2/commands/show/interface/CmdShowInterface.h b/fboss/cli/fboss2/commands/show/interface/CmdShowInterface.h index 7dc523855abf3..da07fea1d51d6 100644 --- a/fboss/cli/fboss2/commands/show/interface/CmdShowInterface.h +++ b/fboss/cli/fboss2/commands/show/interface/CmdShowInterface.h @@ -115,12 +115,17 @@ class CmdShowInterface populateVlanToMtu(vlanToMtu, intfDetails); populateVlanToPrefixes(vlanToPrefixes, intfDetails); - int32_t minSystemPort = 0; + std::optional localSysPortOffset, globalSysPortOffset; for (const auto& idAndNode : dsfNodes) { const auto& node = idAndNode.second; if (utils::removeFbDomains(hostInfo.getName()) == utils::removeFbDomains(*node.name())) { - minSystemPort = *node.systemPortRange()->minimum(); + if (node.localSystemPortOffset().has_value()) { + localSysPortOffset = *node.localSystemPortOffset(); + } + if (node.globalSystemPortOffset().has_value()) { + globalSysPortOffset = *node.globalSystemPortOffset(); + } break; } } @@ -151,8 +156,13 @@ class CmdShowInterface } } - if (dsfNodes.size() > 0) { - int systemPortId = minSystemPort + portId; + if (localSysPortOffset.has_value() && globalSysPortOffset.has_value()) { + // TODO factor in portId range for this switchId. Needed for + // multi-asic systems + int systemPortId = + (portInfo.scope() == cfg::Scope::GLOBAL ? *globalSysPortOffset + : *localSysPortOffset) + + portId; ifModel.systemPortId() = systemPortId; // Extract IP addresses for DSF switches diff --git a/fboss/fsdb/if/oss/fsdb_model_thriftpath.h b/fboss/fsdb/if/oss/fsdb_model_thriftpath.h index a3dfd2ab7dc6b..e663c340a3578 100755 --- a/fboss/fsdb/if/oss/fsdb_model_thriftpath.h +++ b/fboss/fsdb/if/oss/fsdb_model_thriftpath.h @@ -3109,7 +3109,7 @@ std::pair>, std::pair>, std::pair>, -std::pair>, +std::pair>, std::pair>>; using ChildrenById = fatal::tuple< std::pair, Child<::facebook::fboss::cfg::L2LearningMode, ::apache::thrift::type_class::enumeration, ::apache::thrift::type::enum_t<::facebook::fboss::cfg::L2LearningMode>>>, std::pair, Child>, @@ -3202,7 +3202,7 @@ std::pair>, std::pair>, std::pair>, -std::pair>, +std::pair>, std::pair>>::template type_of; template @@ -3253,7 +3253,7 @@ std::pair @@ -3302,7 +3302,7 @@ std::pair, :: std::pair>, std::pair>, std::pair>, -std::pair>, +std::pair>, std::pair>>; using ChildrenById = fatal::tuple< std::pair, Child<::facebook::fboss::cfg::L2LearningMode, ::apache::thrift::type_class::enumeration, ::apache::thrift::type::enum_t<::facebook::fboss::cfg::L2LearningMode>>>, std::pair, Child>, @@ -13127,7 +13127,7 @@ std::pair>, std::pair>, std::pair>, -std::pair>, +std::pair>, std::pair>>::template type_of; template @@ -13154,7 +13154,7 @@ std::pair @@ -13179,7 +13179,7 @@ std::pair error{}; @@ -21,27 +21,25 @@ int16_t PidLogic::calculatePwm(float measurement) { if (error.has_value()) { integral_ = integral_ + *error * dT_; auto derivative = (*error - lastError_) / dT_; - newPwm = (*pidSetting_.kp() * error.value()) + + pwmDelta = (*pidSetting_.kp() * error.value()) + (*pidSetting_.ki() * integral_) + (*pidSetting_.kd() * derivative); + newPwm = lastPwm_ + pwmDelta; } else { newPwm = lastPwm_; } - if (measurement <= maxVal) { - integral_ = newPwm / *pidSetting_.ki(); - } - if (newPwm < 0) { newPwm = 0; } else if (newPwm > 100) { newPwm = 100; } - XLOG(DBG1) << fmt::format( - "Measurement: {}, Error: {}, Last PWM: {}, New PWM: {}", + XLOG(DBG2) << fmt::format( + "Measurement: {}, Error: {}, Last PWM: {}, PWM Delta: {}, New PWM: {}", measurement, error.value_or(0), lastPwm_, + pwmDelta, newPwm); lastPwm_ = newPwm; diff --git a/fboss/platform/fan_service/tests/PidLogicTests.cpp b/fboss/platform/fan_service/tests/PidLogicTests.cpp index de272cb6fb526..6c62437346c8f 100644 --- a/fboss/platform/fan_service/tests/PidLogicTests.cpp +++ b/fboss/platform/fan_service/tests/PidLogicTests.cpp @@ -19,10 +19,10 @@ TEST(PIDLogicTest, Basic) { pidSetting.posHysteresis() = 0; // CASE 1: The read value is lower than setPoint, and the previous target PWM - // is too high. PWM should go down. (60 -> 0) + // is too high. PWM should go down. (60 -> 35) auto pidLogic1 = PidLogic(pidSetting, 5); pidLogic1.updateLastPwm(60); - EXPECT_EQ(pidLogic1.calculatePwm(50), 0); + EXPECT_EQ(pidLogic1.calculatePwm(50), 35); EXPECT_EQ(pidLogic1.getLastError(), 8); // CASE 2: The read value is within desired range. @@ -33,17 +33,16 @@ TEST(PIDLogicTest, Basic) { EXPECT_EQ(pidLogic2.getLastError(), 0); // CASE 3: The read value is higher than setPoint, and the current target PWM - // is too low. PWM should go up a bit. (30 -> 24) + // is too low. PWM should go up a bit. (30 -> 54) auto pidLogic3 = PidLogic(pidSetting, 5); pidLogic3.updateLastPwm(30); - EXPECT_EQ(pidLogic3.calculatePwm(68), 24); + EXPECT_EQ(pidLogic3.calculatePwm(68), 54); EXPECT_EQ(pidLogic3.getLastError(), -8); - // CASE 4: Though read value is higher than setPoint, the current target PWM - // is overly high. PWM should go down. (99 -> 24) + // CASE 4: Read value is higher than setPoint, PWM should go up (99 -> 100) auto pidLogic4 = PidLogic(pidSetting, 5); pidLogic4.updateLastPwm(99); - EXPECT_EQ(pidLogic4.calculatePwm(68), 24); + EXPECT_EQ(pidLogic4.calculatePwm(68), 100); EXPECT_EQ(pidLogic4.getLastError(), -8); }