Skip to content

Commit

Permalink
bgpd: bmp unset v6 flag + address PR#14188 comments
Browse files Browse the repository at this point in the history 
use CHECK_FLAG
fix comment spaces
change zlog_debug to zlog_warn
safeguard on updated_route
added doc/developer/bmp.rst to subdir.am
other qol changes

Signed-off-by: Maxence Younsi <[email protected]>
  • Loading branch information
@mxyns
mxyns committed Sep 11, 2023
1 parent 34b8320 commit 58ecd79
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 33 deletions.
76 changes: 44 additions & 32 deletions bgpd/bgp_bmp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ static inline int bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi,
static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type)
{
stream_putc(s, ver);
stream_putl(s, 0); /*dummy message length. will be set later. */
stream_putl(s, 0); /* dummy message length. will be set later. */
stream_putc(s, type);
}

Expand All @@ -305,7 +305,7 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp *bgp,
stream_putc(s, peer_type_flag);

/* Peer Flags */
if (peer->su.sa.sa_family == AF_INET6)
if (!is_locrib && peer->su.sa.sa_family == AF_INET6)
SET_FLAG(flags, BMP_PEER_FLAG_V);
else
UNSET_FLAG(flags, BMP_PEER_FLAG_V);
Expand All @@ -325,7 +325,7 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp *bgp,
stream_putl(s, 0);
stream_putl(s, 0);
} else if (peer->su.sa.sa_family == AF_INET6)
stream_put(s, &peer->su.sin6.sin6_addr, 16);
stream_put(s, &peer->su.sin6.sin6_addr, IPV6_MAX_BYTELEN);
else if (peer->su.sa.sa_family == AF_INET) {
stream_putl(s, 0);
stream_putl(s, 0);
Expand Down Expand Up @@ -398,7 +398,7 @@ static int bmp_send_initiation(struct bmp *bmp)
bmp_put_info_tlv(s, BMP_INFO_TYPE_SYSNAME, cmd_hostname_get());

len = stream_get_endp(s);
stream_putl_at(s, BMP_LENGTH_POS, len); /*message length is set. */
stream_putl_at(s, BMP_LENGTH_POS, len); /* message length is set. */

pullwr_write_stream(bmp->pullwr, s);
stream_free(s);
Expand Down Expand Up @@ -534,7 +534,7 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down)
}

len = stream_get_endp(s);
stream_putl_at(s, BMP_LENGTH_POS, len); /*message length is set. */
stream_putl_at(s, BMP_LENGTH_POS, len); /* message length is set. */
return s;
}

Expand Down Expand Up @@ -882,7 +882,7 @@ static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags,
/* skip this message if peer distinguisher is not available */
if (bmp_get_peer_distinguisher(bmp, afi, peer_type_flag,
&peer_distinguisher)) {
zlog_debug(
zlog_warn(
"skipping bmp message for reason: can't get peer distinguisher");
continue;
}
Expand Down Expand Up @@ -1010,7 +1010,7 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags,
/* skip this message if peer distinguisher is not available */
if (bmp_get_peer_distinguisher(bmp, afi, peer_type_flag,
&peer_distinguisher)) {
zlog_debug(
zlog_warn(
"skipping bmp message for reason: can't get peer distinguisher");
return;
}
Expand Down Expand Up @@ -1153,8 +1153,10 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr)
prefix_copy(&bmp->syncpos, bgp_dest_get_prefix(bn));
}

if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY ||
bmp->targets->afimon[afi][safi] & BMP_MON_LOC_RIB) {
if (CHECK_FLAG(bmp->targets->afimon[afi][safi],
BMP_MON_POSTPOLICY) ||
CHECK_FLAG(bmp->targets->afimon[afi][safi],
BMP_MON_LOC_RIB)) {
for (bpiter = bgp_dest_get_bgp_path_info(bn); bpiter;
bpiter = bpiter->next) {
if (!CHECK_FLAG(bpiter->flags,
Expand All @@ -1171,7 +1173,8 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr)
bpi = bpiter;
}
}
if (bmp->targets->afimon[afi][safi] & BMP_MON_PREPOLICY) {
if (CHECK_FLAG(bmp->targets->afimon[afi][safi],
BMP_MON_PREPOLICY)) {
for (adjiter = bn->adj_in; adjiter;
adjiter = adjiter->next) {
if (adjiter->peer->qobj_node.nid
Expand Down Expand Up @@ -1301,8 +1304,6 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr)
/* currently syncing & haven't reached this prefix yet
* => it'll be sent as part of the table sync, no need here
*/
zlog_info(
"bmp: skipping direct monitor msg bc will be sent with sync :)");
goto out;
case BMP_AFI_LIVE:
break;
Expand Down Expand Up @@ -1330,8 +1331,7 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr)

struct bgp_path_info *bpi;

for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi;
bpi = bpi->next) {
for (bpi = bgp_dest_get_bgp_path_info(bn); bpi; bpi = bpi->next) {
if (!CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED))
continue;
if (bpi->peer == peer)
Expand Down Expand Up @@ -1400,10 +1400,10 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr)
bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], safi,
&bqe->p, prd);

if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY) {
if (CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_POSTPOLICY)) {
struct bgp_path_info *bpi;

for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi;
for (bpi = bgp_dest_get_bgp_path_info(bn); bpi;
bpi = bpi->next) {
if (!CHECK_FLAG(bpi->flags, BGP_PATH_VALID))
continue;
Expand All @@ -1418,7 +1418,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr)
written = true;
}

if (bmp->targets->afimon[afi][safi] & BMP_MON_PREPOLICY) {
if (CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_PREPOLICY)) {
struct bgp_adj_in *adjin;

for (adjin = bn ? bn->adj_in : NULL; adjin;
Expand Down Expand Up @@ -1544,7 +1544,7 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi,
/* check if any monitoring is enabled (ignoring loc-rib since it
* uses another hook & queue
*/
if (!(bt->afimon[afi][safi] & ~BMP_MON_LOC_RIB))
if (!CHECK_FLAG(bt->afimon[afi][safi], ~BMP_MON_LOC_RIB))
continue;

struct bmp_queue_entry *last_item =
Expand Down Expand Up @@ -2453,6 +2453,9 @@ DEFPY(bmp_stats_cfg,
return CMD_SUCCESS;
}

#define BMP_POLICY_IS_LOCRIB(str) ((str)[0] == 'l') /* __l__oc-rib */
#define BMP_POLICY_IS_PRE(str) ((str)[1] == 'r') /* p__r__e-policy */

DEFPY(bmp_monitor_cfg, bmp_monitor_cmd,
"[no] bmp monitor <ipv4|ipv6|l2vpn> <unicast|multicast|evpn|vpn> <pre-policy|post-policy|loc-rib>$policy",
NO_STR BMP_STR
Expand All @@ -2473,9 +2476,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 (BMP_POLICY_IS_LOCRIB(policy))
flag = BMP_MON_LOC_RIB;
else if (policy[1] == 'r')
else if (BMP_POLICY_IS_PRE(policy))
flag = BMP_MON_PREPOLICY;
else
flag = BMP_MON_POSTPOLICY;
Expand Down Expand Up @@ -2613,15 +2616,17 @@ DEFPY(show_bmp,
continue;

const char *pre_str =
afimon_flag & BMP_MON_PREPOLICY
CHECK_FLAG(afimon_flag,
BMP_MON_PREPOLICY)
? "pre-policy "
: "";
const char *post_str =
afimon_flag & BMP_MON_POSTPOLICY
CHECK_FLAG(afimon_flag,
BMP_MON_POSTPOLICY)
? "post-policy "
: "";
const char *locrib_str =
afimon_flag & BMP_MON_LOC_RIB
CHECK_FLAG(afimon_flag, BMP_MON_LOC_RIB)
? "loc-rib"
: "";

Expand Down Expand Up @@ -2748,14 +2753,16 @@ static int bmp_config_write(struct bgp *bgp, struct vty *vty)
vty_out(vty, " bmp mirror\n");

FOREACH_AFI_SAFI (afi, safi) {
if (bt->afimon[afi][safi] & BMP_MON_PREPOLICY)
if (CHECK_FLAG(bt->afimon[afi][safi],
BMP_MON_PREPOLICY))
vty_out(vty, " bmp monitor %s %s pre-policy\n",
afi2str_lower(afi), safi2str(safi));
if (bt->afimon[afi][safi] & BMP_MON_POSTPOLICY)
if (CHECK_FLAG(bt->afimon[afi][safi],
BMP_MON_POSTPOLICY))
vty_out(vty,
" bmp monitor %s %s post-policy\n",
afi2str_lower(afi), safi2str(safi));
if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB)
if (CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB))
vty_out(vty, " bmp monitor %s %s loc-rib\n",
afi2str(afi), safi2str(safi));
}
Expand Down Expand Up @@ -2806,7 +2813,6 @@ static int bgp_bmp_init(struct event_loop *tm)
return 0;
}

/* 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 *old_route,
Expand All @@ -2816,17 +2822,23 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi,
bool is_withdraw = old_route && !new_route;
struct bgp_path_info *updated_route =
is_withdraw ? old_route : new_route;


/* this should never happen */
if (!updated_route) {
zlog_warn("%s: no updated route found!", __func__);
}

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) {
is_locribmon_enabled |=
(bt->afimon[afi][safi] & BMP_MON_LOC_RIB);

if (is_locribmon_enabled)
if (CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB)) {
is_locribmon_enabled = true;
break;
}
}

if (!is_locribmon_enabled)
Expand All @@ -2845,7 +2857,7 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi,
monotime(NULL);

frr_each (bmp_targets, &bmpbgp->targets, bt) {
if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB) {
if (CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB)) {

struct bmp_queue_entry *last_item = bmp_process_one(
bt, &bt->locupdhash, &bt->locupdlist, bgp, afi,
Expand Down
1 change: 1 addition & 0 deletions doc/developer/subdir.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
dev_RSTFILES = \
doc/developer/bgp-typecodes.rst \
doc/developer/bgpd.rst \
doc/developer/bmp.rst \
doc/developer/building-frr-for-alpine.rst \
doc/developer/building-frr-for-centos6.rst \
doc/developer/building-frr-for-centos7.rst \
Expand Down
2 changes: 1 addition & 1 deletion doc/user/bmp.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down

0 comments on commit 58ecd79

Please sign in to comment.