pimd: retry MSDP listener bind after VRF master interface up

[hnattamaisub]

Issue:
When pimd parses an "ip msdp peer X source Y" inside a VRF block at config time, libvrf has not yet activated the VRF and pim->vrf is a name-only stub with vrf_id == VRF_UNKNOWN. For passive (listener-side) peers, pim_msdp_peer_listen() immediately calls pim_msdp_sock_listen(), whose if_lookup_by_name(vrf->name, VRF_UNKNOWN) deterministically returns NULL. The function logs "Unable to lookup vrf interface" and returns -1. No code path ever retries it

The race is timing-dependent. Some startup orderings happen to populate pim->vrf->vrf_id before MSDP peer config is processed and the listener binds normally.

Fix:
1)Adding a small pim_msdp_vrf_iface_up() helper that retries pim_msdp_sock_listen() when pim_iface.c sees the VRF-master interface appear (pim_ifp_create / pim_ifp_up).
2)tear down MSDP listener in pim_msdp_exit()

[greptile-apps]

Greptile Summary

This PR fixes a startup race in pimd where MSDP listener peers configured inside a VRF block fail to bind because pim->vrf->vrf_id is VRF_UNKNOWN at config-parse time, and no code path previously retried the bind. It also adds an explicit listener teardown in pim_msdp_exit() to prevent fd and libevent-watcher leaks on VRF delete.

• New pim_msdp_vrf_iface_up() helper (pim_msdp.c): called from pim_ifp_create and pim_ifp_up when the VRF master interface appears; iterates peer_list for listener peers and retries pim_msdp_sock_listen(). The retry correctly handles the non-auth case but unconditionally calls the global (non-auth) socket path even for auth-configured peers, silently skipping the per-peer pim_msdp_sock_auth_listen() call they require.
• Listener teardown in pim_msdp_exit() (pim_msdp.c): cancels the libevent watcher and closes the fd before freeing peer sessions, fixing a real resource leak. This part is straightforward and correct.
• pim_iface.c hook points: the VRF master interface detection (name equals VRF name, non-default VRF) is used consistently in both pim_ifp_create and pim_ifp_up; idempotency is guaranteed by the PIM_MSDPF_LISTENER flag checked inside the helper.

Confidence Score: 3/5

Safe to merge for the common non-auth VRF MSDP case, but auth-configured MSDP peers in VRFs will remain broken after this change

The retry helper unconditionally calls the global non-auth socket path for every listener peer it finds — including auth-configured ones. For an auth peer this creates the wrong shared listener, sets PIM_MSDPF_LISTENER, and permanently blocks the per-peer pim_msdp_sock_auth_listen() path from ever being retried. Operators combining MSDP peer authentication with VRF isolation will see the same silent bind failure the PR set out to fix.

pimd/pim_msdp.c — specifically the pim_msdp_vrf_iface_up() function where the auth/non-auth branch is missing

Important Files Changed

Filename	Overview
pimd/pim_msdp.c	Adds pim_msdp_vrf_iface_up() retry helper and listener teardown in pim_msdp_exit(); retry logic handles only non-auth listener peers, leaving auth-configured VRF peers broken
pimd/pim_iface.c	Calls pim_msdp_vrf_iface_up() in both pim_ifp_create and pim_ifp_up when the VRF master interface is detected; correctly guarded by VRF_DEFAULT check and name comparison, idempotency handled downstream by PIM_MSDPF_LISTENER flag
pimd/pim_msdp.h	Adds pim_msdp_vrf_iface_up() declaration, guarded by PIM_IPV==4 at call sites; minimal, correct change

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Conf as Config Parser
    participant MSDP as pim_msdp.c
    participant Iface as pim_iface.c
    participant Sock as pim_msdp_socket.c

    Conf->>MSDP: ip msdp peer X source Y (in VRF)
    MSDP->>MSDP: pim_msdp_peer_add() → pim_msdp_peer_listen()
    MSDP->>Sock: "pim_msdp_sock_listen(pim) [vrf_id==VRF_UNKNOWN]"
    Sock->>Sock: "if_lookup_by_name(vrf->name, VRF_UNKNOWN) → NULL"
    Sock-->>MSDP: return -1 (bind fails, no listener)

    Note over Conf,Sock: Later: libvrf activates VRF

    Iface->>Iface: pim_ifp_create(vrf_master_ifp) or pim_ifp_up(vrf_master_ifp)
    Iface->>MSDP: pim_msdp_vrf_iface_up(pim)
    MSDP->>MSDP: check PIM_MSDPF_LISTENER (not set yet)
    MSDP->>MSDP: find first listener peer in peer_list
    alt "auth_type == MSDP_AUTH_NONE"
        MSDP->>Sock: pim_msdp_sock_listen(pim) [vrf_id now valid]
        Sock->>Sock: if_lookup_by_name succeeds, vrf_bind + bind + event_add_read
        Sock-->>MSDP: fd (sets PIM_MSDPF_LISTENER)
    else "auth_type != MSDP_AUTH_NONE (bug: not handled)"
        MSDP->>Sock: pim_msdp_sock_listen(pim) [wrong path]
        Note over MSDP,Sock: Should call pim_msdp_sock_auth_listen(mp)
    end

    Note over Conf,Sock: On VRF delete

    Iface->>MSDP: pim_msdp_exit(pim)
    MSDP->>MSDP: event_cancel + close(listener.fd) + clear flag
    MSDP->>MSDP: pim_msdp_mg_free() → free all peers

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Conf as Config Parser
    participant MSDP as pim_msdp.c
    participant Iface as pim_iface.c
    participant Sock as pim_msdp_socket.c

    Conf->>MSDP: ip msdp peer X source Y (in VRF)
    MSDP->>MSDP: pim_msdp_peer_add() → pim_msdp_peer_listen()
    MSDP->>Sock: "pim_msdp_sock_listen(pim) [vrf_id==VRF_UNKNOWN]"
    Sock->>Sock: "if_lookup_by_name(vrf->name, VRF_UNKNOWN) → NULL"
    Sock-->>MSDP: return -1 (bind fails, no listener)

    Note over Conf,Sock: Later: libvrf activates VRF

    Iface->>Iface: pim_ifp_create(vrf_master_ifp) or pim_ifp_up(vrf_master_ifp)
    Iface->>MSDP: pim_msdp_vrf_iface_up(pim)
    MSDP->>MSDP: check PIM_MSDPF_LISTENER (not set yet)
    MSDP->>MSDP: find first listener peer in peer_list
    alt "auth_type == MSDP_AUTH_NONE"
        MSDP->>Sock: pim_msdp_sock_listen(pim) [vrf_id now valid]
        Sock->>Sock: if_lookup_by_name succeeds, vrf_bind + bind + event_add_read
        Sock-->>MSDP: fd (sets PIM_MSDPF_LISTENER)
    else "auth_type != MSDP_AUTH_NONE (bug: not handled)"
        MSDP->>Sock: pim_msdp_sock_listen(pim) [wrong path]
        Note over MSDP,Sock: Should call pim_msdp_sock_auth_listen(mp)
    end

    Note over Conf,Sock: On VRF delete

    Iface->>MSDP: pim_msdp_exit(pim)
    MSDP->>MSDP: event_cancel + close(listener.fd) + clear flag
    MSDP->>MSDP: pim_msdp_mg_free() → free all peers

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
pimd/pim_msdp.c:948-960
**Auth-configured listener peers silently skipped by retry**

`pim_msdp_vrf_iface_up` unconditionally calls `pim_msdp_sock_listen(pim)` — the shared global (non-auth) listener — even when the first `PIM_MSDP_PEER_IS_LISTENER` peer has `auth_type != MSDP_AUTH_NONE`. That call succeeds, sets `PIM_MSDPF_LISTENER`, and returns. The early-return guard at line 945 then silently prevents any future retry for the *per-peer* auth socket that should have been set up via `pim_msdp_sock_auth_listen(mp)`. Auth VRF peers are left permanently without a listening socket after the first VRF master interface-up event.

The original `pim_msdp_peer_listen()` (line 979) already has the correct branch: call `pim_msdp_sock_listen` for `MSDP_AUTH_NONE`, `pim_msdp_sock_auth_listen` otherwise. The retry helper should mirror that logic, and should *not* `break` early for auth peers since each one needs its own per-peer socket.

_{Reviews (1): Last reviewed commit: "pimd: retry MSDP listener bind after VRF..." | Re-trigger Greptile}