From: Pascal Mathis Date: Wed, 13 Jun 2018 17:34:17 +0000 (+0200) Subject: bgpd: Implement group-overrides for peer attrs X-Git-Tag: frr-6.1-dev~290^2~1 X-Git-Url: https://git.proxmox.com/?a=commitdiff_plain;h=a14810f43f6a7970468eb2568385ca75c4708e4d;p=mirror_frr.git bgpd: Implement group-overrides for peer attrs This commit introduces BGP peer-group overrides for the last set of peer-level attrs which did not offer that feature yet. The following attributes have been implemented: description, local-as, password and update-source. Each attribute, with the exception of description because it does not offer any inheritance between peer-groups and peers, is now also setting a peer-flag instead of just modifying the internal data structures. This made it possible to also re-use the same implementation for attribute overrides as already done for peer flags, AF flags and AF attrs. The `no neighbor description` command has been slightly changed to support negation for no parameters, one parameter or * parameters (LINE...). This was needed for the test suite to pass and is a small change without any bigger impact on the CLI. Signed-off-by: Pascal Mathis --- diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 57e900d97..f0fc3a89e 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -4655,12 +4655,11 @@ DEFUN (neighbor_description, DEFUN (no_neighbor_description, no_neighbor_description_cmd, - "no neighbor description [LINE]", + "no neighbor description", NO_STR NEIGHBOR_STR NEIGHBOR_ADDR_STR2 - "Neighbor specific description\n" - "Up to 80 characters describing this neighbor\n") + "Neighbor specific description\n") { int idx_peer = 2; struct peer *peer; @@ -4674,6 +4673,11 @@ DEFUN (no_neighbor_description, return CMD_SUCCESS; } +ALIAS(no_neighbor_description, no_neighbor_description_comment_cmd, + "no neighbor description LINE...", + NO_STR NEIGHBOR_STR NEIGHBOR_ADDR_STR2 + "Neighbor specific description\n" + "Up to 80 characters describing this neighbor\n") /* Neighbor update-source. */ static int peer_update_source_vty(struct vty *vty, const char *peer_str, @@ -4681,6 +4685,7 @@ static int peer_update_source_vty(struct vty *vty, const char *peer_str, { struct peer *peer; struct prefix p; + union sockunion su; peer = peer_and_group_lookup_vty(vty, peer_str); if (!peer) @@ -4690,10 +4695,7 @@ static int peer_update_source_vty(struct vty *vty, const char *peer_str, return CMD_WARNING; if (source_str) { - union sockunion su; - int ret = str2sockunion(source_str, &su); - - if (ret == 0) + if (str2sockunion(source_str, &su) == 0) peer_update_source_addr_set(peer, &su); else { if (str2prefix(source_str, &p)) { @@ -13030,6 +13032,7 @@ void bgp_vty_init(void) /* "neighbor description" commands. */ install_element(BGP_NODE, &neighbor_description_cmd); install_element(BGP_NODE, &no_neighbor_description_cmd); + install_element(BGP_NODE, &no_neighbor_description_comment_cmd); /* "neighbor update-source" commands. "*/ install_element(BGP_NODE, &neighbor_update_source_cmd); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 4b150ad4e..ecd15b9d9 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1494,10 +1494,12 @@ void bgp_peer_conf_if_to_su_update(struct peer *peer) * needed. */ if (peer_addr_updated) { - if (peer->password && prev_family == AF_UNSPEC) + if (CHECK_FLAG(peer->flags, PEER_FLAG_PASSWORD) + && prev_family == AF_UNSPEC) bgp_md5_set(peer); } else { - if (peer->password && prev_family != AF_UNSPEC) + if (CHECK_FLAG(peer->flags, PEER_FLAG_PASSWORD) + && prev_family != AF_UNSPEC) bgp_md5_unset(peer); peer->su.sa.sa_family = AF_UNSPEC; memset(&peer->su.sin6.sin6_addr, 0, sizeof(struct in6_addr)); @@ -1719,8 +1721,9 @@ void peer_as_change(struct peer *peer, as_t as, int as_specified) /* local-as reset */ if (peer_sort(peer) != BGP_PEER_EBGP) { peer->change_local_as = 0; - UNSET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); - UNSET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); + peer_flag_unset(peer, PEER_FLAG_LOCAL_AS); + peer_flag_unset(peer, PEER_FLAG_LOCAL_AS_NO_PREPEND); + peer_flag_unset(peer, PEER_FLAG_LOCAL_AS_REPLACE_AS); } } @@ -2230,9 +2233,8 @@ int peer_delete(struct peer *peer) bgp_unlink_nexthop_by_peer(peer); /* Password configuration */ - if (peer->password) { + if (CHECK_FLAG(peer->flags, PEER_FLAG_PASSWORD)) { XFREE(MTYPE_PEER_PASSWORD, peer->password); - peer->password = NULL; if (!accept_peer && !BGP_PEER_SU_UNSPEC(peer) && !CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) @@ -2422,8 +2424,8 @@ static void peer_group2peer_config_copy(struct peer_group *group, if (conf->as) peer->as = conf->as; - /* remote-as */ - if (conf->change_local_as) + /* local-as */ + if (!CHECK_FLAG(peer->flags_override, PEER_FLAG_LOCAL_AS)) peer->change_local_as = conf->change_local_as; /* TTL */ @@ -2462,30 +2464,23 @@ static void peer_group2peer_config_copy(struct peer_group *group, } /* password apply */ - if (conf->password && !peer->password) - peer->password = XSTRDUP(MTYPE_PEER_PASSWORD, conf->password); + if (!CHECK_FLAG(peer->flags_override, PEER_FLAG_PASSWORD)) + PEER_STR_ATTR_INHERIT(peer, group, password, + MTYPE_PEER_PASSWORD); if (!BGP_PEER_SU_UNSPEC(peer)) bgp_md5_set(peer); /* update-source apply */ - if (conf->update_source) { - if (peer->update_source) - sockunion_free(peer->update_source); - if (peer->update_if) { + if (!CHECK_FLAG(peer->flags_override, PEER_FLAG_UPDATE_SOURCE)) { + if (conf->update_source) { XFREE(MTYPE_PEER_UPDATE_SOURCE, peer->update_if); - peer->update_if = NULL; - } - peer->update_source = sockunion_dup(conf->update_source); - } else if (conf->update_if) { - if (peer->update_if) - XFREE(MTYPE_PEER_UPDATE_SOURCE, peer->update_if); - if (peer->update_source) { + PEER_SU_ATTR_INHERIT(peer, group, update_source); + } else if (conf->update_if) { sockunion_free(peer->update_source); - peer->update_source = NULL; + PEER_STR_ATTR_INHERIT(peer, group, update_if, + MTYPE_PEER_UPDATE_SOURCE); } - peer->update_if = - XSTRDUP(MTYPE_PEER_UPDATE_SOURCE, conf->update_if); } bgp_bfd_peer_group2peer_copy(conf, peer); @@ -2764,10 +2759,12 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer, /* local-as reset */ if (peer_sort(group->conf) != BGP_PEER_EBGP) { group->conf->change_local_as = 0; - UNSET_FLAG(peer->flags, - PEER_FLAG_LOCAL_AS_NO_PREPEND); - UNSET_FLAG(peer->flags, - PEER_FLAG_LOCAL_AS_REPLACE_AS); + peer_flag_unset(group->conf, + PEER_FLAG_LOCAL_AS); + peer_flag_unset(group->conf, + PEER_FLAG_LOCAL_AS_NO_PREPEND); + peer_flag_unset(group->conf, + PEER_FLAG_LOCAL_AS_REPLACE_AS); } } @@ -3841,6 +3838,11 @@ static const struct peer_flag_action peer_flag_action_list[] = { {PEER_FLAG_ROUTEADV, 0, peer_change_none}, {PEER_FLAG_TIMER, 0, peer_change_none}, {PEER_FLAG_TIMER_CONNECT, 0, peer_change_none}, + {PEER_FLAG_PASSWORD, 0, peer_change_none}, + {PEER_FLAG_LOCAL_AS, 0, peer_change_none}, + {PEER_FLAG_LOCAL_AS_NO_PREPEND, 0, peer_change_none}, + {PEER_FLAG_LOCAL_AS_REPLACE_AS, 0, peer_change_none}, + {PEER_FLAG_UPDATE_SOURCE, 0, peer_change_none}, {0, 0, 0}}; static const struct peer_flag_action peer_af_flag_action_list[] = { @@ -4447,183 +4449,195 @@ int peer_description_unset(struct peer *peer) /* Neighbor update-source. */ int peer_update_source_if_set(struct peer *peer, const char *ifname) { - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; + /* Set flag and configuration on peer. */ + peer_flag_set(peer, PEER_FLAG_UPDATE_SOURCE); if (peer->update_if) { - if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) - && strcmp(peer->update_if, ifname) == 0) + if (strcmp(peer->update_if, ifname) == 0) return 0; - XFREE(MTYPE_PEER_UPDATE_SOURCE, peer->update_if); - peer->update_if = NULL; - } - - if (peer->update_source) { - sockunion_free(peer->update_source); - peer->update_source = NULL; } - peer->update_if = XSTRDUP(MTYPE_PEER_UPDATE_SOURCE, ifname); + sockunion_free(peer->update_source); + peer->update_source = NULL; + /* Check if handling a regular peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; bgp_notify_send(peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else bgp_session_reset(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)) { - if (peer->update_if) { - if (strcmp(peer->update_if, ifname) == 0) - continue; - - XFREE(MTYPE_PEER_UPDATE_SOURCE, peer->update_if); - peer->update_if = NULL; - } + /* + * 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_UPDATE_SOURCE)) + continue; - if (peer->update_source) { - sockunion_free(peer->update_source); - peer->update_source = NULL; + /* Skip peers with the same configuration. */ + if (member->update_if) { + if (strcmp(member->update_if, ifname) == 0) + continue; + XFREE(MTYPE_PEER_UPDATE_SOURCE, member->update_if); } - peer->update_if = XSTRDUP(MTYPE_PEER_UPDATE_SOURCE, ifname); - - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { - peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; - bgp_notify_send(peer, BGP_NOTIFY_CEASE, + /* Set flag and configuration on peer-group member. */ + SET_FLAG(member->flags, PEER_FLAG_UPDATE_SOURCE); + member->update_if = XSTRDUP(MTYPE_PEER_UPDATE_SOURCE, ifname); + sockunion_free(member->update_source); + member->update_source = NULL; + + /* Send notification or reset peer depending on state. */ + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->status)) { + member->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; + bgp_notify_send(member, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - bgp_session_reset(peer); + bgp_session_reset(member); } + return 0; } int peer_update_source_addr_set(struct peer *peer, const union sockunion *su) { - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; + /* Set flag and configuration on peer. */ + peer_flag_set(peer, PEER_FLAG_UPDATE_SOURCE); if (peer->update_source) { - if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) - && sockunion_cmp(peer->update_source, su) == 0) + if (sockunion_cmp(peer->update_source, su) == 0) return 0; sockunion_free(peer->update_source); - peer->update_source = NULL; } - - if (peer->update_if) { - XFREE(MTYPE_PEER_UPDATE_SOURCE, peer->update_if); - peer->update_if = NULL; - } - peer->update_source = sockunion_dup(su); + XFREE(MTYPE_PEER_UPDATE_SOURCE, peer->update_if); + /* Check if handling a regular peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; bgp_notify_send(peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else bgp_session_reset(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)) { - if (peer->update_source) { - if (sockunion_cmp(peer->update_source, su) == 0) - continue; - sockunion_free(peer->update_source); - peer->update_source = NULL; - } + /* + * 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_UPDATE_SOURCE)) + continue; - if (peer->update_if) { - XFREE(MTYPE_PEER_UPDATE_SOURCE, peer->update_if); - peer->update_if = NULL; + /* Skip peers with the same configuration. */ + if (member->update_source) { + if (sockunion_cmp(member->update_source, su) == 0) + continue; + sockunion_free(member->update_source); } - peer->update_source = sockunion_dup(su); - - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { - peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; - bgp_notify_send(peer, BGP_NOTIFY_CEASE, + /* Set flag and configuration on peer-group member. */ + SET_FLAG(member->flags, PEER_FLAG_UPDATE_SOURCE); + member->update_source = sockunion_dup(su); + XFREE(MTYPE_PEER_UPDATE_SOURCE, member->update_if); + + /* Send notification or reset peer depending on state. */ + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->status)) { + member->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; + bgp_notify_send(member, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - bgp_session_reset(peer); + bgp_session_reset(member); } + return 0; } int peer_update_source_unset(struct peer *peer) { - union sockunion *su; - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; - if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) && !peer->update_source - && !peer->update_if) + if (!CHECK_FLAG(peer->flags, PEER_FLAG_UPDATE_SOURCE)) return 0; - if (peer->update_source) { + /* Inherit configuration from peer-group if peer is member. */ + if (peer_group_active(peer)) { + peer_flag_inherit(peer, PEER_FLAG_UPDATE_SOURCE); + PEER_SU_ATTR_INHERIT(peer, peer->group, update_source); + PEER_STR_ATTR_INHERIT(peer, peer->group, update_if, + MTYPE_PEER_UPDATE_SOURCE); + } else { + /* Otherwise remove flag and configuration from peer. */ + peer_flag_unset(peer, PEER_FLAG_UPDATE_SOURCE); sockunion_free(peer->update_source); peer->update_source = NULL; - } - if (peer->update_if) { XFREE(MTYPE_PEER_UPDATE_SOURCE, peer->update_if); - peer->update_if = NULL; - } - - if (peer_group_active(peer)) { - group = peer->group; - - if (group->conf->update_source) { - su = sockunion_dup(group->conf->update_source); - peer->update_source = su; - } else if (group->conf->update_if) - peer->update_if = XSTRDUP(MTYPE_PEER_UPDATE_SOURCE, - group->conf->update_if); } + /* Check if handling a regular peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; bgp_notify_send(peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else bgp_session_reset(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)) { - if (!peer->update_source && !peer->update_if) + /* + * 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_UPDATE_SOURCE)) continue; - if (peer->update_source) { - sockunion_free(peer->update_source); - peer->update_source = NULL; - } - - if (peer->update_if) { - XFREE(MTYPE_PEER_UPDATE_SOURCE, peer->update_if); - peer->update_if = NULL; - } + /* Skip peers with the same configuration. */ + if (!CHECK_FLAG(member->flags, PEER_FLAG_UPDATE_SOURCE) + && !member->update_source && !member->update_if) + continue; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { - peer->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; - bgp_notify_send(peer, BGP_NOTIFY_CEASE, + /* Remove flag and configuration on peer-group member. */ + UNSET_FLAG(member->flags, PEER_FLAG_UPDATE_SOURCE); + sockunion_free(member->update_source); + member->update_source = NULL; + XFREE(MTYPE_PEER_UPDATE_SOURCE, member->update_if); + + /* Send notification or reset peer depending on state. */ + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->status)) { + member->last_reset = PEER_DOWN_UPDATE_SOURCE_CHANGE; + bgp_notify_send(member, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - bgp_session_reset(peer); + bgp_session_reset(member); } + return 0; } @@ -5310,8 +5324,9 @@ int peer_allowas_in_unset(struct peer *peer, afi_t afi, safi_t safi) int peer_local_as_set(struct peer *peer, as_t as, int no_prepend, int replace_as) { + bool old_no_prepend, old_replace_as; struct bgp *bgp = peer->bgp; - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; if (peer_sort(peer) != BGP_PEER_EBGP @@ -5324,57 +5339,71 @@ int peer_local_as_set(struct peer *peer, as_t as, int no_prepend, if (peer->as == as) return BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_AS_REMOTE_AS; - if (peer->change_local_as == as - && ((CHECK_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND) - && no_prepend) - || (!CHECK_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND) - && !no_prepend)) - && ((CHECK_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS) - && replace_as) - || (!CHECK_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS) - && !replace_as))) - return 0; + /* Save previous flag states. */ + old_no_prepend = + !!CHECK_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); + old_replace_as = + !!CHECK_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); - peer->change_local_as = as; - if (no_prepend) - SET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); - else - UNSET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); + /* Set flag and configuration on peer. */ + peer_flag_set(peer, PEER_FLAG_LOCAL_AS); + peer_flag_modify(peer, PEER_FLAG_LOCAL_AS_NO_PREPEND, no_prepend); + peer_flag_modify(peer, PEER_FLAG_LOCAL_AS_REPLACE_AS, replace_as); - if (replace_as) - SET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); - else - UNSET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); + if (peer->change_local_as == as && old_no_prepend == no_prepend + && old_replace_as == replace_as) + return 0; + peer->change_local_as = as; + /* Check if handling a regular peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { peer->last_reset = PEER_DOWN_LOCAL_AS_CHANGE; bgp_notify_send(peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else bgp_session_reset(peer); + + /* Skip peer-group mechanics for regular peers. */ return 0; } - group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, peer)) { - peer->change_local_as = as; - if (no_prepend) - SET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); - else - UNSET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); + /* + * 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_LOCAL_AS)) + continue; - if (replace_as) - SET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); - else - UNSET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); + /* Skip peers with the same configuration. */ + old_no_prepend = CHECK_FLAG(member->flags, + PEER_FLAG_LOCAL_AS_NO_PREPEND); + old_replace_as = CHECK_FLAG(member->flags, + PEER_FLAG_LOCAL_AS_REPLACE_AS); + if (member->change_local_as == as + && CHECK_FLAG(member->flags, PEER_FLAG_LOCAL_AS) + && old_no_prepend == no_prepend + && old_replace_as == replace_as) + continue; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { - peer->last_reset = PEER_DOWN_LOCAL_AS_CHANGE; - bgp_notify_send(peer, BGP_NOTIFY_CEASE, + /* Set flag and configuration on peer-group member. */ + SET_FLAG(member->flags, PEER_FLAG_LOCAL_AS); + COND_FLAG(member->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND, + no_prepend); + COND_FLAG(member->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS, + replace_as); + member->change_local_as = as; + + /* Send notification or stop peer depending on state. */ + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->status)) { + member->last_reset = PEER_DOWN_LOCAL_AS_CHANGE; + bgp_notify_send(member, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD(peer, BGP_Stop); + BGP_EVENT_ADD(member, BGP_Stop); } return 0; @@ -5382,17 +5411,29 @@ int peer_local_as_set(struct peer *peer, as_t as, int no_prepend, int peer_local_as_unset(struct peer *peer) { - struct peer_group *group; + struct peer *member; struct listnode *node, *nnode; - if (!peer->change_local_as) + if (!CHECK_FLAG(peer->flags, PEER_FLAG_LOCAL_AS)) return 0; - peer->change_local_as = 0; - UNSET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); - UNSET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); + /* Inherit configuration from peer-group if peer is member. */ + if (peer_group_active(peer)) { + peer_flag_inherit(peer, PEER_FLAG_LOCAL_AS); + peer_flag_inherit(peer, PEER_FLAG_LOCAL_AS_NO_PREPEND); + peer_flag_inherit(peer, PEER_FLAG_LOCAL_AS_REPLACE_AS); + PEER_ATTR_INHERIT(peer, peer->group, change_local_as); + } else { + /* Otherwise remove flag and configuration from peer. */ + peer_flag_unset(peer, PEER_FLAG_LOCAL_AS); + peer_flag_unset(peer, PEER_FLAG_LOCAL_AS_NO_PREPEND); + peer_flag_unset(peer, PEER_FLAG_LOCAL_AS_REPLACE_AS); + peer->change_local_as = 0; + } + /* Check if handling a regular peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Send notification or stop peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { peer->last_reset = PEER_DOWN_LOCAL_AS_CHANGE; bgp_notify_send(peer, BGP_NOTIFY_CEASE, @@ -5400,77 +5441,103 @@ int peer_local_as_unset(struct peer *peer) } else BGP_EVENT_ADD(peer, BGP_Stop); + /* Skip peer-group mechanics for regular peers. */ return 0; } - group = peer->group; - for (ALL_LIST_ELEMENTS(group->peer, node, nnode, peer)) { - peer->change_local_as = 0; - UNSET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); - UNSET_FLAG(peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); + /* + * 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_LOCAL_AS)) + continue; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { - peer->last_reset = PEER_DOWN_LOCAL_AS_CHANGE; - bgp_notify_send(peer, BGP_NOTIFY_CEASE, + /* Remove flag and configuration on peer-group member. */ + UNSET_FLAG(member->flags, PEER_FLAG_LOCAL_AS); + UNSET_FLAG(member->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND); + UNSET_FLAG(member->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS); + member->change_local_as = 0; + + /* Send notification or stop peer depending on state. */ + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->status)) { + member->last_reset = PEER_DOWN_LOCAL_AS_CHANGE; + bgp_notify_send(member, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - bgp_session_reset(peer); + bgp_session_reset(member); } + return 0; } /* Set password for authenticating with the peer. */ int peer_password_set(struct peer *peer, const char *password) { - struct listnode *nn, *nnode; + struct peer *member; + struct listnode *node, *nnode; int len = password ? strlen(password) : 0; int ret = BGP_SUCCESS; if ((len < PEER_PASSWORD_MINLEN) || (len > PEER_PASSWORD_MAXLEN)) return BGP_ERR_INVALID_VALUE; - if (peer->password && strcmp(peer->password, password) == 0 - && !CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) + /* Set flag and configuration on peer. */ + peer_flag_set(peer, PEER_FLAG_PASSWORD); + if (peer->password && strcmp(peer->password, password) == 0) return 0; - - if (peer->password) - XFREE(MTYPE_PEER_PASSWORD, peer->password); - + XFREE(MTYPE_PEER_PASSWORD, peer->password); peer->password = XSTRDUP(MTYPE_PEER_PASSWORD, password); + /* Check if handling a regular peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) bgp_notify_send(peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); else bgp_session_reset(peer); + /* + * Attempt to install password on socket and skip peer-group + * mechanics. + */ if (BGP_PEER_SU_UNSPEC(peer)) return BGP_SUCCESS; - return (bgp_md5_set(peer) >= 0) ? BGP_SUCCESS : BGP_ERR_TCPSIG_FAILED; } - for (ALL_LIST_ELEMENTS(peer->group->peer, nn, nnode, peer)) { - if (peer->password && strcmp(peer->password, password) == 0) + /* + * 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_PASSWORD)) continue; - if (peer->password) - XFREE(MTYPE_PEER_PASSWORD, peer->password); - - peer->password = XSTRDUP(MTYPE_PEER_PASSWORD, password); + /* Skip peers with the same password. */ + if (member->password && strcmp(member->password, password) == 0) + continue; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) - bgp_notify_send(peer, BGP_NOTIFY_CEASE, + /* Set flag and configuration on peer-group member. */ + SET_FLAG(member->flags, PEER_FLAG_PASSWORD); + if (member->password) + XFREE(MTYPE_PEER_PASSWORD, member->password); + member->password = XSTRDUP(MTYPE_PEER_PASSWORD, password); + + /* Send notification or reset peer depending on state. */ + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->status)) + bgp_notify_send(member, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); else - bgp_session_reset(peer); + bgp_session_reset(member); - if (!BGP_PEER_SU_UNSPEC(peer)) { - if (bgp_md5_set(peer) < 0) - ret = BGP_ERR_TCPSIG_FAILED; - } + /* Attempt to install password on socket. */ + if (!BGP_PEER_SU_UNSPEC(member) && bgp_md5_set(member) < 0) + ret = BGP_ERR_TCPSIG_FAILED; } return ret; @@ -5478,47 +5545,63 @@ int peer_password_set(struct peer *peer, const char *password) int peer_password_unset(struct peer *peer) { - struct listnode *nn, *nnode; + struct peer *member; + struct listnode *node, *nnode; - if (!peer->password && !CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) + if (!CHECK_FLAG(peer->flags, PEER_FLAG_PASSWORD)) return 0; + /* Inherit configuration from peer-group if peer is member. */ + if (peer_group_active(peer)) { + peer_flag_inherit(peer, PEER_FLAG_PASSWORD); + PEER_STR_ATTR_INHERIT(peer, peer->group, password, + MTYPE_PEER_PASSWORD); + } else { + /* Otherwise remove flag and configuration from peer. */ + peer_flag_unset(peer, PEER_FLAG_PASSWORD); + XFREE(MTYPE_PEER_PASSWORD, peer->password); + } + + /* Check if handling a regular peer. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { + /* Send notification or reset peer depending on state. */ if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) bgp_notify_send(peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); else bgp_session_reset(peer); - if (peer->password) - XFREE(MTYPE_PEER_PASSWORD, peer->password); - - peer->password = NULL; - + /* Attempt to uninstall password on socket. */ if (!BGP_PEER_SU_UNSPEC(peer)) bgp_md5_unset(peer); + /* Skip peer-group mechanics for regular peers. */ return 0; } - XFREE(MTYPE_PEER_PASSWORD, peer->password); - peer->password = NULL; - - for (ALL_LIST_ELEMENTS(peer->group->peer, nn, nnode, peer)) { - if (!peer->password) + /* + * 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_PASSWORD)) continue; - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) - bgp_notify_send(peer, BGP_NOTIFY_CEASE, + /* Remove flag and configuration on peer-group member. */ + UNSET_FLAG(member->flags, PEER_FLAG_PASSWORD); + XFREE(MTYPE_PEER_PASSWORD, member->password); + + /* Send notification or reset peer depending on state. */ + if (BGP_IS_VALID_STATE_FOR_NOTIF(member->status)) + bgp_notify_send(member, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); else - bgp_session_reset(peer); + bgp_session_reset(member); - XFREE(MTYPE_PEER_PASSWORD, peer->password); - peer->password = NULL; - - if (!BGP_PEER_SU_UNSPEC(peer)) - bgp_md5_unset(peer); + /* Attempt to uninstall password on socket. */ + if (!BGP_PEER_SU_UNSPEC(member)) + bgp_md5_unset(member); } return 0; @@ -6929,24 +7012,14 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp, } /* local-as */ - if (peer->change_local_as) { - if (!peer_group_active(peer) - || peer->change_local_as != g_peer->change_local_as - || peergroup_flag_check(peer, PEER_FLAG_LOCAL_AS_NO_PREPEND) - || peergroup_flag_check(peer, - PEER_FLAG_LOCAL_AS_REPLACE_AS)) { - vty_out(vty, " neighbor %s local-as %u", addr, - peer->change_local_as); - - if (peergroup_flag_check(peer, - PEER_FLAG_LOCAL_AS_NO_PREPEND)) - vty_out(vty, " no-prepend"); - if (peergroup_flag_check(peer, - PEER_FLAG_LOCAL_AS_REPLACE_AS)) - vty_out(vty, " replace-as"); - - vty_out(vty, "\n"); - } + if (peergroup_flag_check(peer, PEER_FLAG_LOCAL_AS)) { + vty_out(vty, " neighbor %s local-as %u", addr, + peer->change_local_as); + if (peergroup_flag_check(peer, PEER_FLAG_LOCAL_AS_NO_PREPEND)) + vty_out(vty, " no-prepend"); + if (peergroup_flag_check(peer, PEER_FLAG_LOCAL_AS_REPLACE_AS)) + vty_out(vty, " replace-as"); + vty_out(vty, "\n"); } /* description */ @@ -6971,13 +7044,9 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp, } /* password */ - if (peer->password) { - if (!peer_group_active(peer) || !g_peer->password - || strcmp(peer->password, g_peer->password) != 0) { - vty_out(vty, " neighbor %s password %s\n", addr, - peer->password); - } - } + if (peergroup_flag_check(peer, PEER_FLAG_PASSWORD)) + vty_out(vty, " neighbor %s password %s\n", addr, + peer->password); /* neighbor solo */ if (CHECK_FLAG(peer->flags, PEER_FLAG_LONESOUL)) { @@ -7027,21 +7096,14 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp, vty_out(vty, " neighbor %s enforce-first-as\n", addr); /* update-source */ - if (peer->update_if) { - if (!peer_group_active(peer) || !g_peer->update_if - || strcmp(g_peer->update_if, peer->update_if) != 0) { - vty_out(vty, " neighbor %s update-source %s\n", addr, - peer->update_if); - } - } - if (peer->update_source) { - if (!peer_group_active(peer) || !g_peer->update_source - || sockunion_cmp(g_peer->update_source, peer->update_source) - != 0) { + if (peergroup_flag_check(peer, PEER_FLAG_UPDATE_SOURCE)) { + if (peer->update_source) vty_out(vty, " neighbor %s update-source %s\n", addr, sockunion2str(peer->update_source, buf, SU_ADDRSTRLEN)); - } + else if (peer->update_if) + vty_out(vty, " neighbor %s update-source %s\n", addr, + peer->update_if); } /* advertisement-interval */ diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 3ce309db6..1fbc0a0db 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -898,6 +898,9 @@ struct peer { #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 */ +#define PEER_FLAG_PASSWORD (1 << 20) /* password */ +#define PEER_FLAG_LOCAL_AS (1 << 21) /* local-as */ +#define PEER_FLAG_UPDATE_SOURCE (1 << 22) /* update-source */ /* outgoing message sent in CEASE_ADMIN_SHUTDOWN notify */ char *tx_shutdown_message; @@ -1188,6 +1191,15 @@ DECLARE_QOBJ_TYPE(peer) else \ (peer)->attr = NULL; \ } while (0) +#define PEER_SU_ATTR_INHERIT(peer, group, attr) \ + do { \ + if ((peer)->attr) \ + sockunion_free((peer)->attr); \ + if ((group)->conf->attr) \ + (peer)->attr = sockunion_dup((group)->conf->attr); \ + else \ + (peer)->attr = NULL; \ + } while (0) /* Check if suppress start/restart of sessions to peer. */ #define BGP_PEER_START_SUPPRESSED(P) \ diff --git a/tests/bgpd/test_peer_attr.c b/tests/bgpd/test_peer_attr.c index 5e09bf61a..362919cf3 100644 --- a/tests/bgpd/test_peer_attr.c +++ b/tests/bgpd/test_peer_attr.c @@ -210,6 +210,12 @@ static struct test_peer_attr test_peer_attrs[] = { .o.invert_peer = true, .o.use_iface_peer = true, }, + { + .cmd = "description", + .peer_cmd = "description FRR Peer", + .group_cmd = "description FRR Group", + .type = PEER_AT_GLOBAL_CUSTOM, + }, { .cmd = "disable-connected-check", .u.flag = PEER_FLAG_DISABLE_CONNECTED_CHECK, @@ -225,6 +231,22 @@ static struct test_peer_attr test_peer_attrs[] = { .u.flag = PEER_FLAG_ENFORCE_FIRST_AS, .type = PEER_AT_GLOBAL_FLAG, }, + { + .cmd = "local-as", + .peer_cmd = "local-as 1", + .group_cmd = "local-as 2", + .type = PEER_AT_GLOBAL_CUSTOM, + }, + { + .cmd = "local-as 1 no-prepend", + .u.flag = PEER_FLAG_LOCAL_AS_NO_PREPEND, + .type = PEER_AT_GLOBAL_FLAG, + }, + { + .cmd = "local-as 1 no-prepend replace-as", + .u.flag = PEER_FLAG_LOCAL_AS_REPLACE_AS, + .type = PEER_AT_GLOBAL_FLAG, + }, { .cmd = "override-capability", .u.flag = PEER_FLAG_OVERRIDE_CAPABILITY, @@ -235,6 +257,12 @@ static struct test_peer_attr test_peer_attrs[] = { .u.flag = PEER_FLAG_PASSIVE, .type = PEER_AT_GLOBAL_FLAG, }, + { + .cmd = "password", + .peer_cmd = "password FRR-Peer", + .group_cmd = "password FRR-Group", + .type = PEER_AT_GLOBAL_CUSTOM, + }, { .cmd = "shutdown", .u.flag = PEER_FLAG_SHUTDOWN, @@ -259,6 +287,18 @@ static struct test_peer_attr test_peer_attrs[] = { .u.flag = PEER_FLAG_TIMER_CONNECT, .type = PEER_AT_GLOBAL_FLAG, }, + { + .cmd = "update-source", + .peer_cmd = "update-source 255.255.255.1", + .group_cmd = "update-source 255.255.255.2", + .type = PEER_AT_GLOBAL_CUSTOM, + }, + { + .cmd = "update-source", + .peer_cmd = "update-source IFACE-PEER", + .group_cmd = "update-source IFACE-GROUP", + .type = PEER_AT_GLOBAL_CUSTOM, + }, /* Address Family Attributes */ { diff --git a/tests/bgpd/test_peer_attr.py b/tests/bgpd/test_peer_attr.py index bd377ca3a..7a4dd2b04 100644 --- a/tests/bgpd/test_peer_attr.py +++ b/tests/bgpd/test_peer_attr.py @@ -6,16 +6,26 @@ class TestFlag(frrtest.TestMultiOut): # List of tests can be generated by executing: # $> ./test_peer_attr 2>&1 | sed -n 's/\\/\\\\/g; s/\S\+ \[test\] \(.\+\)/TestFlag.okfail(\x27\1\x27)/pg' # +TestFlag.okfail('peer\\advertisement-interval') TestFlag.okfail('peer\\capability dynamic') TestFlag.okfail('peer\\capability extended-nexthop') TestFlag.okfail('peer\\capability extended-nexthop') +TestFlag.okfail('peer\\description') TestFlag.okfail('peer\\disable-connected-check') TestFlag.okfail('peer\\dont-capability-negotiate') TestFlag.okfail('peer\\enforce-first-as') +TestFlag.okfail('peer\\local-as') +TestFlag.okfail('peer\\local-as 1 no-prepend') +TestFlag.okfail('peer\\local-as 1 no-prepend replace-as') TestFlag.okfail('peer\\override-capability') TestFlag.okfail('peer\\passive') +TestFlag.okfail('peer\\password') TestFlag.okfail('peer\\shutdown') TestFlag.okfail('peer\\strict-capability-match') +TestFlag.okfail('peer\\timers') +TestFlag.okfail('peer\\timers connect') +TestFlag.okfail('peer\\update-source') +TestFlag.okfail('peer\\update-source') TestFlag.okfail('peer\\ipv4-unicast\\addpath-tx-all-paths') TestFlag.okfail('peer\\ipv4-multicast\\addpath-tx-all-paths') TestFlag.okfail('peer\\ipv6-unicast\\addpath-tx-all-paths')