🔴 Live TV: FRRouting/frr/pull/

usrivastava-nvidia · 2026-02-10T04:18:54Z

While registering the sessions with BFD, pimd and ospfd are passing 0.0.0.0 as local source address to BFD.
With this fix, correct local source address will be passed for BFD session creation like BGP

greptile-apps · 2026-02-10T04:22:45Z

Greptile Overview

Greptile Summary

Changed OSPF and PIM to pass explicit local source addresses when creating BFD sessions (similar to BGP), instead of passing NULL which causes BFD to auto-select the source.

OSPF (ospfd/ospf_bfd.c): Added proper null check oi->address ? &oi->address->u.prefix4 : NULL before passing local source, matching the pattern used in BGP
PIM (pimd/pim_bfd.c): Passes &pim_ifp->primary_address directly for both IPv4 and IPv6 without validation

The OSPF implementation correctly handles the case where the interface address might not be set. The PIM implementation assumes primary_address is always valid when neighbors exist, based on architectural guarantees that neighbors are only created after receiving hello messages on an interface with a configured address.

Confidence Score: 4/5

Safe to merge with minor review note - OSPF implementation is robust, PIM relies on architectural guarantee
OSPF properly guards against null addresses with a ternary check. PIM passes primary_address without validation, but according to the codebase architecture, neighbors are only created after hello messages on interfaces with valid addresses. The existing thread discussions confirm that the senior developers believe the address will always be valid at neighbor creation time. Score is 4 (not 5) because PIM could be more defensive with an explicit guard.
No files require special attention - the code is architecturally sound

Important Files Changed

Filename	Overview
ospfd/ospf_bfd.c	Added null check for `oi->address` before passing local source to BFD session, preventing potential null dereference crashes
pimd/pim_bfd.c	Changed to pass `primary_address` as local source to BFD for both IPv4 and IPv6, matching BGP behavior but without null/unspecified address guard

Sequence Diagram

sequenceDiagram
    participant OSPF as OSPF Neighbor
    participant PIM as PIM Neighbor
    participant BFD as BFD Session
    participant BFDD as BFD Daemon
    
    Note over OSPF,BFDD: OSPF BFD Session Creation
    OSPF->>BFD: ospf_neighbor_bfd_apply()
    OSPF->>BFD: bfd_sess_new()
    OSPF->>BFD: bfd_sess_set_ipv4_addrs(oi->address ? &oi->address->u.prefix4 : NULL, &nbr->src)
    Note right of OSPF: Now passes local source<br/>with null check
    OSPF->>BFD: bfd_sess_set_interface()
    OSPF->>BFD: bfd_sess_set_vrf()
    OSPF->>BFD: bfd_sess_set_timers()
    OSPF->>BFD: bfd_sess_install()
    BFD->>BFDD: Register session with local & remote addresses
    
    Note over PIM,BFDD: PIM BFD Session Creation
    PIM->>BFD: pim_bfd_info_nbr_create()
    PIM->>BFD: bfd_sess_new()
    PIM->>BFD: bfd_sess_set_timers()
    PIM->>BFD: bfd_sess_set_ipv4_addrs(&pim_ifp->primary_address, &neigh->source_addr)
    Note right of PIM: Now passes primary_address<br/>as local source
    PIM->>BFD: bfd_sess_set_interface()
    PIM->>BFD: bfd_sess_set_vrf()
    PIM->>BFD: bfd_sess_install()
    BFD->>BFDD: Register session with local & remote addresses

greptile-apps

_{2 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-10T04:22:53Z

 		pim_ifp->bfd_config.min_rx, pim_ifp->bfd_config.min_tx);
+	/* Pass local interface primary address as source (like BGP does with su_local) */
 #if PIM_IPV == 4
-	bfd_sess_set_ipv4_addrs(neigh->bfd_session, NULL, &neigh->source_addr);
+	bfd_sess_set_ipv4_addrs(neigh->bfd_session,
+				&pim_ifp->primary_address,  /* local source */
+				&neigh->source_addr);        /* remote dest */
 #else
-	bfd_sess_set_ipv6_addrs(neigh->bfd_session, NULL, &neigh->source_addr);
+	bfd_sess_set_ipv6_addrs(neigh->bfd_session,
+				&pim_ifp->primary_address,  /* local source */
+				&neigh->source_addr);        /* remote dest */
 #endif


Source may be 0/unspecified

This change always passes &pim_ifp->primary_address as the BFD local source (pimd/pim_bfd.c:85-91). primary_address is allowed to be PIMADDR_ANY/unspecified during interface bring-up or address loss (pimd/pim_iface.c:316-334), and the code here doesn't guard against that. That can result in creating a session with an invalid explicit source (where previously NULL would allow BFD to auto-pick a source), or hit assertions/installation failures downstream depending on bfdd behavior.

Prompt To Fix With AI

This is a comment left during a code review. Path: pimd/pim_bfd.c Line: 82:92 Comment: **Source may be 0/unspecified** This change always passes `&pim_ifp->primary_address` as the BFD local source (`pimd/pim_bfd.c:85-91`). `primary_address` is allowed to be `PIMADDR_ANY`/unspecified during interface bring-up or address loss (`pimd/pim_iface.c:316-334`), and the code here doesn't guard against that. That can result in creating a session with an invalid explicit source (where previously `NULL` would allow BFD to auto-pick a source), or hit assertions/installation failures downstream depending on bfdd behavior. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-10T04:22:55Z

Additional Comments (1)

pimd/pim_bfd.c
BFD session not updated

pim_bfd_info_nbr_create() updates timers, addrs, interface, vrf, and profile, then calls bfd_sess_install() unconditionally (pimd/pim_bfd.c:80-97). If the interface primary address changes, bfd_sess_set_ipv[4|6]_addrs() will internally uninstall the session (per lib/bfd.h:99-116 and lib/bfd.c:581-613), so reinstalling is correct — but this function is only called from pim_bfd_reg_dereg_all_nbr() which appears to run on enable/disable, not on address-change events. As a result, PIM's BFD sessions may keep using the old local source until BFD is toggled or neighbors are re-registered.

If address-change hooks already call pim_bfd_info_nbr_create() elsewhere, please point to that callsite; otherwise this needs wiring so sessions update when pim_ifp->primary_address changes.

Prompt To Fix With AI

This is a comment left during a code review.
Path: pimd/pim_bfd.c
Line: 77:97

Comment:
**BFD session not updated**

`pim_bfd_info_nbr_create()` updates timers, addrs, interface, vrf, and profile, then calls `bfd_sess_install()` unconditionally (`pimd/pim_bfd.c:80-97`). If the interface primary address changes, `bfd_sess_set_ipv[4|6]_addrs()` will internally uninstall the session (per `lib/bfd.h:99-116` and `lib/bfd.c:581-613`), so reinstalling is correct — but this function is only called from `pim_bfd_reg_dereg_all_nbr()` which appears to run on enable/disable, not on address-change events. As a result, PIM's BFD sessions may keep using the old local source until BFD is toggled or neighbors are re-registered.

If address-change hooks already call `pim_bfd_info_nbr_create()` elsewhere, please point to that callsite; otherwise this needs wiring so sessions update when `pim_ifp->primary_address` changes.

How can I resolve this? If you propose a fix, please make it concise.

…stered by pim Issue: While using BFD distributed mode to offload the sessions PIM is not sending out the local address. Fix: Fill the source address in bfd structure passed for neighbor creation Ticket: #4769364 Signed-off-by: Utkarsh Srivastava <usrivastava@nvidia.com>

…ed by OSPF. Issue: While using BFD distributed mode to offload the sessions OSPF is not sending out the local address. Fix: Fill the source address in bfd structure passed for neighbor creation Ticket: #4769347 Signed-off-by: Utkarsh Srivastava <usrivastava@nvidia.com>

Jafaral · 2026-02-10T19:11:11Z

@greptile review

Jafaral · 2026-02-10T22:37:08Z

@greptile review

greptile-apps

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

riw777

code looks okay to me ... the AI comments might need to be addressed, though (?)

usrivastava-nvidia · 2026-02-12T04:34:12Z

@greptile review

Jafaral · 2026-02-13T16:36:02Z

@greptile review

I don't see any changes since the last review, so this request to review was skipped.

greptile-apps · 2026-02-13T16:36:45Z

I don't see any changes since the last review either. If you've made updates to address previous comments, please push the new commits to the PR branch. Once there are new changes, I'll be able to provide a fresh review.

If you believe there are changes that aren't being detected, you might want to check:

The commits have been pushed to the remote branch
The PR branch is up to date

riw777

looks good ... will wait on @Jafaral to review

Jafaral · 2026-02-14T17:06:06Z

@Mergifyio backport dev/10.6

mergify · 2026-02-14T17:06:10Z

backport dev/10.6

✅ Backports have been created

Details

#20848 pimd,ospfd: Passing local source address as part of BFD session creation (backport #20739) has been created for branch dev/10.6

pimd,ospfd: Passing local source address as part of BFD session creation (backport #20739)

frrbot Bot added ospf pim labels Feb 10, 2026

github-actions Bot added master size/S labels Feb 10, 2026

greptile-apps Bot reviewed Feb 10, 2026

View reviewed changes

usrivastava-nvidia force-pushed the master branch from 1f87bbe to 2f220de Compare February 10, 2026 07:14

usrivastava-nvidia force-pushed the master branch from 2f220de to 2b4604b Compare February 10, 2026 07:19

usrivastava-nvidia force-pushed the master branch from 2b4604b to 0df9faa Compare February 10, 2026 07:22

Jafaral self-requested a review February 10, 2026 16:33

greptile-apps Bot reviewed Feb 10, 2026

View reviewed changes

Comment thread pimd/pim_bfd.c

riw777 reviewed Feb 11, 2026

View reviewed changes

Comment thread pimd/pim_bfd.c

riw777 approved these changes Feb 14, 2026

View reviewed changes

Jafaral merged commit 7057873 into FRRouting:master Feb 14, 2026
20 checks passed

mergify Bot mentioned this pull request Feb 14, 2026

pimd,ospfd: Passing local source address as part of BFD session creation (backport #20739) #20848

Merged

github-actions Bot added the backport label Feb 14, 2026

donaldsharp added a commit that referenced this pull request Feb 14, 2026

Merge pull request #20848 from FRRouting/mergify/bp/dev/10.6/pr-20739

5fc6409

pimd,ospfd: Passing local source address as part of BFD session creation (backport #20739)

Uh oh!

Conversation

usrivastava-nvidia commented Feb 10, 2026

Uh oh!

greptile-apps Bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps Bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Feb 10, 2026

Uh oh!

Jafaral commented Feb 10, 2026

Uh oh!

Jafaral commented Feb 10, 2026

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

riw777 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

usrivastava-nvidia commented Feb 12, 2026

Uh oh!

Jafaral commented Feb 13, 2026

Uh oh!

greptile-apps Bot commented Feb 13, 2026

Uh oh!

riw777 left a comment

Choose a reason for hiding this comment

Uh oh!

Jafaral commented Feb 14, 2026

Uh oh!

mergify Bot commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

greptile-apps Bot commented Feb 10, 2026 •

edited

Loading

mergify Bot commented Feb 14, 2026 •

edited

Loading