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

Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID #2538

Merged
merged 5 commits into from
Dec 22, 2022
Merged

Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID #2538

merged 5 commits into from
Dec 22, 2022

Conversation

tapashdas
Copy link
Contributor

What I did
Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID

Why I did it
When a VNI (L3 VNI) is mapped to VRF, corresponding VNI (L2 VNI) to Vlan mapping is not required.
Updated VxLAN Orchagent code to remove Vlan-VNI mapping entry when VRF is mapped to same VNI.
Also Re-add Vlan-VNI mapping entry when VRF-VNI mapping for same VNI is removed.

How I verified it

Details if related

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 21, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: tapashdas / name: Tapash Das (cd0d289)

@tapashdas
Copy link
Contributor Author

@srj102 @RajeshPukhrajJain Please review

@tapashdas
Copy link
Contributor Author

@dgsudharsan Please review

@tapashdas
Copy link
Contributor Author

@LaveenBrcm Please review

@tapashdas
Copy link
Contributor Author

@hasan-brcm Please review

@srj102
Copy link
Contributor

srj102 commented Nov 21, 2022

Please update the pytest as well.

dgsudharsan
dgsudharsan requested changes Nov 23, 2022
View reviewed changes
Copy link
Collaborator

@dgsudharsan dgsudharsan left a comment

Choose a reason for hiding this comment

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

Please add UT.
There is a flow where VRF - VNI gets mapped first and then VLAN - VNI map arrives later. This PR doesn't handle it. Please handle that flow.

@tapashdas
Copy link
Contributor Author

Done

@dgsudharsan
Copy link
Collaborator

Done

Thanks. The existing UT fail due to new logic. Can you please update the UT?

@lgtm-com
Copy link

lgtm-com bot commented Dec 3, 2022

This pull request introduces 1 alert when merging 423395b into ec507a4 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@dgsudharsan
Copy link
Collaborator

@tapashdas Can you please address the failures in swss VS tests?

@tapashdas
Copy link
Contributor Author

tapashdas commented Dec 6, 2022 •
edited
Loading

Will do. Occupied with other work items. Updated VRF-VNI test case. They are passing now.
Current failures are from the newly added test case you added.

@dgsudharsan
Copy link
Collaborator

Will do. Occupied with other work items. Updated VRF-VNI test case. They are passing now. Current failures are from the newly added test case you added.

Understood. Can you please provide an ETA for addressing them?

@tapashdas
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tapashdas
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tapashdas
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Collaborator

prsunny commented Dec 19, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tapashdas
Copy link
Contributor Author

Please merge the changes.

prsunny
prsunny approved these changes Dec 22, 2022
View reviewed changes
orchagent/vxlanorch.cpp
SWSS_LOG_DEBUG("add_tunnel_map_entry name %s, vlan %d, vni %d\n", entry.vniVlanMapName.c_str(), entry.vlan_id, entry.vni_id);
}
}
if(entry.isL2Vni)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please have space afterif.

@prsunny prsunny merged commit a443945 into sonic-net:master Dec 22, 2022
@dgsudharsan
Copy link
Collaborator

@tapashdas Can you please raise submodule update?

@tapashdas tapashdas mentioned this pull request Dec 28, 2022
Closed
8 tasks
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Jan 1, 2023
@dprital
[submodule] Advance sonic-swss pointer
ea8bd29 
Update sonic-swss submodule pointer to include the following:
* bdedf69 Modify coppmgr mergeConfig to support preserving copp tables through reboot. ([sonic-net#2548](sonic-net/sonic-swss#2548))
* 7891e78 Fixed set mtu for deleted subintf due to late notification ([sonic-net#2571](sonic-net/sonic-swss#2571))
* a443945 Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID ([sonic-net#2538](sonic-net/sonic-swss#2538))

Signed-off-by: dprital <drorp@nvidia.com>
@dprital dprital mentioned this pull request Jan 1, 2023
Merged
8 tasks
liat-grozovik pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jan 2, 2023
@dprital
[submodule] Advance sonic-swss pointer (#13223)
aeaec40 
Update sonic-swss submodule pointer to include the following:

* bdedf69 Modify coppmgr mergeConfig to support preserving copp tables through reboot. ([#2548](sonic-net/sonic-swss#2548))
* 7891e78 Fixed set mtu for deleted subintf due to late notification ([#2571](sonic-net/sonic-swss#2571))
* a443945 Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID ([#2538](sonic-net/sonic-swss#2538))

Signed-off-by: dprital <drorp@nvidia.com>
@keboliu
Copy link
Collaborator

keboliu commented Jan 5, 2023

@StormLiangMS would you please help to cherry-pick to the 202211 branch?

StormLiangMS pushed a commit that referenced this pull request Feb 10, 2023
@tapashdas @StormLiangMS
Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI…
63c0234 
… ID (#2538)

* Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID
lukasstockner pushed a commit to genesiscloud/sonic-swss that referenced this pull request Mar 14, 2023
@tapashdas @lukasstockner
Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI…
cbf0709 
… ID (sonic-net#2538)

* Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID
lukasstockner pushed a commit to genesiscloud/sonic-swss that referenced this pull request Apr 2, 2023
@tapashdas @lukasstockner
Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI…
78b59e6 
… ID (sonic-net#2538)

* Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID
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

@dgsudharsan dgsudharsan dgsudharsan approved these changes

Assignees
No one assigned
Labels
Included in 202211 Branch Request for 202211 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tapashdas @srj102 @dgsudharsan @mssonicbld @prsunny @keboliu @StormLiangMS