From: Pascal Mathis Date: Wed, 13 Jun 2018 00:34:43 +0000 (+0200) Subject: bgpd: Implement group-overrides for peer timers X-Git-Tag: frr-6.1-dev~290^2~2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=b90a8e13eea49404d72dd0cdf94343d8a1831d69;p=matthieu%2Ffrr.git bgpd: Implement group-overrides for peer timers This commit implements BGP peer-group overrides for the timer flags, which control the value of the hold, keepalive, advertisement-interval and connect connect timers. It was kept separated on purpose as the whole timer implementation is quite complex and merging this commit together with with the other flag implementations did not seem right. Basically three new peer flags were introduced, namely *PEER_FLAG_ROUTEADV*, *PEER_FLAG_TIMER* and *PEER_FLAG_TIMER_CONNECT*. The overrides work exactly the same way as they did before, but introducing these flags made a few conditionals simpler as they no longer had to compare internal data structures against eachother. Last but not least, the test suite has been adjusted accordingly to test the newly implemented flag overrides. Signed-off-by: Pascal Mathis --- diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 3f5ff12cbc..2fd63531db 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -194,7 +194,6 @@ static struct peer *peer_xfer_conn(struct peer *from_peer) peer->as = from_peer->as; peer->v_holdtime = from_peer->v_holdtime; peer->v_keepalive = from_peer->v_keepalive; - peer->routeadv = from_peer->routeadv; peer->v_routeadv = from_peer->v_routeadv; peer->v_gr_restart = from_peer->v_gr_restart; peer->cap = from_peer->cap; @@ -1144,7 +1143,7 @@ int bgp_stop(struct peer *peer) } /* Reset keepalive and holdtime */ - if (PEER_OR_GROUP_TIMER_SET(peer)) { + if (CHECK_FLAG(peer->flags, PEER_FLAG_TIMER)) { peer->v_keepalive = peer->keepalive; peer->v_holdtime = peer->holdtime; } else { diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 9ea2f22cec..446dc5ac12 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -503,7 +503,7 @@ void bgp_open_send(struct peer *peer) uint16_t send_holdtime; as_t local_as; - if (PEER_OR_GROUP_TIMER_SET(peer)) + if (CHECK_FLAG(peer->flags, PEER_FLAG_TIMER)) send_holdtime = peer->holdtime; else send_holdtime = peer->bgp->default_holdtime; @@ -1237,7 +1237,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) implementation MAY adjust the rate at which it sends KEEPALIVE messages as a function of the Hold Time interval. */ - if (PEER_OR_GROUP_TIMER_SET(peer)) + if (CHECK_FLAG(peer->flags, PEER_FLAG_TIMER)) send_holdtime = peer->holdtime; else send_holdtime = peer->bgp->default_holdtime; @@ -1247,7 +1247,7 @@ static int bgp_open_receive(struct peer *peer, bgp_size_t size) else peer->v_holdtime = send_holdtime; - if ((PEER_OR_GROUP_TIMER_SET(peer)) + if ((CHECK_FLAG(peer->flags, PEER_FLAG_TIMER)) && (peer->keepalive < peer->v_holdtime / 3)) peer->v_keepalive = peer->keepalive; else diff --git a/bgpd/bgp_snmp.c b/bgpd/bgp_snmp.c index 241b23a62d..0700e0ac24 100644 --- a/bgpd/bgp_snmp.c +++ b/bgpd/bgp_snmp.c @@ -487,17 +487,17 @@ static int write_bgpPeerTable(int action, uint8_t *var_val, return SNMP_ERR_NOSUCHNAME; break; case BGPPEERCONNECTRETRYINTERVAL: - SET_FLAG(peer->config, PEER_CONFIG_CONNECT); + peer_flag_set(peer, PEER_FLAG_TIMER_CONNECT); peer->connect = intval; peer->v_connect = intval; break; case BGPPEERHOLDTIMECONFIGURED: - SET_FLAG(peer->config, PEER_CONFIG_TIMER); + peer_flag_set(peer, PEER_FLAG_TIMER); peer->holdtime = intval; peer->v_holdtime = intval; break; case BGPPEERKEEPALIVECONFIGURED: - SET_FLAG(peer->config, PEER_CONFIG_TIMER); + peer_flag_set(peer, PEER_FLAG_TIMER); peer->keepalive = intval; peer->v_keepalive = intval; break; @@ -617,14 +617,14 @@ static uint8_t *bgpPeerTable(struct variable *v, oid name[], size_t *length, break; case BGPPEERHOLDTIMECONFIGURED: *write_method = write_bgpPeerTable; - if (PEER_OR_GROUP_TIMER_SET(peer)) + if (CHECK_FLAG(peer->flags, PEER_FLAG_TIMER)) return SNMP_INTEGER(peer->holdtime); else return SNMP_INTEGER(peer->v_holdtime); break; case BGPPEERKEEPALIVECONFIGURED: *write_method = write_bgpPeerTable; - if (PEER_OR_GROUP_TIMER_SET(peer)) + if (CHECK_FLAG(peer->flags, PEER_FLAG_TIMER)) return SNMP_INTEGER(peer->keepalive); else return SNMP_INTEGER(peer->v_keepalive); diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 9857c5b7c8..57e900d97e 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -9191,8 +9191,7 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, uint8_t use_json, json_object_int_add(json_neigh, "bgpTimerKeepAliveIntervalMsecs", p->v_keepalive * 1000); - - if (PEER_OR_GROUP_TIMER_SET(p)) { + if (CHECK_FLAG(p->flags, PEER_FLAG_TIMER)) { json_object_int_add(json_neigh, "bgpTimerConfiguredHoldTimeMsecs", p->holdtime * 1000); @@ -9256,7 +9255,7 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, uint8_t use_json, vty_out(vty, " Hold time is %d, keepalive interval is %d seconds\n", p->v_holdtime, p->v_keepalive); - if (PEER_OR_GROUP_TIMER_SET(p)) { + if (CHECK_FLAG(p->flags, PEER_FLAG_TIMER)) { vty_out(vty, " Configured hold time is %d", p->holdtime); vty_out(vty, ", keepalive interval is %d seconds\n", diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 278e5b7868..4b150ad4e0 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -993,7 +993,6 @@ static void peer_global_config_reset(struct peer *peer) peer->flags = 0; SET_FLAG(peer->flags, saved_flags); - peer->config = 0; peer->holdtime = 0; peer->keepalive = 0; peer->connect = 0; @@ -1309,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->cap = peer_src->cap; - peer_dst->config = peer_src->config; peer_dst->local_as = peer_src->local_as; peer_dst->port = peer_src->port; @@ -1576,10 +1574,9 @@ struct peer *peer_create(union sockunion *su, const char *conf_if, peer->local_id = bgp->router_id; peer->v_holdtime = bgp->default_holdtime; peer->v_keepalive = bgp->default_keepalive; - if (peer_sort(peer) == BGP_PEER_IBGP) - peer->v_routeadv = BGP_DEFAULT_IBGP_ROUTEADV; - else - peer->v_routeadv = BGP_DEFAULT_EBGP_ROUTEADV; + peer->v_routeadv = (peer_sort(peer) == BGP_PEER_IBGP) + ? BGP_DEFAULT_IBGP_ROUTEADV + : BGP_DEFAULT_EBGP_ROUTEADV; peer = peer_lock(peer); /* bgp peer list reference */ peer->group = group; @@ -1655,7 +1652,6 @@ int bgp_afi_safi_peer_exists(struct bgp *bgp, afi_t afi, safi_t safi) void peer_as_change(struct peer *peer, as_t as, int as_specified) { bgp_peer_sort_t type; - struct peer *conf; /* Stop peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { @@ -1678,22 +1674,12 @@ void peer_as_change(struct peer *peer, as_t as, int as_specified) peer->local_as = peer->bgp->as; /* Advertisement-interval reset */ - conf = NULL; - if (peer->group) - conf = peer->group->conf; - - if (conf && CHECK_FLAG(conf->config, PEER_CONFIG_ROUTEADV)) { - peer->v_routeadv = conf->routeadv; - } - /* Only go back to the default advertisement-interval if the user had - * not - * already configured it */ - else if (!CHECK_FLAG(peer->config, PEER_CONFIG_ROUTEADV)) { - if (peer_sort(peer) == BGP_PEER_IBGP) - peer->v_routeadv = BGP_DEFAULT_IBGP_ROUTEADV; - else - peer->v_routeadv = BGP_DEFAULT_EBGP_ROUTEADV; + if (!CHECK_FLAG(peer->flags, PEER_FLAG_ROUTEADV)) { + peer->v_routeadv = (peer_sort(peer) == BGP_PEER_IBGP) + ? BGP_DEFAULT_IBGP_ROUTEADV + : BGP_DEFAULT_EBGP_ROUTEADV; } + /* TTL reset */ if (peer_sort(peer) == BGP_PEER_IBGP) peer->ttl = MAXTTL; @@ -2419,12 +2405,6 @@ struct peer_group *peer_group_get(struct bgp *bgp, const char *name) group->conf->ttl = 1; group->conf->gtsm_hops = 0; group->conf->v_routeadv = BGP_DEFAULT_EBGP_ROUTEADV; - UNSET_FLAG(group->conf->config, PEER_CONFIG_TIMER); - UNSET_FLAG(group->conf->config, PEER_GROUP_CONFIG_TIMER); - UNSET_FLAG(group->conf->config, PEER_CONFIG_CONNECT); - group->conf->keepalive = 0; - group->conf->holdtime = 0; - group->conf->connect = 0; SET_FLAG(group->conf->sflags, PEER_STATUS_GROUP); listnode_add_sort(bgp->group, group); @@ -2456,25 +2436,30 @@ static void peer_group2peer_config_copy(struct peer_group *group, peer->flags |= conf->flags & ~peer->flags_override; peer->flags_invert |= conf->flags_invert; - /* peer config apply */ - peer->config = conf->config; - /* peer timers apply */ - peer->holdtime = conf->holdtime; - peer->keepalive = conf->keepalive; - peer->connect = conf->connect; - if (CHECK_FLAG(conf->config, PEER_CONFIG_CONNECT)) - peer->v_connect = conf->connect; - else - peer->v_connect = BGP_DEFAULT_CONNECT_RETRY; + if (!CHECK_FLAG(peer->flags_override, PEER_FLAG_TIMER)) { + PEER_ATTR_INHERIT(peer, group, holdtime); + PEER_ATTR_INHERIT(peer, group, keepalive); + } - /* advertisement-interval reset */ - if (CHECK_FLAG(conf->config, PEER_CONFIG_ROUTEADV)) - peer->v_routeadv = conf->routeadv; - else if (peer_sort(peer) == BGP_PEER_IBGP) - peer->v_routeadv = BGP_DEFAULT_IBGP_ROUTEADV; - else - peer->v_routeadv = BGP_DEFAULT_EBGP_ROUTEADV; + if (!CHECK_FLAG(peer->flags_override, PEER_FLAG_TIMER_CONNECT)) { + PEER_ATTR_INHERIT(peer, group, connect); + if (CHECK_FLAG(conf->flags, PEER_FLAG_TIMER_CONNECT)) + peer->v_connect = conf->connect; + else + peer->v_connect = BGP_DEFAULT_CONNECT_RETRY; + } + + /* advertisement-interval apply */ + if (!CHECK_FLAG(peer->flags_override, PEER_FLAG_ROUTEADV)) { + PEER_ATTR_INHERIT(peer, group, routeadv); + if (CHECK_FLAG(conf->flags, PEER_FLAG_ROUTEADV)) + peer->v_routeadv = conf->routeadv; + else + peer->v_routeadv = (peer_sort(peer) == BGP_PEER_IBGP) + ? BGP_DEFAULT_IBGP_ROUTEADV + : BGP_DEFAULT_EBGP_ROUTEADV; + } /* password apply */ if (conf->password && !peer->password) @@ -2763,14 +2748,13 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer, if (first_member) { /* Advertisement-interval reset */ - if (!CHECK_FLAG(group->conf->config, - PEER_CONFIG_ROUTEADV)) { - if (peer_sort(group->conf) == BGP_PEER_IBGP) - group->conf->v_routeadv = - BGP_DEFAULT_IBGP_ROUTEADV; - else - group->conf->v_routeadv = - BGP_DEFAULT_EBGP_ROUTEADV; + if (!CHECK_FLAG(group->conf->flags, + PEER_FLAG_ROUTEADV)) { + group->conf->v_routeadv = + (peer_sort(group->conf) + == BGP_PEER_IBGP) + ? BGP_DEFAULT_IBGP_ROUTEADV + : BGP_DEFAULT_EBGP_ROUTEADV; } /* ebgp-multihop reset */ @@ -3854,6 +3838,9 @@ static const struct peer_flag_action peer_flag_action_list[] = { {PEER_FLAG_DISABLE_CONNECTED_CHECK, 0, peer_change_reset}, {PEER_FLAG_CAPABILITY_ENHE, 0, peer_change_reset}, {PEER_FLAG_ENFORCE_FIRST_AS, 0, peer_change_reset_in}, + {PEER_FLAG_ROUTEADV, 0, peer_change_none}, + {PEER_FLAG_TIMER, 0, peer_change_none}, + {PEER_FLAG_TIMER_CONNECT, 0, peer_change_none}, {0, 0, 0}}; static const struct peer_flag_action peer_af_flag_action_list[] = { @@ -4914,44 +4901,40 @@ int peer_weight_unset(struct peer *peer, afi_t afi, safi_t safi) int peer_timers_set(struct peer *peer, uint32_t keepalive, uint32_t holdtime) { - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; - /* keepalive value check. */ if (keepalive > 65535) return BGP_ERR_INVALID_VALUE; - /* Holdtime value check. */ if (holdtime > 65535) return BGP_ERR_INVALID_VALUE; - /* Holdtime value must be either 0 or greater than 3. */ if (holdtime < 3 && holdtime != 0) return BGP_ERR_INVALID_VALUE; - /* Set value to the configuration. */ + /* Set flag and configuration on peer. */ + peer_flag_set(peer, PEER_FLAG_TIMER); peer->holdtime = holdtime; peer->keepalive = (keepalive < holdtime / 3 ? keepalive : holdtime / 3); - /* First work on real peers with timers */ - if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { - SET_FLAG(peer->config, PEER_CONFIG_TIMER); - UNSET_FLAG(peer->config, PEER_GROUP_CONFIG_TIMER); - } else { - /* Now work on the peer-group timers */ - SET_FLAG(peer->config, PEER_GROUP_CONFIG_TIMER); + /* Skip peer-group mechanics for regular peers. */ + if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) + return 0; - /* peer-group member updates. */ - group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, peer)) { - /* Skip peers that have their own timers */ - if (CHECK_FLAG(peer->config, PEER_CONFIG_TIMER)) - continue; + /* + * Set flag and configuration on all 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->flags_override, PEER_FLAG_TIMER)) + continue; - SET_FLAG(peer->config, PEER_GROUP_CONFIG_TIMER); - peer->holdtime = group->conf->holdtime; - peer->keepalive = group->conf->keepalive; - } + /* Set flag and configuration on peer-group member. */ + SET_FLAG(member->flags, PEER_FLAG_TIMER); + PEER_ATTR_INHERIT(peer, peer->group, holdtime); + PEER_ATTR_INHERIT(peer, peer->group, keepalive); } return 0; @@ -4959,35 +4942,38 @@ int peer_timers_set(struct peer *peer, uint32_t keepalive, uint32_t holdtime) int peer_timers_unset(struct peer *peer) { - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; - /* First work on real peers vs the peer-group */ - if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { - UNSET_FLAG(peer->config, PEER_CONFIG_TIMER); - peer->keepalive = 0; + /* Inherit configuration from peer-group if peer is member. */ + if (peer_group_active(peer)) { + peer_flag_inherit(peer, PEER_FLAG_TIMER); + PEER_ATTR_INHERIT(peer, peer->group, holdtime); + PEER_ATTR_INHERIT(peer, peer->group, keepalive); + } else { + /* Otherwise remove flag and configuration from peer. */ + peer_flag_unset(peer, PEER_FLAG_TIMER); peer->holdtime = 0; + peer->keepalive = 0; + } - if (peer->group && peer->group->conf->holdtime) { - SET_FLAG(peer->config, PEER_GROUP_CONFIG_TIMER); - peer->keepalive = peer->group->conf->keepalive; - peer->holdtime = peer->group->conf->holdtime; - } - } else { - /* peer-group member updates. */ - group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, peer)) { - if (!CHECK_FLAG(peer->config, PEER_CONFIG_TIMER)) { - UNSET_FLAG(peer->config, - PEER_GROUP_CONFIG_TIMER); - peer->holdtime = 0; - peer->keepalive = 0; - } - } + /* Skip peer-group mechanics for regular peers. */ + if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) + return 0; + + /* + * Remove flag and configuration from all 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->flags_override, PEER_FLAG_TIMER)) + continue; - UNSET_FLAG(group->conf->config, PEER_GROUP_CONFIG_TIMER); - group->conf->holdtime = 0; - group->conf->keepalive = 0; + /* Remove flag and configuration on peer-group member. */ + UNSET_FLAG(member->flags, PEER_FLAG_TIMER); + member->holdtime = 0; + member->keepalive = 0; } return 0; @@ -4995,85 +4981,124 @@ int peer_timers_unset(struct peer *peer) int peer_timers_connect_set(struct peer *peer, uint32_t connect) { - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; if (connect > 65535) return BGP_ERR_INVALID_VALUE; - /* Set value to the configuration. */ - SET_FLAG(peer->config, PEER_CONFIG_CONNECT); + /* Set flag and configuration on peer. */ + peer_flag_set(peer, PEER_FLAG_TIMER_CONNECT); peer->connect = connect; - - /* Set value to timer setting. */ peer->v_connect = connect; + /* Skip peer-group mechanics for regular peers. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) return 0; - /* peer-group member updates. */ - group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, peer)) { - SET_FLAG(peer->config, PEER_CONFIG_CONNECT); - peer->connect = connect; - peer->v_connect = connect; + /* + * Set flag and configuration on all 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->flags_override, PEER_FLAG_TIMER_CONNECT)) + continue; + + /* Set flag and configuration on peer-group member. */ + SET_FLAG(member->flags, PEER_FLAG_TIMER_CONNECT); + member->connect = connect; + member->v_connect = connect; } + return 0; } int peer_timers_connect_unset(struct peer *peer) { - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; - /* Clear configuration. */ - UNSET_FLAG(peer->config, PEER_CONFIG_CONNECT); - peer->connect = 0; + /* Inherit configuration from peer-group if peer is member. */ + if (peer_group_active(peer)) { + peer_flag_inherit(peer, PEER_FLAG_TIMER_CONNECT); + PEER_ATTR_INHERIT(peer, peer->group, connect); + } else { + /* Otherwise remove flag and configuration from peer. */ + peer_flag_unset(peer, PEER_FLAG_TIMER_CONNECT); + peer->connect = 0; + } - /* Set timer setting to default value. */ - peer->v_connect = BGP_DEFAULT_CONNECT_RETRY; + /* Set timer with fallback to default value. */ + if (peer->connect) + peer->v_connect = peer->connect; + else + peer->v_connect = BGP_DEFAULT_CONNECT_RETRY; + /* Skip peer-group mechanics for regular peers. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) return 0; - /* peer-group member updates. */ - group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, peer)) { - UNSET_FLAG(peer->config, PEER_CONFIG_CONNECT); - peer->connect = 0; - peer->v_connect = BGP_DEFAULT_CONNECT_RETRY; + /* + * Remove flag and configuration from all 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->flags_override, PEER_FLAG_TIMER_CONNECT)) + continue; + + /* Remove flag and configuration on peer-group member. */ + UNSET_FLAG(member->flags, PEER_FLAG_TIMER_CONNECT); + member->connect = 0; + member->v_connect = BGP_DEFAULT_CONNECT_RETRY; } + return 0; } int peer_advertise_interval_set(struct peer *peer, uint32_t routeadv) { - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; if (routeadv > 600) return BGP_ERR_INVALID_VALUE; - SET_FLAG(peer->config, PEER_CONFIG_ROUTEADV); + /* Set flag and configuration on peer. */ + peer_flag_set(peer, PEER_FLAG_ROUTEADV); peer->routeadv = routeadv; peer->v_routeadv = routeadv; + /* Check if handling a regular peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Update peer route announcements. */ update_group_adjust_peer_afs(peer); if (peer->status == Established) bgp_announce_route_all(peer); + + /* Skip peer-group mechanics for regular peers. */ return 0; } - /* peer-group member updates. */ - group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, peer)) { - SET_FLAG(peer->config, PEER_CONFIG_ROUTEADV); - peer->routeadv = routeadv; - peer->v_routeadv = routeadv; - update_group_adjust_peer_afs(peer); - if (peer->status == Established) - bgp_announce_route_all(peer); + /* + * Set flag and configuration on all 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->flags_override, PEER_FLAG_ROUTEADV)) + continue; + + /* Set flag and configuration on peer-group member. */ + SET_FLAG(member->flags, PEER_FLAG_ROUTEADV); + member->routeadv = routeadv; + member->v_routeadv = routeadv; + + /* Update peer route announcements. */ + update_group_adjust_peer_afs(member); + if (member->status == Established) + bgp_announce_route_all(member); } return 0; @@ -5081,38 +5106,58 @@ int peer_advertise_interval_set(struct peer *peer, uint32_t routeadv) int peer_advertise_interval_unset(struct peer *peer) { - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; - UNSET_FLAG(peer->config, PEER_CONFIG_ROUTEADV); - peer->routeadv = 0; + /* Inherit configuration from peer-group if peer is member. */ + if (peer_group_active(peer)) { + peer_flag_inherit(peer, PEER_FLAG_ROUTEADV); + PEER_ATTR_INHERIT(peer, peer->group, routeadv); + } else { + /* Otherwise remove flag and configuration from peer. */ + peer_flag_unset(peer, PEER_FLAG_ROUTEADV); + peer->routeadv = 0; + } - if (peer->sort == BGP_PEER_IBGP) - peer->v_routeadv = BGP_DEFAULT_IBGP_ROUTEADV; + /* Set timer with fallback to default value. */ + if (peer->routeadv) + peer->v_routeadv = peer->routeadv; else - peer->v_routeadv = BGP_DEFAULT_EBGP_ROUTEADV; + peer->v_routeadv = (peer->sort == BGP_PEER_IBGP) + ? BGP_DEFAULT_IBGP_ROUTEADV + : BGP_DEFAULT_EBGP_ROUTEADV; + /* Check if handling a regular peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Update peer route announcements. */ update_group_adjust_peer_afs(peer); if (peer->status == Established) bgp_announce_route_all(peer); + + /* Skip peer-group mechanics for regular peers. */ return 0; } - /* peer-group member updates. */ - group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, peer)) { - UNSET_FLAG(peer->config, PEER_CONFIG_ROUTEADV); - peer->routeadv = 0; + /* + * Remove flag and configuration from all 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->flags_override, PEER_FLAG_ROUTEADV)) + continue; - if (peer->sort == BGP_PEER_IBGP) - peer->v_routeadv = BGP_DEFAULT_IBGP_ROUTEADV; - else - peer->v_routeadv = BGP_DEFAULT_EBGP_ROUTEADV; + /* Remove flag and configuration on peer-group member. */ + UNSET_FLAG(member->flags, PEER_FLAG_ROUTEADV); + member->routeadv = 0; + member->v_routeadv = (member->sort == BGP_PEER_IBGP) + ? BGP_DEFAULT_IBGP_ROUTEADV + : BGP_DEFAULT_EBGP_ROUTEADV; - update_group_adjust_peer_afs(peer); - if (peer->status == Established) - bgp_announce_route_all(peer); + /* Update peer route announcements. */ + update_group_adjust_peer_afs(member); + if (member->status == Established) + bgp_announce_route_all(member); } return 0; @@ -7000,37 +7045,19 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp, } /* advertisement-interval */ - if (CHECK_FLAG(peer->config, PEER_CONFIG_ROUTEADV) - && ((!peer_group_active(peer) - && peer->v_routeadv != BGP_DEFAULT_EBGP_ROUTEADV) - || (peer_group_active(peer) - && peer->v_routeadv != g_peer->v_routeadv))) { + if (peergroup_flag_check(peer, PEER_FLAG_ROUTEADV)) vty_out(vty, " neighbor %s advertisement-interval %u\n", addr, - peer->v_routeadv); - } + peer->routeadv); /* timers */ - if ((PEER_OR_GROUP_TIMER_SET(peer)) - && ((!peer_group_active(peer) - && (peer->keepalive != BGP_DEFAULT_KEEPALIVE - || peer->holdtime != BGP_DEFAULT_HOLDTIME)) - || (peer_group_active(peer) - && (peer->keepalive != g_peer->keepalive - || peer->holdtime != g_peer->holdtime)))) { + if (peergroup_flag_check(peer, PEER_FLAG_TIMER)) vty_out(vty, " neighbor %s timers %u %u\n", addr, peer->keepalive, peer->holdtime); - } - if (CHECK_FLAG(peer->config, PEER_CONFIG_CONNECT) - && ((!peer_group_active(peer) - && peer->connect != BGP_DEFAULT_CONNECT_RETRY) - || (peer_group_active(peer) - && peer->connect != g_peer->connect))) - - { + /* timers connect */ + if (peergroup_flag_check(peer, PEER_FLAG_TIMER_CONNECT)) vty_out(vty, " neighbor %s timers connect %u\n", addr, peer->connect); - } /* capability dynamic */ if (peergroup_flag_check(peer, PEER_FLAG_DYNAMIC_CAPABILITY)) diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index fdc363d1f4..3ce309db6f 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -895,6 +895,9 @@ struct peer { #define PEER_FLAG_IFPEER_V6ONLY (1 << 14) /* if-based peer is v6 only */ #define PEER_FLAG_IS_RFAPI_HD (1 << 15) /* attached to rfapi HD */ #define PEER_FLAG_ENFORCE_FIRST_AS (1 << 16) /* enforce-first-as */ +#define PEER_FLAG_ROUTEADV (1 << 17) /* route advertise */ +#define PEER_FLAG_TIMER (1 << 18) /* keepalive & holdtime */ +#define PEER_FLAG_TIMER_CONNECT (1 << 19) /* connect timer */ /* outgoing message sent in CEASE_ADMIN_SHUTDOWN notify */ char *tx_shutdown_message; @@ -968,17 +971,7 @@ struct peer { #define PEER_STATUS_EOR_SEND (1 << 4) /* end-of-rib send to peer */ #define PEER_STATUS_EOR_RECEIVED (1 << 5) /* end-of-rib received from peer */ - /* Default attribute value for the peer. */ - uint32_t config; -#define PEER_CONFIG_TIMER (1 << 0) /* keepalive & holdtime */ -#define PEER_CONFIG_CONNECT (1 << 1) /* connect */ -#define PEER_CONFIG_ROUTEADV (1 << 2) /* route advertise */ -#define PEER_GROUP_CONFIG_TIMER (1 << 3) /* timers from peer-group */ - -#define PEER_OR_GROUP_TIMER_SET(peer) \ - (CHECK_FLAG(peer->config, PEER_CONFIG_TIMER) \ - || CHECK_FLAG(peer->config, PEER_GROUP_CONFIG_TIMER)) - + /* Configured timer values. */ _Atomic uint32_t holdtime; _Atomic uint32_t keepalive; _Atomic uint32_t connect; diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c index b61e13c4b4..5e09bf61a9 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -183,17 +183,6 @@ static char *str_printf(const char *fmt, ...) return buf; } -static void _test_advertisement_interval(struct test *test, struct peer *peer, - struct peer *group, bool peer_set, - bool group_set) -{ - uint32_t def = BGP_DEFAULT_EBGP_ROUTEADV; - - TEST_ASSERT_EQ(test, peer->v_routeadv, - (peer_set || group_set) ? 10 : def); - TEST_ASSERT_EQ(test, group->v_routeadv, group_set ? 20 : def); -} - /* clang-format off */ static struct test_peer_attr test_peer_attrs[] = { /* Peer Attributes */ @@ -201,8 +190,8 @@ static struct test_peer_attr test_peer_attrs[] = { .cmd = "advertisement-interval", .peer_cmd = "advertisement-interval 10", .group_cmd = "advertisement-interval 20", - .type = PEER_AT_GLOBAL_CUSTOM, - .custom_handler = &_test_advertisement_interval, + .u.flag = PEER_FLAG_ROUTEADV, + .type = PEER_AT_GLOBAL_FLAG, }, { .cmd = "capability dynamic", @@ -256,6 +245,20 @@ static struct test_peer_attr test_peer_attrs[] = { .u.flag = PEER_FLAG_STRICT_CAP_MATCH, .type = PEER_AT_GLOBAL_FLAG, }, + { + .cmd = "timers", + .peer_cmd = "timers 10 30", + .group_cmd = "timers 20 60", + .u.flag = PEER_FLAG_TIMER, + .type = PEER_AT_GLOBAL_FLAG, + }, + { + .cmd = "timers connect", + .peer_cmd = "timers connect 10", + .group_cmd = "timers connect 20", + .u.flag = PEER_FLAG_TIMER_CONNECT, + .type = PEER_AT_GLOBAL_FLAG, + }, /* Address Family Attributes */ { @@ -541,6 +544,11 @@ static const char *str_from_attr_type(enum test_peer_attr_type at) } } +static bool is_attr_type_global(enum test_peer_attr_type at) +{ + return at == PEER_AT_GLOBAL_FLAG || at == PEER_AT_GLOBAL_CUSTOM; +} + static void test_log(struct test *test, const char *fmt, ...) { va_list ap; @@ -944,7 +952,6 @@ static void test_process(struct test *test, struct test_peer_attr *pa, static void test_peer_attr(struct test *test, struct test_peer_attr *pa) { int tc = 1; - bool is_address_family; const char *type; const char *ecp = pa->o.invert_peer ? "no " : ""; const char *dcp = pa->o.invert_peer ? "" : "no "; @@ -964,12 +971,8 @@ static void test_peer_attr(struct test *test, struct test_peer_attr *pa) return; } - is_address_family = - (pa->type == PEER_AT_AF_FLAG || pa->type == PEER_AT_AF_FILTER - || pa->type == PEER_AT_AF_CUSTOM); - /* Test Preparation: Switch and activate address-family. */ - if (is_address_family) { + if (!is_attr_type_global(pa->type)) { test_log(test, "prepare: switch address-family to [%s]", afi_safi_print(pa->afi, pa->safi)); test_execute(test, "address-family %s %s", @@ -1026,7 +1029,7 @@ static void test_peer_attr(struct test *test, struct test_peer_attr *pa) g = test->group; /* Test Preparation: Switch and activate address-family. */ - if (is_address_family) { + if (!is_attr_type_global(pa->type)) { test_log(test, "prepare: switch address-family to [%s]", afi_safi_print(pa->afi, pa->safi)); test_execute(test, "address-family %s %s", @@ -1193,7 +1196,7 @@ int main(void) pa = &test_peer_attrs[i++]; /* Just copy the peer attribute structure for global flags. */ - if (pa->type == PEER_AT_GLOBAL_FLAG) { + if (is_attr_type_global(pa->type)) { pac = XMALLOC(MTYPE_TMP, sizeof(struct test_peer_attr)); memcpy(pac, pa, sizeof(struct test_peer_attr)); listnode_add(pa_list, pac);