BFD Tx, Rx interval support for Vnet Monitored routes. #3335
Conversation
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
|
Commenter does not have sufficient privileges for PR 3335 in repo sonic-net/sonic-swss |
|
/azpw run |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
kperumalbfn
reviewed
Oct 28, 2024
| @@ -1857,6 +1859,13 @@ void VNetRouteOrch::createBfdSession(const string& vnet, const NextHopKey& endpo | |||
| // when the device goes into TSA. The following parameter ensures that these session are | |||
| // brought down while transitioning to TSA and brought back up when transitioning to TSB. | |||
| data.emplace_back("shutdown_bfd_during_tsa", "true"); | |||
| if (has_monitor_interval) | |||
There was a problem hiding this comment.
Please add a note that without tx/rx interval in the DB, BFD session will use the default timers BFD_SESSION_DEFAULT_TX_INTERVAL and BFD_SESSION_DEFAULT_RX_INTERVAL
| @@ -1080,6 +1080,7 @@ bool VNetRouteOrch::doRouteTask<VNetVrfObject>(const string& vnet, IpPrefix& ipP | |||
| NextHopGroupKey& nexthops, string& op, string& profile, | |||
| const string& monitoring, NextHopGroupKey& nexthops_secondary, | |||
| const IpPrefix& adv_prefix, | |||
| bool check_directly_connected, | |||
There was a problem hiding this comment.
This check will be added later, right?
| @@ -1857,6 +1859,13 @@ void VNetRouteOrch::createBfdSession(const string& vnet, const NextHopKey& endpo | |||
| // when the device goes into TSA. The following parameter ensures that these session are | |||
| // brought down while transitioning to TSA and brought back up when transitioning to TSB. | |||
| data.emplace_back("shutdown_bfd_during_tsa", "true"); | |||
| if (has_monitor_interval) | |||
| { | |||
| FieldValueTuple fvTuple1("tx_interval", to_string(tx_monitor_interval)); | |||
There was a problem hiding this comment.
Please include sonic-mgmt tests with Rx/Tx timer to check if these are supported by vendor SAI for BFD hardware offload.
There was a problem hiding this comment.
ill add those tests after I have made the changes for directly connected nexthops.
| intervals.tx_monitor_timer = tx_monitor_timer; | ||
| prefix_to_monitor_intervals_[ip_pfx] = intervals; | ||
| } | ||
| if ( op == DEL_COMMAND) |
prsunny
reviewed
Oct 29, 2024
| @@ -2620,6 +2641,10 @@ bool VNetRouteOrch::handleTunnel(const Request& request) | |||
| swss::IpPrefix adv_prefix; | |||
| bool has_priority_ep = false; | |||
| bool has_adv_pfx = false; | |||
| bool has_monitor_intervals = false; | |||
| uint64_t rx_monitor_timer = 0; | |||
There was a problem hiding this comment.
these must be initialized to defaults instead of 0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What I did
This change add support for
rx_monitor_timer
tx_monitor_timer
check_directly_connected
Why I did it
This change is beneficial because it adds support for BFD Tx, Rx interval for Vnet Monitored routes as required by DASH project.
How I verified it
Added a unit test to verify the functionality.
Details if related
The Tx and Rx values are added on per-route basis. Therefore the same values are used for each nexthop. if the user needs to se different values for each next hop then they can add the values and nexthops incrementally.
e.g.
The user can also update these values by selectively removing and readding a nexthop with different Tx and Rx values.