Trey Aspelund [Wed, 18 Dec 2019 20:58:26 +0000 (15:58 -0500)]
bgpd: Remove misleading 'NOTIFICATION' string from End-of-RIB log
'NOTIFICATION' string in this message incorrectly implies a BGP
Notification message was the cause of this log. Removing it to
reduce confusion and replacing with function name.
Donald Sharp [Wed, 18 Dec 2019 14:23:38 +0000 (09:23 -0500)]
ospfd: Prevent use after free on shutdown
Address Sanitizer is reporting this issue:
==26177==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120000238d8 at pc 0x7f88f7c4fa93 bp 0x7fff9a641830 sp 0x7fff9a641820
READ of size 8 at 0x6120000238d8 thread T0
#0 0x7f88f7c4fa92 in if_delete lib/if.c:290
#1 0x42192e in ospf_vl_if_delete ospfd/ospf_interface.c:912
#2 0x42192e in ospf_vl_delete ospfd/ospf_interface.c:990
#3 0x4a6208 in no_ospf_area_vlink ospfd/ospf_vty.c:1227
#4 0x7f88f7c1553d in cmd_execute_command_real lib/command.c:1073
#5 0x7f88f7c19b1e in cmd_execute_command lib/command.c:1132
#6 0x7f88f7c19e8e in cmd_execute lib/command.c:1288
#7 0x7f88f7cd7523 in vty_command lib/vty.c:516
#8 0x7f88f7cd79ff in vty_execute lib/vty.c:1285
#9 0x7f88f7cde4f9 in vtysh_read lib/vty.c:2119
#10 0x7f88f7ccb845 in thread_call lib/thread.c:1549
#11 0x7f88f7c5d6a7 in frr_run lib/libfrr.c:1093
#12 0x412976 in main ospfd/ospf_main.c:221
#13 0x7f88f73b082f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#14 0x413c78 in _start (/usr/local/master/sbin/ospfd+0x413c78)
Effectively we are in a shutdown phase and as part of shutdown we delete the
ospf interface pointer ( ifp->info ). The interface deletion code
was modified in the past year to pass in the address of operator
to allow us to NULL out the holding pointer. The catch here
is that we free the oi and then delete the interface passing
in the address of the oi->ifp pointer, causing a use after free.
Fixes: #5555 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Stephen Worley [Mon, 16 Dec 2019 21:46:30 +0000 (16:46 -0500)]
zebra: pass type when finding individual nexthop
When we are doing a lookup on an individual nexthop,
we should still be passing along the type that gets passed
via the arguments. Otherwise, we will always think we own that
NHE when in reality anyone could have put that into the
kernel.
Before this patch, nexthops in the kernel will get swepped
out even if we didn't create them.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Stephen Worley [Mon, 16 Dec 2019 21:37:14 +0000 (16:37 -0500)]
zebra: null check re->nhe not re->nhe->nhg on attach
We should be NULL checking the entire re->nhe struct, not
the group inside of it. When we get routes from the kernel
using a nexthop group (and future protocols) they will only
pass us an ID to use. Hence, this struct can (and will be)
NULL on first attach when only passed an ID.
There shouldn't be a situation where we have an re->nhe
and don't have an re->nhe->nhg anyway.
Before this patch you can easily make zebra crash by creating a
route in the kernel using a nexthop group and starting zebra.
`ip next add dev lo id 111`
`ip route add 1.1.1.1/32 nhid 111`
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Stephen Worley [Mon, 16 Dec 2019 20:42:37 +0000 (15:42 -0500)]
lib: default nexthop weights to one
Default all nexthop weights to one. The linux kernel does
some weird stuff where it adds one to all nexthop weight values
it gets. So, we added df7fb5800b3798057747873c8be245eb13f3ec36 with
some special subtracing/adding to account for this. Though, that patch
did not account for the default case of the weight being zero for
elements in the group.
Hence, this patch defaults the nexthop weight to one during creation.
This should be a valid value on all platforms anyway so shouldn't
affect anything.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Donald Sharp [Tue, 10 Dec 2019 19:06:33 +0000 (14:06 -0500)]
zebra: Do not build mlag protobuf support if version 3 is not avail
Older versions of protobuf-c do not support version 3 of the
protocol. Add a check into the system to see if we have
version 3 available and if so, compile it in.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Tue, 10 Dec 2019 01:48:21 +0000 (20:48 -0500)]
zebra: Allow zebra_mlag to compile with no j factor
If you compile FRR with no j factor zebra_mlag.c fails to
build because the vtysh extraction methodology runs first
before the protobuf compiler runs and that compilation does
not have the proper dependancy chain built for the inclusions
that zebra_mlag.c had. Moving the DEF* code into a zebra_mlag_vty.c
which can be included in the vtysh extraction code and has
no mlag.proto dependancies makes the compilation work better.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
David Lamparter [Wed, 27 Nov 2019 21:52:50 +0000 (22:52 +0100)]
lib/linklist: flip the bitrot compost
The whole lib/linklist.c code shouldn't really be used for new code (the
lib/typesafe.h bits are better.) So, a new need for these unused
functions shouldn't be coming up.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Wed, 13 Nov 2019 23:21:10 +0000 (00:21 +0100)]
lib: completely get rid of the MTYPE alias hack
Sometimes the easiest solution is hardest to find... the whole point of
all this "static const", aliasing, & co. was to make "MTYPE_FOO" usable
without adding the extra & as in "&MTYPE_FOO". Making it a size-1 array
does that perfectly through the magic of ISO C array decay...
Signed-off-by: David Lamparter <equinox@diac24.net>
Donald Sharp [Fri, 13 Dec 2019 00:30:21 +0000 (19:30 -0500)]
isisd: Free memory when confused
When you call into lsp_update with confusion, the lsp is purged
and we do not do anything with the created tlv's from parsing
the incoming data. To prevent the tlv's from being leaked
note confusion and delete the unneeded data.
Fixes: #5496 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Liam McBirnie [Thu, 5 Dec 2019 20:57:50 +0000 (21:57 +0100)]
pimd: Add command to join any-source multicast.
Allow 'ip igmp join' to join group for any source if no source is
specified.
Disallow joining source "0.0.0.0" as it is used to define an
any-source multicast group.
David Lamparter [Wed, 11 Dec 2019 12:33:36 +0000 (13:33 +0100)]
lib,nhrpd,bgpd/bmp: pass resolver failure details
To keep the calling code agnostic of the DNS resolver libary used, pass
a strerror-style string instead of a status code that would need extra
handling.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Wed, 11 Dec 2019 11:27:05 +0000 (12:27 +0100)]
lib/resolver: support/bypass IP literals
libc-ares doesn't do IP literals, so we have to do that before running
off to do DNS. Since this isn't BMP specific, move to lib/ so NHRP can
benefit too.
Signed-off-by: David Lamparter <equinox@diac24.net>
Under some circumstances (apparently depends on several optimization
flags), gcc-9 throws an used-uninitialized warning for this variable in
the skiplist code. Just initialize to NULL.
Signed-off-by: David Lamparter <equinox@diac24.net>
Mark Stapp [Mon, 9 Dec 2019 21:02:57 +0000 (16:02 -0500)]
lib,zebra: add api to enforce nexthop sort order when copying
Add an api that creates a copy of a list of nexthops and
enforces the canonical sort ordering; consolidate some nhg
code to avoid copy-and-paste. The zebra dplane uses
that api when a plugin sets up a list of nexthops, ensuring
that the plugin's list is ordered when it's processed in
zebra.
Mark Stapp [Mon, 2 Dec 2019 17:03:57 +0000 (12:03 -0500)]
zebra: align dplane notify processing with nhg work
The processing of dataplane route notifications was a little
off-target after the nexthop-group re-work. This should allow
notifications to work better.
Donald Sharp [Fri, 6 Dec 2019 13:58:47 +0000 (08:58 -0500)]
lib, zebra: Allow for installation of a weighted nexthop
Linux has the idea of allowing a weight to be sent
down as part of a nexthop group to allow the kernel
to weight particular nexthop paths a bit more or less
than others.
Allow for installation into the kernel using the weight attribute
associated with the nexthop.
This code is foundational in that it just sets up the ability
to do this, we do not use it yet. Further commits will
allow for the pass through of this data from upper level protocols.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
when deleting an isis interface config with 'no [ip|ipv6] router isis'
we are destroying the isis yang container for that interface, but the
actual circuit struct is kept, and so are the flgs determining whether
that circuit is configured for ipv4 and/or ipv6. This caused issues
when removing and re-adding configuration, as the area counters for
ip circuits were not correctly updated and the topology was never
populated.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Santosh P K [Mon, 25 Nov 2019 16:49:38 +0000 (08:49 -0800)]
bgpd: BGP assert when it tries to access peer which is closed.
Problem: BGP peer pointer is present in keepalive hash table
even when socket has been closed in some race condition.
When keepalive tries to access this peer it asserts.
RCA: Below sequence of events causing assert.
1. Config node peer has went down due to TCP reset
it's FD has been set to -1.
2. Doppelganger peer goes to established state and it has
been added to peer hash table for keepalive when it was
in openconfirm state.
3. Config node parameters including FD are exchanged with
doppelganger. Doppelganger will not have FD -1.
4. Doppelganger will be deleted as part of this it will
remove it from the keepalive peer hash table.
5. While removing from hash table it tries to acquire lock.
6. During this time keepalive thread has the lock and in
a loop trying to send keepalive for peers in hash table.
7. It tries to send keepalive for doppelganger peer with fd
set to -1 and asserts.
Quentin Young [Tue, 3 Dec 2019 20:48:27 +0000 (15:48 -0500)]
bgpd: more attribute parsing cleanup & paranoia
* Move VNC interning to the appropriate spot
* Use existing bgp_attr_flush_encap to free encap sets
* Assert that refcounts are correct before exiting to keep the demons
contained in their fiery prison
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Stephen Worley [Thu, 5 Dec 2019 22:00:32 +0000 (17:00 -0500)]
pbrd: use spaces in show pbr map vty output
We were using a mix of spaces and tabsin show pbr map vty output.
Tabs can be inconsistent depending on the system settings.
Using spaces is a safer option for more consistent output.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Stephen Worley [Thu, 5 Dec 2019 21:17:42 +0000 (16:17 -0500)]
pbrd: make vty nexthop/nexthop-group output consistent
The vty output for pbr maps with a nexthop-group was not
consistent with those configured with an individual nexthop.
Fix that so its easier for users to read.
Stephen Worley [Thu, 5 Dec 2019 20:52:08 +0000 (15:52 -0500)]
pbrd: make show pbr map detail actually work
The `detail` keyword was doing literally nothing. Changed the
default show to be a bit more user friendly and detail
to give the information you might would need for
debugging.
David Lamparter [Tue, 13 Aug 2019 16:24:11 +0000 (18:24 +0200)]
tools/frr-reload.py: remove stderr redirects
These make no sense. stderr=subprocess.STDOUT means that vtysh's stdout
and stderr are combined and returned by check_output. We don't expect
errors in that, and we certainly don't log them.
Leaving vtysh's stderr as stderr is perfectly fine, it'll be captured
for logging just like stderr output from frr-reload.py.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>