David Lamparter [Fri, 21 Jun 2019 08:58:02 +0000 (10:58 +0200)]
lib: add frr_with_mutex() block-wrapper
frr_with_mutex(...) { ... } locks and automatically unlocks the listed
mutex(es) when the block is exited. This adds a bit of safety against
forgetting the unlock in error paths & co. and makes the code a slight
bit more readable.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
upon vrf disable, an event informs bfd daemon that the vrf contexts
should be removed. in the case a vrf backend is netns based, all sockets
opened under that netns have to be closed. otherwise it is impossible
for the system to completely close the network namespace. that implies
that some interfaces may not be deleted, and may not be given back to
default vrf.
PR=65291 Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com> Acked-by: Julien Floret <julien.floret@6wind.com>
Donald Sharp [Tue, 27 Aug 2019 11:45:02 +0000 (07:45 -0400)]
lib: Cleanup return codes to use enum values
A couple functions in routemap.c were returning
0/1 that were being mapped into the appropriate
enum values on the calling functions to check return
values. This matches the return values to the actual
enum for future readability.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
bgpd: Add Established and Dropped counts to JSON output of bgp summary
Based on a suggestion by Donald Sharp, this patch adds the counts of the
number of times a BGP peering session has transitioned from Estd->NotEstd
and from NotEstd->Estd to the JSON output only of the
"show [ip] bgp [vrf <vrf>] summary" command. The idea is that even if the
current session is well and up, but a sessions has trasnitionined in and
out of Estd state multiple times, its worth noting that. We cannot change
the non-JSON output as easily, and so this command only addresses the JSON
part for now. The fields added are the ones that were provided only as part
of the "show bgp neighbor" command.
Signed-off-by: Dinesh G Dutt <5016467+ddutt@users.noreply.github.com>
circuit deletion was being enforced by sending a fake IF_DOWN_FROM_Z
event for the circuit interface. This created a problem when the
circuit was enabled again, since isisd internal state machine was
expecting to see an IF_UP_FROM_Z that never came, as the interface
had not actually gone down.
As a consequence, disabling + re-enabling isis on an interface or
area would leave interfaces in a CONFIG state, and adjacencies were
not restored. Fix this by following the state machine and simply
disabling circuits rather than attempting to delete them forcefully.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Dinesh G Dutt [Sat, 31 Aug 2019 16:24:49 +0000 (16:24 +0000)]
bgpd: Add a new command to only show failed peerings
In a data center, having 32-128 peers is not uncommon. In such a situation, to find a
peer that has failed and why is several commands. This hinders both the automatability of
failure detection and the ease/speed with which the reason can be found. To simplify this
process of catching a failure and its cause quicker, this patch does the following:
1. Created a new function, bgp_show_failed_summary to display the
failed summary output for JSON and vty
2. Created a new function to display the reset code/subcode. This is now used in the
failed summary code and in the show neighbors code
3. Added a new variable failedPeers in all the JSON outputs, including the vanilla
"show bgp summary" family. This lists the failed session count.
4. Display peer, dropped count, estd count, uptime and the reason for failure as the
output of "show bgp summary failed" family of commands
5. Added three resset codes for the case where we're waiting for NHT, waiting for peer
IPv6 addr, waiting for VRF to init.
This also counts the case where only one peer has advertised an AFI/SAFI.
The new command has the optional keyword "failed" added to the classical summary command.
The changes affect only one existing output, that of "show [ip] bgp neighbors <nbr>". As
we track the lack of NHT resolution for a peer or the lack of knowing a peer IPv6 addr,
the output of that command will show a "waiting for NHT" etc. as the last reset reason.
This patch includes update to the documentation too.
Signed-off-by: Dinesh G Dutt <5016467+ddutt@users.noreply.github.com>
Donald Sharp [Fri, 30 Aug 2019 20:14:38 +0000 (16:14 -0400)]
ospfd: Cleanup oi->obuf to always be created
This looks like a finish up of the partial cleanup that
ocurred at some point in time in the past. When we
alloc oi also always alloc the oi->obuf. When we delete
oi always delete the oi->obuf right before.
This cleans up a bunch of code to be simpler and hopefully
easier to follow.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Fri, 30 Aug 2019 10:03:09 +0000 (06:03 -0400)]
ospfd: Do not turn on write thread unless we have something in it
I am rarely seeing this crash:
r2: ospfd crashed. Core file found - Backtrace follows:
[New LWP 32748]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/aarch64-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/lib/frr/ospfd'.
Program terminated with signal SIGABRT, Aborted.
2019-08-29 15:59:36,149 ERROR: assert failed at "test_ospf_sr_topo1/test_memory_leak":
Which translates to this code:
node = listhead(ospf->oi_write_q);
assert(node);
oi = listgetdata(node);
assert(oi);
So if we get into ospf_write without anything on the oi_write_q
we are stopping the program.
This is happening because in ospf_ls_upd_queue_send we are calling
ospf_write. Imagine that we have a interface already on the on_write_q
and then ospf_write handles the packet send for all functions. We
are not clearing the t_write thread and we are popping and causing
a crash.
Additionally modify OSPF_ISM_WRITE_ON(O) to not just blindly
turn on the t_write thread. Only do so if we have data.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
ospfd: Remove redundant asserts
assert(oi) is impossible all listgetdata(node) directly proceeding
it already asserts here, besides a node cannot be created
with a null pointer!
If list_isempty is called directly before the listhead call
it is impossilbe that we do not have a valid pointer here.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
David Lamparter [Wed, 17 Jul 2019 13:24:28 +0000 (15:24 +0200)]
bgpd: add timestamp to bgp_adj_in
If we reject a received update in a filter, it never turns into a
bgp_path_info but stays in adj_in. For that case, we don't have any
timestamp for the update.
Currently, this isn't visible anywhere; BMP will make use of this
timestamp (and we can add a CLI option if we want.)
Signed-off-by: David Lamparter <equinox@diac24.net>
Donald Sharp [Thu, 29 Aug 2019 13:33:47 +0000 (09:33 -0400)]
tests: Ensure topotests are actually waiting appropriately for peers to come up
The FRR bgp topotests are employing a luCommand that looks for bgp peering
to be up on the first router with a `wait` sub-command. Please note that
a variety of tests are using this. This wait command has a variety of time
outs being used `30`, `90`, and `300`. BGP peering with how we compile
it have very long timers and 30( and possibly 90) seconds is clearly not enough when
we are waiting, given the nature of our test beds. Additionally we were employing a model
where once the first summary command succeeded we automatically assumed that all
subsuquent summary commands( to look at other routers ) would not need to
possibly wait. This is insufficient in that if I have multiple peerings in
multiple vrf's there is no guarantee that one router peers being up will
be sufficient information to know that all the other routers peers are up.
Modify the test cases to be a bit more conformant about this and to
allow peer checks to actually wait a reasonable amount of time for
all peers to have a chance to come up.
Signed-off-by: Donald Sharp <sharpdc@cumulusnetworks.com>
Donald Sharp [Wed, 28 Aug 2019 19:50:23 +0000 (15:50 -0400)]
tests: Modify docker build for local to have dev build
Allow a local build of a frr docker container to be built with
`--enable-dev-build`. This allows better decodes of symbols
which could be useful when you are trying to fix something
that is broken inside the docker container.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 28 Aug 2019 16:09:41 +0000 (12:09 -0400)]
lib: Stop arm crash on shutdown
Arm platforms are crashing in our topotests with this callstack;
50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
[Current thread is 1 (Thread 0xffffabb591d0 (LWP 18947))]
(gdb) bt
file=file@entry=0xaaaadfed1e48 "lib/memory.c", line=line@entry=80,
function=function@entry=0xaaaadfed1db8 <__func__.10514> "mt_count_free") at lib/log.c:837
(gdb)
So we are crashing because we are attempting to free a mtype that has no allocations
associated with it.
I added this debug code:
@@ -227,7 +230,9 @@ static void rcu_bump(void)
struct rcu_next *rn;
rn = XMALLOC(MTYPE_RCU_NEXT, sizeof(*rn));
-
+ zlog_debug("RCU_BUMP");
+ mtype_dump(MTYPE_RCU_THREAD);
+ mtype_dump(MTYPE_RCU_NEXT);
/* note: each RCUA_NEXT item corresponds to exactly one seqno bump.
* This means we don't need to communicate which seqno is which
* RCUA_NEXT, since we really don't care.
and added a mtype_dump function:
+void mtype_dump(struct memtype *mt)
+{
+ zlog_debug("%s: %d", mt->name, (int)mt->n_alloc);
+}
I would have expected the output to be:
RCU_BUMP
RCU thread: 3
RCU sequence barrier: X
instead.
As a thought experiment I reduced the number of static memory types
to 1 in the file and the crash stopped happening.
I suspect we have a systematic error on arm in lib/memory.h
due to the asm code. I am going to leave that alone for the
moment ( and leave the crash issue open ), but see if we
can get this code change into the system so that our CI
system becomes happy again.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This is the code:
bgp_reads_off(peer);
bgp_writes_off(peer);
assert(!CHECK_FLAG(peer->thread_flags, PEER_THREAD_WRITES_ON));
assert(!CHECK_FLAG(peer->thread_flags, PEER_THREAD_READS_ON));
The line crashing is the first assert. We know in bgp_writes_off we unset this flag:
Donald Sharp [Tue, 27 Aug 2019 18:55:48 +0000 (14:55 -0400)]
*: Start process of possibly deprecating Solaris
The FRR community has run into an issue where keeping up our
CI system to work with solaris has become a fairly large burden.
We have also sent emails and asked around and have not found
anyone standing up saying that they are using Solaris.
Given the fact that we do not have any comprehensive testing
being done w/ solaris and the fact that we are getting a steady
stream of new features that will never work on solaris and
we cannot find anyone to say that they are using it. Let's
start the drawn out process of deprecating the code.
If in the mean-time someone comes forward with the fact that
they are using it we can then not deprecate it.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Dinesh G Dutt [Tue, 27 Aug 2019 01:48:53 +0000 (01:48 +0000)]
bgpd: Eliminate all incorrect formulations of afi/safi in JSON
In a number of places, the JSON output had invalid key names for
AFI/SAFI. For example, the key name in JSON was "IPv4 Unicast" which
is invalid as a JSON Key name. Many JSON tools such as those used in
Ansible, jq etc. all fail to parse the output in these scenarios. The
valid name is ipv4Unicast. There's already a routine afi_safi_json()
defined to handle this change, but it was not consistently called.
The non-JSON version was called afi_safi_print() and it merely returned
the CLI version of the string, didn't print anything.
This patch deals with this issue by:
- Renaming afi_safi_print to get_afi_safi_str()
- get_afi_safi_str takes an additional param, for_json which if true
will return the JSON-valid string
- Renaming afi_safi_json to get_afi_safi_json_str()
- Creating a new routine get_afi_safi_vty_str() for printing to vty
- Consistently using get_afi_safi_str() with the appropriate for_json
value
Signed-off-by: Dinesh G Dutt <5016467+ddutt@users.noreply.github.com>
Satheesh Kumar K [Tue, 27 Aug 2019 06:38:26 +0000 (23:38 -0700)]
pimd: sh config is showing detail debug, though not configured
Original Idea is to display normal & detailed debugs when detailed
debug alone is configured. because of this "sh debugs" are showing
wrong Information, because same macro is used to disply the configured
debugs.
that means even if Normal debug is configured, detailed macro returns
TRUE. To avoid this ambiguity check whetehr detailed debug is configured
or not during dumping configured debugs. In all other places using
old macro.
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
eva# show bgp ipv4 uni 4.4.4.4
BGP routing table entry for 4.4.4.4/32
Paths: (1 available, best #1, table default)
Advertised to non peer-group peers:
10.50.12.118
999 999 999 999
10.50.12.118 from 10.50.12.118 (10.50.12.118)
Origin incomplete, metric 0, valid, external, best (First path received)
Last update: Mon Aug 26 09:47:17 2019
eva# conf
eva(config)# route-map BLARBLE permit 10
eva(config-route-map)# no set as-path prepend last-as 3
eva(config-route-map)# end
eva# clear bgp ipv4 uni *
eva# show bgp ipv4 uni 4.4.4.4
BGP routing table entry for 4.4.4.4/32
Paths: (1 available, best #1, table default)
Advertised to non peer-group peers:
10.50.12.118
999
10.50.12.118 from 10.50.12.118 (10.50.12.118)
Origin incomplete, metric 0, valid, external, best (First path received)
Last update: Mon Aug 26 09:48:31 2019
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Dinesh G Dutt [Mon, 26 Aug 2019 12:38:28 +0000 (12:38 +0000)]
lib: Make if_lookup_by_index understand if VRF is backed by netns or not
FRR has two implementations of VRF, one backed by netns and the other by
the proper VRF implementation in the Linux kernel. In certain places, the
code assumes that a VRF is netns and so lookups fail. One example of this
is in IPv6 RA code. This causes functionality such as Unnumbered BGP to
fail. To fix this, this patch makes if_lookup_by_index handle the
behavior based on the backend, similar to if_get_by_index. For the two
places in if.c that were calling if_lookup_by_index to be specific to
the VRF, I renamed the existing code, if_lookup_by_ifindex and made it a
static function that is never exposed or called by any routine outside of
if.c.
Signed-off-by: Dinesh G Dutt <5016467+ddutt@users.noreply.github.com>
vdhingra [Mon, 12 Aug 2019 11:15:29 +0000 (04:15 -0700)]
bgpd : Ambiguous command: show bgp large-community-list 1
When user wants to dump individual large-community-list with the name
then bgp throws an error. It is due to command to dump the bgp RIB routes
having a particular large-community-list values. To segregate both the
commands this fix has added the detail keyword in the below command.
show bgp large-community-list <(1-500)|WORD> detail
The same code change is applicable for community-list also.
Donald Sharp [Fri, 23 Aug 2019 19:44:32 +0000 (15:44 -0400)]
bgpd: Allow shortened input for `show bgp l2vpn evpn route type e|p`
The `show bgp l2vpn evpn route type <es|prefix>` commands
only accepted 2 letters. You could not type `show bgp l2vpn evpn route type e`
or `show bgp l2vpn evpn route type p` although both are technically legal
since nothing overlaps with them.
Ticket: CM-25988 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
vdhingra [Fri, 23 Aug 2019 05:36:36 +0000 (22:36 -0700)]
zebra: Added a few more information in route_entry while dumping
re->nexthop_num and re->nexthop_active_num are calculated while rib
processing. Also It helps in encoding the ZAPI message.
It's good to dump these parameters also, when the system is in
abnormal state.
Renato Westphal [Thu, 22 Aug 2019 14:29:37 +0000 (11:29 -0300)]
isisd: add support for IPv6 BFD peers
Make isisd create BFD sessions over IPv6 when IS-IS is configured
for IPv6 operation only.
When IS-IS is enabled for both IPv4 and IPv6 on a given interface,
prefer creating a BFD session over IPv6 to avoid having two BFD
sessions protecting the same IS-IS adjacency.
Ashish Pant [Tue, 20 Aug 2019 04:21:59 +0000 (09:51 +0530)]
tests: Enhance execution logs in topojson
Signed-off-by: Ashish Pant <ashish12pant@gmail.com>
1. Add check if show running output is corrupted as frr-reload does
not return cause of failure, just the error codes.
2. Change logger level to debug for some extra information.
3. Modify logger messages for more clear information.
4. Print configuration commands to console instead of show running
5. Print show command output to console.
6. Move show running output within flag show_router_config.
7. Add retry decorator for retyring show commands.
Donald Sharp [Wed, 21 Aug 2019 15:44:48 +0000 (11:44 -0400)]
tests: Add a couple simple Null0 spelling tests
Add a bit of code to test different spelling of Null0 routes.
This was broken at some point in the past and with recent
changes is working again, but it would be nice to
know when this breaks again.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Renato Westphal [Tue, 6 Aug 2019 20:18:41 +0000 (17:18 -0300)]
isisd: remove unnecessary null checks
These null checks don't make sense because a) these two functions
are never called with a NULL IP address and b) the same pointers are
dereferenced later without any protection. Remove these NULL checks
to make the code less confusing.
Renato Westphal [Wed, 7 Aug 2019 00:30:08 +0000 (21:30 -0300)]
lib: don't ignore stream errors in some zapi decode functions
Some other ZAPI decode functions still use void return values and
can't propagate stream errors to their callers. They need to be fixed
as well in the future.
Don Slice [Mon, 12 Aug 2019 19:02:39 +0000 (19:02 +0000)]
bgpd: make clear bgp * clear all peers in all afi/safis
Problem reported that "clear bgp *" only cleared ipv6 peers.
Changed the logic to clear all afi/safis of all peers in
that case. Also improved the operation of clearing
individual afi/safi using soft/in/out to do the right thing.
Ticket: CM-25887 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Donald Sharp [Mon, 19 Aug 2019 14:25:40 +0000 (10:25 -0400)]
pbrd, sharpd, staticd: Add some color to sample configuration files
The sample configuration files for pbrd, sharpd and staticd
where all the same. Add some bit of color to help new people
get rolling on these three daemons.
Signed-off-by: Donald Sharp <sharpd@cumulusnetwork.com>
Donald Sharp [Mon, 19 Aug 2019 14:12:30 +0000 (10:12 -0400)]
*: Fix spelling errors pointed out by debian packaging
Debian packaging when run finds a bunch of spelling errors:
I: frr: spelling-error-in-binary usr/bin/vtysh occurences occurrences
I: frr: spelling-error-in-binary usr/lib/frr/bfdd Amount of times Number of times
I: frr: spelling-error-in-binary usr/lib/frr/bgpd occurences occurrences
I: frr: spelling-error-in-binary usr/lib/frr/bgpd recieved received
I: frr: spelling-error-in-binary usr/lib/frr/isisd betweeen between
I: frr: spelling-error-in-binary usr/lib/frr/ospf6d Infomation Information
I: frr: spelling-error-in-binary usr/lib/frr/ospfd missmatch mismatch
I: frr: spelling-error-in-binary usr/lib/frr/pimd bootsrap bootstrap
I: frr: spelling-error-in-binary usr/lib/frr/pimd Unknwon Unknown
I: frr: spelling-error-in-binary usr/lib/frr/zebra Requsted Requested
I: frr: spelling-error-in-binary usr/lib/frr/zebra uknown unknown
I: frr: spelling-error-in-binary usr/lib/x86_64-linux-gnu/frr/libfrr.so.0.0.0 overriden overridden
This commit fixes all of them except the bgp `recieved` issue due to
it being part of json output. That one will need to go through
a deprecation cycle.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
projects / mirror / frr.git / log