diff options
| -rw-r--r-- | bgpd/bgp_bmp.c | 68 | ||||
| -rw-r--r-- | bgpd/bgp_io.c | 2 | ||||
| -rw-r--r-- | bgpd/bgp_nht.c | 6 | ||||
| -rw-r--r-- | bgpd/bgp_nht.h | 5 | ||||
| -rw-r--r-- | bgpd/bgp_trace.h | 18 | ||||
| -rw-r--r-- | bgpd/bgp_vty.c | 12 | ||||
| -rw-r--r-- | bgpd/bgpd.c | 4 | ||||
| -rw-r--r-- | bgpd/bgpd.h | 3 | ||||
| -rw-r--r-- | tests/topotests/bgp_bmp/bgpbmp.py | 28 | ||||
| -rw-r--r-- | tests/topotests/bgp_bmp/test_bgp_bmp_2.py | 163 | ||||
| -rw-r--r-- | tests/topotests/lib/bmp_collector/bgp/update/rd.py | 9 |
11 files changed, 307 insertions, 11 deletions
diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index acc49cac94..036bece359 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" @@ -1708,6 +1709,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) { @@ -2013,11 +2034,16 @@ static void bmp_bgp_peer_vrf(struct bmp_bgp_peer *bbpeer, struct bgp *bgp) size_t open_len = stream_get_endp(s); bbpeer->open_rx_len = open_len; + if (bbpeer->open_rx) + XFREE(MTYPE_BMP_OPEN, bbpeer->open_rx); bbpeer->open_rx = XMALLOC(MTYPE_BMP_OPEN, open_len); memcpy(bbpeer->open_rx, s->data, open_len); bbpeer->open_tx_len = open_len; - bbpeer->open_tx = bbpeer->open_rx; + if (bbpeer->open_tx) + XFREE(MTYPE_BMP_OPEN, bbpeer->open_tx); + bbpeer->open_tx = XMALLOC(MTYPE_BMP_OPEN, open_len); + memcpy(bbpeer->open_tx, s->data, open_len); stream_free(s); } @@ -2057,6 +2083,7 @@ bool bmp_bgp_update_vrf_status(struct bmp_bgp *bmpbgp, enum bmp_vrf_state force) } else { bbpeer = bmp_bgp_peer_find(peer->qobj_node.nid); if (bbpeer) { + XFREE(MTYPE_BMP_OPEN, bbpeer->open_tx); XFREE(MTYPE_BMP_OPEN, bbpeer->open_rx); bmp_peerh_del(&bmp_peerh, bbpeer); XFREE(MTYPE_BMP_PEER, bbpeer); @@ -3090,11 +3117,14 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, return 0; } - struct bmp_bgp *bmpbgp = bmp_bgp_get(bgp); + struct bmp_bgp *bmpbgp = bmp_bgp_find(bgp); struct peer *peer = updated_route->peer; struct bmp_targets *bt; struct bmp *bmp; + if (!bmpbgp) + return 0; + frr_each (bmp_targets, &bmpbgp->targets, bt) { if (CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB)) { is_locribmon_enabled = true; @@ -3149,6 +3179,37 @@ static int bgp_bmp_early_fini(void) return 0; } +/* called when the routerid of an instance changes */ +static int bmp_bgp_attribute_updated(struct bgp *bgp, bool withdraw) +{ + struct bmp_bgp *bmpbgp = bmp_bgp_find(bgp); + + if (!bmpbgp) + return 0; + + bmp_bgp_update_vrf_status(bmpbgp, vrf_state_unknown); + + if (bmpbgp->vrf_state == vrf_state_down) + /* do not send peer events, router id will not be enough to set state to up + */ + return 0; + + /* vrf_state is up: trigger a peer event + */ + bmp_send_all_safe(bmpbgp, bmp_peerstate(bgp->peer_self, withdraw)); + return 1; +} + +static int bmp_routerid_update(struct bgp *bgp, bool withdraw) +{ + return bmp_bgp_attribute_updated(bgp, withdraw); +} + +static int bmp_route_distinguisher_update(struct bgp *bgp, afi_t afi, bool preconfig) +{ + return bmp_bgp_attribute_updated(bgp, preconfig); +} + /* called when a bgp instance goes up/down, implying that the underlying VRF * has been created or deleted in zebra */ @@ -3194,6 +3255,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); @@ -3201,6 +3263,8 @@ static int bgp_bmp_module_init(void) hook_register(frr_early_fini, bgp_bmp_early_fini); hook_register(bgp_instance_state, bmp_vrf_state_changed); hook_register(bgp_vrf_status_changed, bmp_vrf_itf_state_changed); + hook_register(bgp_routerid_update, bmp_routerid_update); + hook_register(bgp_route_distinguisher_update, bmp_route_distinguisher_update); return 0; } diff --git a/bgpd/bgp_io.c b/bgpd/bgp_io.c index 9e9251c854..5d0f14cc5c 100644 --- a/bgpd/bgp_io.c +++ b/bgpd/bgp_io.c @@ -199,7 +199,7 @@ static int read_ibuf_work(struct peer_connection *connection) assert(ringbuf_get(ibw, pkt->data, pktsize) == pktsize); stream_set_endp(pkt, pktsize); - frrtrace(2, frr_bgp, packet_read, connection->peer, pkt); + frrtrace(2, frr_bgp, packet_read, connection, pkt); frr_with_mutex (&connection->io_mtx) { stream_fifo_push(connection->ibuf, pkt); } diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index ed83757ea3..164e2300c0 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 @@ -1449,6 +1452,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 e7c6fdc281..345089ac5a 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 a77a25e435..43bc7a5a17 100644 --- a/bgpd/bgp_trace.h +++ b/bgpd/bgp_trace.h @@ -211,6 +211,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 */ TRACEPOINT_EVENT( diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 40b2847c6f..c6b09481b6 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -141,6 +141,8 @@ DEFINE_HOOK(bgp_inst_config_write, DEFINE_HOOK(bgp_snmp_update_last_changed, (struct bgp *bgp), (bgp)); DEFINE_HOOK(bgp_snmp_init_stats, (struct bgp *bgp), (bgp)); DEFINE_HOOK(bgp_snmp_traps_config_write, (struct vty * vty), (vty)); +DEFINE_HOOK(bgp_route_distinguisher_update, (struct bgp *bgp, afi_t afi, bool preconfig), + (bgp, afi, preconfig)); static struct peer_group *listen_range_exists(struct bgp *bgp, struct prefix *range, int exact); @@ -9805,6 +9807,14 @@ DEFPY (af_rd_vpn_export, vpn_leak_prechange(BGP_VPN_POLICY_DIR_TOVPN, afi, bgp_get_default(), bgp); + if (!bgp->vpn_policy[afi].tovpn_rd_pretty && !rd_str) + return CMD_SUCCESS; + + if (yes && bgp->vpn_policy[afi].tovpn_rd_pretty && rd_str && + strmatch(rd_str, bgp->vpn_policy[afi].tovpn_rd_pretty)) + return CMD_SUCCESS; + + hook_call(bgp_route_distinguisher_update, bgp, afi, true); if (yes) { if (bgp->vpn_policy[afi].tovpn_rd_pretty) XFREE(MTYPE_BGP_NAME, bgp->vpn_policy[afi].tovpn_rd_pretty); @@ -9815,9 +9825,11 @@ DEFPY (af_rd_vpn_export, BGP_VPN_POLICY_TOVPN_RD_SET); } else { XFREE(MTYPE_BGP_NAME, bgp->vpn_policy[afi].tovpn_rd_pretty); + bgp->vpn_policy[afi].tovpn_rd_pretty = NULL; UNSET_FLAG(bgp->vpn_policy[afi].flags, BGP_VPN_POLICY_TOVPN_RD_SET); } + hook_call(bgp_route_distinguisher_update, bgp, afi, false); /* post-change: re-export vpn routes */ vpn_leak_postchange(BGP_VPN_POLICY_DIR_TOVPN, afi, diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index eda6bc31d2..05bc804db4 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -86,6 +86,7 @@ DEFINE_QOBJ_TYPE(bgp); DEFINE_QOBJ_TYPE(peer); DEFINE_HOOK(bgp_inst_delete, (struct bgp *bgp), (bgp)); DEFINE_HOOK(bgp_instance_state, (struct bgp *bgp), (bgp)); +DEFINE_HOOK(bgp_routerid_update, (struct bgp *bgp, bool withdraw), (bgp, withdraw)); /* BGP process wide configuration. */ static struct bgp_master bgp_master; @@ -301,6 +302,8 @@ static int bgp_router_id_set(struct bgp *bgp, const struct in_addr *id, vpn_handle_router_id_update(bgp, true, is_config); + hook_call(bgp_routerid_update, bgp, true); + IPV4_ADDR_COPY(&bgp->router_id, id); /* Set all peer's local identifier with this value. */ @@ -318,6 +321,7 @@ static int bgp_router_id_set(struct bgp *bgp, const struct in_addr *id, vpn_handle_router_id_update(bgp, false, is_config); + hook_call(bgp_routerid_update, bgp, false); return 0; } diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 47214e52e5..65b268c4ea 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -907,6 +907,9 @@ DECLARE_HOOK(bgp_config_end, (struct bgp *bgp), (bgp)); DECLARE_HOOK(bgp_hook_vrf_update, (struct vrf *vrf, bool enabled), (vrf, enabled)); DECLARE_HOOK(bgp_instance_state, (struct bgp *bgp), (bgp)); +DECLARE_HOOK(bgp_routerid_update, (struct bgp *bgp, bool withdraw), (bgp, withdraw)); +DECLARE_HOOK(bgp_route_distinguisher_update, (struct bgp *bgp, afi_t afi, bool preconfig), + (bgp, afi, preconfig)); /* Thread callback information */ struct afi_safi_info { diff --git a/tests/topotests/bgp_bmp/bgpbmp.py b/tests/topotests/bgp_bmp/bgpbmp.py index eac78a63f7..acbc405aa4 100644 --- a/tests/topotests/bgp_bmp/bgpbmp.py +++ b/tests/topotests/bgp_bmp/bgpbmp.py @@ -187,12 +187,19 @@ def bmp_check_for_prefixes( def bmp_check_for_peer_message( - expected_peers, bmp_log_type, bmp_collector, bmp_log_file, is_rd_instance=False + expected_peers, + bmp_log_type, + bmp_collector, + bmp_log_file, + is_rd_instance=False, + peer_bgp_id=None, + peer_distinguisher=None, ): """ Check for the presence of a peer up message for the peer """ global SEQ + last_seq = SEQ # we care only about the new messages messages = [ @@ -208,6 +215,10 @@ def bmp_check_for_peer_message( for m in messages: if is_rd_instance and m["peer_distinguisher"] == "0:0": continue + if peer_distinguisher and m["peer_distinguisher"] != peer_distinguisher: + continue + if peer_bgp_id and m["peer_bgp_id"] != peer_bgp_id: + continue if ( "peer_ip" in m.keys() and m["peer_ip"] != "0.0.0.0" @@ -215,16 +226,23 @@ def bmp_check_for_peer_message( ): if is_rd_instance and m["peer_type"] != "route distinguisher instance": continue - peers.append(m["peer_ip"]) + peers.append((m["peer_ip"], m["seq"])) elif m["policy"] == "loc-rib" and m["bmp_log_type"] == bmp_log_type: - peers.append("0.0.0.0") + peers.append(("0.0.0.0", m["seq"])) # check for prefixes for ep in expected_peers: - if ep not in peers: + for _ip, _seq in peers: + if ep == _ip: + msg = "The peer {} is present in the {} log messages." + logger.debug(msg.format(ep, bmp_log_type)) + if _seq > last_seq: + last_seq = _seq + break + else: msg = "The peer {} is not present in the {} log messages." logger.debug(msg.format(ep, bmp_log_type)) return False - SEQ = messages[-1]["seq"] + SEQ = last_seq return True diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp_2.py b/tests/topotests/bgp_bmp/test_bgp_bmp_2.py index f16ff2b445..e0b9a0f607 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp_2.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp_2.py @@ -252,6 +252,169 @@ def test_peer_down(): assert success, "Checking the updated prefixes has been failed !." +def test_bgp_instance_flapping(): + """ + Checking for BGP loc-rib up messages + """ + tgen = get_topogen() + + # create flapping at BMP + tgen.net["r1vrf"].cmd("ip link set dev vrf1 down") + + peers = ["0.0.0.0"] + logger.info("checking for BMP peer down LOC-RIB message.") + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer down", + tgen.gears["bmp1vrf"], + os.path.join(tgen.logdir, "bmp1vrf", "bmp.log"), + is_rd_instance=True, + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert success, "Checking the BMP peer down LOC-RIB message failed !." + + tgen.net["r1vrf"].cmd("ip link set dev vrf1 up") + + logger.info("checking for BMP peer up LOC-RIB message.") + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer up", + tgen.gears["bmp1vrf"], + os.path.join(tgen.logdir, "bmp1vrf", "bmp.log"), + is_rd_instance=True, + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert success, "Checking the BMP peer up LOC-RIB message failed !." + + +def test_bgp_routerid_changed(): + """ + Checking for BGP loc-rib up messages with new router-id + """ + tgen = get_topogen() + + tgen.gears["r1vrf"].vtysh_cmd( + """ + configure terminal + router bgp 65501 vrf vrf1 + bgp router-id 192.168.1.77 + """ + ) + + peers = ["0.0.0.0"] + + logger.info( + "checking for BMP peer down LOC-RIB message with router-id set to 192.168.0.1." + ) + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer down", + tgen.gears["bmp1vrf"], + os.path.join(tgen.logdir, "bmp1vrf", "bmp.log"), + is_rd_instance=True, + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert ( + success + ), "Checking the BMP peer down LOC-RIB message with router-id set to 192.168.0.1 failed !." + + logger.info( + "checking for BMP peer up LOC-RIB message with router-id set to 192.168.1.77." + ) + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer up", + tgen.gears["bmp1vrf"], + os.path.join(tgen.logdir, "bmp1vrf", "bmp.log"), + is_rd_instance=True, + peer_bgp_id="192.168.1.77", + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert ( + success + ), "Checking the BMP peer up LOC-RIB message with router-id set to 192.168.1.77 failed !." + + +def test_reconfigure_route_distinguisher_vrf1(): + """ + Checking for BMP peers down messages + """ + tgen = get_topogen() + + bmp_update_seq( + tgen.gears["bmp1vrf"], os.path.join(tgen.logdir, "bmp1vrf", "bmp.log") + ) + peers = ["0.0.0.0"] + + tgen.gears["r1vrf"].vtysh_cmd( + """ + configure terminal + router bgp 65501 vrf vrf1 + address-family ipv4 unicast + rd vpn export 666:22 + exit-address-family + address-family ipv6 unicast + rd vpn export 666:22 + """ + ) + logger.info( + "checking for BMP peer down LOC-RIB message with route-distinguisher set to 444:1" + ) + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer down", + tgen.gears["bmp1vrf"], + os.path.join(tgen.logdir, "bmp1vrf", "bmp.log"), + is_rd_instance=True, + peer_distinguisher="444:1", + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert ( + success + ), "Checking the BMP peer down LOC-RIB message with route-distinguisher set to 444:1 failed !." + + logger.info( + "checking for BMP peer up LOC-RIB messages with route-distinguisher set to 666:22" + ) + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer up", + tgen.gears["bmp1vrf"], + os.path.join(tgen.logdir, "bmp1vrf", "bmp.log"), + is_rd_instance=True, + peer_bgp_id="192.168.1.77", + peer_distinguisher="666:22", + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert ( + success + ), "Checking the BMP peer up LOC-RIB message with route-distinguisher set to 666:22 failed !." + + logger.info( + "checking for BMP peer up messages with route-distinguisher set to 666:22" + ) + peers = ["192.168.0.2", "192:168::2"] + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer up", + tgen.gears["bmp1vrf"], + os.path.join(tgen.logdir, "bmp1vrf", "bmp.log"), + is_rd_instance=True, + peer_distinguisher="666:22", + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert ( + success + ), "Checking the BMP peer up messages with route-distinguisher set to 666:22 failed !." + + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] sys.exit(pytest.main(args)) diff --git a/tests/topotests/lib/bmp_collector/bgp/update/rd.py b/tests/topotests/lib/bmp_collector/bgp/update/rd.py index 3f08de5ae9..d44060bf2f 100644 --- a/tests/topotests/lib/bmp_collector/bgp/update/rd.py +++ b/tests/topotests/lib/bmp_collector/bgp/update/rd.py @@ -4,6 +4,7 @@ # Authored by Farid Mihoub <farid.mihoub@6wind.com> # import ipaddress +import socket import struct @@ -45,9 +46,11 @@ class RouteDistinguisher: self.repr_str = f"{self.as_number}:{self.assigned_sp}" elif rd_type == 1: - (self.admin_ipv4, self.assigned_sp) = struct.unpack_from("!IH", self.rd[2:]) - ipv4 = str(ipaddress.IPv4Address(self.admin_ipv4)) - self.repr_str = f"{self.as_number}:{self.assigned_sp}" + (self.admin_ipv4, self.assigned_sp) = struct.unpack_from( + "!4sH", self.rd[2:] + ) + ipv4_str = socket.inet_ntoa(self.admin_ipv4) + self.repr_str = f"{ipv4_str}:{self.assigned_sp}" elif rd_type == 2: (self.four_bytes_as, self.assigned_sp) = struct.unpack_from( |