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

Generic Hash HLD #1101

Merged
merged 4 commits into from
Jan 29, 2023
Merged

Generic Hash HLD #1101

merged 4 commits into from
Jan 29, 2023
+1,782 −0

Conversation

nazariig
Copy link
Collaborator

@nazariig nazariig commented Oct 24, 2022 •
edited
Loading

Signed-off-by: Nazarii Hnydyn [email protected]

This document provides general information about GH implementation in SONiC
This scope of this HLD is:

  • In scope:
    ECMP/LAG switch hash configuration

  • Out of scope:
    ECMP/LAG switch hash seed configuration
    ECMP/LAG switch hash algorithm configuration

PR title state context
GH YANG Model GitHub issue/pull request detail GitHub pull request check contexts
GH DB Schema GitHub issue/pull request detail GitHub pull request check contexts
GH VS Library GitHub issue/pull request detail GitHub pull request check contexts
GH Switch OA GitHub issue/pull request detail GitHub pull request check contexts
GH CLI GitHub issue/pull request detail GitHub pull request check contexts
GH Test Plan GitHub issue/pull request detail GitHub pull request check contexts
GH Test GitHub issue/pull request detail GitHub pull request check contexts

@nazariig
[hash]: Add GH HLD.
56bfff2 
Signed-off-by: Nazarii Hnydyn <[email protected]>
@nazariig
[hash]: GH HLD rev01.
cd680c6 
Signed-off-by: Nazarii Hnydyn <[email protected]>
nazariig
nazariig commented Oct 25, 2022
View reviewed changes
doc/hash/hash-design.md Outdated Show resolved Hide resolved
@rlhui
Copy link
Contributor

rlhui commented Oct 25, 2022

Does the hash field configuration apply for original incoming packets of the switch or final modified packets of the switch?

@rlhui
Copy link
Contributor

rlhui commented Oct 25, 2022

please look into how we can add api to expose what fields are actually being used in the hash by the switch.

@liat-grozovik
Copy link
Collaborator

@prsunny @rlhui kindly reminder to review. the follow up HLD review with the community is set for 12/06.
Appreciate if you can review it following the prev review feedback

@nazariig
Copy link
Collaborator Author

nazariig commented Dec 5, 2022

Does the hash field configuration apply for original incoming packets of the switch or final modified packets of the switch?

@rlhui to the original packets only - configuration is tunnel agnostic according to the SAI behavioral model

@nazariig
Copy link
Collaborator Author

nazariig commented Dec 5, 2022 •
edited
Loading

please look into how we can add api to expose what fields are actually being used in the hash by the switch.

@rlhui done

@nazariig nazariig requested a review from rlhui December 5, 2022 14:29
@nazariig
[hash]: GH HLD rev02.
1235e84 
Signed-off-by: Nazarii Hnydyn <[email protected]>
@zhangyanzhao
Copy link
Collaborator

reviewed in Sonic community meeting.

@nazariig nazariig mentioned this pull request Dec 12, 2022
Closed
@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Dec 13, 2022 •
edited
Loading

@rlhui and @prsunny
Following the community review it was agreed to move forward with this HLD and have a second phase with new SAI API.
As of that, if any additional feedback to this HLD is available please send the sooner the better. Otherwise we will assume all is agreed and we will merge this HLD in the next few days.

This was referenced Jan 4, 2023
Merged
Merged
Merged
Merged
Merged
@liat-grozovik
Copy link
Collaborator

@zhangyanzhao the HLD was reviewed and then we had a follow up. Code PRs are ready.
We should move forward with approval and merge as no feedback provided since the last review which was about month ago.

@liat-grozovik
Copy link
Collaborator

The feature and its current content is expected to be in 202305 and under discussion if to take into 202211 as originally aimed.

@liat-grozovik liat-grozovik merged commit d25b4ed into sonic-net:master Jan 29, 2023
@nazariig nazariig mentioned this pull request Feb 8, 2023
Merged
@congh-nvidia congh-nvidia mentioned this pull request Feb 21, 2023
Merged
7 tasks
prsunny pushed a commit to sonic-net/sonic-swss that referenced this pull request Jun 26, 2023
@nazariig
[hash]: Implement GH backend (#2598)
a67f684 
* [hash]: Implement GH backend as per HLD: sonic-net/SONiC#1101
liat-grozovik pushed a commit to sonic-net/sonic-utilities that referenced this pull request Jul 10, 2023
@nazariig
[hash]: Implement GH frontend (#2580)
ff380e0 
HLD sonic-net/SONiC#1101

- What I did
Implemented CLI for Generic Hash feature

- How I did it
Integrated Generic Hash interface into config and show CLI root

- How to verify it
Run Generic Hash CLI UTs

Signed-off-by: Nazarii Hnydyn <[email protected]>
theasianpianist pushed a commit to theasianpianist/sonic-swss that referenced this pull request Jul 20, 2023
@nazariig @theasianpianist
[hash]: Implement GH backend (sonic-net#2598)
75dd6b5 
* [hash]: Implement GH backend as per HLD: sonic-net/SONiC#1101
pdhruv-marvell pushed a commit to pdhruv-marvell/sonic-utilities that referenced this pull request Aug 23, 2023
@nazariig @pdhruv-marvell
[hash]: Implement GH frontend (sonic-net#2580)
b4c79ee 
HLD sonic-net/SONiC#1101

- What I did
Implemented CLI for Generic Hash feature

- How I did it
Integrated Generic Hash interface into config and show CLI root

- How to verify it
Run Generic Hash CLI UTs

Signed-off-by: Nazarii Hnydyn <[email protected]>
liat-grozovik pushed a commit to sonic-net/sonic-mgmt that referenced this pull request Oct 10, 2023
@congh-nvidia
Test plan for feature Generic Hash (#7524)
40e10c4 
Test plan for the new feature Generic Hash.
The HLD of Generic Hash: sonic-net/SONiC#1101
The test implementation is not included in this PR, will open another PR.
@ZhaohuiS ZhaohuiS mentioned this pull request Jun 19, 2024
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liat-grozovik liat-grozovik liat-grozovik approved these changes

@prsunny prsunny Awaiting requested review from prsunny

@volodymyrsamotiy volodymyrsamotiy Awaiting requested review from volodymyrsamotiy

@rlhui rlhui Awaiting requested review from rlhui

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
@nazariig @rlhui @liat-grozovik @zhangyanzhao