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

VNET/VXLAN changes #643

Merged
merged 10 commits into from
Nov 26, 2018
Merged

VNET/VXLAN changes #643

merged 10 commits into from
Nov 26, 2018

Conversation

prsunny
Copy link
Collaborator

@prsunny prsunny commented Oct 16, 2018

What I did
Modified Vxlanorch to support VNet peering feature.
Introduced the following:

  1. Vxlan VRF mapper
  2. Generic code to handle BRIDGE/VRF/VLAN encap/decap mapper
  3. Test case update for existing Vxlan VS test.

Why I did it
Support VNet peering using Vxlan (VNI <-> VRF)

How I verified it
Verified by VS test and checking the ASIC DB for intended SAI tables

Details if related
Design Document:
https://github.com/prsunny/SONiC/blob/gh-pages/doc/vxlan/Vxlan_hld.md
Related PR
#632

Pending:

  1. VS test case for VNET (new code)
  2. Delete sequence.

@prsunny
Copy link
Collaborator Author

prsunny commented Oct 17, 2018

retest this please

@prsunny prsunny closed this Oct 17, 2018
@prsunny prsunny reopened this Oct 17, 2018
@sonic-net sonic-net deleted a comment from qiluo-msft Oct 22, 2018
@prsunny
Copy link
Collaborator Author

prsunny commented Oct 22, 2018

retest this please

@lguohan
Copy link
Contributor

lguohan commented Oct 22, 2018

@prsunny , can you rebase your pr and do a test again? I fixed a few things on the vstest.

@lguohan
Copy link
Contributor

lguohan commented Oct 23, 2018

retest this please

lguohan
lguohan reviewed Oct 26, 2018
View reviewed changes
orchagent/vnetorch.h Outdated Show resolved Hide resolved
lguohan
lguohan reviewed Oct 26, 2018
View reviewed changes
orchagent/vxlanorch.h Outdated Show resolved Hide resolved
lguohan
lguohan reviewed Oct 26, 2018
View reviewed changes
orchagent/vxlanorch.h Outdated Show resolved Hide resolved
lguohan
lguohan reviewed Oct 26, 2018
View reviewed changes
orchagent/vxlanorch.h Outdated Show resolved Hide resolved
@lguohan
Copy link
Contributor

lguohan commented Oct 26, 2018

is the vs test in this PR or a new PR?

lguohan
lguohan reviewed Oct 26, 2018
View reviewed changes
orchagent/vxlanorch.cpp Outdated Show resolved Hide resolved
lguohan
lguohan reviewed Oct 30, 2018
View reviewed changes
tests/test_vnet.py Outdated Show resolved Hide resolved
lguohan
lguohan reviewed Oct 30, 2018
View reviewed changes
tests/test_vnet.py Outdated Show resolved Hide resolved
lguohan
lguohan reviewed Oct 30, 2018
View reviewed changes
tests/test_vnet.py Outdated Show resolved Hide resolved
lguohan
lguohan reviewed Oct 30, 2018
View reviewed changes
tests/test_vnet.py Outdated Show resolved Hide resolved
lguohan
lguohan requested changes Oct 30, 2018
View reviewed changes
Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

refactor test cases to make it more readable.

pavel-shirshov
pavel-shirshov suggested changes Nov 1, 2018
View reviewed changes
Copy link
Contributor

@pavel-shirshov pavel-shirshov left a comment

Choose a reason for hiding this comment

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

as comments

orchagent/vnetorch.cpp Outdated Show resolved Hide resolved
orchagent/vnetorch.h Show resolved Hide resolved
orchagent/vnetorch.h Show resolved Hide resolved
orchagent/vxlanorch.h Outdated Show resolved Hide resolved
tests/test_vxlan_tunnel.py Outdated Show resolved Hide resolved
orchagent/vxlanorch.cpp Outdated Show resolved Hide resolved
orchagent/vxlanorch.cpp Outdated Show resolved Hide resolved
orchagent/vxlanorch.cpp Outdated Show resolved Hide resolved
orchagent/vxlanorch.cpp Outdated Show resolved Hide resolved
orchagent/vxlanorch.cpp Outdated Show resolved Hide resolved
@pavel-shirshov
Copy link
Contributor

I see two functions with the same name, but with different signatures. If you need both functions I'd suggest to change names, otherwise you'd better to remove unused one.

sai_object_id_t create_tunnel_map_entry(
    MAP_T map_t,
    sai_object_id_t tunnel_map_id,
    sai_object_id_t obj_id,
    sai_uint32_t vni,
    bool encap)

vs

static sai_object_id_t
create_tunnel_map_entry(
    MAP_T map_t,
    sai_object_id_t tunnel_map_id,
    sai_uint32_t vni,
    sai_uint16_t vlan_id)

@prsunny
Copy link
Collaborator Author

prsunny commented Nov 9, 2018

@pavel-shirshov , addressed the comment by combining the two APIs. Initially thought to leave the existing function unmodified.

There are new changes to the PR based on VNET Table flow redesign. The new flow is updated in the HLD document.

jipanyang
jipanyang reviewed Nov 20, 2018
View reviewed changes
Copy link
Contributor

@jipanyang jipanyang left a comment

Choose a reason for hiding this comment

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

Have you updated Application DB schema with VXLAN_TUNNEL_MAP in swss-schema.md?

@prsunny prsunny merged commit bffa01f into sonic-net:master Nov 26, 2018
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
@rajkumar38
[syncd] ARM32 bit fixes, for 64bit printf format specifier. (sonic-ne…
8d3ae8c 
…t#643)

* [syncd] ARM32 bit fixes, for 64bit printf format specifier.

Signed-off-by: Rajkumar Pennadam Ramamoorthy <rpennadamram@marvell.com>

* [syncd] ARM32 bit fixes, for 64bit printf format specifier.

Signed-off-by: Rajkumar Pennadam Ramamoorthy <rpennadamram@marvell.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jipanyang jipanyang jipanyang approved these changes

@marian-pritsak marian-pritsak marian-pritsak approved these changes

@pavel-shirshov pavel-shirshov pavel-shirshov approved these changes

@lguohan lguohan lguohan approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@prsunny @lguohan @pavel-shirshov @marian-pritsak @jipanyang @stcheng