[fpmsyncd]: add nhgmgr to support RIB/FIB design - #4395

Open

LARLSN wants to merge 39 commits into

sonic-net:masterfrom

LARLSN:dev-ribfib-nhgmgr

LARLSN commented Mar 26, 2026 •

edited

Loading

Contributor

What I did

Adding nhg mgr to realize fib block in RIB/FIB design.

Why I did it

In the RIB/FIB design, we need to add a fib block to fpmsyncd to process the NHG information received from zebra, and create corresponding Sonic NHG objects according to zebra information. Through fib block, NHG ID processing and PIC (prefix-independent convergence) can be introduced.In this way, the goal of improving routing convergence performance in RIB/FIB design can be achieved.

More details about RIB/FIB please refer RIB/FIB HLD pr: sonic-net/SONiC#2060
More details about nhg mgr please refer nhg mgr LLD pr: sonic-net/SONiC#2270

How I verified it

Unit tests in code
Daily vsonic tests

Details if related

LARLSN requested a review from prsunny as a code owner

March 26, 2026 05:41

mssonicbld commented Mar 26, 2026

Collaborator

/azp run

azure-pipelines Bot commented Mar 26, 2026

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

LARLSN force-pushed the dev-ribfib-nhgmgr branch from 47e7733 to 83a9d94 Compare

March 26, 2026 05:44

mssonicbld commented Mar 26, 2026

Collaborator

/azp run

azure-pipelines Bot commented Mar 26, 2026

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

LARLSN force-pushed the dev-ribfib-nhgmgr branch from 83a9d94 to 851dc93 Compare

March 26, 2026 05:46

mssonicbld commented Mar 26, 2026

Collaborator

/azp run

azure-pipelines Bot commented Mar 26, 2026

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

LARLSN closed this

LARLSN reopened this

mssonicbld commented Mar 26, 2026

Collaborator

/azp run

azure-pipelines Bot commented Mar 26, 2026

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

LARLSN changed the title ~~Dev ribfib nhgmgr~~ [fpmsyncd] add nhgmgr to support RIB/FIB design

LARLSN force-pushed the dev-ribfib-nhgmgr branch from 851dc93 to 44f18f3 Compare

March 26, 2026 05:54

mssonicbld commented Mar 26, 2026

Collaborator

/azp run

azure-pipelines Bot commented Mar 26, 2026

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

LARLSN force-pushed the dev-ribfib-nhgmgr branch 2 times, most recently from a3ced51 to 4d06431 Compare

March 26, 2026 05:59

LARLSN changed the title ~~[fpmsyncd] add nhgmgr to support RIB/FIB design~~ [fpmsyncd]: add nhgmgr to support RIB/FIB design

eddieruan-alibaba mentioned this pull request

RIB FIB HLD sonic-net/SONiC#2060

Open

yijingyan2 commented Mar 27, 2026

Contributor

/azp run

azure-pipelines Bot commented Mar 27, 2026

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

This was referenced Mar 30, 2026

Add RIB/RIB nhg mgr LLD sonic-net/SONiC#2270

Open

[fpmsyncd]: add libnexthopgroup in Makefile.am to support rib/fib #4394

Merged

mssonicbld commented Apr 8, 2026

Collaborator

/azp run

azure-pipelines Bot commented Apr 8, 2026

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

eddieruan-alibaba added this to SONiC Routing Dashboard

github-project-automation Bot moved this to Todo in SONiC Routing Dashboard

eddieruan-alibaba assigned LARLSN

LARLSN added 19 commits

August 5, 2026 09:55


          [fpmsyncd]: update gateway nhg creation

05cfa38

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: update the check of the recursive nexthop

9ba38ce

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: update the handle of NHG dependency relationship

932dc03

	1. Not validate the dependency ID in nhg mgr.
	2. Update dependents handle. when A -> B, add dependents of A when add the entry for A, not updated when added the B.
        3. Add deubg info of dependency.

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: update handle of NHG

3bb1b4a

Fix critical bugs in nhgmgr:
- Fix operator< strict weak ordering violation in SonicNHGObjectKey
- Fix use-after-free in SonicGateWayNHGTable::updateEntry
- Fix dangling pointers in cleanUp methods
- Fix SonicIDMgr memory leak, add destructor
- Fix refCount underflow protection
- Fix addNewNHGFull returning success on nullptr
- Remove unused m_db member
- Simplify compareDependsAndDependents
- Move using namespace std into namespace swss scope

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [sonic-swss]: update nhg weight to fix update in sonic-fib

018387b

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: update check of SRv6 NHG

8f904c7

	1. add flag m_is_srv6_nhg to indicate the NHG with srv6 NHG info.
	2. for the multi NHG, check the m_is_srv6_nhg flag of members to determine whether it is SRv6 NHG.

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [tests]: fix SRv6 VPN nexthop UT seg6_seg_stack allocation

658dabb

 Allocate seg6_seg_stack with enough space for the flexible array member
 and pre-populate seg[0] with VPN SID data. The library's singleton
 constructor copies total_size bytes from nh_seg6_segs_in, so the input
 must include space for segments, not just the header.

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: remove repeat check for PIC objetc

881bdf0

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: remove update of pic obj and update sonic gateway obj to …

893a598

…sonic PIC content

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: add unit tests to improve NHG manager coverage

76f4ccd

Add 9 new test cases covering:
- SharedSonicNHGReuse: shared NHG deduplication via refCount
- UpdateSRv6VPNNHGContent: PIC Content update on vpn_sid change
- UpdateWithKeyChange: key change triggers sonic NHG rebuild
- DeleteNonExistentNHG: deleting non-existent entry returns 0
- SegSrcInPICContextTable: seg_src field verification
- EnableDisableNHG: enable/disable status toggle
- CheckNeedUpdateFieldChanges: field-level change detection
- AddWithInvalidGroupMember: invalid group member fails addEntry
- SingleMemberMultiNHG: single resolved member skips sonic NHG creation

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: add unit tests for NHG manager error/exception handling p…

b64e6ba

…aths

Cover error branches in RIBNHGTable, SonicPICContentTable, SonicIDMgr,
and the previously untested updateSonicPICObject function. Tests include
duplicate/non-existent entry operations, unsupported type handling,
reference count underflow, empty fvVector, key comparison edge cases,
enable/disable NHG, SRv6 field getters, and cleanUp operations.

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: add unit tests for NHG manager comparison functions and k…

b2faa09

…ey creation

Add 12 new test cases covering previously untested branches:
- compareDependsAndDependents (depends/dependents vector changes)
- compareNHGFullList (size, id, weight, num_direct mismatches)
- compareNHGSRv6Fields (null vs non-null, different SID content)
- checkNeedUpdate field checks (weight, vrf_id, ifindex, ifname, type, bh_type)
- createSonicNormalNHGObjectKey (single/multi nexthop branches)
- createSonicPICContentObjectKey (entry-based overload)
- recoverSonicIDMapFromDB and getRIBNHGEntryByKey stub verification
- dumpNHGGroupFull no-crash test

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: fix numDirects in CreateSonicNormalNHGObjectKeyMultiNexth…

1a088e5

…op test

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: rename Sonic NHG table to Sonic PIC table

6ab7846

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: remove unused codes

0e52679

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: add libnexthopgroup in Makefile.am to support rib/fib fea…

21a659d

…ture

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [ci]: Re-add libnexthopgroup to Azure Pipeline build

01e532d

Re-add libnexthopgroup deb package from sonic-buildimage common_libs.

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: update codes to resolve comments in nhg mgr

4b11c78

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: update ribid struct

246a4af

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>

LARLSN force-pushed the dev-ribfib-nhgmgr branch from fa68c0e to 19767fe Compare

August 5, 2026 01:59

mssonicbld commented Aug 5, 2026

Collaborator

/azp run

azure-pipelines Bot commented Aug 5, 2026

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld commented Aug 5, 2026

Collaborator

/azp run

azure-pipelines Bot commented Aug 5, 2026

Azure Pipelines: Successfully started running 1 pipeline(s).

LARLSN commented Aug 5, 2026

Contributor Author

@selva-nexthop hi selva, Thanks a lot for the detailed review — really appreciate you taking the time to go through this carefully. I've addressed all of your comments in the latest revision:

#	Comment	Reply
1	Member NHG not in table fails; how are out-of-order FPM messages handled?	Zebra guarantees the ordering here: in zebra's dataplane, an NHG's dependencies (singleton nexthops) are always installed and pushed through FPM before the group that references them, and fpmsyncd processes FPM messages in order. So a parent NHG can never legitimately arrive before its members. If it does happen, it indicates a zebra-side bug, and failing loudly here (rather than buffering) is the intended behavior.
2	[Shared key changed to another existing shared key — stale sonic id?](https://github.com/sonic-net/sonic swss/pull/4395#discussion_r3681043692)	Verified: `updateEntry()` internally calls `setEntry()` → `checkNeedCreateSonicNHGObj()`, which looks up the existing shared object by the new key, sets the entry's sonic id to the existing one and adds a reference. So by the time we reach this branch, the entry already holds the correct id; we only need to drop the reference on the previous key. Added a comment in the code to clarify this.
3	refCount underflow still frees/deletes the object, masking accounting bugs	Code changed: fail loudly, keep object
4	Inconsistent freeID() arguments (`.id` vs `sonicObjectID`)	Done, all `freeID()` call sites now pass `sonicObjectID` consistently.
5	Write the zebra new-NHG-id assumption down in the LLD	Done. Documented in the comment block at the top of `updateExistingNHGFull()`, and also updated the LLD accordingly — added a "Note on update assumptions" section under the update flow in`doc/ribfib/nhgmgr_lld.md`.

When you have a moment, could you take another look?

LARLSN added 2 commits

August 5, 2026 18:52


          [fpmsyncd]: update nhg mgr to resolve review comments

- Fail loudly on shared NHG refcount underflow instead of deleting the
  object, so accounting bugs are not masked
- Pass sonicObjectID consistently to freeID()/removeFromDB()
- Document the zebra new-NHG-id assumption for ownership transitions
  and the shared-key update path

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>


          [fpmsyncd]: update nhgmgr UT to match new refcount underflow semantics

ef75e39

subSonicNHGObjectRef now keeps the entry and fails loudly on refcount
underflow instead of deleting the shared NHG object, so the UT should
assert the entry is preserved rather than removed.

Signed-off-by: linsongnan <linsongnan.lsn@alibaba-inc.com>

LARLSN force-pushed the dev-ribfib-nhgmgr branch from d20b597 to ef75e39 Compare

August 5, 2026 10:52

mssonicbld commented Aug 5, 2026

Collaborator

/azp run

azure-pipelines Bot commented Aug 5, 2026

Azure Pipelines: Successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

+1 more reviewer

At least 1 approving review is required to merge this pull request.

Labels

None yet