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>
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>
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.
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>
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>
Donald Sharp [Thu, 7 Nov 2019 00:49:06 +0000 (19:49 -0500)]
ospfd: Add/fix some debugs to handle vrf
This commit has:
The received packet path in ospf, had absolutely no debugs associated with
it. This makes it extremely hard to know when we receive packets for
consumption. Add some breadcrumbs to this end.
Large chunks of commands have no ability to debug what is happening
in what vrf. With ip overlap X vrf this becomes a bit of a problem
Add some breadcrumbs here.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 6 Nov 2019 22:12:51 +0000 (17:12 -0500)]
ospfd: Remove ORIGINAL_CODING check
We have a bunch of places that look for ORIGINAL_CODING. There is
nothing in our configure system to define this value and a quick
git blame shows this code as being original to the import a very
very long time ago. This is dead code, removing.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Philippe Guibert [Mon, 18 Nov 2019 11:05:47 +0000 (12:05 +0100)]
lib: no filter operations fix on appropriate access-list name
some vty no operations were not removing the entry of the access-list,
since the access-list name was not retrieved correctly. the index was
not correct for 'no ipv6 access-list WORD' operations, but also for one 'no
access-list WORD [..] any' operation.
PR=66244 Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com> Acked-by: Alain Ritoux <alain.ritoux@6wind.com>
Donald Sharp [Mon, 18 Nov 2019 19:38:39 +0000 (14:38 -0500)]
zebra: FPM should have a way of shutting down
When we shut down zebra, we were not doing anything to shut
down the FPM. Perform the necessary occult rituals and
stop the threads from running during early shutdown.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Mitch Skiba [Thu, 14 Nov 2019 19:28:23 +0000 (19:28 +0000)]
bgpd: Fix per afi/safi addpath peer counting
The total_peercount table was created as a short cut for queries about
if addpath was enabled at all on a particular afi/safi. However, the
values weren't updated, so BGP would act as if addpath wasn't enabled
when determining if updates should be sent out. The error in behavior
was much more noticeable in tx-all than best-per-as, since changes in
what is sent by best-per-as would often trigger updates even if addpath
wasn't enabled.
Donald Sharp [Mon, 18 Nov 2019 16:43:52 +0000 (11:43 -0500)]
pimd: Create pimreg interface when we start any interface config
When you configure interface configuration without explicitly
configuring pim on that interface, we were not creating the pimreg
interface and as such we would crash in an attempted register
since the pimreg device is non-existent.
The crash is this:
==8823== Invalid read of size 8
==8823== at 0x468614: pim_channel_add_oif (pim_oil.c:392)
==8823== by 0x46D0F1: pim_register_join (pim_register.c:61)
==8823== by 0x449AB3: pim_mroute_msg_nocache (pim_mroute.c:242)
==8823== by 0x449AB3: pim_mroute_msg (pim_mroute.c:661)
==8823== by 0x449AB3: mroute_read (pim_mroute.c:707)
==8823== by 0x4FC0676: thread_call (thread.c:1549)
==8823== by 0x4EF3A2F: frr_run (libfrr.c:1064)
==8823== by 0x40DCB5: main (pim_main.c:162)
==8823== Address 0xc8 is not stack'd, malloc'd or (recently) free'd
Donald Sharp [Mon, 18 Nov 2019 14:05:55 +0000 (09:05 -0500)]
pimd: Tighten up `show ip pim rp-info`
We were adding a newline for the source in some cases
but not others and tighten up the display of data
eva# show ip pim rp-info
RP address group/prefix-list OIF I am RP Source
10.254.0.1 224.0.0.0/4 lo yes Static
4.4.4.4 225.1.2.3/32 abcdefghijklmno yes Static
10.0.20.45 226.200.100.100/32 r1-eth0 no Static
eva#
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Ameya Dharkar [Thu, 14 Nov 2019 00:46:56 +0000 (16:46 -0800)]
bgpd: Add nexthop of received EVPN RT-5 for nexthop tracking
Problem statement:
When IPv4/IPv6 prefixes are received in BGP, bgp_update function registers the
nexthop of the route with nexthop tracking module. The BGP route is marked as
valid only if the nexthop is resolved.
Even for EVPN RT-5, route should be marked as valid only if the the nexthop is
resolvable.
Code changes:
1. Add nexthop of EVPN RT-5 for nexthop tracking. Route will be marked as valid
only if the nexthop is resolved.
2. Only the valid EVPN routes are imported to the vrf.
3. When nht update is received in BGP, make sure that the EVPN routes are
imported/unimported based on the route becomes valid/invalid.
Testcases:
1. At rtr-1, advertise EVPN RT-5 with a nexthop 10.100.0.2.
10.100.0.2 is resolved at rtr-2 in default vrf.
At rtr-2, remote EVPN RT-5 should be marked as valid and should be imported into
vrfs.
2. Make the nexthop 10.100.0.2 unreachable at rtr-2
Remote EVPN RT-5 should be marked as invalid and should be unimported from the
vrfs. As this code change deals with EVPN type-5 routes only, other EVPN routes
should be valid.
3. At rtr-2, add a static route to make nexthop 10.100.0.2 reachable.
EVPN RT-5 should again become valid and should be imported into the vrfs.
Trey Aspelund [Fri, 8 Nov 2019 21:51:36 +0000 (16:51 -0500)]
staticd: Make blackhole keyword errors more straightforward
Previous error was misleading and made it seem like Null0,
reject, or blackhole nexthops on static routes are invalid.
This commit makes it more clear as to why the error is seen.
Satheesh Kumar K [Tue, 12 Nov 2019 09:41:04 +0000 (01:41 -0800)]
Zebra: ADD Protobuf Encoding & Decoding for MLAG Messages
1. add the Mlag ProtoBuf Lib to Zebra Compilation
2. Encode the messages with protobuf before writing to MLAG
3. Decode the MLAG Messages using protobuf and write to clients
based on their subscrption.
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
Satheesh Kumar K [Tue, 12 Nov 2019 08:40:35 +0000 (00:40 -0800)]
Zebra: adding support for Zebra MLAG Functionality
This includes:
1. Processing client Registrations for MLAG
2. storing client Interests for MLAG updates
3. Opening communication channel to MLAG with First client reg
4. Closing Communication channel with last client De-reg
5. Spawning a new thread for handling MLAG updates peocessing
6. adding Test code
7. advertising MLAG Updates to clients based on their interests
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
Satheesh Kumar K [Tue, 12 Nov 2019 06:36:17 +0000 (22:36 -0800)]
pimd : Add support for MLAG Register & Un-register
when ever a FRR Client wants to send any data to another node
using MLAG Channel, uses below mechanisam.
1. sends a MLAG Registration to zebra with interested messages that
it is intended to receive from peer.
2. In response to this request, Zebra opens communication channel with
MLAG. and also in Rx. diretion zebra forwards only those messages which
client shown interest during registration
3. when client is no-longer interested in communicating with MLAG, client
posts De-register to Zebra
4. if this is the last client which is interested for MLAG Communication,
zebra closes the channel.
why PIM Needs MLAG Communication
================================
1. In general on LAN Networks elecetd DR will send the Join towards
Multicast RP in case of a LHR and Register in case of FHR.
2. But in case DR Goes down, traffic will be re-converged only after
the New DR is elected, but this can take time based on Hold Timer to
detect the DR down.
3. this can be optimised by using MLAG Mecganisam.
4. and also Traffic can be forwarded more efficiently by knowing the cost
towards RP using MLAG
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
Don Slice [Mon, 11 Nov 2019 20:45:32 +0000 (15:45 -0500)]
tools: stop repeating lines to delete for multiple bfd timer changes
Problem reported with tracebacks seen when making multiple bfd timer
changes in frr.conf and applying via frr-reload.py. Found that when
multiple bfd timer changes are made, the same line can be added for
deletion more than once, causing the traceback when the deletion is
performed. This fix verifies the correct line is being appended for
deletion.
Ticket: CM-27233 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
bgpd: Bug fix in "show bgp l2vpn evpn ... advertised-routes'
The bug:
As part of displaying advertised routes to a peer, in the outer loop, we
iterate through all prefixes in the evpn table. In the inner loop,
we iterate through adj_out of each prefix.
If a prefix which is present in the evpn table is not advertised to a peer,
its corresponding attr == NULL. Checking for this condition is the fix.
Stephen Worley [Tue, 12 Nov 2019 19:56:56 +0000 (14:56 -0500)]
zebra: dplane cancel update thread from correct pthread
This code is called from the zebra main pthread during shutdown
but the thread event is scheduled via the zebra dplane pthread.
Hence, we should be using the `thread_cancel_async()` API to
cancel the thread event on a different pthread.
This is only ever hit in the rare case that we still have work left
to do on the update queue during shutdown.
Found via zebra crash:
```
(gdb) bt
\#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
\#1 0x00007f4e4d3f7535 in __GI_abort () at abort.c:79
\#2 0x00007f4e4d3f740f in __assert_fail_base (fmt=0x7f4e4d559ee0 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x7f4e4d9071d0 "master->owner == pthread_self()",
file=0x7f4e4d906cf8 "lib/thread.c", line=1185, function=<optimized out>) at assert.c:92
\#3 0x00007f4e4d405102 in __GI___assert_fail (assertion=assertion@entry=0x7f4e4d9071d0 "master->owner == pthread_self()", file=file@entry=0x7f4e4d906cf8 "lib/thread.c",
line=line@entry=1185, function=function@entry=0x7f4e4d906b68 <__PRETTY_FUNCTION__.15817> "thread_cancel") at assert.c:101
\#4 0x00007f4e4d8d095a in thread_cancel (thread=0x55b40d01a640) at lib/thread.c:1185
\#5 0x000055b40c291845 in zebra_dplane_shutdown () at zebra/zebra_dplane.c:3274
\#6 0x000055b40c27ee13 in zebra_finalize (dummy=<optimized out>) at zebra/main.c:202
\#7 0x00007f4e4d8d1416 in thread_call (thread=thread@entry=0x7ffcbbc08870) at lib/thread.c:1599
\#8 0x00007f4e4d8a1ef8 in frr_run (master=0x55b40ce35510) at lib/libfrr.c:1024
\#9 0x000055b40c270916 in main (argc=8, argv=0x7ffcbbc08c78) at zebra/main.c:483
(gdb) down
\#4 0x00007f4e4d8d095a in thread_cancel (thread=0x55b40d01a640) at lib/thread.c:1185
1185 assert(master->owner == pthread_self());
(gdb)
```
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>