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>
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
exit2-debian-9# show ip bgp ipv4 unicast dampening parameters
Half-life time: 1 min
Reuse penalty: 2
Suppress penalty: 3
Max suppress time: 4 min
Max suppress penalty: 32
exit2-debian-9# show ip bgp ipv4 multicast dampening parameters
Half-life time: 5 min
Reuse penalty: 6
Suppress penalty: 7
Max suppress time: 8 min
Max suppress penalty: 18
ospf: BFD down not tearing down OSPF adjacency for point-to-point network
Root Cause:
Lookup for the point-to-point neighbor was failing because the neighbor
lookup was based on neighbor interface IP address. But, for point-to-point
neighbor the key is router-id for lookup. Lookup failure was causing the
BFD updates from PTM to get dropped.
Fix:
Added walk of the neighbor list if the network type is point-to-point to
find the appropriate neighbor. The match is based on source IP address of
the neighbor since that’s the address registered with BFD for monitoring.
bgpd: Do not send next-hop as :: in MP_REACH_NLRI if no link-local exists
This is the unusual case when you have global IPv6 address and no link-local
on interface attached. Like here:
eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP
link/ether 08:00:27:65:c6:82 brd ff:ff:ff:ff:ff:ff
inet6 2a02:4780:face::1/64 scope global
valid_lft forever preferred_lft forever
Donatas Abraitis [Fri, 25 Oct 2019 10:01:02 +0000 (13:01 +0300)]
nhrp: Make sure `no ip nhrp map <something>` works as expected
We passed peer as NULL and nothing happened.
exit2-debian-9# conf
exit2-debian-9(config)# int gre1
exit2-debian-9(config-if)# ip nhrp map 1.1.1.1 local
exit2-debian-9(config-if)# ip nhrp map 2.2.2.2 3.3.3.3
exit2-debian-9(config-if)# do sh run
...
!
interface gre1
ip nhrp map 1.1.1.1 local
ip nhrp map 2.2.2.2 3.3.3.3
!
...
exit2-debian-9(config-if)# no ip nhrp map 1.1.1.1
exit2-debian-9(config-if)# do sh run
...
!
interface gre1
ip nhrp map 2.2.2.2 3.3.3.3
!
Don Slice [Wed, 23 Oct 2019 16:30:28 +0000 (16:30 +0000)]
bgpd: remove error message for unkown afi/safi combination
Problem reported with error messages appearing in the log
complaining about invalid afi/safi combinations. Determined
that the error messages were recently added in the function
that turns afi and safi values to strings. Unfortunately,
the function is called from places using FOREACH_AFI_SAFI,
which spins thru every afi and safi number including some
that are not legal together (ipv4 evpn and l2vpn multicast
for example.) This fix removes these error messages since
it is not necessarily an error to call it with invalid
combinations.
Ticket: CM-26883 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Stephen Worley [Wed, 9 Oct 2019 23:35:46 +0000 (19:35 -0400)]
lib: Don't add/del from name tree if name isnt set
If the name has not been set yet (we were only passed the
ifindex in some cases like with master/slave timings) then
do not add/del it from the ifname rb tree on the vrf struct.
Doing so causes duplicate entries on the tree and infinte loops
can happen when iterating over it.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Stephen Worley [Wed, 9 Oct 2019 20:43:27 +0000 (16:43 -0400)]
lib: Use correct if compare function in tree proto
We were using the incorrect comparison function for the
ifindex-based rb tree. Luckily, we were using the correct one
in RB_GENERATE so I guess that overwrote what was declared in the
prototype?
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Rafael Zalamena [Tue, 15 Oct 2019 17:56:27 +0000 (14:56 -0300)]
bfdd: bind VRF sockets to devices
Always bind the created sockets to their respective VRF devices. With
this it should be possible to run BFD on VRFs without needing to weaken
the security setting `net.ipv4.udp_l3mdev_accept=1`.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Effectively prd or vnc.import.rd are `struct prefix_rd` which
are being typecast to a `struct prefix`. Not a big deal except commit 1315d74de97be2944d7b005b2f9a50e9ae5eff4d modified the prefix_cmp
function to allow for a sorted prefix_cmp. In prefix_cmp
we were looking at the offset and shift. In the case
of vnc we were passing a prefix length of 64 which is the exact length of
the remaining data structure for struct prefix_rd. So we calculated
a offset of 8 and a shift of 0. The data structures for the prefix
portion happened to be equal to 64 bits of data. So we checked that
with the memcmp got a 0 and promptly read off the end of the data
structure for the numcmp. The fix is if shift is 0 that means thei
the memcmp has checked everything and there is nothing to do.
Please note: We will still crash if we set the prefixlen > then
~312 bits currently( ie if the prefixlen specifies a bit length
longer than the prefix length ). I do not think there is
anything to do here( nor am I sure how to correct this either )
as that we are going to have some severe problems when we muck
up the prefixlen.
Fixes: #5025 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Thu, 10 Oct 2019 00:19:56 +0000 (20:19 -0400)]
bgpd: When creating extra from stack ensure it is zero'ed out
BGP code assumes that the extra data is zero'ed out. Ensure that we
are not leaving any situation that the data on the stack is actually all
0's when we pass it around as a pointer later.
Please note in issue #5025, Lou reported a different valgrind
issue, which is not the same issue:
==7313== Conditional jump or move depends on uninitialised value(s)
==7313== at 0x181F9F: subgroup_announce_check (bgp_route.c:1555)
==7313== by 0x1A112B: subgroup_announce_table (bgp_updgrp_adv.c:641)
==7313== by 0x1A1340: subgroup_announce_route (bgp_updgrp_adv.c:704)
==7313== by 0x1A13E3: subgroup_coalesce_timer (bgp_updgrp_adv.c:331)
==7313== by 0x4EBA615: thread_call (thread.c:1531)
==7313== by 0x4E8AC37: frr_run (libfrr.c:1052)
==7313== by 0x1429E0: main (bgp_main.c:486)
==7313==
==7313== Conditional jump or move depends on uninitialised value(s)
==7313== at 0x201C0E: rfapi_vty_out_vncinfo (rfapi_vty.c:429)
==7313== by 0x18D0D6: route_vty_out (bgp_route.c:7481)
==7313== by 0x18DD76: bgp_show_table (bgp_route.c:9365)
==7313== by 0x1930C4: bgp_show_table_rd (bgp_route.c:9471)
==7313== by 0x1932A3: bgp_show (bgp_route.c:9510)
==7313== by 0x193E68: show_ip_bgp_json (bgp_route.c:10284)
==7313== by 0x4E6D024: cmd_execute_command_real.isra.2 (command.c:1072)
==7313== by 0x4E6F51E: cmd_execute_command (command.c:1131)
==7313== by 0x4E6F686: cmd_execute (command.c:1285)
==7313== by 0x4EBF9C4: vty_command (vty.c:516)
==7313== by 0x4EBFB9F: vty_execute (vty.c:1285)
==7313== by 0x4EC250F: vtysh_read (vty.c:2119)
==7313==
that is causing the actual crash.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Tue, 15 Oct 2019 12:27:22 +0000 (08:27 -0400)]
bgpd: Soft reconfig-in should find the right bgp_path_info
When using soft reconfiguration inbound we are storing packet
data on the side for replaying when necessary. The problem here
is that we are just grabbing the first bgp_path_info and using
that as the base. What happens when we have soft-reconfig turned
on with multiple bgp_path_info's for a path? This was introduced
in commit 8692c506520f6b268525b80890702432c95f13c4, yes back
in 2012! I would argue, though, that it was just broken
in a different way before this.
Choose the correct bgp_path_info that corresponds to the peer
we received the data from for rethinking.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Martin Winter [Tue, 15 Oct 2019 14:44:47 +0000 (16:44 +0200)]
FRRouting Release 7.2
ALL Daemons
-N <namespace> to allow for config file locating when running FRR
inside of a namespace
Impoved Testing across all daemons
BFD
VRF Support
Conversion to Northbound interface
BGP
Aggregate-address add route-map support
BMP Support
Improved JSON output for many commands
`show bgp afi safi summary failed` command
`clear bop *` clears all peers
Show FQDN for `show bgp ipv4 uni` commands
Display BestPath selection reason as part of show commands
EIGRP
Infrastructure changes to allow VRF's
SIGHUP signals the config reload
Conversion to Northbound interface
ISIS
BFD Support
Support for circuits with MTU > 8192
PBRD
fwmark support as part of match criteria
autocompletion of PBRMAPS
Improved Nexthop Support
PIMD
PIM-BSM receive support
Improved debugging support
Store ECMP paths that are not currently legal for use
Disallow igmp query from a non-connected source
Many new cli improvements and changes
VRRPD
Add Support for RFC 3768 and RFC 5798
Route-Maps
Add sequence numbers to access-lists
Add `match ip next-hop type blackhole`
Improved ability to notice dependency changes
SHARPD
`sharp watch [import|nexthop]` you can now specify a prefix instead
of assuming a /32
STATICD
Significantly Improved NHT
ZEBRA
Many dataplane improvements for routes, neighbor table and EVPN
NHT cli can now be specified per VRF and improved ability to control
NHT data being shown
Removed duplicate processing of routes
Improved debugablility
RMAC and VxLan support for the FPM
LIB
RCU support
Nexthop Group Improvements
`log-filter WORD` added
Building
openssl support
libcap should be used as part of build or significant slowdowns will
be experienced
Lua builds have been fixed
Improved Cross building
Snapcraft
Add Fabricd
Add Libyan
Update rtrlib and rpki
Signed-off-by: Martin Winter <mwinter@opensourcerouting.org>
Rafael Zalamena [Fri, 11 Oct 2019 19:13:24 +0000 (16:13 -0300)]
bfdd: simplify session observers code
Don't be selective about what to observe, always observe all possible
aspects of the session that may change on run-time (i.e. bind address,
interface and VRF existence).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Rafael Zalamena [Fri, 11 Oct 2019 16:12:26 +0000 (13:12 -0300)]
bfdd: set session down after disabling it
If a session is no longer able to send/receive packets, it is very
likely it will be down in a few milliseconds so lets speed up the
process and correctly mark it as down.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
broke the usage of ZEBRA_ROUTE_ALL as a valid redistribution
command. This commit puts it back in. LDP uses ZEBRA_ROUTE_ALL
as an option to say it is interested in all REDISTRIBUTION events.
Fixes: #5072 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Mon, 30 Sep 2019 23:53:44 +0000 (01:53 +0200)]
isisd: Fix handling of neighbor circuit id in three way handshake
RFC 5303 states:
If the system ID and Extended Local Circuit ID of the neighboring
system are known (in adjacency three-way state Initializing or
Up), the neighbor's system ID SHALL be reported in the Neighbor
System ID field, and the neighbor's Extended Local Circuit ID
SHALL be reported in the Neighbor Extended Local Circuit ID field.
There is nothing written about only setting the Extended circuit ID of the
adjacency only when we bring the three-way adjacency up.
In fact, we should always update it, to avoid the problem described in #4783.
Fixes: #4783 Signed-off-by: Christian Franke <chris@opensourcerouting.org>
donna.cumulusnetworks.com# conf
donna.cumulusnetworks.com(config)# router bgp
donna.cumulusnetworks.com(config-router)# aggregate-address 30.0.5.0 255.255.255.0 summary-only
donna.cumulusnetworks.com(config-router)# do show run
Building configuration...
Don Slice [Tue, 24 Sep 2019 12:02:02 +0000 (05:02 -0700)]
bgpd: stop sending nexthop set by "route-map in" to eBGP peers
Problem reported that when a "neighbor x.x.x.x route-map FOO in"
set a next-hop value, that modified next-hop value was also sent
to eBGP peers. This is incorrect since bgp is expected to set
next-hop to self when sending to eBGP peers unless third party
next-hop on a shared segment is true. This fix modifies the
behavior to stop sending the modified next-hop to eBGP peers
if the route-map was applied inbound on another peer.
Ticket: CM-26025 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
The "abort_if_not_found" parameter of nb_running_get_entry()
should be set to true only when this function is called during the
NB_EV_APPLY phase of a northbound callback. Failure to respect this
can lead to crashes when multiple configuration changes are being
committed at the same time.
Donald Sharp [Fri, 20 Sep 2019 10:41:02 +0000 (06:41 -0400)]
bgpd: Invalid NH's should send an apropriate reason code
RFC 4271 sec 6.3 p33, In the case of a BGP_NEXTHOP attribute with an
incorrect value, FRR is supposed to send a notification
and include 'Corresponding type, length and value of the NEXT_HOP
attribute in the notification data.
Fixes: #4997 Signed-off-by: Nikos <ntriantafillis@gmail.com> Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Mark Stapp [Tue, 17 Sep 2019 15:33:46 +0000 (11:33 -0400)]
zebra: check all dplane nexthops when processing
[7.2 version]
When processing route updates from the dataplane, we were
terminating the checking of nexthops prematurely, and we could
miss meaningful changes.
Quentin Young [Mon, 16 Sep 2019 15:33:49 +0000 (15:33 +0000)]
bgpd: do not send keepalives when KA timer is 0
RFC4271 specifies behavior when the hold timer is sent to zero - we
should not send keepalives or run a hold timer. But FRR, and other
vendors, allow the keepalive timer to be set to zero with a nonzero hold
timer. In this case we were sending keepalives constantly and maxing out
a pthread to do so. Instead behave similarly to other vendors and do not
send keepalives.
Unsure what the utility of this is, but blasting keepalives is
definitely the wrong thing to do.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Mark Stapp [Thu, 12 Sep 2019 15:30:42 +0000 (11:30 -0400)]
zebra: revise redistribution delete to improve update case
When selecting a new best route, zebra sends a redist update
when the route is installed. There are cases where redist
clients may not see that redist add - clients who are not
subscribed to the new route type, e.g. In that case, attempt
to send a redist delete for the old/previous route type.
Revised the redist delete api to accomodate both cases;
also tightened up the const-ness of a few internal redist apis.