From 2fa79fb01517efd4006f9f98ccb01d695bb95f42 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 29 Oct 2024 16:20:18 +0100 Subject: [PATCH] bgpd: fix do re-send post-policy bgp update when not valid When a BGP listener configured with BMP receives the first BGP IPv6 update from a connected BGP IPv6 peer, the BMP collector receives a withdraw post-policy message. > {"peer_type": "route distinguisher instance", "policy": "post-policy", > "ipv6": true, "peer_ip": "192:167::3", "peer_distinguisher": "444:1", > "peer_asn": 65501, "peer_bgp_id": "192.168.1.3", "timestamp": > "2024-10-29 11:44:47.111962", "bmp_log_type": "withdraw", "afi": 2, > "safi": 1, "ip_prefix": "2001::1125/128", "seq": 22} > {"peer_type": "route distinguisher instance", "policy": "pre-policy", > "ipv6": true, "peer_ip": "192:167::3", "peer_distinguisher": "444:1", > "peer_asn": 65501, "peer_bgp_id": "192.168.1.3", "timestamp": > "2024-10-29 11:44:47.111963", "bmp_log_type": "update", "origin": > "IGP", "as_path": "", "afi": 2, "safi": 1, "nxhp_ip": "192:167::3", > "nxhp_link-local": "fe80::7063:d8ff:fedb:9e11", "ip_prefix": "2001::1125/128", "seq": 23} Actually, the BGP update is not valid, and BMP considers it as a withdraw message. The BGP upate is not valid, because the nexthop reachability is unknown at the time of reception, and no other BMP message is sent. Fix this by re-sending a BMP post update message when nexthop tracking becomes successfull. Generalise the re-sending of messages when nexthop tracking changes. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 22 ++++++++++++++++++++++ bgpd/bgp_nht.c | 6 ++++++ bgpd/bgp_nht.h | 5 +++++ bgpd/bgp_trace.h | 18 ++++++++++++++++++ 4 files changed, 51 insertions(+) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index a3acd35195fc..4249c8af5a3f 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -28,6 +28,7 @@ #include "bgpd/bgp_table.h" #include "bgpd/bgpd.h" #include "bgpd/bgp_route.h" +#include "bgpd/bgp_nht.h" #include "bgpd/bgp_attr.h" #include "bgpd/bgp_dump.h" #include "bgpd/bgp_errors.h" @@ -1679,6 +1680,26 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, return 0; } +static int bmp_nht_path_valid(struct bgp *bgp, struct bgp_path_info *path, bool valid) +{ + struct bgp_dest *dest = path->net; + struct bgp_table *table; + + if (frrtrace_enabled(frr_bgp, bmp_nht_path_valid)) { + char pfxprint[PREFIX2STR_BUFFER]; + + prefix2str(&dest->rn->p, pfxprint, sizeof(pfxprint)); + frrtrace(4, frr_bgp, bmp_nht_path_valid, bgp, pfxprint, path, valid); + } + if (bgp->peer_self == path->peer) + /* self declared networks or redistributed networks are not relevant for bmp */ + return 0; + + table = bgp_dest_table(dest); + + return bmp_process(bgp, table->afi, table->safi, dest, path->peer, !valid); +} + static void bmp_stat_put_u32(struct stream *s, size_t *cnt, uint16_t type, uint32_t value) { @@ -3156,6 +3177,7 @@ static int bgp_bmp_module_init(void) hook_register(peer_status_changed, bmp_peer_status_changed); hook_register(peer_backward_transition, bmp_peer_backward); hook_register(bgp_process, bmp_process); + hook_register(bgp_nht_path_update, bmp_nht_path_valid); hook_register(bgp_inst_config_write, bmp_config_write); hook_register(bgp_inst_delete, bmp_bgp_del); hook_register(frr_late_init, bgp_bmp_init); diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index 9b633b7139b2..f4d83bd647ac 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -41,6 +41,9 @@ static void unregister_zebra_rnh(struct bgp_nexthop_cache *bnc); static int make_prefix(int afi, struct bgp_path_info *pi, struct prefix *p); static void bgp_nht_ifp_initial(struct event *thread); +DEFINE_HOOK(bgp_nht_path_update, (struct bgp *bgp, struct bgp_path_info *pi, bool valid), + (bgp, pi, valid)); + static int bgp_isvalid_nexthop(struct bgp_nexthop_cache *bnc) { return (bgp_zebra_num_connects() == 0 @@ -1442,6 +1445,9 @@ void evaluate_paths(struct bgp_nexthop_cache *bnc) } } + if (path_valid != bnc_is_valid_nexthop) + hook_call(bgp_nht_path_update, bgp_path, path, bnc_is_valid_nexthop); + bgp_process(bgp_path, dest, path, afi, safi); } diff --git a/bgpd/bgp_nht.h b/bgpd/bgp_nht.h index e7c6fdc281b6..0279837b084d 100644 --- a/bgpd/bgp_nht.h +++ b/bgpd/bgp_nht.h @@ -83,4 +83,9 @@ extern void bgp_nht_ifp_up(struct interface *ifp); extern void bgp_nht_ifp_down(struct interface *ifp); extern void bgp_nht_interface_events(struct peer *peer); + +/* called when a path becomes valid or invalid, because of nexthop tracking */ +DECLARE_HOOK(bgp_nht_path_update, (struct bgp * bgp, struct bgp_path_info *pi, bool valid), + (bgp, pi, valid)); + #endif /* _BGP_NHT_H */ diff --git a/bgpd/bgp_trace.h b/bgpd/bgp_trace.h index a77a25e435e1..43bc7a5a1738 100644 --- a/bgpd/bgp_trace.h +++ b/bgpd/bgp_trace.h @@ -210,6 +210,24 @@ TRACEPOINT_EVENT( TRACEPOINT_LOGLEVEL(frr_bgp, bmp_process, TRACE_DEBUG) +/* + * BMP is hooked for a nexthop tracking event + */ +TRACEPOINT_EVENT( + frr_bgp, + bmp_nht_path_valid, + TP_ARGS(struct bgp *, bgp, char *, pfx, struct bgp_path_info *, + path, bool, valid), + TP_FIELDS( + ctf_string(bgp, bgp->name_pretty) + ctf_string(prefix, pfx) + ctf_string(path, PEER_HOSTNAME(path->peer)) + ctf_integer(bool, valid, valid) + ) +) + +TRACEPOINT_LOGLEVEL(frr_bgp, bmp_nht_path_valid, TRACE_DEBUG) + /* * bgp_dest_lock/bgp_dest_unlock */