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.

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

lib: skip route-map optimization if !AF_INET(6) #14094

Conversation

ton31337
Copy link
Member

Backport for 8.5.

Closes #14082

Trey Aspelund added 2 commits July 25, 2023 20:22
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>
Fixes up evpn_prefix2prefix() to use IPV(4|6)_MAX_BITLEN instead of
32/128 directly.

Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Jul 25, 2023

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-13121/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests debian 10 amd64 part 9: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-13121/test

Topology Tests failed for Topotests debian 10 amd64 part 9
see full log at https://ci1.netdef.org/browse/FRR-PULLREQ2-13121/artifact/TOPO9DEB10AMD64/TopotestLogs/log_topotests.txt
Topotests debian 10 amd64 part 9: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-13121/artifact/TOPO9DEB10AMD64/TopotestDetails/

Successful on other platforms/tests
  • Topotests Ubuntu 18.04 amd64 part 0
  • Topotests Ubuntu 18.04 arm8 part 6
  • Topotests Ubuntu 18.04 arm8 part 1
  • Topotests debian 10 amd64 part 6
  • Addresssanitizer topotests part 2
  • Topotests Ubuntu 18.04 amd64 part 7
  • Topotests Ubuntu 18.04 amd64 part 5
  • Topotests debian 10 amd64 part 5
  • Topotests debian 10 amd64 part 0
  • Addresssanitizer topotests part 3
  • Topotests Ubuntu 18.04 arm8 part 2
  • Topotests Ubuntu 18.04 arm8 part 7
  • Topotests Ubuntu 18.04 amd64 part 4
  • CentOS 7 rpm pkg check
  • Addresssanitizer topotests part 9
  • Addresssanitizer topotests part 8
  • Topotests Ubuntu 18.04 arm8 part 8
  • Topotests Ubuntu 18.04 i386 part 7
  • Topotests Ubuntu 18.04 amd64 part 3
  • Topotests Ubuntu 18.04 i386 part 2
  • Topotests Ubuntu 18.04 amd64 part 1
  • Topotests Ubuntu 18.04 i386 part 5
  • Addresssanitizer topotests part 6
  • Ubuntu 18.04 deb pkg check
  • Topotests Ubuntu 18.04 i386 part 0
  • Topotests Ubuntu 18.04 amd64 part 2
  • Topotests debian 10 amd64 part 7
  • Topotests Ubuntu 18.04 i386 part 8
  • Topotests debian 10 amd64 part 8
  • Addresssanitizer topotests part 0
  • Topotests Ubuntu 18.04 arm8 part 4
  • Debian 10 deb pkg check
  • Topotests Ubuntu 18.04 arm8 part 9
  • Topotests Ubuntu 18.04 amd64 part 6
  • Topotests debian 10 amd64 part 3
  • Debian 9 deb pkg check
  • Addresssanitizer topotests part 1
  • Topotests Ubuntu 18.04 arm8 part 3
  • Topotests Ubuntu 18.04 i386 part 4
  • Topotests debian 10 amd64 part 4
  • Topotests Ubuntu 18.04 i386 part 3
  • Addresssanitizer topotests part 4
  • Topotests debian 10 amd64 part 2
  • Topotests Ubuntu 18.04 amd64 part 9
  • Topotests Ubuntu 18.04 i386 part 9
  • Addresssanitizer topotests part 7
  • Static analyzer (clang)
  • Ubuntu 20.04 deb pkg check
  • Topotests debian 10 amd64 part 1
  • Topotests Ubuntu 18.04 amd64 part 8
  • Addresssanitizer topotests part 5
  • Topotests Ubuntu 18.04 i386 part 1
  • Topotests Ubuntu 18.04 arm8 part 0
  • Topotests Ubuntu 18.04 i386 part 6
  • Topotests Ubuntu 18.04 arm8 part 5

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-13121/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

@Jafaral Jafaral merged commit 5ebb9c9 into FRRouting:stable/8.5 Jul 26, 2023
6 checks passed
@ton31337 ton31337 deleted the fix/backport_366a19fe385b85eb46df9f7566a1c09bd5c32770_8.5 branch July 26, 2023 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants