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.
Don Slice [Tue, 18 Oct 2016 17:18:49 +0000 (10:18 -0700)]
zebra: Move netlink error message under a debug
In some circumstances, the quagga log is being filled with repetitive
error messages reporting "network is down" with RTM_NEWROUTE. Moved this
particular scenario under "debug zebra kernel" instead of making it an
unprotected error message. Manually tested using the same script with and
without the fix to verify the message is suppressed.
Ticket: CM-11173 Signed-off-by: Don Slice Reviewed-by: Donald Sharp
David Lamparter [Wed, 12 Oct 2016 15:05:51 +0000 (17:05 +0200)]
vtysh: refactor vtysh_client_{config,execute}
Triggered by a bugreport / patch by Gautam Kumar <gauta@amazon.com>,
this is a full rewrite vtysh_client_{config,execute}. (The patch didn't
quite apply anymore.)
vtysh_client_run() now has a buffering implementation that can be read
without losing one's sanity and/or requiring alcoholic beverages.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd, lib: memory cleanups for valgrind, plus debug changes
Description:
We use valgrind memcheck quite a bit to spot leaks in
our work with bgpd. In order to eliminate false positives,
we added code in the exit path to release the remaining
allocated memory.
Bgpd startup log message now includes pid.
Some little tweaks by Paul Jakma <paul.jakma@hpe.com>:
* bgp_mplsvpn.c: (str2prefix_rd) do the cleanup in common code at the end
and goto it.
[DL: dropped several chunks from original commit which are obsolete by
now on this tree.]
Renato Westphal [Thu, 13 Oct 2016 16:06:10 +0000 (13:06 -0300)]
lib/zebra: remove code duplication in redist_del_instance()
Change redist_check_instance() to return a pointer instead of returning 1
on success. This way this function can be reused in redist_del_instance()
instead of duplicating the same logic there.
Also, remove unnecessary call to redist_check_instance() in
zebra_redistribute_delete().
While here, remove unnecessary cast from void* in redist_add_instance().
Renato Westphal [Thu, 6 Oct 2016 13:16:58 +0000 (10:16 -0300)]
ldpd: remove dead code from zsend_redistribute_route()
As a general rule of thumb, we should write functions that do one thing
and that do it well. All callers of zsend_redistribute_route() are already
checking if the route should be redistributed or not (as the comment
says), so we definitely shouldn't bother with that in this function.
Renato Westphal [Thu, 6 Oct 2016 12:45:27 +0000 (09:45 -0300)]
zebra: fix redistribution of default routes
We were always redistributing the default routes (IPv[46]) in
redistribute_update() because the 'client->redist_default' condition
always returns true.
The 'redist_default' member of the 'zserv' structure is a pointer and is
always initialized with vrf_bitmap_init() in the zebra_client_create()
function.
Renato Westphal [Wed, 5 Oct 2016 20:58:01 +0000 (17:58 -0300)]
zebra/ldpd: introduce ZEBRA_ROUTE_ALL wildcard route type
The ZEBRA_ROUTE_ALL route type can be used by a client to request
all routes from zebra. The main motivation for introducing this is
to allow ldpd to get routes from all OSPF instances, not only from
the default one. Without ZEBRA_ROUTE_ALL, ldpd would need to send a
ZEBRA_REDISTRIBUTE_ADD message for each possible OSPF instance (65k),
which doesn't scale very well.
Paul Jakma [Tue, 6 Sep 2016 16:23:48 +0000 (17:23 +0100)]
bgpd: bgp_nexthop_cache not deleted with peers
* Fix mild leak, bgp_nexthop_caches were not deleted when their peer was.
Not a huge one, but makes valgrinding for other leaks noisier.
Credit to Lou Berger <lberger@labn.net> for doing the hard work of
debugging and pinning down the leak, and supplying an initial fix.
That one didn't quite get the refcounting right, it seemed, hence
this version.
This version also keeps bncs pinned so long as the peer is defined, where
Lou's tried to delete whenever the peer went through bgp_stop. That causes
lots of zebra traffic if down peers go Active->Connect->Active, etc., so
leaving bnc's in place until peer_delete seemed better.
* bgp_nht.c: (bgp_unlink_nexthop_by_peer) similar to bgp_unlink_nexthop, but
by peer.
* bgp_nht.c: (bgp_unlink_nexthop_check) helper to consolidate checking
if a bnc should be deleted.
(bgp_unlink_nexthop_by_peer) ensure the bnc->nht_info peer reference
is removed, and hence allow bncs to be removed by previous.
* bgpd.c: (peer_delete) cleanup the peer's bnc.
Colin Petrie [Wed, 11 May 2016 09:56:58 +0000 (11:56 +0200)]
bgpd: fix MRT table dumps for locally-originated routes
I've been working on a small patch to correct an issue in the BGP MRT
table dump code. It's a quick'n'easy fix initially, and I'd appreciate
any feedback on making it better :)
Issue:
When the BGP table dump code runs, it generates the peer_index_table.
This walks the list of peers, and dumps out their IP, ASN, address
family, etc. It also sets the peer index number in the peer struct.
Then the code walks the RIB, and for each prefix, writes out RIB
entries, that refer to the peer index number.
However, when it finds prefixes that are locally originated, the
associated peer is the 'self' peer, which wasn't in the list of peers,
never gets an index number assigned, but because it is calloc'd, the
index number is set to 0.
End result: locally-originated routes are associated with whichever peer
happens to be first in the list of remote peers in the index table :)
Example (from one of our route collectors) - these are two of our
originated prefixes (bgpdump output):
TABLE_DUMP2|1457568002|B|12.0.1.63|7018|84.205.80.0/24||IGP|193.0.4.28|0|0||NAG|64512
10.255.255.255|
TABLE_DUMP2|1457568006|B|12.0.1.63|7018|2001:7fb:ff00::/48||IGP|::|0|0||NAG||
The prefixes are announced by us (note it has an empty AS PATH (the
field after the prefix)) but also looks like it was received from AS7018
(12.0.1.63). In fact, the AS7018 peer just happens to be the first peer
in the index table.
Fix:
The simplest fix (which is also the method adopted by both OpenBGPd and
the BIRD mrtdump branch) is to create an empty placeholder 'peer' at the
start of the peer index table, for all the routes which are locally
originated to refer to.
I've attached a patch for this.
Here's a resulting bgpdump output after the patch:
TABLE_DUMP2|1458828539|B|0.0.0.0|0|93.175.150.0/24||IGP|0.0.0.0|0|0||NAG||
Now it is more obvious that the prefix is locally originated.
There are more complicated potential ways of fixing it
1) skip the local routes when dumping the RIB. This leads to questions
about what an MRT table dump *should* contain :)
2) include the 'self' peer in the list of peers used to generate the
index table.
etc etc.
But I'm quite happy with my 'create a fake peer, and associate local
routes with it' method :)
Your thoughts and feedback are welcome!
Regards,
Colin Petrie
Systems Engineer
RIPE NCC RIS Project Tested-by: NetDEF CI System <cisystem@netdef.org>
Timo Teräs [Fri, 19 Feb 2016 02:19:54 +0000 (18:19 -0800)]
zebra: Fix route deletion on *BSD
Fix for not handling RTM_CHANGE correctly. This patch change it to
delete/add instead. Using RTM_CHANGE on kernels where it works is better,
but is left as an exercise for developer who has access and will to fix it
on *BSD.
Renato Westphal [Wed, 12 Oct 2016 15:39:02 +0000 (12:39 -0300)]
lib/vtysh: fix duplicate installation of some vty commands
This is a followup to commits 735e62 and 0b1442, where we forgot to apply
the same VIEW/ENABLE consolidation logic to vtysh. Also, we can't call
install_default() for the ENABLE node because some of the vty commands
installed by this function were already installed in the VIEW node before.
Don Slice [Tue, 11 Oct 2016 15:08:37 +0000 (11:08 -0400)]
bgpd: protect "could not get instance" error message with debug
If bgp gets inbound connect messages on an interface associated with
a vrf, but the vrf is not defined yet in bgp, the log is filled with
continual error messages. This change moves that error message to a
debug under "debug bgp neighbor-events". Manual testing results
applied to the ticket.
Ticket: CM-10394 Signed-off-by: Don Slice Reviewed-by: Donald Sharp
Donald Sharp [Tue, 11 Oct 2016 12:47:32 +0000 (08:47 -0400)]
bgp: Fix bi->extra->tag if statement
bi->extra->tag is a 3 byte array, the statement
as written will always be true. Modify code
to see if we actually have any data in the
tag and then print the label.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com> Reviewed-by: Lou Berger <lberger@labn.net>
these files do not belong in the git tree. (They're temporaries during
a ./configure run and normally removed at the end; let's add them to
.gitignore anyway.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
'neighbor x.x.x.x weight' was implemented as a per-peer knob instead of
a per-peer per-afi-safi option. This makes it configurable per-peer
per-afi-safi so that we can do things like soft clear that afi/safi when
weight is modified.
Donald Sharp [Wed, 5 Oct 2016 20:11:12 +0000 (20:11 +0000)]
bgpd: Reduce json memory usage.
When running 'show bgp ipv4 uni summ' (or any variation thereof)
If you have a large # of routes, the json package starts taking
up a tremendous amount of memory and processing power.
Modify the code to output the json as we go instead of gathering
it all up and outputting at the end.
Ticket: CM-13060 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com> Reviewed-by: Don Slice <dslice@cumulusnetworks.com> Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>
Donald Sharp [Fri, 11 Mar 2016 19:27:13 +0000 (14:27 -0500)]
*: Consolidate all double VIEW_NODE and ENABLE_NODE's
If a command is put into the VIEW_NODE, it is going into the
ENABLE_NODE as well. This is especially true for show commands.
As such if a command is in both consolidate it down to VIEW_NODE.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Fri, 11 Mar 2016 19:27:12 +0000 (14:27 -0500)]
lib: Consolidate VIEW_NODE to be ENABLE_NODE as well
If you are in VIEW_NODE, the command should exist in ENABLE_NODE
as well. This is being done to reduce chances of code being
added to one but not the other NODE.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Fri, 11 Mar 2016 19:27:11 +0000 (14:27 -0500)]
bgpd, lib: Remove RESTRICTED_NODE from code base
The RESTRICTED_NODE command is not used, introduces code
complexity and provides no additional levels of security.
The only way to get into RESTRICTED_NODE is to add, under
vty configuration the command 'anonymous restricted', and
then telnet to a daemon, provide a password, then type
'enable' and fail to enter the password three times.
Then the user can enter a very limited set of commands to
monitor bgp and only bgp behavior.
This commit removes both the RESTRICTED_NODE usage as well
as the lib/* usage of the code
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This patch improves zebra,ripd,ripngd,ospfd and bgpd so that they can
make use of 32-bit route tags in the case of zebra,ospf,bgp or 16-bit
route-tags in the case of ripd,ripngd.
David Lamparter [Fri, 30 Sep 2016 13:38:03 +0000 (15:38 +0200)]
zebra: use qobj and enable concurrent config edits
This puts all the proper VTY_DECLVAR_CONTEXT calls in place, removing
all vty->index uses in the process. With that, vty_config_lockless can
be enabled in zebra.
[v2: fix ordering screw-up in "interface XXX" command]
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ldpd was keeping track of the vty session's position in config editing
with 3 global static variables. This worked because only one vty could
be in configuration-editing mode before.
Replace with vty->qobj_index infrastructure and enable
vty_config_lockless.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Renato Westphal [Wed, 22 Jun 2016 12:59:28 +0000 (09:59 -0300)]
ldpd: add in-process API for creating/deleting
These functions are currently unused but will be used by the Cap'n Proto
interface. They're not a particular burden to maintain in-tree, so here
they go.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Renato Westphal [Wed, 22 Jun 2016 12:59:28 +0000 (09:59 -0300)]
ldpd: merge/dup/reload void **ref support
Extend configuration duplication-merge mechanism to allow keeping track
of a single specific object. A "void **" pointer is passed in; the
"void *" pointer it points to is updated with the new address of the
object it points to.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Finally, this disables the config editing lock for isisd. It also
enables deprecation warnings for the lib/ and isisd/ to catch accidental
uses of vty->index.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
projects / mirror / frr.git / log