Signed-off-by: Ashish Pant <ashish12pant@gmail.com>
Fix static route test in bgp-basic-functionality-topo1 and
example-topojson-test after lib changes
Add example-topojson-test to ignore from pytest.ini
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>
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>
The built docker container was installing an old version of libyang.
Updated it to point to the most recent build for Debian.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
This implements BMP. There's no fine-grained history here, the non-BMP
preparations are already split out from here so all that remains is BMP
proper.
Signed-off-by: David Lamparter <equinox@diac24.net>
This is the initial BMP skeleton from Yasuhiro Ohara.
(License/Signoff note: code published on github as GPLv2+.)
Signed-off-by: David Lamparter <equinox@diac24.net>
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>
This - mostly intended for BMP - implements a pull-driven write buffer
filled on demand by a callback with some reasonable buffering logic.
I don't expect it to be that useful in other places, but it's not BMP
specific so it's properly split off in its own place.
Signed-off-by: David Lamparter <equinox@diac24.net>
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>
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>
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);
+}
Which resulted in this output:
2019/08/28 15:41:11 BGP: RCU_BUMP
2019/08/28 15:41:11 BGP: RCU thread: 3
2019/08/28 15:41:11 BGP: RCU thread: 3
If we look at the defintion of the two static memory types:
DEFINE_MTYPE_STATIC(LIB, RCU_THREAD, "RCU thread")
DEFINE_MTYPE_STATIC(LIB, RCU_NEXT, "RCU sequence barrier")
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>
We have this crash:
2019-08-18T07:58:44.831656-04:00 rch2-140-fwK2b bgpd[1791]: %NOTIFICATION: sent to neighbor 10.73.248.8 4/0 (Hold Timer Expired) 0 bytes
2019-08-18T07:58:44.832164-04:00 rch2-140-fwK2b bgpd[1791]: Assertion `!((peer->thread_flags) & ((1 << 0)))' failed in file bgpd.c, line 2173, function peer_delete
2019-08-18T07:58:44.832548-04:00 rch2-140-fwK2b bgpd[1791]: Backtrace for 11 stack frames:
2019-08-18T07:58:44.832942-04:00 rch2-140-fwK2b bgpd[1791]: [bt 0] /usr/lib/libfrr.so.0(zlog_backtrace+0x3a) [0x7f5503c7c31a]
2019-08-18T07:58:44.833311-04:00 rch2-140-fwK2b bgpd[1791]: [bt 1] /usr/lib/libfrr.so.0(_zlog_assert_failed+0x61) [0x7f5503c7c891]
2019-08-18T07:58:44.833684-04:00 rch2-140-fwK2b bgpd[1791]: [bt 2] /usr/lib/frr/bgpd(peer_delete+0x4d5) [0x1432ceea15]
2019-08-18T07:58:44.834095-04:00 rch2-140-fwK2b bgpd[1791]: [bt 3] /usr/lib/frr/bgpd(+0x430e9) [0x1432cfc0e9]
2019-08-18T07:58:44.834479-04:00 rch2-140-fwK2b bgpd[1791]: [bt 4] /usr/lib/frr/bgpd(bgp_event_update+0x121) [0x1432cfe1c1]
2019-08-18T07:58:44.834852-04:00 rch2-140-fwK2b bgpd[1791]: [bt 5] /usr/lib/frr/bgpd(+0x453f1) [0x1432cfe3f1]
2019-08-18T07:58:44.835388-04:00 rch2-140-fwK2b bgpd[1791]: [bt 6] /usr/lib/libfrr.so.0(thread_call+0x60) [0x7f5503c9e3c0]
2019-08-18T07:58:44.835829-04:00 rch2-140-fwK2b bgpd[1791]: [bt 7] /usr/lib/libfrr.so.0(frr_run+0xb8) [0x7f5503c79de8]
2019-08-18T07:58:44.836292-04:00 rch2-140-fwK2b bgpd[1791]: [bt 8] /usr/lib/frr/bgpd(main+0x229) [0x1432ce4a69]
2019-08-18T07:58:44.836729-04:00 rch2-140-fwK2b bgpd[1791]: [bt 9] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5) [0x7f550271bb45]
2019-08-18T07:58:44.837198-04:00 rch2-140-fwK2b bgpd[1791]: [bt 10] /usr/lib/frr/bgpd(+0x2cefc) [0x1432ce5efc]
2019-08-18T07:58:44.837670-04:00 rch2-140-fwK2b bgpd[1791]: Current thread function (bgp_holdtime_timer), scheduled from file bgp_fsm.c, line 380
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:
void bgp_writes_off(struct peer *peer)
{
struct frr_pthread *fpt = bgp_pth_io;
assert(fpt->running);
thread_cancel_async(fpt->master, &peer->t_write, NULL);
THREAD_OFF(peer->t_generate_updgrp_packets);
UNSET_FLAG(peer->thread_flags, PEER_THREAD_WRITES_ON);
}
We also know that the keepalives are not being turned off until we call
bgp_fsm_change_status(peer, Deleted);
later in the function. We know that the keepalive pthread will
write to individual peers and issue a bgp_write_on(), which sets
this flag.
Modify the code base so that we explicitly turn off the keepalives
immediately before the turning of writes off.
Ticket: CM-26119
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
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>
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>
The `set as-path prepend last-as X` command had no, 'no' form
of the command. Add this into the cli.
Testing:
!
route-map BLARBLE permit 10
set as-path prepend last-as 3
!
!
router bgp 9999
neighbor 10.50.12.118 remote-as external
neighbor 10.50.12.118 ebgp-multihop 30
!
address-family ipv4 unicast
neighbor 10.50.12.118 route-map BLARBLE in
!
!
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>
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>
