From 0e5658a6d22b56e66f7377aa1f6dc7be8658ca1c Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 23 Mar 2021 08:48:54 -0400 Subject: [PATCH] tools: limit bgp route-maps to direct changes only 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 (cherry picked from commit 6293c2181a3c58046f94496391d14a921a7fd892) --- tools/frr-reload.py | 47 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tools/frr-reload.py b/tools/frr-reload.py index c28a971525ae..e716ddcadadb 100755 --- a/tools/frr-reload.py +++ b/tools/frr-reload.py @@ -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: