*: explicitly print "exit" at the end of every node config #9331
Conversation
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/054ff00c8784a8d0db9862bb40ae58bf/raw/f4b4aa488bf0f1c10655bae089e297d89a77a561/cr_9331_1628451769.diff | git apply
diff --git a/pathd/path_cli.c b/pathd/path_cli.c
index 509ea8ba1d..43e8a2d855 100644
--- a/pathd/path_cli.c
+++ b/pathd/path_cli.c
@@ -60,7 +60,7 @@ static int segment_list_has_prefix(
DEFINE_MTYPE_STATIC(PATHD, PATH_CLI, "Client");
-DEFINE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DEFINE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
/* Vty node structures. */
static struct cmd_node segment_routing_node = {
diff --git a/pathd/pathd.h b/pathd/pathd.h
index 81d7aa9105..6476708f2b 100644
--- a/pathd/pathd.h
+++ b/pathd/pathd.h
@@ -34,7 +34,7 @@
DECLARE_MGROUP(PATHD);
-DECLARE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DECLARE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
enum srte_protocol_origin {
SRTE_ORIGIN_UNDEFINED = 0,
diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c
index 6ccf6c55f9..e4d3cf4943 100644
--- a/vtysh/vtysh_config.c
+++ b/vtysh/vtysh_config.c
@@ -262,8 +262,8 @@ void vtysh_config_parse_line(void *arg, const char *line)
if (strncmp(line, " exit", strlen(" exit")) == 0) {
config_add_line_uniq_end(config->line, line);
} else if (strncmp(line, " link-params",
- strlen(" link-params"))
- == 0) {
+ strlen(" link-params"))
+ == 0) {
config_add_line(config->line, line);
config->index = LINK_PARAMS_NODE;
} else if (strncmp(line, " ip multicast boundary",
@@ -271,7 +271,8 @@ void vtysh_config_parse_line(void *arg, const char *line)
== 0) {
config_add_line_uniq_end(config->line, line);
} else if (strncmp(line, " ip igmp query-interval",
- strlen(" ip igmp query-interval")) == 0) {
+ strlen(" ip igmp query-interval"))
+ == 0) {
config_add_line_uniq_end(config->line, line);
} else if (config->index == LINK_PARAMS_NODE
&& strncmp(line, " exit-link-params",
@@ -288,7 +289,8 @@ void vtysh_config_parse_line(void *arg, const char *line)
|| !strncmp(line, " no vrrp",
strlen(" no vrrp"))) {
config_add_line(config->line, line);
- } else if (!strncmp(line, " ip mroute", strlen(" ip mroute"))) {
+ } else if (!strncmp(line, " ip mroute",
+ strlen(" ip mroute"))) {
config_add_line_uniq_end(config->line, line);
} else if (config->index == RMAP_NODE
|| config->index == INTERFACE_NODE
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedOpenBSD 6 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for OpenBSD 6 amd64 build: OpenBSD 6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/CI011BUILD/config.status/config.status Ubuntu 16.04 arm8 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 16.04 arm8 build FreeBSD 11 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for FreeBSD 11 amd64 build Fedora 29 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for Fedora 29 amd64 build Ubuntu 16.04 arm7 build: Failed (click for details)Ubuntu 16.04 arm7 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/CI101BUILD/config.status/config.status Ubuntu 16.04 arm7 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/CI101BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Ubuntu 16.04 arm7 build CentOS 8 amd64 build: Failed (click for details)CentOS 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/CENTOS8BUILD/config.status/config.statusDejaGNU Unittests (make check) failed for CentOS 8 amd64 build Debian 10 amd64 build: Failed (click for details)Debian 10 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/DEB10BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Debian 10 amd64 build Ubuntu 18.04 i386 build: Failed (click for details)Ubuntu 18.04 i386 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U18I386BUILD/frr.xref.xz/frr.xref.xz Ubuntu 18.04 i386 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U18I386BUILD/config.log/config.log.gz Ubuntu 18.04 i386 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U18I386BUILD/config.status/config.statusDejaGNU Unittests (make check) failed for Ubuntu 18.04 i386 build FreeBSD 12 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for FreeBSD 12 amd64 build NetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/CI012BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for NetBSD 8 amd64 build Ubuntu 16.04 i386 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 16.04 i386 build Ubuntu 16.04 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 16.04 amd64 build Debian 11 amd64 build: Failed (click for details)Debian 11 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/DEB11AMD64/frr.xref.xz/frr.xref.xz Debian 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/DEB11AMD64/config.status/config.statusDejaGNU Unittests (make check) failed for Debian 11 amd64 build Ubuntu 18.04 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 18.04 amd64 build Ubuntu 18.04 arm8 build: Failed (click for details)Ubuntu 18.04 arm8 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U18ARM8BUILD/config.status/config.status Ubuntu 18.04 arm8 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U18ARM8BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Ubuntu 18.04 arm8 build CentOS 7 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for CentOS 7 amd64 build Ubuntu 18.04 arm7 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 18.04 arm7 build Debian 9 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for Debian 9 amd64 build Ubuntu 18.04 ppc64le build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 18.04 ppc64le build Ubuntu 20.04 amd64 build: Failed (click for details)Ubuntu 20.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U2004AMD64BUILD/config.status/config.statusDejaGNU Unittests (make check) failed for Ubuntu 20.04 amd64 build Warnings Generated during build:Checkout code: Successful with additional warningsOpenBSD 6 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for OpenBSD 6 amd64 build: OpenBSD 6 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/CI011BUILD/config.status/config.status Ubuntu 16.04 arm8 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 16.04 arm8 build FreeBSD 11 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for FreeBSD 11 amd64 build Fedora 29 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for Fedora 29 amd64 build Ubuntu 16.04 arm7 build: Failed (click for details)Ubuntu 16.04 arm7 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/CI101BUILD/config.status/config.status Ubuntu 16.04 arm7 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/CI101BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Ubuntu 16.04 arm7 build CentOS 8 amd64 build: Failed (click for details)CentOS 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/CENTOS8BUILD/config.status/config.statusDejaGNU Unittests (make check) failed for CentOS 8 amd64 build Debian 10 amd64 build: Failed (click for details)Debian 10 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/DEB10BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Debian 10 amd64 build Ubuntu 18.04 i386 build: Failed (click for details)Ubuntu 18.04 i386 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U18I386BUILD/frr.xref.xz/frr.xref.xz Ubuntu 18.04 i386 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U18I386BUILD/config.log/config.log.gz Ubuntu 18.04 i386 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U18I386BUILD/config.status/config.statusDejaGNU Unittests (make check) failed for Ubuntu 18.04 i386 build FreeBSD 12 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for FreeBSD 12 amd64 build NetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/CI012BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for NetBSD 8 amd64 build Ubuntu 16.04 i386 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 16.04 i386 build Ubuntu 16.04 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 16.04 amd64 build Debian 11 amd64 build: Failed (click for details)Debian 11 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/DEB11AMD64/frr.xref.xz/frr.xref.xz Debian 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/DEB11AMD64/config.status/config.statusDejaGNU Unittests (make check) failed for Debian 11 amd64 build Ubuntu 18.04 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 18.04 amd64 build Ubuntu 18.04 arm8 build: Failed (click for details)Ubuntu 18.04 arm8 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U18ARM8BUILD/config.status/config.status Ubuntu 18.04 arm8 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U18ARM8BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Ubuntu 18.04 arm8 build CentOS 7 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for CentOS 7 amd64 build Ubuntu 18.04 arm7 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 18.04 arm7 build Debian 9 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for Debian 9 amd64 build Ubuntu 18.04 ppc64le build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 18.04 ppc64le build Ubuntu 20.04 amd64 build: Failed (click for details)Ubuntu 20.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20888/artifact/U2004AMD64BUILD/config.status/config.statusDejaGNU Unittests (make check) failed for Ubuntu 20.04 amd64 build |
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/eb9e416f09ce40c501d8b2d82ea7cbfb/raw/f4b4aa488bf0f1c10655bae089e297d89a77a561/cr_9331_1628459185.diff | git apply
diff --git a/pathd/path_cli.c b/pathd/path_cli.c
index 509ea8ba1d..43e8a2d855 100644
--- a/pathd/path_cli.c
+++ b/pathd/path_cli.c
@@ -60,7 +60,7 @@ static int segment_list_has_prefix(
DEFINE_MTYPE_STATIC(PATHD, PATH_CLI, "Client");
-DEFINE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DEFINE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
/* Vty node structures. */
static struct cmd_node segment_routing_node = {
diff --git a/pathd/pathd.h b/pathd/pathd.h
index 81d7aa9105..6476708f2b 100644
--- a/pathd/pathd.h
+++ b/pathd/pathd.h
@@ -34,7 +34,7 @@
DECLARE_MGROUP(PATHD);
-DECLARE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DECLARE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
enum srte_protocol_origin {
SRTE_ORIGIN_UNDEFINED = 0,
diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c
index 6ccf6c55f9..e4d3cf4943 100644
--- a/vtysh/vtysh_config.c
+++ b/vtysh/vtysh_config.c
@@ -262,8 +262,8 @@ void vtysh_config_parse_line(void *arg, const char *line)
if (strncmp(line, " exit", strlen(" exit")) == 0) {
config_add_line_uniq_end(config->line, line);
} else if (strncmp(line, " link-params",
- strlen(" link-params"))
- == 0) {
+ strlen(" link-params"))
+ == 0) {
config_add_line(config->line, line);
config->index = LINK_PARAMS_NODE;
} else if (strncmp(line, " ip multicast boundary",
@@ -271,7 +271,8 @@ void vtysh_config_parse_line(void *arg, const char *line)
== 0) {
config_add_line_uniq_end(config->line, line);
} else if (strncmp(line, " ip igmp query-interval",
- strlen(" ip igmp query-interval")) == 0) {
+ strlen(" ip igmp query-interval"))
+ == 0) {
config_add_line_uniq_end(config->line, line);
} else if (config->index == LINK_PARAMS_NODE
&& strncmp(line, " exit-link-params",
@@ -288,7 +289,8 @@ void vtysh_config_parse_line(void *arg, const char *line)
|| !strncmp(line, " no vrrp",
strlen(" no vrrp"))) {
config_add_line(config->line, line);
- } else if (!strncmp(line, " ip mroute", strlen(" ip mroute"))) {
+ } else if (!strncmp(line, " ip mroute",
+ strlen(" ip mroute"))) {
config_add_line_uniq_end(config->line, line);
} else if (config->index == RMAP_NODE
|| config->index == INTERFACE_NODE
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 arm8 part 5: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 5: No useful log foundTopotests Ubuntu 18.04 i386 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO5U18I386-20889/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 5 Topotests debian 10 amd64 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO8DEB10AMD64-20889/test Topology Tests failed for Topotests debian 10 amd64 part 8 Topotests Ubuntu 18.04 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO5U18AMD64-20889/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 5 Topotests debian 10 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO5DEB10AMD64-20889/test Topology Tests failed for Topotests debian 10 amd64 part 5 Topotests debian 10 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO6DEB10AMD64-20889/test Topology Tests failed for Topotests debian 10 amd64 part 6 Topotests Ubuntu 18.04 arm8 part 6: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 6: No useful log foundTopotests Ubuntu 18.04 arm8 part 1: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 1: No useful log foundTopotests Ubuntu 18.04 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO6U18AMD64-20889/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 6 Topotests Ubuntu 18.04 i386 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO6U18I386-20889/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 6 Topotests Ubuntu 18.04 amd64 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TP1U1804AMD64-20889/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 1 Topotests Ubuntu 18.04 amd64 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO8U18ARM64-20889/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 8 Topotests Ubuntu 18.04 arm8 part 8: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 8: No useful log foundTopotests debian 10 amd64 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO1DEB10AMD64-20889/test Topology Tests failed for Topotests debian 10 amd64 part 1 Topotests Ubuntu 18.04 i386 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO1U18I386-20889/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 1 Topotests Ubuntu 18.04 i386 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO8U18I386-20889/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 8 Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 arm8 part 5: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 5: No useful log foundTopotests Ubuntu 18.04 i386 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO5U18I386-20889/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 5 Topotests debian 10 amd64 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO8DEB10AMD64-20889/test Topology Tests failed for Topotests debian 10 amd64 part 8 Topotests Ubuntu 18.04 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO5U18AMD64-20889/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 5 Topotests debian 10 amd64 part 5: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO5DEB10AMD64-20889/test Topology Tests failed for Topotests debian 10 amd64 part 5 Topotests debian 10 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO6DEB10AMD64-20889/test Topology Tests failed for Topotests debian 10 amd64 part 6 Topotests Ubuntu 18.04 arm8 part 6: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 6: No useful log foundTopotests Ubuntu 18.04 arm8 part 1: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 1: No useful log foundTopotests Ubuntu 18.04 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO6U18AMD64-20889/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 6 Topotests Ubuntu 18.04 i386 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO6U18I386-20889/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 6 Topotests Ubuntu 18.04 amd64 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TP1U1804AMD64-20889/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 1 Topotests Ubuntu 18.04 amd64 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO8U18ARM64-20889/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 8 Topotests Ubuntu 18.04 arm8 part 8: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 8: No useful log foundTopotests debian 10 amd64 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO1DEB10AMD64-20889/test Topology Tests failed for Topotests debian 10 amd64 part 1 Topotests Ubuntu 18.04 i386 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO1U18I386-20889/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 1 Topotests Ubuntu 18.04 i386 part 8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO8U18I386-20889/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 8 |
Outdated results 🚧Basic BGPD CI results: Partial FAILURE, autoscript-2021-08-10-21:35:14.log.bz2 tests failed, has VALGRIND issues
For details, please contact louberger |
|
@idryzhov Seem the same problem... |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
idryzhov
force-pushed
the
explicit-exit
branch
from
863d504 to
d074f1b
Compare
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/ea88334050e64b849b971276e00b8537/raw/d65113e531699dfb004fcb9364dd4fa61198ca92/cr_9331_1629212670.diff | git apply
diff --git a/pathd/path_cli.c b/pathd/path_cli.c
index b9d9694d7..2dc706749 100644
--- a/pathd/path_cli.c
+++ b/pathd/path_cli.c
@@ -60,7 +60,7 @@ static int segment_list_has_prefix(
DEFINE_MTYPE_STATIC(PATHD, PATH_CLI, "Client");
-DEFINE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DEFINE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
/* Vty node structures. */
static struct cmd_node segment_routing_node = {
diff --git a/pathd/pathd.h b/pathd/pathd.h
index 81d7aa910..6476708f2 100644
--- a/pathd/pathd.h
+++ b/pathd/pathd.h
@@ -34,7 +34,7 @@
DECLARE_MGROUP(PATHD);
-DECLARE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DECLARE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
enum srte_protocol_origin {
SRTE_ORIGIN_UNDEFINED = 0,
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 arm8 part 1: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 1: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 arm8 part 1: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 1: No useful log found |
|
ci:rerun |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21176/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
idryzhov
force-pushed
the
explicit-exit
branch
from
d074f1b to
ecf580d
Compare
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/b6a0f6c80caf83f75c357c08a5663c53/raw/be4569a9efb5761e9075bb4676e203793fb644ba/cr_9331_1629308761.diff | git apply
diff --git a/pathd/path_cli.c b/pathd/path_cli.c
index bd629a2b7..cf7efc6a3 100644
--- a/pathd/path_cli.c
+++ b/pathd/path_cli.c
@@ -60,7 +60,7 @@ static int segment_list_has_prefix(
DEFINE_MTYPE_STATIC(PATHD, PATH_CLI, "Client");
-DEFINE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DEFINE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
/* Vty node structures. */
static struct cmd_node segment_routing_node = {
diff --git a/pathd/pathd.h b/pathd/pathd.h
index 81d7aa910..6476708f2 100644
--- a/pathd/pathd.h
+++ b/pathd/pathd.h
@@ -34,7 +34,7 @@
DECLARE_MGROUP(PATHD);
-DECLARE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DECLARE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
enum srte_protocol_origin {
SRTE_ORIGIN_UNDEFINED = 0,
diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c
index 54b915614..958b95195 100644
--- a/vtysh/vtysh_config.c
+++ b/vtysh/vtysh_config.c
@@ -281,7 +281,8 @@ void vtysh_config_parse_line(void *arg, const char *line)
|| !strncmp(line, " no vrrp",
strlen(" no vrrp"))) {
config_add_line(config->line, line);
- } else if (!strncmp(line, " ip mroute", strlen(" ip mroute"))) {
+ } else if (!strncmp(line, " ip mroute",
+ strlen(" ip mroute"))) {
config_add_line_uniq_end(config->line, line);
} else if (config->index == RMAP_NODE
|| config->index == INTERFACE_NODE
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21193/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
idryzhov
force-pushed
the
explicit-exit
branch
from
ecf580d to
5adc041
Compare
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/3d3fb8d4955558275c8bc163f6aa1a45/raw/5a63b9482c75122460aab6f01623815e70ed50c3/cr_9331_1629718620.diff | git apply
diff --git a/pathd/path_cli.c b/pathd/path_cli.c
index bd629a2b7..cf7efc6a3 100644
--- a/pathd/path_cli.c
+++ b/pathd/path_cli.c
@@ -60,7 +60,7 @@ static int segment_list_has_prefix(
DEFINE_MTYPE_STATIC(PATHD, PATH_CLI, "Client");
-DEFINE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DEFINE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
/* Vty node structures. */
static struct cmd_node segment_routing_node = {
diff --git a/pathd/pathd.h b/pathd/pathd.h
index 81d7aa910..6476708f2 100644
--- a/pathd/pathd.h
+++ b/pathd/pathd.h
@@ -34,7 +34,7 @@
DECLARE_MGROUP(PATHD);
-DECLARE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DECLARE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
enum srte_protocol_origin {
SRTE_ORIGIN_UNDEFINED = 0,
diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c
index 2e1d7c5ba..cae7d0990 100644
--- a/vtysh/vtysh_config.c
+++ b/vtysh/vtysh_config.c
@@ -281,7 +281,8 @@ void vtysh_config_parse_line(void *arg, const char *line)
|| !strncmp(line, " no vrrp",
strlen(" no vrrp"))) {
config_add_line(config->line, line);
- } else if (!strncmp(line, " ip mroute", strlen(" ip mroute"))) {
+ } else if (!strncmp(line, " ip mroute",
+ strlen(" ip mroute"))) {
config_add_line_uniq_end(config->line, line);
} else if (config->index == RMAP_NODE
|| config->index == INTERFACE_NODE
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 🚧Basic BGPD CI results: Partial FAILURE, 1 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21295/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
Currently, in frr-reload we: - store a list of single-line context keywords which needs to be frequently updated, - have a separate "if" clause for every node and subnode we have in FRR. Instead, we can store the tree of all known FRR nodes. This tree needs to be updated whenever we add a new node, which is not frequent. And, most importantly, it allows us to write node-agnostic code and save more than 250 LOC. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Instead of setting a config_write callback for each node, set a single callback and print all config from there. It is necessary for the following work on explicit "exit" command in every node. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
There is a possibility that the same line can be matched as a command in some node and its parent node. In this case, when reading the config, this line is always executed as a command of the child node. For example, with the following config: ``` router ospf network 193.168.0.0/16 area 0 ! mpls ldp discovery hello interval 111 ! ``` Line `mpls ldp` is processed as command `mpls ldp-sync` inside the `router ospf` node. This leads to a complete loss of `mpls ldp` node configuration. To eliminate this issue and all possible similar issues, let's print an explicit "exit" at the end of every node config. This commit also changes indentation for a couple of existing exit commands so that all existing commands are on the same level as their corresponding node-entering commands. Fixes FRRouting#9206. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
idryzhov
force-pushed
the
explicit-exit
branch
from
5adc041 to
07679ad
Compare
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/0a2e13a8e23f0247c4c2a7a62e2970fd/raw/fb987d0b0c824c2f0153335611f05c26177adcbb/cr_9331_1629746176.diff | git apply
diff --git a/babeld/babel_interface.c b/babeld/babel_interface.c
index 615ed9fee..c609a0f19 100644
--- a/babeld/babel_interface.c
+++ b/babeld/babel_interface.c
@@ -1362,8 +1362,8 @@ interface_config_write (struct vty *vty)
write++;
}
}
- vty_endframe (vty, "exit\n!\n");
- write++;
+ vty_endframe(vty, "exit\n!\n");
+ write++;
}
return write;
}
diff --git a/babeld/babeld.c b/babeld/babeld.c
index b9037423b..b4d4c3e44 100644
--- a/babeld/babeld.c
+++ b/babeld/babeld.c
@@ -132,7 +132,7 @@ babel_config_write (struct vty *vty)
lines += config_write_distribute (vty, babel_routing_process->distribute_ctx);
- vty_out (vty, "exit\n");
+ vty_out(vty, "exit\n");
return lines;
}
diff --git a/ldpd/ldp_vty_conf.c b/ldpd/ldp_vty_conf.c
index fbd718bb0..c4ea86763 100644
--- a/ldpd/ldp_vty_conf.c
+++ b/ldpd/ldp_vty_conf.c
@@ -134,7 +134,7 @@ ldp_af_iface_config_write(struct vty *vty, int af)
vty_out (vty, " discovery hello interval %u\n",
ia->hello_interval);
- vty_out (vty, " exit\n");
+ vty_out(vty, " exit\n");
}
}
@@ -316,7 +316,7 @@ ldp_config_write(struct vty *vty)
ldp_af_config_write(vty, AF_INET, ldpd_conf, &ldpd_conf->ipv4);
ldp_af_config_write(vty, AF_INET6, ldpd_conf, &ldpd_conf->ipv6);
vty_out (vty, " !\n");
- vty_out (vty, "exit\n");
+ vty_out(vty, "exit\n");
vty_out (vty, "!\n");
return (1);
@@ -357,7 +357,7 @@ ldp_l2vpn_pw_config_write(struct vty *vty, struct l2vpn_pw *pw)
if (missing_pwid)
vty_out (vty," ! Incomplete config, specify a pw-id\n");
- vty_out (vty, " exit\n");
+ vty_out(vty, " exit\n");
}
static int
@@ -388,7 +388,7 @@ ldp_l2vpn_config_write(struct vty *vty)
ldp_l2vpn_pw_config_write(vty, pw);
vty_out (vty, " !\n");
- vty_out (vty, "exit\n");
+ vty_out(vty, "exit\n");
vty_out (vty, "!\n");
}
diff --git a/pathd/path_cli.c b/pathd/path_cli.c
index bd629a2b7..cf7efc6a3 100644
--- a/pathd/path_cli.c
+++ b/pathd/path_cli.c
@@ -60,7 +60,7 @@ static int segment_list_has_prefix(
DEFINE_MTYPE_STATIC(PATHD, PATH_CLI, "Client");
-DEFINE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DEFINE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
/* Vty node structures. */
static struct cmd_node segment_routing_node = {
diff --git a/pathd/pathd.h b/pathd/pathd.h
index 81d7aa910..6476708f2 100644
--- a/pathd/pathd.h
+++ b/pathd/pathd.h
@@ -34,7 +34,7 @@
DECLARE_MGROUP(PATHD);
-DECLARE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DECLARE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
enum srte_protocol_origin {
SRTE_ORIGIN_UNDEFINED = 0,
diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c
index 2e1d7c5ba..cae7d0990 100644
--- a/vtysh/vtysh_config.c
+++ b/vtysh/vtysh_config.c
@@ -281,7 +281,8 @@ void vtysh_config_parse_line(void *arg, const char *line)
|| !strncmp(line, " no vrrp",
strlen(" no vrrp"))) {
config_add_line(config->line, line);
- } else if (!strncmp(line, " ip mroute", strlen(" ip mroute"))) {
+ } else if (!strncmp(line, " ip mroute",
+ strlen(" ip mroute"))) {
config_add_line_uniq_end(config->line, line);
} else if (config->index == RMAP_NODE
|| config->index == INTERFACE_NODE
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
|
Please ignore @polychaeta, it's having a bad day today. |
|
@polychaeta rereview |
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/8efbaf8fb1ca2865b1afdc1d6c54d064/raw/5a63b9482c75122460aab6f01623815e70ed50c3/cr_9331_1629746589.diff | git apply
diff --git a/pathd/path_cli.c b/pathd/path_cli.c
index bd629a2b7..cf7efc6a3 100644
--- a/pathd/path_cli.c
+++ b/pathd/path_cli.c
@@ -60,7 +60,7 @@ static int segment_list_has_prefix(
DEFINE_MTYPE_STATIC(PATHD, PATH_CLI, "Client");
-DEFINE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DEFINE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
/* Vty node structures. */
static struct cmd_node segment_routing_node = {
diff --git a/pathd/pathd.h b/pathd/pathd.h
index 81d7aa910..6476708f2 100644
--- a/pathd/pathd.h
+++ b/pathd/pathd.h
@@ -34,7 +34,7 @@
DECLARE_MGROUP(PATHD);
-DECLARE_HOOK(pathd_srte_config_write, (struct vty *vty), (vty));
+DECLARE_HOOK(pathd_srte_config_write, (struct vty * vty), (vty));
enum srte_protocol_origin {
SRTE_ORIGIN_UNDEFINED = 0,
diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c
index 2e1d7c5ba..cae7d0990 100644
--- a/vtysh/vtysh_config.c
+++ b/vtysh/vtysh_config.c
@@ -281,7 +281,8 @@ void vtysh_config_parse_line(void *arg, const char *line)
|| !strncmp(line, " no vrrp",
strlen(" no vrrp"))) {
config_add_line(config->line, line);
- } else if (!strncmp(line, " ip mroute", strlen(" ip mroute"))) {
+ } else if (!strncmp(line, " ip mroute",
+ strlen(" ip mroute"))) {
config_add_line_uniq_end(config->line, line);
} else if (config->index == RMAP_NODE
|| config->index == INTERFACE_NODE
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-21308/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
| @@ -175,31 +175,27 @@ static struct cmd_node pcep_node = { | |||
| .name = "srte pcep", | |||
| .node = PCEP_NODE, | |||
| .parent_node = SR_TRAFFIC_ENG_NODE, | |||
| .config_write = pcep_cli_pcep_config_write, | |||
There was a problem hiding this comment.
Is this function being called somewhere else?
| pcep_cli_pcep_pce_config_write(vty); | ||
| pcep_cli_pce_config_write(vty); | ||
| pcep_cli_pcc_config_write(vty); | ||
| vty_out(vty, " exit\n"); | ||
| return 1; |
There was a problem hiding this comment.
So this function calls these three sub-functions, but all 4 functions were removed above from the *_config_node structures. Is this function being called in some other way?
There was a problem hiding this comment.
Answering both comments – this is now called using a hook: hook_register(pathd_srte_config_write, pcep_cli_pcep_config_write);.
A direct call is necessary to control the order of the configuration output. The hook is needed because pcep is a plugin that may or may not be loaded, so it registers itself as a hook user.
| # This dictionary contains a tree of all commands that we know start a | ||
| # new multi-line context. All other commands are treated either as | ||
| # commands inside a multi-line context or as single-line contexts. This | ||
| # dictionary should be updated whenever a new node is added to FRR. |
There was a problem hiding this comment.
This seems like something that will easily be missed, and then can easily go unnoticed. Any ideas on how to create a test to catch when this happens?
There was a problem hiding this comment.
Previously it was oneline_ctx_keywords list with single-line config commands. And we regularly updated the list because new single-line commands are frequently added to FRR. Now it's a list of nodes and they are not so frequently added so I consider this as an improvement :)
Regarding the question about catching the missing node, I'll think about it but it looks like a complicated thing. I hope we can merge this without waiting for such a test, given that we had the same problem before, but with single-line commands.
There was a problem hiding this comment.
We do not need to wait, but I would appreciate some ideas you have on this. The fact that we no longer have to update it as often may in fact lead to more missed updates I think.
There is a possibility that the same line can be matched as a command in
some node and its parent node. In this case, when reading the config,
this line is always executed as a command of the child node.
For example, with the following config:
Line
mpls ldpis processed as commandmpls ldp-syncinside therouter ospfnode. This leads to a complete loss ofmpls ldpnodeconfiguration.
To eliminate this issue and all possible similar issues, let's print an
explicit "exit" at the end of every node config.
Fixes #9206.
Signed-off-by: Igor Ryzhov iryzhov@nfware.com