diff --git a/bgpd/bgp_ls.c b/bgpd/bgp_ls.c index 7ff82e935a2c..110b3a356a65 100644 --- a/bgpd/bgp_ls.c +++ b/bgpd/bgp_ls.c @@ -729,6 +729,11 @@ bool bgp_ls_register(struct bgp *bgp) return false; } + if (ls_request_sync(bgp_zclient) == -1) + zlog_warn("BGP-LS: Failed to request initial Link State database sync"); + else + zlog_info("BGP-LS: Requested initial Link State database sync"); + bgp->ls_info->registered_ls_db = true; zlog_info("BGP-LS: Registered with Link State database for BGP instance %s", diff --git a/bgpd/bgp_ls_ted.c b/bgpd/bgp_ls_ted.c index b1c77b416a2f..fe94c225c738 100644 --- a/bgpd/bgp_ls_ted.c +++ b/bgpd/bgp_ls_ted.c @@ -20,6 +20,7 @@ #include "bgpd/bgp_ls.h" #include "bgpd/bgp_ls_nlri.h" #include "bgpd/bgp_ls_ted.h" +#include "bgpd/bgp_route.h" /* * =========================================================================== @@ -957,6 +958,7 @@ int bgp_ls_process_vertex(struct bgp *bgp, struct ls_vertex *vertex, uint8_t eve } switch (event) { + case LS_MSG_EVENT_SYNC: case LS_MSG_EVENT_ADD: case LS_MSG_EVENT_UPDATE: return bgp_ls_originate_node(bgp, protocol_id, router_id, router_id_len, area_id, @@ -1030,6 +1032,7 @@ int bgp_ls_process_edge(struct bgp *bgp, struct ls_edge *edge, uint8_t event) } switch (event) { + case LS_MSG_EVENT_SYNC: case LS_MSG_EVENT_ADD: case LS_MSG_EVENT_UPDATE: return bgp_ls_originate_link(bgp, protocol_id, local_router_id, @@ -1092,6 +1095,7 @@ int bgp_ls_process_subnet(struct bgp *bgp, struct ls_subnet *subnet, uint8_t eve } switch (event) { + case LS_MSG_EVENT_SYNC: case LS_MSG_EVENT_ADD: case LS_MSG_EVENT_UPDATE: return bgp_ls_originate_prefix(bgp, protocol_id, router_id, router_id_len, @@ -1168,7 +1172,8 @@ int bgp_ls_process_message(struct bgp *bgp, struct ls_message *msg) if (BGP_DEBUG(zebra, ZEBRA) || BGP_DEBUG(linkstate, LINKSTATE)) zlog_debug("%s: Link edge", __func__); - if (msg->event == LS_MSG_EVENT_ADD || msg->event == LS_MSG_EVENT_UPDATE) { + if (msg->event == LS_MSG_EVENT_SYNC || msg->event == LS_MSG_EVENT_ADD || + msg->event == LS_MSG_EVENT_UPDATE) { /* Search for the reverse edge and link both directions. */ reverse_edge = ls_find_edge_by_destination(bgp->ls_info->ted, edge->attributes); @@ -1212,7 +1217,8 @@ int bgp_ls_process_message(struct bgp *bgp, struct ls_message *msg) * destination. */ if (reverse_edge && - (msg->event == LS_MSG_EVENT_ADD || reverse_edge_dst_updated)) { + (msg->event == LS_MSG_EVENT_SYNC || msg->event == LS_MSG_EVENT_ADD || + reverse_edge_dst_updated)) { uint8_t reverse_event = msg->event; if (msg->event == LS_MSG_EVENT_UPDATE && reverse_edge_dst_updated) @@ -1264,6 +1270,49 @@ int bgp_ls_process_message(struct bgp *bgp, struct ls_message *msg) return 0; } +/* + * Remove all entries from the TED. + */ +static void bgp_ls_ted_clear(struct ls_ted *ted) +{ + struct ls_vertex *vertex; + struct ls_edge *edge; + struct ls_subnet *subnet; + + frr_each_safe (vertices, &ted->vertices, vertex) + ls_vertex_del_all(ted, vertex); + frr_each_safe (edges, &ted->edges, edge) + ls_edge_del_all(ted, edge); + frr_each_safe (subnets, &ted->subnets, subnet) + ls_subnet_del_all(ted, subnet); +} + +/* + * Withdraw all locally originated BGP-LS routes and reset the TED. + * + * Called when the last BGP-LS peer is deactivated: performs a single bulk + * walk of the BGP-LS RIB via bgp_clear_route() to remove all self-originated + * paths at once, then clears all TED entries. + * + * @param bgp - BGP instance + */ +void bgp_ls_withdraw_ted(struct bgp *bgp) +{ + if (!bgp || !bgp->ls_info || !bgp->ls_info->ted) + return; + + if (BGP_DEBUG(linkstate, LINKSTATE)) + zlog_debug("BGP-LS: Withdrawing all locally originated routes and resetting TED"); + + /* Remove all self-originated BGP-LS paths from the RIB */ + bgp_clear_route(bgp->peer_self, AFI_BGP_LS, SAFI_BGP_LS); + + /* Clear all TED entries */ + bgp_ls_ted_clear(bgp->ls_info->ted); + + zlog_info("BGP-LS: All locally originated BGP-LS routes withdrawn and TED reset"); +} + /* * Handle link-state SYNC/UPDATE messages from zebra */ diff --git a/bgpd/bgp_ls_ted.h b/bgpd/bgp_ls_ted.h index 3e60d30cec4d..696df8ffff08 100644 --- a/bgpd/bgp_ls_ted.h +++ b/bgpd/bgp_ls_ted.h @@ -208,4 +208,15 @@ extern int bgp_ls_process_linkstate_message(struct stream *s, uint8_t msg_type); */ extern int bgp_ls_process_message(struct bgp *bgp, struct ls_message *msg); +/* + * Withdraw all locally originated BGP-LS routes and reset the TED. + * + * Called when the last BGP-LS peer is deactivated. Withdraws every node, + * link, and prefix NLRI that was originated from the TED, then discards + * the stale TED. + * + * @param bgp - BGP instance + */ +extern void bgp_ls_withdraw_ted(struct bgp *bgp); + #endif /* _FRR_BGP_LS_TED_H */ diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 0d5615262182..aeee1a13bcd4 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -81,6 +81,7 @@ #include "bgpd/bgp_trace.h" #include "bgpd/bgp_srv6.h" #include "bgpd/bgp_ls.h" +#include "bgpd/bgp_ls_ted.h" DEFINE_MTYPE_STATIC(BGPD, PEER_TX_SHUTDOWN_MSG, "Peer shutdown message (TX)"); DEFINE_QOBJ_TYPE(bgp_master); @@ -2850,6 +2851,8 @@ int peer_deactivate(struct peer *peer, afi_t afi, safi_t safi) } if (last_peer) { + bgp_ls_withdraw_ted(bgp); + if (!bgp_ls_unregister(bgp)) { zlog_err("BGP-LS: Failed to unregister from link-state database for instance %s", bgp->name_pretty); diff --git a/tests/topotests/bgp_link_state/test_bgp_link_state.py b/tests/topotests/bgp_link_state/test_bgp_link_state.py index 541875c3172d..f646134ba96b 100644 --- a/tests/topotests/bgp_link_state/test_bgp_link_state.py +++ b/tests/topotests/bgp_link_state/test_bgp_link_state.py @@ -213,6 +213,22 @@ def get_bgp_ls_count(router): return len(data) +def check_bgp_ls_empty(router): + """ + Check that the BGP-LS routing table is empty. + + Args: + router: Router instance + + Returns: + None if table is empty, error message otherwise + """ + count = get_bgp_ls_count(router) + if count > 0: + return f"BGP-LS table not empty: {count} route(s) still present" + return None + + def build_topo(tgen): """Build the test topology""" @@ -853,6 +869,97 @@ def test_bgp_ls_r4_link_no_shutdown(): assert result is None, '"rr" did not receive BGP-LS r4 link up update' +def test_bgp_ls_peer_deactivate(): + """ + Test that deactivating the last BGP-LS peer: + - Withdraws all locally originated BGP-LS routes on the producer (r2) + - Clears all received BGP-LS routes on the consumer (rr) + """ + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + logger.info( + "Deactivating BGP-LS peer on r2 and verifying route withdrawal on r2 and rr" + ) + + r2 = tgen.gears["r2"] + + # Deactivate the only BGP-LS peer on r2 (neighbor 10.0.3.4 = rr). + r2.vtysh_cmd( + "configure terminal\n" + "router bgp 65000\n" + "address-family link-state\n" + "no neighbor 10.0.3.4 activate" + ) + + # r2 must have withdrawn all its locally originated BGP-LS routes + test_func = functools.partial(check_bgp_ls_empty, r2) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, '"r2" BGP-LS routes not withdrawn after peer deactivation' + + # rr (consumer) must have no BGP-LS routes once r2's session goes down + consumer = tgen.gears["rr"] + test_func = functools.partial(check_bgp_ls_empty, consumer) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, '"rr" BGP-LS routes not cleared after r2 peer deactivation' + + +def test_bgp_ls_peer_reactivate(): + """ + Test that reactivating a BGP-LS peer after deactivation: + - Triggers a fresh TED sync from the IGP + - Re-originates all IGP topology as BGP-LS NLRIs + - Re-advertises all routes to the consumer (rr) + + The previous test (test_bgp_ls_peer_deactivate) must run first. + """ + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + logger.info( + "Reactivating BGP-LS peer on r2 and verifying route re-advertisement on r2 and rr" + ) + + r2 = tgen.gears["r2"] + + # Reactivate the BGP-LS peer. + r2.vtysh_cmd( + "configure terminal\n" + "router bgp 65000\n" + "address-family link-state\n" + "neighbor 10.0.3.4 activate" + ) + + # r2 must re-originate all BGP-LS NLRIs for the full ISIS topology + reffile = os.path.join(CWD, "r2/bgp_ls_nlri.json") + expected = json.loads(open(reffile).read()) + test_func = functools.partial( + topotest.router_json_cmp, + r2, + "show bgp link-state link-state json", + expected, + ) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assert result is None, '"r2" did not re-originate BGP-LS routes after peer reactivation' + + # rr (consumer) must receive all BGP-LS routes again + consumer = tgen.gears["rr"] + reffile = os.path.join(CWD, "rr/bgp_ls_nlri.json") + expected = json.loads(open(reffile).read()) + test_func = functools.partial( + topotest.router_json_cmp, + consumer, + "show bgp link-state link-state json", + expected, + ) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assert result is None, '"rr" did not receive BGP-LS routes after r2 peer reactivation' + + def test_memory_leak(): """Run the memory leak test and report results""" tgen = get_topogen()