ospfd: don't rely on instance existence in vty #7957
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/3eb5e32ef48887c732d24180b2adf166/raw/d71138e85ab7b2079ebdb4cb3c7329474043470c/cr_7957_1611791141.diff | git apply
diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c
index a9be4d0ed..5f94be276 100644
--- a/ospfd/ospf_vty.c
+++ b/ospfd/ospf_vty.c
@@ -140,10 +140,8 @@ int ospf_oi_count(struct interface *ifp)
all_vrf = strmatch(vrf_name, "all"); \
}
-static int ospf_router_cmd_parse(struct vty *vty,
- struct cmd_token *argv[],
- const int argc,
- unsigned short *instance,
+static int ospf_router_cmd_parse(struct vty *vty, struct cmd_token *argv[],
+ const int argc, unsigned short *instance,
const char **vrf_name)
{
int idx_vrf = 0, idx_inst = 0;
@@ -151,7 +149,8 @@ static int ospf_router_cmd_parse(struct vty *vty,
*instance = 0;
if (argv_find(argv, argc, "(1-65535)", &idx_inst)) {
if (ospf_instance == 0) {
- vty_out(vty, "%% OSPF is not running in instance mode\n");
+ vty_out(vty,
+ "%% OSPF is not running in instance mode\n");
return CMD_WARNING_CONFIG_FAILED;
}
@@ -161,7 +160,8 @@ static int ospf_router_cmd_parse(struct vty *vty,
*vrf_name = NULL;
if (argv_find(argv, argc, "vrf", &idx_vrf)) {
if (ospf_instance != 0) {
- vty_out(vty, "%% VRF is not supported in instance mode\n");
+ vty_out(vty,
+ "%% VRF is not supported in instance mode\n");
return CMD_WARNING_CONFIG_FAILED;
}
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.
idryzhov
force-pushed
the
ospf-instance
branch
from
0a77b0e to
a2b3dad
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/3939e739e17b451a4df94374db7df6b0/raw/d71138e85ab7b2079ebdb4cb3c7329474043470c/cr_7957_1611794226.diff | git apply
diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c
index a9be4d0ed..5f94be276 100644
--- a/ospfd/ospf_vty.c
+++ b/ospfd/ospf_vty.c
@@ -140,10 +140,8 @@ int ospf_oi_count(struct interface *ifp)
all_vrf = strmatch(vrf_name, "all"); \
}
-static int ospf_router_cmd_parse(struct vty *vty,
- struct cmd_token *argv[],
- const int argc,
- unsigned short *instance,
+static int ospf_router_cmd_parse(struct vty *vty, struct cmd_token *argv[],
+ const int argc, unsigned short *instance,
const char **vrf_name)
{
int idx_vrf = 0, idx_inst = 0;
@@ -151,7 +149,8 @@ static int ospf_router_cmd_parse(struct vty *vty,
*instance = 0;
if (argv_find(argv, argc, "(1-65535)", &idx_inst)) {
if (ospf_instance == 0) {
- vty_out(vty, "%% OSPF is not running in instance mode\n");
+ vty_out(vty,
+ "%% OSPF is not running in instance mode\n");
return CMD_WARNING_CONFIG_FAILED;
}
@@ -161,7 +160,8 @@ static int ospf_router_cmd_parse(struct vty *vty,
*vrf_name = NULL;
if (argv_find(argv, argc, "vrf", &idx_vrf)) {
if (ospf_instance != 0) {
- vty_out(vty, "%% VRF is not supported in instance mode\n");
+ vty_out(vty,
+ "%% VRF is not supported in instance mode\n");
return CMD_WARNING_CONFIG_FAILED;
}
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: FailedNetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16744/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 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-16744/artifact/U18ARM8BUILD/config.status/config.status Ubuntu 18.04 arm8 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16744/artifact/U18ARM8BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Ubuntu 18.04 arm8 build Ubuntu 18.04 arm7 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 18.04 arm7 build CentOS 7 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for CentOS 7 amd64 build Debian 9 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for Debian 9 amd64 build Debian 8 amd64 build: Failed (click for details)Debian 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16744/artifact/CI008BLD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Debian 8 amd64 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-16744/artifact/U2004AMD64BUILD/config.status/config.statusDejaGNU Unittests (make check) failed for Ubuntu 20.04 amd64 build Ubuntu 18.04 ppc64le build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 18.04 ppc64le build OpenBSD 6 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for OpenBSD 6 amd64 build 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 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-16744/artifact/CENTOS8BUILD/config.status/config.statusDejaGNU Unittests (make check) failed for CentOS 8 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-16744/artifact/CI101BUILD/config.status/config.status Ubuntu 16.04 arm7 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16744/artifact/CI101BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Ubuntu 16.04 arm7 build Debian 10 amd64 build: Failed (click for details)Debian 10 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16744/artifact/DEB10BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Debian 10 amd64 build FreeBSD 12 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for FreeBSD 12 amd64 build Warnings Generated during build:Checkout code: Successful with additional warningsNetBSD 8 amd64 build: Failed (click for details)NetBSD 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16744/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 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-16744/artifact/U18ARM8BUILD/config.status/config.status Ubuntu 18.04 arm8 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16744/artifact/U18ARM8BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Ubuntu 18.04 arm8 build Ubuntu 18.04 arm7 build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 18.04 arm7 build CentOS 7 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for CentOS 7 amd64 build Debian 9 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for Debian 9 amd64 build Debian 8 amd64 build: Failed (click for details)Debian 8 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16744/artifact/CI008BLD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Debian 8 amd64 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-16744/artifact/U2004AMD64BUILD/config.status/config.statusDejaGNU Unittests (make check) failed for Ubuntu 20.04 amd64 build Ubuntu 18.04 ppc64le build: Failed (click for details)DejaGNU Unittests (make check) failed for Ubuntu 18.04 ppc64le build OpenBSD 6 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for OpenBSD 6 amd64 build 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 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-16744/artifact/CENTOS8BUILD/config.status/config.statusDejaGNU Unittests (make check) failed for CentOS 8 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-16744/artifact/CI101BUILD/config.status/config.status Ubuntu 16.04 arm7 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16744/artifact/CI101BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Ubuntu 16.04 arm7 build Debian 10 amd64 build: Failed (click for details)Debian 10 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16744/artifact/DEB10BUILD/config.log/config.log.gzDejaGNU Unittests (make check) failed for Debian 10 amd64 build FreeBSD 12 amd64 build: Failed (click for details)DejaGNU Unittests (make check) failed for FreeBSD 12 amd64 build |
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-16745/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
|
is this ready ? |
|
@lucize yes this is finished. Nothing changed for For the |
|
@idryzhov so you are saying that the daemon is not started correctly ? |
|
@lucize yes, you should not have |
|
I've updated the init script seems to be ok now, thanks for pointing out! |
ospfd/ospf_vty.c
Outdated
| return CMD_NOT_MY_INSTANCE; | ||
|
|
||
| if (!ospf->oi_running) { | ||
| ospf = ospf_lookup_instance(instance); | ||
| if (!ospf || !ospf->oi_running) { | ||
| vty_out(vty, "%% OSPF instance not found\n"); |
There was a problem hiding this comment.
I know you didn't change this but I think this message is wrong, right? Can we remove it if you agree?
There was a problem hiding this comment.
I agree. Removed the log.
ospfd/ospf_vty.c
Outdated
| if (!ospf->oi_running) { | ||
|
|
||
| ospf = ospf_lookup_instance(instance); | ||
| if (!ospf || !ospf->oi_running) { | ||
| vty_out(vty, "%% OSPF instance not found\n"); |
idryzhov
force-pushed
the
ospf-instance
branch
from
a2b3dad to
9f3a202
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/d514982c1cc160d9a1e9048876251b47/raw/4c1321155143a16c4e79f8fee62f65505ce93308/cr_7957_1614169815.diff | git apply
diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c
index 0b8e256e2..2ff59ccf4 100644
--- a/ospfd/ospf_vty.c
+++ b/ospfd/ospf_vty.c
@@ -140,10 +140,8 @@ int ospf_oi_count(struct interface *ifp)
all_vrf = strmatch(vrf_name, "all"); \
}
-static int ospf_router_cmd_parse(struct vty *vty,
- struct cmd_token *argv[],
- const int argc,
- unsigned short *instance,
+static int ospf_router_cmd_parse(struct vty *vty, struct cmd_token *argv[],
+ const int argc, unsigned short *instance,
const char **vrf_name)
{
int idx_vrf = 0, idx_inst = 0;
@@ -151,7 +149,8 @@ static int ospf_router_cmd_parse(struct vty *vty,
*instance = 0;
if (argv_find(argv, argc, "(1-65535)", &idx_inst)) {
if (ospf_instance == 0) {
- vty_out(vty, "%% OSPF is not running in instance mode\n");
+ vty_out(vty,
+ "%% OSPF is not running in instance mode\n");
return CMD_WARNING_CONFIG_FAILED;
}
@@ -161,7 +160,8 @@ static int ospf_router_cmd_parse(struct vty *vty,
*vrf_name = NULL;
if (argv_find(argv, argc, "vrf", &idx_vrf)) {
if (ospf_instance != 0) {
- vty_out(vty, "%% VRF is not supported in instance mode\n");
+ vty_out(vty,
+ "%% VRF is not supported in instance mode\n");
return CMD_WARNING_CONFIG_FAILED;
}
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.
Store instance index at startup and use it when processing vty commands. The instance itself may be created and deleted by the user in runtime using `[no] router ospf X` command. Fixes FRRouting#7908 Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
idryzhov
force-pushed
the
ospf-instance
branch
from
9f3a202 to
a4e2dac
Compare
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
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-17300/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
Continuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: IncompleteTopotests Ubuntu 16.04 amd64 part 1: Incomplete(check logs for details)Topotests Ubuntu 16.04 amd64 part 1: Incomplete(check logs for details)Addresssanitizer topotests part 4: Incomplete(check logs for details)Successful on other platforms/tests
|
Store instance index at startup and use it when processing vty commands.
The instance itself may be created and deleted by the user in runtime
using
[no] router ospf Xcommand.Fixes #7908
And a separate commit to fix vtysh for multi-instance ospfd.