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.

Sign up for GitHub

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

Jump to bottom

[subnet decap] Add subnet decap rule based on overlay ECMP #3153

Merged
merged 3 commits into from
May 31, 2024
Merged

[subnet decap] Add subnet decap rule based on overlay ECMP #3153

merged 3 commits into from
May 31, 2024

Conversation

lolyu
Copy link
Contributor

@lolyu lolyu commented May 17, 2024 •
edited
Loading

What I did
Support dynamic decap rule generation based on the Vxlan tunnel route of Overlay ECMP.
This depends on: #3117

Why I did it
To enable SONiC with the capability to decap IPinIP packets with dest IP in the Overlay ECMP Vxlan tunnel route prefix.

  • Microsoft ADO (number only): 28294953

How I verified it
VS test:

test_vnet.py::TestVnetOrch::test_vnet_orch_1 PASSED                                                                                                                                            [  3%]
test_vnet.py::TestVnetOrch::test_vnet_orch_2 PASSED                                                                                                                                            [  6%]
test_vnet.py::TestVnetOrch::test_vnet_orch_3 PASSED                                                                                                                                            [  9%]
test_vnet.py::TestVnetOrch::test_vnet_orch_4 SKIPPED (Failing. Under investigation)                                                                                                            [ 12%]
test_vnet.py::TestVnetOrch::test_vnet_orch_5 PASSED                                                                                                                                            [ 15%]
test_vnet.py::TestVnetOrch::test_vnet_vxlan_multi_map PASSED                                                                                                                                   [ 18%]
test_vnet.py::TestVnetOrch::test_vnet_orch_7[true] PASSED                                                                                                                                      [ 21%]
test_vnet.py::TestVnetOrch::test_vnet_orch_7[false] PASSED                                                                                                                                     [ 24%]
test_vnet.py::TestVnetOrch::test_vnet_orch_8[true] PASSED                                                                                                                                      [ 27%]
test_vnet.py::TestVnetOrch::test_vnet_orch_8[false] PASSED                                                                                                                                     [ 30%]
test_vnet.py::TestVnetOrch::test_vnet_orch_9[true] PASSED                                                                                                                                      [ 33%]
test_vnet.py::TestVnetOrch::test_vnet_orch_9[false] PASSED                                                                                                                                     [ 36%]
test_vnet.py::TestVnetOrch::test_vnet_orch_10[true] PASSED                                                                                                                                     [ 39%]
test_vnet.py::TestVnetOrch::test_vnet_orch_10[false] PASSED                                                                                                                                    [ 42%]
test_vnet.py::TestVnetOrch::test_vnet_orch_11[true] PASSED                                                                                                                                     [ 45%]
test_vnet.py::TestVnetOrch::test_vnet_orch_11[false] PASSED                                                                                                                                    [ 48%]
test_vnet.py::TestVnetOrch::test_vnet_orch_12 PASSED                                                                                                                                           [ 51%]
test_vnet.py::TestVnetOrch::test_vnet_orch_13 PASSED                                                                                                                                           [ 54%]
test_vnet.py::TestVnetOrch::test_vnet_orch_14 PASSED                                                                                                                                           [ 57%]
test_vnet.py::TestVnetOrch::test_vnet_orch_15 PASSED                                                                                                                                           [ 60%]
test_vnet.py::TestVnetOrch::test_vnet_orch_16 PASSED                                                                                                                                           [ 63%]
test_vnet.py::TestVnetOrch::test_vnet_orch_17 PASSED                                                                                                                                           [ 66%]
test_vnet.py::TestVnetOrch::test_vnet_orch_18 PASSED                                                                                                                                           [ 69%]
test_vnet.py::TestVnetOrch::test_vnet_orch_19 PASSED                                                                                                                                           [ 72%]
test_vnet.py::TestVnetOrch::test_vnet_orch_20 PASSED                                                                                                                                           [ 75%]
test_vnet.py::TestVnetOrch::test_vnet_orch_21 PASSED                                                                                                                                           [ 78%]
test_vnet.py::TestVnetOrch::test_vnet_orch_22 PASSED                                                                                                                                           [ 81%]
test_vnet.py::TestVnetOrch::test_vnet_orch_23 PASSED                                                                                                                                           [ 84%]
test_vnet.py::TestVnetOrch::test_vnet_orch_24 PASSED                                                                                                                                           [ 87%]
test_vnet.py::TestVnetOrch::test_vnet_orch_25 PASSED                                                                                                                                           [ 90%]
test_vnet.py::TestVnetOrch::test_vnet_orch_26 PASSED                                                                                                                                           [ 93%]
test_vnet.py::TestVnetOrch::test_vnet_orch_27 PASSED                                                                                                                                           [ 96%]
test_vnet.py::test_nonflaky_dummy PASSED                                                                                                                                                       [100%]

Details if related

@lolyu lolyu requested a review from bingwang-ms May 17, 2024 12:15
@lolyu lolyu marked this pull request as ready for review May 20, 2024 09:39
@lolyu lolyu requested a review from prsunny as a code owner May 20, 2024 09:39
@lolyu lolyu mentioned this pull request May 20, 2024
Merged
bingwang-ms
bingwang-ms reviewed May 21, 2024
View reviewed changes
orchagent/vnetorch.cpp
@@ -2088,6 +2123,14 @@ void VNetRouteOrch::postRouteState(const string& vnet, IpPrefix& ipPrefix, NextH
removeRouteAdvertisement(prefix_to_use);
}
}
if (route_state == "active")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this code be in the if block at line#2115?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The if block at LINE#2115 checks if the vnet is allowed to advertise prefix. The decap term should be added even if the vnet is not allowed to advertise prefix.

bingwang-ms
bingwang-ms reviewed May 21, 2024
View reviewed changes
orchagent/vnetorch.cpp Outdated
inline void VNetRouteOrch::removeSubnetDecapTerm(const IpPrefix &ipPrefix)
{
const SubnetDecapConfig &config = gTunneldecapOrch->getSubnetDecapConfig();
if (!config.enable || subnet_decap_terms_created_.find(ipPrefix) == subnet_decap_terms_created_.end())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest not checking config.enable. Even if it's disabled, user should be able to remove it.

@lolyu lolyu requested a review from bingwang-ms May 22, 2024 02:20
@bingwang-ms bingwang-ms mentioned this pull request May 24, 2024
Open
8 tasks
@prsunny
Copy link
Collaborator

prsunny commented May 29, 2024

@lolyu , we need to ensure test_vnet_vxlan in sonic-mgmt passes and no stale decap entries present. Could you please confirm?

lolyu added 3 commits May 30, 2024 06:24
@lolyu
[subnet decap] Add subnet decap rule based on overlay ECMP vxlan tunn…
bd75f77 
…el route

Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
@lolyu
Fix/improve vs test to validate the subnet decap tunnel
2f2e1ea 
Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
@lolyu
Fix comments
075af2f 
Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
@lolyu lolyu force-pushed the subnet_decap_overlay_ecmp branch from abc7e91 to 075af2f Compare May 30, 2024 06:24
@lolyu
Copy link
Contributor Author

lolyu commented May 31, 2024

@lolyu , we need to ensure test_vnet_vxlan in sonic-mgmt passes and no stale decap entries present. Could you please confirm?

Hi @prsunny, as long as subnet decap feature is not enabled, no more decap term entries will be created. So test_vnet_vxlan should be safe here.

@prsunny prsunny merged commit 1f12a8d into sonic-net:master May 31, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prsunny prsunny prsunny approved these changes

@bingwang-ms bingwang-ms bingwang-ms approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lolyu @prsunny @bingwang-ms