bgpd: improve peer-group remote-as definitions

Problem reported that with certain sequences of defining the
remote-as on the peer-group and the members, the configuration would
become wrong, with configured remote-as settings not reflected in
the config but peers unable to come up.  This fix resolves these
inconsistencies.

Ticket: CM-19560
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
This commit is contained in:
Don Slice 2018-12-27 11:03:38 -05:00
parent 313731ac92
commit faa16034cb
3 changed files with 53 additions and 15 deletions

View File

@ -2846,13 +2846,11 @@ static int peer_remote_as_vty(struct vty *vty, const char *peer_str,
switch (ret) {
case BGP_ERR_PEER_GROUP_MEMBER:
vty_out(vty,
"%% Peer-group AS %u. Cannot configure remote-as for member\n",
as);
"%% Peer-group member cannot override remote-as of peer-group\n");
return CMD_WARNING_CONFIG_FAILED;
case BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT:
vty_out(vty,
"%% The AS# can not be changed from %u to %s, peer-group members must be all internal or all external\n",
as, as_str);
"%% Peer-group members must be all internal or all external\n");
return CMD_WARNING_CONFIG_FAILED;
}
return bgp_vty_return(vty, ret);
@ -9212,8 +9210,8 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json,
? " replace-as"
: "");
}
/* peer type internal, external, confed-internal or confed-external */
if (p->as == p->local_as) {
/* peer type internal or confed-internal */
if ((p->as == p->local_as) || (p->as_type == AS_INTERNAL)) {
if (use_json) {
if (CHECK_FLAG(bgp->config, BGP_CONFIG_CONFEDERATION))
json_object_boolean_true_add(
@ -9227,7 +9225,8 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json,
else
vty_out(vty, "internal link\n");
}
} else {
/* peer type external or confed-external */
} else if (p->as || (p->as_type == AS_EXTERNAL)) {
if (use_json) {
if (CHECK_FLAG(bgp->config, BGP_CONFIG_CONFEDERATION))
json_object_boolean_true_add(
@ -9241,6 +9240,12 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, bool use_json,
else
vty_out(vty, "external link\n");
}
} else {
if (use_json)
json_object_boolean_true_add(json_neigh,
"nbrUnspecifiedLink");
else
vty_out(vty, "unspecified link\n");
}
/* Description. */

View File

@ -1001,7 +1001,26 @@ static inline bgp_peer_sort_t peer_calc_sort(struct peer *peer)
return BGP_PEER_EBGP;
} else {
if (peer->as_type != AS_SPECIFIED)
if (peer->as_type == AS_UNSPECIFIED) {
/* check if in peer-group with AS information */
if (peer->group
&& (peer->group->conf->as_type != AS_UNSPECIFIED)) {
if (peer->group->conf->as_type
== AS_SPECIFIED) {
if (peer->local_as
== peer->group->conf->as)
return BGP_PEER_IBGP;
else
return BGP_PEER_EBGP;
} else if (peer->group->conf->as_type
== AS_INTERNAL)
return BGP_PEER_IBGP;
else
return BGP_PEER_EBGP;
}
/* no AS information anywhere, let caller know */
return BGP_PEER_UNSPECIFIED;
} else if (peer->as_type != AS_SPECIFIED)
return (peer->as_type == AS_INTERNAL ? BGP_PEER_IBGP
: BGP_PEER_EBGP);
@ -1711,20 +1730,32 @@ int peer_remote_as(struct bgp *bgp, union sockunion *su, const char *conf_if,
/* When this peer is a member of peer-group. */
if (peer->group) {
if (peer->group->conf->as) {
/* peer-group already has AS number/internal/external */
if (peer->group->conf->as
|| peer->group->conf->as_type) {
/* Return peer group's AS number. */
*as = peer->group->conf->as;
return BGP_ERR_PEER_GROUP_MEMBER;
}
if (peer_sort(peer->group->conf) == BGP_PEER_IBGP) {
if ((as_type != AS_INTERNAL)
&& (bgp->as != *as)) {
bgp_peer_sort_t peer_sort_type =
peer_sort(peer->group->conf);
/* Explicit AS numbers used, compare AS numbers */
if (as_type == AS_SPECIFIED) {
if (((peer_sort_type == BGP_PEER_IBGP)
&& (bgp->as != *as))
|| ((peer_sort_type == BGP_PEER_EBGP)
&& (bgp->as == *as))) {
*as = peer->as;
return BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT;
}
} else {
if ((as_type != AS_EXTERNAL)
&& (bgp->as == *as)) {
/* internal/external used, compare as-types */
if (((peer_sort_type == BGP_PEER_IBGP)
&& (as_type != AS_INTERNAL))
|| ((peer_sort_type == BGP_PEER_EBGP)
&& (as_type != AS_EXTERNAL))) {
*as = peer->as;
return BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT;
}
@ -2688,6 +2719,7 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer,
if (peer->as_type == AS_UNSPECIFIED) {
peer->as_type = group->conf->as_type;
peer->as = group->conf->as;
peer->sort = group->conf->sort;
}
if (!group->conf->as) {

View File

@ -646,7 +646,8 @@ struct bgp_filter {
/* IBGP/EBGP identifier. We also have a CONFED peer, which is to say,
a peer who's AS is part of our Confederation. */
typedef enum {
BGP_PEER_IBGP = 1,
BGP_PEER_UNSPECIFIED,
BGP_PEER_IBGP,
BGP_PEER_EBGP,
BGP_PEER_INTERNAL,
BGP_PEER_CONFED,