Policy Based Hashing HLD#773
Merged
liat-grozovik merged 11 commits intosonic-net:masterfrom Jul 27, 2021
Merged
Conversation
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
3 tasks
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
madhupalu
reviewed
Jun 9, 2021
madhupalu
reviewed
Jun 9, 2021
madhupalu
reviewed
Jun 9, 2021
madhupalu
reviewed
Jun 9, 2021
madhupalu
reviewed
Jun 9, 2021
Contributor
madhupalu
left a comment
There was a problem hiding this comment.
Provided few comments.
Collaborator
|
I don't think this is available yet
If I remember correct in the HLD it was mentioned that the infra will be provided and later on decide which to listen
I would wait first to see the infra in place and available at the time the PBH is upstream and then it will be added.
Get Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: Madhu ***@***.***>
Sent: Wednesday, June 9, 2021 6:05:32 PM
To: Azure/SONiC ***@***.***>
Cc: Liat Grozovik ***@***.***>; Review requested ***@***.***>
Subject: Re: [Azure/SONiC] Policy Based Hashing HLD (#773)
@madhupalu commented on this pull request.
________________________________
In doc/pbh/pbh-design.md<#773 (comment)>:
+--hash_list 'inner_ip_proto,inner_l4_dst_port,inner_l4_src_port,inner_dst_ipv6,inner_src_ipv6' \
+--action set_ecmp_hash --counter enabled
+config pbh rule update 'vxlan' 'pbh_table' --counter disabled
+config pbh rule remove 'vxlan' 'pbh_table'
+```
+
+**The following command adds/updates/removes hash:**
+```bash
+config pbh hash add 'inner_dst_ipv6' --field 'INNER_DST_IPV6' --mask 'FFFF::' --sequence 4
+config pbh hash update 'inner_dst_ipv6' --mask 'FFFF:FFFF::'
+config pbh hash remove 'inner_dst_ipv6'
+```
+
+#### 2.6.2.2 Show command group
+
+**The following command shows table configuration:**
You are right for CLI validations, however, there is a SAI Error handling HLD - https://github.com/Azure/SONiC/blob/master/doc/error-handling/error_handling_design_spec.md
you can refer this for SAI error handling for PBH SAI object failiures. It would be advice to list down what are the applications should listen/react to PBH failures?
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub<#773 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKTABAYQ3ANZWLOLIRSA5G3TR57LZANCNFSM42ZEWTUQ>.
|
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Collaborator
|
@anish-n can you please approve the HLD? code is ready and aligned with all the feedback provided offline |
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
anish-n
previously approved these changes
Jul 14, 2021
Signed-off-by: Nazarii Hnydyn <nazariig@nvidia.com>
Collaborator
Author
|
@anish-n if no more objections, can we merge this? |
anish-n
approved these changes
Jul 16, 2021
Contributor
|
@nazariig Looks good to me, I don't have merge permission so please request someone with merge permission for the merge. |
yxieca
pushed a commit
to sonic-net/sonic-mgmt
that referenced
this pull request
Jul 19, 2021
Description of PR Introduces a test similar to the hash_test in SONiC, but for hashing based on the inner packet tuples. The test validates that packets are ECMP'd across multiple nexthops in an balanced way using the inner tuples only. Summary: Approach What is the motivation for this PR? To test packet hashing based on the inner packet tuples instead of the standard outer packet tuples. The test checks the distribution of packets is as expected, and that each of the inner packet hash parameters(inner src ip, dst ip, src port, dst port and ip proto) leads to a variation of ports hashed to, ie: ECMP spreading. The test also validates, as an optional mode, symmetric hashing: 2 directions of a flow end up on the same next-hop. Note: The test assumes that inner hashing is configured on DUT prior to the test, in the future once inner hashing becomes a configuration parameter via config db, the test will be enhanced with configuration abilities. Feature which will expose this via configuration: sonic-net/SONiC#773 How did you do it? PTF test which generates inner packet tuples How did you verify/test it? Developed the test and ran it on a DUT configured with inner hashing Any platform specific information? Supported on select Mellanox DUTs for now, other platforms may be added as they are tested Supported testbed topology if it's a new test case? T0 Other topologies may be added as they are tested.
Collaborator
Author
|
@qiluo-msft / @yxieca / @prsunny please help to merge |
liat-grozovik
approved these changes
Jul 26, 2021
vmittal-msft
pushed a commit
to vmittal-msft/sonic-mgmt
that referenced
this pull request
Sep 28, 2021
Description of PR Introduces a test similar to the hash_test in SONiC, but for hashing based on the inner packet tuples. The test validates that packets are ECMP'd across multiple nexthops in an balanced way using the inner tuples only. Summary: Approach What is the motivation for this PR? To test packet hashing based on the inner packet tuples instead of the standard outer packet tuples. The test checks the distribution of packets is as expected, and that each of the inner packet hash parameters(inner src ip, dst ip, src port, dst port and ip proto) leads to a variation of ports hashed to, ie: ECMP spreading. The test also validates, as an optional mode, symmetric hashing: 2 directions of a flow end up on the same next-hop. Note: The test assumes that inner hashing is configured on DUT prior to the test, in the future once inner hashing becomes a configuration parameter via config db, the test will be enhanced with configuration abilities. Feature which will expose this via configuration: sonic-net/SONiC#773 How did you do it? PTF test which generates inner packet tuples How did you verify/test it? Developed the test and ran it on a DUT configured with inner hashing Any platform specific information? Supported on select Mellanox DUTs for now, other platforms may be added as they are tested Supported testbed topology if it's a new test case? T0 Other topologies may be added as they are tested.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Nazarii Hnydyn nazariig@nvidia.com