Skip to content

Commit

Permalink
bgpd: fix do re-send post-policy bgp update when not valid
Browse files Browse the repository at this point in the history
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 <philippe.guibert@6wind.com>
  • Loading branch information
pguibert6WIND committed Nov 18, 2024
1 parent b8a6514 commit 6ed7ba5
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 0 deletions.
22 changes: 22 additions & 0 deletions bgpd/bgp_bmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions bgpd/bgp_nht.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down
5 changes: 5 additions & 0 deletions bgpd/bgp_nht.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
18 changes: 18 additions & 0 deletions bgpd/bgp_trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down

0 comments on commit 6ed7ba5

Please sign in to comment.