From 527de3dca21f1428d24d5c3c41111c00b31a1945 Mon Sep 17 00:00:00 2001 From: Pascal Mathis Date: Tue, 12 Jun 2018 18:50:51 +0200 Subject: [PATCH] bgpd: Cleanup peer/AF-flag override implementation This commit cleans up some ugly leftovers from previous flag-override implementation and refactors the AF-flag override implementation to match the same behavior the newly added peer-flag override implementation has. Signed-off-by: Pascal Mathis --- bgpd/bgp_vty.c | 6 +-- bgpd/bgpd.c | 144 +++++++++++++++++++++++-------------------------- 2 files changed, 71 insertions(+), 79 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 046f6f6de7..9857c5b7c8 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -2832,7 +2832,7 @@ static int peer_conf_interface_get(struct vty *vty, const char *conf_if, } if (v6only) - SET_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY); + peer_flag_set(peer, PEER_FLAG_IFPEER_V6ONLY); /* Request zebra to initiate IPv6 RAs on this interface. We do * this @@ -2849,9 +2849,9 @@ static int peer_conf_interface_get(struct vty *vty, const char *conf_if, if ((v6only && !CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY)) || (!v6only && CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY))) { if (v6only) - SET_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY); + peer_flag_set(peer, PEER_FLAG_IFPEER_V6ONLY); else - UNSET_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY); + peer_flag_unset(peer, PEER_FLAG_IFPEER_V6ONLY); /* v6only flag changed. Reset bgp seesion */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index f332c7c9bb..278e5b7868 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -815,6 +815,8 @@ int peer_af_flag_check(struct peer *peer, afi_t afi, safi_t safi, uint32_t flag) void peer_af_flag_inherit(struct peer *peer, afi_t afi, safi_t safi, uint32_t flag) { + bool group_val; + /* Skip if peer is not a peer-group member. */ if (!peer_group_active(peer)) return; @@ -822,11 +824,17 @@ void peer_af_flag_inherit(struct peer *peer, afi_t afi, safi_t safi, /* Unset override flag to signal inheritance from peer-group. */ UNSET_FLAG(peer->af_flags_override[afi][safi], flag); - /* Inherit flag state from peer-group. */ - if (CHECK_FLAG(peer->group->conf->af_flags[afi][safi], flag)) - SET_FLAG(peer->af_flags[afi][safi], flag); + /* + * Inherit flag state from peer-group. If the flag of the peer-group is + * not being inverted, the peer must inherit the inverse of the current + * peer-group flag state. + */ + group_val = CHECK_FLAG(peer->group->conf->af_flags[afi][safi], flag); + if (!CHECK_FLAG(peer->group->conf->af_flags_invert[afi][safi], flag) + && CHECK_FLAG(peer->af_flags_invert[afi][safi], flag)) + COND_FLAG(peer->af_flags[afi][safi], flag, !group_val); else - UNSET_FLAG(peer->af_flags[afi][safi], flag); + COND_FLAG(peer->af_flags[afi][safi], flag, group_val); } static bool peergroup_flag_check(struct peer *peer, uint32_t flag) @@ -1300,7 +1308,6 @@ void peer_xfer_config(struct peer *peer_dst, struct peer *peer_src) /* peer flags apply */ peer_dst->flags = peer_src->flags; - peer_dst->flags_invert = peer_src->flags_invert; peer_dst->cap = peer_src->cap; peer_dst->config = peer_src->config; @@ -1326,8 +1333,6 @@ void peer_xfer_config(struct peer *peer_dst, struct peer *peer_src) FOREACH_AFI_SAFI (afi, safi) { peer_dst->afc[afi][safi] = peer_src->afc[afi][safi]; peer_dst->af_flags[afi][safi] = peer_src->af_flags[afi][safi]; - peer_dst->af_flags_invert[afi][safi] = - peer_src->af_flags_invert[afi][safi]; peer_dst->allowas_in[afi][safi] = peer_src->allowas_in[afi][safi]; peer_dst->weight[afi][safi] = peer_src->weight[afi][safi]; @@ -2430,7 +2435,6 @@ static void peer_group2peer_config_copy(struct peer_group *group, struct peer *peer) { struct peer *conf; - int saved_flags = 0; conf = group->conf; @@ -2449,11 +2453,7 @@ static void peer_group2peer_config_copy(struct peer_group *group, peer->gtsm_hops = conf->gtsm_hops; /* These are per-peer specific flags and so we must preserve them */ - saved_flags |= CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY); - saved_flags |= peer->flags & peer->flags_override; - - peer->flags = conf->flags & ~peer->flags_override; - peer->flags |= saved_flags; + peer->flags |= conf->flags & ~peer->flags_override; peer->flags_invert |= conf->flags_invert; /* peer config apply */ @@ -4064,7 +4064,9 @@ static int peer_flag_modify(struct peer *peer, uint32_t flag, int set) if (CHECK_FLAG(member->flags_override, flag)) continue; - member_invert = CHECK_FLAG(member->flags_invert, flag); + /* Check if only member without group is inverted. */ + member_invert = + CHECK_FLAG(member->flags_invert, flag) && !invert; /* Skip peers with equivalent configuration. */ if (set != member_invert && CHECK_FLAG(member->flags, flag)) @@ -4100,12 +4102,11 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi, int found; int size; int addpath_tx_used; - bool invert; + bool invert, member_invert; + struct bgp *bgp; + struct peer *member; struct listnode *node, *nnode; - struct peer_group *group; struct peer_flag_action action; - struct peer *tmp_peer; - struct bgp *bgp; memset(&action, 0, sizeof(struct peer_flag_action)); size = sizeof peer_af_flag_action_list @@ -4115,7 +4116,7 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi, found = peer_flag_action_set(peer_af_flag_action_list, size, &action, flag); - /* No flag action is found. */ + /* Abort if flag action exists. */ if (!found) return BGP_ERR_INVALID_FLAG; @@ -4133,25 +4134,17 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi, if (flag & PEER_FLAG_AS_OVERRIDE && peer_sort(peer) == BGP_PEER_IBGP) return BGP_ERR_AS_OVERRIDE; - /* When current flag configuration is same as requested one. */ + /* Handle flag updates where desired state matches current state. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { if (set && CHECK_FLAG(peer->af_flags[afi][safi], flag)) { - if (invert) - UNSET_FLAG(peer->af_flags_override[afi][safi], - flag); - else - SET_FLAG(peer->af_flags_override[afi][safi], - flag); + COND_FLAG(peer->af_flags_override[afi][safi], flag, + !invert); return 0; } if (!set && !CHECK_FLAG(peer->af_flags[afi][safi], flag)) { - if (invert) - SET_FLAG(peer->af_flags_override[afi][safi], - flag); - else - UNSET_FLAG(peer->af_flags_override[afi][safi], - flag); + COND_FLAG(peer->af_flags_override[afi][safi], flag, + invert); return 0; } } @@ -4185,22 +4178,11 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi, } } - /* Set/unset flag or inherit from peer-group if appropriate. */ - if (invert) { - if (!set) - UNSET_FLAG(peer->af_flags[afi][safi], flag); - else if (peer_group_active(peer)) - peer_af_flag_inherit(peer, afi, safi, flag); - else - SET_FLAG(peer->af_flags[afi][safi], flag); - } else { - if (set) - SET_FLAG(peer->af_flags[afi][safi], flag); - else if (peer_group_active(peer)) - peer_af_flag_inherit(peer, afi, safi, flag); - else - UNSET_FLAG(peer->af_flags[afi][safi], flag); - } + /* Inherit from peer-group or set/unset flags accordingly. */ + if (peer_group_active(peer) && set == invert) + peer_af_flag_inherit(peer, afi, safi, flag); + else + COND_FLAG(peer->af_flags[afi][safi], flag, set); /* Execute action when peer is established. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) @@ -4221,57 +4203,67 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi, } } - /* Peer group member updates. */ - if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { - group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, tmp_peer)) { - if (CHECK_FLAG(tmp_peer->af_flags_override[afi][safi], + /* Check if handling a regular peer. */ + if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + COND_FLAG(peer->af_flags_override[afi][safi], flag, + set != invert); + } else { + /* + * Update peer-group members, unless they are explicitely + * overriding peer-group configuration. + */ + for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, + member)) { + /* Skip peers with overridden configuration. */ + if (CHECK_FLAG(member->af_flags_override[afi][safi], flag)) continue; - if (set - && CHECK_FLAG(tmp_peer->af_flags[afi][safi], flag)) + /* Check if only member without group is inverted. */ + member_invert = + CHECK_FLAG(member->af_flags_invert[afi][safi], + flag) + && !invert; + + /* Skip peers with equivalent configuration. */ + if (set != member_invert + && CHECK_FLAG(member->af_flags[afi][safi], flag)) continue; - if (!set - && !CHECK_FLAG(tmp_peer->af_flags[afi][safi], flag)) + if (set == member_invert + && !CHECK_FLAG(member->af_flags[afi][safi], flag)) continue; - if (set) - SET_FLAG(tmp_peer->af_flags[afi][safi], flag); - else - UNSET_FLAG(tmp_peer->af_flags[afi][safi], flag); + /* Update flag on peer-group member. */ + COND_FLAG(member->af_flags[afi][safi], flag, + set != member_invert); - if (tmp_peer->status == Established) { + /* Execute flag action on peer-group member. */ + if (member->status == Established) { if (!set && flag == PEER_FLAG_SOFT_RECONFIG) - bgp_clear_adj_in(tmp_peer, afi, safi); + bgp_clear_adj_in(member, afi, safi); else { if (flag == PEER_FLAG_REFLECTOR_CLIENT) - tmp_peer->last_reset = + member->last_reset = PEER_DOWN_RR_CLIENT_CHANGE; else if (flag == PEER_FLAG_RSERVER_CLIENT) - tmp_peer->last_reset = + member->last_reset = PEER_DOWN_RS_CLIENT_CHANGE; else if (flag == PEER_FLAG_ORF_PREFIX_SM) - tmp_peer->last_reset = + member->last_reset = PEER_DOWN_CAPABILITY_CHANGE; else if (flag == PEER_FLAG_ORF_PREFIX_RM) - tmp_peer->last_reset = + member->last_reset = PEER_DOWN_CAPABILITY_CHANGE; - peer_change_action(tmp_peer, afi, safi, + peer_change_action(member, afi, safi, action.type); } } } - } else { - if (set != invert) - SET_FLAG(peer->af_flags_override[afi][safi], flag); - else - UNSET_FLAG(peer->af_flags_override[afi][safi], flag); } /* Track if addpath TX is in use */ @@ -4298,11 +4290,11 @@ static int peer_af_flag_modify(struct peer *peer, afi_t afi, safi_t safi, } } else { for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, - tmp_peer)) { - if (CHECK_FLAG(tmp_peer->af_flags[afi][safi], + member)) { + if (CHECK_FLAG(member->af_flags[afi][safi], PEER_FLAG_ADDPATH_TX_ALL_PATHS) || CHECK_FLAG( - tmp_peer->af_flags[afi][safi], + member->af_flags[afi][safi], PEER_FLAG_ADDPATH_TX_BESTPATH_PER_AS)) { addpath_tx_used = 1; break; -- 2.39.5