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>

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index c6e48cc16083d97fdfc7061c8ef0e2ed40a01f43..d7c1febb06e6652ef5e050fac9e1be837689d784 100644 (file)
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -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. */

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 5b8ceb054149e1999990d802cd1872a6aca531ed..6411865d45ac1f1c844c70d0ec8aeb1522367e31 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -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) {

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 484fc105e808dccee102462faad766bdf0776e9d..58ae119af1f075fe59823060654be8da37c9a6fb 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -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,