From 7603eca36d7bf7d73f14f46aa5006063214f03e1 Mon Sep 17 00:00:00 2001 From: Trey Aspelund Date: Fri, 17 Feb 2023 21:47:09 +0000 Subject: [PATCH] lib: skip route-map optimization if !AF_INET(6) Currently we unconditionally send a prefix through the optimized route-map codepath if the v4 and v6 LPM tables have been allocated and optimization has not been disabled. However prefixes from address-families that are not IPv4/IPv6 unicast always fail the optimized route-map index lookup, because they occur on an LPM tree that is IPv4 or IPv6 specific. e.g. Even if you have an empty permit route-map clause, Type-3 EVPN routes are always denied: ``` --config route-map soo-foo permit 10 --logs 2023/02/17 19:38:42 BGP: [KZK58-6T4Y6] No best match sequence for pfx: [3]:[0]:[32]:[2.2.2.2] in route-map: soo-foo, result: no match 2023/02/17 19:38:42 BGP: [H5AW4-JFYQC] Route-map: soo-foo, prefix: [3]:[0]:[32]:[2.2.2.2], result: deny ``` There is some existing code that creates an AF_INET/AF_INET6 prefix using the IP/prefix information from a Type-2/5 EVPN route, which allowed only these two route-types to successfully attempt an LPM lookup in the route-map optimization trees via the converted prefix. This commit does 3 things: 1) Reverts to non-optimized route-map lookup for prefixes that are not AF_INET or AF_INET6. 2) Cleans up the route-map code so that the AF check is part of the index lookup + the EVPN RT-2/5 -> AF_INET/6 prefix conversion occurs outside the index lookup. 3) Adds "debug route-map detail" logs to indicate when we attempt to convert an AF_EVPN prefix into an AF_INET/6 prefix + when we fallback to a non-optimized lookup. Additional functionality for optimized lookups of prefixes from other address-families can be added prior to the index lookup, similar to how the existing EVPN conversion works today. New behavior: ``` 2023/02/17 21:44:27 BGP: [WYP1M-NE4SY] Converted EVPN prefix [5]:[0]:[32]:[192.0.2.7] into 192.0.2.7/32 for optimized route-map lookup 2023/02/17 21:44:27 BGP: [MT1SJ-WEJQ1] Best match route-map: soo-foo, sequence: 10 for pfx: 192.0.2.7/32, result: match 2023/02/17 21:44:27 BGP: [H5AW4-JFYQC] Route-map: soo-foo, prefix: 192.0.2.7/32, result: permit 2023/02/17 21:44:27 BGP: [WYP1M-NE4SY] Converted EVPN prefix [2]:[0]:[48]:[aa:bb:cc:00:22:22]:[32]:[20.0.0.2] into 20.0.0.2/32 for optimized route-map lookup 2023/02/17 21:44:27 BGP: [MT1SJ-WEJQ1] Best match route-map: soo-foo, sequence: 10 for pfx: 20.0.0.2/32, result: match 2023/02/17 21:44:27 BGP: [H5AW4-JFYQC] Route-map: soo-foo, prefix: 20.0.0.2/32, result: permit 2023/02/17 21:44:27 BGP: [KHG7H-RH4PN] Unable to convert EVPN prefix [3]:[0]:[32]:[2.2.2.2] into IPv4/IPv6 prefix. Falling back to non-optimized route-map lookup 2023/02/17 21:44:27 BGP: [MT1SJ-WEJQ1] Best match route-map: soo-foo, sequence: 10 for pfx: [3]:[0]:[32]:[2.2.2.2], result: match 2023/02/17 21:44:27 BGP: [H5AW4-JFYQC] Route-map: soo-foo, prefix: [3]:[0]:[32]:[2.2.2.2], result: permit ``` Signed-off-by: Trey Aspelund --- lib/routemap.c | 99 ++++++++++++++++++++++++++++---------------------- 1 file changed, 56 insertions(+), 43 deletions(-) diff --git a/lib/routemap.c b/lib/routemap.c index 210027105d9e..010d4bff0b77 100644 --- a/lib/routemap.c +++ b/lib/routemap.c @@ -1817,26 +1817,24 @@ route_map_get_index(struct route_map *map, const struct prefix *prefix, struct route_map_index *index = NULL, *best_index = NULL; struct route_map_index *head_index = NULL; struct route_table *table = NULL; - struct prefix conv; - unsigned char family; - /* - * Handling for matching evpn_routes in the prefix table. - * - * We convert type2/5 prefix to ipv4/6 prefix to do longest - * prefix matching on. + /* Route-map optimization relies on LPM lookups of the prefix to reduce + * the amount of route-map clauses a given prefix needs to be processed + * against. These LPM trees are IPv4/IPv6-specific and prefix->family + * must be AF_INET or AF_INET6 in order for the lookup to succeed. So if + * the AF doesn't line up with the LPM trees, skip the optimization. */ - if (prefix->family == AF_EVPN) { - if (evpn_prefix2prefix(prefix, &conv) != 0) - return NULL; - - prefix = &conv; + if (map->optimization_disabled || + (prefix->family == AF_INET && !map->ipv4_prefix_table) || + (prefix->family == AF_INET6 && !map->ipv6_prefix_table)) { + if (rmap_debug) + zlog_debug( + "Skipping route-map optimization for route-map: %s, pfx: %pFX, family: %d", + map->name, prefix, prefix->family); + return map->head; } - - family = prefix->family; - - if (family == AF_INET) + if (prefix->family == AF_INET) table = map->ipv4_prefix_table; else table = map->ipv6_prefix_table; @@ -2558,6 +2556,7 @@ route_map_result_t route_map_apply_ext(struct route_map *map, struct route_map_index *index = NULL; struct route_map_rule *set = NULL; bool skip_match_clause = false; + struct prefix conv; if (recursion > RMAP_RECURSION_LIMIT) { flog_warn( @@ -2575,37 +2574,51 @@ route_map_result_t route_map_apply_ext(struct route_map *map, map->applied++; - if ((!map->optimization_disabled) - && (map->ipv4_prefix_table || map->ipv6_prefix_table)) { - index = route_map_get_index(map, prefix, match_object, - &match_ret); - if (index) { - index->applied++; - if (rmap_debug) - zlog_debug( - "Best match route-map: %s, sequence: %d for pfx: %pFX, result: %s", - map->name, index->pref, prefix, - route_map_cmd_result_str(match_ret)); + /* + * Handling for matching evpn_routes in the prefix table. + * + * We convert type2/5 prefix to ipv4/6 prefix to do longest + * prefix matching on. + */ + if (prefix->family == AF_EVPN) { + if (evpn_prefix2prefix(prefix, &conv) != 0) { + zlog_debug( + "Unable to convert EVPN prefix %pFX into IPv4/IPv6 prefix. Falling back to non-optimized route-map lookup", + prefix); } else { - if (rmap_debug) - zlog_debug( - "No best match sequence for pfx: %pFX in route-map: %s, result: %s", - prefix, map->name, - route_map_cmd_result_str(match_ret)); - /* - * No index matches this prefix. Return deny unless, - * match_ret = RMAP_NOOP. - */ - if (match_ret == RMAP_NOOP) - ret = RMAP_PERMITMATCH; - else - ret = RMAP_DENYMATCH; - goto route_map_apply_end; + zlog_debug( + "Converted EVPN prefix %pFX into %pFX for optimized route-map lookup", + prefix, &conv); + + prefix = &conv; } - skip_match_clause = true; + } + + index = route_map_get_index(map, prefix, match_object, &match_ret); + if (index) { + index->applied++; + if (rmap_debug) + zlog_debug( + "Best match route-map: %s, sequence: %d for pfx: %pFX, result: %s", + map->name, index->pref, prefix, + route_map_cmd_result_str(match_ret)); } else { - index = map->head; + if (rmap_debug) + zlog_debug( + "No best match sequence for pfx: %pFX in route-map: %s, result: %s", + prefix, map->name, + route_map_cmd_result_str(match_ret)); + /* + * No index matches this prefix. Return deny unless, + * match_ret = RMAP_NOOP. + */ + if (match_ret == RMAP_NOOP) + ret = RMAP_PERMITMATCH; + else + ret = RMAP_DENYMATCH; + goto route_map_apply_end; } + skip_match_clause = true; for (; index; index = index->next) { if (!skip_match_clause) {