-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Sharp nhg #3550
Sharp nhg #3550
Conversation
💚 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-6293/ This is a comment from an EXPERIMENTAL automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings:
Warnings Generated during build:Ubuntu 14.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 14.04 amd64 build:
Ubuntu 16.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 16.04 amd64 build:
Ubuntu 18.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 18.04 amd64 build:
Ubuntu 16.04 i386 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 16.04 i386 build:
Debian 8 amd64 build: Successful with additional warnings:Debian Package lintian failed for Debian 8 amd64 build:
Debian 9 amd64 build: Successful with additional warnings:Debian Package lintian failed for Debian 9 amd64 build:
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base |
sharpd/sharp_vty.c
Outdated
@@ -81,46 +82,63 @@ DEFPY(watch_nexthop_v4, watch_nexthop_v4_cmd, | |||
|
|||
DEFPY (install_routes, | |||
install_routes_cmd, | |||
"sharp install routes A.B.C.D$start nexthop <A.B.C.D$nexthop4|X:X::X:X$nexthop6> (1-1000000)$routes [instance (0-255)$instance]", | |||
"sharp install routes A.B.C.D$start [nexthop <A.B.C.D$nexthop4|X:X::X:X$nexthop6>|nexthop-group NAME$nexthop_group] (1-1000000)$routes [instance (0-255)$instance]", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we use <> instead of [] so that either nexthop
or nexthop-group
needs to be specified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah my mistake. Fixed
sharpd/sharp_vty.c
Outdated
nhop.gate.ipv4 = nexthop4; | ||
nhop.type = NEXTHOP_TYPE_IPV4; | ||
if (nexthop_group) { | ||
nhgc = nhgc_find(nexthop_group); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you could reduce the scope of nhgc
by declaring it in this block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
sharpd/sharp_vty.c
Outdated
nhop.gate.ipv4 = nexthop4; | ||
nhop.type = NEXTHOP_TYPE_IPV4; | ||
} else { | ||
memcpy(&nhop.gate.ipv6, &nexthop6, IPV6_MAX_BYTELEN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think nhop.gate.ipv6 = nexthop6
would read better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Modify the route_add function to take nexthop groups. Future commits will allow sharpd to use nexthop groups as the install mechanism for routes. Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Allow the sharp daemon to understand and use nexthop-groups. This commit is merely to allow sharpd to understand them when accepted in a future commit Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When installing routes via sharpd 'sharp install route... ' command add the ability to specify a nexthop-group to use. This will allow sharpd to create ECMP routes into zebra. Nexthop-group: ! nexthop-group JANELLE nexthop 192.168.209.1 nexthop 192.168.210.1 ! The install: donna.cumulusnetworks.com# sharp install routes 10.0.50.0 nexthop-group JANELLE 10 donna.cumulusnetworks.com# end donna.cumulusnetworks.com# show ip route Codes: K - kernel route, C - connected, S - static, R - RIP, O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP, T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP, F - PBR, f - OpenFabric, > - selected route, * - FIB route K>* 0.0.0.0/0 [0/106] via 10.0.2.2, enp0s3, 00:20:38 C>* 10.0.2.0/24 is directly connected, enp0s3, 00:20:38 D>* 10.0.50.0/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02 * via 192.168.210.1, enp0s9, 00:00:02 D>* 10.0.50.1/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02 * via 192.168.210.1, enp0s9, 00:00:02 D>* 10.0.50.2/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02 * via 192.168.210.1, enp0s9, 00:00:02 D>* 10.0.50.3/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02 * via 192.168.210.1, enp0s9, 00:00:02 D>* 10.0.50.4/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02 * via 192.168.210.1, enp0s9, 00:00:02 D>* 10.0.50.5/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02 * via 192.168.210.1, enp0s9, 00:00:02 D>* 10.0.50.6/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02 * via 192.168.210.1, enp0s9, 00:00:02 D>* 10.0.50.7/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02 * via 192.168.210.1, enp0s9, 00:00:02 D>* 10.0.50.8/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02 * via 192.168.210.1, enp0s9, 00:00:02 D>* 10.0.50.9/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02 * via 192.168.210.1, enp0s9, 00:00:02 C>* 192.168.209.0/24 is directly connected, enp0s8, 00:20:38 C>* 192.168.210.0/24 is directly connected, enp0s9, 00:20:38 donna.cumulusnetworks.com# Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
💚 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-6296/ This is a comment from an EXPERIMENTAL automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings:
Warnings Generated during build:Ubuntu 16.04 i386 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 16.04 i386 build:
Ubuntu 16.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 16.04 amd64 build:
Ubuntu 18.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 18.04 amd64 build:
Debian 8 amd64 build: Successful with additional warnings:Debian Package lintian failed for Debian 8 amd64 build:
Debian 9 amd64 build: Successful with additional warnings:Debian Package lintian failed for Debian 9 amd64 build:
Ubuntu 14.04 amd64 build: Successful with additional warnings:Debian Package lintian failed for Ubuntu 14.04 amd64 build:
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base |
Allow the sharp daemon to take advantage of nexthop-groups as part of route-installation.
example:
!
nexthop-group JANELLE
nexthop 192.168.209.1
nexthop 192.168.210.1
!
line vty
!
end
donna.cumulusnetworks.com# conf t
donna.cumulusnetworks.com# sharp install routes 10.0.50.0 nexthop-group JANELLE 10