bgpd: Add nexthop of received EVPN RT-5 for nexthop tracking #5334

adharkar · 2019-11-14T00:48:41Z

Problem statement:
When IPv4/IPv6 prefixes are received in BGP, bgp_update function registers the
nexthop of the route with nexthop tracking module. The BGP route is marked as
valid only if the nexthop is resolved.

Even for EVPN RT-5, route should be marked as valid only if the the nexthop is
resolvable.

Code changes:

Add nexthop of EVPN RT-5 for nexthop tracking. Route will be marked as valid
only if the nexthop is resolved.
Only the valid EVPN routes are imported to the vrf.
When nht update is received in BGP, make sure that the EVPN routes are
imported/unimported based on the route becomes valid/invalid.

Testcases:

At rtr-1, advertise EVPN RT-5 with a nexthop 10.100.0.2.
10.100.0.2 is resolved at rtr-2 in default vrf.
At rtr-2, remote EVPN RT-5 should be marked as valid and should be imported into
vrfs.
Make the nexthop 10.100.0.2 unreachable at rtr-2
Remote EVPN RT-5 should be marked as invalid and should be unimported from the
vrfs. As this code change deals with EVPN type-5 routes only, other EVPN routes
should be valid.
At rtr-2, add a static route to make nexthop 10.100.0.2 reachable.
EVPN RT-5 should again become valid and should be imported into the vrfs.

Signed-off-by: Ameya Dharkar adharkar@vmware.com

LabN-CI · 2019-11-14T01:16:29Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/5334 `dd63e74`
Date	11/13/2019
Start	19:50:41
Finish	20:16:26
Run-Time	25:45
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-11-13-19:50:41.txt
Log	autoscript-2019-11-13-19:51:34.log.bz2
Memory	433 422 360

For details, please contact louberger

NetDEF-CI · 2019-11-14T03:38:48Z

Continuous Integration Result: FAILED

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

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

Topology tests on Ubuntu 18.04 amd64: Failed (click for details)

Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1804-9695/test

Topology Tests failed for Topology tests on Ubuntu 18.04 amd64:

2019-11-14 01:45:27,313 ERROR: assert failed at "test_ibgp_ecmp_topo2/test_ecmp_after_clear_bgp": Testcase test_ecmp_after_clear_bgp : Failed 
   Error: TIMEOUT!! BGP is not converged in 30 seconds for router r3
assert 'TIMEOUT!! BGP is not converged in 30 seconds for router r3' is True
*** defaultIntf: warning: r1 has no interfaces
2019-11-14 02:19:06,826 ERROR: Traceback (most recent call last):
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 1505, in create_bgp_community_lists
    build=build)
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 235, in create_common_configuration
    load_config_to_router(tgen, router)
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 380, in load_config_to_router
    raise InvalidCLIError("%s" % output)
InvalidCLIError: % Malformed community-list value
line 2: Failure to communicate[13] to bgpd, line: bgp community-list standard ANY permit 0:-1 



2019-11-14 02:19:06,986 ERROR: Traceback (most recent call last):
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 1505, in create_bgp_community_lists
    build=build)
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 235, in create_common_configuration
    load_config_to_router(tgen, router)
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 380, in load_config_to_router
    raise InvalidCLIError("%s" % output)
InvalidCLIError: % Malformed community-list value
line 2: Failure to communicate[13] to bgpd, line: bgp community-list standard ANY permit 0:65536 



2019-11-14 02:19:07,134 ERROR: Traceback (most recent call last):
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 1505, in create_bgp_community_lists
    build=build)
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 235, in create_common_configuration
    load_config_to_router(tgen, router)
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 380, in load_config_to_router
    raise InvalidCLIError("%s" % output)
InvalidCLIError: % Malformed community-list value
line 2: Failure to communicate[13] to bgpd, line: bgp large-community-list standard ANY permit 0:4294967296 



2019-11-14 02:19:07,285 ERROR: Traceback (most recent call last):
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 1505, in create_bgp_community_lists
    build=build)
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 235, in create_common_configuration
    load_config_to_router(tgen, router)
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 380, in load_config_to_router
    raise InvalidCLIError("%s" % output)
InvalidCLIError: % Malformed community-list value
line 2: Failure to communicate[13] to bgpd, line: bgp large-community-list standard ANY permit 0:-1:1 



2019-11-14 02:21:04,161 ERROR: Traceback (most recent call last):
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 1505, in create_bgp_community_lists
    build=build)
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 235, in create_common_configuration
    load_config_to_router(tgen, router)
  File "/root/bamboo-agent-home/xml-data/build-dir/FRR-FRRPULLREQ-TOPOU1804/topotests/lib/common_config.py", line 380, in load_config_to_router
    raise InvalidCLIError("%s" % output)
InvalidCLIError: line 2: % Command incomplete[4]: bgp large-community-list standard Test1 permit

see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9695/artifact/TOPOU1804/ErrorLog/log_topotests.txt

Successful on other platforms

Ubuntu 12.04 deb pkg check
Static analyzer (clang)
Fedora 29 rpm pkg check
IPv6 protocols on Ubuntu 14.04
Ubuntu 16.04 deb pkg check
Topotest tests on Ubuntu 16.04 i386
Debian 10 deb pkg check
Ubuntu 14.04 deb pkg check
IPv4 ldp protocol on Ubuntu 16.04
CentOS 7 rpm pkg check
Addresssanitizer topotest
Debian 8 deb pkg check
Topology tests on Ubuntu 16.04 amd64
IPv4 protocols on Ubuntu 14.04
Debian 9 deb pkg check

Topology Tests memory analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9695/artifact/TOPOU1804/MemoryLeaks/

CLANG Static Analyzer Summary

Github Pull Request 5334, comparing to Git base SHA b6af40b

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9695/artifact/shared/static_analysis/index.html

ton31337 · 2019-11-14T17:39:02Z

ci:rerun

NetDEF-CI · 2019-11-14T21:03:30Z

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-FRRPULLREQ-9707/

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.

CLANG Static Analyzer Summary

Github Pull Request 5334, comparing to Git base SHA b6af40b

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9707/artifact/shared/static_analysis/index.html

LabN-CI · 2019-11-14T22:38:36Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/5334 `d9266a4`
Date	11/14/2019
Start	17:12:57
Finish	17:38:34
Run-Time	25:37
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-11-14-17:12:57.txt
Log	autoscript-2019-11-14-17:13:49.log.bz2
Memory	429 436 359

For details, please contact louberger

NetDEF-CI · 2019-11-15T00:01:55Z

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-FRRPULLREQ-9717/

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.

CLANG Static Analyzer Summary

Github Pull Request 5334, comparing to Git base SHA afc4680
Base image data for Git afc4680 does not exist - compare skipped

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9717/artifact/shared/static_analysis/index.html

LabN-CI · 2019-11-15T00:24:26Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/5334 `b315b89`
Date	11/14/2019
Start	18:58:39
Finish	19:24:24
Run-Time	25:45
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-11-14-18:58:39.txt
Log	autoscript-2019-11-14-18:59:35.log.bz2
Memory	432 429 360

For details, please contact louberger

NetDEF-CI · 2019-11-15T00:35:44Z

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-FRRPULLREQ-9718/

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.

CLANG Static Analyzer Summary

Github Pull Request 5334, comparing to Git base SHA afc4680
Base image data for Git afc4680 does not exist - compare skipped

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9718/artifact/shared/static_analysis/index.html

bgpd/bgp_route.c

Problem statement: When IPv4/IPv6 prefixes are received in BGP, bgp_update function registers the nexthop of the route with nexthop tracking module. The BGP route is marked as valid only if the nexthop is resolved. Even for EVPN RT-5, route should be marked as valid only if the the nexthop is resolvable. Code changes: 1. Add nexthop of EVPN RT-5 for nexthop tracking. Route will be marked as valid only if the nexthop is resolved. 2. Only the valid EVPN routes are imported to the vrf. 3. When nht update is received in BGP, make sure that the EVPN routes are imported/unimported based on the route becomes valid/invalid. Testcases: 1. At rtr-1, advertise EVPN RT-5 with a nexthop 10.100.0.2. 10.100.0.2 is resolved at rtr-2 in default vrf. At rtr-2, remote EVPN RT-5 should be marked as valid and should be imported into vrfs. 2. Make the nexthop 10.100.0.2 unreachable at rtr-2 Remote EVPN RT-5 should be marked as invalid and should be unimported from the vrfs. As this code change deals with EVPN type-5 routes only, other EVPN routes should be valid. 3. At rtr-2, add a static route to make nexthop 10.100.0.2 reachable. EVPN RT-5 should again become valid and should be imported into the vrfs. Signed-off-by: Ameya Dharkar <adharkar@vmware.com>

LabN-CI · 2019-11-15T19:01:33Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/5334 `7c31238`
Date	11/15/2019
Start	13:35:41
Finish	14:01:30
Run-Time	25:49
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-11-15-13:35:41.txt
Log	autoscript-2019-11-15-13:36:33.log.bz2
Memory	433 436 359

For details, please contact louberger

NetDEF-CI · 2019-11-15T21:02:19Z

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-FRRPULLREQ-9730/

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.

CLANG Static Analyzer Summary

Github Pull Request 5334, comparing to Git base SHA 1e5fe0e
Base image data for Git 1e5fe0e does not exist - compare skipped

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9730/artifact/shared/static_analysis/index.html

vivek-cumulus · 2019-11-16T00:15:32Z

bgpd/bgp_attr.h

+#define BGP_ATTR_NH_AFI(afi, attr) \
+	((afi != AFI_L2VPN) ? afi : \
+	((attr->mp_nexthop_len == BGP_ATTR_NHLEN_IPV4) ? AFI_IP : AFI_IP6))
+


We should try to do subsequent changes to base the nh_afi just on the nexthop length, but for now, this seems fine.

vivek-cumulus · 2019-11-16T00:17:29Z

bgpd/bgp_evpn.c

+	 *
+	 * Ideally, a BGP route should be marked as valid only if the
+	 * nexthop is reachable. Thus, other EVPN route types also should be
+	 * added here after testing is performed for them.


Quite correct for most and probably all EVPN routes, we should certainly enable for RT-2 and RT-3 after testing. That can be in a subsequent commit.

This changeset follows the PR FRRouting#5334 Above PR adds nexthop tracking support for EVPN RT-5 nexthops. This route is marked VALID only if the BGP route has a valid nexthop. If the EVPN peer is an EBGP pee and "disable_connected_check" flag is not set, "connected" check is performed for the EVPN nexthop. But, usually EVPN nexthop is not the BGP peering address, but the VTEP address. Also, NEXTHOP_UNCHANGED flag is enabled by default for EVPN. As a result, in a common deployment for EVPN, EVPN nexthop is not connected. Thus, adding a fix to remove the "connected" check for EVPN nexthops. Signed-off-by: Ameya Dharkar <adharkar@vmware.com>

This changeset follows the PR FRRouting/frr#5334 Above PR adds nexthop tracking support for EVPN RT-5 nexthops. This route is marked VALID only if the BGP route has a valid nexthop. If the EVPN peer is an EBGP pee and "disable_connected_check" flag is not set, "connected" check is performed for the EVPN nexthop. But, usually EVPN nexthop is not the BGP peering address, but the VTEP address. Also, NEXTHOP_UNCHANGED flag is enabled by default for EVPN. As a result, in a common deployment for EVPN, EVPN nexthop is not connected. Thus, adding a fix to remove the "connected" check for EVPN nexthops. Signed-off-by: Ameya Dharkar <adharkar@vmware.com> Signed-off-by: harios <hari@niralnetworks.com>

This changeset follows the PR FRRouting#5334 Above PR adds nexthop tracking support for EVPN RT-5 nexthops. This route is marked VALID only if the BGP route has a valid nexthop. If the EVPN peer is an EBGP pee and "disable_connected_check" flag is not set, "connected" check is performed for the EVPN nexthop. But, usually EVPN nexthop is not the BGP peering address, but the VTEP address. Also, NEXTHOP_UNCHANGED flag is enabled by default for EVPN. As a result, in a common deployment for EVPN, EVPN nexthop is not connected. Thus, adding a fix to remove the "connected" check for EVPN nexthops. Signed-off-by: Ameya Dharkar <adharkar@vmware.com> (cherry picked from commit 41a28a2)

polychaeta added the bgp label Nov 14, 2019

adharkar force-pushed the frr-master-nexthop_check branch 2 times, most recently from d9266a4 to b315b89 Compare November 14, 2019 21:20

ton31337 reviewed Nov 15, 2019

View reviewed changes

bgpd/bgp_route.c Outdated Show resolved Hide resolved

adharkar force-pushed the frr-master-nexthop_check branch from b315b89 to 7c31238 Compare November 15, 2019 18:21

vivek-cumulus approved these changes Nov 16, 2019

View reviewed changes

ton31337 approved these changes Nov 18, 2019

View reviewed changes

ton31337 merged commit 839bdd0 into FRRouting:master Nov 18, 2019

adharkar deleted the frr-master-nexthop_check branch November 18, 2019 17:45

adharkar mentioned this pull request Nov 23, 2019

bgpd: Do not perform "connected" check for EVPN nexthop #5419

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bgpd: Add nexthop of received EVPN RT-5 for nexthop tracking #5334

bgpd: Add nexthop of received EVPN RT-5 for nexthop tracking #5334

adharkar commented Nov 14, 2019

LabN-CI commented Nov 14, 2019

NetDEF-CI commented Nov 14, 2019

ton31337 commented Nov 14, 2019

NetDEF-CI commented Nov 14, 2019

LabN-CI commented Nov 14, 2019

NetDEF-CI commented Nov 15, 2019

LabN-CI commented Nov 15, 2019

NetDEF-CI commented Nov 15, 2019

LabN-CI commented Nov 15, 2019

NetDEF-CI commented Nov 15, 2019

vivek-cumulus Nov 16, 2019

vivek-cumulus Nov 16, 2019

bgpd: Add nexthop of received EVPN RT-5 for nexthop tracking #5334

bgpd: Add nexthop of received EVPN RT-5 for nexthop tracking #5334

Conversation

adharkar commented Nov 14, 2019

LabN-CI commented Nov 14, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Nov 14, 2019

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topology Tests memory analysis: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9695/artifact/TOPOU1804/MemoryLeaks/

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

ton31337 commented Nov 14, 2019

NetDEF-CI commented Nov 14, 2019

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

LabN-CI commented Nov 14, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Nov 15, 2019

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

1 Static Analyzer issues remaining.

LabN-CI commented Nov 15, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Nov 15, 2019

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

1 Static Analyzer issues remaining.

LabN-CI commented Nov 15, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Nov 15, 2019

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

1 Static Analyzer issues remaining.

vivek-cumulus Nov 16, 2019

Choose a reason for hiding this comment

vivek-cumulus Nov 16, 2019

Choose a reason for hiding this comment