David Lamparter [Mon, 28 Nov 2016 17:40:25 +0000 (18:40 +0100)]
tests: update testcli reference output
These are expected changes in CLI behaviour. There's still a diff of
unexpected changes that need fixing (or are missing features, e.g.
keyword arguments).
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Renato Westphal [Fri, 18 Nov 2016 17:39:25 +0000 (15:39 -0200)]
ripngd: fix drop of multicast membership when the interface is down
When an interface is shut down, ripng_multicast_leave() is called after
ifp->flags is updated in ripng_interface_down(). So we shouldn't check
if the interface is up in order to proceed with the membership drop.
For consistency's sake, don't check for if_is_up() in
ripng_multicast_join() as well. In this case, this function is only
called when the interface is up, so the check was unnecessary.
Renato Westphal [Thu, 17 Nov 2016 18:33:09 +0000 (16:33 -0200)]
ripngd: implement the "clear ipv6 ripng" vty command
This command deletes all received routes from the RIPng routing table. It
should be used with caution as it can create black holes in the network
(until it reconverges). Very useful to make automated testing (e.g. ANVL)
more predictable.
Renato Westphal [Wed, 16 Nov 2016 18:14:45 +0000 (16:14 -0200)]
ripngd: implement optional heuristic suggested by RFC 2080
RFC 2080 - Section 2.4.2:
"If the new metric is the same as the old one, examine the timeout for the
existing route. If it is at least halfway to the expiration point, switch
to the new route. This heuristic is optional, but highly recommended".
Implement this optional heuristic only when ECMP is disabled globally ("no
allow-ecmp"), otherwise all routes with the same metric should be used.
Renato Westphal [Sat, 12 Nov 2016 21:34:37 +0000 (19:34 -0200)]
ripd: minor code simplification
* Simplify the RIP_TIMER_OFF macro and use it on more places;
* Be more explicit when creating the RIP UDP socket - cosmetic change
since socket(AF_INET,SOCK_DGRAM,0) defaults to UDP on every known
UNIX/Linux platform.
Renato Westphal [Sat, 12 Nov 2016 21:11:13 +0000 (19:11 -0200)]
ripd: make use of the IP_MULTICAST_LOOP sockoption
We still need to check for self-generated packets on rip_read() because
ripd may also send broadcast packets. But using IP_MULTICAST_LOOP on the
ripd socket will at least prevent us from receiving a lot unnecessary
multicast packets when RIPv2 is being used, thus improving performance.
Renato Westphal [Sat, 12 Nov 2016 20:39:51 +0000 (18:39 -0200)]
pimd/zebra: fix setting of IP_MULTICAST_LOOP on OpenBSD
Linux, FreeBSD and NetBSD (and possibly others too) accept both uint8_t
and int for the IP_MULTICAST_LOOP sockoption. OpenBSD, in the other hand,
accepts only uint8_t. To make setting IP_MULTICAST_LOOP work on every
supported platform, always pass a uint8_t variable as a parameter.
Renato Westphal [Fri, 11 Nov 2016 22:19:13 +0000 (20:19 -0200)]
ripd: fix race condition on input processing
In the early days of ripd, we supported running RIP on secondary IP
addresses. To do that, everytime we needed to send a multicast packet,
we would create a new temporary socket for each of the interface's
addresses and call bind() to change the source IP of the outgoing packets.
The problem with these temporary sockets is that they are more specific
than the global RIP socket (bound to INADDR_ANY). Then, even though these
sockets only exist for a short amount of time, they can receive some RIP
packets that were supposed to be received on the global RIP socket. And
since we never read from the temporary sockets, these packets are dropped.
Since we don't support secondary addresses anymore, the simplest way to
fix this problem is to stop using temporary sockets for sending multicast
packets. We are already setting IP_MULTICAST_IF before sending each
multicast packet, and in this case the primary address of the selected
interface is used as the source IP of the outgoing packets, which is
exactly what we want.
If we decide to reintroduce support for secondary addresses in the future,
we should try one of the following:
* Use IP_SENDSRCADDR/IP_PKTINFO to set the source address of the outgoing
multicast packets;
* Create one permanent UDP socket for each possible interface address,
and enable reading on all sockets.
Fixes the following IxANVL RIP tests: 7.10 and 14.1.
Renato Westphal [Thu, 10 Nov 2016 17:35:47 +0000 (15:35 -0200)]
ripd: fix the "neighbor" command.
We can't use if_lookup_prefix() in rip_update_process() because this
function uses prefix_cmp() internally to try matching an interface
address to a static neighbor's address.
Since prefix_cmp() tries to match exact prefixes, if_lookup_prefix()
was always returning NULL.
What we really need here is to use prefix_match(), which checks if
one prefix includes the other (e.g. one /24 interface including a /32
static neighbor's address). The fix then is to replace the call to
if_lookup_prefix() and use if_lookup_address() instead, which uses
prefix_match() internally.
Renato Westphal [Thu, 10 Nov 2016 15:15:43 +0000 (13:15 -0200)]
ripd: add "none" option to the "ip rip receive version" command
RFC 2453 says (section 5.1):
"(...) For completeness, routers should also implement a receive control
switch which would determine whether to accept, RIP-1 only, RIP-2 only,
both, or none. It should also be configurable on a per-interface basis".
For the "ip rip send version" command, we don't need to implement the
"none" option because there's already the "passive-interface" command
for that.
Renato Westphal [Thu, 10 Nov 2016 14:55:09 +0000 (12:55 -0200)]
ripd: implement the "ip rip v2-broadcast" CLI command
This command allows ripd to send v2 updates as broadcast packets instead
of multicast packets. Useful as a technique to help with RIPv1/v2
interop issues.
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 [Fri, 11 Nov 2016 06:23:02 +0000 (07:23 +0100)]
vtysh: fix some macro breakage
vtysh's extract.pl script doesn't cope with install_element(var, ) where
"var" is not one of the FOO_NODE constants. Also, the future defun_lex
tool doesn't deal well with preprocessor expansions in the same regard.
This tries simplifying out some of these. lib/distribute.c needs
further thinking.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Sat, 19 Nov 2016 09:42:52 +0000 (10:42 +0100)]
vtysh: initialise readline before binding key
when libreadline initialises itself, it also reads .inputrc, which may
result in keybindings being set up. This means the builtin help on the
'?' key can be overridden by that. Consistent availability of '?'
behaviour trumps .inputrc setup here, so let's initialise readline
first before binding that key (which means our keybinding wins).
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Quentin Young [Tue, 15 Nov 2016 22:15:18 +0000 (22:15 +0000)]
lib: Fix nondeterministic command matches in rare cases
When a user erroneously defines two commands which can
match the same input and at least one of the tokens defined
last in the command is a selector or option, the matcher
does not detect an ambiguous match and matches the command
installed first (leftmost in the graph).
Fix is to do a full walkthrough of the follow set when
matching the final token in a command to check that there
is exactly one possible match, and to throw an ambiguity
error otherwise.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
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
Quentin Young [Sat, 12 Nov 2016 01:06:32 +0000 (01:06 +0000)]
lib, vtysh: Fix memory leaks, change cmd_element to const
Fix a few memory issues:
* Not freeing tab-completions upon input match failure
* Invalid write when null-terminating tab-completions
* Not freeing argv[] itself in additinon to elements
* Use XFREE() instead of free() as appropriate
* Not freeing final token of an [option] during parsing
Make a few minor changes to CLI internals:
* Improve documentation on matching & completion functions
* Only make one copy of cmd_token's when building argv,
instead of three
* Don't make a copy of the matching cmd_element
Make one major(ish) change to CLI internals:
* Change all pointers to struct cmd_element to const
Code outside of the core CLI units should never have an
occasion to modify the internal state of the command system.
Doing so could easily amount to having a CLI interface that
changes during runtime, and could conceivably lead to security
issues. Explicitly disallowing this removes any chance of
confusion.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Quentin Young [Thu, 10 Nov 2016 23:17:07 +0000 (23:17 +0000)]
lib: Implement hidden and deprecated commands
Each token now knows whether it is part of a hidden
or deprecated command. Command completion logic hides
such tokens when generating completions. Command
matching logic works as before and will still match on
hidden and deprecated commands.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
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>