-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Policy Based Hashing HLD #773
Conversation
Signed-off-by: Nazarii Hnydyn <[email protected]>
Signed-off-by: Nazarii Hnydyn <[email protected]>
Signed-off-by: Nazarii Hnydyn <[email protected]>
Signed-off-by: Nazarii Hnydyn <[email protected]>
Signed-off-by: Nazarii Hnydyn <[email protected]>
Signed-off-by: Nazarii Hnydyn <[email protected]>
Signed-off-by: Nazarii Hnydyn <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provided few comments.
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>.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the responses.
Signed-off-by: Nazarii Hnydyn <[email protected]>
Signed-off-by: Nazarii Hnydyn <[email protected]>
@anish-n can you please approve the HLD? code is ready and aligned with all the feedback provided offline |
Signed-off-by: Nazarii Hnydyn <[email protected]>
Signed-off-by: Nazarii Hnydyn <[email protected]>
@anish-n if no more objections, can we merge this? |
@nazariig Looks good to me, I don't have merge permission so please request someone with merge permission for the merge. |
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.
@qiluo-msft / @yxieca / @prsunny please help to merge |
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.
Signed-off-by: Nazarii Hnydyn [email protected]