From: Donald Sharp Date: Fri, 29 Sep 2017 17:36:54 +0000 (-0400) Subject: bgpd: Fix peer group copying of data for late activation X-Git-Tag: frr-4.0-dev~203^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=9790f44f390f870afab8dfdebe822f4c83bea902;p=matthieu%2Ffrr.git bgpd: Fix peer group copying of data for late activation 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 --- diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 54155290d6..fa98238c33 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -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)