bgpd: clear ip bgp instances with safi invalid #1816

pguibert6WIND · 2018-03-02T10:38:38Z

This commit fixes the handling of incoming parameters passed in
following vty functions:

clear ip bgp ipv6 [safi] prefix []
clear ip bgp [vrf ] ipv6 [safi] prefix []

Signed-off-by: Philippe Guibert philippe.guibert@6wind.com

donaldsharp · 2018-03-02T10:52:54Z

bgpd/bgp_vty.c

@@ -6413,8 +6413,8 @@ DEFUN (clear_bgp_ipv6_safi_prefix,
       "Clear bestpath and re-advertise\n"
       "IPv6 prefix\n")
 {
-	int idx_safi = 3;
-	int idx_ipv6_prefixlen = 5;
+	int idx_safi = 4;


This is the wrong approach. safi can be 3 or 4. I would recommend searching for the safi string.

donaldsharp · 2018-03-02T10:53:17Z

bgpd/bgp_vty.c

-	int idx_safi = 3;
-	int idx_ipv6_prefixlen = 5;
+	int idx_safi = 4;
+	int idx_ipv6_prefixlen = 6;


As above, this can be 5 or 6 depending on how the function is called.

donaldsharp · 2018-03-02T10:54:07Z

bgpd/bgp_vty.c

-	int idx_word = 3;
-	int idx_safi = 5;
-	int idx_ipv6_prefixlen = 7;
+	int idx_word = 4;


The same issues exist here and should be fixed in the same manner

NetDEF-CI · 2018-03-02T11:30:49Z

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

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 1816, comparing to Git base SHA c98f4d8

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

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

pguibert6WIND · 2018-03-02T14:14:14Z

Hi Donald,
I was thinking the vty was rebuilding the whole passed string, so that the argument number is unique.
but ok. I made a new proposal.

LabN-CI · 2018-03-02T14:38:08Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/1816 `aa4ed1f`
Date	03/02/2018
Start	09:15:16
Finish	09:38:06
Run-Time	22:50
Total	1808
Pass	1808
Fail	0
Valgrind-Errors
Valgrind-Loss
Details	vncregress-2018-03-02-09:15:16.txt
Log	autoscript-2018-03-02-09:15:59.log.bz2

For details, please contact louberger

NetDEF-CI · 2018-03-02T15:21:41Z

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

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_vty.c
===============================================
< ERROR: spaces required around that '+=' (ctx:VxV)
< #6460: FILE: /tmp/f1/bgp_vty.c:6460:
< +		idx_ipv6_prefix+=1;
<  		               ^
--
< ERROR: spaces required around that '+=' (ctx:VxV)
< #6490: FILE: /tmp/f1/bgp_vty.c:6490:
< +		idx_ipv6_prefix+=1;
<  		               ^

CLANG Static Analyzer Summary

Github Pull Request 1816, comparing to Git base SHA a8e31fc

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

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

LabN-CI · 2018-03-02T15:53:06Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/1816 `f07f629`
Date	03/02/2018
Start	10:30:15
Finish	10:53:04
Run-Time	22:49
Total	1808
Pass	1808
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2018-03-02-10:30:15.txt
Log	autoscript-2018-03-02-10:30:52.log.bz2

For details, please contact louberger

qlyoung · 2018-03-02T16:09:36Z

bgpd/bgp_vty.c

-	int idx_safi = 5;
-	int idx_ipv6_prefixlen = 7;
+	int idx_safi;
+	int idx_ipv6_prefix = 7;


since you can avoid using indices entirely here, I would do so, as it'll avoid introducing bugs in the future when someone modifies the command and forgets to update the indices...you're already using argv_find() so why not just get rid of the indices entirely?

qlyoung · 2018-03-02T16:11:54Z

bgpd/bgp_vty.c

+
+	if (argv_find(argv, argc, "view", idx)
+	    || argv_find(argv, argc, "vrf", idx)) {
+		vrf_name = argv[*idx + 1]->arg;


this is pointless, you can just do
argv_find(argv, argc, "VIEWVRFNAME", idx)

qlyoung · 2018-03-02T16:15:15Z

bgpd/bgp_vty.c

+
+	argv_find_and_parse_safi(argv, argc, &idx_safi, &safi);
+	if (argv_find(argv, argc, "prefix", &idx_ipv6_prefix))
+		idx_ipv6_prefix += 1;


the whole idea of argv_find (and DEFPY) is to avoid manual index calculations, here you can simply do

int idx = 0; ... const char *prefix = argv_find(argv, argc, "X:X::X:X/M", &idx) ? argv[idx]->arg : NULL;

qlyoung · 2018-03-02T16:15:37Z

bgpd/bgp_vty.c

+	bgp_vty_find_and_parse_bgp(vty, argv, argc, &idx_word, &bgp);
+
+	if (argv_find(argv, argc, "prefix", &idx_ipv6_prefix))
+		idx_ipv6_prefix += 1;


NetDEF-CI · 2018-03-02T16:22:23Z

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

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 1816, comparing to Git base SHA a8e31fc

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

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

LabN-CI · 2018-03-05T08:33:31Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/1816 `266e6e4`
Date	03/05/2018
Start	03:10:30
Finish	03:33:28
Run-Time	22:58
Total	1813
Pass	1813
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2018-03-05-03:10:30.txt
Log	autoscript-2018-03-05-03:11:06.log.bz2

For details, please contact louberger

NetDEF-CI · 2018-03-05T09:03:36Z

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

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 1816, comparing to Git base SHA 6768912

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

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

louberger

LGTM

qlyoung · 2018-03-06T16:55:07Z

bgpd/bgp_vty.c

+	int idx_safi;
+	int idx_ipv6_prefix;
+	safi_t safi = SAFI_UNICAST;
+	char *prefix = argv_find(argv, argc, "X:X::X:X/M", &idx_ipv6_prefix) ?


This is undefined behavior, you need to set idx_ipv6_prefix = 0. You need to do the same for idx_safi.

qlyoung · 2018-03-06T16:57:29Z

bgpd/bgp_vty.c

-	int idx_safi = 5;
-	int idx_ipv6_prefixlen = 7;
+	int idx_safi;
+	int idx_ipv6_prefix;


The same here, both of these need to be set to zero.

LabN-CI · 2018-03-07T14:55:47Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/1816 `9b475e7`
Date	03/07/2018
Start	09:33:09
Finish	09:55:45
Run-Time	22:36
Total	1816
Pass	1816
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2018-03-07-09:33:09.txt
Log	autoscript-2018-03-07-09:33:45.log.bz2

For details, please contact louberger

This commit fixes the handling of incoming parameters passed in following vty functions: clear ip bgp ipv6 [safi] prefix [] clear ip bgp [vrf ] ipv6 [safi] prefix [] Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>

NetDEF-CI · 2018-03-07T15:18:54Z

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

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 1816, comparing to Git base SHA 5fe3789

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

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

donaldsharp reviewed Mar 2, 2018

View reviewed changes

pguibert6WIND force-pushed the issue_clear_ipv6 branch from 06697cb to aa4ed1f Compare March 2, 2018 14:12

pguibert6WIND force-pushed the issue_clear_ipv6 branch from aa4ed1f to f07f629 Compare March 2, 2018 15:29

qlyoung reviewed Mar 2, 2018

View reviewed changes

pguibert6WIND force-pushed the issue_clear_ipv6 branch from f07f629 to 266e6e4 Compare March 5, 2018 08:10

louberger approved these changes Mar 6, 2018

View reviewed changes

qlyoung requested changes Mar 6, 2018

View reviewed changes

pguibert6WIND force-pushed the issue_clear_ipv6 branch from 266e6e4 to 9b475e7 Compare March 7, 2018 14:26

bgpd: clear ip bgp instances with invalid safi

9b475e7

This commit fixes the handling of incoming parameters passed in following vty functions: clear ip bgp ipv6 [safi] prefix [] clear ip bgp [vrf ] ipv6 [safi] prefix [] Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>

qlyoung approved these changes Mar 7, 2018

View reviewed changes

qlyoung merged commit 8590fba into FRRouting:master Mar 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bgpd: clear ip bgp instances with safi invalid #1816

bgpd: clear ip bgp instances with safi invalid #1816

pguibert6WIND commented Mar 2, 2018

donaldsharp Mar 2, 2018

pguibert6WIND Mar 2, 2018

donaldsharp Mar 2, 2018

donaldsharp Mar 2, 2018

NetDEF-CI commented Mar 2, 2018

pguibert6WIND commented Mar 2, 2018

LabN-CI commented Mar 2, 2018

NetDEF-CI commented Mar 2, 2018

LabN-CI commented Mar 2, 2018

qlyoung Mar 2, 2018

pguibert6WIND Mar 5, 2018

qlyoung Mar 2, 2018

pguibert6WIND Mar 5, 2018

qlyoung Mar 2, 2018

pguibert6WIND Mar 5, 2018

qlyoung Mar 2, 2018

pguibert6WIND Mar 5, 2018

NetDEF-CI commented Mar 2, 2018

LabN-CI commented Mar 5, 2018

NetDEF-CI commented Mar 5, 2018

louberger left a comment

qlyoung Mar 6, 2018

pguibert6WIND Mar 7, 2018

qlyoung Mar 6, 2018

pguibert6WIND Mar 7, 2018

LabN-CI commented Mar 7, 2018

NetDEF-CI commented Mar 7, 2018

bgpd: clear ip bgp instances with safi invalid #1816

bgpd: clear ip bgp instances with safi invalid #1816

Conversation

pguibert6WIND commented Mar 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NetDEF-CI commented Mar 2, 2018

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

pguibert6WIND commented Mar 2, 2018

LabN-CI commented Mar 2, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Mar 2, 2018

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

Checkout code: Successful with additional warnings:

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

LabN-CI commented Mar 2, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NetDEF-CI commented Mar 2, 2018

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

LabN-CI commented Mar 5, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Mar 5, 2018

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.

louberger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LabN-CI commented Mar 7, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Mar 7, 2018

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

19 Static Analyzer issues remaining.