]> git.puffer.fish Git - matthieu/frr.git/commitdiff
bgpd: Fix peer group copying of data for late activation
authorDonald Sharp <sharpd@cumulusnetworks.com>
Fri, 29 Sep 2017 17:36:54 +0000 (13:36 -0400)
committerDonald Sharp <sharpd@cumulusnetworks.com>
Fri, 29 Sep 2017 17:36:54 +0000 (13:36 -0400)
If we are configuring a peer in multiple address families
and assigning the peer group valid configuration.  If you
delay the non-automatically address family activation you
will not copy the peer group data into that peer.

Suppose we enter this:
router bgp 65001

bgp router-id 6.0.0.17
  neighbor ISL peer-group
  neighbor ISL advertisement-interval 0
  neighbor ISL timers connect 5
  neighbor ISL timers 3 10
  address-family ipv4 unicast
    neighbor ISL allowas-in 1
    neighbor swp31s0 interface
    neighbor swp31s0 peer-group ISL
  address-family ipv6 unicast
    neighbor ISL allowas-in 1

We've assigned allowas-in to the ISL peer group.  Now suppose
we have a peer start connection to swp31s0.  We startup and
auto copy the v4 peer group information onto the peer.  We
do not copy the v6 peer group information because it has
not started yet.

Now at a later time if we enter:

address-family ipv6 unicast
  neighbor ISL activate

We start the swp31s0 peer in v6, but we are not copying the
peer group data into the v6 swp31s0 peer data structure.  As
such we are not respecting the v6 peer group config.

This Change modifies and renames the non_peergroup_activate_af
function to peer_activate_af.  We also call the function
peer_group2peer_config_copy_af if the peer is part of a peer
group.

The static function peer_group2peer_config_copy_af I have moved
to higher up so we don't have to add a static function declaration

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
bgpd/bgpd.c

index 54155290d6412d7fc17b4e71bc270dabe395453f..fa98238c3355aad829ccfc78fea91802f5e3342d 100644 (file)
@@ -1683,7 +1683,150 @@ int peer_remote_as(struct bgp *bgp, union sockunion *su, const char *conf_if,
        return 0;
 }
 
-static int non_peergroup_activate_af(struct peer *peer, afi_t afi, safi_t safi)
+static void peer_group2peer_config_copy_af(struct peer_group *group,
+                                          struct peer *peer, afi_t afi,
+                                          safi_t safi)
+{
+       int in = FILTER_IN;
+       int out = FILTER_OUT;
+       struct peer *conf;
+       struct bgp_filter *pfilter;
+       struct bgp_filter *gfilter;
+
+       conf = group->conf;
+       pfilter = &peer->filter[afi][safi];
+       gfilter = &conf->filter[afi][safi];
+
+       /* peer af_flags apply */
+       peer->af_flags[afi][safi] = conf->af_flags[afi][safi];
+
+       /* maximum-prefix */
+       peer->pmax[afi][safi] = conf->pmax[afi][safi];
+       peer->pmax_threshold[afi][safi] = conf->pmax_threshold[afi][safi];
+       peer->pmax_restart[afi][safi] = conf->pmax_restart[afi][safi];
+
+       /* allowas-in */
+       peer->allowas_in[afi][safi] = conf->allowas_in[afi][safi];
+
+       /* weight */
+       peer->weight[afi][safi] = conf->weight[afi][safi];
+
+       /* default-originate route-map */
+       if (conf->default_rmap[afi][safi].name) {
+               if (peer->default_rmap[afi][safi].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME,
+                             peer->default_rmap[afi][safi].name);
+               peer->default_rmap[afi][safi].name =
+                       XSTRDUP(MTYPE_BGP_FILTER_NAME,
+                               conf->default_rmap[afi][safi].name);
+               peer->default_rmap[afi][safi].map =
+                       conf->default_rmap[afi][safi].map;
+       }
+
+       /* inbound filter apply */
+       if (gfilter->dlist[in].name && !pfilter->dlist[in].name) {
+               if (pfilter->dlist[in].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->dlist[in].name);
+               pfilter->dlist[in].name =
+                       XSTRDUP(MTYPE_BGP_FILTER_NAME, gfilter->dlist[in].name);
+               pfilter->dlist[in].alist = gfilter->dlist[in].alist;
+       }
+
+       if (gfilter->plist[in].name && !pfilter->plist[in].name) {
+               if (pfilter->plist[in].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->plist[in].name);
+               pfilter->plist[in].name =
+                       XSTRDUP(MTYPE_BGP_FILTER_NAME, gfilter->plist[in].name);
+               pfilter->plist[in].plist = gfilter->plist[in].plist;
+       }
+
+       if (gfilter->aslist[in].name && !pfilter->aslist[in].name) {
+               if (pfilter->aslist[in].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->aslist[in].name);
+               pfilter->aslist[in].name = XSTRDUP(MTYPE_BGP_FILTER_NAME,
+                                                  gfilter->aslist[in].name);
+               pfilter->aslist[in].aslist = gfilter->aslist[in].aslist;
+       }
+
+       if (gfilter->map[RMAP_IN].name && !pfilter->map[RMAP_IN].name) {
+               if (pfilter->map[RMAP_IN].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME,
+                             pfilter->map[RMAP_IN].name);
+               pfilter->map[RMAP_IN].name = XSTRDUP(
+                       MTYPE_BGP_FILTER_NAME, gfilter->map[RMAP_IN].name);
+               pfilter->map[RMAP_IN].map = gfilter->map[RMAP_IN].map;
+       }
+
+       /* outbound filter apply */
+       if (gfilter->dlist[out].name) {
+               if (pfilter->dlist[out].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->dlist[out].name);
+               pfilter->dlist[out].name = XSTRDUP(MTYPE_BGP_FILTER_NAME,
+                                                  gfilter->dlist[out].name);
+               pfilter->dlist[out].alist = gfilter->dlist[out].alist;
+       } else {
+               if (pfilter->dlist[out].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->dlist[out].name);
+               pfilter->dlist[out].name = NULL;
+               pfilter->dlist[out].alist = NULL;
+       }
+
+       if (gfilter->plist[out].name) {
+               if (pfilter->plist[out].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->plist[out].name);
+               pfilter->plist[out].name = XSTRDUP(MTYPE_BGP_FILTER_NAME,
+                                                  gfilter->plist[out].name);
+               pfilter->plist[out].plist = gfilter->plist[out].plist;
+       } else {
+               if (pfilter->plist[out].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->plist[out].name);
+               pfilter->plist[out].name = NULL;
+               pfilter->plist[out].plist = NULL;
+       }
+
+       if (gfilter->aslist[out].name) {
+               if (pfilter->aslist[out].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->aslist[out].name);
+               pfilter->aslist[out].name = XSTRDUP(MTYPE_BGP_FILTER_NAME,
+                                                   gfilter->aslist[out].name);
+               pfilter->aslist[out].aslist = gfilter->aslist[out].aslist;
+       } else {
+               if (pfilter->aslist[out].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->aslist[out].name);
+               pfilter->aslist[out].name = NULL;
+               pfilter->aslist[out].aslist = NULL;
+       }
+
+       if (gfilter->map[RMAP_OUT].name) {
+               if (pfilter->map[RMAP_OUT].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME,
+                             pfilter->map[RMAP_OUT].name);
+               pfilter->map[RMAP_OUT].name = XSTRDUP(
+                       MTYPE_BGP_FILTER_NAME, gfilter->map[RMAP_OUT].name);
+               pfilter->map[RMAP_OUT].map = gfilter->map[RMAP_OUT].map;
+       } else {
+               if (pfilter->map[RMAP_OUT].name)
+                       XFREE(MTYPE_BGP_FILTER_NAME,
+                             pfilter->map[RMAP_OUT].name);
+               pfilter->map[RMAP_OUT].name = NULL;
+               pfilter->map[RMAP_OUT].map = NULL;
+       }
+
+       if (gfilter->usmap.name) {
+               if (pfilter->usmap.name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->usmap.name);
+               pfilter->usmap.name =
+                       XSTRDUP(MTYPE_BGP_FILTER_NAME, gfilter->usmap.name);
+               pfilter->usmap.map = gfilter->usmap.map;
+       } else {
+               if (pfilter->usmap.name)
+                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->usmap.name);
+               pfilter->usmap.name = NULL;
+               pfilter->usmap.map = NULL;
+       }
+}
+
+static int peer_activate_af(struct peer *peer, afi_t afi, safi_t safi)
 {
        int active;
 
@@ -1709,6 +1852,10 @@ static int non_peergroup_activate_af(struct peer *peer, afi_t afi, safi_t safi)
        active = peer_active(peer);
        peer->afc[afi][safi] = 1;
 
+       if (peer->group)
+               peer_group2peer_config_copy_af(peer->group, peer,
+                                              afi, safi);
+
        if (!active && peer_active(peer)) {
                bgp_timer_set(peer);
        } else {
@@ -1764,10 +1911,10 @@ int peer_activate(struct peer *peer, afi_t afi, safi_t safi)
                group = peer->group;
 
                for (ALL_LIST_ELEMENTS(group->peer, node, nnode, tmp_peer)) {
-                       ret |= non_peergroup_activate_af(tmp_peer, afi, safi);
+                       ret |= peer_activate_af(tmp_peer, afi, safi);
                }
        } else {
-               ret |= non_peergroup_activate_af(peer, afi, safi);
+               ret |= peer_activate_af(peer, afi, safi);
        }
 
        /* If this is the first peer to be activated for this afi/labeled-unicast
@@ -2245,149 +2392,6 @@ static void peer_group2peer_config_copy(struct peer_group *group,
        bgp_bfd_peer_group2peer_copy(conf, peer);
 }
 
-static void peer_group2peer_config_copy_af(struct peer_group *group,
-                                          struct peer *peer, afi_t afi,
-                                          safi_t safi)
-{
-       int in = FILTER_IN;
-       int out = FILTER_OUT;
-       struct peer *conf;
-       struct bgp_filter *pfilter;
-       struct bgp_filter *gfilter;
-
-       conf = group->conf;
-       pfilter = &peer->filter[afi][safi];
-       gfilter = &conf->filter[afi][safi];
-
-       /* peer af_flags apply */
-       peer->af_flags[afi][safi] = conf->af_flags[afi][safi];
-
-       /* maximum-prefix */
-       peer->pmax[afi][safi] = conf->pmax[afi][safi];
-       peer->pmax_threshold[afi][safi] = conf->pmax_threshold[afi][safi];
-       peer->pmax_restart[afi][safi] = conf->pmax_restart[afi][safi];
-
-       /* allowas-in */
-       peer->allowas_in[afi][safi] = conf->allowas_in[afi][safi];
-
-       /* weight */
-       peer->weight[afi][safi] = conf->weight[afi][safi];
-
-       /* default-originate route-map */
-       if (conf->default_rmap[afi][safi].name) {
-               if (peer->default_rmap[afi][safi].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME,
-                             peer->default_rmap[afi][safi].name);
-               peer->default_rmap[afi][safi].name =
-                       XSTRDUP(MTYPE_BGP_FILTER_NAME,
-                               conf->default_rmap[afi][safi].name);
-               peer->default_rmap[afi][safi].map =
-                       conf->default_rmap[afi][safi].map;
-       }
-
-       /* inbound filter apply */
-       if (gfilter->dlist[in].name && !pfilter->dlist[in].name) {
-               if (pfilter->dlist[in].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->dlist[in].name);
-               pfilter->dlist[in].name =
-                       XSTRDUP(MTYPE_BGP_FILTER_NAME, gfilter->dlist[in].name);
-               pfilter->dlist[in].alist = gfilter->dlist[in].alist;
-       }
-
-       if (gfilter->plist[in].name && !pfilter->plist[in].name) {
-               if (pfilter->plist[in].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->plist[in].name);
-               pfilter->plist[in].name =
-                       XSTRDUP(MTYPE_BGP_FILTER_NAME, gfilter->plist[in].name);
-               pfilter->plist[in].plist = gfilter->plist[in].plist;
-       }
-
-       if (gfilter->aslist[in].name && !pfilter->aslist[in].name) {
-               if (pfilter->aslist[in].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->aslist[in].name);
-               pfilter->aslist[in].name = XSTRDUP(MTYPE_BGP_FILTER_NAME,
-                                                  gfilter->aslist[in].name);
-               pfilter->aslist[in].aslist = gfilter->aslist[in].aslist;
-       }
-
-       if (gfilter->map[RMAP_IN].name && !pfilter->map[RMAP_IN].name) {
-               if (pfilter->map[RMAP_IN].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME,
-                             pfilter->map[RMAP_IN].name);
-               pfilter->map[RMAP_IN].name = XSTRDUP(
-                       MTYPE_BGP_FILTER_NAME, gfilter->map[RMAP_IN].name);
-               pfilter->map[RMAP_IN].map = gfilter->map[RMAP_IN].map;
-       }
-
-       /* outbound filter apply */
-       if (gfilter->dlist[out].name) {
-               if (pfilter->dlist[out].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->dlist[out].name);
-               pfilter->dlist[out].name = XSTRDUP(MTYPE_BGP_FILTER_NAME,
-                                                  gfilter->dlist[out].name);
-               pfilter->dlist[out].alist = gfilter->dlist[out].alist;
-       } else {
-               if (pfilter->dlist[out].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->dlist[out].name);
-               pfilter->dlist[out].name = NULL;
-               pfilter->dlist[out].alist = NULL;
-       }
-
-       if (gfilter->plist[out].name) {
-               if (pfilter->plist[out].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->plist[out].name);
-               pfilter->plist[out].name = XSTRDUP(MTYPE_BGP_FILTER_NAME,
-                                                  gfilter->plist[out].name);
-               pfilter->plist[out].plist = gfilter->plist[out].plist;
-       } else {
-               if (pfilter->plist[out].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->plist[out].name);
-               pfilter->plist[out].name = NULL;
-               pfilter->plist[out].plist = NULL;
-       }
-
-       if (gfilter->aslist[out].name) {
-               if (pfilter->aslist[out].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->aslist[out].name);
-               pfilter->aslist[out].name = XSTRDUP(MTYPE_BGP_FILTER_NAME,
-                                                   gfilter->aslist[out].name);
-               pfilter->aslist[out].aslist = gfilter->aslist[out].aslist;
-       } else {
-               if (pfilter->aslist[out].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->aslist[out].name);
-               pfilter->aslist[out].name = NULL;
-               pfilter->aslist[out].aslist = NULL;
-       }
-
-       if (gfilter->map[RMAP_OUT].name) {
-               if (pfilter->map[RMAP_OUT].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME,
-                             pfilter->map[RMAP_OUT].name);
-               pfilter->map[RMAP_OUT].name = XSTRDUP(
-                       MTYPE_BGP_FILTER_NAME, gfilter->map[RMAP_OUT].name);
-               pfilter->map[RMAP_OUT].map = gfilter->map[RMAP_OUT].map;
-       } else {
-               if (pfilter->map[RMAP_OUT].name)
-                       XFREE(MTYPE_BGP_FILTER_NAME,
-                             pfilter->map[RMAP_OUT].name);
-               pfilter->map[RMAP_OUT].name = NULL;
-               pfilter->map[RMAP_OUT].map = NULL;
-       }
-
-       if (gfilter->usmap.name) {
-               if (pfilter->usmap.name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->usmap.name);
-               pfilter->usmap.name =
-                       XSTRDUP(MTYPE_BGP_FILTER_NAME, gfilter->usmap.name);
-               pfilter->usmap.map = gfilter->usmap.map;
-       } else {
-               if (pfilter->usmap.name)
-                       XFREE(MTYPE_BGP_FILTER_NAME, pfilter->usmap.name);
-               pfilter->usmap.name = NULL;
-               pfilter->usmap.map = NULL;
-       }
-}
-
 /* Peer group's remote AS configuration.  */
 int peer_group_remote_as(struct bgp *bgp, const char *group_name, as_t *as,
                         int as_type)