RIB FIB HLD#2060
Conversation
|
/azp run |
|
No pipelines are associated with this pull request. |
|
Could we add a knob to enable this feature selectively, based on demand? The reason I ask is that the NHG process between kernel and zebra has not been stable—we’ve had it disabled since 202405 to address a production issue (see sonic-net/sonic-buildimage#23459 |
selva-nexthop
left a comment
selva-nexthop
left a comment
There was a problem hiding this comment.
I have one design question regarding where the NHG block is being maintained. Currently, this HLD proposes to keep the NHG block in fpmsyncd and persist it in Redis for warm restart reconciliation. While this works for the FRR-based stack, it introduces a few long-term architectural risks:
Tight Coupling to FRR:
I am not sure about other deployments of SONiC using different RIB entities. But modularization-wise, just want to understand the rationale of the placement of NHG blcok.
Maintaining NHG mapping in fpmsyncd makes the design dependent on FRR as the RIB. If in future deployments another routing entity (or alternate northbound programming mechanism) is used instead of FRR, the cache management logic would need to be duplicated in other agents
Placement of State Ownership:
Conceptually, NHG reconciliation feels closer to orchagent, since orchagent is the consumer responsible for applying NHG state into ASIC DB. Having the reconciliation state live in fpmsyncd risks splitting ownership of critical NHG state between two daemons.
Redis as Central Persistency:
Persisting the cache in Redis is fine for warm restart, but the ownership boundary still matters. If orchagent owns NHG lifecycle, it would be more consistent for it to also own the NHG cache, while Redis just acts as the persistence layer.
EDDIE: Once we use FPM, it is tight coupled with FRR. Currently, I don't see any valid use cases to replace FRR to somethingelse. I would not worry about this tight couple to FRR, since it is current arch design. Any change in this arch design would need to be discussed in TSC.
EDDIE:
EDDIE: We discussed it in WG before. Current logic, during warmreboot, fpmsyncd would check if routes information are changed or not and skip routes updating to avoid unnecessary hardware update. We want to keep this design philosophy as well. Redis is an expensive access. We want to keep only SONIC OBJs in there. FIB would break RIB information into corresponding SONiC OBJs. |
That is a valid ask. We don't plan to remove current code path. If NHG usage is disabled, we will use current code path. But if NHG usage is enabled, we will go with RIB/FIB path. |
|
Regarding the comment about FRR and FIB manager being tightly coupled, I must agree with it. FIB manager should also work for controllers taping on SONiC. Perhaps, Requirements should be clearly stated at the beginning of this document. |
I added a section at the very beginning of this document to clarify these discussions are not in the scope of this document. https://github.com/eddieruan-alibaba/SONiC/blob/ribfib/doc/ribfib/ribfib.md#topics-not-in-the-scope |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
No pipelines are associated with this pull request. |
Signed-off-by: Eddie Ruan <eddie.ruan@alibaba-inc.com>
Signed-off-by: Eddie Ruan <eddie.ruan@alibaba-inc.com>
…led in fpmsyncd Signed-off-by: Eddie Ruan <eddie.ruan@alibaba-inc.com>
Signed-off-by: Eddie Ruan <eddie.ruan@alibaba-inc.com>
Signed-off-by: Eddie Ruan <eddie.ruan@alibaba-inc.com>
|
Hi @eddieruan-alibaba — the 202605 branch cut is at end of May. @selva-nexthop left review comments in March. Please address them. CC: @vaibhavhd @radha-danda (branch managers) |
The review comments have been addressed. Currently, I am waiting all Code PRs to be merged |
) * [fpmsyncd]: add libnexthopgroup in Makefile.am to support rib/fib feature What I did Add libnexthopgroup in fpmsyncd Makefile.am to support RIB/FIB Why I did it In the RIB/FIB design, we need the libnexthopgroup compiled from sonic-fib to communicate with Zebra. More details please refer the documents following: RIB/FIB HLD: sonic-net/SONiC#2060 nhg mgr LLD: sonic-net/SONiC#2270 sonic-fib PR: sonic-net/SONiC#2270 RIB/FIB nhg mgr code: #4395
|
This PR/feature will not be included in the SONiC 202605 release. It can be retargeted for the 202611 release. |
| * Each entry in the SONiC Zebra NHG table must correspond to an entry in Zebra’s NHG hash table. | ||
| * The entry stores the struct zebra_dplane_ctx as the Zebra context, which is the yellow and green parts in the following diagram. | ||
| * In addition, it maintains SONiC-specific metadata, such as the SONiC NHG ID, which is the blue part in the following diagram. | ||
|  |
There was a problem hiding this comment.
What is the "uint32_t id" in the yellow part of the diagram?
There was a problem hiding this comment.
It is zebra NHG id. It is just a logic view.
| This design allows the SONiC Zebra NHG table to serve as a mapping table between Zebra NHG IDs and SONiC NHG IDs. | ||
|
|
||
| #### NEXTHOP KEY to Zebra NHG ID mapping | ||
| This mapping table is used during warm reboot to mapping new zebra NHG ID to existing SONiC NHG ID. |
There was a problem hiding this comment.
Can you add details on what constitutes a nexthop key?
There was a problem hiding this comment.
I just want to use generate term in HLD. The key would be defined in LLD, based on the features we supports.
Logically, it is the same set of fields used in zebra. But since we don't support some MPLS features, some fields would not be carried over. So it would be a trimmed version.
| ### SONiC NHG table | ||
| In certain scenarios, it is necessary to create a new SONIC Next Hop Group (NHG) upon receiving a zebra NHG event. In such cases, the corresponding SONiC NHG will be maintained in the SONiC NHG table. | ||
|
|
||
| For the SRv6 VPN use case, NHG messages from Zebra carry not only forwarding information but also additional context, such as the VPN SID or a policy SID list. To handle this, SONiC will create a gateway NHG that contains the forwarding information only. This gateway NHG is also referred to as the PIC NHG. The accompanying context information will be used to construct a PIC Context object in SONiC. |
There was a problem hiding this comment.
Can we stick with a single term..either "gateway" or "pic" NHG and use that consistently in the document. It would also be good to use that consistently in the code.
There was a problem hiding this comment.
Also can you list what exactly is part of "gateway"/"pic" NHG.
In the HLD, it would be good to have lesser code and have more logical content. Or you can remove the rest and only highlight the part of the code that is pertinent (with explanations about what it is doing or what the field means).
There was a problem hiding this comment.
Good point, I will remove PIC NHG term, only use gateway nhg.
There was a problem hiding this comment.
Removed PIC NHG and only use gateway NHG in eddieruan-alibaba@06b4f30
| This mapping table is used during warm reboot to mapping new zebra NHG ID to existing SONiC NHG ID. | ||
|
|
||
| ### SONiC NHG table | ||
| In certain scenarios, it is necessary to create a new SONIC Next Hop Group (NHG) upon receiving a zebra NHG event. In such cases, the corresponding SONiC NHG will be maintained in the SONiC NHG table. |
There was a problem hiding this comment.
The description is not very clear to me. Which scenarios require the creation of SONiC NHG? I thought that would get created all the time? Can you add more explanation here?
There was a problem hiding this comment.
PE3# show ip route vrf Vrf1 192.100.0.1 next
Routing entry for 192.100.0.1/32
Known via "bgp", distance 20, metric 0, vrf Vrf1, best
Last update 00:16:37 ago
Nexthop Group ID: 246
Received Nexthop Group ID: 244
q 2064:100::1d(vrf default) (recursive), label 16, weight 1
q fc06::2, via Ethernet12(vrf default), label 16, weight 1
q fc08::2, via Ethernet4(vrf default), label 16, weight 1
q 2064:200::1e(vrf default) (recursive), label 32, weight 1
q fc06::2, via Ethernet12(vrf default), label 32, weight 1
q fc08::2, via Ethernet4(vrf default), label 32, weight 1
PE3# show nexthop rib 244
ID: 244 (zebra)
RefCnt: 10 Flags: 0x403
Uptime: 00:16:46
VRF: default(No AFI)
Nexthop Count: 2
Valid, Installed
Depends: (238) (245)
via 2064:100::1d (vrf default) inactive, label 16, seg6 fd00:201:201:1::, weight 1
via 2064:200::1e (vrf default) inactive, label 32, seg6 fd00:202:202:2::, weight 1
For this NHG, it has 2064:100::1d and 2064:200::1e as nexthops, with corresponding VPN contexts.
This zebra NHG would create two SONIC object, SONiC NHG and SONiC PIC CONTEXT
127.0.0.1:6378> hgetall "NEXTHOP_GROUP_TABLE:4"
- "nexthop"
- "2064:100::1d,2064:200::1e"
- "ifname"
- "unknown,unknown"
- "weight"
- "1,1"
- "seg_src"
- "2064:300::1f,2064:300::1f"
127.0.0.1:6378> hgetall "PIC_CONTEXT_TABLE:3"
- "nexthop"
- "2064:200::1e,2064:100::1d"
- "ifname"
- "unknown,unknown"
- "weight"
- "1,1"
- "vpn_sid"
- "fd00:202:202:2::,fd00:201:201:1::"
- "seg_src"
- "2064:300::1f,2064:300::1f"
SONiC NHG is sharable, while SONiC PIC Context is not since it has VPN context.
Since SONiC NHG is sharable, it stores in SONiC NHG table.
Signed-off-by: Eddie Ruan <eddie.ruan@alibaba-inc.com>
|
/azp run |
|
No pipelines are associated with this pull request. |
Signed-off-by: Yuqing Zhao <galadriel.zyq@alibaba-inc.com>
|
/azp run |
|
No pipelines are associated with this pull request. |
…nic-net#4394) * [fpmsyncd]: add libnexthopgroup in Makefile.am to support rib/fib feature What I did Add libnexthopgroup in fpmsyncd Makefile.am to support RIB/FIB Why I did it In the RIB/FIB design, we need the libnexthopgroup compiled from sonic-fib to communicate with Zebra. More details please refer the documents following: RIB/FIB HLD: sonic-net/SONiC#2060 nhg mgr LLD: sonic-net/SONiC#2270 sonic-fib PR: sonic-net/SONiC#2270 RIB/FIB nhg mgr code: sonic-net#4395 Signed-off-by: Yogapriya Mohankumar <ymohanku@cisco.com>
8 participants
This HLD describes why we need to introduce RIB/FIB in SONiC and how we approach it.
Phrase 1 target: routes download