]> git.puffer.fish Git - mirror/frr.git/commitdiff
lib: skip route-map optimization if !AF_INET(6) 12839/head
authorTrey Aspelund <taspelund@nvidia.com>
Fri, 17 Feb 2023 21:47:09 +0000 (21:47 +0000)
committerTrey Aspelund <taspelund@nvidia.com>
Tue, 21 Feb 2023 16:36:20 +0000 (16:36 +0000)
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 <taspelund@nvidia.com>
lib/routemap.c

index 6b4627082e5a94e0ec3caeca091234e16b7f4c20..ccf733e8b82bb315602e18aeb228c6bc0a2b3ca4 100644 (file)
@@ -1804,26 +1804,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 (CHECK_FLAG(rmap_debug, DEBUG_ROUTEMAP_DETAIL))
+                       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;
@@ -2545,6 +2543,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(
@@ -2562,37 +2561,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 (CHECK_FLAG(rmap_debug, DEBUG_ROUTEMAP))
-                               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 (CHECK_FLAG(rmap_debug, DEBUG_ROUTEMAP))
-                               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 (CHECK_FLAG(rmap_debug, DEBUG_ROUTEMAP))
+                       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 (CHECK_FLAG(rmap_debug, DEBUG_ROUTEMAP))
+                       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) {