Commit f1d2cd403e for frr
commit f1d2cd403e03c8f2b682570c761e1ffdfc1e079f
Author: Sougata Barik <sougatab@nvidia.com>
Date: Wed Oct 14 15:39:19 2026 +0530
bgpd: register single-hop BFD sessions already keyed
Problem:
During a graceful restart of a router with many single-hop BFD peers,
the neighbors reset the BGP sessions with Cease/BFD Down while the
router was still restarting, adding collisions and churn to the GR
window.
RCA:
bgpd registered single-hop BFD sessions at config time without an
egress interface (and, without update-source, without a local address)
and only filled them in from bgp_peer_bfd_update_source() once the TCP
connection set p->nexthop.ifp / su_local. bfdd keys single-hop sessions
on interface and local address, so lib/bfd.c deregistered the session
(possibly already Up) and registered a new one with a new discriminator.
The remote saw BFD go Down and tore BGP down.
Fix:
- Resolve the egress interface of single-hop peers from configuration
and connected routes only, so it is the same before and after TCP
connects: conf_if/ifname then update-source interface for link-local
peers, update-source interface, neighbor interface, then the connected
subnet of the peer otherwise. p->nexthop.ifp, which follows the TCP
local address, is only a fallback when nothing else resolves (an
update-source address on another interface, e.g. a loopback, must not
move the session off the link at connect).
- Take the local address only from what the connection is bound to
(update-source address/interface, then su_local), so the key cannot
change at connect.
- Hold back the install of a single-hop session until interface and local
address are known; re-drive it from bgp_peer_bfd_update_source()
(connected address add, TCP connect). Install as before once the
connection is up.
- Strict mode (effective flag, so also inherited from a peer-group) needs
the session before BGP can connect: install it at once, keyed with the
connected address closest to the peer if possible (best effort, never a
link-local source for a non-link-local peer). Setting
'neighbor bfd strict' on a peer or a peer-group re-evaluates held
installs.
- Multihop sessions are unchanged.
Ticket: #5324193
Signed-off-by: Sougata Barik <sougatab@nvidia.com>
diff --git a/bgpd/bgp_bfd.c b/bgpd/bgp_bfd.c
index 88d3e20df9..a4601ceb68 100644
--- a/bgpd/bgp_bfd.c
+++ b/bgpd/bgp_bfd.c
@@ -27,6 +27,7 @@
#include "bgpd/bgp_vty.h"
#include "bgpd/bgp_packet.h"
#include "bgpd/bgp_network.h"
+#include "bgpd/bgp_zebra.h"
#ifndef VTYSH_EXTRACT_PL
#include "bgpd/bgp_bfd_clippy.c"
#endif
@@ -182,6 +183,86 @@ static void bfd_session_status_update(struct bfd_session_params *bsp,
}
}
+/*
+ * Egress interface of a single-hop peer. It must come out the same before
+ * and after TCP connects, or the session is re-keyed at connect: so it only
+ * depends on configuration and connected routes, and p->nexthop.ifp (which
+ * follows the TCP local address) is used only when nothing else resolves.
+ */
+static struct interface *bgp_peer_bfd_egress_ifp(struct peer *p)
+{
+ union sockunion *su = &p->connection->su;
+ vrf_id_t vrf_id = p->bgp->vrf_id;
+ struct interface *ifp = NULL;
+ const char *ifname = p->conf_if ? p->conf_if : p->ifname;
+
+ /* Link-local (or not yet learnt) peer: the local address is link-local. */
+ if ((su->sa.sa_family == AF_INET6 && IN6_IS_ADDR_LINKLOCAL(&su->sin6.sin6_addr)) ||
+ (su->sa.sa_family != AF_INET && su->sa.sa_family != AF_INET6)) {
+ if (ifname)
+ ifp = if_lookup_by_name(ifname, vrf_id);
+ else if (p->update_if)
+ ifp = if_lookup_by_name(p->update_if, vrf_id);
+ } else if (p->update_if)
+ ifp = if_lookup_by_name(p->update_if, vrf_id);
+ else if (ifname)
+ ifp = if_lookup_by_name(ifname, vrf_id);
+ else if (su->sa.sa_family == AF_INET)
+ ifp = if_lookup_by_ipv4(&su->sin.sin_addr, vrf_id);
+ else
+ ifp = if_lookup_by_ipv6(&su->sin6.sin6_addr, 0, vrf_id);
+
+ return ifp ? ifp : p->nexthop.ifp;
+}
+
+static bool bgp_peer_bfd_keyed(struct bfd_session_params *session)
+{
+ int family;
+ union {
+ struct in_addr v4;
+ struct in6_addr v6;
+ } src, dst;
+
+ if (!bfd_sess_interface(session))
+ return false;
+
+ bfd_sess_addresses(session, &family, &src.v6, &dst.v6);
+ if (family == AF_INET)
+ return src.v4.s_addr != INADDR_ANY;
+
+ return !IN6_IS_ADDR_UNSPECIFIED(&src.v6);
+}
+
+/*
+ * bfdd keys single-hop sessions on the interface and local address, so
+ * registering before either is known and filling them in at TCP connect
+ * deletes the (possibly already Up) session and creates a new one; the
+ * remote sees BFD Down and resets BGP. Hold a single-hop install back until
+ * both are known, except when nothing better can be learnt any more
+ * (connection already up) or BGP itself waits for BFD (strict mode).
+ */
+static void bgp_peer_bfd_install(struct peer *p)
+{
+ struct bfd_session_params *session = p->bfd_config->session;
+
+ if (!PEER_IS_MULTIHOP(p) && !bgp_peer_bfd_keyed(session) && !p->connection->su_local &&
+ !CHECK_FLAG(p->flags, PEER_FLAG_BFD_STRICT)) {
+ if (BGP_DEBUG(bfd, BFD_LIB) && !p->bfd_config->install_pending)
+ zlog_debug("%s: deferring BFD session for peer %pBP vrf %s(%u) until interface and source are known",
+ __func__, p, p->bgp->name_pretty, p->bgp->vrf_id);
+ p->bfd_config->install_pending = true;
+ return;
+ }
+
+ if (BGP_DEBUG(bfd, BFD_LIB))
+ zlog_debug("%s: registering BFD session for peer %pBP vrf %s(%u)", __func__, p,
+ p->bgp->name_pretty, p->bgp->vrf_id);
+ p->bfd_config->install_pending = false;
+ bfd_sess_install(session);
+}
+
+static bool bgp_peer_bfd_sync(struct peer *p);
+
void bgp_peer_config_apply(struct peer *p, struct peer_group *pg)
{
struct listnode *n;
@@ -209,7 +290,9 @@ void bgp_peer_config_apply(struct peer *p, struct peer_group *pg)
bfd_sess_set_cbit(p->bfd_config->session, p->bfd_config->cbit);
bfd_sess_set_profile(p->bfd_config->session,
p->bfd_config->profile);
- bfd_sess_install(p->bfd_config->session);
+ if (!PEER_IS_MULTIHOP(p))
+ bgp_peer_bfd_sync(p);
+ bgp_peer_bfd_install(p);
return;
}
@@ -219,9 +302,9 @@ void bgp_peer_config_apply(struct peer *p, struct peer_group *pg)
*/
gconfig = pg->conf;
- if (CHECK_FLAG(gconfig->flags, PEER_FLAG_UPDATE_SOURCE) ||
+ if (!PEER_IS_MULTIHOP(p) || CHECK_FLAG(gconfig->flags, PEER_FLAG_UPDATE_SOURCE) ||
CHECK_FLAG(p->flags_override, PEER_FLAG_UPDATE_SOURCE))
- bgp_peer_bfd_update_source(p);
+ bgp_peer_bfd_sync(p);
/*
* If using default control plane independent configuration,
@@ -259,12 +342,16 @@ void bgp_peer_config_apply(struct peer *p, struct peer_group *pg)
p->bfd_config->min_rx,
p->bfd_config->min_tx);
- bfd_sess_install(p->bfd_config->session);
+ bgp_peer_bfd_install(p);
}
-void bgp_peer_bfd_update_source(struct peer *p)
+/*
+ * Bring the session parameters in line with the peer; returns true if
+ * anything changed and the session needs to be (re-)installed.
+ */
+static bool bgp_peer_bfd_sync(struct peer *p)
{
- struct bfd_session_params *session;
+ struct bfd_session_params *session = p->bfd_config->session;
const union sockunion *source = NULL;
bool changed = false;
int family;
@@ -273,16 +360,9 @@ void bgp_peer_bfd_update_source(struct peer *p)
struct in6_addr v6;
} src, dst;
struct interface *ifp;
+ struct interface *egress_ifp;
union sockunion addr;
- if (!p->bfd_config)
- return;
-
- session = p->bfd_config->session;
- /* Nothing to do for groups. */
- if (CHECK_FLAG(p->sflags, PEER_STATUS_GROUP))
- return;
-
/* Figure out the correct source to use. */
if (CHECK_FLAG(p->flags, PEER_FLAG_UPDATE_SOURCE)) {
if (p->update_source) {
@@ -304,6 +384,22 @@ void bgp_peer_bfd_update_source(struct peer *p)
source = p->connection->su_local;
}
+ egress_ifp = PEER_IS_MULTIHOP(p) ? p->nexthop.ifp : bgp_peer_bfd_egress_ifp(p);
+
+ /*
+ * Without update-source the kernel picks the TCP source, so a guess can
+ * differ and rekey the session at connect: wait for su_local instead.
+ * Strict mode must install before TCP is up, so guess there.
+ */
+ if (!source && !PEER_IS_MULTIHOP(p) && egress_ifp &&
+ CHECK_FLAG(p->flags, PEER_FLAG_BFD_STRICT)) {
+ sockunion_init(&addr);
+ if (!bgp_update_address(egress_ifp, &p->connection->su, &addr) &&
+ (addr.sa.sa_family != AF_INET6 || !IN6_IS_ADDR_LINKLOCAL(&addr.sin6.sin6_addr) ||
+ IN6_IS_ADDR_LINKLOCAL(&p->connection->su.sin6.sin6_addr)))
+ source = &addr;
+ }
+
/* Update peer's source/destination addresses. */
bfd_sess_addresses(session, &family, &src.v6, &dst.v6);
if (family == AF_INET) {
@@ -341,16 +437,15 @@ void bgp_peer_bfd_update_source(struct peer *p)
}
/* Update interface. */
- const char *ifname = bfd_sess_interface(session);
-
- if (p->nexthop.ifp &&
- (!ifname || !strmatch(ifname, p->nexthop.ifp->name))) {
+ if (egress_ifp && (!bfd_sess_interface(session) ||
+ !strmatch(bfd_sess_interface(session), egress_ifp->name))) {
if (BGP_DEBUG(bfd, BFD_LIB))
zlog_debug("%s: interface %s to %s", __func__,
- ifname ? ifname : "none",
- p->nexthop.ifp->name);
+ bfd_sess_interface(session) ? bfd_sess_interface(session)
+ : "none",
+ egress_ifp->name);
- bfd_sess_set_interface(session, p->nexthop.ifp->name);
+ bfd_sess_set_interface(session, egress_ifp->name);
changed = true;
}
@@ -391,8 +486,20 @@ void bgp_peer_bfd_update_source(struct peer *p)
changed = true;
}
- if (changed)
- bfd_sess_install(session);
+ return changed;
+}
+
+void bgp_peer_bfd_update_source(struct peer *p)
+{
+ if (!p->bfd_config)
+ return;
+
+ /* Nothing to do for groups. */
+ if (CHECK_FLAG(p->sflags, PEER_STATUS_GROUP))
+ return;
+
+ if (bgp_peer_bfd_sync(p) || p->bfd_config->install_pending)
+ bgp_peer_bfd_install(p);
}
/**
@@ -698,16 +805,29 @@ DEFPY (neighbor_bfd_strict,
"BFD support\n"
"Strict mode\n")
{
- struct peer *peer;
+ struct peer *peer, *pn;
+ struct listnode *n;
+ int ret;
peer = peer_and_group_lookup_vty(vty, neighbor);
if (!peer)
return CMD_WARNING_CONFIG_FAILED;
if (no)
- return peer_flag_unset(peer, PEER_FLAG_BFD_STRICT);
+ ret = peer_flag_unset(peer, PEER_FLAG_BFD_STRICT);
+ else
+ ret = peer_flag_set(peer, PEER_FLAG_BFD_STRICT);
+
+ /* Strict mode needs the session installed for BGP to come up. */
+ if (!no) {
+ if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) {
+ for (ALL_LIST_ELEMENTS_RO(peer->group->peer, n, pn))
+ bgp_peer_bfd_update_source(pn);
+ } else
+ bgp_peer_bfd_update_source(peer);
+ }
- return peer_flag_set(peer, PEER_FLAG_BFD_STRICT);
+ return ret;
}
DEFPY (neighbor_bfd_strict_hold_time,
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 543a698f2d..6df6910bde 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -2307,6 +2307,8 @@ struct peer {
* BFD session does not enter the Up state.
*/
struct event *t_hold_timer;
+ /* Single-hop install held back until the egress interface is known. */
+ bool install_pending;
} * bfd_config;
/* hostname and domainname advertised by host */