From 89261d38e804c1b163d50068c73b84841d2d6519 Mon Sep 17 00:00:00 2001 From: mxyns Date: Wed, 28 Sep 2022 08:05:25 +0200 Subject: [PATCH] bgpd: loc-rib uptime moved to bgp_path_info_extra and set in header moved loc-rib uptime field "bgp_rib_uptime" to struct bgp_path_info_extra for memory concerns moved logic into bgp_route_update's callback bmp_route_update written timestamp in per peer header Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 134 +++++++++++++++++++++++++++++------------- bgpd/bgp_bmp.h | 7 +-- bgpd/bgp_route.c | 19 ++---- bgpd/bgp_route.h | 8 ++- doc/developer/bmp.rst | 49 +++++++++++++++ doc/user/bmp.rst | 49 +-------------- 6 files changed, 158 insertions(+), 108 deletions(-) create mode 100644 doc/developer/bmp.rst diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index cf41972c454f..9a6d054314df 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -250,24 +250,40 @@ static inline uint64_t bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi, { /* remove this check when the other peer types get correct peer dist. - // (RFC7854) impl. */ + *(RFC7854) impl. + */ if (peer_type != BMP_PEER_TYPE_LOC_RIB_INSTANCE) return 0; /* sending vrf_id or rd could be turned into an option at some point */ struct bgp *bgp = bmp->targets->bgp; - struct prefix_rd *prd = &bgp->vpn_policy[afi].tovpn_rd; - /* - * default vrf => can't have RD => 0 + + /* default vrf => can't have RD => 0 * vrf => has RD? * if yes => use RD value - * else => use vrf_id and convert it so that - * peer_distinguisher is 0::vrf_id + * else => use vrf_id + * vrf_id == VRF_UNKNOWN ? + * if yes => 0 + * else => convert it so that + * peer_distinguisher is 0::vrf_id */ - return bgp->inst_type == VRF_DEFAULT - ? 0 - : prd ? *(uint64_t *)prd->val - : (((uint64_t)htonl(bgp->vrf_id)) << 32); + if (bgp->inst_type == VRF_DEFAULT) + return 0; + + struct prefix_rd *prd = &bgp->vpn_policy[afi].tovpn_rd; + + if (CHECK_FLAG(bgp->vpn_policy[afi].flags, + BGP_VPN_POLICY_TOVPN_RD_SET)) { + uint64_t peerd = 0; + + memcpy(&peerd, prd->val, sizeof(prd->val)); + return peerd; + } + + if (bgp->vrf_id == VRF_UNKNOWN) + return 0; + + return ((uint64_t)htonl(bgp->vrf_id)) << 32; } static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type) @@ -304,7 +320,8 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp *bgp, /* Peer Address */ /* Set to 0 if it's a LOC-RIB INSTANCE (RFC 9069) or if it's not an - * IPv4/6 address */ + * IPv4/6 address + */ if (is_locrib || (peer->connection->su.sa.sa_family != AF_INET6 && peer->connection->su.sa.sa_family != AF_INET)) { stream_putl(s, 0); @@ -322,20 +339,23 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp *bgp, /* Peer AS */ /* set peer ASN but for LOC-RIB INSTANCE (RFC 9069) put the local bgp - * ASN if available or 0 */ + * ASN if available or 0 + */ as_t asn = !is_locrib ? peer->as : bgp ? bgp->as : 0L; + stream_putl(s, asn); /* Peer BGP ID */ /* set router-id but for LOC-RIB INSTANCE (RFC 9069) put the instance - * router-id if available or 0 */ + * router-id if available or 0 + */ struct in_addr *bgp_id = !is_locrib ? &peer->remote_id : bgp ? &bgp->router_id : NULL; + stream_put_in_addr(s, bgp_id); /* Timestamp */ - /* set to 0 for LOC-RIB INSTANCE as install uptime is not saved atm */ - if (!is_locrib && tv) { + if (tv) { stream_putl(s, tv->tv_sec); stream_putl(s, tv->tv_usec); } else { @@ -361,6 +381,7 @@ bmp_put_vrftablename_info_tlv(struct stream *s, struct bmp *bmp) const char *vrftablename = "global"; if (bmp->targets->bgp->inst_type != BGP_INSTANCE_TYPE_DEFAULT) { struct vrf *vrf = vrf_lookup_by_id(bmp->targets->bgp->vrf_id); + vrftablename = vrf ? vrf->name : NULL; } if (vrftablename != NULL) @@ -715,8 +736,7 @@ static bool bmp_wrmirror(struct bmp *bmp, struct pullwr *pullwr) bmp_common_hdr(s, BMP_VERSION_3, BMP_TYPE_ROUTE_MIRRORING); bmp_per_peer_hdr(s, bmp->targets->bgp, peer, 0, - BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, - &bmq->tv); + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &bmq->tv); /* BMP Mirror TLV. */ stream_putw(s, BMP_MIRROR_TLV_TYPE_BGP_MESSAGE); @@ -995,7 +1015,7 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, bmp_common_hdr(hdr, BMP_VERSION_3, BMP_TYPE_ROUTE_MONITORING); bmp_per_peer_hdr(hdr, bmp->targets->bgp, peer, flags, peer_type_flag, bmp_get_peer_distinguisher(bmp, afi, peer_type_flag), - &uptime_real); + uptime == (time_t)(-1L) ? NULL : &uptime_real); stream_putl_at(hdr, BMP_LENGTH_POS, stream_get_endp(hdr) + stream_get_endp(msg)); @@ -1180,16 +1200,17 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) prd = (struct prefix_rd *)bgp_dest_get_prefix(bmp->syncrdpos); if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED) && - CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { + CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { bmp_monitor(bmp, bpi->peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, - bn_p, prd, bpi->attr, afi, safi, bpi->rib_uptime); + bn_p, prd, bpi->attr, afi, safi, + bpi && bpi->extra ? bpi->extra->bgp_rib_uptime + : (time_t)(-1L)); } if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_VALID) && CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_POSTPOLICY)) bmp_monitor(bmp, bpi->peer, BMP_PEER_FLAG_L, - BMP_PEER_TYPE_GLOBAL_INSTANCE, bn_p, prd, - bpi->attr, + BMP_PEER_TYPE_GLOBAL_INSTANCE, bn_p, prd, bpi->attr, afi, safi, bpi->uptime); if (adjin) @@ -1253,9 +1274,8 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) afi_t afi = bqe->afi; safi_t safi = bqe->safi; - if (!CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { + if (!CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) goto out; - } switch (bmp->afistate[afi][safi]) { case BMP_AFI_INACTIVE: @@ -1280,11 +1300,13 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) peer = QOBJ_GET_TYPESAFE(bqe->peerid, peer); if (!peer) { - // skipping queued item for deleted peer + /* skipping queued item for deleted peer + */ goto out; } if (peer != bmp->targets->bgp->peer_self && !peer_established(peer)) { - // peer is neither self, nor established + /* peer is neither self, nor established + */ goto out; } @@ -1292,10 +1314,12 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) (bqe->safi == SAFI_MPLS_VPN); struct prefix_rd *prd = is_vpn ? &bqe->rd : NULL; + bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], safi, - &bqe->p, prd); + &bqe->p, prd); struct bgp_path_info *bpi; + for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; bpi = bpi->next) { if (!CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED)) @@ -1306,7 +1330,8 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, &bqe->p, prd, bpi ? bpi->attr : NULL, afi, safi, - bpi ? bpi->rib_uptime : monotime(NULL)); + bpi && bpi->extra ? bpi->extra->bgp_rib_uptime + : (time_t)(-1L)); written = true; out: @@ -1358,7 +1383,6 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) if (!peer_established(peer->connection)) goto out; - bool is_vpn = (bqe->afi == AFI_L2VPN && bqe->safi == SAFI_EVPN) || (bqe->safi == SAFI_MPLS_VPN); @@ -1484,7 +1508,8 @@ bmp_process_one(struct bmp_targets *bt, struct bmp_qhash_head *updhash, return bqe; /* need to update correct queue pos for all sessions of the target after - * a call to this function */ + * a call to this function + */ } static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, @@ -1507,7 +1532,8 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, frr_each(bmp_targets, &bmpbgp->targets, bt) { /* check if any monitoring is enabled (ignoring loc-rib since it - * uses another hook & queue */ + * uses another hook & queue + */ if (!(bt->afimon[afi][safi] & ~BMP_MON_LOC_RIB)) continue; @@ -1515,8 +1541,9 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, bmp_process_one(bt, &bt->updhash, &bt->updlist, bgp, afi, safi, bn, peer); - if (!last_item) // if bmp_process_one returns NULL we don't have - // anything to do next + // if bmp_process_one returns NULL + // we don't have anything to do next + if (!last_item) continue; frr_each(bmp_session, &bt->sessions, bmp) { @@ -1562,8 +1589,7 @@ static void bmp_stats(struct event *thread) s = stream_new(BGP_MAX_PACKET_SIZE); bmp_common_hdr(s, BMP_VERSION_3, BMP_TYPE_STATISTICS_REPORT); bmp_per_peer_hdr(s, bt->bgp, peer, 0, - BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, - &tv); + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &tv); count_pos = stream_get_endp(s); stream_putl(s, 0); @@ -2436,9 +2462,9 @@ DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, argv_find_and_parse_afi(argv, argc, &index, &afi); argv_find_and_parse_safi(argv, argc, &index, &safi); - if (policy[0] == 'l') { + if (policy[0] == 'l') flag = BMP_MON_LOC_RIB; - } else if (policy[1] == 'r') + else if (policy[1] == 'r') flag = BMP_MON_PREPOLICY; else flag = BMP_MON_POSTPOLICY; @@ -2772,14 +2798,39 @@ static int bgp_bmp_init(struct event_loop *tm) /* TODO remove "bn", redundant with updated_route->net ? */ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, - struct bgp_path_info *updated_route, bool withdraw) + struct bgp_path_info *old_route, + struct bgp_path_info *new_route) { - + bool is_locribmon_enabled = false; + bool is_withdraw = old_route && !new_route; + struct bgp_path_info *updated_route = + is_withdraw ? old_route : new_route; struct bmp_bgp *bmpbgp = bmp_bgp_get(bgp); struct peer *peer = updated_route->peer; struct bmp_targets *bt; struct bmp *bmp; + frr_each (bmp_targets, &bmpbgp->targets, bt) { + if ((is_locribmon_enabled |= + (bt->afimon[afi][safi] & BMP_MON_LOC_RIB))) + break; + } + + if (!is_locribmon_enabled) + return 0; + + /* route is not installed in locrib anymore and rib uptime was saved */ + if (old_route && old_route->extra) + bgp_path_info_extra_get(old_route)->bgp_rib_uptime = + (time_t)(-1L); + + /* route is installed in locrib from now on so + * save rib uptime in bgp_path_info_extra + */ + if (new_route) + bgp_path_info_extra_get(new_route)->bgp_rib_uptime = + monotime(NULL); + frr_each (bmp_targets, &bmpbgp->targets, bt) { if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB) { @@ -2787,8 +2838,9 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, bt, &bt->locupdhash, &bt->locupdlist, bgp, afi, safi, bn, peer); - if (!last_item) // if bmp_process_one returns NULL we - // don't have anything to do next + // if bmp_process_one returns NULL + // we don't have anything to do next + if (!last_item) continue; frr_each (bmp_session, &bt->sessions, bmp) { diff --git a/bgpd/bgp_bmp.h b/bgpd/bgp_bmp.h index 152e7eb0e2ef..dadd99eb6d0a 100644 --- a/bgpd/bgp_bmp.h +++ b/bgpd/bgp_bmp.h @@ -216,15 +216,14 @@ struct bmp_targets { int stat_msec; /* only supporting: - * - IPv4 / unicast & multicast - * - IPv6 / unicast & multicast + * - IPv4 / unicast & multicast & VPN + * - IPv6 / unicast & multicast & VPN * - L2VPN / EVPN */ #define BMP_MON_PREPOLICY (1 << 0) #define BMP_MON_POSTPOLICY (1 << 1) - -/* TODO define BMP_MON_LOC_RIB flag */ #define BMP_MON_LOC_RIB (1 << 2) + uint8_t afimon[AFI_MAX][SAFI_MAX]; bool mirror; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index d2b36dd8fb80..18cad398418c 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -87,8 +87,8 @@ DEFINE_HOOK(bgp_rpki_prefix_status, DEFINE_HOOK(bgp_route_update, (struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, - struct bgp_path_info *updated_route, bool withdraw), - (bgp, afi, safi, bn, updated_route, withdraw)); + struct bgp_path_info *old_route, struct bgp_path_info *new_route), + (bgp, afi, safi, bn, old_route, new_route)); /* Extern from bgp_dump.c */ extern const char *bgp_origin_str[]; @@ -3447,17 +3447,11 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, } /* call bmp hook for loc-rib route update / withdraw after flags were - * set */ + * set + */ if (old_select || new_select) { - - if (old_select) /* route is not installed in locrib anymore */ - old_select->rib_uptime = (time_t)(-1L); - if (new_select) /* route is installed in locrib from now on */ - new_select->rib_uptime = bgp_clock(); - bool is_withdraw = old_select && !new_select; - - hook_call(bgp_route_update, bgp, afi, safi, dest, - is_withdraw ? old_select : new_select, is_withdraw); + hook_call(bgp_route_update, bgp, afi, safi, dest, old_select, + new_select); } @@ -3985,7 +3979,6 @@ struct bgp_path_info *info_make(int type, int sub_type, unsigned short instance, new->peer = peer; new->attr = attr; new->uptime = monotime(NULL); - new->rib_uptime = (time_t)(-1L); new->net = dest; return new; } diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index dc0d0be69e4d..67d9b5bf844b 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -211,6 +211,9 @@ struct bgp_path_info_extra { mpls_label_t label[BGP_MAX_LABELS]; uint32_t num_labels; + /* timestamp of the rib installation */ + time_t bgp_rib_uptime; + /*For EVPN*/ struct bgp_path_info_extra_evpn *evpn; @@ -295,7 +298,6 @@ struct bgp_path_info { /* Uptime. */ time_t uptime; - time_t rib_uptime; /* reference count */ int lock; @@ -678,8 +680,8 @@ DECLARE_HOOK(bgp_process, /* called when a route is updated in the rib */ DECLARE_HOOK(bgp_route_update, (struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, - struct bgp_path_info *updated_route, bool withdraw), - (bgp, afi, safi, bn, updated_route, withdraw)); + struct bgp_path_info *old_route, struct bgp_path_info *new_route), + (bgp, afi, safi, bn, old_route, new_route)); /* BGP show options */ #define BGP_SHOW_OPT_JSON (1 << 0) diff --git a/doc/developer/bmp.rst b/doc/developer/bmp.rst new file mode 100644 index 000000000000..1c0e4b045426 --- /dev/null +++ b/doc/developer/bmp.rst @@ -0,0 +1,49 @@ +.. _bmp: + +*** +BMP +*** + +RFC 7854 +======== +Missing features (non exhaustive): + - Per-Peer Header + + - Peer Type Flag + - Peer Distingsher + + - Peer Up + + - Reason codes (according to TODO comments in code) + +Peer Type Flag and Peer Distinguisher can be implemented easily using RFC 9069's base code. + +RFC 9069 +======== +Everything that isn't listed here is implemented and should be working. +Missing features (should be exhaustive): + +- Per-Peer Header + + - Timestamp + + - set to 0 + - value is now saved `struct bgp_path_info -> locrib_uptime` + - needs testing + +- Peer Up/Down + + - VRF/Table Name TLV + + - code for TLV exists + - need better RFC understanding + +- Peer Down Only + + - Reason code (bc not supported in RFC 7854 either) + +- Statistics Report + + - Stat Type = 8: (64-bit Gauge) Number of routes in Loc-RIB. + - Stat Type = 10: Number of routes in per-AFI/SAFI Loc-RIB. The value is + structured as: 2-byte AFI, 1-byte SAFI, followed by a 64-bit Gauge. diff --git a/doc/user/bmp.rst b/doc/user/bmp.rst index e02c24c688af..6cd7e19cac3a 100644 --- a/doc/user/bmp.rst +++ b/doc/user/bmp.rst @@ -36,7 +36,7 @@ The `BMP` implementation in FRR has the following properties: successfully. OPEN messages for failed sessions cannot currently be mirrored. -- **route monitoring** is available for IPv4 and IPv6 AFIs, unicast, multicast +- **route monitoring** is available for IPv4 and IPv6 AFIs, unicast, multicast EVPN and VPN SAFIs. Other SAFIs (VPN, Labeled-Unicast, Flowspec, etc.) are not currently supported. @@ -57,51 +57,6 @@ The `BMP` implementation in FRR has the following properties: - monitoring peers with :rfc:`5549` extended next-hops has not been tested. -RFC 7854 -======== -Unsupported features (non exhaustive): - - Per-Peer Header - - - Peer Type Flag - - Peer Distingsher - - - Peer Up - - - Reason codes (according to TODO comments in code) - -Peer Type Flag and Peer Distinguisher can be implemented easily using RFC 9069's base code. - -RFC 9069 -======== -Everything that isn't listed here is implemented and should be working. -Unsupported features (should be exhaustive): - -- Per-Peer Header - - - Timestamp - - - set to 0 - - value is now saved `struct bgp_path_info -> locrib_uptime` - - needs testing - -- Peer Up/Down - - - VRF/Table Name TLV - - - code for TLV exists - - need better RFC understanding - -- Peer Down Only - - - Reason code (bc not supported in RFC 7854 either) - -- Statistics Report - - - Stat Type = 8: (64-bit Gauge) Number of routes in Loc-RIB. - - Stat Type = 10: Number of routes in per-AFI/SAFI Loc-RIB. The value is - structured as: 2-byte AFI, 1-byte SAFI, followed by a 64-bit Gauge. - - Starting BMP ============ @@ -194,7 +149,7 @@ associated with a particular ``bmp targets``: .. clicmd:: bmp monitor AFI SAFI Perform Route Monitoring for the specified AFI and SAFI. Only IPv4 and - IPv6 are currently valid for AFI. SAFI valid values are currently + IPv6 are currently valid for AFI. SAFI valid values are currently unicast, multicast, evpn and vpn. Other AFI/SAFI combinations may be added in the future.