Renato Westphal [Thu, 24 Nov 2016 23:28:03 +0000 (21:28 -0200)]
isisd: fix loss of packets after circuit is brought up
The last parameter of THREAD_TIMER_ON() is the timeout, and we were
using circuit->fd for that. So, when a circuit was brought up, isisd
would miss all received packets on this circuit for quite a few seconds,
slowing down the convergence process.
To fix this, use the same logic we use in isis_receive() to calculate
this timeout.
This bug doesn't happen on Linux, which uses a different method to read
packets from the network.
Fixes the following ANVL tests on FreeBSD: ISIS-17.1, ISIS-18.6 (and
probably others too).
David Lamparter [Tue, 15 Nov 2016 02:18:43 +0000 (11:18 +0900)]
build: chop down complicated CFLAGS logic
Other packages don't have --with-cflags; we don't need it either. The
user can specify CFLAGS= in the environment or on ./configure and that
would work perfectly fine. If only it weren't for autoconf being an
idiot and adding its own "-g -O2" ... so we work around that.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Problem reported that in certain configs, when a router is initially
booted and the link is bounced, we can end up with a bogus static route
in the table. This was due to the assumption in zebra_rnh that a static
route would not be recursively resolved through another static route with
a different next-hop. This fix changes this assumption. Tested manually
and bgp-min, ospf-min, and vrf-min run with no new failures.
Ticket: CM-13328 Signed-off-by: Don Slice Reviewed-by: CCR-5338
David Lamparter [Wed, 9 Nov 2016 13:42:47 +0000 (14:42 +0100)]
lib: add minimal no-config VTY mode
This silences the following warning from watchquagga:
"Can't save to configuration file, using vtysh."
which otherwise appears when doing a "write file" in vtysh when no
integrated-config is in use.
Also make "show memory" available in watchquagga.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Donald Sharp [Wed, 9 Nov 2016 15:22:22 +0000 (10:22 -0500)]
watchquagga: Signal when we are actually up and running
When Quagga is starting up, it is returning immediately.
This is leaving us in a state where systemd believes
Quagga is up and running, while the sytem might actually
not have restarted the code yet.
Modify the code so that when watchquagga starts up
it doesn't start communicating with systemd until
such time that it detects that all daemons are
running.
Additionally modify watchquagga to touch a
file in /var/run/quagga/ that the /usr/lib/quagga/quagga
script looks for for 10 seconds. If it finds this
Quagga started file then we know watchquagga
has successfully communicated with all daemons.
If after 10 seconds we haven't communicated
with Quagga, continue on for the start and let the
normal start failure code work.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
David Lamparter [Tue, 8 Nov 2016 22:36:16 +0000 (23:36 +0100)]
vtysh: funnel integrated write through watchquagga
Running vtysh as normal user won't have permissions to write
Quagga.conf. If we're connected to watchquagga, try "write integrated"
first. In all cases if something fails, try directly.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Tue, 8 Nov 2016 18:41:48 +0000 (19:41 +0100)]
vtysh: add watchquagga to target list
Also tag some commands as VTYSH_REALLYALL; these are absolutely
neccessary for correct vtysh operation and will cause "interesting"
breakage if not present on all daemons.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Wed, 9 Nov 2016 13:15:34 +0000 (14:15 +0100)]
watchquagga: add "write integrated"
This new command - available for internal use by vtysh and explicit
usage by users - calls "vtysh -w" from watchquagga. This ensures vtysh
is run with privileges to actually write the integrated-config file.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Tue, 8 Nov 2016 22:36:01 +0000 (23:36 +0100)]
vtysh: set config file permissions
As vtysh may hopefully be running as root from watchquagga here, let's
try to fix up ownership and permissions for Quagga.conf. Doing
chown/chmod instead of changing the process's user/group IDs has the
advantage of fixing up preexisting misconfigurations.
Note errors in chmod/chown will print a message but the config is
already written at that point.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Tue, 8 Nov 2016 18:01:06 +0000 (19:01 +0100)]
vtysh: add -w option for integrated-config write
This new option is intended to be used both by watchquagga as well as
directly by users. It performs the collect-configuration operation and
writes out Quagga.conf, regardless of whether integrated-config is
enabled or not.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Tue, 8 Nov 2016 17:22:30 +0000 (18:22 +0100)]
vtysh: detangle configuration writes
vtysh has a very convoluted and confusing setup where it isn't even
clear which files are written where (since some filenames come
indirectly from loading config). Detangle.
This also removes writing vtysh.conf. The file is intended to be
manually edited since it has some vague security concerns (if PAM is
used).
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Tue, 8 Nov 2016 18:42:01 +0000 (19:42 +0100)]
lib: privs: always look up VTY group
Even if we're running without user switch, we should still try to honor
the VTY group. This applies both to watchquagga (which always runs as
root) as well as "no-userswitch" configurations for other daemons.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Tue, 8 Nov 2016 19:46:05 +0000 (20:46 +0100)]
lib: add and use set_cloexec()
watchquagga is already leaking an open file descriptor on its pid file
on fork+exec() invocations; next up is adding vtysh support with even
more fds. Mark things CLOEXEC before going there.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Paul Jakma [Fri, 22 Apr 2016 11:48:49 +0000 (12:48 +0100)]
bgpd: Squash spurious "unknown afi" log messages
* bgp_packet.c: (bgp_update_receive) doesn't differentiate between NLRIs that
are 0 AFI/SAFI cause they weren't set, and those because a peer sent a
bogus AFI/SAFI, before sending sending what may be a misleading, spurious
log message. Check the .nlri pointer is set and avoid this.
Incorporating a suggestion from: G. Paul Ziemba <unp@ziemba.us>
Paul Jakma [Thu, 4 Feb 2016 17:00:18 +0000 (17:00 +0000)]
bgpd: Remove the double-pass parsing of NLRIs
* bgpd parses NLRIs twice, a first pass "sanity check" and then a second pass
that changes actual state. For most AFI/SAFIs this is done by
bgp_nlri_sanity_check and bgp_nlri_parse, which are almost identical.
As the required action on a syntactic error in an NLRI is to NOTIFY and
shut down the session, it should be acceptable to just do a one pass
parse. There is no need to atomically handle the NLRIs.
* bgp_route.h: (bgp_nlri_sanity_check) Delete
* bgp_route.c: (bgp_nlri_parse) Make the prefixlen size check more general
and don't hard-code AFI/SAFI details, e.g. use prefix_blen library function.
Add error logs consistent with bgp_nlri_sanity_check as much as possible.
Add a "defense in depth" type check of the prefixlen against the sizeof
the (struct prefix) storage - ala bgp_nlri_parse_vpn.
Update standards text from draft RFC4271 to the actual RFC4271 text.
Extend the semantic consistency test of IPv6. E.g. it should skip mcast
NLRIs for unicast safi as v4 does.
* bgp_mplsvpn.{c,h}: Delete bgp_nlri_sanity_check_vpn and make
bgp_nlri_parse_vpn_body the bgp_nlri_parse_vpn function again.
(bgp_nlri_parse_vpn) Remove the notifies. The sanity checks were
responsible for this, but bgp_update_receive handles sending NOTIFY
generically for bgp_nlri_parse.
* bgp_attr.c: (bgp_mp_reach_parse,bgp_mp_unreach_parse) Delete sanity check.
NLRI parsing done after attr parsing by bgp_update_receive.
Arising out of discussions on the need for two-pass NLRI parse with:
Lou Berger <lberger@labn.net>
Donald Sharp <sharpd@cumulusnetworks.com>
Paul Jakma [Thu, 4 Feb 2016 13:27:04 +0000 (13:27 +0000)]
bgpd: Regularise bgp_update_receive, add missing notifies and checks
* bgp_packet.c: (bgp_update_receive) Lots of repeated code, doing same
thing for each AFI/SAFI. Except when it doesn't, e.g. the IPv4/VPN
case was missing the EoR bgp_clear_stale_route call - the only action
really needed for EoR.
Make this function a lot more regular, using common, AFI/SAFI
independent blocks so far as possible.
Replace the 4 separate bgp_nlris with an array, indexed by an enum.
The distinct blocks that handle calling bgp_nlri_parse for each
different AFI/SAFI can now be replaced with a loop.
Transmogrify the nlri SAFI from the SAFI_MPLS_LABELED_VPN code-point
used on the wire, to the SAFI_MPLS_VPN safi_t enum we use internally
as early as possible.
The existing code was not necessarily sending a NOTIFY for NLRI
parsing errors, if they arose via bgp_nlri_sanity_check. Send the
correct NOTIFY - INVAL_NETWORK for the classic NLRIs and OPT_ATTR_ERR
for the MP ones.
EoR can now be handled in one block. The existing code seemed broken
for EoR recognition in a number of ways:
1. A v4/unicast EoR should be an empty UPDATE. However, it seemed
to be treating an UPDATE with attributes, inc. MP REACH/UNREACH,
but no classic NLRIs, as a v4/uni EoR.
2. For other AFI/SAFIs, it was treating UPDATEs with no classic
withraw and with a zero-length MP withdraw as EoRs. However, that
would mean an UPDATE packet _with_ update NLRIs and a 0-len MP
withdraw could be classed as an EoR.
This seems to be loose coding leading to ambiguous protocol
situations and likely incorrect behaviour, rather than simply being
liberal. Be more strict about checking that an UPDATE really is an
EoR and definitely is not trying to update any NLRIs.
This same loose EoR parsing was noted by Chris Hall previously on
list.
(bgp_nlri_parse) Front end NLRI parse function, to fan-out to the correct
parser for the AFI/SAFI.
* bgp_route.c: (bgp_nlri_sanity_check) We try convert NLRI safi to
internal code-point ASAP, adjust switch for that. Leave the wire
code point in for defensive coding.
(bgp_nlri_parse) rename to bgp_nlri_parse_ip.
* tests/bgp_mp_attr_test.c: Can just use bgp_nlri_parse frontend.
Paul Jakma [Wed, 27 Jan 2016 16:37:33 +0000 (16:37 +0000)]
bgpd: Regularise BGP NLRI sanity checks a bit
* bgp_route.h: (bgp_nlri_sanity_check) The bulk of the args are equivalent
to a (struct bgp_nlri), consolidate.
* bgp_route.c: (bgp_nlri_sanity_check) Make this a frontend for all afi/safis.
Including SAFI_MPLS_LABELED_VPN.
(bgp_nlri_sanity_check_ip) Regular IP NLRI sanity check based on the
existing code, and adjusted for (struct bgp_nlri *) arg.
* bgp_attr.c: (bgp_mp_reach_parse) Adjust for passing (struct bgp_nlri *)
to bgp_nlri_sanity_check.
Get rid of special-casing to not sanity check VPN.
(bgp_mp_unreach_parse) Ditto.
* bgp_mplsvpn.c: Use the same VPN parsing code for both the sanity
check and the actual parse.
(bgp_nlri_parse_vpn) renamed to bgp_nlri_parse_vpn_body and made
internal.
(bgp_nlri_parse_vpn_body) Added (bool) argument to control whether it
is sanity checking or whether it should update routing state for each
NLRI. Send a NOTIFY and reset the session, if there's a parsing
error, as bgp_nlri_sanity_check_ip does, and as is required by the
RFC.
(bgp_nlri_parse_vpn) now a wrapper to call _body with update.
(bgp_nlri_sanity_check_vpn) wrapper to call parser without
updating.
* bgp_mplsvpn.h: (bgp_nlri_sanity_check_vpn) export for
bgp_nlri_sanity_check.
* bgp_packet.c: (bgp_update_receive) Adjust for bgp_nlri_sanity_check
argument changes.
* test/bgp_mp_attr_test.c: Extend to also test the NLRI parsing functions,
if the initial MP-attr parsing has succeeded. Fix the NLRI in the
VPN cases. Add further VPN tests.
* tests/bgpd.tests/testbgpmpattr.exp: Add the new test cases.
This commit a joint effort of:
Lou Berger <lberger@labn.net>
Donald Sharp <sharpd@cumulusnetworks.com>
Paul Jakma <paul.jakma@hpe.com> / <paul@jakma.org>
Paul Jakma [Fri, 5 Feb 2016 14:57:17 +0000 (14:57 +0000)]
bgpd: make bgp_nlri_parse_encap conform with other nlri_parse funcs
* bgp_encap.{c,h} (bgp_nlri_parse_encap) afi is already in the NLRI argument.
update or withdraw is signalled by attr being non-NULL or NULL.
* bgp_packet.c: (update_receive) fixup to match, and also make the attr
argument conform with NLRI_ATTR_ARG for correct error handling on
optional, transitive, partial, attributes.
Donald Sharp [Tue, 25 Oct 2016 18:25:29 +0000 (14:25 -0400)]
zebra: Disable mpls slightly different
When mpls is not turned on in the kernel, we
are not installing the mpls commands into the cli.
This results in vtysh attempting to run the command
and receiving a 'WTF is this command' back from zebra.
Modify the mpls code to install commands and to check
to see if the command should be accepted based
upon mpls working or not.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com> Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>
Daniel Walton [Tue, 25 Oct 2016 14:16:31 +0000 (14:16 +0000)]
bgpd: dynamically grow 'show ip bgp summ' Neighbor column width
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com> Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Now that we display hostnames in 'show ip bgp summary' it is really easy
to have the first column be greater than 16 characters which causes a
line wrap. The line wrap makes the output difficult to read.
Before
======
superm-redxp-05# show ip bgp summ
BGP router identifier 6.0.0.11, local AS number 65001 vrf-id 0
BGP table version 56
RIB entries 19, using 2280 bytes of memory
Peers 2, using 41 KiB of memory
Peer groups 1, using 64 bytes of memory
After
=====
superm-redxp-05# show ip bgp summ
BGP router identifier 6.0.0.11, local AS number 65001 vrf-id 0
BGP table version 10
RIB entries 19, using 2280 bytes of memory
Peers 2, using 41 KiB of memory
Peer groups 1, using 64 bytes of memory
Quentin Young [Tue, 25 Oct 2016 00:29:25 +0000 (00:29 +0000)]
tools: cmd_check.py checks headers too
Sometimes commands are externed and installed in another
file, so check for a command's name in the header file
corresponding to the file it's defined in before marking
it uninstalled.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Donald Sharp [Mon, 24 Oct 2016 17:28:35 +0000 (13:28 -0400)]
lib: Fix race condition in shutdown of routemap
When shutting down a daemon that uses an update
timer to handle route map processing, there
exists a race condition where if you change
a route map and then immediately shutdown
quagga before the update timer for the routemap
runs, you will be placed in a infinite loop.
This condition happens because this commit introduces
route map memory free'ing but never tests to see
if the to_be_processed flag has happened or not
before deleting:
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com> Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Ticket: CM-13207
normal table on spine-1....we do not see 6.0.0.10 (spine-2's loopback)
spine-1 and spine-2 are in AS 65200
superm-redxp-05# show ip bgp
BGP table version is 13, local router ID is 6.0.0.9
Status codes: s suppressed, d damped, h history, * valid, > best, =
multipath,
i internal, r RIB-failure, S Stale, R Removed
Origin codes: i - IGP, e - EGP, ? - incomplete
Displayed 9 out of 13 total prefixes
superm-redxp-05#
spine-1 with "neighbor x.x.x.x allowas-in origin", we now see 6.0.0.10
superm-redxp-05# show ip bgp
BGP table version is 14, local router ID is 6.0.0.9
Status codes: s suppressed, d damped, h history, * valid, > best, =
multipath,
i internal, r RIB-failure, S Stale, R Removed
Origin codes: i - IGP, e - EGP, ? - incomplete
Displayed 10 out of 21 total prefixes
superm-redxp-05#
The only as-paths with 65200 that made it through were the ones that
originated from 65200
superm-redxp-05# show ip bgp regexp _65200_
BGP table version is 14, local router ID is 6.0.0.9
Status codes: s suppressed, d damped, h history, * valid, > best, =
multipath,
i internal, r RIB-failure, S Stale, R Removed
Origin codes: i - IGP, e - EGP, ? - incomplete
bgpd: add L3/L2VPN Virtual Network Control feature
Added vnc support to the bgp daemon. In addition
it modified vtysh_config.c to help vtysh understand
bgp sub-modes. This caused the output of the show
run bgp command to be displayed incorrectly:
Backing out this change allows vtysh to have the
correct display of bgp now.
Ticket: CM-13136 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com> Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com> Reviewed-by: David Lamparter <equinox@opensourcerouting.org>
Donald Sharp [Fri, 21 Oct 2016 01:20:15 +0000 (21:20 -0400)]
lib: Fix for int64 and json on some systems
When compiling json on systems with json/json.h
that don't have json_object_new_int64, just
use json_object_new_int instead and accept
we might truncate data.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Tue, 14 Jun 2016 18:07:09 +0000 (20:07 +0200)]
vtysh: handle case if there is no match in "write terminal $daemon"
While the DEFUN should match the list of clients registered in
vtysh, it seems better to handle the case explicitly instead of
relying on the client list and the DEFUN signature being in sync.
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Tue, 14 Jun 2016 18:07:08 +0000 (20:07 +0200)]
ripd: print md5 auth digest correctly
The dump of the md5 hash was missing one byte of the hash.
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Tue, 14 Jun 2016 18:07:07 +0000 (20:07 +0200)]
pimd: don't leak original_s_route on error
original_s_route is allocated on the heap and was not freed during the
error case.
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Tue, 14 Jun 2016 18:07:06 +0000 (20:07 +0200)]
isisd: Fix size of malloc
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Tue, 14 Jun 2016 18:07:05 +0000 (20:07 +0200)]
isisd: fix an error that was probably a result of copypasting
The code should check for the existance of the correct list prior to
accessing it.
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Tue, 14 Jun 2016 18:07:01 +0000 (20:07 +0200)]
bgpd: fix memory leaks in show commands
sockunion_str2su allocates a struct sockunion that used to be leaked
in the show commands. Use str2sockunion and keep the information
on the stack instead.
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com> Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Christian Franke [Tue, 14 Jun 2016 18:07:04 +0000 (20:07 +0200)]
ospf6d: fix off-by-one on display of spf reasons
The loop should only iterate to array_size - 1.
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Tue, 14 Jun 2016 18:07:02 +0000 (20:07 +0200)]
ospfd: fix double assignment in ospf_vl_set_timers
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Tue, 14 Jun 2016 18:07:00 +0000 (20:07 +0200)]
bgpd: fix off-by-one in attribute flags handling
bgp_attr_flag_invalid can access beyond the last element of attr_flags_values.
Fix this by initializing attr_flags_values_max to the correct value.
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Tue, 14 Jun 2016 18:06:59 +0000 (20:06 +0200)]
bgpd: don't leak memory in community_regexp_include
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Christian Franke [Tue, 14 Jun 2016 18:06:56 +0000 (20:06 +0200)]
bgpd: setting nexthop doesn't need inet_pton
Signed-off-by: Christian Franke <chris@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org> Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
Daniel Walton [Thu, 20 Oct 2016 17:21:34 +0000 (17:21 +0000)]
bgpd: 'show ip bgp summary json' shows large negative value for "peerUptimeMsec"
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com> Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com> Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Ticket: CM-13239
Paul Jakma [Thu, 16 Jun 2016 15:03:11 +0000 (16:03 +0100)]
lib: keep hash of node's commands to detect duplicate installs
* command.h: (struct cmd_node) Add a hash, so duplicate installs of
a cmd_element to a command node can be detected. To help catch
strays from the VIEW/ENABLE node consolidation particularly
(installs to VIEW automatically install to ENABLE too now).
* command.c: (cmd_hash_{key,cmp}) helpers for the hash - just directly
on the pointer value is sufficient to catch the main problem.
(install_node) setup the hash for the command node.
(install_element) check for duplicate installs.
The assert on the cmd_parse_format seems misplaced.
(install_default_basic) separate the basic, VIEW, node default commands
to here.
(cmd_init) get rid of dupes, given consolidation.
(cmd_terminate) clean up the node command hash.
Not done: The (struct cmd_node)'s vector could be replaced with the
cmd hash, however much of the command parser depends heavily on the
vector and it's a lot of work to change. A vector_lookup_value could
also work, particularly if vector could be backed by a hash.
The duplicate check could be disabled in releases - but useful in
development. It's a little extra overhead at startup. The command
initialisation overhead is already something that bites in
micro-benchmarks - makes it easy for other implementations to show
how much faster they are with benchmarks where other load is low
enough that startup time is a factor.
projects / mirror_frr.git / log