Skip to content
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

VNET and BGP route coexistence #3345

Merged
merged 25 commits into from
Dec 2, 2024

Conversation

siqbal1986
Copy link
Contributor

@siqbal1986 siqbal1986 commented Oct 29, 2024

What I did
This PR adds the logic to give Vnet routes precedence over BGP learnt route.
This PR also refactors the test_vnet.py to break out the common code into a library.
Why I did it
Currently if a route is learnt via BGP and added into the hardware, adding a VNET route results in failure. In addition due to a bug in VnetOrch, we start advertising the failed route, This prompts the BGP to remove the learnt route in favor of the local route. Since the Vnet Orch doesn't retry adding the Vnet route, This results in no route being present in the Hardware.
How I verified it
Added 2 tests to verify the learnt route is removed when a Vnet route is added.
One test covers the scnerio where BFD sessions are brought up before the Vnet route is added and the 2nd covers when the Vnet route is added before the BFD sessions are brought up.
Details if related

Tests covering this chage.
sonic-net/sonic-mgmt#15710

image

@siqbal1986 siqbal1986 requested a review from prsunny as a code owner October 29, 2024 20:30
@siqbal1986 siqbal1986 changed the title vnet route precedence over BGP learnt route. VNET and BGP route coexistence Oct 29, 2024
@prsunny
Copy link
Collaborator

prsunny commented Oct 30, 2024

Can you fix coverage?

@siqbal1986
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@prsunny prsunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, minor comment. please fix as a different PR

@@ -2639,6 +2639,51 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx)
return true;
}

bool RouteOrch::hasBgpRoute(const IpPrefix& prefix)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename to isRouteExists. bgp is not applicable in orchagent.

@prsunny prsunny merged commit dd420da into sonic-net:master Dec 2, 2024
17 checks passed
bradh352 pushed a commit to bradh352/sonic-swss that referenced this pull request Dec 4, 2024
What I did
This PR adds the logic to give Vnet routes precedence over BGP learnt route.
This PR also refactors the test_vnet.py to break out the common code into a library.
Why I did it
Currently if a route is learnt via BGP and added into the hardware, adding a VNET route results in failure. In addition due to a bug in VnetOrch, we start advertising the failed route, This prompts the BGP to remove the learnt route in favor of the local route. Since the Vnet Orch doesn't retry adding the Vnet route, This results in no route being present in the Hardware.
bradh352 pushed a commit to bradh352/sonic-swss that referenced this pull request Dec 4, 2024
What I did
This PR adds the logic to give Vnet routes precedence over BGP learnt route.
This PR also refactors the test_vnet.py to break out the common code into a library.
Why I did it
Currently if a route is learnt via BGP and added into the hardware, adding a VNET route results in failure. In addition due to a bug in VnetOrch, we start advertising the failed route, This prompts the BGP to remove the learnt route in favor of the local route. Since the Vnet Orch doesn't retry adding the Vnet route, This results in no route being present in the Hardware.
divyachandralekha pushed a commit to divyachandralekha/sonic-swss that referenced this pull request Dec 12, 2024
What I did
This PR adds the logic to give Vnet routes precedence over BGP learnt route.
This PR also refactors the test_vnet.py to break out the common code into a library.
Why I did it
Currently if a route is learnt via BGP and added into the hardware, adding a VNET route results in failure. In addition due to a bug in VnetOrch, we start advertising the failed route, This prompts the BGP to remove the learnt route in favor of the local route. Since the Vnet Orch doesn't retry adding the Vnet route, This results in no route being present in the Hardware.
divyachandralekha pushed a commit to divyachandralekha/sonic-swss that referenced this pull request Dec 12, 2024
What I did
This PR adds the logic to give Vnet routes precedence over BGP learnt route.
This PR also refactors the test_vnet.py to break out the common code into a library.
Why I did it
Currently if a route is learnt via BGP and added into the hardware, adding a VNET route results in failure. In addition due to a bug in VnetOrch, we start advertising the failed route, This prompts the BGP to remove the learnt route in favor of the local route. Since the Vnet Orch doesn't retry adding the Vnet route, This results in no route being present in the Hardware.
StormLiangMS pushed a commit to sonic-net/sonic-mgmt that referenced this pull request Dec 19, 2024
What is the motivation for this PR?
Currently if a route is learnt via BGP and added into the hardware, adding a VNET route results in failure. In addition due to a bug in VnetOrch, we start advertising the failed route, This prompts the BGP to remove the learnt route in favor of the local route. Since the Vnet Orch doesn't retry adding the Vnet route, This results in no route being present in the Hardware.

How I verified it
The fix is in PR sonic-net/sonic-swss#3345
These tests cover various scenario in which VNET and BGP routes are added and removed in different order

How did you do it?
How did you verify/test it?
Any platform specific information?
Cisco-8000 and mlnx.

Supported testbed topology if it's a new test case?
Documentation
There are 5 differnet scenarios which are checked. Each scenario is tested with with the following options.
Encap types [v4_inv4, v6_inV4]
Monitor Type [BFD, Custom]
Init NH state( BFD/monitor sessions for nexthops are initially up or not.)

test_vnet_route_after_bgp
ADD BGP ROUTE on TOR
Add VNET route
Configure monitor (BFD or custom) with nexthop state (UP)
Test with traffic
Remove VNET route
Remove BGP route
test_vnet_route_before_bgp_after_ep_up
Add VNET route
Configure monitor (BFD or custom) with nexthop state (UP)
Add BGP ROUTE on TOR
Test with traffic
Remove VNET ROUTE
Remove BGP route
test_vnet_route_bgp_removal_before_ep
ADD BGP ROUTE on TOR
Add VNET route
Remove BGP route
Configure monitor (BFD or custom) with nexthop state (UP)
Test with traffic
Remove VNET route
test_vnet_route_after_bgp_with_early_bgp_removal
Add VNET route
Add BGP ROUTE on TOR
Configure monitor (BFD or custom) with nexthop state (UP)
Test with traffic
Remove BGP route
Test with traffic
Remove VNET route
test_vnet_route_after_bgp_multi_flap
ADD BGP ROUTE on TOR
Add VNET route
Configure monitor (BFD or custom) with nexthop state (UP)
Test with traffic
flap the bfd/monitor sessions.
Test with traffic
Remove VNET route
Remove BGP route
stepanblyschak pushed a commit to stepanblyschak/sonic-swss that referenced this pull request Jan 27, 2025
What I did
This PR adds the logic to give Vnet routes precedence over BGP learnt route.
This PR also refactors the test_vnet.py to break out the common code into a library.
Why I did it
Currently if a route is learnt via BGP and added into the hardware, adding a VNET route results in failure. In addition due to a bug in VnetOrch, we start advertising the failed route, This prompts the BGP to remove the learnt route in favor of the local route. Since the Vnet Orch doesn't retry adding the Vnet route, This results in no route being present in the Hardware.
shiraez pushed a commit to Marvell-switching/sonic-swss that referenced this pull request Feb 17, 2025
What I did
This PR adds the logic to give Vnet routes precedence over BGP learnt route.
This PR also refactors the test_vnet.py to break out the common code into a library.
Why I did it
Currently if a route is learnt via BGP and added into the hardware, adding a VNET route results in failure. In addition due to a bug in VnetOrch, we start advertising the failed route, This prompts the BGP to remove the learnt route in favor of the local route. Since the Vnet Orch doesn't retry adding the Vnet route, This results in no route being present in the Hardware.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants