Donald Sharp [Mon, 2 Dec 2019 14:37:47 +0000 (09:37 -0500)]
bgpd: Prevent crash in bgp_table_range_lookup
The function bgp_table_range_lookup attempts to walk down
the table node data structures to find a list of matching
nodes. We need to guard against the current node from
not matching and not having anything in the child nodes.
Add a bit of code to guard against this.
Traceback that lead me down this path:
Nov 24 12:22:38 frr bgpd[20257]: Received signal 11 at 1574616158 (si_addr 0x2, PC 0x46cdc3); aborting...
Nov 24 12:22:38 frr bgpd[20257]: Backtrace for 11 stack frames:
Nov 24 12:22:38 frr bgpd[20257]: /lib64/libfrr.so.0(zlog_backtrace_sigsafe+0x67) [0x7fd1ad445957]
Nov 24 12:22:38 frr bgpd[20257]: /lib64/libfrr.so.0(zlog_signal+0x113) [0x7fd1ad445db3]1ad445957]
Nov 24 12:22:38 frr bgpd[20257]: /lib64/libfrr.so.0(+0x70e65) [0x7fd1ad465e65]ad445db3]1ad445957]
Nov 24 12:22:38 frr bgpd[20257]: /lib64/libpthread.so.0(+0xf5f0) [0x7fd1abd605f0]45db3]1ad445957]
Nov 24 12:22:38 frr bgpd[20257]: /usr/lib/frr/bgpd(bgp_table_range_lookup+0x63) [0x46cdc3]445957]
Nov 24 12:22:38 frr bgpd[20257]: /usr/lib64/frr/modules/bgpd_rpki.so(+0x4f0d) [0x7fd1a934ff0d]57]
Nov 24 12:22:38 frr bgpd[20257]: /lib64/libfrr.so.0(thread_call+0x60) [0x7fd1ad4736e0]934ff0d]57]
Nov 24 12:22:38 frr bgpd[20257]: /lib64/libfrr.so.0(frr_run+0x128) [0x7fd1ad443ab8]e0]934ff0d]57]
Nov 24 12:22:38 frr bgpd[20257]: /usr/lib/frr/bgpd(main+0x2e3) [0x41c043]1ad443ab8]e0]934ff0d]57]
Nov 24 12:22:38 frr bgpd[20257]: /lib64/libc.so.6(__libc_start_main+0xf5) [0x7fd1ab9a5505]f0d]57]
Nov 24 12:22:38 frr bgpd[20257]: /usr/lib/frr/bgpd() [0x41d9bb]main+0xf5) [0x7fd1ab9a5505]f0d]57]
Nov 24 12:22:38 frr bgpd[20257]: in thread bgpd_sync_callback scheduled from bgpd/bgp_rpki.c:351#012; aborting...
Nov 24 12:22:38 frr watchfrr[6779]: [EC 268435457] bgpd state -> down : read returned EOF
Nov 24 12:22:38 frr zebra[5952]: [EC 4043309116] Client 'bgp' encountered an error and is shutting down.
Nov 24 12:22:38 frr zebra[5952]: zebra/zebra_ptm.c:1345 failed to find process pid registration
Nov 24 12:22:38 frr zebra[5952]: client 15 disconnected. 0 bgp routes removed from the rib
I am not really 100% sure what we are really trying to do with this function, but we must
guard against child nodes not having any data.
Fixes: #5440 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Sun, 1 Dec 2019 14:29:32 +0000 (09:29 -0500)]
bgpd: Fix memory leak in json output of show commands
When dumping a large bit of table data via bgp_show_table
and if there is no information to display for a particular
`struct bgp_node *` the data allocated via json_object_new_array()
is leaked. Not a big deal on small tables but if you have a full
bgp feed and issue a show command that does not match any of
the route nodes ( say `vtysh -c "show bgp ipv4 large-community-list FOO"`)
then we will leak memory.
Before code change and issuing the above show bgp large-community-list command 15-20 times:
Memory statistics for bgpd:
System allocator statistics:
Total heap allocated: > 2GB
Holding block headers: 0 bytes
Used small blocks: 0 bytes
Used ordinary blocks: > 2GB
Free small blocks: 31 MiB
Free ordinary blocks: 616 KiB
Ordinary blocks: 0
Small blocks: 0
Holding blocks: 0
After:
Memory statistics for bgpd:
System allocator statistics:
Total heap allocated: 924 MiB
Holding block headers: 0 bytes
Used small blocks: 0 bytes
Used ordinary blocks: 558 MiB
Free small blocks: 26 MiB
Free ordinary blocks: 340 MiB
Ordinary blocks: 0
Small blocks: 0
Holding blocks: 0
Please note the 340mb of free ordinary blocks is from the fact I issued a
`show bgp ipv4 uni json` command and generated a large amount of data.
Fixes: #5445 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Renato Westphal [Thu, 7 Nov 2019 16:20:06 +0000 (13:20 -0300)]
lib: fix display of candidate configurations
Commit 5e6a9350c16 implemented an optimization where candidate
configurations are validated only before being displayed. The
validation is done only to create default child nodes (due to
how libyang works) and any possible error is ignored (candidate
configurations can be invalid/incomplete).
The problem is that we were calling lyd_validate() only when the
CLI "with-defaults" option was used. But some cli_show() callbacks
assume that default nodes exist and can crash when displaying a
candidate configuration that isn't validated. To fix this, call
lyd_validate() before displaying candidate configuration even when
"with-defaults" is not used (that was a micro-optimization that
shouldn't have been done).
David Lamparter [Fri, 29 Nov 2019 23:36:45 +0000 (00:36 +0100)]
lib: gcc 4.x workaround v2 for frr_interface_info
The previous workaround only works for -O0, at higher optimization
levels gcc reorders the statements in the file global scope which breaks
the asm statement :(.
Fixes: #4563 Fixes: #5074 Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Tue, 26 Nov 2019 16:05:47 +0000 (17:05 +0100)]
lib: add gcc 4.x workaround for frr_interface_info
gcc 4.x does not properly support structs with variable length array
members. Specifically, for global variables, it completely ignores the
array, coming up with a size much smaller than what is correct. This is
broken for both sizeof() as well as ELF object size.
This breaks for frr_interface_info since this variable is in some cases
copy relocated by the linker. (The linker does this to make the address
of the variable a "constant" for the main program.) This copying uses
the ELF object size, thereby copying only the non-array part of the
struct.
Breakage ensues...
(This fix is a bit ugly, but it's limited to very old gcc, and it's
better than changing the array to "nodes[1000]" and wasting memory...)
Fixes: #4563 Fixes: #5074 Signed-off-by: David Lamparter <equinox@diac24.net>
Chirag Shah [Mon, 25 Nov 2019 22:34:29 +0000 (14:34 -0800)]
bgpd: Handle possible non-selection of local route
In rare situations, the local route in a VNI may not get selected as the
best route. One situation is during a race between bgp and zebra which
was addressed in a prior commit. This change addresses another situation
where due to a change of tunnel IP, it is possible that a received route
may be selected as the best route if the path selection needs to take
next hop IPs into consideration. This is a pretty convoluted scenario,
but the code should handle it and delete and withdraw the local route
as well as (re)install the received route.
Ticket: CM-24114
Reviewed By: CCR-9487
Testing Done:
1. Manual tests - note, problem is not readily reproducible
2. evpn-smoke - results documented in the ticket
Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com> Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Ameya Dharkar [Fri, 22 Nov 2019 23:48:37 +0000 (15:48 -0800)]
bgpd: Do not perform "connected" check for EVPN nexthop
This changeset follows the PR
https://github.com/FRRouting/frr/pull/5334
Above PR adds nexthop tracking support for EVPN RT-5 nexthops.
This route is marked VALID only if the BGP route has a valid nexthop.
If the EVPN peer is an EBGP pee and "disable_connected_check" flag is not set,
"connected" check is performed for the EVPN nexthop.
But, usually EVPN nexthop is not the BGP peering address, but the VTEP address.
Also, NEXTHOP_UNCHANGED flag is enabled by default for EVPN.
As a result, in a common deployment for EVPN, EVPN nexthop is not connected.
Thus, adding a fix to remove the "connected" check for EVPN nexthops.
Don Slice [Fri, 22 Nov 2019 17:31:29 +0000 (17:31 +0000)]
zebra: knob to make ra retransmit interval rfc compliant
Problem reported by testing facility that our sending of Router
Advertisements more frequently than once very three seconds is not
compliant with rfc4861. Added a knob to turn off fast retransmits
in order to meet the requirement of the RFC.
Ticket: CM-27063 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Chirag Shah [Mon, 28 Oct 2019 22:05:05 +0000 (15:05 -0700)]
bgpd: adv pip to throw warning under default vrf
Instead of CMD_WARNING, use CMD_WARNING_CONFIG_FAILED
for any mis-configuration scenario.
Testing Done:
TOR(config)# router bgp 5548
TOR(config-router)# address-family l2vpn evpn
TOR(config-router-af)# no advertise-pip
This command is supported under L3VNI BGP EVPN VRF
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Chirag Shah [Tue, 15 Oct 2019 00:46:10 +0000 (17:46 -0700)]
zebra: print rmac and sys mac values
"show vrf vni" and "show evpn vni <l3vni>" commands
need to display correct router mac value.
"show evpn vni <l3vni>" detail l3vni needs to display
system mac as in PIP scenario value can be different.
Syste MAC would be derived from SVI interface MAC wherelse
Router MAC would be derived from macvlan interface MAC value.
Chirag Shah [Fri, 9 Aug 2019 01:58:03 +0000 (18:58 -0700)]
bgpd: evpn pip handle svi ip route
By default announct Self Type-2 routes with
system IP as nexthop and system MAC as
nexthop.
An API to check type-2 is self route via
checking ipv4/ipv6 address from connected interfaces list.
An API to extract RMAC and nexthop for type-2
routes based on advertise-svi-ip knob is enabled.
When advertise-pip is enabled/disabled, trigger type-2
route update. For self type-2 routes to use
anycast or individual (rmac, nexthop) addresses.
Ticket:CM-26190
Reviewed By:
Testing Done:
Enable 'advertise-svi-ip' knob in bgp default instance.
the vrf instance svi ip is advertised with nexthop
as default instance router-id and RMAC as system MAC.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Chirag Shah [Thu, 18 Apr 2019 07:17:57 +0000 (10:17 +0300)]
bgpd: evpn pip data struct and cli
Evpn Primary IP advertisement feature uses
individual system IP and system MAC for prefix (type-5)
and self type-2 routes.
The PIP knob is enabled by default for bgp vrf instance.
Configuration CLI for enable/disable PIP feature knob.
User can configure PIP system IP and MAC to retain as
permanent values.
For the PIP IP, the default behavior is to accept bgp default
instance's router-id. When the default instance router-id change,
reflect PIP IP assignment.
Reflect type-5 to use system-IP and system MAC as nexthop and RMAC
values.
Ticket:CM-26190
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Quentin Young [Thu, 21 Nov 2019 23:55:59 +0000 (18:55 -0500)]
bgpd: fix heap buffer overflow in lcom -> str enc
Spaces were not being accounted for in the heap buffer sizing, leading
to a heap buffer overflow when encoding large communities to their
string representations.
This patch also uses safer functions to do the encoding instead of
pointer math.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Satheesh Kumar K [Fri, 22 Nov 2019 04:33:42 +0000 (20:33 -0800)]
zebra: Fixing Comments in MLAG Read scheduling Events
Zebra MLAG is using "t_read" for multiple tasks, such as
1. For opening Communication channel with MLAG
2. In case conncetion fails, same event is used for retries
3. after the connection establishment, same event is used to
read the data from MLAG
since all these taks will never schedule together, this will not
cause any issues.
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
Donald Sharp [Fri, 22 Nov 2019 00:41:48 +0000 (19:41 -0500)]
bgpd: Prevent possible SA thinking we'll divide by zero
The half and reuse variables can never be 1 but the
SA systems we have do not know this and think it is possible.
Provide the kick in the snarples that the SA needs to know
this is not true.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Thu, 21 Nov 2019 23:47:13 +0000 (18:47 -0500)]
ospfd: nbr->oi is never null
We test nbr->oi in a couple of places for null, but
in the majority of places of the nbr->oi data is being
used we just access it. Touch up code to trust this
assertion and make the code more consistent in others.
Found in Coverity. Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
bgpd: Blank RD in "sh bgp l2vpn evpn all neighbors <ip> advertised-routes json"
Bug: While preparing the JSON output, 2 loops are traversed: the outer loop
loops through RD, and the inner loop loops through the prefixes of that RD.
We hit the bug (printing blank RD and stale or null prefix info) when the inner
loop exits with nothing to print, (without allocating json_routes) and the outer
loop still tries to attach it to the parent, json_adv. Thus, we have
key=<BLANK RD>, value=<junk or prev json_routes>
The fix: Avoid attaching json_routes to the parent json if there
is nothing to print.
root@ubuntu_nh:/home# ip rule show
0: from all lookup local
300: from 3.3.3.3 iif dummy2 lookup main
300: from 3.3.3.3 iif dummyVRF lookup 1111
301: from 4.4.4.4 iif dummy1 lookup 1111
301: from 4.4.4.4 iif dummy3 lookup 1111
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com-
Stephen Worley [Thu, 21 Nov 2019 20:53:59 +0000 (15:53 -0500)]
zebra: Set resolved inactive when > multipath_num
Apparently the multipath_num functionatlity has been broken
for a while because we were ignoring the recusive nexthops
when marking them inactive based on it.
This sets them as inactive as well if the parent breaks it.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Stephen Worley [Thu, 21 Nov 2019 20:27:12 +0000 (15:27 -0500)]
zebra: Use curr_active to check multipath_num
We were re-counting the entire group's active number on
every iteration of this nexthop_active_update() loop.
This is not great from a performance perspective but also
it was failing to properly mark things according to the
specified multipath_num.
Since a nexthop is set as active before this check, if its == to
the set ecmp, it gets marked inactive even though if its
under the max ecmp wanted!
ex)
set ecmp to 1.
`/usr/lib/frr/zebra -e 1`
All kernel routes will be marked inactive even with just one nexthop!
K 1.1.1.1/32 [0/0] is directly connected, dummy1 inactive, 00:00:10
K 1.1.1.2/32 [0/0] is directly connected, dummy2 inactive, 00:00:10
K 1.1.1.3/32 [0/0] is directly connected, dummy3 inactive, 00:00:10
K 1.1.1.4/32 [0/0] is directly connected, dummy4 inactive, 00:00:10
K 1.1.1.5/32 [0/0] is directly connected, dummy5 inactive, 00:00:10
K 1.1.1.6/32 [0/0] is directly connected, dummy6 inactive, 00:00:10
K 1.1.1.7/32 [0/0] is directly connected, dummy7 inactive, 00:00:10
K 1.1.1.8/32 [0/0] is directly connected, dummy8 inactive, 00:00:10
K 1.1.1.9/32 [0/0] is directly connected, dummy9 inactive, 00:00:10
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Donald Sharp [Thu, 21 Nov 2019 14:37:36 +0000 (09:37 -0500)]
zebra: BSD null routes were not being installed
On BSD systems null routes were not being installed into the
kernel. This is because commit 08ea27d1121ef5989cdc54fb178c05a7efc4cd3e
introduced a bug where we were attempting to use the wrong
prefix afi types and as such we were going down the v6 code path.
test27.lab.netdef.org# 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, q - queued route, r - rejected route
K>* 0.0.0.0/0 [0/0] via 192.168.122.1, 00:00:23
S>* 4.5.6.8/32 [1/0] unreachable (blackhole), 00:00:11
C>* 192.168.122.0/24 [0/1] is directly connected, vtnet0, 00:00:23
test27.lab.netdef.org# exit
[ci@test27 ~/frr]$ netstat -rn
Routing tables
Internet:
Destination Gateway Flags Netif Expire
default 192.168.122.1 UGS vtnet0
4.5.6.8/32 127.0.0.1 UG1B lo0
127.0.0.1 link#2 UH lo0
192.168.122.0/24 link#1 U vtnet0
192.168.122.108 link#1 UHS lo0
Fixes: #4843 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
v00lk [Sat, 16 Nov 2019 07:22:01 +0000 (10:22 +0300)]
bgpd: IPv4 LU withdraw using 0x000000 label
According to RFC 8277 IPv4 LU NLRI can be withdrawn using label 0x000000.
This RFC updates RFC3101 where it should be done only with 0x800000 label value.
Juniper implementation sets value 0x000000 when prefix is being withdrawn.
Page 12 RFC8277 states:
[RFC3107] also made it possible to withdraw a binding without
specifying the label explicitly, by setting the Compatibility field
to 0x800000. However, some implementations set it to 0x000000. In
order to ensure backwards compatibility, it is RECOMMENDED by this
document that the Compatibility field be set to 0x800000, but it is
REQUIRED that it be ignored upon reception.
Now FRR drops BGP session when receives such BGP update.
Donald Sharp [Wed, 20 Nov 2019 00:36:19 +0000 (19:36 -0500)]
pimd: Various buffer overflow reads and crashes
A variety of buffer overflow reads and crashes
that could occur if you fed bad info into pim.
1) When type is setup incorrectly we were printing the first 8 bytes
of the pim_parse_addr_source, but the min encoding length is
4 bytes. As such we will read beyond end of buffer.
2) The RP(pim, grp) macro can return a NULL value
Do not automatically assume that we can deref
the data.
3) BSM parsing was not properly sanitizing data input from wire
and we could enter into situations where we would read beyond
the end of the buffer. Prevent this from happening, we are
probably left in a bad way.
4) The received bit length cannot be greater than 32 bits,
refuse to allow it to happen.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Tue, 19 Nov 2019 20:46:42 +0000 (15:46 -0500)]
zebra: Router Advertisement socket mess up
The code for when a new vrf is created to properly handle
router advertisement for it is messed up in several ways:
1) Generation of the zrouter data structure should set the rtadv
socket to -1 so that we don't accidently close someone elses
open file descriptor
2) When you created a new zvrf instance *after* bootup we are XCALLOC'ing
the data structure so the zvrf->fd was 0. The shutdown code was looking
for the >= 0 to know if the fd existed (since fd 0 is valid!)
This sequence of events would cause zebra to consume 100% of the
cpu:
Run zebra by itself ( no other programs )
ip link add vrf1 type vrf table 1003
ip link del vrf vrf1
vtysh -c "configure" -c "no interface vrf1"
This commit fixes this issue.
Fixes: #5376 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
bisdhdh [Wed, 30 Oct 2019 09:42:25 +0000 (15:12 +0530)]
bgpd: Implementing a hash table for connected address - ipv4/ipv6
* IPv6 routes received via a ibgp session with one of its own interface as
nexthop are getting installed in the BGP table.
*A common table to be implemented should take cares of both
ipv4 and ipv6 connected addresses.
Don Slice [Tue, 19 Nov 2019 13:03:51 +0000 (05:03 -0800)]
tools: resolve issue with bfd timer change fix in frr-reload.py
Found that while the previous fix solved the traceback and created
the correct configuration, it was doing a delete/add process rather
than just an add. This was due to an incorrectly created search
string. This commit fixes that search string and testing verifies
that the correct thing is now being done.
Ticket: CM-27233 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Donald Sharp [Tue, 19 Nov 2019 13:22:50 +0000 (08:22 -0500)]
pimd: Fix possible read beyond end of data received
If a register packet is received that is less than the PIM_MSG_REGISTER_LEN
in size we can have a possible situation where the data being
checksummed is just random data from the buffer we read into.
2019/11/18 21:45:46 warnings: PIM: int pim_if_add_vif(struct interface *, _Bool, _Bool): could not get address for interface fuzziface ifindex=0
==27636== Invalid read of size 4
==27636== at 0x4E6EB0D: in_cksum (checksum.c:28)
==27636== by 0x4463CC: pim_pim_packet (pim_pim.c:194)
==27636== by 0x40E2B4: main (pim_main.c:117)
==27636== Address 0x771f818 is 0 bytes after a block of size 24 alloc'd
==27636== at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==27636== by 0x40E261: main (pim_main.c:112)
==27636==
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We already have this debugged, it's not going to change and the
end developer can stick this back in if needed by hand to debug
something that is not working properly.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
projects / mirror_frr.git / log