SONiC
SONiC copied to clipboard
SAG HLD
Static Anycast Gateway (SAG) HLD
Repo | PR title | State | Context |
---|---|---|---|
sonic-swss-common | [schema] Add SAG table for static anycast gateway | ||
sonic-swss | [SAG]: Add SAG implementation | ||
sonic-utilities | Add SAG implementation | ||
sonic-buildimage | Add SAG SONiC Yang | ||
sonic-mgmt | Add static-anycast-gateway(SAG) test plan | ||
sonic-mgmt | Add static-anycast-gateway(SAG) test cases |
@kperumalbfn @venkatmahalingam @preetham-singh @hasan-brcm @dgsudharsan @liat-grozovik
All comments are replied and the HLD is updated.
Please help to check again.
All related PR's are sent and updated tracking table in this PR. Please help to review it.
@superchild would you please add the test PR into the code PR list? Thanks.
@superchild would you please add the test PR into the code PR list? Thanks.
@zhangyanzhao What is the test PR? The unit test code is included in the following 3 PRs
- sonic-swss: https://github.com/Azure/sonic-swss/pull/1974
- sonic-utilities: https://github.com/Azure/sonic-utilities/pull/1887
- sonic-buildimage(yang model): https://github.com/Azure/sonic-buildimage/pull/9018
sonic-swss-common#540 and sonic-utilities#1887 don't have reviewer yet, could you please assign someone help to review?
Thanks.
@superchild looks like all code PRs are still open, need defer this feature to next release (202211). Thanks.
@zhangyanzhao These PRs are already ready but always doesn't get feedback of the reviewer, is that possible to push reviewers and get this feature to include in this release? (some PR still lack of authorized reviewer to merge)
@superchild looks like all code PRs are still open, need defer this feature to next release (202211). Thanks.
Is there a reason SAG is not listed under the 202211 section on the roadmap? https://github.com/sonic-net/SONiC/wiki/Sonic-Roadmap-Planning
@superchild looks like all code PRs are still open, need defer this feature to next release (202211). Thanks.
Is there a reason SAG is not listed under the 202211 section on the roadmap? https://github.com/sonic-net/SONiC/wiki/Sonic-Roadmap-Planning
@superchild - Also interested to know the answer to the question above. When will this feature and all PR's be merged ?
It's not merged due to code PR isn't merged by the reviewer.
The related code PR had some discussion.
@zhangyanzhao is there any other way to speed up the review process and get merged by the reviewer?
In previous release, some code PR is approved by the reviewer but can't be merged due to their permission.
When time goes by, the code is out-of-date with the base branch, there's much effort to merge/resolve the conflict again.
Please help if there's any other suggestion, then I'll follow and sync the PR again.
Thanks.
This is not in 202211 release plan.
@superchild looks like all code PRs are still open, need defer this feature to next release (202211). Thanks.
Is there a reason SAG is not listed under the 202211 section on the roadmap? https://github.com/sonic-net/SONiC/wiki/Sonic-Roadmap-Planning
This is not in 202211 release plan.
@superchild would you please add the test PR into the code PR list? Thanks.
@zhangyanzhao What is the test PR? The unit test code is included in the following 3 PRs
- sonic-swss: [SAG]: Add SAG implementation sonic-swss#1974
- sonic-utilities: Add SAG implementation sonic-utilities#1887
- sonic-buildimage(yang model): Add SAG SONiC Yang sonic-buildimage#9018
sonic-swss-common#540 and sonic-utilities#1887 don't have reviewer yet, could you please assign someone help to review?
Thanks.
@superchild this is not in 202211 release plan. We need the reviewer to finish the review, approve the PRs to merge.
@superchild what are your plans? do you plan to drive this PR?
This feature is added into 202305 release plan. Let's finish the code review and merge the code @venkatmahalingam @hasan-brcm
@zhangyanzhao Thanks for your help, I'll merge the master branch's change for 202305 release.
Reviewers, can you please help to finish your review on this feature? Thanks. @gord1306 @kperumalbfn @venkatmahalingam @preetham-singh @hasan-brcm
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: superchild / name: Jimi Chen (cd6941fcd13c926e7e29a4c65a0d35a9f07cac7b, b0b5c10fa4416e36b60fd11b8dacb12714e98213, ed64df7a08fb937cf322677ff6aff0eccc50016d, 950267e2217e43aefcc53f7a5d4bfa0ef60b9217, 63c7a3bc2c1d9945639aa19cead72c8b3d8eba87, 3237e5032108204ed2c8f817b12d3bf53dc1f4fa, 5dbc369e93247665e5cfb12504ba4460497b7da1)
@superchild could you please sign the easyCLA?
@liat-grozovik thanks for your reminder, already signed.
@liat-grozovik can you please let me know if we can merge this PR? Nvidia is the registered reviewer. Thanks.
It was reviewed by Nvidia and approved (@dgsudharsan and @Junchao-Mellanox ). please go a head and merge the HLD. as for code PRs, need to see the review and feedback
General comments:
- The HLD is missing a high level description before even discussing the config db which should be followed by yang model. Please check the HLD Template guidelines (https://github.com/Azure/SONiC/blob/master/doc/hld_template.md) it is very helpful to understand the feature and be able to review the code later on.
- The CLI as described is very high level and the part of the interface does not define and of the checkers expected on the CLI level. The CLI itself IMO should be described in the same way we have in the command reference md file.
- The flow is very high level and does to explain clearly (although there is a flow image). i don't see any reference to the SAI it self and the parameters used. Missing flow of vlan interface delete, not the sag part.
- The default behaviour should be clearly mentioned. it is disabled by default.
- It was mentioned it must go with VXLAN/EVPN. Is there any validation? what will happen if not? what tis the expected behaviour? ignored? error?
@superchild can you please address this comment? Thanks.
HLD has been reviewed in SONiC community on 8/24/2021
HLD was reviewed and approved by reviewers, merge the HLD.
@liat-grozovik NVIDIA is the reviewing company, can you please merge this PR? Thanks.
New Updated PRs: sonic-swss : https://github.com/sonic-net/sonic-swss/pull/3167 sonic-utility: https://github.com/sonic-net/sonic-utilities/pull/3339 sonic-buildimage: https://github.com/sonic-net/sonic-buildimage/pull/19069