-
Notifications
You must be signed in to change notification settings - Fork 525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BFD Tx, Rx interval support for Vnet Monitored routes. #3335
base: master
Are you sure you want to change the base?
Conversation
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). |
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.
ill add it.
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check will be added later, right?
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.
yes
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
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.
extra space
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these must be initialized to defaults instead of 0
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.