]> git.puffer.fish Git - mirror/frr.git/commitdiff
tools: limit bgp route-maps to direct changes only 9138/head
authorDonald Sharp <sharpd@nvidia.com>
Tue, 23 Mar 2021 12:48:54 +0000 (08:48 -0400)
committermergify-bot <noreply@mergify.io>
Fri, 23 Jul 2021 09:12:32 +0000 (09:12 +0000)
When using frr-reload.py to modify a bgp neighbors route-map
the code was doing this:

a) deleting the previous route-map: `no neighbor XX route-map YY (in|out)`
b) Adding the new route-map back in `neighbor XX route-may ZZ (in|out)`

Now imagine that we have an outgoing route-map that we are changing
and the reload is large because of a large number of lines in frr.conf

Item (a) will happen.  BGP will immediately start sending all local
routes.  At some point in time in the future (b) will be applied.
This of course causes a withdraw but for a short amount of time we
are leaking unintended routes.  This is bad for several reasons
not 1) route churn upstream, 2) we might influence traffic to go the
wrong way. 3) if upstream has a maximum-prefix command the routes
being sent might trip its circuitry and shutdown the peer entirely
not even allowing you to get to (b).

Ticket: #2589685
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
(cherry picked from commit 6293c2181a3c58046f94496391d14a921a7fd892)

tools/frr-reload.py

index c28a971525aeaf76185db21e8b7c21a7b3f971bd..e716ddcadadbd807362eec98732334ca58c68d0e 100755 (executable)
@@ -1219,6 +1219,53 @@ def ignore_delete_re_add_lines(lines_to_add, lines_to_del):
                                 if found_add_bfd_nbr:
                                     lines_to_del_to_del.append((ctx_keys, line))
 
+                """
+                Neighbor changes of route-maps need to be accounted for in that we
+                do not want to do a `no route-map...` `route-map ....` when changing
+                a route-map.  This is bad mojo as that we will send/receive
+                data we don't want.
+                Additionally we need to ensure that if we have different afi/safi
+                variants that they actually match and if we are going from a very
+                old style command such that the neighbor command is under the
+                `router bgp ..` node that we need to handle that appropriately
+                """
+                re_nbr_rm = re.search("neighbor(.*)route-map(.*)(in|out)$", line)
+                if re_nbr_rm:
+                    adjust_for_bgp_node = 0
+                    neighbor_name = re_nbr_rm.group(1)
+                    rm_name_del = re_nbr_rm.group(2)
+                    dir = re_nbr_rm.group(3)
+                    search = "neighbor%sroute-map(.*)%s" % (neighbor_name, dir)
+                    save_line = "EMPTY"
+                    for (ctx_keys_al, add_line) in lines_to_add:
+                        if ctx_keys_al[0].startswith("router bgp"):
+                            if add_line:
+                                rm_match = re.search(search, add_line)
+                            if rm_match:
+                                rm_name_add = rm_match.group(1)
+                                if rm_name_add == rm_name_del:
+                                    continue
+                                if len(ctx_keys_al) == 1:
+                                    save_line = line
+                                    adjust_for_bgp_node = 1
+                                else:
+                                    if (
+                                        len(ctx_keys) > 1
+                                        and len(ctx_keys_al) > 1
+                                        and ctx_keys[1] == ctx_keys_al[1]
+                                    ):
+                                        lines_to_del_to_del.append((ctx_keys_al, line))
+
+                    if adjust_for_bgp_node == 1:
+                        for (ctx_keys_dl, dl_line) in lines_to_del:
+                            if (
+                                ctx_keys_dl[0].startswith("router bgp")
+                                and len(ctx_keys_dl) > 1
+                                and ctx_keys_dl[1] == "address-family ipv4 unicast"
+                            ):
+                                if save_line == dl_line:
+                                    lines_to_del_to_del.append((ctx_keys_dl, save_line))
+
                 """
                 We changed how we display the neighbor interface command. Older
                 versions of frr would display the following: