Issue 1968 defaultvrfonly #2042

pguibert6WIND · 2018-04-09T09:25:36Z

This permits configuring static routes per table identifier.

NetDEF-CI · 2018-04-09T10:16:13Z

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

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 zebra_static.c
===============================================
< WARNING: line over 80 characters
< #161: FILE: /tmp/f1-544/zebra_static.c:161:
< +			re->table = zebra_vrf_lookup_by_id(si->vrf_id)->table_id;
<

CLANG Static Analyzer Summary

Github Pull Request 2042, comparing to Git base SHA 8227cf9

No Changes in Static Analysis warnings compared to base

20 Static Analyzer issues remaining.

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

LabN-CI · 2018-04-09T11:04:57Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/2042 `d7cd097`
Date	04/09/2018
Start	06:41:53
Finish	07:04:54
Run-Time	23:01
Total	1813
Pass	1813
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2018-04-09-06:41:53.txt
Log	autoscript-2018-04-09-06:42:41.log.bz2

For details, please contact louberger

NetDEF-CI · 2018-04-09T14:02:37Z

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

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 2042, comparing to Git base SHA 8227cf9

No Changes in Static Analysis warnings compared to base

20 Static Analyzer issues remaining.

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

LabN-CI · 2018-04-09T14:16:44Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/2042 `9266916`
Date	04/09/2018
Start	09:33:28
Finish	09:56:26
Run-Time	22:58
Total	1813
Pass	1813
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2018-04-09-09:33:28.txt
Log	autoscript-2018-04-09-09:34:15.log.bz2

For details, please contact louberger

LabN-CI · 2018-04-12T10:05:00Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/2042 `e407804`
Date	04/12/2018
Start	05:20:21
Finish	05:43:18
Run-Time	22:57
Total	1816
Pass	1816
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2018-04-12-05:20:21.txt
Log	autoscript-2018-04-12-05:21:09.log.bz2

For details, please contact louberger

NetDEF-CI · 2018-04-12T10:12:24Z

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

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 2042, comparing to Git base SHA 0b98482

No Changes in Static Analysis warnings compared to base

20 Static Analyzer issues remaining.

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

donaldsharp · 2018-04-12T14:42:03Z

zebra/zebra_vrf.c

-		}
-
-		if (vrf_is_user_cfged(vrf)) {
+		} else if (vrf_is_user_cfged(vrf)) {


This is the wrong fix. We should use the vty_frame() code to display vrf config information.

Hi Donald,

the issue is related to default routing table.
please tell me if I am wrong.
I am on VRF netns mode.
if I configure a static route on default VRF, then I should not see default VRF table appear.
I expect static route be on configure terminal.
Actually this is not the case.

#configure terminal ip route 5.5.5.0/24 tun1 # #show running-config ... vrf Default-IP-Routing-Table ip route 5.5.5.0/24 tun1 ! ...

I don't see the point here with vty_frame().
Unless I did not understand how to solve the issue, please refine.
Thanks,
Cheers,Philippe

@pguibert6WIND, Donald (and I) agree with you that it should not show the default VRF. If I understand Donald correctly, he is saying that your approach to fixing this is not ideal (again I agree). We have facilities in lib/vty.c for this exact scenario that allow you to enter a "config frame" by calling vty_frame(). You exit the frame by calling vty_endframe(). Any calls to vty_out() are cached in that frame between entry and exit. If you ended up not calling vty_out() at all, then that entire frame is destroyed and nothing is output (including the very first line that you started the frame with).

Incidentally, I have touched this code to use vty_frame() in an unrelated but overlapping PR:
#2079

Ergo you can just revert this part of the patch.

Add the table keyword for all ip route/ip mroute/ipv6 route commands that are available. Also, the main structure is being added a table identifier. Signed-off-by: Philippe Guibert <[email protected]>

In the case where vrf backend is netns, then the list of ns tables may be extended. A single list is kept,but an attribute is added: the ns_id. Signed-off-by: Philippe Guibert <[email protected]>

when configuring a route on default vrf, the show running displays default vrf subnode. Instead, if it is the default vrf that is used, then the vrf subnode is not displayed. Signed-off-by: Philippe Guibert <[email protected]>

NetDEF-CI · 2018-04-17T10:49:46Z

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

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 2042, comparing to Git base SHA 673c2b7

No Changes in Static Analysis warnings compared to base

6 Static Analyzer issues remaining.

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

LabN-CI · 2018-04-17T11:31:07Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/2042 `dbac81e`
Date	04/17/2018
Start	06:46:20
Finish	07:09:12
Run-Time	22:52
Total	1816
Pass	1816
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2018-04-17-06:46:20.txt
Log	autoscript-2018-04-17-06:47:08.log.bz2

For details, please contact louberger

qlyoung · 2018-04-17T18:21:13Z

zebra/zebra_static.c

+			re->table = zebra_vrf_lookup_by_id(si->vrf_id)
+				->table_id;
+		else if (si->table_id) /* case default VRF, table used
+					*/


Please put this comment on its own line above the else, or move the terminating */ onto the same line as /*.

qlyoung · 2018-04-17T18:21:19Z

zebra/zebra_static.c

+					*/
+			re->table = si->table_id;
+		else /* case default VRF, default table
+		      */


qlyoung · 2018-04-17T20:08:12Z

zebra/zebra_vrf.c

-		}
-
-		if (vrf_is_user_cfged(vrf)) {
+		} else if (vrf_is_user_cfged(vrf)) {


@pguibert6WIND, Donald (and I) agree with you that it should not show the default VRF. If I understand Donald correctly, he is saying that your approach to fixing this is not ideal (again I agree). We have facilities in lib/vty.c for this exact scenario that allow you to enter a "config frame" by calling vty_frame(). You exit the frame by calling vty_endframe(). Any calls to vty_out() are cached in that frame between entry and exit. If you ended up not calling vty_out() at all, then that entire frame is destroyed and nothing is output (including the very first line that you started the frame with).

Incidentally, I have touched this code to use vty_frame() in an unrelated but overlapping PR:
#2079

Ergo you can just revert this part of the patch.

pguibert6WIND mentioned this pull request Apr 9, 2018

Issue 1968 / ip route <> table X #1976

Merged

pguibert6WIND force-pushed the issue_1968_defaultvrfonly branch from d7cd097 to 9266916 Compare April 9, 2018 13:06

pguibert6WIND mentioned this pull request Apr 9, 2018

zebra / static route on table X #1968

Closed

pguibert6WIND force-pushed the issue_1968_defaultvrfonly branch from 9266916 to e407804 Compare April 12, 2018 09:17

donaldsharp reviewed Apr 12, 2018

View reviewed changes

pguibert6WIND force-pushed the issue_1968_defaultvrfonly branch from e407804 to dbac81e Compare April 17, 2018 09:55

pguibert6WIND added 3 commits April 17, 2018 12:24

zebra: add table keyword to route configuration vty command

b805778

Add the table keyword for all ip route/ip mroute/ipv6 route commands that are available. Also, the main structure is being added a table identifier. Signed-off-by: Philippe Guibert <[email protected]>

zebra: ns_table list is extended to support multiple NETNS

c15c832

In the case where vrf backend is netns, then the list of ns tables may be extended. A single list is kept,but an attribute is added: the ns_id. Signed-off-by: Philippe Guibert <[email protected]>

zebra: don't display default vrf subnode

dbac81e

when configuring a route on default vrf, the show running displays default vrf subnode. Instead, if it is the default vrf that is used, then the vrf subnode is not displayed. Signed-off-by: Philippe Guibert <[email protected]>

qlyoung reviewed Apr 17, 2018

View reviewed changes

donaldsharp closed this Apr 24, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 1968 defaultvrfonly #2042

Issue 1968 defaultvrfonly #2042

pguibert6WIND commented Apr 9, 2018

NetDEF-CI commented Apr 9, 2018

LabN-CI commented Apr 9, 2018

NetDEF-CI commented Apr 9, 2018

LabN-CI commented Apr 9, 2018

LabN-CI commented Apr 12, 2018

NetDEF-CI commented Apr 12, 2018

donaldsharp Apr 12, 2018

pguibert6WIND Apr 16, 2018

qlyoung Apr 17, 2018

NetDEF-CI commented Apr 17, 2018

LabN-CI commented Apr 17, 2018

qlyoung Apr 17, 2018

qlyoung Apr 17, 2018

qlyoung Apr 17, 2018

Issue 1968 defaultvrfonly #2042

Issue 1968 defaultvrfonly #2042

Conversation

pguibert6WIND commented Apr 9, 2018

NetDEF-CI commented Apr 9, 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

20 Static Analyzer issues remaining.

LabN-CI commented Apr 9, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Apr 9, 2018

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

20 Static Analyzer issues remaining.

LabN-CI commented Apr 9, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

LabN-CI commented Apr 12, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Apr 12, 2018

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

20 Static Analyzer issues remaining.

donaldsharp Apr 12, 2018

Choose a reason for hiding this comment

pguibert6WIND Apr 16, 2018

Choose a reason for hiding this comment

qlyoung Apr 17, 2018

Choose a reason for hiding this comment

NetDEF-CI commented Apr 17, 2018

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

6 Static Analyzer issues remaining.

LabN-CI commented Apr 17, 2018

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

qlyoung Apr 17, 2018

Choose a reason for hiding this comment

qlyoung Apr 17, 2018

Choose a reason for hiding this comment

qlyoung Apr 17, 2018

Choose a reason for hiding this comment