Don Slice [Mon, 11 Feb 2019 19:17:40 +0000 (14:17 -0500)]
tools: keep exit-vrf to change context correctly between vrfs
Discovered in testing that if a static route in the default table
was entered immediately after a vrf static block, the static route
intended for the default table was put in the vrf instead. This
fix retains the "exit-vrf" statement which causes the following
static routes to appear in the default table correctly.
Ticket: CM-23985 Signed-off-by: Don Slice <dslice@cumulusnetwork.com>
==17102== Invalid read of size 8
==17102== at 0x44D84C: rib_dest_from_rnode (rib.h:375)
==17102== by 0x4546ED: rib_process_result (zebra_rib.c:1904)
==17102== by 0x45436D: rib_process_dplane_results (zebra_rib.c:3295)
==17102== by 0x4D0902B: thread_call (thread.c:1607)
==17102== by 0x4CC3983: frr_run (libfrr.c:1011)
==17102== by 0x4266F6: main (main.c:473)
==17102== Address 0x83bd468 is 88 bytes inside a block of size 96 free'd
==17102== at 0x4A35F54: free (vg_replace_malloc.c:530)
==17102== by 0x4CCAC00: qfree (memory.c:129)
==17102== by 0x4D03DC6: route_node_destroy (table.c:501)
==17102== by 0x4D039EE: route_node_free (table.c:90)
==17102== by 0x4D03971: route_node_delete (table.c:382)
==17102== by 0x44D82A: route_unlock_node (table.h:256)
==17102== by 0x454617: rib_process_result (zebra_rib.c:1882)
==17102== by 0x45436D: rib_process_dplane_results (zebra_rib.c:3295)
==17102== by 0x4D0902B: thread_call (thread.c:1607)
==17102== by 0x4CC3983: frr_run (libfrr.c:1011)
==17102== by 0x4266F6: main (main.c:473)
==17102== Block was alloc'd at
==17102== at 0x4A36FF6: calloc (vg_replace_malloc.c:752)
==17102== by 0x4CCAA2D: qcalloc (memory.c:110)
==17102== by 0x4D03D88: route_node_create (table.c:489)
==17102== by 0x4D0360F: route_node_new (table.c:65)
==17102== by 0x4D034F8: route_node_set (table.c:74)
==17102== by 0x4D03486: route_node_get (table.c:327)
==17102== by 0x4CFB700: srcdest_rnode_get (srcdest_table.c:243)
==17102== by 0x4545C1: rib_process_result (zebra_rib.c:1872)
==17102== by 0x45436D: rib_process_dplane_results (zebra_rib.c:3295)
==17102== by 0x4D0902B: thread_call (thread.c:1607)
==17102== by 0x4CC3983: frr_run (libfrr.c:1011)
==17102== by 0x4266F6: main (main.c:473)
==17102==
This is happening because of this order of events:
1) Route is deleted in the main thread and scheduled for rib processing.
2) Rib garbage collection is run and we remove the route node since it
is no longer needed.
3) Data plane returns from the deletion in the kernel and we call
the srcdest_rnode_get function to get the prefix that was deleted.
This recreates a new route node. This creates a route_node with
a lock count of 1, which we freed via the route_unlock_node call.
Then we continued to use the rn pointer. Which leaves us with use
after frees.
The solution is, of course, to just move the unlock the node at the
end of the function if we have a route_node.
Fixes: #3854 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Renato Westphal [Wed, 20 Feb 2019 18:54:32 +0000 (15:54 -0300)]
bgpd: fix the add-path code to understand the mpls-vpn safi
In order to iterate over MPLS VPN routes, it's necessary to use
two nested loops (the outer loop iterates over the MPLS VPN RDs,
and the inner loop iterates over the VPN routes from that RD).
The add-path code wasn't doing this, which was leading to lots of
crashes when add-path was enabled for the MPLS VPN SAFI. This patch
fixes the problem.
Renato Westphal [Wed, 20 Feb 2019 18:37:29 +0000 (15:37 -0300)]
bgpd: fix null pointer dereference bug
If path->net is NULL in the bgp_path_info_free() function, then
bgpd would crash in bgp_addpath_free_info_data() with the following
backtrace:
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ff7b267a42a in __GI_abort () at abort.c:89
#2 0x00007ff7b39c1ca0 in core_handler (signo=11, siginfo=0x7ffff66414f0, context=<optimized out>) at lib/sigevent.c:249
#3 <signal handler called>
#4 idalloc_free_to_pool (pool_ptr=pool_ptr@entry=0x0, id=3) at lib/id_alloc.c:368
#5 0x0000560096246688 in bgp_addpath_free_info_data (d=d@entry=0x560098665468, nd=0x0) at bgpd/bgp_addpath.c:100
#6 0x00005600961bb522 in bgp_path_info_free (path=0x560098665400) at bgpd/bgp_route.c:252
#7 bgp_path_info_unlock (path=0x560098665400) at bgpd/bgp_route.c:276
#8 0x00005600961bb719 in bgp_path_info_reap (rn=rn@entry=0x5600986b2110, pi=pi@entry=0x560098665400) at bgpd/bgp_route.c:320
#9 0x00005600961bf4db in bgp_process_main_one (safi=SAFI_MPLS_VPN, afi=AFI_IP, rn=0x5600986b2110, bgp=0x560098587320) at bgpd/bgp_route.c:2476
#10 bgp_process_wq (wq=<optimized out>, data=0x56009869b8f0) at bgpd/bgp_route.c:2503
#11 0x00007ff7b39d5fcc in work_queue_run (thread=0x7ffff6641e10) at lib/workqueue.c:294
#12 0x00007ff7b39ce3b1 in thread_call (thread=thread@entry=0x7ffff6641e10) at lib/thread.c:1606
#13 0x00007ff7b39a3538 in frr_run (master=0x5600980795b0) at lib/libfrr.c:1011
#14 0x000056009618a5a3 in main (argc=3, argv=0x7ffff6642078) at bgpd/bgp_main.c:481
Renato Westphal [Wed, 20 Feb 2019 18:36:50 +0000 (15:36 -0300)]
isisd: fix crash when entering "no ip[v6] router isis" twice
isisd CLI has some housekeeping code that removes the
"frr-isisd:isis" container from the interface configuration when
IS-IS is disabled for both IPv4 and IPv6 in the corresponding
interface.
The problem is that the code was checking the values of the
"ipv4-routing" and "ipv6-routing" leafs without checking if the
parent "frr-isisd:isis" container was present. So, entering "no
ip[v6] router isis" twice would cause isisd to crash since the
"frr-isisd:isis" container wouldn't be present the second time the
command is processed. Fix this.
isisd aborted: vtysh -c "configure terminal" -c "interface eth99" -c "no ip router isis WORD"
isisd aborted: vtysh -c "configure terminal" -c "interface eth99" -c "no ipv6 router isis"
Renato Westphal [Wed, 20 Feb 2019 18:32:55 +0000 (15:32 -0300)]
pbrd: fix removal of ipv6 nexthops
Fix bug in the code that compares IPv6 addresses. If memcmp()
returns 0 then the two addresses are equal.
Because of this problem, hash_release() could return NULL in a few
places, leading to the following crashes (found by the CLI fuzzer):
pbrd aborted: vtysh -c "configure terminal" -c "pbr-map WORD seq 100" -c "no set nexthop 2001:db8::1"
pbrd aborted: vtysh -c "configure terminal" -c "nexthop-group NHGROUP" -c "no nexthop 2001:db8::1"
Add a test command to pim that allows you to reset the keepalive timer
for an upstream to it's max value. This is to allow purposeful testing
of cleanup code in pim, by forcing the keeaplive timer to expire later.
robot# show ip pim upstream
Iif Source Group State Uptime JoinTimer RSTimer KATimer RefCnt
enp3s0 192.168.201.136 225.1.0.0 NotJ,RegP 00:00:10 00:00:52 00:00:25 00:02:54 1
robot# show ip pim upstream
Iif Source Group State Uptime JoinTimer RSTimer KATimer RefCnt
enp3s0 192.168.201.136 225.1.0.0 NotJ,RegP 00:00:11 00:00:51 00:00:24 00:02:53 1
robot# test pim keep 192.168.201.136 225.1.0.0
Setting (192.168.201.136,225.1.0.0) to current keep alive time: 210
robot# show ip pim upstream
Iif Source Group State Uptime JoinTimer RSTimer KATimer RefCnt
enp3s0 192.168.201.136 225.1.0.0 NotJ,RegP 00:00:27 00:00:35 00:00:08 00:03:27 1
robot#
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
David Lamparter [Mon, 4 Feb 2019 21:56:50 +0000 (22:56 +0100)]
lib: yang: use common yang_ctx_new_setup()
After creating a libyang context, we need to hook up our callback to use
embedded built-in modules. I hadn't added this to the yang translator
code.
Also, ly_ctx_new fails if the search directory doesn't exist. Since
that's not a hard error for us, work around that and ignore inaccessible
YANG_MODELS_DIR. (This is needed for snap packages.)
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Mon, 18 Feb 2019 23:27:45 +0000 (00:27 +0100)]
tools: fix new init script wrt. multi-instance
TBH when I looked at watchfrr I didn't see any MI support and hence
assumed this just didn't work to begin with. However, it actually does
(transparently to watchfrr, by just using "ospfd-1" as daemon name.)
So, fix this up and make it work again.
(Also remove 2 extraneous \n in messages.)
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Thu, 24 Jan 2019 17:17:40 +0000 (18:17 +0100)]
watchfrr: build in defaults for -r/-s/-k
There's no good reason to not have these options default to the
installation path of tools/watchfrr.sh. Doing so allows us to ditch
watchfrr_options from daemons/daemons.conf completely.
Fixes: #3652 Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Mon, 4 Feb 2019 19:16:34 +0000 (20:16 +0100)]
debian: add missing Conflicts:
We were missing several Conflicts: (or Breaks:) lines. Specifically,
- the .png diagrams in frr-doc conflict with quagga-doc
- the quagga package was split up and we conflict on each on the
daemon's man pages
- pimd also conflicts on the man page.
This is a "conservative" fix for the time being, putting everything into
Conflicts:. Some of these might have other options to fix them (e.g.
renaming the diagrams or man pages) but that needs more thought and
isn't appropriate for a simple fix.
There is also the "layer 9" consideration of whether to add "Replaces:
quagga" lines. For the time being I'd say it's a bit early to have that
discussion.
Reported-by: Andreas Beckmann <anbe@debian.org>
References: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=921376 Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Mon, 4 Feb 2019 03:52:16 +0000 (04:52 +0100)]
debian: remove bogus libjson0 dependency
Dependencies for the actual library packages are autogenerated by shlib
handling. Removing the bogus line should hopefully get this to build
on Debian buildd...
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Sun, 27 Jan 2019 16:24:36 +0000 (17:24 +0100)]
debian: extend comments on pre/postinst hooks
While originally created to support upgrading within non-official
previous FRR packages, the same logic makes upgrading from Quagga
configs more straightforward.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Fri, 21 Dec 2018 16:00:42 +0000 (17:00 +0100)]
tools: don't watch nonexistent daemons
If we try to monitor a nonexisting daemon in watchfrr, it will
(currently) forever wait at startup since the vty connection will never
come up. Just drop the daemon from the daemon list in such a case.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Mon, 17 Dec 2018 17:55:31 +0000 (18:55 +0100)]
debian: rework autopkgtests
Ditch the old non-working one and add 3 new ones to check:
- that zebra can talk to the kernel at least somewhat
- that SNMP and RPKI modules can be loaded
- that frr-reload.py works
This should catch most build environment SNAFUs.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Fri, 14 Dec 2018 15:08:25 +0000 (16:08 +0100)]
debian: cleanly split off from dist tarball
The debian/ directory is distributed separately for tarballs in 3.0
(quilt) format. Including it in the dist tarball causes problems with
automake when the separately distributed debian directory is unpacked on
top of the dist tarball; the clean and correct thing to do here is to
not include the debian/ directory in dist tarballs.
Users have two choices for building FRR Debian packages:
- build straight off git
- build from a "frr.tar" + "frr-debian.tar"
The tarsource.sh tool does the right thing when invoked with the -D
("Debian") option.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Mon, 10 Dec 2018 21:46:55 +0000 (22:46 +0100)]
debian: clean up debian/ directory for git build
Running `dpkg-buildpackage` with source-format "git" complains about
newly created files under debian/. Remove the build-created frr.init &
frr.service to avoid the build erroring out due to this.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Sun, 28 Oct 2018 17:54:14 +0000 (18:54 +0100)]
debianpkg: install libraries to /usr/lib64/frr
This makes them "private libraries" (which they are, since we don't
maintain a proper versioned ABI on libfrr.) This also properly fixes
another few lintian warnings.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Tue, 23 Oct 2018 11:55:36 +0000 (13:55 +0200)]
debianpkg: add -0 to version
We need a -something suffix since otherwise part of our version number
(e.g. -dev or -DATE) will get taken as Debian sub-version. (Everything
after the last -)
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Thu, 25 Oct 2018 13:37:28 +0000 (15:37 +0200)]
debianpkg: always install /etc/init.d/frr
There is no point in making this conditional, systemd correctly prefers
the service file over the init script when it is present. Also, people
can install an init system that doesn't match their distribution and
even change init systems on an installation.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Thu, 25 Oct 2018 13:00:55 +0000 (15:00 +0200)]
debianpkg: ditch development-only files
It is currently impossible to build external stuff that links against
installed FRR headers or libraries. Such projects need to directly
reference an FRR source tree until we revamp the library installation
semantics.
In any case these files would then be in a frr-dev Debian package.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Wed, 24 Oct 2018 18:57:32 +0000 (20:57 +0200)]
debianpkg: raise debhelper compat level to 9
Compat level 7 has long been deprecated. 9 works fine for us, though it
does change the library installation directories to multi-arch. (Which
is not a problem.)
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Wed, 24 Oct 2018 15:20:29 +0000 (17:20 +0200)]
debianpkg: make frr-pythontools a "Recommends:"
From the Debian policy:
Recommends:
This declares a strong, but not absolute, dependency.
The Recommends field should list packages that would be found
together with this one in all but unusual installations.
I'd say, yes, we do want the python-based reload functionality in all
but unusual installations.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Wed, 3 Oct 2018 17:14:27 +0000 (19:14 +0200)]
debianpkg: create frr-snmp and frr-rpki-rtrlib
This splits off SNMP and RPKI support so that users can install these
packages (with the appropriate dependencies) independently of main FRR.
It also obsoletes out the weird multi-variant package distribution we've
been doing for RPKI support.
The snmpd dependency is also changed to Recommends: on frr-snmp since
the frr-snmp package is essentially useless without snmpd.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Tue, 23 Oct 2018 11:54:07 +0000 (13:54 +0200)]
debianpkg: use better dependency for pythontools
As described in https://wiki.debian.org/binNMU, arch-indep packages
should have an "almost identical" dependency so "+..." changes can be
made to arch-dep packages without breaking the arch-indep pkgs.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Wed, 24 Oct 2018 15:01:11 +0000 (17:01 +0200)]
tools: add new tarsource.sh helper
It cleans your house and cooks dinner. Or maybe it creates a clean dist
tarball for you, plus a Debian .dsc if you have dpkg installed - and
GPG-signs the result appropriately if requested.
In any case the resulting tarball should be distributed for our
releases.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Mon, 22 Oct 2018 11:37:53 +0000 (13:37 +0200)]
debianpkg: kill backports
The debianpkg/backports system is rather complicated and actually slows
down Debian package building quite a lot since the backports/rules file
is evaluated a zillion times during a normal build.
This just folds up everything into a single Debian package build that
works on all OSes. The only real difference that the backports stuff
was used for is switching between systemd and init.d, the latter for
Ubuntu 12.04 and 14.04.
With this, that switch is controlled by the pkg.frr.nosystemd
Build-Profile instead. Package builds for Ubuntu 14.04 need to supply
the -Ppkg.frr.nosystemd option to dpkg-buildpackage. (12.04 isn't
supported anymore anyway.)
Note that the update-rc.d step that was previously coded into
postinst/postrm is now handled by the dh_installinit magic.
Other than this, there were some minor build dependency differences, all
of which are now just handled as | in the central deps.
Signed-off-by: David Lamparter <equinox@diac24.net>
projects / mirror / frr.git / log