-
Notifications
You must be signed in to change notification settings - Fork 648
[vnetorch] missing handling of rx and tx interval of monitoring session #3878
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
base: master
Are you sure you want to change the base?
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi @prsunny - can you help merge this? Without the request description change, orchagent will throw exception when we add The code change should be covered by DVS UT. |
prsunny
left a comment
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 check backward compatibility.
|
@zjswhhh , possible to fix UT coverage? |
|
@zjswhhh , would you also update the BFD hld with these changes? |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Pull Request Overview
This PR adds support for configurable BFD monitoring intervals (rx_monitor_timer and tx_monitor_timer) for VNET tunnel routes, as specified in the Overlay ECMP enhancements HLD. This is needed for SSW HA scenarios where DPU-side BFD operates in software and requires reasonable interval values.
Key changes:
- Added
rx_monitor_timerandtx_monitor_timerparameters to VNET route configuration - Extended BFD session creation to accept and apply these timer values
- Added unit test coverage for the new timer functionality
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/vnet_lib.py | Added rx_monitor_timer and tx_monitor_timer parameters to route creation/update helper functions |
| tests/test_vnet.py | Added new test case to verify timer handling in VNET routes with BFD monitoring |
| orchagent/vnetorch.h | Updated function signatures and request descriptor to support timer parameters |
| orchagent/vnetorch.cpp | Implemented timer handling throughout the route processing pipeline and BFD session creation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@zjswhhh , please fix conflicts |
… rx_monitor_timer
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi @prsunny - can you help merge please? |
correct typo in comments Co-authored-by: Copilot <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
What I did
Adding
rx_monitor_timerandtx_monitor_timerhandling per HLD: https://github.com/sonic-net/SONiC/blob/master/doc/vxlan/Overlay%20ECMP%20ehancements.mdsign-off: Jing Zhang [email protected]
Why I did it
It's needed for SSW HA scenario as DPU side bfd is a software solution, interval must be set to a reasonable value.
How I verified it
UTs.
Details if related