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

adharkar · 2019-11-23T00:08:21Z

This changeset follows the PR
#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#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>

LabN-CI · 2019-11-23T00:36:30Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/5419 `41a28a2`
Date	11/22/2019
Start	19:10:36
Finish	19:36:28
Run-Time	25:52
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-11-22-19:10:36.txt
Log	autoscript-2019-11-22-19:11:27.log.bz2
Memory	436 434 360

For details, please contact louberger

NetDEF-CI · 2019-11-23T02:58:45Z

Continuous Integration Result: FAILED

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

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-9837/test

Topology Tests failed for Topology tests on Ubuntu 18.04 amd64:

*** defaultIntf: warning: r1 has no interfaces
2019-11-23 01:21:23,685 ERROR: ******************************************************************************
2019-11-23 01:21:23,686 ERROR: Test Target Summary                                                  Pass Fail
2019-11-23 01:21:23,686 ERROR: ******************************************************************************
2019-11-23 01:21:23,686 ERROR: FILE: scripts/check_linux_vrf.py
2019-11-23 01:21:23,686 ERROR: FILE: scripts/adjacencies.py
2019-11-23 01:21:23,686 ERROR: FILE: scripts/check_routes.py
2019-11-23 01:21:23,686 ERROR: FILE: scripts/check_linux_mpls.py
2019-11-23 01:21:23,686 ERROR: FILE: scripts/scale_up.py
2019-11-23 01:21:23,686 ERROR: FILE: scripts/scale_down.py
2019-11-23 01:21:23,686 ERROR: 146  ce1    BGP routes removed +13.57 secs                           0    1
2019-11-23 01:21:23,686 ERROR: See /tmp/topotests/bgp_l3vpn_to_bgp_vrf.test_bgp_l3vpn_to_bgp_vrf/output.log for details of errors
2019-11-23 01:21:23,689 ERROR: assert failed at "bgp_l3vpn_to_bgp_vrf.test_bgp_l3vpn_to_bgp_vrf/test_check_scale_down": 1 tests failed
2019-11-23 01:22:51,363 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-23 01:22:51,481 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-23 01:22:51,601 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-23 01:22:51,724 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-23 01:24:49,034 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-9837/artifact/TOPOU1804/ErrorLog/log_topotests.txt

Successful on other platforms

Addresssanitizer topotest
Debian 8 deb pkg check
Topology tests on Ubuntu 16.04 amd64
Debian 10 deb pkg check
CentOS 7 rpm pkg check
Topotest tests on Ubuntu 16.04 i386
IPv4 protocols on Ubuntu 14.04
IPv4 ldp protocol on Ubuntu 16.04
Ubuntu 14.04 deb pkg check
Debian 9 deb pkg check
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

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

<TITLE>clang_check</TITLE>

clang_check

ton31337 · 2019-11-23T21:58:38Z

bgpd/bgp_route.c

@@ -3454,7 +3454,8 @@ int bgp_update(struct peer *peer, struct prefix *p, uint32_t addpath_id,
 		    && (safi == SAFI_UNICAST || safi == SAFI_LABELED_UNICAST))
 		    || (safi == SAFI_EVPN &&
 			bgp_evpn_is_prefix_nht_supported(p))) {
-			if (peer->sort == BGP_PEER_EBGP && peer->ttl == 1
+			if (safi != SAFI_EVPN
+			    && peer->sort == BGP_PEER_EBGP && peer->ttl == 1


can we replace 1 with BGP_DEFAULT_TTL?

yes -- no magic numbers... using the #def would be better here

We will fix this. But Just to clarify, the ttl part is not that was modified now, but already present in the code.

srimohans · 2019-11-26T00:39:41Z

checked the CI logs and its not quite clear if the failure is related to changes. Giving it another try.

CI:rerun

NetDEF-CI · 2019-11-26T03:30:57Z

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-9854/

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.

<TITLE>clang_check</TITLE>

clang_check

vivek-cumulus

The changes look good to me. When I reported the problem, I'd suggested that we need to introduce a per-afi/safi disable_nexthop_connected_check flag (and config) to handle this in a comprehensive way. This can of course be in a subsequent commit.

[By way of explanation, whenever the sender of the prefix is setup to pass along the next hop unchanged (whether by explicit config or due to defaults), the receiver must be setup (by config or via defaults) to skip the "next hop must be connected" check for EBGP peerings. This can be achieved be configuring the peering itself for multihop or via configs to disable the connected check, which need to be at the afi/safi granularity to match the sender's next hop settings.]

polychaeta added the bgp label Nov 23, 2019

ton31337 requested changes Nov 23, 2019

View reviewed changes

qlyoung added the bugfix label Nov 26, 2019

vivek-cumulus approved these changes Nov 26, 2019

View reviewed changes

ton31337 approved these changes Nov 27, 2019

View reviewed changes

ton31337 merged commit 3e1cc63 into FRRouting:master Nov 27, 2019

adharkar deleted the frr-master-nh_connected branch February 6, 2020 21:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

adharkar commented Nov 23, 2019

LabN-CI commented Nov 23, 2019

NetDEF-CI commented Nov 23, 2019

ton31337 Nov 23, 2019

riw777 Nov 26, 2019

srimohans Nov 26, 2019

srimohans commented Nov 26, 2019

NetDEF-CI commented Nov 26, 2019

vivek-cumulus left a comment

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

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

Conversation

adharkar commented Nov 23, 2019

LabN-CI commented Nov 23, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Nov 23, 2019

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

clang_check

ton31337 Nov 23, 2019

Choose a reason for hiding this comment

riw777 Nov 26, 2019

Choose a reason for hiding this comment

srimohans Nov 26, 2019

Choose a reason for hiding this comment

srimohans commented Nov 26, 2019

NetDEF-CI commented Nov 26, 2019

Continuous Integration Result: SUCCESSFUL

clang_check

vivek-cumulus left a comment

Choose a reason for hiding this comment