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

Support tunnel traffic QoS remapping #2190

Merged
merged 13 commits into from
Apr 27, 2022
Merged

Support tunnel traffic QoS remapping #2190

merged 13 commits into from
Apr 27, 2022

Conversation

bingwang-ms
Copy link
Contributor

@bingwang-ms bingwang-ms commented Mar 21, 2022 •
edited
Loading

Signed-off-by: bingwang [email protected]

What I did
This PR is to update orchagent to support tunnel traffic remapping.
HLD sonic-net/SONiC#950
Changes include

  • Change the termination type of mux tunnel to P2P to apply different attributes
  • Update tunneldecaporch to apply new SAI attributes to tunnel
  • Update muxorch to apply new SAI attributes to tunnel
  • Update qosorch to parse new QoS map tables

Why I did it
Please check HLD sonic-net/SONiC#950 for more details.

How I verified it
Verified by vs test

sudo pytest test_mux.py -v --dvsname=vs   
collected 8 items                                                                                                                                                                                     

test_mux.py::TestMuxTunnel::test_Tunnel PASSED                                                                                                                                                  [ 12%]
test_mux.py::TestMuxTunnel::test_Peer PASSED                                                                                                                                                    [ 25%]
test_mux.py::TestMuxTunnel::test_Neighbor PASSED                                                                                                                                                [ 37%]
test_mux.py::TestMuxTunnel::test_Fdb PASSED                                                                                                                                                     [ 50%]
test_mux.py::TestMuxTunnel::test_Route PASSED                                                                                                                                                   [ 62%]
test_mux.py::TestMuxTunnel::test_acl PASSED                                                                                                                                                     [ 75%]
test_mux.py::TestMuxTunnel::test_mux_metrics PASSED                                                                                                                                             [ 87%]
test_mux.py::test_nonflaky_dummy PASSED                                                                                                                                                         [100%]

sudo pytest test_tunnel.py -v --dvsname=vs
collected 6 items                                                                                                                                                                                     

test_tunnel.py::TestDecapTunnel::test_TunnelDecap_v4 PASSED                                                                                                                                     [ 16%]
test_tunnel.py::TestDecapTunnel::test_TunnelDecap_v6 PASSED                                                                                                                                     [ 33%]
test_tunnel.py::TestDecapTunnel::test_TunnelDecap_MuxTunnel PASSED                                                                                                                              [ 50%]
test_tunnel.py::TestSymmetricTunnel::test_TunnelSymmetric_v4 PASSED                                                                                                                             [ 66%]
test_tunnel.py::TestSymmetricTunnel::test_TunnelSymmetric_v6 PASSED                                                                                                                             [ 83%]
test_tunnel.py::test_nonflaky_dummy PASSED

System test is work in progress.

Details if related

@bingwang-ms bingwang-ms requested a review from neethajohn March 21, 2022 12:04
prsunny
prsunny requested changes Mar 21, 2022
View reviewed changes
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
@prsunny
Copy link
Collaborator

prsunny commented Mar 21, 2022

We would need VS test update and also reminder to change the dual tor sonic-mgmt test for IPinIP cases.

@bingwang-ms
Address comments
094494b 
Signed-off-by: bingwang <[email protected]>
@bingwang-ms
Copy link
Contributor Author

PR still WIP. Thanks

@bingwang-ms
Read all tables
19f120a 
Signed-off-by: bingwang <[email protected]>
neethajohn
neethajohn reviewed Apr 4, 2022
View reviewed changes
orchagent/qosorch.cpp Outdated Show resolved Hide resolved
@bingwang-ms
Build pass
6168628 
Signed-off-by: bingwang <[email protected]>
@bingwang-ms
Merge Master into tunnel_dscp_remap
22a7b24 
Signed-off-by: bingwang <[email protected]>
@bingwang-ms
Update vs test
dbf28d3 
Signed-off-by: bingwang <[email protected]>
@bingwang-ms
Add vs test cases
2af94da 
Signed-off-by: bingwang <[email protected]>
@bingwang-ms bingwang-ms marked this pull request as ready for review April 12, 2022 08:35
@bingwang-ms
Copy link
Contributor Author

PR still WIP. Thanks

PR is ready for review.

@lgtm-com
Copy link

lgtm-com bot commented Apr 12, 2022

This pull request introduces 1 alert when merging 2af94da into 6301db7 - view on LGTM.com

new alerts:

  • 1 for Declaration hides parameter

@bingwang-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

prsunny
prsunny requested changes Apr 20, 2022
View reviewed changes
orchagent/muxorch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
orchagent/muxorch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
orchagent/tunneldecaporch.cpp Outdated Show resolved Hide resolved
@bingwang-ms
Address comments
11efd68 
Signed-off-by: bingwang <[email protected]>
@lgtm-com
Copy link

lgtm-com bot commented Apr 20, 2022

This pull request fixes 1 alert when merging 11efd68 into 1fd1dbf - view on LGTM.com

fixed alerts:

  • 1 for Unused import

prsunny
prsunny reviewed Apr 20, 2022
View reviewed changes
orchagent/muxorch.cpp Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Apr 22, 2022

This pull request fixes 1 alert when merging a82a593 into 1fd1dbf - view on LGTM.com

fixed alerts:

  • 1 for Unused import

prsunny
prsunny reviewed Apr 25, 2022
View reviewed changes
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.

Overally, the changes lgtm. One minor comment regarding backward compatibility .

orchagent/muxorch.cpp Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Apr 26, 2022

This pull request fixes 1 alert when merging 151255d into 7f53db7 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@prsunny
Copy link
Collaborator

prsunny commented Apr 26, 2022

Please fix coverage

@bingwang-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bingwang-ms
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bingwang-ms bingwang-ms merged commit 88fad97 into sonic-net:master Apr 27, 2022
@bingwang-ms bingwang-ms mentioned this pull request Apr 29, 2022
Merged
@msosyak msosyak mentioned this pull request Jun 10, 2022
Merged
@bingwang-ms bingwang-ms mentioned this pull request Jun 13, 2022
Merged
prsunny pushed a commit that referenced this pull request Jun 13, 2022
Fix key generation in removeDecapTunnel (#2322)
bf4d890 
*After the latest changes in this PR #2190 an issue was introduced. When the tunnel was deleted the TunnelTermEntries were deleted from ASIC but not from the OA cache. Due to that then the same tunnel is created the TunnelTermEntries are not created as OA has it in local cache.
Signed-off-by: Myron Sosyak <[email protected]>
yxieca pushed a commit that referenced this pull request Jun 20, 2022
@yxieca
Fix key generation in removeDecapTunnel (#2322)
83a1306 
*After the latest changes in this PR #2190 an issue was introduced. When the tunnel was deleted the TunnelTermEntries were deleted from ASIC but not from the OA cache. Due to that then the same tunnel is created the TunnelTermEntries are not created as OA has it in local cache.
Signed-off-by: Myron Sosyak <[email protected]>
preetham-singh pushed a commit to preetham-singh/sonic-swss that referenced this pull request Aug 6, 2022
@bingwang-ms @preetham-singh
Support tunnel traffic QoS remapping (sonic-net#2190)
c9ad7c3 
* Support tunnel traffic remap

Signed-off-by: bingwang <[email protected]>
preetham-singh pushed a commit to preetham-singh/sonic-swss that referenced this pull request Aug 6, 2022
@preetham-singh
Fix key generation in removeDecapTunnel (sonic-net#2322)
b1d8dba 
*After the latest changes in this PR sonic-net#2190 an issue was introduced. When the tunnel was deleted the TunnelTermEntries were deleted from ASIC but not from the OA cache. Due to that then the same tunnel is created the TunnelTermEntries are not created as OA has it in local cache.
Signed-off-by: Myron Sosyak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neethajohn neethajohn neethajohn left review comments

@prsunny prsunny prsunny 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.
4 participants
@bingwang-ms @prsunny @neethajohn @yxieca