bgpd: Send route update when modifying access/aspath/prefix lists

Handle ORF REMOVE_ALL events as well, because now we just silently return, and
a stale dynamic prefix-list is used instead of the new one.

Before this, soft clear/route refresh was needed. Don't know the reason, but
we didn't send updates when modifying the filters.

Probably due to a massive change of filters and to avoid automatic updates :/

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 0bbf6f8a21130b51273bbbc53912b6d12d6805c2..fe1887565e51385755e07134b12240a42e27c614 100644 (file)
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -2283,17 +2283,26 @@ static int bgp_route_refresh_receive(struct peer *peer, bgp_size_t size)
                                                peer, orf_type, orf_len);
                                }
 
+                               /* ORF prefix-list name */
+                               snprintf(name, sizeof(name), "%s.%d.%d",
+                                        peer->host, afi, safi);
+
                                /* we're going to read at least 1 byte of common
                                 * ORF header,
                                 * and 7 bytes of ORF Address-filter entry from
                                 * the stream
                                 */
-                               if (orf_len < 7)
+                               if (*p_pnt & ORF_COMMON_PART_REMOVE_ALL) {
+                                       if (bgp_debug_neighbor_events(peer))
+                                               zlog_debug(
+                                                       "%pBP rcvd Remove-All pfxlist ORF request",
+                                                       peer);
+                                       prefix_bgp_orf_remove_all(afi, name);
                                        break;
+                               }
 
-                               /* ORF prefix-list name */
-                               snprintf(name, sizeof(name), "%s.%d.%d",
-                                        peer->host, afi, safi);
+                               if (orf_len < 7)
+                                       break;
 
                                while (p_pnt < p_end) {
                                        /* If the ORF entry is malformed, want
@@ -2306,17 +2315,6 @@ static int bgp_route_refresh_receive(struct peer *peer, bgp_size_t size)
                                        memset(&orfp, 0, sizeof(orfp));
                                        common = *p_pnt++;
                                        /* after ++: p_pnt <= p_end */
-                                       if (common
-                                           & ORF_COMMON_PART_REMOVE_ALL) {
-                                               if (bgp_debug_neighbor_events(
-                                                           peer))
-                                                       zlog_debug(
-                                                               "%pBP rcvd Remove-All pfxlist ORF request",
-                                                               peer);
-                                               prefix_bgp_orf_remove_all(afi,
-                                                                         name);
-                                               break;
-                                       }
                                        ok = ((uint32_t)(p_end - p_pnt)
                                              >= sizeof(uint32_t));
                                        if (ok) {

diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index e94b633b3b8eb3b59400927337c09edead2b4a1e..33f68c9e8811107426d58ecf849d5307176e36b0 100644 (file)
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -3867,7 +3867,7 @@ static void bgp_route_map_update_peer_group(const char *rmap_name,
  * network statements, etc looking to see if they use this route-map.
  */
 static void bgp_route_map_process_update(struct bgp *bgp, const char *rmap_name,
-                                        int route_update)
+                                        bool route_update)
 {
        int i;
        bool matched;
@@ -4080,7 +4080,7 @@ static void bgp_route_map_process_update_cb(char *rmap_name)
        struct bgp *bgp;
 
        for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) {
-               bgp_route_map_process_update(bgp, rmap_name, 1);
+               bgp_route_map_process_update(bgp, rmap_name, true);
 
 #ifdef ENABLE_BGP_VNC
                vnc_routemap_update(bgp, __func__);
@@ -4116,12 +4116,11 @@ static void bgp_route_map_mark_update(const char *rmap_name)
                /* Signal the groups that a route-map update event has
                 * started */
                for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp))
-                       update_group_policy_update(bgp,
-                                                  BGP_POLICY_ROUTE_MAP,
-                                                  rmap_name, 1, 1);
+                       update_group_policy_update(bgp, BGP_POLICY_ROUTE_MAP,
+                                                  rmap_name, true, 1);
        } else {
                for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) {
-                       bgp_route_map_process_update(bgp, rmap_name, 0);
+                       bgp_route_map_process_update(bgp, rmap_name, false);
 #ifdef ENABLE_BGP_VNC
                        vnc_routemap_update(bgp, __func__);
 #endif

diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c
index 405dd8f6ec2ed599141c0c2ddede5b3cce2b5dc1..13d5ec6b863e8a001554a3112663697e62d1a93d 100644 (file)
--- a/bgpd/bgp_updgrp.c
+++ b/bgpd/bgp_updgrp.c
@@ -1547,7 +1547,7 @@ static int update_group_periodic_merge_walkcb(struct update_group *updgrp,
  *             update groups.
  */
 void update_group_policy_update(struct bgp *bgp, enum bgp_policy_type ptype,
-                               const char *pname, int route_update,
+                               const char *pname, bool route_update,
                                int start_event)
 {
        struct updwalk_context ctx;

diff --git a/bgpd/bgp_updgrp.h b/bgpd/bgp_updgrp.h
index 473017c8098258167b5abe4e664bc2d2ea52f409..56289d399d3b4c7827907edc38a41532e5f54a12 100644 (file)
--- a/bgpd/bgp_updgrp.h
+++ b/bgpd/bgp_updgrp.h
@@ -298,7 +298,7 @@ struct updwalk_context {
        enum bgp_policy_type policy_type;
        const char *policy_name;
        int policy_event_start_flag;
-       int policy_route_update;
+       bool policy_route_update;
        updgrp_walkcb cb;
        void *context;
        uint8_t flags;
@@ -377,7 +377,7 @@ extern bool update_subgroup_trigger_merge_check(struct update_subgroup *,
                                                int force);
 extern void update_group_policy_update(struct bgp *bgp,
                                       enum bgp_policy_type ptype,
-                                      const char *pname, int route_update,
+                                      const char *pname, bool route_update,
                                       int start_event);
 extern void update_group_af_walk(struct bgp *bgp, afi_t afi, safi_t safi,
                                 updgrp_walkcb cb, void *ctx);

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index bd3e61377a65d6615d1b7f508f0b7106d3465898..219dee4693f8d3231219135a52140f0f78d8d70e 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -5497,12 +5497,23 @@ static void peer_on_policy_change(struct peer *peer, afi_t afi, safi_t safi,
                        return;
 
                if (CHECK_FLAG(peer->af_flags[afi][safi],
-                              PEER_FLAG_SOFT_RECONFIG))
+                              PEER_FLAG_SOFT_RECONFIG)) {
                        bgp_soft_reconfig_in(peer, afi, safi);
-               else if (CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_OLD_RCV)
-                        || CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_NEW_RCV))
-                       bgp_route_refresh_send(peer, afi, safi, 0, 0, 0,
-                                              BGP_ROUTE_REFRESH_NORMAL);
+               } else if (CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_OLD_RCV) ||
+                          CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_NEW_RCV)) {
+                       if (CHECK_FLAG(peer->af_cap[afi][safi],
+                                      PEER_CAP_ORF_PREFIX_SM_ADV) &&
+                           (CHECK_FLAG(peer->af_cap[afi][safi],
+                                       PEER_CAP_ORF_PREFIX_RM_RCV) ||
+                            CHECK_FLAG(peer->af_cap[afi][safi],
+                                       PEER_CAP_ORF_PREFIX_RM_OLD_RCV)))
+                               peer_clear_soft(peer, afi, safi,
+                                               BGP_CLEAR_SOFT_IN_ORF_PREFIX);
+                       else
+                               bgp_route_refresh_send(
+                                       peer, afi, safi, 0, 0, 0,
+                                       BGP_ROUTE_REFRESH_NORMAL);
+               }
        }
 }
 
@@ -6546,7 +6557,7 @@ static void peer_distribute_update(struct access_list *access)
                if (access->name)
                        update_group_policy_update(bgp,
                                                   BGP_POLICY_DISTRIBUTE_LIST,
-                                                  access->name, 0, 0);
+                                                  access->name, true, 0);
                for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) {
                        FOREACH_AFI_SAFI (afi, safi) {
                                filter = &peer->filter[afi][safi];
@@ -6735,7 +6746,7 @@ static void peer_prefix_list_update(struct prefix_list *plist)
                 */
                update_group_policy_update(
                        bgp, BGP_POLICY_PREFIX_LIST,
-                       plist ? prefix_list_name(plist) : NULL, 0, 0);
+                       plist ? prefix_list_name(plist) : NULL, true, 0);
 
                for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) {
                        FOREACH_AFI_SAFI (afi, safi) {
@@ -6753,6 +6764,14 @@ static void peer_prefix_list_update(struct prefix_list *plist)
                                                filter->plist[direct].plist =
                                                        NULL;
                                }
+
+                               /* If we touch prefix-list, we need to process
+                                * new updates. This is important for ORF to
+                                * work correctly as well.
+                                */
+                               if (peer->afc_nego[afi][safi])
+                                       peer_on_policy_change(peer, afi, safi,
+                                                             0);
                        }
                }
                for (ALL_LIST_ELEMENTS(bgp->group, node, nnode, group)) {
@@ -6912,7 +6931,7 @@ static void peer_aslist_update(const char *aslist_name)
 
        for (ALL_LIST_ELEMENTS(bm->bgp, mnode, mnnode, bgp)) {
                update_group_policy_update(bgp, BGP_POLICY_FILTER_LIST,
-                                          aslist_name, 0, 0);
+                                          aslist_name, true, 0);
 
                for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) {
                        FOREACH_AFI_SAFI (afi, safi) {