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

Addpath - Reuse IDs #3051

Merged
merged 3 commits into from
Nov 13, 2018
Merged

Conversation

mitch-skiba
Copy link
Contributor

Summary

Lifted from my main commit message:

The motivation for this patch is to address a concerning behavior of
tx-addpath-bestpath-per-AS. Prior to this patch, all paths' TX ID was
pre-determined as the path was received from a peer. However, this meant
that any time the path selected as best from an AS changed, bgpd had no
choice but to withdraw the previous best path, and advertise the new
best-path under a new TX ID. This could cause significant network
disruption, especially for the subset of prefixes coming from only one
AS that were also communicated over a bestpath-per-AS session.

There are a lot of parts of this patch I expect to need some discussion or attention. Some notable areas:

  • I changed the tx-addpath type from flags to an enum. (It is still per peer/afi/safi, just not in the flag bit field anymore.) I had noticed that a peer/afi/safi could be marked as being multiple kinds of addpath before, and it was up to the ordering of each if-else tree that checked the flags to determine what that meant. Anyway, this ended up being a minor source of bugs during development, since I had to track down every place peer structures had data copied around.
  • I attempted to count the number of configured peers and groups using each addpath strategy per afi/safi. I use the information to avoid doing ID work that isn't needed. However, keeping track of deltas as different events happen got a lot more complicated than I had initially anticipated. It might make sense to go back to iterating all of the peers on event to make sure it stays in sync.
  • I moved a lot of the addpath TX ID selection logic into a separate file. This was helpful to me during development, but might not actually be the way we want to leave things.
  • Paths now need to track the TX ID per addpath strategy. For now, this only grows the path structure by 4 bytes, but if more addpath strategies are added in the future, this could end up being unwanted dead weight.
  • I'm not sure what should be done with the TX ID in the path JSON output. There is no longer a single TX ID to output, but I'm hesitant to remove a key from a JSON structure. For now I'm just pushing the tx-addpath-all ID, but I'm open to suggestions.

Related Issue

Components

bgpd, lib

Always remember to follow proper coding style etc. as
described in the FRRouting Dev Guide.
http://docs.frrouting.org/projects/dev-guide/en/latest/workflow.html

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: FAILED

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

This is a comment from an EXPERIMENTAL 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 and apply patch from patchwork: Successful

Building Stage: Failed

Ubuntu1204 amd64 build: Successful

CentOS6 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI006BUILD/ErrorLog/log_pytests.txt
CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI006BUILD/config.status/config.status

FreeBSD9 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI004BUILD/ErrorLog/log_pytests.txt
FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI004BUILD/config.status/config.status

Debian9 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI021BUILD/ErrorLog/log_pytests.txt
Debian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI021BUILD/config.status/config.status

OmniOS amd64 build: Failed

Make failed for OmniOS amd64 build:
(see full make log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI010BUILD/ErrorLog/log_make.txt)

checking for minix/config.h... no
checking whether it is safe to define __EXTENSIONS__... yes
checking whether gcc supports -diag-error 10006... no
checking whether gcc supports -std=gnu11... yes
checking whether gcc -std=gnu11 supports -g... yes
checking whether gcc -std=gnu11 supports -Os... yes
checking whether gcc -std=gnu11 supports -fno-omit-frame-pointer... yes
checking whether gcc -std=gnu11 supports -funwind-tables... yes
checking whether gcc -std=gnu11 supports -Wall... yes

OmniOS amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI010BUILD/config.status/config.status

Ubuntu 16.04 i386: Failed

DejaGNU Unittests (make check) failed for Ubuntu 16.04 i386
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/U1604I386/ErrorLog/log_pytests.txt
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/U1604I386/config.status/config.status

NetBSD6 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI007BUILD/ErrorLog/log_pytests.txt
NetBSD6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI007BUILD/config.status/config.status

CentOS7 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI005BUILD/ErrorLog/log_pytests.txt
CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI005BUILD/config.status/config.status

Debian8 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian8 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI008BLD/ErrorLog/log_pytests.txt
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI008BLD/config.status/config.status

FreeBSD10 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD10 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI003BUILD/ErrorLog/log_pytests.txt
FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI003BUILD/config.status/config.status

Ubuntu1404 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1404 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI001BUILD/ErrorLog/log_pytests.txt
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI001BUILD/config.status/config.status

OpenBSD60 amd64 build: Failed

DejaGNU Unittests (make check) failed for OpenBSD60 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI011BUILD/ErrorLog/log_pytests.txt
OpenBSD60 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI011BUILD/config.status/config.status

NetBSD7 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI012BUILD/ErrorLog/log_pytests.txt
NetBSD7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI012BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1604 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI014BUILD/ErrorLog/log_pytests.txt
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI014BUILD/config.status/config.status

Fedora24 amd64 build: Failed

DejaGNU Unittests (make check) failed for Fedora24 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI015BUILD/ErrorLog/log_pytests.txt
Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI015BUILD/config.status/config.status

FreeBSD11 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD11 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI009BUILD/ErrorLog/log_pytests.txt
FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI009BUILD/config.status/config.status

Ubuntu 18.04 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu 18.04 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/U1804AMD64/ErrorLog/log_pytests.txt
Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/U1804AMD64/config.status/config.status


Warnings Generated during build:

Checkout code: Successful with additional warnings:

CentOS6 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI006BUILD/ErrorLog/log_pytests.txt
CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI006BUILD/config.status/config.status

FreeBSD9 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI004BUILD/ErrorLog/log_pytests.txt
FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI004BUILD/config.status/config.status

Debian9 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI021BUILD/ErrorLog/log_pytests.txt
Debian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI021BUILD/config.status/config.status

OmniOS amd64 build: Failed

Make failed for OmniOS amd64 build:
(see full make log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI010BUILD/ErrorLog/log_make.txt)

checking for minix/config.h... no
checking whether it is safe to define __EXTENSIONS__... yes
checking whether gcc supports -diag-error 10006... no
checking whether gcc supports -std=gnu11... yes
checking whether gcc -std=gnu11 supports -g... yes
checking whether gcc -std=gnu11 supports -Os... yes
checking whether gcc -std=gnu11 supports -fno-omit-frame-pointer... yes
checking whether gcc -std=gnu11 supports -funwind-tables... yes
checking whether gcc -std=gnu11 supports -Wall... yes

OmniOS amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI010BUILD/config.status/config.status

Ubuntu 16.04 i386: Failed

DejaGNU Unittests (make check) failed for Ubuntu 16.04 i386
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/U1604I386/ErrorLog/log_pytests.txt
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/U1604I386/config.status/config.status

NetBSD6 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI007BUILD/ErrorLog/log_pytests.txt
NetBSD6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI007BUILD/config.status/config.status

CentOS7 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI005BUILD/ErrorLog/log_pytests.txt
CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI005BUILD/config.status/config.status

Debian8 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian8 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI008BLD/ErrorLog/log_pytests.txt
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI008BLD/config.status/config.status

FreeBSD10 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD10 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI003BUILD/ErrorLog/log_pytests.txt
FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI003BUILD/config.status/config.status

Ubuntu1404 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1404 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI001BUILD/ErrorLog/log_pytests.txt
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI001BUILD/config.status/config.status

OpenBSD60 amd64 build: Failed

DejaGNU Unittests (make check) failed for OpenBSD60 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI011BUILD/ErrorLog/log_pytests.txt
OpenBSD60 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI011BUILD/config.status/config.status

NetBSD7 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI012BUILD/ErrorLog/log_pytests.txt
NetBSD7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI012BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1604 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI014BUILD/ErrorLog/log_pytests.txt
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI014BUILD/config.status/config.status

Fedora24 amd64 build: Failed

DejaGNU Unittests (make check) failed for Fedora24 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI015BUILD/ErrorLog/log_pytests.txt
Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI015BUILD/config.status/config.status

FreeBSD11 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD11 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI009BUILD/ErrorLog/log_pytests.txt
FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/CI009BUILD/config.status/config.status

Ubuntu 18.04 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu 18.04 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/U1804AMD64/ErrorLog/log_pytests.txt
Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5360/artifact/U1804AMD64/config.status/config.status

<stdin>:544: trailing whitespace.
	
<stdin>:2098: trailing whitespace.
	
<stdin>:2135: trailing whitespace.
	
warning: 3 lines add whitespace errors.
Report for bgpd.c | 14 issues
===============================================
< WARNING: line over 80 characters
< #887: FILE: /tmp/f1-13700/bgpd.c:887:
< WARNING: braces {} are not necessary for any arm of this statement
< #898: FILE: /tmp/f1-13700/bgpd.c:898:
< WARNING: braces {} are not necessary for single statement blocks
< #1064: FILE: /tmp/f1-13700/bgpd.c:1064:
< WARNING: line over 80 characters
< #1065: FILE: /tmp/f1-13700/bgpd.c:1065:
< ERROR: code indent should use tabs where possible
< #1842: FILE: /tmp/f1-13700/bgpd.c:1842:
< ERROR: code indent should use tabs where possible
< #7019: FILE: /tmp/f1-13700/bgpd.c:7019:
< ERROR: code indent should use tabs where possible
< #7023: FILE: /tmp/f1-13700/bgpd.c:7023:
Report for bgp_open.c | 2 issues
===============================================
< WARNING: braces {} are not necessary for single statement blocks
< #1380: FILE: /tmp/f1-13700/bgp_open.c:1380:
Report for bgp_route.c | 6 issues
===============================================
< WARNING: line over 80 characters
< #2103: FILE: /tmp/f1-13700/bgp_route.c:2103:
< WARNING: Block comments use a trailing */ on a separate line
< #8067: FILE: /tmp/f1-13700/bgp_route.c:8067:
< WARNING: Block comments use a trailing */ on a separate line
< #8085: FILE: /tmp/f1-13700/bgp_route.c:8085:
Report for bgp_tx_addpath.c | 136 issues
===============================================
ERROR: space required after that close brace '}'
#33: FILE: /tmp/f1-13700/bgp_tx_addpath.c:33:
+	 .id_json_name = "addpathTxIdBestPerAS"}};

WARNING: braces {} are not necessary for any arm of this statement
#49: FILE: /tmp/f1-13700/bgp_tx_addpath.c:49:
+	if (strat < BGP_ADDPATH_MAX) {
[...]
+	} else {
[...]

WARNING: Missing a blank line after declarations
#92: FILE: /tmp/f1-13700/bgp_tx_addpath.c:92:
+	int i;
+	for (i = 0; i < BGP_ADDPATH_MAX; i++) {

ERROR: Please, no nonstandard integer types in new code.
#104: FILE: /tmp/f1-13700/bgp_tx_addpath.c:104:
+u_int32_t txaddpath_id_for_peer(struct peer *peer, afi_t afi, safi_t safi,

WARNING: braces {} are not necessary for any arm of this statement
#107: FILE: /tmp/f1-13700/bgp_tx_addpath.c:107:
+	if (peer->addpath_type[afi][safi] < BGP_ADDPATH_MAX) {
[...]
+	} else {
[...]

WARNING: Missing a blank line after declarations
#121: FILE: /tmp/f1-13700/bgp_tx_addpath.c:121:
+	int i;
+	for (i = 0; i < BGP_ADDPATH_MAX; i++) {

WARNING: braces {} are not necessary for single statement blocks
#122: FILE: /tmp/f1-13700/bgp_tx_addpath.c:122:
+		if (d->addpath_tx_id != 0) {
+			return 1;
+		}

WARNING: break is not useful after a goto or return
#163: FILE: /tmp/f1-13700/bgp_tx_addpath.c:163:
+		return 0;
+		break;

WARNING: break is not useful after a goto or return
#166: FILE: /tmp/f1-13700/bgp_tx_addpath.c:166:
+		return 1;
+		break;

WARNING: braces {} are not necessary for any arm of this statement
#168: FILE: /tmp/f1-13700/bgp_tx_addpath.c:168:
+		if (CHECK_FLAG(ri->flags, BGP_INFO_DMED_SELECTED)) {
[...]
+		} else {
[...]

WARNING: Missing a blank line after declarations
#227: FILE: /tmp/f1-13700/bgp_tx_addpath.c:227:
+	char buf[200];
+	snprintf(buf, sizeof(buf), "Addpath ID Allocator %s:%d/%d",

ERROR: trailing whitespace
#236: FILE: /tmp/f1-13700/bgp_tx_addpath.c:236:
+^I$

WARNING: braces {} are not necessary for single statement blocks
#263: FILE: /tmp/f1-13700/bgp_tx_addpath.c:263:
+	if (old_strat == new_strat) {
+		return;
+	}

WARNING: braces {} are not necessary for single statement blocks
#271: FILE: /tmp/f1-13700/bgp_tx_addpath.c:271:
+		if (bgp->tx_addpath.peercount[afi][safi][old_strat] == 0) {
+			txaddpath_flush_type(bgp, afi, safi, old_strat);
+		}

WARNING: braces {} are not necessary for single statement blocks
#277: FILE: /tmp/f1-13700/bgp_tx_addpath.c:277:
+		if (bgp->tx_addpath.peercount[afi][safi][new_strat] == 0) {
+			txaddpath_populate_type(bgp, afi, safi, new_strat);
+		}

WARNING: C99 // comments do not match recommendation
#302: FILE: /tmp/f1-13700/bgp_tx_addpath.c:302:
+		// We only care about counting peers configured to each type

WARNING: braces {} are not necessary for single statement blocks
#306: FILE: /tmp/f1-13700/bgp_tx_addpath.c:306:
+	if (addpath_type == old_type) {
+		return;
+	}

WARNING: quoted string split across lines
#319: FILE: /tmp/f1-13700/bgp_tx_addpath.c:319:
+					"%s: enabling bgp deterministic-med, this is required"
+					" for addpath-tx-bestpath-per-AS",

ERROR: code indent should use tabs where possible
#328: FILE: /tmp/f1-13700/bgp_tx_addpath.c:328:
+^I          CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) ? "group ": "",$

ERROR: spaces required around that ':' (ctx:VxW)
#328: FILE: /tmp/f1-13700/bgp_tx_addpath.c:328:
+	          CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) ? "group ": "",
 	                                                                ^

ERROR: code indent should use tabs where possible
#329: FILE: /tmp/f1-13700/bgp_tx_addpath.c:329:
+^I          peer->host);$

WARNING: Block comments use a trailing */ on a separate line
#336: FILE: /tmp/f1-13700/bgp_tx_addpath.c:336:
+		 * the group was configured to tx addpaths. */

WARNING: braces {} are not necessary for single statement blocks
#363: FILE: /tmp/f1-13700/bgp_tx_addpath.c:363:
+		if (bgp->tx_addpath.peercount[afi][safi][i] == 0) {
+			continue;
+		}

WARNING: C99 // comments do not match recommendation
#367: FILE: /tmp/f1-13700/bgp_tx_addpath.c:367:
+		// Free Unused IDs back to the pool.

WARNING: C99 // comments do not match recommendation
#379: FILE: /tmp/f1-13700/bgp_tx_addpath.c:379:
+		// Give IDs to paths that need them (pulling from the pool)

WARNING: line over 80 characters
#387: FILE: /tmp/f1-13700/bgp_tx_addpath.c:387:
+							.id_allocators[afi][safi][i],

WARNING: C99 // comments do not match recommendation
#392: FILE: /tmp/f1-13700/bgp_tx_addpath.c:392:
+		// Free any IDs left in the pool to the main allocator
Report for bgp_tx_addpath.h | 8 issues
===============================================
ERROR: Please, no nonstandard integer types in new code.
#48: FILE: /tmp/f1-13700/bgp_tx_addpath.h:48:
+u_int32_t txaddpath_id_for_peer(struct peer *peer, afi_t afi, safi_t safi,

WARNING: line over 80 characters
#68: FILE: /tmp/f1-13700/bgp_tx_addpath.h:68:
+void txaddpath_update_ids(struct bgp *bgp, struct bgp_node *bn, afi_t afi, safi_t safi);
Report for bgp_tx_addpath_types.h | 20 issues
===============================================
ERROR: Please, no nonstandard integer types in new code.
#43: FILE: /tmp/f1-13700/bgp_tx_addpath_types.h:43:
+	u_int32_t addpath_tx_id[BGP_ADDPATH_MAX];

WARNING: C99 // comments do not match recommendation
#48: FILE: /tmp/f1-13700/bgp_tx_addpath_types.h:48:
+	const char *human_name;	// used in path detail non-json

WARNING: C99 // comments do not match recommendation
#49: FILE: /tmp/f1-13700/bgp_tx_addpath_types.h:49:
+	const char *human_description; // used in non-json peer descriptions

WARNING: C99 // comments do not match recommendation
#50: FILE: /tmp/f1-13700/bgp_tx_addpath_types.h:50:
+	const char *type_json_name;    // use in json peer listings

WARNING: C99 // comments do not match recommendation
#51: FILE: /tmp/f1-13700/bgp_tx_addpath_types.h:51:
+	const char *id_json_name;      // using in path json output for tx ID#
Report for bgp_updgrp_adv.c | 4 issues
===============================================
< WARNING: line over 80 characters
< #146: FILE: /tmp/f1-13700/bgp_updgrp_adv.c:146:
< WARNING: line over 80 characters
< #149: FILE: /tmp/f1-13700/bgp_updgrp_adv.c:149:
Report for bgp_vty.c | 2 issues
===============================================
< ERROR: code indent should use tabs where possible
< #6136: FILE: /tmp/f1-13700/bgp_vty.c:6136:
Report for id_alloc.c | 115 issues
===============================================
WARNING: externs should be avoided in .c files
#35: FILE: /tmp/f1-13700/id_alloc.c:35:
+uint32_t allocate_id(struct id_alloc *alloc);

WARNING: C99 // comments do not match recommendation
#40: FILE: /tmp/f1-13700/id_alloc.c:40:
+// ints less than 32 bits? Yikes.

ERROR: code indent should use tabs where possible
#51: FILE: /tmp/f1-13700/id_alloc.c:51:
+                      IDALLOC_PAGE_BITS + IDALLOC_SUBDIR_BITS)$

WARNING: please, no spaces at the start of a line
#51: FILE: /tmp/f1-13700/id_alloc.c:51:
+                      IDALLOC_PAGE_BITS + IDALLOC_SUBDIR_BITS)$

ERROR: code indent should use tabs where possible
#53: FILE: /tmp/f1-13700/id_alloc.c:53:
+                      IDALLOC_PAGE_BITS)$

WARNING: please, no spaces at the start of a line
#53: FILE: /tmp/f1-13700/id_alloc.c:53:
+                      IDALLOC_PAGE_BITS)$

WARNING: C99 // comments do not match recommendation
#150: FILE: /tmp/f1-13700/id_alloc.c:150:
+		// first bit in this block of 32 to be freed.

WARNING: C99 // comments do not match recommendation
#156: FILE: /tmp/f1-13700/id_alloc.c:156:
+			// first bit in page freed, add this to the allocator's

WARNING: C99 // comments do not match recommendation
#157: FILE: /tmp/f1-13700/id_alloc.c:157:
+			// list of pages with free space

WARNING: braces {} are not necessary for single statement blocks
#170: FILE: /tmp/f1-13700/id_alloc.c:170:
+	if (alloc->capacity == 0 && alloc->sublevels[0]) {
+		return NULL; // All IDs allocated and the capacity looped.
+	}

WARNING: C99 // comments do not match recommendation
#171: FILE: /tmp/f1-13700/id_alloc.c:171:
+		return NULL; // All IDs allocated and the capacity looped.

WARNING: C99 // comments do not match recommendation
#197: FILE: /tmp/f1-13700/id_alloc.c:197:
+				// allocate always pulls from alloc->has_free

WARNING: C99 // comments do not match recommendation
#200: FILE: /tmp/f1-13700/id_alloc.c:200:
+				// reserve could pull from any page with free

WARNING: C99 // comments do not match recommendation
#201: FILE: /tmp/f1-13700/id_alloc.c:201:
+				// bits

WARNING: braces {} are not necessary for single statement blocks
#227: FILE: /tmp/f1-13700/id_alloc.c:227:
+	if (alloc->has_free == NULL) {
+		create_next_page(alloc);
+	}

WARNING: braces {} are not necessary for single statement blocks
#271: FILE: /tmp/f1-13700/id_alloc.c:271:
+	while (alloc->capacity <= id) {
+		create_next_page(alloc);
+	}

WARNING: C99 // comments do not match recommendation
#278: FILE: /tmp/f1-13700/id_alloc.c:278:
+	// page can't be null because the loop above ensured it was created.

WARNING: Missing a blank line after declarations
#312: FILE: /tmp/f1-13700/id_alloc.c:312:
+	int i;
+	for (i = 0; i < IDALLOC_PAGE_COUNT; i++) {

WARNING: braces {} are not necessary for any arm of this statement
#313: FILE: /tmp/f1-13700/id_alloc.c:313:
+		if (subdir->sublevels[i]) {
[...]
+		} else {
[...]

WARNING: Missing a blank line after declarations
#328: FILE: /tmp/f1-13700/id_alloc.c:328:
+	int i;
+	for (i = 0; i < IDALLOC_SUBDIR_COUNT; i++) {

WARNING: braces {} are not necessary for any arm of this statement
#329: FILE: /tmp/f1-13700/id_alloc.c:329:
+		if (dir->sublevels[i]) {
[...]
+		} else {
[...]

WARNING: Missing a blank line after declarations
#344: FILE: /tmp/f1-13700/id_alloc.c:344:
+	int i;
+	for (i = 0; i < IDALLOC_DIR_COUNT; i++) {

WARNING: braces {} are not necessary for any arm of this statement
#345: FILE: /tmp/f1-13700/id_alloc.c:345:
+		if (alloc->sublevels[i]) {
[...]
+		} else {
[...]

WARNING: Missing a blank line after declarations
#374: FILE: /tmp/f1-13700/id_alloc.c:374:
+	struct id_alloc_pool *current, *next;
+	while (*pool_ptr) {
Report for id_alloc.h | 20 issues
===============================================
WARNING: C99 // comments do not match recommendation
#41: FILE: /tmp/f1-13700/id_alloc.h:41:
+	// Bitmask of allocations. 1s indicates the ID is already allocated.

WARNING: C99 // comments do not match recommendation
#44: FILE: /tmp/f1-13700/id_alloc.h:44:
+	// Bitmask for free space in allocated_mask. 1s indicate whole 32 bit

WARNING: C99 // comments do not match recommendation
#45: FILE: /tmp/f1-13700/id_alloc.h:45:
+	// section is full.

WARNING: C99 // comments do not match recommendation
#48: FILE: /tmp/f1-13700/id_alloc.h:48:
+	// The ID that bit 0 in allocated_mask corresponds to.

WARNING: C99 // comments do not match recommendation
#52: FILE: /tmp/f1-13700/id_alloc.h:52:
+		*next_has_free; // Next page with at least one bit open
Report for test_idalloc.c | 18 issues
===============================================
WARNING: C99 // comments do not match recommendation
#17: FILE: /tmp/f1-13700/test_idalloc.c:17:
+	// 1. Rattle test, shake it a little and make sure it doesn't make any

WARNING: C99 // comments do not match recommendation
#18: FILE: /tmp/f1-13700/test_idalloc.c:18:
+	// noise :)

ERROR: spaces required around that '=' (ctx:VxV)
#20: FILE: /tmp/f1-13700/test_idalloc.c:20:
+	for (i=0; i<1000000; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#20: FILE: /tmp/f1-13700/test_idalloc.c:20:
+	for (i=0; i<1000000; i++) {
 	           ^

WARNING: braces {} are not necessary for single statement blocks
#20: FILE: /tmp/f1-13700/test_idalloc.c:20:
+	for (i=0; i<1000000; i++) {
+		assert(idalloc_allocate(a) != 0);
+	}

WARNING: line over 80 characters
#25: FILE: /tmp/f1-13700/test_idalloc.c:25:
+	// 2. Reserve a few low IDs, make sure they are skipped by normal allocation.

WARNING: C99 // comments do not match recommendation
#25: FILE: /tmp/f1-13700/test_idalloc.c:25:
+	// 2. Reserve a few low IDs, make sure they are skipped by normal allocation.

ERROR: spaces required around that '=' (ctx:VxV)
#30: FILE: /tmp/f1-13700/test_idalloc.c:30:
+	for (i=0; i<100; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#30: FILE: /tmp/f1-13700/test_idalloc.c:30:
+	for (i=0; i<100; i++) {
 	           ^

WARNING: line over 80 characters
#36: FILE: /tmp/f1-13700/test_idalloc.c:36:
+	// 3. Single page testing. Check that IDs are kept unique, and all IDs in the

WARNING: C99 // comments do not match recommendation
#36: FILE: /tmp/f1-13700/test_idalloc.c:36:
+	// 3. Single page testing. Check that IDs are kept unique, and all IDs in the

WARNING: C99 // comments do not match recommendation
#37: FILE: /tmp/f1-13700/test_idalloc.c:37:
+	// existing page are allocated before a new page is added.

WARNING: C99 // comments do not match recommendation
#43: FILE: /tmp/f1-13700/test_idalloc.c:43:
+	// reserve the rest of the first page

ERROR: spaces required around that '=' (ctx:VxV)
#44: FILE: /tmp/f1-13700/test_idalloc.c:44:
+	for (i=0; i<IDS_PER_PAGE-1; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#44: FILE: /tmp/f1-13700/test_idalloc.c:44:
+	for (i=0; i<IDS_PER_PAGE-1; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#51: FILE: /tmp/f1-13700/test_idalloc.c:51:
+	// Check that the count is right

WARNING: C99 // comments do not match recommendation
#54: FILE: /tmp/f1-13700/test_idalloc.c:54:
+	// Free some IDs out of the middle.

ERROR: trailing whitespace
#61: FILE: /tmp/f1-13700/test_idalloc.c:61:
+^I$

WARNING: line over 80 characters
#64: FILE: /tmp/f1-13700/test_idalloc.c:64:
+	// Allocate the three IDs back and make sure they are pulled from the set just freed

WARNING: C99 // comments do not match recommendation
#64: FILE: /tmp/f1-13700/test_idalloc.c:64:
+	// Allocate the three IDs back and make sure they are pulled from the set just freed

ERROR: spaces required around that '=' (ctx:VxV)
#65: FILE: /tmp/f1-13700/test_idalloc.c:65:
+	for (i=0; i<3; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#65: FILE: /tmp/f1-13700/test_idalloc.c:65:
+	for (i=0; i<3; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#74: FILE: /tmp/f1-13700/test_idalloc.c:74:
+	// 4. Multi-page testing.

WARNING: C99 // comments do not match recommendation
#80: FILE: /tmp/f1-13700/test_idalloc.c:80:
+	// reserve the rest of the first page and all of the second and third

ERROR: spaces required around that '=' (ctx:VxV)
#81: FILE: /tmp/f1-13700/test_idalloc.c:81:
+	for (i=0; i<3*IDS_PER_PAGE-1; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#81: FILE: /tmp/f1-13700/test_idalloc.c:81:
+	for (i=0; i<3*IDS_PER_PAGE-1; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#90: FILE: /tmp/f1-13700/test_idalloc.c:90:
+	// Free two IDs from each page.

ERROR: spaces required around that '=' (ctx:VxV)
#91: FILE: /tmp/f1-13700/test_idalloc.c:91:
+	for (i=0; i<3; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#91: FILE: /tmp/f1-13700/test_idalloc.c:91:
+	for (i=0; i<3; i++) {
 	           ^

ERROR: trailing whitespace
#98: FILE: /tmp/f1-13700/test_idalloc.c:98:
+^I$

WARNING: line over 80 characters
#101: FILE: /tmp/f1-13700/test_idalloc.c:101:
+	// Allocate the six IDs back and make sure they are pulled from the set just freed.

WARNING: C99 // comments do not match recommendation
#101: FILE: /tmp/f1-13700/test_idalloc.c:101:
+	// Allocate the six IDs back and make sure they are pulled from the set just freed.

ERROR: spaces required around that '=' (ctx:VxV)
#102: FILE: /tmp/f1-13700/test_idalloc.c:102:
+	for (i=0; i<6; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#102: FILE: /tmp/f1-13700/test_idalloc.c:102:
+	for (i=0; i<6; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#113: FILE: /tmp/f1-13700/test_idalloc.c:113:
+	// Walk each allocated ID. Free it, then re-allocate it back.

ERROR: spaces required around that '=' (ctx:VxV)
#114: FILE: /tmp/f1-13700/test_idalloc.c:114:
+	for (i=1; i<3*IDS_PER_PAGE-1; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#114: FILE: /tmp/f1-13700/test_idalloc.c:114:
+	for (i=1; i<3*IDS_PER_PAGE-1; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#123: FILE: /tmp/f1-13700/test_idalloc.c:123:
+	// 5. Weird Reservations

WARNING: line over 80 characters
#124: FILE: /tmp/f1-13700/test_idalloc.c:124:
+	// idalloc_reserve exists primarily to black out low numbered IDs that are

WARNING: C99 // comments do not match recommendation
#124: FILE: /tmp/f1-13700/test_idalloc.c:124:
+	// idalloc_reserve exists primarily to black out low numbered IDs that are

WARNING: C99 // comments do not match recommendation
#125: FILE: /tmp/f1-13700/test_idalloc.c:125:
+	// reserved for special cases. However, we will test it for more complex

WARNING: C99 // comments do not match recommendation
#126: FILE: /tmp/f1-13700/test_idalloc.c:126:
+	// use cases to avoid unpleasant surprises.

WARNING: C99 // comments do not match recommendation
#133: FILE: /tmp/f1-13700/test_idalloc.c:133:
+	// Start with 3 pages fully allocated.

ERROR: spaces required around that '=' (ctx:VxV)
#134: FILE: /tmp/f1-13700/test_idalloc.c:134:
+	for (i=0; i<3*IDS_PER_PAGE-1; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#134: FILE: /tmp/f1-13700/test_idalloc.c:134:
+	for (i=0; i<3*IDS_PER_PAGE-1; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#143: FILE: /tmp/f1-13700/test_idalloc.c:143:
+	// Free a bit out of each of the three pages. Then reserve one of the

WARNING: C99 // comments do not match recommendation
#144: FILE: /tmp/f1-13700/test_idalloc.c:144:
+	// three freed IDs. Finally, allocate the other two freed IDs. Do this

WARNING: C99 // comments do not match recommendation
#145: FILE: /tmp/f1-13700/test_idalloc.c:145:
+	// each of three ways. (Reserve out of the first, seconds then third

WARNING: C99 // comments do not match recommendation
#146: FILE: /tmp/f1-13700/test_idalloc.c:146:
+	// page.)

WARNING: C99 // comments do not match recommendation
#147: FILE: /tmp/f1-13700/test_idalloc.c:147:
+	// The intent here is to exercise the rare cases on reserve_bit's

WARNING: C99 // comments do not match recommendation
#148: FILE: /tmp/f1-13700/test_idalloc.c:148:
+	// linked-list removal in the case that it is not removing the first

WARNING: C99 // comments do not match recommendation
#149: FILE: /tmp/f1-13700/test_idalloc.c:149:
+	// page with a free bit in its list of pages with free bits.

ERROR: spaces required around that '=' (ctx:VxV)
#151: FILE: /tmp/f1-13700/test_idalloc.c:151:
+	for (pg=0; pg<3; pg++) {
 	       ^

ERROR: spaces required around that '<' (ctx:VxV)
#151: FILE: /tmp/f1-13700/test_idalloc.c:151:
+	for (pg=0; pg<3; pg++) {
 	             ^

WARNING: C99 // comments do not match recommendation
#152: FILE: /tmp/f1-13700/test_idalloc.c:152:
+		// free a bit out of each of the three pages

ERROR: spaces required around that '=' (ctx:VxV)
#153: FILE: /tmp/f1-13700/test_idalloc.c:153:
+		for (i=0; i<3; i++) {
 		      ^

ERROR: spaces required around that '<' (ctx:VxV)
#153: FILE: /tmp/f1-13700/test_idalloc.c:153:
+		for (i=0; i<3; i++) {
 		           ^

WARNING: C99 // comments do not match recommendation
#161: FILE: /tmp/f1-13700/test_idalloc.c:161:
+		// Reserve one of the freed IDs

WARNING: line over 80 characters
#162: FILE: /tmp/f1-13700/test_idalloc.c:162:
+		assert(idalloc_reserve(a, pg*IDS_PER_PAGE + 17) == pg*IDS_PER_PAGE + 17);

WARNING: C99 // comments do not match recommendation
#168: FILE: /tmp/f1-13700/test_idalloc.c:168:
+		// Allocate the other two back

ERROR: spaces required around that '=' (ctx:VxV)
#169: FILE: /tmp/f1-13700/test_idalloc.c:169:
+		for (i=0; i<2; i++) {
 		      ^

ERROR: spaces required around that '<' (ctx:VxV)
#169: FILE: /tmp/f1-13700/test_idalloc.c:169:
+		for (i=0; i<2; i++) {
 		           ^

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: FAILED

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

This is a comment from an EXPERIMENTAL 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 and apply patch from patchwork: Successful

Building Stage: Failed

Ubuntu1204 amd64 build: Successful

CentOS6 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI006BUILD/ErrorLog/log_pytests.txt
CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI006BUILD/config.status/config.status

Debian9 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI021BUILD/ErrorLog/log_pytests.txt
Debian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI021BUILD/config.status/config.status

FreeBSD9 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI004BUILD/ErrorLog/log_pytests.txt
FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI004BUILD/config.status/config.status

OmniOS amd64 build: Failed

Make failed for OmniOS amd64 build:
(see full make log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI010BUILD/ErrorLog/log_make.txt)

                 from rfp_internal.h:25,
                 from rfp_example.c:22:
../../../bgpd/bgp_tx_addpath_types.h:43:2: error: unknown type name u_int32_t
  u_int32_t addpath_tx_id[BGP_ADDPATH_MAX];
  ^
Makefile:452: recipe for target 'rfp_example.o' failed
gmake[2]: Leaving directory '/export/home/ci/cibuild.5359/frr-source/bgpd/rfp-example/librfp'
gmake[2]: *** [rfp_example.o] Error 1
Makefile:4579: recipe for target 'all-recursive' failed

OmniOS amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI010BUILD/config.status/config.status

Ubuntu 16.04 i386: Failed

DejaGNU Unittests (make check) failed for Ubuntu 16.04 i386
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/U1604I386/ErrorLog/log_pytests.txt
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/U1604I386/config.status/config.status

NetBSD6 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI007BUILD/ErrorLog/log_pytests.txt
NetBSD6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI007BUILD/config.status/config.status

CentOS7 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI005BUILD/ErrorLog/log_pytests.txt
CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI005BUILD/config.status/config.status

Debian8 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian8 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI008BLD/ErrorLog/log_pytests.txt
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI008BLD/config.status/config.status

FreeBSD10 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD10 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI003BUILD/ErrorLog/log_pytests.txt
FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI003BUILD/config.status/config.status

Ubuntu1404 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1404 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI001BUILD/ErrorLog/log_pytests.txt
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI001BUILD/config.status/config.status

OpenBSD60 amd64 build: Failed

DejaGNU Unittests (make check) failed for OpenBSD60 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI011BUILD/ErrorLog/log_pytests.txt
OpenBSD60 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI011BUILD/config.status/config.status

NetBSD7 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI012BUILD/ErrorLog/log_pytests.txt
NetBSD7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI012BUILD/config.status/config.status

Fedora24 amd64 build: Failed

DejaGNU Unittests (make check) failed for Fedora24 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI015BUILD/ErrorLog/log_pytests.txt
Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI015BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1604 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI014BUILD/ErrorLog/log_pytests.txt
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI014BUILD/config.status/config.status

Ubuntu 18.04 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu 18.04 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/U1804AMD64/ErrorLog/log_pytests.txt
Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/U1804AMD64/config.status/config.status

FreeBSD11 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD11 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI009BUILD/ErrorLog/log_pytests.txt
FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5359/artifact/CI009BUILD/config.status/config.status

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: FAILED

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

This is a comment from an EXPERIMENTAL 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 and apply patch from patchwork: Successful

Building Stage: Failed

Ubuntu1204 amd64 build: Successful

CentOS6 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI006BUILD/ErrorLog/log_pytests.txt
CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI006BUILD/config.status/config.status

FreeBSD9 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI004BUILD/ErrorLog/log_pytests.txt
FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI004BUILD/config.status/config.status

Ubuntu 16.04 i386: Failed

DejaGNU Unittests (make check) failed for Ubuntu 16.04 i386
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/U1604I386/ErrorLog/log_pytests.txt
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/U1604I386/config.status/config.status

CentOS7 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI005BUILD/ErrorLog/log_pytests.txt
CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI005BUILD/config.status/config.status

FreeBSD10 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD10 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI003BUILD/ErrorLog/log_pytests.txt
FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI003BUILD/config.status/config.status

Ubuntu1404 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1404 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI001BUILD/ErrorLog/log_pytests.txt
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI001BUILD/config.status/config.status

Debian8 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian8 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI008BLD/ErrorLog/log_pytests.txt
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI008BLD/config.status/config.status

OmniOS amd64 build: Failed

Make failed for OmniOS amd64 build:
(see full make log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI010BUILD/ErrorLog/log_make.txt)

checking for minix/config.h... no
checking whether it is safe to define __EXTENSIONS__... yes
checking whether gcc supports -diag-error 10006... no
checking whether gcc supports -std=gnu11... yes
checking whether gcc -std=gnu11 supports -g... yes
checking whether gcc -std=gnu11 supports -Os... yes
checking whether gcc -std=gnu11 supports -fno-omit-frame-pointer... yes
checking whether gcc -std=gnu11 supports -funwind-tables... yes
checking whether gcc -std=gnu11 supports -Wall... yes

OmniOS amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI010BUILD/config.status/config.status

OpenBSD60 amd64 build: Failed

DejaGNU Unittests (make check) failed for OpenBSD60 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI011BUILD/ErrorLog/log_pytests.txt
OpenBSD60 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI011BUILD/config.status/config.status

NetBSD7 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI012BUILD/ErrorLog/log_pytests.txt
NetBSD7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI012BUILD/config.status/config.status

Debian9 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI021BUILD/ErrorLog/log_pytests.txt
Debian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI021BUILD/config.status/config.status

FreeBSD11 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD11 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI009BUILD/ErrorLog/log_pytests.txt
FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI009BUILD/config.status/config.status

Fedora24 amd64 build: Failed

DejaGNU Unittests (make check) failed for Fedora24 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI015BUILD/ErrorLog/log_pytests.txt
Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI015BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1604 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI014BUILD/ErrorLog/log_pytests.txt
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI014BUILD/config.status/config.status

Ubuntu 18.04 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu 18.04 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/U1804AMD64/ErrorLog/log_pytests.txt
Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/U1804AMD64/config.status/config.status

NetBSD6 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI007BUILD/ErrorLog/log_pytests.txt
NetBSD6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5361/artifact/CI007BUILD/config.status/config.status

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: FAILED

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

This is a comment from an EXPERIMENTAL 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 and apply patch from patchwork: Successful

Building Stage: Failed

Ubuntu1204 amd64 build: Successful

FreeBSD9 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI004BUILD/ErrorLog/log_pytests.txt
FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI004BUILD/config.status/config.status

Ubuntu 16.04 i386: Failed

DejaGNU Unittests (make check) failed for Ubuntu 16.04 i386
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/U1604I386/ErrorLog/log_pytests.txt
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/U1604I386/config.status/config.status

Ubuntu 18.04 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu 18.04 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/U1804AMD64/ErrorLog/log_pytests.txt
Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/U1804AMD64/config.status/config.status

CentOS7 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI005BUILD/ErrorLog/log_pytests.txt
CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI005BUILD/config.status/config.status

FreeBSD10 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD10 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI003BUILD/ErrorLog/log_pytests.txt
FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI003BUILD/config.status/config.status

Ubuntu1404 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1404 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI001BUILD/ErrorLog/log_pytests.txt
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI001BUILD/config.status/config.status

Debian8 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian8 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI008BLD/ErrorLog/log_pytests.txt
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI008BLD/config.status/config.status

OmniOS amd64 build: Failed

Make failed for OmniOS amd64 build:
(see full make log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI010BUILD/ErrorLog/log_make.txt)

gmake[2]: Entering directory '/export/home/ci/cibuild.5362/frr-source'
  CC       ripd/rip_debug.o
  CC       ripd/rip_errors.o
  CC       ripd/rip_interface.o
  CC       ripd/rip_memory.o
  CC       ripd/rip_offset.o
  CC       ripd/rip_peer.o
  CC       ripd/rip_routemap.o
  CC       ripd/rip_zebra.o

OmniOS amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI010BUILD/config.status/config.status

OpenBSD60 amd64 build: Failed

DejaGNU Unittests (make check) failed for OpenBSD60 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI011BUILD/ErrorLog/log_pytests.txt
OpenBSD60 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI011BUILD/config.status/config.status

Debian9 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI021BUILD/ErrorLog/log_pytests.txt
Debian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI021BUILD/config.status/config.status

NetBSD7 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI012BUILD/ErrorLog/log_pytests.txt
NetBSD7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI012BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1604 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI014BUILD/ErrorLog/log_pytests.txt
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI014BUILD/config.status/config.status

FreeBSD11 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD11 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI009BUILD/ErrorLog/log_pytests.txt
FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI009BUILD/config.status/config.status

CentOS6 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI006BUILD/ErrorLog/log_pytests.txt
CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI006BUILD/config.status/config.status

NetBSD6 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI007BUILD/ErrorLog/log_pytests.txt
NetBSD6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI007BUILD/config.status/config.status

Fedora24 amd64 build: Failed

DejaGNU Unittests (make check) failed for Fedora24 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI015BUILD/ErrorLog/log_pytests.txt
Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI015BUILD/config.status/config.status


Warnings Generated during build:

Checkout code: Successful with additional warnings:

FreeBSD9 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI004BUILD/ErrorLog/log_pytests.txt
FreeBSD9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI004BUILD/config.status/config.status

Ubuntu 16.04 i386: Failed

DejaGNU Unittests (make check) failed for Ubuntu 16.04 i386
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/U1604I386/ErrorLog/log_pytests.txt
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/U1604I386/config.status/config.status

Ubuntu 18.04 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu 18.04 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/U1804AMD64/ErrorLog/log_pytests.txt
Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/U1804AMD64/config.status/config.status

CentOS7 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI005BUILD/ErrorLog/log_pytests.txt
CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI005BUILD/config.status/config.status

FreeBSD10 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD10 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI003BUILD/ErrorLog/log_pytests.txt
FreeBSD10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI003BUILD/config.status/config.status

Ubuntu1404 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1404 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI001BUILD/ErrorLog/log_pytests.txt
Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI001BUILD/config.status/config.status

Debian8 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian8 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI008BLD/ErrorLog/log_pytests.txt
Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI008BLD/config.status/config.status

OmniOS amd64 build: Failed

Make failed for OmniOS amd64 build:
(see full make log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI010BUILD/ErrorLog/log_make.txt)

gmake[2]: Entering directory '/export/home/ci/cibuild.5362/frr-source'
  CC       ripd/rip_debug.o
  CC       ripd/rip_errors.o
  CC       ripd/rip_interface.o
  CC       ripd/rip_memory.o
  CC       ripd/rip_offset.o
  CC       ripd/rip_peer.o
  CC       ripd/rip_routemap.o
  CC       ripd/rip_zebra.o

OmniOS amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI010BUILD/config.status/config.status

OpenBSD60 amd64 build: Failed

DejaGNU Unittests (make check) failed for OpenBSD60 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI011BUILD/ErrorLog/log_pytests.txt
OpenBSD60 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI011BUILD/config.status/config.status

Debian9 amd64 build: Failed

DejaGNU Unittests (make check) failed for Debian9 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI021BUILD/ErrorLog/log_pytests.txt
Debian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI021BUILD/config.status/config.status

NetBSD7 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD7 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI012BUILD/ErrorLog/log_pytests.txt
NetBSD7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI012BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

DejaGNU Unittests (make check) failed for Ubuntu1604 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI014BUILD/ErrorLog/log_pytests.txt
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI014BUILD/config.status/config.status

FreeBSD11 amd64 build: Failed

DejaGNU Unittests (make check) failed for FreeBSD11 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI009BUILD/ErrorLog/log_pytests.txt
FreeBSD11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI009BUILD/config.status/config.status

CentOS6 amd64 build: Failed

DejaGNU Unittests (make check) failed for CentOS6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI006BUILD/ErrorLog/log_pytests.txt
CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI006BUILD/config.status/config.status

NetBSD6 amd64 build: Failed

DejaGNU Unittests (make check) failed for NetBSD6 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI007BUILD/ErrorLog/log_pytests.txt
NetBSD6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI007BUILD/config.status/config.status

Fedora24 amd64 build: Failed

DejaGNU Unittests (make check) failed for Fedora24 amd64 build
see PyTest log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI015BUILD/ErrorLog/log_pytests.txt
Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5362/artifact/CI015BUILD/config.status/config.status

<stdin>:544: trailing whitespace.
	
<stdin>:2098: trailing whitespace.
	
<stdin>:2135: trailing whitespace.
	
warning: 3 lines add whitespace errors.
Report for bgpd.c | 14 issues
===============================================
< WARNING: line over 80 characters
< #887: FILE: /tmp/f1-29186/bgpd.c:887:
< WARNING: braces {} are not necessary for any arm of this statement
< #898: FILE: /tmp/f1-29186/bgpd.c:898:
< WARNING: braces {} are not necessary for single statement blocks
< #1064: FILE: /tmp/f1-29186/bgpd.c:1064:
< WARNING: line over 80 characters
< #1065: FILE: /tmp/f1-29186/bgpd.c:1065:
< ERROR: code indent should use tabs where possible
< #1842: FILE: /tmp/f1-29186/bgpd.c:1842:
< ERROR: code indent should use tabs where possible
< #7020: FILE: /tmp/f1-29186/bgpd.c:7020:
< ERROR: code indent should use tabs where possible
< #7024: FILE: /tmp/f1-29186/bgpd.c:7024:
Report for bgp_open.c | 2 issues
===============================================
< WARNING: braces {} are not necessary for single statement blocks
< #1380: FILE: /tmp/f1-29186/bgp_open.c:1380:
Report for bgp_route.c | 6 issues
===============================================
< WARNING: line over 80 characters
< #2103: FILE: /tmp/f1-29186/bgp_route.c:2103:
< WARNING: Block comments use a trailing */ on a separate line
< #8067: FILE: /tmp/f1-29186/bgp_route.c:8067:
< WARNING: Block comments use a trailing */ on a separate line
< #8085: FILE: /tmp/f1-29186/bgp_route.c:8085:
Report for bgp_tx_addpath.c | 136 issues
===============================================
ERROR: space required after that close brace '}'
#33: FILE: /tmp/f1-29186/bgp_tx_addpath.c:33:
+	 .id_json_name = "addpathTxIdBestPerAS"}};

WARNING: braces {} are not necessary for any arm of this statement
#49: FILE: /tmp/f1-29186/bgp_tx_addpath.c:49:
+	if (strat < BGP_ADDPATH_MAX) {
[...]
+	} else {
[...]

WARNING: Missing a blank line after declarations
#92: FILE: /tmp/f1-29186/bgp_tx_addpath.c:92:
+	int i;
+	for (i = 0; i < BGP_ADDPATH_MAX; i++) {

ERROR: Please, no nonstandard integer types in new code.
#104: FILE: /tmp/f1-29186/bgp_tx_addpath.c:104:
+u_int32_t txaddpath_id_for_peer(struct peer *peer, afi_t afi, safi_t safi,

WARNING: braces {} are not necessary for any arm of this statement
#107: FILE: /tmp/f1-29186/bgp_tx_addpath.c:107:
+	if (peer->addpath_type[afi][safi] < BGP_ADDPATH_MAX) {
[...]
+	} else {
[...]

WARNING: Missing a blank line after declarations
#121: FILE: /tmp/f1-29186/bgp_tx_addpath.c:121:
+	int i;
+	for (i = 0; i < BGP_ADDPATH_MAX; i++) {

WARNING: braces {} are not necessary for single statement blocks
#122: FILE: /tmp/f1-29186/bgp_tx_addpath.c:122:
+		if (d->addpath_tx_id != 0) {
+			return 1;
+		}

WARNING: break is not useful after a goto or return
#163: FILE: /tmp/f1-29186/bgp_tx_addpath.c:163:
+		return 0;
+		break;

WARNING: break is not useful after a goto or return
#166: FILE: /tmp/f1-29186/bgp_tx_addpath.c:166:
+		return 1;
+		break;

WARNING: braces {} are not necessary for any arm of this statement
#168: FILE: /tmp/f1-29186/bgp_tx_addpath.c:168:
+		if (CHECK_FLAG(ri->flags, BGP_INFO_DMED_SELECTED)) {
[...]
+		} else {
[...]

WARNING: Missing a blank line after declarations
#227: FILE: /tmp/f1-29186/bgp_tx_addpath.c:227:
+	char buf[200];
+	snprintf(buf, sizeof(buf), "Addpath ID Allocator %s:%d/%d",

ERROR: trailing whitespace
#236: FILE: /tmp/f1-29186/bgp_tx_addpath.c:236:
+^I$

WARNING: braces {} are not necessary for single statement blocks
#263: FILE: /tmp/f1-29186/bgp_tx_addpath.c:263:
+	if (old_strat == new_strat) {
+		return;
+	}

WARNING: braces {} are not necessary for single statement blocks
#271: FILE: /tmp/f1-29186/bgp_tx_addpath.c:271:
+		if (bgp->tx_addpath.peercount[afi][safi][old_strat] == 0) {
+			txaddpath_flush_type(bgp, afi, safi, old_strat);
+		}

WARNING: braces {} are not necessary for single statement blocks
#277: FILE: /tmp/f1-29186/bgp_tx_addpath.c:277:
+		if (bgp->tx_addpath.peercount[afi][safi][new_strat] == 0) {
+			txaddpath_populate_type(bgp, afi, safi, new_strat);
+		}

WARNING: C99 // comments do not match recommendation
#302: FILE: /tmp/f1-29186/bgp_tx_addpath.c:302:
+		// We only care about counting peers configured to each type

WARNING: braces {} are not necessary for single statement blocks
#306: FILE: /tmp/f1-29186/bgp_tx_addpath.c:306:
+	if (addpath_type == old_type) {
+		return;
+	}

WARNING: quoted string split across lines
#319: FILE: /tmp/f1-29186/bgp_tx_addpath.c:319:
+					"%s: enabling bgp deterministic-med, this is required"
+					" for addpath-tx-bestpath-per-AS",

ERROR: code indent should use tabs where possible
#328: FILE: /tmp/f1-29186/bgp_tx_addpath.c:328:
+^I          CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) ? "group ": "",$

ERROR: spaces required around that ':' (ctx:VxW)
#328: FILE: /tmp/f1-29186/bgp_tx_addpath.c:328:
+	          CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP) ? "group ": "",
 	                                                                ^

ERROR: code indent should use tabs where possible
#329: FILE: /tmp/f1-29186/bgp_tx_addpath.c:329:
+^I          peer->host);$

WARNING: Block comments use a trailing */ on a separate line
#336: FILE: /tmp/f1-29186/bgp_tx_addpath.c:336:
+		 * the group was configured to tx addpaths. */

WARNING: braces {} are not necessary for single statement blocks
#363: FILE: /tmp/f1-29186/bgp_tx_addpath.c:363:
+		if (bgp->tx_addpath.peercount[afi][safi][i] == 0) {
+			continue;
+		}

WARNING: C99 // comments do not match recommendation
#367: FILE: /tmp/f1-29186/bgp_tx_addpath.c:367:
+		// Free Unused IDs back to the pool.

WARNING: C99 // comments do not match recommendation
#379: FILE: /tmp/f1-29186/bgp_tx_addpath.c:379:
+		// Give IDs to paths that need them (pulling from the pool)

WARNING: line over 80 characters
#387: FILE: /tmp/f1-29186/bgp_tx_addpath.c:387:
+							.id_allocators[afi][safi][i],

WARNING: C99 // comments do not match recommendation
#392: FILE: /tmp/f1-29186/bgp_tx_addpath.c:392:
+		// Free any IDs left in the pool to the main allocator
Report for bgp_tx_addpath.h | 8 issues
===============================================
ERROR: Please, no nonstandard integer types in new code.
#48: FILE: /tmp/f1-29186/bgp_tx_addpath.h:48:
+u_int32_t txaddpath_id_for_peer(struct peer *peer, afi_t afi, safi_t safi,

WARNING: line over 80 characters
#68: FILE: /tmp/f1-29186/bgp_tx_addpath.h:68:
+void txaddpath_update_ids(struct bgp *bgp, struct bgp_node *bn, afi_t afi, safi_t safi);
Report for bgp_tx_addpath_types.h | 20 issues
===============================================
ERROR: Please, no nonstandard integer types in new code.
#44: FILE: /tmp/f1-29186/bgp_tx_addpath_types.h:44:
+	u_int32_t addpath_tx_id[BGP_ADDPATH_MAX];

WARNING: C99 // comments do not match recommendation
#49: FILE: /tmp/f1-29186/bgp_tx_addpath_types.h:49:
+	const char *human_name;	// used in path detail non-json

WARNING: C99 // comments do not match recommendation
#50: FILE: /tmp/f1-29186/bgp_tx_addpath_types.h:50:
+	const char *human_description; // used in non-json peer descriptions

WARNING: C99 // comments do not match recommendation
#51: FILE: /tmp/f1-29186/bgp_tx_addpath_types.h:51:
+	const char *type_json_name;    // use in json peer listings

WARNING: C99 // comments do not match recommendation
#52: FILE: /tmp/f1-29186/bgp_tx_addpath_types.h:52:
+	const char *id_json_name;      // using in path json output for tx ID#
Report for bgp_updgrp_adv.c | 4 issues
===============================================
< WARNING: line over 80 characters
< #146: FILE: /tmp/f1-29186/bgp_updgrp_adv.c:146:
< WARNING: line over 80 characters
< #149: FILE: /tmp/f1-29186/bgp_updgrp_adv.c:149:
Report for bgp_vty.c | 2 issues
===============================================
< ERROR: code indent should use tabs where possible
< #6136: FILE: /tmp/f1-29186/bgp_vty.c:6136:
Report for id_alloc.c | 115 issues
===============================================
WARNING: externs should be avoided in .c files
#35: FILE: /tmp/f1-29186/id_alloc.c:35:
+uint32_t allocate_id(struct id_alloc *alloc);

WARNING: C99 // comments do not match recommendation
#40: FILE: /tmp/f1-29186/id_alloc.c:40:
+// ints less than 32 bits? Yikes.

ERROR: code indent should use tabs where possible
#51: FILE: /tmp/f1-29186/id_alloc.c:51:
+                      IDALLOC_PAGE_BITS + IDALLOC_SUBDIR_BITS)$

WARNING: please, no spaces at the start of a line
#51: FILE: /tmp/f1-29186/id_alloc.c:51:
+                      IDALLOC_PAGE_BITS + IDALLOC_SUBDIR_BITS)$

ERROR: code indent should use tabs where possible
#53: FILE: /tmp/f1-29186/id_alloc.c:53:
+                      IDALLOC_PAGE_BITS)$

WARNING: please, no spaces at the start of a line
#53: FILE: /tmp/f1-29186/id_alloc.c:53:
+                      IDALLOC_PAGE_BITS)$

WARNING: C99 // comments do not match recommendation
#150: FILE: /tmp/f1-29186/id_alloc.c:150:
+		// first bit in this block of 32 to be freed.

WARNING: C99 // comments do not match recommendation
#156: FILE: /tmp/f1-29186/id_alloc.c:156:
+			// first bit in page freed, add this to the allocator's

WARNING: C99 // comments do not match recommendation
#157: FILE: /tmp/f1-29186/id_alloc.c:157:
+			// list of pages with free space

WARNING: braces {} are not necessary for single statement blocks
#170: FILE: /tmp/f1-29186/id_alloc.c:170:
+	if (alloc->capacity == 0 && alloc->sublevels[0]) {
+		return NULL; // All IDs allocated and the capacity looped.
+	}

WARNING: C99 // comments do not match recommendation
#171: FILE: /tmp/f1-29186/id_alloc.c:171:
+		return NULL; // All IDs allocated and the capacity looped.

WARNING: C99 // comments do not match recommendation
#197: FILE: /tmp/f1-29186/id_alloc.c:197:
+				// allocate always pulls from alloc->has_free

WARNING: C99 // comments do not match recommendation
#200: FILE: /tmp/f1-29186/id_alloc.c:200:
+				// reserve could pull from any page with free

WARNING: C99 // comments do not match recommendation
#201: FILE: /tmp/f1-29186/id_alloc.c:201:
+				// bits

WARNING: braces {} are not necessary for single statement blocks
#227: FILE: /tmp/f1-29186/id_alloc.c:227:
+	if (alloc->has_free == NULL) {
+		create_next_page(alloc);
+	}

WARNING: braces {} are not necessary for single statement blocks
#271: FILE: /tmp/f1-29186/id_alloc.c:271:
+	while (alloc->capacity <= id) {
+		create_next_page(alloc);
+	}

WARNING: C99 // comments do not match recommendation
#278: FILE: /tmp/f1-29186/id_alloc.c:278:
+	// page can't be null because the loop above ensured it was created.

WARNING: Missing a blank line after declarations
#312: FILE: /tmp/f1-29186/id_alloc.c:312:
+	int i;
+	for (i = 0; i < IDALLOC_PAGE_COUNT; i++) {

WARNING: braces {} are not necessary for any arm of this statement
#313: FILE: /tmp/f1-29186/id_alloc.c:313:
+		if (subdir->sublevels[i]) {
[...]
+		} else {
[...]

WARNING: Missing a blank line after declarations
#328: FILE: /tmp/f1-29186/id_alloc.c:328:
+	int i;
+	for (i = 0; i < IDALLOC_SUBDIR_COUNT; i++) {

WARNING: braces {} are not necessary for any arm of this statement
#329: FILE: /tmp/f1-29186/id_alloc.c:329:
+		if (dir->sublevels[i]) {
[...]
+		} else {
[...]

WARNING: Missing a blank line after declarations
#344: FILE: /tmp/f1-29186/id_alloc.c:344:
+	int i;
+	for (i = 0; i < IDALLOC_DIR_COUNT; i++) {

WARNING: braces {} are not necessary for any arm of this statement
#345: FILE: /tmp/f1-29186/id_alloc.c:345:
+		if (alloc->sublevels[i]) {
[...]
+		} else {
[...]

WARNING: Missing a blank line after declarations
#374: FILE: /tmp/f1-29186/id_alloc.c:374:
+	struct id_alloc_pool *current, *next;
+	while (*pool_ptr) {
Report for id_alloc.h | 20 issues
===============================================
WARNING: C99 // comments do not match recommendation
#41: FILE: /tmp/f1-29186/id_alloc.h:41:
+	// Bitmask of allocations. 1s indicates the ID is already allocated.

WARNING: C99 // comments do not match recommendation
#44: FILE: /tmp/f1-29186/id_alloc.h:44:
+	// Bitmask for free space in allocated_mask. 1s indicate whole 32 bit

WARNING: C99 // comments do not match recommendation
#45: FILE: /tmp/f1-29186/id_alloc.h:45:
+	// section is full.

WARNING: C99 // comments do not match recommendation
#48: FILE: /tmp/f1-29186/id_alloc.h:48:
+	// The ID that bit 0 in allocated_mask corresponds to.

WARNING: C99 // comments do not match recommendation
#52: FILE: /tmp/f1-29186/id_alloc.h:52:
+		*next_has_free; // Next page with at least one bit open
Report for test_idalloc.c | 18 issues
===============================================
WARNING: C99 // comments do not match recommendation
#17: FILE: /tmp/f1-29186/test_idalloc.c:17:
+	// 1. Rattle test, shake it a little and make sure it doesn't make any

WARNING: C99 // comments do not match recommendation
#18: FILE: /tmp/f1-29186/test_idalloc.c:18:
+	// noise :)

ERROR: spaces required around that '=' (ctx:VxV)
#20: FILE: /tmp/f1-29186/test_idalloc.c:20:
+	for (i=0; i<1000000; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#20: FILE: /tmp/f1-29186/test_idalloc.c:20:
+	for (i=0; i<1000000; i++) {
 	           ^

WARNING: braces {} are not necessary for single statement blocks
#20: FILE: /tmp/f1-29186/test_idalloc.c:20:
+	for (i=0; i<1000000; i++) {
+		assert(idalloc_allocate(a) != 0);
+	}

WARNING: line over 80 characters
#25: FILE: /tmp/f1-29186/test_idalloc.c:25:
+	// 2. Reserve a few low IDs, make sure they are skipped by normal allocation.

WARNING: C99 // comments do not match recommendation
#25: FILE: /tmp/f1-29186/test_idalloc.c:25:
+	// 2. Reserve a few low IDs, make sure they are skipped by normal allocation.

ERROR: spaces required around that '=' (ctx:VxV)
#30: FILE: /tmp/f1-29186/test_idalloc.c:30:
+	for (i=0; i<100; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#30: FILE: /tmp/f1-29186/test_idalloc.c:30:
+	for (i=0; i<100; i++) {
 	           ^

WARNING: line over 80 characters
#36: FILE: /tmp/f1-29186/test_idalloc.c:36:
+	// 3. Single page testing. Check that IDs are kept unique, and all IDs in the

WARNING: C99 // comments do not match recommendation
#36: FILE: /tmp/f1-29186/test_idalloc.c:36:
+	// 3. Single page testing. Check that IDs are kept unique, and all IDs in the

WARNING: C99 // comments do not match recommendation
#37: FILE: /tmp/f1-29186/test_idalloc.c:37:
+	// existing page are allocated before a new page is added.

WARNING: C99 // comments do not match recommendation
#43: FILE: /tmp/f1-29186/test_idalloc.c:43:
+	// reserve the rest of the first page

ERROR: spaces required around that '=' (ctx:VxV)
#44: FILE: /tmp/f1-29186/test_idalloc.c:44:
+	for (i=0; i<IDS_PER_PAGE-1; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#44: FILE: /tmp/f1-29186/test_idalloc.c:44:
+	for (i=0; i<IDS_PER_PAGE-1; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#51: FILE: /tmp/f1-29186/test_idalloc.c:51:
+	// Check that the count is right

WARNING: C99 // comments do not match recommendation
#54: FILE: /tmp/f1-29186/test_idalloc.c:54:
+	// Free some IDs out of the middle.

ERROR: trailing whitespace
#61: FILE: /tmp/f1-29186/test_idalloc.c:61:
+^I$

WARNING: line over 80 characters
#64: FILE: /tmp/f1-29186/test_idalloc.c:64:
+	// Allocate the three IDs back and make sure they are pulled from the set just freed

WARNING: C99 // comments do not match recommendation
#64: FILE: /tmp/f1-29186/test_idalloc.c:64:
+	// Allocate the three IDs back and make sure they are pulled from the set just freed

ERROR: spaces required around that '=' (ctx:VxV)
#65: FILE: /tmp/f1-29186/test_idalloc.c:65:
+	for (i=0; i<3; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#65: FILE: /tmp/f1-29186/test_idalloc.c:65:
+	for (i=0; i<3; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#74: FILE: /tmp/f1-29186/test_idalloc.c:74:
+	// 4. Multi-page testing.

WARNING: C99 // comments do not match recommendation
#80: FILE: /tmp/f1-29186/test_idalloc.c:80:
+	// reserve the rest of the first page and all of the second and third

ERROR: spaces required around that '=' (ctx:VxV)
#81: FILE: /tmp/f1-29186/test_idalloc.c:81:
+	for (i=0; i<3*IDS_PER_PAGE-1; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#81: FILE: /tmp/f1-29186/test_idalloc.c:81:
+	for (i=0; i<3*IDS_PER_PAGE-1; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#90: FILE: /tmp/f1-29186/test_idalloc.c:90:
+	// Free two IDs from each page.

ERROR: spaces required around that '=' (ctx:VxV)
#91: FILE: /tmp/f1-29186/test_idalloc.c:91:
+	for (i=0; i<3; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#91: FILE: /tmp/f1-29186/test_idalloc.c:91:
+	for (i=0; i<3; i++) {
 	           ^

ERROR: trailing whitespace
#98: FILE: /tmp/f1-29186/test_idalloc.c:98:
+^I$

WARNING: line over 80 characters
#101: FILE: /tmp/f1-29186/test_idalloc.c:101:
+	// Allocate the six IDs back and make sure they are pulled from the set just freed.

WARNING: C99 // comments do not match recommendation
#101: FILE: /tmp/f1-29186/test_idalloc.c:101:
+	// Allocate the six IDs back and make sure they are pulled from the set just freed.

ERROR: spaces required around that '=' (ctx:VxV)
#102: FILE: /tmp/f1-29186/test_idalloc.c:102:
+	for (i=0; i<6; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#102: FILE: /tmp/f1-29186/test_idalloc.c:102:
+	for (i=0; i<6; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#113: FILE: /tmp/f1-29186/test_idalloc.c:113:
+	// Walk each allocated ID. Free it, then re-allocate it back.

ERROR: spaces required around that '=' (ctx:VxV)
#114: FILE: /tmp/f1-29186/test_idalloc.c:114:
+	for (i=1; i<3*IDS_PER_PAGE-1; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#114: FILE: /tmp/f1-29186/test_idalloc.c:114:
+	for (i=1; i<3*IDS_PER_PAGE-1; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#123: FILE: /tmp/f1-29186/test_idalloc.c:123:
+	// 5. Weird Reservations

WARNING: line over 80 characters
#124: FILE: /tmp/f1-29186/test_idalloc.c:124:
+	// idalloc_reserve exists primarily to black out low numbered IDs that are

WARNING: C99 // comments do not match recommendation
#124: FILE: /tmp/f1-29186/test_idalloc.c:124:
+	// idalloc_reserve exists primarily to black out low numbered IDs that are

WARNING: C99 // comments do not match recommendation
#125: FILE: /tmp/f1-29186/test_idalloc.c:125:
+	// reserved for special cases. However, we will test it for more complex

WARNING: C99 // comments do not match recommendation
#126: FILE: /tmp/f1-29186/test_idalloc.c:126:
+	// use cases to avoid unpleasant surprises.

WARNING: C99 // comments do not match recommendation
#133: FILE: /tmp/f1-29186/test_idalloc.c:133:
+	// Start with 3 pages fully allocated.

ERROR: spaces required around that '=' (ctx:VxV)
#134: FILE: /tmp/f1-29186/test_idalloc.c:134:
+	for (i=0; i<3*IDS_PER_PAGE-1; i++) {
 	      ^

ERROR: spaces required around that '<' (ctx:VxV)
#134: FILE: /tmp/f1-29186/test_idalloc.c:134:
+	for (i=0; i<3*IDS_PER_PAGE-1; i++) {
 	           ^

WARNING: C99 // comments do not match recommendation
#143: FILE: /tmp/f1-29186/test_idalloc.c:143:
+	// Free a bit out of each of the three pages. Then reserve one of the

WARNING: C99 // comments do not match recommendation
#144: FILE: /tmp/f1-29186/test_idalloc.c:144:
+	// three freed IDs. Finally, allocate the other two freed IDs. Do this

WARNING: C99 // comments do not match recommendation
#145: FILE: /tmp/f1-29186/test_idalloc.c:145:
+	// each of three ways. (Reserve out of the first, seconds then third

WARNING: C99 // comments do not match recommendation
#146: FILE: /tmp/f1-29186/test_idalloc.c:146:
+	// page.)

WARNING: C99 // comments do not match recommendation
#147: FILE: /tmp/f1-29186/test_idalloc.c:147:
+	// The intent here is to exercise the rare cases on reserve_bit's

WARNING: C99 // comments do not match recommendation
#148: FILE: /tmp/f1-29186/test_idalloc.c:148:
+	// linked-list removal in the case that it is not removing the first

WARNING: C99 // comments do not match recommendation
#149: FILE: /tmp/f1-29186/test_idalloc.c:149:
+	// page with a free bit in its list of pages with free bits.

ERROR: spaces required around that '=' (ctx:VxV)
#151: FILE: /tmp/f1-29186/test_idalloc.c:151:
+	for (pg=0; pg<3; pg++) {
 	       ^

ERROR: spaces required around that '<' (ctx:VxV)
#151: FILE: /tmp/f1-29186/test_idalloc.c:151:
+	for (pg=0; pg<3; pg++) {
 	             ^

WARNING: C99 // comments do not match recommendation
#152: FILE: /tmp/f1-29186/test_idalloc.c:152:
+		// free a bit out of each of the three pages

ERROR: spaces required around that '=' (ctx:VxV)
#153: FILE: /tmp/f1-29186/test_idalloc.c:153:
+		for (i=0; i<3; i++) {
 		      ^

ERROR: spaces required around that '<' (ctx:VxV)
#153: FILE: /tmp/f1-29186/test_idalloc.c:153:
+		for (i=0; i<3; i++) {
 		           ^

WARNING: C99 // comments do not match recommendation
#161: FILE: /tmp/f1-29186/test_idalloc.c:161:
+		// Reserve one of the freed IDs

WARNING: line over 80 characters
#162: FILE: /tmp/f1-29186/test_idalloc.c:162:
+		assert(idalloc_reserve(a, pg*IDS_PER_PAGE + 17) == pg*IDS_PER_PAGE + 17);

WARNING: C99 // comments do not match recommendation
#168: FILE: /tmp/f1-29186/test_idalloc.c:168:
+		// Allocate the other two back

ERROR: spaces required around that '=' (ctx:VxV)
#169: FILE: /tmp/f1-29186/test_idalloc.c:169:
+		for (i=0; i<2; i++) {
 		      ^

ERROR: spaces required around that '<' (ctx:VxV)
#169: FILE: /tmp/f1-29186/test_idalloc.c:169:
+		for (i=0; i<2; i++) {
 		           ^

@donaldsharp
Copy link
Member

This is a small behavioral change and as such it should not be a PR against dev/6.0 it should be in master. Looking at the code now otherwise

{
int i;
for (i = 0; i < BGP_ADDPATH_MAX; i++) {
if (d->addpath_tx_id != 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bgpd/bgp_tx_addpath.c:122:10: error: comparison of array 'd->addpath_tx_id' not equal to a null pointer is always true [-Werror,-Wtautological-pointer-compare]
if (d->addpath_tx_id != 0) {
~~~^~~~~~~~~~~~~ ~
1 error generated.


peer = SUBGRP_PEER(subgrp);
afi = SUBGRP_AFI(subgrp);
safi = SUBGRP_AFI(subgrp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be SUBGRP_SAFI?

@donaldsharp
Copy link
Member

donaldsharp commented Sep 24, 2018

Additionally the Signed-off-by: ... line typically needs an email address here and I'd like the warnings cleaned up.

@donaldsharp
Copy link
Member

I have more comments but am having issues with the tool complaining at me. I'm going to wait on resubmittal for more detailed notes, One major change needed is flog_err instead of zlog_error

@mitch-skiba mitch-skiba force-pushed the addpath_change_V1 branch 2 times, most recently from f325ce9 to 8cab244 Compare September 24, 2018 20:27
@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: FAILED

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

This is a comment from an EXPERIMENTAL 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 and apply patch from patchwork: Successful

Building Stage: Failed

OpenBSD60 amd64 build: Successful
FreeBSD11 amd64 build: Successful
NetBSD7 amd64 build: Successful
NetBSD6 amd64 build: Successful
FreeBSD9 amd64 build: Successful
Ubuntu1204 amd64 build: Successful
FreeBSD10 amd64 build: Successful
OmniOS amd64 build: Successful

Fedora24 amd64 build: Failed

Package building failed for Fedora24 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI015BUILD/ErrorLog/log_package_build.txt)

 ^~~~~~~~~~
In file included from bgpd.c:49:0:
../bgpd/bgpd.h:41:31: fatal error: bgp_addpath_types.h: No such file or directory
 #include "bgp_addpath_types.h"
                               ^
compilation terminated.
make[3]: *** [bgpd.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1

Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI015BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

Ubuntu1604 amd64 build: Unknown Log <log_snapcraft.txt>
URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI014BUILD/ErrorLog/log_snapcraft.txt
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI014BUILD/config.status/config.status
Ubuntu1604 amd64 build: No useful log found

Ubuntu 16.04 i386: Failed

Ubuntu 16.04 i386: Unknown Log <log_snapcraft.txt>
URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/U1604I386/ErrorLog/log_snapcraft.txt
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/U1604I386/config.status/config.status
Ubuntu 16.04 i386: No useful log found

Ubuntu 18.04 amd64 build: Failed

Package building failed for Ubuntu 18.04 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/U1804AMD64/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/U1804AMD64/config.status/config.status

CentOS6 amd64 build: Failed

Package building failed for CentOS6 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI006BUILD/ErrorLog/log_package_build.txt)

cc1: warning: unrecognized command line option "-Wno-unused-result"
In file included from bgpd.c:49:
../bgpd/bgpd.h:41:31: error: bgp_addpath_types.h: No such file or directory
In file included from bgpd.c:49:
../bgpd/bgpd.h:467: error: field 'tx_addpath' has incomplete type
../bgpd/bgpd.h:941: error: expected specifier-qualifier-list before 'bgp_addpath_strategy_e'
../bgpd/bgpd.h:1608: error: expected declaration specifiers or '...' before 'bgp_addpath_strategy_e'
../bgpd/bgpd.h: In function 'peer_group_active':
../bgpd/bgpd.h:1783: error: 'struct peer' has no member named 'sflags'

CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI006BUILD/config.status/config.status

CentOS7 amd64 build: Failed

Package building failed for CentOS7 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI005BUILD/ErrorLog/log_package_build.txt)

 ^
In file included from bgpd.c:49:0:
../bgpd/bgpd.h:41:31: fatal error: bgp_addpath_types.h: No such file or directory
 #include "bgp_addpath_types.h"
                               ^
compilation terminated.
make[3]: *** [bgpd.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1

CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI005BUILD/config.status/config.status

Debian9 amd64 build: Failed

Package building failed for Debian9 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI021BUILD/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Debian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI021BUILD/config.status/config.status

Debian8 amd64 build: Failed

Package building failed for Debian8 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI008BLD/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI008BLD/config.status/config.status

Ubuntu1404 amd64 build: Failed

Package building failed for Ubuntu1404 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI001BUILD/ErrorLog/log_package_build.txt)

debian/rules:56: "DEBIAN: SNMP disabled, see README.Debian"
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
	dh_auto_configure -- \
		--enable-exampledir=/usr/share/doc/frr/examples/ \
		--localstatedir=/var/run/frr \
		--sbindir=/usr/lib/frr \

Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI001BUILD/config.status/config.status


Warnings Generated during build:

Checkout code: Successful with additional warnings:

Fedora24 amd64 build: Failed

Package building failed for Fedora24 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI015BUILD/ErrorLog/log_package_build.txt)

 ^~~~~~~~~~
In file included from bgpd.c:49:0:
../bgpd/bgpd.h:41:31: fatal error: bgp_addpath_types.h: No such file or directory
 #include "bgp_addpath_types.h"
                               ^
compilation terminated.
make[3]: *** [bgpd.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1

Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI015BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

Ubuntu1604 amd64 build: Unknown Log <log_snapcraft.txt>
URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI014BUILD/ErrorLog/log_snapcraft.txt
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI014BUILD/config.status/config.status
Ubuntu1604 amd64 build: No useful log found

Ubuntu 16.04 i386: Failed

Ubuntu 16.04 i386: Unknown Log <log_snapcraft.txt>
URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/U1604I386/ErrorLog/log_snapcraft.txt
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/U1604I386/config.status/config.status
Ubuntu 16.04 i386: No useful log found

Ubuntu 18.04 amd64 build: Failed

Package building failed for Ubuntu 18.04 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/U1804AMD64/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/U1804AMD64/config.status/config.status

CentOS6 amd64 build: Failed

Package building failed for CentOS6 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI006BUILD/ErrorLog/log_package_build.txt)

cc1: warning: unrecognized command line option "-Wno-unused-result"
In file included from bgpd.c:49:
../bgpd/bgpd.h:41:31: error: bgp_addpath_types.h: No such file or directory
In file included from bgpd.c:49:
../bgpd/bgpd.h:467: error: field 'tx_addpath' has incomplete type
../bgpd/bgpd.h:941: error: expected specifier-qualifier-list before 'bgp_addpath_strategy_e'
../bgpd/bgpd.h:1608: error: expected declaration specifiers or '...' before 'bgp_addpath_strategy_e'
../bgpd/bgpd.h: In function 'peer_group_active':
../bgpd/bgpd.h:1783: error: 'struct peer' has no member named 'sflags'

CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI006BUILD/config.status/config.status

CentOS7 amd64 build: Failed

Package building failed for CentOS7 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI005BUILD/ErrorLog/log_package_build.txt)

 ^
In file included from bgpd.c:49:0:
../bgpd/bgpd.h:41:31: fatal error: bgp_addpath_types.h: No such file or directory
 #include "bgp_addpath_types.h"
                               ^
compilation terminated.
make[3]: *** [bgpd.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1

CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI005BUILD/config.status/config.status

Debian9 amd64 build: Failed

Package building failed for Debian9 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI021BUILD/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Debian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI021BUILD/config.status/config.status

Debian8 amd64 build: Failed

Package building failed for Debian8 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI008BLD/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI008BLD/config.status/config.status

Ubuntu1404 amd64 build: Failed

Package building failed for Ubuntu1404 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI001BUILD/ErrorLog/log_package_build.txt)

debian/rules:56: "DEBIAN: SNMP disabled, see README.Debian"
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
	dh_auto_configure -- \
		--enable-exampledir=/usr/share/doc/frr/examples/ \
		--localstatedir=/var/run/frr \
		--sbindir=/usr/lib/frr \

Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5432/artifact/CI001BUILD/config.status/config.status

<stdin>:425: trailing whitespace.
				idalloc_free_to_pool(pool_ptr, 
warning: 1 line adds whitespace errors.
Report for bgp_addpath.c | 41 issues
===============================================
WARNING: braces {} are not necessary for any arm of this statement
#55: FILE: /tmp/f1-18096/bgp_addpath.c:55:
+	if (strat < BGP_ADDPATH_MAX) {
[...]
+	} else {
[...]

WARNING: Missing a blank line after declarations
#126: FILE: /tmp/f1-18096/bgp_addpath.c:126:
+	int i;
+	for (i = 0; i < BGP_ADDPATH_MAX; i++) {

WARNING: break is not useful after a goto or return
#167: FILE: /tmp/f1-18096/bgp_addpath.c:167:
+		return 0;
+		break;

WARNING: break is not useful after a goto or return
#170: FILE: /tmp/f1-18096/bgp_addpath.c:170:
+		return 1;
+		break;

WARNING: braces {} are not necessary for single statement blocks
#262: FILE: /tmp/f1-18096/bgp_addpath.c:262:
+		for (bi = rn->info; bi; bi = bi->next) {
+			bgp_addpath_populate_path(allocator, bi, addpath_type);
+		}

WARNING: quoted string split across lines
#335: FILE: /tmp/f1-18096/bgp_addpath.c:335:
+					"%s: enabling bgp deterministic-med, this is required"
+					" for addpath-tx-bestpath-per-AS",

ERROR: trailing whitespace
#396: FILE: /tmp/f1-18096/bgp_addpath.c:396:
+^I^I^I^Iidalloc_free_to_pool(pool_ptr, $

WARNING: C99 // comments do not match recommendation
#413: FILE: /tmp/f1-18096/bgp_addpath.c:413:
+		// Free any IDs left in the pool to the main allocator
Report for bgp_addpath.h | 20 issues
===============================================
WARNING: line over 80 characters
#68: FILE: /tmp/f1-18096/bgp_addpath.h:68:
+void bgp_addpath_update_ids(struct bgp *bgp, struct bgp_node *bn, afi_t afi, safi_t safi);

ERROR: code indent should use tabs where possible
#70: FILE: /tmp/f1-18096/bgp_addpath.h:70:
+                              bgp_addpath_strategy_e old_strat,$

WARNING: please, no spaces at the start of a line
#70: FILE: /tmp/f1-18096/bgp_addpath.h:70:
+                              bgp_addpath_strategy_e old_strat,$

ERROR: code indent should use tabs where possible
#71: FILE: /tmp/f1-18096/bgp_addpath.h:71:
+                              bgp_addpath_strategy_e new_strat);$

WARNING: please, no spaces at the start of a line
#71: FILE: /tmp/f1-18096/bgp_addpath.h:71:
+                              bgp_addpath_strategy_e new_strat);$
Report for bgp_addpath_types.h | 20 issues
===============================================
WARNING: do not add new typedefs
#25: FILE: /tmp/f1-18096/bgp_addpath_types.h:25:
+typedef enum {

WARNING: C99 // comments do not match recommendation
#49: FILE: /tmp/f1-18096/bgp_addpath_types.h:49:
+	const char *human_name;	       // used in path detail non-json

WARNING: C99 // comments do not match recommendation
#50: FILE: /tmp/f1-18096/bgp_addpath_types.h:50:
+	const char *human_description; // used in non-json peer descriptions

WARNING: C99 // comments do not match recommendation
#51: FILE: /tmp/f1-18096/bgp_addpath_types.h:51:
+	const char *type_json_name;    // use in json peer listings

WARNING: C99 // comments do not match recommendation
#52: FILE: /tmp/f1-18096/bgp_addpath_types.h:52:
+	const char *id_json_name;      // using in path json output for tx ID#
Report for bgpd.c | 10 issues
===============================================
< WARNING: line over 80 characters
< #887: FILE: /tmp/f1-18096/bgpd.c:887:
< WARNING: braces {} are not necessary for any arm of this statement
< #898: FILE: /tmp/f1-18096/bgpd.c:898:
< ERROR: code indent should use tabs where possible
< #1066: FILE: /tmp/f1-18096/bgpd.c:1066:
< ERROR: code indent should use tabs where possible
< #7023: FILE: /tmp/f1-18096/bgpd.c:7023:
< ERROR: code indent should use tabs where possible
< #7027: FILE: /tmp/f1-18096/bgpd.c:7027:
Report for bgp_open.c | 2 issues
===============================================
< WARNING: braces {} are not necessary for single statement blocks
< #1380: FILE: /tmp/f1-18096/bgp_open.c:1380:
Report for bgp_route.c | 8 issues
===============================================
< WARNING: line over 80 characters
< #2103: FILE: /tmp/f1-18096/bgp_route.c:2103:
< WARNING: line over 80 characters
< #7305: FILE: /tmp/f1-18096/bgp_route.c:7305:
< WARNING: Block comments use a trailing */ on a separate line
< #8067: FILE: /tmp/f1-18096/bgp_route.c:8067:
< WARNING: Block comments use a trailing */ on a separate line
< #8085: FILE: /tmp/f1-18096/bgp_route.c:8085:
Report for bgp_updgrp_adv.c | 4 issues
===============================================
< WARNING: line over 80 characters
< #146: FILE: /tmp/f1-18096/bgp_updgrp_adv.c:146:
< WARNING: line over 80 characters
< #149: FILE: /tmp/f1-18096/bgp_updgrp_adv.c:149:
Report for bgp_vty.c | 2 issues
===============================================
< ERROR: code indent should use tabs where possible
< #6136: FILE: /tmp/f1-18096/bgp_vty.c:6136:
Report for test_peer_attr.c | 2 issues
===============================================
< WARNING: line over 80 characters
< #257: FILE: /tmp/f1-18096/test_peer_attr.c:257:

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: FAILED

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

This is a comment from an EXPERIMENTAL 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 and apply patch from patchwork: Successful

Building Stage: Failed

FreeBSD11 amd64 build: Successful
NetBSD7 amd64 build: Successful
NetBSD6 amd64 build: Successful
FreeBSD10 amd64 build: Successful
FreeBSD9 amd64 build: Successful
OmniOS amd64 build: Successful
OpenBSD60 amd64 build: Successful
Ubuntu1204 amd64 build: Successful

Fedora24 amd64 build: Failed

Package building failed for Fedora24 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI015BUILD/ErrorLog/log_package_build.txt)

 ^~~~~~~~~~
In file included from bgpd.c:49:0:
../bgpd/bgpd.h:41:31: fatal error: bgp_addpath_types.h: No such file or directory
 #include "bgp_addpath_types.h"
                               ^
compilation terminated.
make[3]: *** [bgpd.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1

Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI015BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

Ubuntu1604 amd64 build: Unknown Log <log_snapcraft.txt>
URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI014BUILD/ErrorLog/log_snapcraft.txt
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI014BUILD/config.status/config.status
Ubuntu1604 amd64 build: No useful log found

Ubuntu 18.04 amd64 build: Failed

Package building failed for Ubuntu 18.04 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/U1804AMD64/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/U1804AMD64/config.status/config.status

Ubuntu 16.04 i386: Failed

Ubuntu 16.04 i386: Unknown Log <log_snapcraft.txt>
URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/U1604I386/ErrorLog/log_snapcraft.txt
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/U1604I386/config.status/config.status
Ubuntu 16.04 i386: No useful log found

CentOS7 amd64 build: Failed

Package building failed for CentOS7 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI005BUILD/ErrorLog/log_package_build.txt)

 ^
In file included from bgpd.c:49:0:
../bgpd/bgpd.h:41:31: fatal error: bgp_addpath_types.h: No such file or directory
 #include "bgp_addpath_types.h"
                               ^
compilation terminated.
make[3]: *** [bgpd.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1

CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI005BUILD/config.status/config.status

Debian8 amd64 build: Failed

Package building failed for Debian8 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI008BLD/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI008BLD/config.status/config.status

CentOS6 amd64 build: Failed

Package building failed for CentOS6 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI006BUILD/ErrorLog/log_package_build.txt)

cc1: warning: unrecognized command line option "-Wno-unused-result"
In file included from bgpd.c:49:
../bgpd/bgpd.h:41:31: error: bgp_addpath_types.h: No such file or directory
In file included from bgpd.c:49:
../bgpd/bgpd.h:467: error: field 'tx_addpath' has incomplete type
../bgpd/bgpd.h:941: error: expected specifier-qualifier-list before 'bgp_addpath_strategy_e'
../bgpd/bgpd.h:1608: error: expected declaration specifiers or '...' before 'bgp_addpath_strategy_e'
../bgpd/bgpd.h: In function 'peer_group_active':
../bgpd/bgpd.h:1783: error: 'struct peer' has no member named 'sflags'

CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI006BUILD/config.status/config.status

Debian9 amd64 build: Failed

Package building failed for Debian9 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI021BUILD/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Debian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI021BUILD/config.status/config.status

Ubuntu1404 amd64 build: Failed

Package building failed for Ubuntu1404 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI001BUILD/ErrorLog/log_package_build.txt)

debian/rules:56: "DEBIAN: SNMP disabled, see README.Debian"
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
	dh_auto_configure -- \
		--enable-exampledir=/usr/share/doc/frr/examples/ \
		--localstatedir=/var/run/frr \
		--sbindir=/usr/lib/frr \

Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI001BUILD/config.status/config.status


Warnings Generated during build:

Checkout code: Successful with additional warnings:

Fedora24 amd64 build: Failed

Package building failed for Fedora24 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI015BUILD/ErrorLog/log_package_build.txt)

 ^~~~~~~~~~
In file included from bgpd.c:49:0:
../bgpd/bgpd.h:41:31: fatal error: bgp_addpath_types.h: No such file or directory
 #include "bgp_addpath_types.h"
                               ^
compilation terminated.
make[3]: *** [bgpd.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1

Fedora24 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI015BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

Ubuntu1604 amd64 build: Unknown Log <log_snapcraft.txt>
URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI014BUILD/ErrorLog/log_snapcraft.txt
Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI014BUILD/config.status/config.status
Ubuntu1604 amd64 build: No useful log found

Ubuntu 18.04 amd64 build: Failed

Package building failed for Ubuntu 18.04 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/U1804AMD64/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/U1804AMD64/config.status/config.status

Ubuntu 16.04 i386: Failed

Ubuntu 16.04 i386: Unknown Log <log_snapcraft.txt>
URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/U1604I386/ErrorLog/log_snapcraft.txt
Ubuntu 16.04 i386: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/U1604I386/config.status/config.status
Ubuntu 16.04 i386: No useful log found

CentOS7 amd64 build: Failed

Package building failed for CentOS7 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI005BUILD/ErrorLog/log_package_build.txt)

 ^
In file included from bgpd.c:49:0:
../bgpd/bgpd.h:41:31: fatal error: bgp_addpath_types.h: No such file or directory
 #include "bgp_addpath_types.h"
                               ^
compilation terminated.
make[3]: *** [bgpd.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1

CentOS7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI005BUILD/config.status/config.status

Debian8 amd64 build: Failed

Package building failed for Debian8 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI008BLD/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Debian8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI008BLD/config.status/config.status

CentOS6 amd64 build: Failed

Package building failed for CentOS6 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI006BUILD/ErrorLog/log_package_build.txt)

cc1: warning: unrecognized command line option "-Wno-unused-result"
In file included from bgpd.c:49:
../bgpd/bgpd.h:41:31: error: bgp_addpath_types.h: No such file or directory
In file included from bgpd.c:49:
../bgpd/bgpd.h:467: error: field 'tx_addpath' has incomplete type
../bgpd/bgpd.h:941: error: expected specifier-qualifier-list before 'bgp_addpath_strategy_e'
../bgpd/bgpd.h:1608: error: expected declaration specifiers or '...' before 'bgp_addpath_strategy_e'
../bgpd/bgpd.h: In function 'peer_group_active':
../bgpd/bgpd.h:1783: error: 'struct peer' has no member named 'sflags'

CentOS6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI006BUILD/config.status/config.status

Debian9 amd64 build: Failed

Package building failed for Debian9 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI021BUILD/ErrorLog/log_package_build.txt)

dh: Unknown sequence debian/backports/rules (choose from: binary binary-arch binary-indep build build-arch build-indep clean install install-arch install-indep)
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
dh_auto_configure -- \
	--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \

Debian9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI021BUILD/config.status/config.status

Ubuntu1404 amd64 build: Failed

Package building failed for Ubuntu1404 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI001BUILD/ErrorLog/log_package_build.txt)

debian/rules:56: "DEBIAN: SNMP disabled, see README.Debian"
# Frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
if ! [ -e config.status ]; then \
	dh_auto_configure -- \
		--enable-exampledir=/usr/share/doc/frr/examples/ \
		--localstatedir=/var/run/frr \
		--sbindir=/usr/lib/frr \

Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-5431/artifact/CI001BUILD/config.status/config.status

<stdin>:425: trailing whitespace.
				idalloc_free_to_pool(pool_ptr, 
warning: 1 line adds whitespace errors.
Report for bgp_addpath.c | 41 issues
===============================================
WARNING: braces {} are not necessary for any arm of this statement
#55: FILE: /tmp/f1-17595/bgp_addpath.c:55:
+	if (strat < BGP_ADDPATH_MAX) {
[...]
+	} else {
[...]

WARNING: Missing a blank line after declarations
#126: FILE: /tmp/f1-17595/bgp_addpath.c:126:
+	int i;
+	for (i = 0; i < BGP_ADDPATH_MAX; i++) {

WARNING: break is not useful after a goto or return
#167: FILE: /tmp/f1-17595/bgp_addpath.c:167:
+		return 0;
+		break;

WARNING: break is not useful after a goto or return
#170: FILE: /tmp/f1-17595/bgp_addpath.c:170:
+		return 1;
+		break;

WARNING: braces {} are not necessary for single statement blocks
#262: FILE: /tmp/f1-17595/bgp_addpath.c:262:
+		for (bi = rn->info; bi; bi = bi->next) {
+			bgp_addpath_populate_path(allocator, bi, addpath_type);
+		}

WARNING: quoted string split across lines
#335: FILE: /tmp/f1-17595/bgp_addpath.c:335:
+					"%s: enabling bgp deterministic-med, this is required"
+					" for addpath-tx-bestpath-per-AS",

ERROR: trailing whitespace
#396: FILE: /tmp/f1-17595/bgp_addpath.c:396:
+^I^I^I^Iidalloc_free_to_pool(pool_ptr, $

WARNING: C99 // comments do not match recommendation
#413: FILE: /tmp/f1-17595/bgp_addpath.c:413:
+		// Free any IDs left in the pool to the main allocator
Report for bgp_addpath.h | 20 issues
===============================================
WARNING: line over 80 characters
#68: FILE: /tmp/f1-17595/bgp_addpath.h:68:
+void bgp_addpath_update_ids(struct bgp *bgp, struct bgp_node *bn, afi_t afi, safi_t safi);

ERROR: code indent should use tabs where possible
#70: FILE: /tmp/f1-17595/bgp_addpath.h:70:
+                              bgp_addpath_strategy_e old_strat,$

WARNING: please, no spaces at the start of a line
#70: FILE: /tmp/f1-17595/bgp_addpath.h:70:
+                              bgp_addpath_strategy_e old_strat,$

ERROR: code indent should use tabs where possible
#71: FILE: /tmp/f1-17595/bgp_addpath.h:71:
+                              bgp_addpath_strategy_e new_strat);$

WARNING: please, no spaces at the start of a line
#71: FILE: /tmp/f1-17595/bgp_addpath.h:71:
+                              bgp_addpath_strategy_e new_strat);$
Report for bgp_addpath_types.h | 20 issues
===============================================
WARNING: do not add new typedefs
#25: FILE: /tmp/f1-17595/bgp_addpath_types.h:25:
+typedef enum {

WARNING: C99 // comments do not match recommendation
#49: FILE: /tmp/f1-17595/bgp_addpath_types.h:49:
+	const char *human_name;	       // used in path detail non-json

WARNING: C99 // comments do not match recommendation
#50: FILE: /tmp/f1-17595/bgp_addpath_types.h:50:
+	const char *human_description; // used in non-json peer descriptions

WARNING: C99 // comments do not match recommendation
#51: FILE: /tmp/f1-17595/bgp_addpath_types.h:51:
+	const char *type_json_name;    // use in json peer listings

WARNING: C99 // comments do not match recommendation
#52: FILE: /tmp/f1-17595/bgp_addpath_types.h:52:
+	const char *id_json_name;      // using in path json output for tx ID#
Report for bgpd.c | 10 issues
===============================================
< WARNING: line over 80 characters
< #887: FILE: /tmp/f1-17595/bgpd.c:887:
< WARNING: braces {} are not necessary for any arm of this statement
< #898: FILE: /tmp/f1-17595/bgpd.c:898:
< ERROR: code indent should use tabs where possible
< #1066: FILE: /tmp/f1-17595/bgpd.c:1066:
< ERROR: code indent should use tabs where possible
< #7023: FILE: /tmp/f1-17595/bgpd.c:7023:
< ERROR: code indent should use tabs where possible
< #7027: FILE: /tmp/f1-17595/bgpd.c:7027:
Report for bgp_open.c | 2 issues
===============================================
< WARNING: braces {} are not necessary for single statement blocks
< #1380: FILE: /tmp/f1-17595/bgp_open.c:1380:
Report for bgp_route.c | 8 issues
===============================================
< WARNING: line over 80 characters
< #2103: FILE: /tmp/f1-17595/bgp_route.c:2103:
< WARNING: line over 80 characters
< #7305: FILE: /tmp/f1-17595/bgp_route.c:7305:
< WARNING: Block comments use a trailing */ on a separate line
< #8067: FILE: /tmp/f1-17595/bgp_route.c:8067:
< WARNING: Block comments use a trailing */ on a separate line
< #8085: FILE: /tmp/f1-17595/bgp_route.c:8085:
Report for bgp_updgrp_adv.c | 4 issues
===============================================
< WARNING: line over 80 characters
< #146: FILE: /tmp/f1-17595/bgp_updgrp_adv.c:146:
< WARNING: line over 80 characters
< #149: FILE: /tmp/f1-17595/bgp_updgrp_adv.c:149:
Report for bgp_vty.c | 2 issues
===============================================
< ERROR: code indent should use tabs where possible
< #6136: FILE: /tmp/f1-17595/bgp_vty.c:6136:
Report for test_peer_attr.c | 2 issues
===============================================
< WARNING: line over 80 characters
< #257: FILE: /tmp/f1-17595/test_peer_attr.c:257:

@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-FRRPULLREQ-5434/

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


Warnings Generated during build:

Checkout code: Successful with additional warnings:

<stdin>:426: trailing whitespace.
				idalloc_free_to_pool(pool_ptr, 
warning: 1 line adds whitespace errors.
Report for bgp_addpath.c | 41 issues
===============================================
WARNING: braces {} are not necessary for any arm of this statement
#55: FILE: /tmp/f1-9326/bgp_addpath.c:55:
+	if (strat < BGP_ADDPATH_MAX) {
[...]
+	} else {
[...]

WARNING: Missing a blank line after declarations
#126: FILE: /tmp/f1-9326/bgp_addpath.c:126:
+	int i;
+	for (i = 0; i < BGP_ADDPATH_MAX; i++) {

WARNING: break is not useful after a goto or return
#167: FILE: /tmp/f1-9326/bgp_addpath.c:167:
+		return 0;
+		break;

WARNING: break is not useful after a goto or return
#170: FILE: /tmp/f1-9326/bgp_addpath.c:170:
+		return 1;
+		break;

WARNING: braces {} are not necessary for single statement blocks
#262: FILE: /tmp/f1-9326/bgp_addpath.c:262:
+		for (bi = rn->info; bi; bi = bi->next) {
+			bgp_addpath_populate_path(allocator, bi, addpath_type);
+		}

WARNING: quoted string split across lines
#335: FILE: /tmp/f1-9326/bgp_addpath.c:335:
+					"%s: enabling bgp deterministic-med, this is required"
+					" for addpath-tx-bestpath-per-AS",

ERROR: trailing whitespace
#396: FILE: /tmp/f1-9326/bgp_addpath.c:396:
+^I^I^I^Iidalloc_free_to_pool(pool_ptr, $

WARNING: C99 // comments do not match recommendation
#413: FILE: /tmp/f1-9326/bgp_addpath.c:413:
+		// Free any IDs left in the pool to the main allocator
Report for bgp_addpath_types.h | 8 issues
===============================================
WARNING: do not add new typedefs
#25: FILE: /tmp/f1-9326/bgp_addpath_types.h:25:
+typedef enum {

WARNING: line over 80 characters
#52: FILE: /tmp/f1-9326/bgp_addpath_types.h:52:
+	const char *id_json_name;      /* using in path json output for tx ID# */
Report for bgp_route.c | 2 issues
===============================================
< ERROR: code indent should use tabs where possible
< #7307: FILE: /tmp/f1-9326/bgp_route.c:7307:
Report for bgp_updgrp_adv.c | 4 issues
===============================================
< WARNING: line over 80 characters
< #146: FILE: /tmp/f1-9326/bgp_updgrp_adv.c:146:
< WARNING: line over 80 characters
< #149: FILE: /tmp/f1-9326/bgp_updgrp_adv.c:149:

CLANG Static Analyzer Summary

  • Github Pull Request 3051, comparing to Git base SHA 9b00962

No Changes in Static Analysis warnings compared to base

4 Static Analyzer issues remaining.

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

@mitch-skiba
Copy link
Contributor Author

I'll clean up the final checkpatch warnings while I rebase this to the dev branch. I made some minor code changes to improve readability, so I'll be re-running our internal tests on this as well.

@srimohans
Copy link
Contributor

Are you going to generate a new PR with the above modifications ?

@mitch-skiba
Copy link
Contributor Author

I had planned on just updating this PR. (Our internal process for contribution has a reference to this PR, but if the history is getting too messy I can jump onto a new one.)

@srimohans
Copy link
Contributor

No. I think it is not Required.

Will review once you update this PR with your new changes.

@eqvinox eqvinox requested review from riw777 and srimohans September 25, 2018 16:01
@louberger

This comment has been minimized.

Copy link
Member

@louberger louberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to go into master first and then only after is stable in master submitted against 6.0.

@mitch-skiba mitch-skiba changed the base branch from dev/6.0 to master September 27, 2018 21:24
@LabN-CI
Copy link
Collaborator

LabN-CI commented Sep 27, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/3051 8a4432d
Date 09/27/2018
Start 17:31:34
Finish 17:54:41
Run-Time 23:07
Total 1816
Pass 1816
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2018-09-27-17:31:34.txt
Log autoscript-2018-09-27-17:32:11.log.bz2

For details, please contact louberger

@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-FRRPULLREQ-5478/

This is a comment from an EXPERIMENTAL 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 3051, comparing to Git base SHA 9b00962

Fixed warnings:

  • Memory error: Use-after-free in ldpd/lde.c, function lde_address_list_free, line 1624
  • Memory error: Use-after-free in lib/imsg-buffer.c, function msgbuf_clear, line 199
  • Memory error: Use-after-free in lib/imsg.c, function imsg_get_fd, line 305
  • Logic error: Assigned value is garbage or undefined in lib/skiplist.c, function skiplist_insert, line 224

Static Analysis warning summary compared to base:

  • Fixed warnings: 4
  • New warnings: 0

@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-FRRPULLREQ-5479/

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


Warnings Generated during build:

Checkout code: Successful with additional warnings:

Report for bgp_addpath.c | 12 issues
===============================================
WARNING: braces {} are not necessary for any arm of this statement
#55: FILE: /tmp/f1-7373/bgp_addpath.c:55:
+	if (strat < BGP_ADDPATH_MAX) {
[...]
+	} else {
[...]

WARNING: Missing a blank line after declarations
#126: FILE: /tmp/f1-7373/bgp_addpath.c:126:
+	int i;
+	for (i = 0; i < BGP_ADDPATH_MAX; i++)
Report for bgp_addpath_types.h | 8 issues
===============================================
WARNING: do not add new typedefs
#25: FILE: /tmp/f1-7373/bgp_addpath_types.h:25:
+typedef enum {

WARNING: line over 80 characters
#52: FILE: /tmp/f1-7373/bgp_addpath_types.h:52:
+	const char *id_json_name;      /* using in path json output for tx ID# */
Report for bgp_route.c | 2 issues
===============================================
< ERROR: code indent should use tabs where possible
< #7310: FILE: /tmp/f1-7373/bgp_route.c:7310:
Report for bgp_updgrp_adv.c | 4 issues
===============================================
< WARNING: line over 80 characters
< #146: FILE: /tmp/f1-7373/bgp_updgrp_adv.c:146:
< WARNING: line over 80 characters
< #149: FILE: /tmp/f1-7373/bgp_updgrp_adv.c:149:

CLANG Static Analyzer Summary

  • Github Pull Request 3051, comparing to Git base SHA 979ee88

No Changes in Static Analysis warnings compared to base

Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

==
I attempted to count the number of configured peers and groups using each addpath strategy per afi/safi. I use the information to avoid doing ID work that isn't needed. However, keeping track of deltas as different events happen got a lot more complicated than I had initially anticipated. It might make sense to go back to iterating all of the peers on event to make sure it stays in sync.

After looking at the code for a bit, I tend to agree it would be simpler just to iterate across the peers rather than trying to keep track. I'm a little concerned that someone is going to mis the "keeping track" bit when working on this code in the future, and cause a mess...

==
I moved a lot of the addpath TX ID selection logic into a separate file. This was helpful to me during development, but might not actually be the way we want to leave things.

I'm actually okay with leaving this, this way. It makes it easier to read, I think.

==
Paths now need to track the TX ID per addpath strategy. For now, this only grows the path structure by 4 bytes, but if more addpath strategies are added in the future, this could end up being unwanted dead weight.

I know @donaldsharp has been trying to reduce the size of the structures here, so I'll defer to him on this point.

==
I'm not sure what should be done with the TX ID in the path JSON output. There is no longer a single TX ID to output, but I'm hesitant to remove a key from a JSON structure. For now I'm just pushing the tx-addpath-all ID, but I'm open to suggestions.

Would it make more sense to change the json to show the TX ID per addpath strategy, as above?

==
The code looks okay to me -- should have a couple of more people look at it, though.

Copy link
Member

@mwinter-osr mwinter-osr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full BGP RFC compliance check shows no new error introduced and all good.

* adjusting the counts, peer sessions will be reset as needed to make the
* change take effect.
*/
void peer_af_addpath_set_type(struct peer *peer, afi_t afi, safi_t safi,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to bgp_addpath_set_type_for_peer or bgp_addpath_peer_af_set_type? Ok otherwise for me.

@eqvinox
Copy link
Contributor

eqvinox commented Oct 23, 2018

@mitch-skiba sorry, I think between all the commenting noone felt responsible to hit the merge button once it was done, but now it has merge conflicts :(

Can you rebase this?

@louberger
Copy link
Member

do you have any information on cpu and memory impact?

@mitch-skiba
Copy link
Contributor Author

Sorry folks I've been traveling for a bit, so I haven't been able to tend to this as well as I would have liked.

I think I'd like to change the accounting style for what addpath strategies are in use back to the original count-the-peers style instead of the the delta maintenance. I think the simplicity outweighs saving a small number of cycles while reconfiguring peers.

Memory impact to users without addpath enabled will be 4 bytes per path. (This is in raw data size. There will be some interference from sizeof alignment requirements, and beyond that the malloc implementation bin size. On my amd64 build there isn't even a sizeof change. The structure ends up being 112 bytes before an after the change. Still, this could represent a 3.5% increase in structure size for what I presume is the most repeated structure in the system. I had spoken to Nikos about this, and he suggested putting in a pointer to an external structure. I think if we add more addpath strategies that would begin to make sense, so we don't penalize non-addpath users for addpath tracking. (At the moment though, the pointer would be the same size as the two 32 bit values on amd64 systems, so it seems a bit hard to justify moving it out at this point.)

If addpath is enabled, we begin to track IDs for it. I wanted an allocator instead of a free running counter. (In our history we have seen a refcount leak count all the way to 2^32 and trigger an assert, so we generally don't trust that to be a big enough number to prevent mishaps.) The allocator on an amd64 box costs about 160K for each million IDs tracked. I don't imagine that is too harsh.

Regrettably I don't have a good baseline for CPU utilization. I'm fine if folks want to hold off until it can be characterized in numbers. For now, I have some words though.

The two biggest hits I can think of:

  1. There could be a lot of allocations required to clear a session from a peer contributing a paths to an addpath advertisement, since the freed IDs will be temporarily held on the bgp_nodes until bestpath is run. (I'd expect memory to still drop quickly, since the ID cache entry is still smaller than the path being freed, but this will be more churn in and out of the malloc heap.)
  2. Enabling an addpath style for the first time, when a lot of paths are already in the table will trigger a sizable one time hit to CPU as it backfills all the IDs for that addpath style.

Most other actions are fairly inexpensive. The ID allocator was designed with low incremental cost in mind. Shuffling IDs on the paths after best-path calculation re-scans the prefix's path list twice. Since bestpath was just run, this list should be hot in the CPU cache, and there isn't a tremendous amount of actual work going on. (This penalty is only paid when addpath is enabled.)

@louberger
Copy link
Member

needs to be rebased - then I'm okay with merging

@louberger
Copy link
Member

also please fix code format error

cfra and others added 3 commits November 9, 2018 21:45
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
This commit introduces lib/id_alloc, which has facilities for both an ID number
allocator, and less efficient ID holding pools. The pools are meant to be a
temporary holding area for ID numbers meant to be re-used, and are implemented
as a linked-list stack.

The allocator itself is much more efficient with memory. Based on sizeof
values on my 64 bit desktop, the allocator requires around 155 KiB per
million IDs tracked.

IDs are ultimately tracked in a bit-map split into many "pages." The
allocator tracks a list of pages that have free bits, and which sections
of each page have free IDs, so there isn't any scanning required to find
a free ID. (The library utility ffs, or "Find First Set," is generally a
single CPU instruction.) At the moment, totally empty pages will not be
freed, so the memory utilization of this allocator will remain at the
high water mark.

The initial intended use case is for BGP's TX Addpath IDs to be pulled
from an allocator that tracks which IDs are in use, rather than a free
running counter.  The allocator reserves ID #0 as a sentinel value for
an invalid ID numbers, and BGP will want ID FRRouting#1 reserved as well. To
support this, the allocator allows for IDs to be explicitly reserved,
though be aware this is only practical to use with low numbered IDs
because the allocator must allocate pages in order.

Signed-off-by Mitchell Skiba <mskiba@amazon.com>
The motivation for this patch is to address a concerning behavior of
tx-addpath-bestpath-per-AS. Prior to this patch, all paths' TX ID was
pre-determined as the path was received from a peer. However, this meant
that any time the path selected as best from an AS changed, bgpd had no
choice but to withdraw the previous best path, and advertise the new
best-path under a new TX ID. This could cause significant network
disruption, especially for the subset of prefixes coming from only one
AS that were also communicated over a bestpath-per-AS session.

The patch's general approach is best illustrated by
txaddpath_update_ids. After a bestpath run (required for best-per-AS to
know what will and will not be sent as addpaths) ID numbers will be
stripped from paths that no longer need to be sent, and held in a pool.
Then, paths that will be sent as addpaths and do not already have ID
numbers will allocate new ID numbers, pulling first from that pool.
Finally, anything left in the pool will be returned to the allocator.

In order for this to work, ID numbers had to be split by strategy. The
tx-addpath-All strategy would keep every ID number "in use" constantly,
preventing IDs from being transferred to different paths. Rather than
create two variables for ID, this patch create a more generic array that
will easily enable more addpath strategies to be implemented. The
previously described ID manipulations will happen per addpath strategy,
and will only be run for strategies that are enabled on at least one
peer.

Finally, the ID numbers are allocated from an allocator that tracks per
AFI/SAFI/Addpath Strategy which IDs are in use. Though it would be very
improbable, there was the possibility with the free-running counter
approach for rollover to cause two paths on the same prefix to get
assigned the same TX ID. As remote as the possibility is, we prefer to
not leave it to chance.

This ID re-use method is not perfect. In some cases you could still get
withdraw-then-add behaviors where not strictly necessary. In the case of
bestpath-per-AS this requires one AS to advertise a prefix for the first
time, then a second AS withdraws that prefix, all within the space of an
already pending MRAI timer. In those situations a withdraw-then-add is
more forgivable, and fixing it would probably require a much more
significant effort, as IDs would need to be moved to ADVs instead of
paths.

Signed-off-by Mitchell Skiba <mskiba@amazon.com>
@LabN-CI
Copy link
Collaborator

LabN-CI commented Nov 10, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/3051 dcc68b5
Date 11/09/2018
Start 19:31:43
Finish 19:55:58
Run-Time 24:15
Total 1813
Pass 1813
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2018-11-09-19:31:43.txt
Log autoscript-2018-11-09-19:32:31.log.bz2

For details, please contact louberger

@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-FRRPULLREQ-5866/

This is a comment from an EXPERIMENTAL 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 3051, comparing to Git base SHA 47a2d5e

No Changes in Static Analysis warnings compared to base

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.