Skip to content
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

Add source of route as protocol string in ip route pushed into kernel #384

Merged
merged 2 commits into from
Apr 26, 2017

Conversation

donaldsharp
Copy link
Member

Ticket: CM-14313
Reviewed By:
Testing Done: bgpmin, ospfmin, bgp_kitchen_sink_test

'ip route show' displays all routes as belonging to protocol zebra.
The user has to run an additional command (in vtysh) to get the actual
source of a route (bgp/ospf/static etc.). This patch addresses that by
pushing the appropriate protocol string into the protocol field of the
netlink route update message. Now you can see routes with the correct
origin as well as filter on them (ip route show proto ospf).

'ospf' is used for both IPv4 and IPv6 routes, even though the OSPF
version is different in both cases.

Sample output (old):
9.9.12.13 via 69.254.2.38 dev swp3.2 proto zebra metric 20
9.9.13.3 proto zebra metric 20
nexthop via 69.254.2.30 dev swp1.2 weight 1
nexthop via 69.254.2.34 dev swp2.2 weight 1
nexthop via 69.254.2.38 dev swp3.2 weight 1

Sample output (new):
9.9.12.13 via 69.254.2.38 dev swp3.2 proto bgp metric 20
9.9.13.3 proto bgp metric 20
nexthop via 69.254.2.30 dev swp1.2 weight 1
nexthop via 69.254.2.34 dev swp2.2 weight 1
nexthop via 69.254.2.38 dev swp3.2 weight 1

Ticket: CM-14313
Reviewed By:
Testing Done: bgpmin, ospfmin, bgp_kitchen_sink_test

'ip route show' displays all routes as belonging to protocol zebra.
The user has to run an additional command (in vtysh) to get the actual
source of a route (bgp/ospf/static etc.). This patch addresses that by
pushing the appropriate protocol string into the protocol field of the
netlink route update message. Now you can see routes with the correct
origin as well as filter on them (ip route show proto ospf).

'ospf' is used for both IPv4 and IPv6 routes, even though the OSPF
version is different in both cases.

Sample output (old):
9.9.12.13 via 69.254.2.38 dev swp3.2  proto zebra  metric 20
9.9.13.3  proto zebra  metric 20
        nexthop via 69.254.2.30  dev swp1.2 weight 1
        nexthop via 69.254.2.34  dev swp2.2 weight 1
        nexthop via 69.254.2.38  dev swp3.2 weight 1

Sample output (new):
9.9.12.13 via 69.254.2.38 dev swp3.2  proto bgp  metric 20
9.9.13.3  proto bgp  metric 20
        nexthop via 69.254.2.30  dev swp1.2 weight 1
        nexthop via 69.254.2.34  dev swp2.2 weight 1
        nexthop via 69.254.2.38  dev swp3.2 weight 1
@donaldsharp
Copy link
Member Author

I had originally submitted this against stable/2.0 but @eqvinox wisely pointed out that it was a new feature and we were attempting to stabilize the code. So now time to circle back around and get this into master

@NetDEF-CI
Copy link
Collaborator

Continous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-494/

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.

Get source and apply patch from patchwork: Successful

Building Stage: Successful

Basic Tests: Failed

Static analyzer (clang): Successful
IPv6 protocols on Ubuntu 14.04: Successful
IPv4 protocols on Ubuntu 14.04: Successful
IPv4 ldp protocol on Ubuntu 16.04: Successful

Topology tests on Ubuntu 16.04: Failed

Topology tests on Ubuntu 16.04: Unknown Log
URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-494/artifact/TOPOU1604/ErrorLog/
Topology tests on Ubuntu 16.04: No useful log found

@mwinter-osr
Copy link
Member

Just looking at the error (see https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOU1604-494):

It seems the Linux Routing table shows proto 188 instead of proto zebra
ie:
fc00:2:2:2::/64 via fe80::__(r2-sw5)__ dev r1-sw5 proto 188 metric 20 pref medium
instead of
fc00:2:2:2::/64 via fe80::__(r2-sw5)__ dev r1-sw5 proto zebra metric 20 pref medium

Please let me know if this isn't an error and the Topotests should accept it.

@donaldsharp
Copy link
Member Author

Put cumulus/etc/iproute2/ip_protos.d/frr.conf in /etc/iproute2/ip_protos.d/

@mwinter-osr mwinter-osr self-requested a review April 21, 2017 23:45
Copy link
Member

@mwinter-osr mwinter-osr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is less of a technical request, so feel free to bring this up at next technical call for discussion if you disagree.

I always thought (and many other comments I've heard) is that the cumulus subdirectory should be cumulus specific changes and we may decide to remove this directory at a later stage. So I really don't like adding anything new to it, specially not etc/iproute2/rt_protos.d/frr.conf which (in my mind) isn't cumulus specific and should be somewhere else.

My request is to move this file to an appropriate location outside the cumulus directory.

Moving cumulus/etc/rt_protos.d/frr.conf to tools/etc/rt_protos.d/frr.conf

Requested in Review.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
@NetDEF-CI
Copy link
Collaborator

Continous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-511/

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.

Get source and apply patch from patchwork: Successful

Building Stage: Failed

NetBSD6 amd64 build: Successful
CentOS7 amd64 build: Successful
Debian8 amd64 build: Successful
FreeBSD10 amd64 build: Successful
OpenBSD60 amd64 build: Successful
NetBSD7 amd64 build: Successful
Fedora24 amd64 build: Successful
FreeBSD11 amd64 build: Successful
CentOS6 amd64 build: Successful
FreeBSD9 amd64 build: Successful
OmniOS amd64 build: Successful

Ubuntu1404 amd64 build: Failed

Package building failed for Ubuntu1404 amd64 build:
(see full package build log at /browse/FRR-FRRPULLREQ-511/artifact/CI001BUILD/ErrorLog/log_package_build.txt)

debian/rules:24: "MPLS disabled"
# frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
dh_auto_configure -- \
 		--enable-exampledir=/usr/share/doc/frr/examples/ \
		--localstatedir=/var/run/frr \
		--sbindir=/usr/lib/frr \
		--sysconfdir=/etc/frr \

Ubuntu1404 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-511/artifact/CI001BUILD/config.status/config.status

Ubuntu1604 amd64 build: Failed

Package building failed for Ubuntu1604 amd64 build:
(see full package build log at /browse/FRR-FRRPULLREQ-511/artifact/CI014BUILD/ErrorLog/log_package_build.txt)

debian/rules:22: "MPLS enabled"
# frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
dh_auto_configure -- \
 		--enable-exampledir=/usr/share/doc/frr/examples/ \
	--localstatedir=/var/run/frr \
	--sbindir=/usr/lib/frr \
	--sysconfdir=/etc/frr \

Ubuntu1604 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-511/artifact/CI014BUILD/config.status/config.status

Ubuntu1204 amd64 build: Failed

Package building failed for Ubuntu1204 amd64 build:
(see full package build log at /browse/FRR-FRRPULLREQ-511/artifact/CI002BUILD/ErrorLog/log_package_build.txt)

debian/rules:24: "MPLS disabled"
# frr needs /proc to check some BSD vs Linux specific stuff.
# Else it fails with an obscure error message pointing out that
# IPCTL_FORWARDING is an undefined symbol which is not very helpful.
dh_auto_configure -- \
 		--enable-exampledir=/usr/share/doc/frr/examples/ \
		--localstatedir=/var/run/frr \
		--sbindir=/usr/lib/frr \
		--sysconfdir=/etc/frr \

Ubuntu1204 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-511/artifact/CI002BUILD/config.status/config.status

@mwinter-osr
Copy link
Member

I've tested to adjust the package to get this working, but in my case it fails when I copy the frr.conf to /etc/iproute2/rt_protos.d/frr.conf
It seems to work if I append the contents of frr.conf to /etc/iproute2/rt_protos

This is as tested/seen with Ubuntu 16.04 (iproute2 4.3.0-1ubuntu3)

As I'm not very familiar with adding extra protocols to iproute2, can you check how/where this needs to be correctly installed?

@mwinter-osr
Copy link
Member

Just found the issue. Directory is called /etc/iproute2/rt_protos.d/, but this requires iproute2 v4.10 or higher (Seems the patch went in this year Jan). Not something we can depend or require on at this time.

Unless there is a better idea, I'm just going to install to /etc/iproute2/rt_protos.d/ and relax the topotest to ignore whatever word or number is after the proto.

The real solution can be done by whoever builds the proper debian package in the future.

@NetDEF-CI
Copy link
Collaborator

Continous 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-521/

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.

@mwinter-osr mwinter-osr merged commit 77e17a6 into FRRouting:master Apr 26, 2017
@donaldsharp donaldsharp deleted the dr2 branch November 30, 2017 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants