Add an additional router to the basic pim tests.
1) This test will add a link between r1 and a new rp
2) This test will ensure that r1 and rp have the expected
who is the rp.
3) This test will ensure that the rp has received the upstream
data for the multicast stream that is started.
Ostensibly commit
68a02e06e5 is the first bad commit
commit 68a02e06e5
Author: Mark Stapp <mjs@voltanet.io>
Date: Wed Nov 13 16:06:06 2019 -0500
*: revise zapi nexthop encoding
Use a per-nexthop flag to indicate the presence of labels; add
some utility zapi encode/decode apis for nexthops; use the zapi
apis more consistently.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Sparked this commit in that it broke nexthop reporting to upper
level protocols. Ensure that this expectation stays working
in the future.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Commit
68a02e06e5 broke nexthop encoding
for nexthop tracking.
This code combined the different types of nexthop encoding
being done in the zapi protocol. What was missed that
resolved nexthops of type NEXTHOP_TYPE_IPV4|6 have an ifindex
value that was not being reported. This commit ensures
that we always send this data( even if it is 0).
The following test commit will ensure that this stays working
as is expected by an upper level protocol.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This script was written back when `git describe` would abbreviate to
7-char commit IDs; they're longer now and we're grabbing the tail
end...
Signed-off-by: David Lamparter <equinox@diac24.net>
The "test" program uses =, not ==. A lot of shells accept == as an
extension, but not all do and it's technically out of spec.
Signed-off-by: David Lamparter <equinox@diac24.net>
Like gcov but better!
Also has a driveby fix for --enable-dev-build being mutually exclusive
with --enable-gcov...
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Duplicated domain name capability messages cause memory leak. The amount
of leaked memory is proportional to the size of the duplicated
capabilities. This bug was introduced in 2015.
To hit this, a BGP OPEN message must contain multiple FQDN capabilities.
Memory is leaked when the hostname portion of the capability is of
length 0, but the domainname portion is not, for any of the duplicated
capabilities beyond the first one.
https://tools.ietf.org/html/draft-walton-bgp-hostname-capability-00
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Problem seen when deleting many static routes or access-lists due
to frr-reload.py issuing individual vtysh -c commands for every
line. On slow switches, this can take long enough for systemd to
time out the reload process and restart frr. This fix uses add
logic for static routes, prefix-lists, and access-lists to gang
the changes together.
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Ticket: CM-27856
Add a very basic nexthop group functionality test.
This test creates a 2-way ecmp group and installs a route
with it using sharpd. Then we check to see that the nexthop
groups are marked valid/installed in zebra.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
My previous patch to fix a memory leak, caused by not properly freeing
the iptable iface list on stream parse failure, created/exposed a heap
use after free because we were not doing a deep copy
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
If someone tries to add a nexthop with a list of nexthops
already attached to it, let's just assert. This standardizes
the API to say we assume this is an individual nexthop
you are appending to a group.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
With recent changes to the lib nexthop_group
APIs (e1f3a8eb19), we are making
new assumptions that this should be adding a single nexthop
to a group, not a list of nexthops.
This broke the case of a recursive nexthop resolving to a group:
```
D> 2.2.2.1/32 [150/0] via 1.1.1.1 (recursive), 00:00:09
* via 1.1.1.1, dummy1 onlink, 00:00:09
via 1.1.1.2 (recursive), 00:00:09
* via 1.1.1.2, dummy2 onlink, 00:00:09
D> 3.3.3.1/32 [150/0] via 2.2.2.1 (recursive), 00:00:04
* via 1.1.1.1, dummy1 onlink, 00:00:04
K * 10.0.0.0/8 [0/1] via 172.27.227.148, tun0, 00:00:21
```
This group can instead just directly point to the nh that was passed.
Its only being used for a lookup (the memory gets copied and used
elsewhere if the nexthop is not found).
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Make the nexthop_copy/nexthop_dup APIs more consistent by
adding a secondary, non-recursive, version of them. Before,
it was inconsistent whether the APIs were expected to copy
recursive info or not. Make it clear now that the default is
recursive info is copied unless the _no_recurse() version is
called. These APIs are not heavily used so it is fine to
change them for now.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
cb86eba3ab was causing zebra to crash
when handling a nexthop group that had a nexthop which was recursively resolved.
Steps to recreate:
!
nexthop-group red
nexthop 1.1.1.1
nexthop 1.1.1.2
!
sharp install routes 8.8.8.1 nexthop-group red 1
=========================================
==11898== Invalid write of size 8
==11898== at 0x48E53B4: _nexthop_add_sorted (nexthop_group.c:254)
==11898== by 0x48E5336: nexthop_group_add_sorted (nexthop_group.c:296)
==11898== by 0x453593: handle_recursive_depend (zebra_nhg.c:481)
==11898== by 0x451CA8: zebra_nhg_find (zebra_nhg.c:572)
==11898== by 0x4530FB: zebra_nhg_find_nexthop (zebra_nhg.c:597)
==11898== by 0x4536B4: depends_find (zebra_nhg.c:1065)
==11898== by 0x453526: depends_find_add (zebra_nhg.c:1087)
==11898== by 0x451C4D: zebra_nhg_find (zebra_nhg.c:567)
==11898== by 0x4519DE: zebra_nhg_rib_find (zebra_nhg.c:1126)
==11898== by 0x452268: nexthop_active_update (zebra_nhg.c:1729)
==11898== by 0x461517: rib_process (zebra_rib.c:1049)
==11898== by 0x4610C8: process_subq_route (zebra_rib.c:1967)
==11898== Address 0x0 is not stack'd, malloc'd or (recently) free'd
Zebra crashes because we weren't handling the case of the depend nexthop
being recursive.
For this case, we cannot make the function more efficient. A nexthop
could resolve to a group of any size, thus we need allocs/frees.
To solve this and retain the goal of the original patch, we separate out the
two cases so it will still be more efficient if the nexthop is not recursive.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
When withdrawing addpaths, adj_lookup was called to find the path that
needed to be withdrawn. It would lookup in the RB tree based on subgroup
pointer alone, often find the path with the wrong addpath ID, and return
null. Only the path highest in the tree sent to the subgroup could be
found, thus withdrawn.
Adding the addpath ID to the sort criteria for the RB tree allows us to
simplify the logic for adj_lookup, and address this problem. We are able
to remove the logic around non-addpath subgroups because the addpath ID
is consistently 0 for non-addpath adj_outs, so special logic to skip
matching the addpath ID isn't required. (As a side note, addpath will
also never use ID 0, so there won't be any ambiguity when looking at the
structure content.)
Signed-off-by: Mitchell Skiba <mskiba@amazon.com>
