Add SWSS support for link event damping feature #2933
+672
−0
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ using namespace swss; | |
| // types -------------------------------------------------------------------------------------------------------------- | ||
|
|
||
| typedef decltype(PortConfig::serdes) PortSerdes_t; | ||
| typedef decltype(PortConfig::link_event_damping_config) PortDampingConfig_t; | ||
|
|
||
| // constants ---------------------------------------------------------------------------------------------------------- | ||
|
|
||
|
|
@@ -126,6 +127,12 @@ static const std::unordered_map<std::string, sai_port_path_tracing_timestamp_typ | |
| { PORT_PT_TIMESTAMP_TEMPLATE_4, SAI_PORT_PATH_TRACING_TIMESTAMP_TYPE_20_27 } | ||
| }; | ||
|
|
||
| static const std::unordered_map<std::string, sai_redis_link_event_damping_algorithm_t> g_linkEventDampingAlgorithmMap = | ||
| { | ||
| { "disabled", SAI_REDIS_LINK_EVENT_DAMPING_ALGORITHM_DISABLED }, | ||
| { "aied", SAI_REDIS_LINK_EVENT_DAMPING_ALGORITHM_AIED } | ||
| }; | ||
|
|
||
| // functions ---------------------------------------------------------------------------------------------------------- | ||
|
|
||
| template<typename T> | ||
|
|
@@ -246,6 +253,11 @@ std::string PortHelper::getPtTimestampTemplateStr(const PortConfig &port) const | |
| return this->getFieldValueStr(port, PORT_PT_TIMESTAMP_TEMPLATE); | ||
| } | ||
|
|
||
| std::string PortHelper::getDampingAlgorithm(const PortConfig &port) const | ||
| { | ||
| return this->getFieldValueStr(port, PORT_DAMPING_ALGO); | ||
| } | ||
|
|
||
| bool PortHelper::parsePortAlias(PortConfig &port, const std::string &field, const std::string &value) const | ||
| { | ||
| SWSS_LOG_ENTER(); | ||
|
|
@@ -786,6 +798,60 @@ bool PortHelper::parsePortSubport(PortConfig &port, const std::string &field, co | |
| return true; | ||
| } | ||
|
|
||
| bool PortHelper::parsePortLinkEventDampingAlgorithm(PortConfig &port, const std::string &field, const std::string &value) const | ||
|
There was a problem hiding this comment. Can you please confirm that this is applied when user dynamically changes the link damping configs and not just during initialization? There was a problem hiding this comment. Yes, that is correct. Link event damping algorithm and config can be applied at runtime by changing the entries in CONFIG DB. |
||
| { | ||
| SWSS_LOG_ENTER(); | ||
|
|
||
| if (value.empty()) | ||
| { | ||
| SWSS_LOG_ERROR("Failed to parse field(%s): empty value is prohibited", field.c_str()); | ||
| return false; | ||
| } | ||
|
|
||
| const auto &cit = g_linkEventDampingAlgorithmMap.find(value); | ||
| if (cit == g_linkEventDampingAlgorithmMap.cend()) | ||
| { | ||
| SWSS_LOG_ERROR("Failed to parse field(%s): invalid value(%s)", field.c_str(), value.c_str()); | ||
| return false; | ||
| } | ||
|
|
||
| port.link_event_damping_algorithm.value = cit->second; | ||
| port.link_event_damping_algorithm.is_set = true; | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| template<typename T> | ||
| bool PortHelper::parsePortLinkEventDampingConfig(T &damping_config_attr, const std::string &field, const std::string &value) const | ||
| { | ||
| SWSS_LOG_ENTER(); | ||
|
|
||
| if (value.empty()) | ||
| { | ||
| SWSS_LOG_ERROR("Failed to parse field(%s): empty string is prohibited", field.c_str()); | ||
| return false; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| damping_config_attr.value = to_uint<std::uint32_t>(value); | ||
| damping_config_attr.is_set = true; | ||
| } | ||
| catch (const std::exception &e) | ||
| { | ||
| SWSS_LOG_ERROR("Failed to parse field(%s): %s", field.c_str(), e.what()); | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| template bool PortHelper::parsePortLinkEventDampingConfig(decltype(PortDampingConfig_t::max_suppress_time) &damping_config_attr, const std::string &field, const std::string &value) const; | ||
| template bool PortHelper::parsePortLinkEventDampingConfig(decltype(PortDampingConfig_t::decay_half_life) &damping_config_attr, const std::string &field, const std::string &value) const; | ||
| template bool PortHelper::parsePortLinkEventDampingConfig(decltype(PortDampingConfig_t::suppress_threshold) &damping_config_attr, const std::string &field, const std::string &value) const; | ||
| template bool PortHelper::parsePortLinkEventDampingConfig(decltype(PortDampingConfig_t::reuse_threshold) &damping_config_attr, const std::string &field, const std::string &value) const; | ||
| template bool PortHelper::parsePortLinkEventDampingConfig(decltype(PortDampingConfig_t::flap_penalty) &damping_config_attr, const std::string &field, const std::string &value) const; | ||
|
|
||
| bool PortHelper::parsePortPtIntfId(PortConfig &port, const std::string &field, const std::string &value) const | ||
| { | ||
| SWSS_LOG_ENTER(); | ||
|
|
@@ -1121,6 +1187,48 @@ bool PortHelper::parsePortConfig(PortConfig &port) const | |
| return false; | ||
| } | ||
| } | ||
| else if (field == PORT_DAMPING_ALGO) | ||
| { | ||
| if (!this->parsePortLinkEventDampingAlgorithm(port, field, value)) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| else if (field == PORT_MAX_SUPPRESS_TIME) | ||
| { | ||
| if (!this->parsePortLinkEventDampingConfig(port.link_event_damping_config.max_suppress_time, field, value)) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| else if (field == PORT_DECAY_HALF_LIFE) | ||
| { | ||
| if (!this->parsePortLinkEventDampingConfig(port.link_event_damping_config.decay_half_life, field, value)) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| else if (field == PORT_SUPPRESS_THRESHOLD) | ||
| { | ||
| if (!this->parsePortLinkEventDampingConfig(port.link_event_damping_config.suppress_threshold, field, value)) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| else if (field == PORT_REUSE_THRESHOLD) | ||
| { | ||
| if (!this->parsePortLinkEventDampingConfig(port.link_event_damping_config.reuse_threshold, field, value)) | ||
| { | ||
| return false; | ||
| } | ||
| } | ||
| else if (field == PORT_FLAP_PENALTY) | ||
| { | ||
| if (!this->parsePortLinkEventDampingConfig(port.link_event_damping_config.flap_penalty, field, value)) | ||
| { | ||
| return false; | ||
|
Comment on lines
+1190
to
+1229
There was a problem hiding this comment. all this and entire function could be refactored to have a stdL::map<string,function> to execute if string is compared and seems all the functions take teh same 3 parameters There was a problem hiding this comment. Hi @kcudnik, thanks for reviewing the PR. I prefer to hold off refactoring the parsePortConfig function for now, since the first parameter is different between some of the functions. If you have more suggestions, please feel free to let me know. There was a problem hiding this comment. you have opportunity right now to do refactoring, because you understand surrounding code that you are adding, and leaving this now as is, will result to add more and more code like you are adding that instead of refactoring, take this from my experience. |
||
| } | ||
| } | ||
| else | ||
| { | ||
| SWSS_LOG_WARN("Unknown field(%s): skipping ...", field.c_str()); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to use std::optional or boost::optional here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Junchao-Mellanox, thanks for your review. There was a refactor change in portsorch.cpp, and my change follows the existing port config. Maybe there can be a cleanup task in the future to update the port config to use std::optional.