Skip to content

Commit

Permalink
[202311][FRR]Fix EVPN route type not matching route map
Browse files Browse the repository at this point in the history
  • Loading branch information
@dgsudharsan
dgsudharsan committed Apr 18, 2024
1 parent e0f8753 commit 20a75e9
Show file tree
Hide file tree
Showing 2 changed files with 279 additions and 0 deletions.
278 changes: 278 additions & 0 deletions src/sonic-frr/patch/0037-lib-Do-not-convert-EVPN-prefixes-into-IPv4-IPv6-if-n.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,278 @@
From 799c131b6a41262322a68252e44624e052b23cfa Mon Sep 17 00:00:00 2001
From: Trey Aspelund <taspelund@nvidia.com>
Date: Fri, 17 Feb 2023 21:47:09 +0000
Subject: [PATCH 1/2] 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 <taspelund@nvidia.com>
---
lib/routemap.c | 99 ++++++++++++++++++++++++++++----------------------
1 file changed, 56 insertions(+), 43 deletions(-)

diff --git a/lib/routemap.c b/lib/routemap.c
index 210027105d..010d4bff0b 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) {
--
2.17.1


From 950cf63054fa36be57f4aa751243f5425793584b Mon Sep 17 00:00:00 2001
From: Donatas Abraitis <donatas@opensourcerouting.org>
Date: Thu, 15 Feb 2024 12:07:43 +0200
Subject: [PATCH 2/2] lib: Do not convert EVPN prefixes into IPv4/IPv6 if not
needed

Convert only when this is really needed, e.g. `match ip address prefix-list ...`.

Otherwise, we can't have mixed match clauses, like:

```
match ip address prefix-list p1
match evpn route-type prefix
```

This won't work, because the prefix is already converted, and we can't extract
route type, vni, etc. from the original EVPN prefix.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit 439b739495e86912c8b9ec36b84e55311c549ba0)
---
lib/routemap.c | 25 +++++--------------------
1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/lib/routemap.c b/lib/routemap.c
index 010d4bff0b..408faae49e 100644
--- a/lib/routemap.c
+++ b/lib/routemap.c
@@ -2556,7 +2556,6 @@ 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(
@@ -2574,27 +2573,14 @@ route_map_result_t route_map_apply_ext(struct route_map *map,

map->applied++;

- /*
- * 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 {
- zlog_debug(
- "Converted EVPN prefix %pFX into %pFX for optimized route-map lookup",
- prefix, &conv);
-
- prefix = &conv;
- }
+ index = map->head;
+ } else {
+ skip_match_clause = true;
+ index = route_map_get_index(map, prefix, match_object,
+ &match_ret);
}

- index = route_map_get_index(map, prefix, match_object, &match_ret);
if (index) {
index->applied++;
if (rmap_debug)
@@ -2618,7 +2604,6 @@ route_map_result_t route_map_apply_ext(struct route_map *map,
ret = RMAP_DENYMATCH;
goto route_map_apply_end;
}
- skip_match_clause = true;

for (; index; index = index->next) {
if (!skip_match_clause) {
--
2.17.1

1 change: 1 addition & 0 deletions src/sonic-frr/patch/series
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@ cross-compile-changes.patch
0034-fpm-Use-vrf_id-for-vrf-not-tabled_id.patch
0035-fpm-ignore-route-from-default-table.patch
0036-Add-support-of-bgp-l3vni-evpn.patch
0037-lib-Do-not-convert-EVPN-prefixes-into-IPv4-IPv6-if-n.patch

0 comments on commit 20a75e9

Please sign in to comment.