Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[202311][FRR]Fix EVPN route type not matching route map #18732

Merged
merged 1 commit into from
Apr 23, 2024
Merged

[202311][FRR]Fix EVPN route type not matching route map #18732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
278 changes: 278 additions & 0 deletions src/sonic-frr/patch/0038-lib-Do-not-convert-EVPN-prefixes-into-IPv4-IPv6-if-n.patch
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ cross-compile-changes.patch
0035-fpm-ignore-route-from-default-table.patch
0036-Add-support-of-bgp-l3vni-evpn.patch
0037-bgp-community-memory-leak-fix.patch
0038-lib-Do-not-convert-EVPN-prefixes-into-IPv4-IPv6-if-n.patch
Loading