Skip to content

Commit

Permalink
bgpd: fixing strict aliasing warning in bmp_get_peer_distinguisher
Browse files Browse the repository at this point in the history
bmp_get_peer_distinguisher warning fix
bmp_get_peer_distinguisher use of flag check for prefix_rd

Signed-off-by: Maxence Younsi <[email protected]>
  • Loading branch information
mxyns committed Aug 15, 2022
1 parent c79198f commit fc08e45
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 42 deletions.
76 changes: 49 additions & 27 deletions bgpd/bgp_bmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -262,25 +262,35 @@ static inline uint64_t bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi,
uint8_t peer_type)
{

// remove this check when the other peer types get correct peer dist.
// (RFC7854) impl.
/* remove this check when the other peer types get correct peer dist.
*(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
* if yes => use RD value
* else => use vrf_id and 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;
}

return ((uint64_t)htonl(bgp->vrf_id)) << 32;
}

static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type)
Expand Down Expand Up @@ -317,7 +327,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->su.sa.sa_family != AF_INET6 &&
peer->su.sa.sa_family != AF_INET)) {
stream_putl(s, 0);
Expand All @@ -335,15 +346,19 @@ 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 */
Expand Down Expand Up @@ -1243,8 +1258,9 @@ static inline struct bmp_queue_entry *bmp_pull_locrib(struct bmp *bmp)
&bmp->locrib_queuepos);
}

// TODO BMP_MON_LOCRIB find a way to merge properly this function with
// bmp_wrqueue or abstract it if possible
/* TODO BMP_MON_LOCRIB find a way to merge properly this function with
* bmp_wrqueue or abstract it if possible
*/
static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr)
{

Expand All @@ -1260,9 +1276,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:
Expand All @@ -1275,7 +1290,8 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr)
break;

/* currently syncing & haven't reached this prefix yet
* => it'll be sent as part of the table sync, no need here */
* => 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;
Expand All @@ -1297,10 +1313,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_afi_node_lookup(bmp->targets->bgp->rib[afi][safi], afi, safi,
&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))
Expand Down Expand Up @@ -1489,7 +1507,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,
Expand All @@ -1512,16 +1531,18 @@ 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;

struct bmp_queue_entry *last_item =
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) {
Expand Down Expand Up @@ -2442,9 +2463,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;
Expand Down Expand Up @@ -2793,8 +2814,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) {
Expand Down
7 changes: 3 additions & 4 deletions bgpd/bgp_bmp.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,15 +229,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)
#define BMP_MON_LOC_RIB (1 << 2)

// TODO define BMP_MON_LOC_RIB flag
#define BMP_MON_LOC_RIB (1 << 2)
uint8_t afimon[AFI_MAX][SAFI_MAX];
bool mirror;

Expand Down
6 changes: 4 additions & 2 deletions bgpd/bgp_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ DEFINE_HOOK(bgp_rpki_prefix_status,
(peer, attr, prefix));

DEFINE_HOOK(bgp_route_update,
(struct bgp * bgp, afi_t afi, safi_t safi, struct bgp_dest *bn,
(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));

Expand Down Expand Up @@ -3150,7 +3150,8 @@ 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 */
Expand All @@ -3159,6 +3160,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest,
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);
}
Expand Down
2 changes: 1 addition & 1 deletion bgpd/bgp_route.h
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ 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 *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));

Expand Down
16 changes: 8 additions & 8 deletions doc/user/bmp.rst
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 All @@ -61,12 +61,12 @@ 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.
Expand All @@ -87,7 +87,7 @@ Unsupported features (should be exhaustive):
- Peer Up/Down

- VRF/Table Name TLV

- code for TLV exists
- need better RFC understanding

Expand All @@ -100,8 +100,8 @@ Unsupported features (should be exhaustive):
- 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
============

Expand Down Expand Up @@ -194,7 +194,7 @@ associated with a particular ``bmp targets``:
.. clicmd:: bmp monitor AFI SAFI <pre-policy|post-policy|loc-rib>

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.

Expand Down

0 comments on commit fc08e45

Please sign in to comment.