bgpd: Fix GR helper retaining stale routes after Hard Reset

[selva-nexthop]

Problem:
When a BGP peer sends a Hard Reset (Cease/subcode 9, per RFC 8538), the GR helper side must flush peer routes immediately rather than retaining them as stale. Four code gaps allow the NSF_WAIT flag to be set (or the hard_reset flag to be missed) after a Hard Reset, causing stale routes to remain in the RIB:

1. bgp_packet.c (bgp_notify_receive): peer->notify.hard_reset is only set when the received Hard Reset has a non-empty encapsulated payload (outer.length > 0). A valid bare Hard Reset (Cease/subcode 9 with no inner notification) leaves peer->notify.hard_reset = false. This bypasses the existing !hard_reset guard for the TCP error handler and doppelganger handler paths below.

2. bgpd.c (bgp_process_conn_error / TCP error handler): When the TCP socket reports an error or close, the handler unconditionally sets NSF_WAIT if NSF_MODE is set. If the peer sent a Hard Reset and the TCP stack delivered the RST before the NOTIFICATION was read from the socket, peer->notify.hard_reset is already true but is ignored here, causing GR helper mode to be entered and routes kept stale.

3. bgp_network.c (bgp_accept / doppelganger handler): Same issue -- when an incoming TCP collision tears down the existing established session, NSF_WAIT is set without checking peer->notify.hard_reset. A peer that sends Hard Reset and immediately reconnects can trigger this path before the NOTIFICATION event is processed by the FSM.

4. bgp_fsm.c (bgp_establish): peer->notify.hard_reset is NOT cleared when a new BGP session is established. After a Hard Reset the peer reconnects and establishes a new session. If that new session later drops normally (TCP drop, no notification), the TCP error handler sees the stale hard_reset=true from the previous session and skips NSF_WAIT, permanently defeating GR for that neighbor until a restart.

Note: bgp_packet.c already has correct !hard_reset guards for the NOTIFICATION send path (~line 1058) and receive path (~line 2704). The bgp_packet.c fix here closes the gap that lets those guards be bypassed.

FRR upstream master and stable/10.4 have the same gaps as of April 2026. The bgp_packet.c send/recv path fix corresponds to upstream PR #18498 (merged to master 2026-03-26, not backported to stable/10.4).

Solution:

• bgp_packet.c: Set peer->notify.hard_reset = true whenever hard_reset is true, independently of whether outer.length > 0.
• bgpd.c: Add !peer->notify.hard_reset check in the TCP error handler before setting NSF_WAIT.
• bgp_network.c: Add !peer->notify.hard_reset check in the doppelganger handler before setting NSF_WAIT.
• bgp_fsm.c: Clear peer->notify.hard_reset in bgp_establish() alongside PEER_STATUS_NSF_WAIT to prevent the flag from leaking across sessions.

Topotest:

• TC3: Add test_bgp_hard_reset_gr() -- Hard Reset with delayopen=60 (both sides). Exercises bgp_process_conn_error (bgpd.c fix). Verifies R1 does not retain stale routes after receiving a Hard Reset.
• TC4: Add test_bgp_hard_reset_gr_doppelganger() -- Hard Reset with delayopen=5 on R1 only, no delayopen on R2. R2 reconnects immediately after Hard Reset, maximising the probability that R2's new TCP SYN reaches R1's listen socket while R1 is still in Established state, triggering the bgp_accept() doppelganger handler (bgp_network.c fix). R1's delayopen=5 holds the new session in OpenWait for ~5 s providing a stable observation window. Verifies routes are flushed in either code path (TCP error handler or doppelganger).

Resolves: #21822

[greptile-apps]

Greptile Summary

This PR fixes four related gaps that allowed peer->notify.hard_reset to be missed or stale, causing the GR helper to incorrectly retain routes as stale after receiving an RFC 8538 Hard Reset (Cease/subcode 9).

• bgp_packet.c: peer->notify.hard_reset = true is now set unconditionally when the received notification is a Hard Reset, regardless of payload length; inner = outer is pre-initialised before the if (hard_reset) block, replacing the three-branch if/else with a simpler two-branch form.
• bgpd.c / bgp_network.c: Both the TCP error handler (bgp_process_conn_error) and the doppelganger handler (bgp_accept) now guard the SET_FLAG(peer->sflags, PEER_STATUS_NSF_WAIT) call with !peer->notify.hard_reset, so a Hard Reset received before the TCP close is no longer overridden.
• bgp_fsm.c: peer->notify.hard_reset is cleared in bgp_peer_process_gr_cap_clear_stale (called from bgp_establish) alongside PEER_STATUS_NSF_WAIT, preventing the flag from leaking into future sessions.
• topotests: TC3 exercises the TCP error handler path with delayopen=60; TC4 exercises the doppelganger path with delayopen=5 on R1 only and no delay on R2 to maximise the reconnect-race window.

Confidence Score: 5/5

Safe to merge — all four code paths are addressed with minimal, targeted one-liner guards; no existing GR behaviour is changed for normal (non-Hard-Reset) sessions.

Each of the four gaps described in the PR is directly closed by the corresponding change. Memory management in bgp_packet.c is unaffected (outer.raw_data is freed unconditionally at line 2689 regardless of whether inner and outer share the pointer). The bgp_fsm.c clear is placed in bgp_peer_process_gr_cap_clear_stale, which is the single site already responsible for resetting NSF state on session establishment. The new topotests cover both the TCP-error-handler and doppelganger code paths.

No files require special attention.

Important Files Changed

Filename	Overview
bgpd/bgp_packet.c	Refactors bgp_notify_receive to set peer->notify.hard_reset unconditionally when hard_reset is true and pre-initialises inner=outer before the if block, eliminating the code path where a bare Hard Reset (no payload) left hard_reset unset.
bgpd/bgpd.c	Adds !peer->notify.hard_reset guard to bgp_process_conn_error before setting NSF_WAIT, preventing stale routes from being retained when TCP closes after a Hard Reset.
bgpd/bgp_network.c	Adds !peer->notify.hard_reset guard to the doppelganger handler in bgp_accept, preventing NSF_WAIT from being set when a peer sends Hard Reset and immediately reconnects.
bgpd/bgp_fsm.c	Clears peer->notify.hard_reset in bgp_peer_process_gr_cap_clear_stale (called from bgp_establish) alongside PEER_STATUS_NSF_WAIT to prevent the flag from leaking across sessions.
tests/topotests/bgp_gr_notification/test_bgp_gr_notification.py	Adds TC3 (bgp_process_conn_error path) and TC4 (doppelganger path) topotests for Hard Reset GR handling; both verify routes are flushed and not retained as stale after a Hard Reset.

Sequence Diagram

sequenceDiagram
    participant R2
    participant R1_recv as R1 bgp_notify_receive
    participant R1_tcp as R1 TCP handler
    participant R1_dp as R1 bgp_accept (doppelganger)
    participant R1_fsm as R1 bgp_establish

    R2->>R1_recv: NOTIFICATION Cease/Hard-Reset (subcode 9)
    Note over R1_recv: peer->notify.hard_reset = true (unconditional, Fix 1)
    R2->>R1_tcp: TCP RST / close
    alt TCP error handler path (Fix 2)
        R1_tcp->>R1_tcp: bgp_process_conn_error
        Note over R1_tcp: !peer->notify.hard_reset -> skip NSF_WAIT
        R1_tcp->>R1_tcp: Flush stale routes
    else Doppelganger path (Fix 3)
        R2->>R1_dp: New TCP SYN (fast reconnect)
        Note over R1_dp: bgp_accept: peer still Established+NSF_MODE
        Note over R1_dp: !peer->notify.hard_reset -> skip NSF_WAIT
        R1_dp->>R1_tcp: "TCP_connection_closed event -> flush routes"
    end
    R2->>R1_fsm: New BGP session establishes
    Note over R1_fsm: bgp_peer_process_gr_cap_clear_stale (Fix 4)
    Note over R1_fsm: peer->notify.hard_reset = false (cleared for next session)

Loading

_{Reviews (2): Last reviewed commit: "bgpd: Fix GR helper retaining stale rout..." | Re-trigger Greptile}

[ton31337]

That's the completely wrong place to set peer->notify foobar... It should be either at the reception or when we send it...

[selva-nexthop]

Thanks @ton31337 for your comment.

The race condition this fix is handling like below;

1. Hard Reset NOTIFICATION received → bgp_notify_receive → peer->notify.hard_reset = true --> BGP_Stop FSM event
2. FSM --> bgp_stop() - session tearing down
3. TCP RST arrives late --> bgp_process_conn_error fires --> must see hard_reset = true to skip NSF_WAIT

If we do clear it "at reception" (i.e., in bgp_notify_receive or bgp_stop), step 3 would see hard_reset = false — NSF_WAIT gets set — routes retained as stale. That's the exact bug we're fixing.

[ton31337]

Why we do need to keep peer->notify at all once the session is established?

[riw777]

looks good

[riw777]

@greptile review

[riw777]

lints need to be fixed.

[selva-nexthop]

@ton31337 could you do the needful to merge?