|
| 1 | +From 799c131b6a41262322a68252e44624e052b23cfa Mon Sep 17 00:00:00 2001 |
| 2 | +From: Trey Aspelund <taspelund@nvidia.com> |
| 3 | +Date: Fri, 17 Feb 2023 21:47:09 +0000 |
| 4 | +Subject: [PATCH 1/2] lib: skip route-map optimization if !AF_INET(6) |
| 5 | + |
| 6 | +Currently we unconditionally send a prefix through the optimized |
| 7 | +route-map codepath if the v4 and v6 LPM tables have been allocated and |
| 8 | +optimization has not been disabled. |
| 9 | +However prefixes from address-families that are not IPv4/IPv6 unicast |
| 10 | +always fail the optimized route-map index lookup, because they occur on |
| 11 | +an LPM tree that is IPv4 or IPv6 specific. |
| 12 | +e.g. |
| 13 | +Even if you have an empty permit route-map clause, Type-3 EVPN routes |
| 14 | +are always denied: |
| 15 | +``` |
| 16 | +--config |
| 17 | +route-map soo-foo permit 10 |
| 18 | + |
| 19 | +--logs |
| 20 | +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 |
| 21 | +2023/02/17 19:38:42 BGP: [H5AW4-JFYQC] Route-map: soo-foo, prefix: [3]:[0]:[32]:[2.2.2.2], result: deny |
| 22 | +``` |
| 23 | + |
| 24 | +There is some existing code that creates an AF_INET/AF_INET6 prefix |
| 25 | +using the IP/prefix information from a Type-2/5 EVPN route, which |
| 26 | +allowed only these two route-types to successfully attempt an LPM lookup |
| 27 | +in the route-map optimization trees via the converted prefix. |
| 28 | + |
| 29 | +This commit does 3 things: |
| 30 | +1) Reverts to non-optimized route-map lookup for prefixes that are not |
| 31 | + AF_INET or AF_INET6. |
| 32 | +2) Cleans up the route-map code so that the AF check is part of the |
| 33 | + index lookup + the EVPN RT-2/5 -> AF_INET/6 prefix conversion occurs |
| 34 | + outside the index lookup. |
| 35 | +3) Adds "debug route-map detail" logs to indicate when we attempt to |
| 36 | + convert an AF_EVPN prefix into an AF_INET/6 prefix + when we fallback |
| 37 | + to a non-optimized lookup. |
| 38 | + |
| 39 | +Additional functionality for optimized lookups of prefixes from other |
| 40 | +address-families can be added prior to the index lookup, similar to how |
| 41 | +the existing EVPN conversion works today. |
| 42 | + |
| 43 | +New behavior: |
| 44 | +``` |
| 45 | +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 |
| 46 | +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 |
| 47 | +2023/02/17 21:44:27 BGP: [H5AW4-JFYQC] Route-map: soo-foo, prefix: 192.0.2.7/32, result: permit |
| 48 | + |
| 49 | +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 |
| 50 | +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 |
| 51 | +2023/02/17 21:44:27 BGP: [H5AW4-JFYQC] Route-map: soo-foo, prefix: 20.0.0.2/32, result: permit |
| 52 | + |
| 53 | +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 |
| 54 | +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 |
| 55 | +2023/02/17 21:44:27 BGP: [H5AW4-JFYQC] Route-map: soo-foo, prefix: [3]:[0]:[32]:[2.2.2.2], result: permit |
| 56 | +``` |
| 57 | + |
| 58 | +Signed-off-by: Trey Aspelund <taspelund@nvidia.com> |
| 59 | +--- |
| 60 | + lib/routemap.c | 99 ++++++++++++++++++++++++++++---------------------- |
| 61 | + 1 file changed, 56 insertions(+), 43 deletions(-) |
| 62 | + |
| 63 | +diff --git a/lib/routemap.c b/lib/routemap.c |
| 64 | +index 210027105d..010d4bff0b 100644 |
| 65 | +--- a/lib/routemap.c |
| 66 | ++++ b/lib/routemap.c |
| 67 | +@@ -1817,26 +1817,24 @@ route_map_get_index(struct route_map *map, const struct prefix *prefix, |
| 68 | + struct route_map_index *index = NULL, *best_index = NULL; |
| 69 | + struct route_map_index *head_index = NULL; |
| 70 | + struct route_table *table = NULL; |
| 71 | +- struct prefix conv; |
| 72 | +- unsigned char family; |
| 73 | + |
| 74 | +- /* |
| 75 | +- * Handling for matching evpn_routes in the prefix table. |
| 76 | +- * |
| 77 | +- * We convert type2/5 prefix to ipv4/6 prefix to do longest |
| 78 | +- * prefix matching on. |
| 79 | ++ /* Route-map optimization relies on LPM lookups of the prefix to reduce |
| 80 | ++ * the amount of route-map clauses a given prefix needs to be processed |
| 81 | ++ * against. These LPM trees are IPv4/IPv6-specific and prefix->family |
| 82 | ++ * must be AF_INET or AF_INET6 in order for the lookup to succeed. So if |
| 83 | ++ * the AF doesn't line up with the LPM trees, skip the optimization. |
| 84 | + */ |
| 85 | +- if (prefix->family == AF_EVPN) { |
| 86 | +- if (evpn_prefix2prefix(prefix, &conv) != 0) |
| 87 | +- return NULL; |
| 88 | +- |
| 89 | +- prefix = &conv; |
| 90 | ++ if (map->optimization_disabled || |
| 91 | ++ (prefix->family == AF_INET && !map->ipv4_prefix_table) || |
| 92 | ++ (prefix->family == AF_INET6 && !map->ipv6_prefix_table)) { |
| 93 | ++ if (rmap_debug) |
| 94 | ++ zlog_debug( |
| 95 | ++ "Skipping route-map optimization for route-map: %s, pfx: %pFX, family: %d", |
| 96 | ++ map->name, prefix, prefix->family); |
| 97 | ++ return map->head; |
| 98 | + } |
| 99 | + |
| 100 | +- |
| 101 | +- family = prefix->family; |
| 102 | +- |
| 103 | +- if (family == AF_INET) |
| 104 | ++ if (prefix->family == AF_INET) |
| 105 | + table = map->ipv4_prefix_table; |
| 106 | + else |
| 107 | + table = map->ipv6_prefix_table; |
| 108 | +@@ -2558,6 +2556,7 @@ route_map_result_t route_map_apply_ext(struct route_map *map, |
| 109 | + struct route_map_index *index = NULL; |
| 110 | + struct route_map_rule *set = NULL; |
| 111 | + bool skip_match_clause = false; |
| 112 | ++ struct prefix conv; |
| 113 | + |
| 114 | + if (recursion > RMAP_RECURSION_LIMIT) { |
| 115 | + flog_warn( |
| 116 | +@@ -2575,37 +2574,51 @@ route_map_result_t route_map_apply_ext(struct route_map *map, |
| 117 | + |
| 118 | + map->applied++; |
| 119 | + |
| 120 | +- if ((!map->optimization_disabled) |
| 121 | +- && (map->ipv4_prefix_table || map->ipv6_prefix_table)) { |
| 122 | +- index = route_map_get_index(map, prefix, match_object, |
| 123 | +- &match_ret); |
| 124 | +- if (index) { |
| 125 | +- index->applied++; |
| 126 | +- if (rmap_debug) |
| 127 | +- zlog_debug( |
| 128 | +- "Best match route-map: %s, sequence: %d for pfx: %pFX, result: %s", |
| 129 | +- map->name, index->pref, prefix, |
| 130 | +- route_map_cmd_result_str(match_ret)); |
| 131 | ++ /* |
| 132 | ++ * Handling for matching evpn_routes in the prefix table. |
| 133 | ++ * |
| 134 | ++ * We convert type2/5 prefix to ipv4/6 prefix to do longest |
| 135 | ++ * prefix matching on. |
| 136 | ++ */ |
| 137 | ++ if (prefix->family == AF_EVPN) { |
| 138 | ++ if (evpn_prefix2prefix(prefix, &conv) != 0) { |
| 139 | ++ zlog_debug( |
| 140 | ++ "Unable to convert EVPN prefix %pFX into IPv4/IPv6 prefix. Falling back to non-optimized route-map lookup", |
| 141 | ++ prefix); |
| 142 | + } else { |
| 143 | +- if (rmap_debug) |
| 144 | +- zlog_debug( |
| 145 | +- "No best match sequence for pfx: %pFX in route-map: %s, result: %s", |
| 146 | +- prefix, map->name, |
| 147 | +- route_map_cmd_result_str(match_ret)); |
| 148 | +- /* |
| 149 | +- * No index matches this prefix. Return deny unless, |
| 150 | +- * match_ret = RMAP_NOOP. |
| 151 | +- */ |
| 152 | +- if (match_ret == RMAP_NOOP) |
| 153 | +- ret = RMAP_PERMITMATCH; |
| 154 | +- else |
| 155 | +- ret = RMAP_DENYMATCH; |
| 156 | +- goto route_map_apply_end; |
| 157 | ++ zlog_debug( |
| 158 | ++ "Converted EVPN prefix %pFX into %pFX for optimized route-map lookup", |
| 159 | ++ prefix, &conv); |
| 160 | ++ |
| 161 | ++ prefix = &conv; |
| 162 | + } |
| 163 | +- skip_match_clause = true; |
| 164 | ++ } |
| 165 | ++ |
| 166 | ++ index = route_map_get_index(map, prefix, match_object, &match_ret); |
| 167 | ++ if (index) { |
| 168 | ++ index->applied++; |
| 169 | ++ if (rmap_debug) |
| 170 | ++ zlog_debug( |
| 171 | ++ "Best match route-map: %s, sequence: %d for pfx: %pFX, result: %s", |
| 172 | ++ map->name, index->pref, prefix, |
| 173 | ++ route_map_cmd_result_str(match_ret)); |
| 174 | + } else { |
| 175 | +- index = map->head; |
| 176 | ++ if (rmap_debug) |
| 177 | ++ zlog_debug( |
| 178 | ++ "No best match sequence for pfx: %pFX in route-map: %s, result: %s", |
| 179 | ++ prefix, map->name, |
| 180 | ++ route_map_cmd_result_str(match_ret)); |
| 181 | ++ /* |
| 182 | ++ * No index matches this prefix. Return deny unless, |
| 183 | ++ * match_ret = RMAP_NOOP. |
| 184 | ++ */ |
| 185 | ++ if (match_ret == RMAP_NOOP) |
| 186 | ++ ret = RMAP_PERMITMATCH; |
| 187 | ++ else |
| 188 | ++ ret = RMAP_DENYMATCH; |
| 189 | ++ goto route_map_apply_end; |
| 190 | + } |
| 191 | ++ skip_match_clause = true; |
| 192 | + |
| 193 | + for (; index; index = index->next) { |
| 194 | + if (!skip_match_clause) { |
| 195 | +-- |
| 196 | +2.17.1 |
| 197 | + |
| 198 | + |
| 199 | +From 950cf63054fa36be57f4aa751243f5425793584b Mon Sep 17 00:00:00 2001 |
| 200 | +From: Donatas Abraitis <donatas@opensourcerouting.org> |
| 201 | +Date: Thu, 15 Feb 2024 12:07:43 +0200 |
| 202 | +Subject: [PATCH 2/2] lib: Do not convert EVPN prefixes into IPv4/IPv6 if not |
| 203 | + needed |
| 204 | + |
| 205 | +Convert only when this is really needed, e.g. `match ip address prefix-list ...`. |
| 206 | + |
| 207 | +Otherwise, we can't have mixed match clauses, like: |
| 208 | + |
| 209 | +``` |
| 210 | +match ip address prefix-list p1 |
| 211 | +match evpn route-type prefix |
| 212 | +``` |
| 213 | + |
| 214 | +This won't work, because the prefix is already converted, and we can't extract |
| 215 | +route type, vni, etc. from the original EVPN prefix. |
| 216 | + |
| 217 | +Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org> |
| 218 | +(cherry picked from commit 439b739495e86912c8b9ec36b84e55311c549ba0) |
| 219 | +--- |
| 220 | + lib/routemap.c | 25 +++++-------------------- |
| 221 | + 1 file changed, 5 insertions(+), 20 deletions(-) |
| 222 | + |
| 223 | +diff --git a/lib/routemap.c b/lib/routemap.c |
| 224 | +index 010d4bff0b..408faae49e 100644 |
| 225 | +--- a/lib/routemap.c |
| 226 | ++++ b/lib/routemap.c |
| 227 | +@@ -2556,7 +2556,6 @@ route_map_result_t route_map_apply_ext(struct route_map *map, |
| 228 | + struct route_map_index *index = NULL; |
| 229 | + struct route_map_rule *set = NULL; |
| 230 | + bool skip_match_clause = false; |
| 231 | +- struct prefix conv; |
| 232 | + |
| 233 | + if (recursion > RMAP_RECURSION_LIMIT) { |
| 234 | + flog_warn( |
| 235 | +@@ -2574,27 +2573,14 @@ route_map_result_t route_map_apply_ext(struct route_map *map, |
| 236 | + |
| 237 | + map->applied++; |
| 238 | + |
| 239 | +- /* |
| 240 | +- * Handling for matching evpn_routes in the prefix table. |
| 241 | +- * |
| 242 | +- * We convert type2/5 prefix to ipv4/6 prefix to do longest |
| 243 | +- * prefix matching on. |
| 244 | +- */ |
| 245 | + if (prefix->family == AF_EVPN) { |
| 246 | +- if (evpn_prefix2prefix(prefix, &conv) != 0) { |
| 247 | +- zlog_debug( |
| 248 | +- "Unable to convert EVPN prefix %pFX into IPv4/IPv6 prefix. Falling back to non-optimized route-map lookup", |
| 249 | +- prefix); |
| 250 | +- } else { |
| 251 | +- zlog_debug( |
| 252 | +- "Converted EVPN prefix %pFX into %pFX for optimized route-map lookup", |
| 253 | +- prefix, &conv); |
| 254 | +- |
| 255 | +- prefix = &conv; |
| 256 | +- } |
| 257 | ++ index = map->head; |
| 258 | ++ } else { |
| 259 | ++ skip_match_clause = true; |
| 260 | ++ index = route_map_get_index(map, prefix, match_object, |
| 261 | ++ &match_ret); |
| 262 | + } |
| 263 | + |
| 264 | +- index = route_map_get_index(map, prefix, match_object, &match_ret); |
| 265 | + if (index) { |
| 266 | + index->applied++; |
| 267 | + if (rmap_debug) |
| 268 | +@@ -2618,7 +2604,6 @@ route_map_result_t route_map_apply_ext(struct route_map *map, |
| 269 | + ret = RMAP_DENYMATCH; |
| 270 | + goto route_map_apply_end; |
| 271 | + } |
| 272 | +- skip_match_clause = true; |
| 273 | + |
| 274 | + for (; index; index = index->next) { |
| 275 | + if (!skip_match_clause) { |
| 276 | +-- |
| 277 | +2.17.1 |
| 278 | + |
0 commit comments