Stephen Worley <sworley@cumulusnetworks.com>: Author Summary

Builds triggered by Stephen Worley <sworley@cumulusnetworks.com>

Builds triggered by an author are those builds which contains changes committed by the author.
228
140 (61%)
88 (39%)

Breakages and fixes

Broken means the build has failed but the previous build was successful.
Fixed means that the build was successful but the previous build has failed.
56 (25% of all builds triggered)
11 (5% of all builds triggered)
-45
Build Completed Code commits Tests
FRR › FRR › #2943 2 weeks ago
doc: indicate non-support for dynamic pbr maps
Indicate in the doc clearly that we do not support the dynamic
creation of pbr maps for sub-interfaces when the master has
an interface. Such as in vlans. It must be explicitly added.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
8278 passed
FRR › FRR › #2932 2 weeks ago
zebra: add null check before connecting recursive depend
Add a null check in `handle_recursive_depend()` so it
doesn't try to add a NULL pointer to the RB tree.

This was found with clang SA.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
zebra: reset nexthop pointer in zread of nexthops
We were not resetting the nexthop pointer to NULL for each
new read of a nexthop from the zapi route. On the chance we
get a nexthop that does not have a proper type, we will not
create a new nexthop and update that pointer, thus it still
has the last valid one and will create a group with two
pointers to the same nexthop.

Then when it enters any code that iterates the group, it loops
endlessly.

This was found with zapi fuzzing.

```
0x00007f728891f1c3 in jhash2 (k=<optimized out>, length=<optimized out>, initval=12183506) at lib/jhash.c:138
0x00007f728896d92c in nexthop_hash (nexthop=<optimized out>) at lib/nexthop.c:563
0x00007f7288979ece in nexthop_group_hash (nhg=<optimized out>) at lib/nexthop_group.c:394
0x0000000000621036 in zebra_nhg_hash_key (arg=<optimized out>) at zebra/zebra_nhg.c:356
0x00007f72888ec0e1 in hash_get (hash=<optimized out>, data=0x7ffffb94aef0, alloc_func=0x0) at lib/hash.c:138
0x00007f72888ee118 in hash_lookup (hash=0x7f7288de2f10, data=0x7f728908e7fc) at lib/hash.c:183
0x0000000000626613 in zebra_nhg_find (nhe=0x7ffffb94b080, id=0, nhg=0x6020000032d0, nhg_depends=0x0, vrf_id=<optimized out>,
    afi=<optimized out>, type=<optimized out>) at zebra/zebra_nhg.c:541
0x0000000000625f39 in zebra_nhg_rib_find (id=0, nhg=<optimized out>, rt_afi=AFI_IP) at zebra/zebra_nhg.c:1126
0x000000000065f953 in rib_add_multipath (afi=AFI_IP, safi=<optimized out>, p=0x7ffffb94b370, src_p=0x0, re=0x6070000013d0,
    ng=0x7f728908e7fc) at zebra/zebra_rib.c:2616
0x0000000000768f90 in zread_route_add (client=0x61f000000080, hdr=<optimized out>, msg=<optimized out>, zvrf=<optimized out>)
    at zebra/zapi_msg.c:1596
0x000000000077c135 in zserv_handle_commands (client=<optimized out>, msg=0x61b000000780) at zebra/zapi_msg.c:2636
0x0000000000575e1f in main (argc=<optimized out>, argv=<optimized out>) at zebra/main.c:309
```

```
(gdb) p *nhg->nexthop
$4 = {next = 0x5488e0, prev = 0x5488e0, vrf_id = 16843009, ifindex = 16843009, type = NEXTHOP_TYPE_IFINDEX, flags = 8 '\b', {gate = {ipv4 = {s_addr = 0},
      ipv6 = {__in6_u = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}},
    bh_type = BLACKHOLE_UNSPEC}, src = {ipv4 = {s_addr = 0}, ipv6 = {__in6_u = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 0, 0, 0, 0, 0,
          0}, __u6_addr32 = {0, 0, 0, 0}}}}, rmap_src = {ipv4 = {s_addr = 0}, ipv6 = {__in6_u = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0,
          0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}}, resolved = 0x0, rparent = 0x0, nh_label_type = ZEBRA_LSP_NONE, nh_label = 0x0, weight = 1 '\001'}
(gdb) quit

```

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
zebra: don't created connected if duplicate depend
Since we are using a UNIQUE RB tree, we need to handle the
case of adding in a duplicate entry into it.

The list API code returns NULL when a successfull add
occurs, so lets pull that handling further up into
the connected handlers. Then, free the allocated
connected struct if it is a duplicate.

This is a pretty unlikely situation to happen.

Also, pull up the RB handling of _del RB API as well.

This was found with the zapi fuzzing code.

```
==1052840==
==1052840== 200 bytes in 5 blocks are definitely lost in loss record 545 of 663
==1052840==    at 0x483BB1A: calloc (vg_replace_malloc.c:762)
==1052840==    by 0x48E1008: qcalloc (memory.c:110)
==1052840==    by 0x44D357: nhg_connected_new (zebra_nhg.c:73)
==1052840==    by 0x44D300: nhg_connected_tree_add_nhe (zebra_nhg.c:123)
==1052840==    by 0x44FBDC: depends_add (zebra_nhg.c:1077)
==1052840==    by 0x44FD62: depends_find_add (zebra_nhg.c:1090)
==1052840==    by 0x44E46D: zebra_nhg_find (zebra_nhg.c:567)
==1052840==    by 0x44E1FE: zebra_nhg_rib_find (zebra_nhg.c:1126)
==1052840==    by 0x45AD3D: rib_add_multipath (zebra_rib.c:2616)
==1052840==    by 0x4977DC: zread_route_add (zapi_msg.c:1596)
==1052840==    by 0x49ABB9: zserv_handle_commands (zapi_msg.c:2636)
==1052840==    by 0x428B11: main (main.c:309)
```

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
8284 passed
FRR › FRR › #2907 1 month ago
tests: add basic nexthop group functionality test
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>
8268 passed
You have insufficient permissions to see all of the builds.
Build Completed Code commits Tests
FRR › FRR › #2901 1 month ago
lib,zebra: tighten up the nexthop_copy/nexthop_dup APIs
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>
zebra: can't improve efficiency for recursive depends
cb86eba3ab3d82f540bdb9ed5f65d361ca301ea8 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>
7193 passed
FRR › FRR › #2537 5 months ago
zebra: Don't resolve to diff interface if given
If the nexthop is of type `GATEWAY_IFINDEX` then the nexthop
should not resolve to a nexthop that has a different ifindex
from the one given.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
1 of 8933 failed
FRR › FRR › #2395 7 months ago
zebra: Add a conditional guard if zvrf lookup fail
Add a conditional to guard against segfaulting on the debug
statement when zvrf lookup fails.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
1 of 8820 failed
FRR › FRR › #2331 7 months ago
doc: Add firewalld notice in fedora
Add a note in the fedora build guide on how to disable
firewalld and clear iptables that come by default.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
7139 passed
FRR › FRR › #2298 8 months ago
doc: Add systemd-devel to fedora build instruction
systemd-devel is a neccessary package for building frr, added to
the install list.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
1 of 8229 failed
You have insufficient permissions to see all of the builds.
Build Completed Code commits Tests
FRR › FRR › #2900 1 month ago
zebra: just set nexthop member in handle_recursive_depend()
With recent changes to the lib nexthop_group
APIs (e1f3a8eb193267da195088cc515b598ae5a92a12), 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>
lib: assert if someone adds to nexthop list to nhg
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>
8265 passed
FRR › FRR › #2618 4 months ago
lib: Decode vrf_id update appropriately from zapi
The vrf_id in `zsend_interface_vrf_update()` is encoded as
a long via `stream_putl()`, we should decode it as such
as well.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
8898 passed
FRR › FRR › #2560 5 months ago
pbrd: Handle GATEWAY_IFINDEX nht conflicts
In pbrd we did not care if the nexthop interface nexthop tracking
sent us back did not match the one specified with `nexthop [GATEWAY]
[INTERFACE]`. This happened if the gateway was resolvable via a
different interface and the inteface we specified in the config was
unnumbered (no ipv4 address on it) since the default route gets forced
onlink when it gets into zebra.

This patch adds a check to not install the rule if the interface we got
back was different from the specified.

This patch also reworks the nexthop update path to make it a little more
clear what its doing.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
8790 passed
FRR › FRR › #2495 5 months ago
zebra: Append rparent labels when resolving nexthop
When resolving a nexthop, append its labels to the one its
resolving to along with the labels that may already be present there.

Before we were ignoring labels if the resolving level was greater than
two.

Before:
```
S>  2.2.2.2/32 [1/0] via 7.7.7.7 (recursive), label 2222, 00:00:07
  *                    via 7.7.7.7, dummy1 onlink, label 1111, 00:00:07
S>  3.3.3.3/32 [1/0] via 2.2.2.2 (recursive), label 3333, 00:00:04
  *                    via 7.7.7.7, dummy1 onlink, label 1111, 00:00:04
K>* 7.7.7.7/32 [0/0] is directly connected, dummy1, label 1111, 00:00:17
C>* 192.168.122.0/24 is directly connected, ens3, 00:00:17
K>* 192.168.122.1/32 [0/100] is directly connected, ens3, 00:00:17
ubuntu_nh#
```

This patch:
```
S>  2.2.2.2/32 [1/0] via 7.7.7.7 (recursive), label 2222, 00:00:04
  *                    via 7.7.7.7, dummy1 onlink, label 1111/2222, 00:00:04
S>  3.3.3.3/32 [1/0] via 2.2.2.2 (recursive), label 3333, 00:00:02
  *                    via 7.7.7.7, dummy1 onlink, label 1111/2222/3333, 00:00:02
K>* 7.7.7.7/32 [0/0] is directly connected, dummy1, label 1111, 00:00:11
C>* 192.168.122.0/24 is directly connected, ens3, 00:00:11
K>* 192.168.122.1/32 [0/100] is directly connected, ens3, 00:00:11
ubuntu_nh#
```

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
8816 passed
FRR › FRR › #2288 8 months ago
pbrd: Set next/prev to NULL on cached nexthops
When we copy a new nexthop to cache and track, set its
next and prev pointers to NULL. We don't want those since
this is to be treated as a single nexthop.Other nexthops that
aren't in a group could hash to this nexthop so it doesn't
make sense to keep those pointers in the cache.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
zebra,pbrd: Update pbrd to handle NHT properly
Update pbrd to properly handle nexthop tracking.

When we get a notification that a change happened on a nexthop,
re-install it if its still valid.

Before, we were running over all routes and re-queueing them if they
were PBR routes. This commit removes that and puts all the processing
in PBR instead.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
8179 passed
FRR › FRR › #2239 8 months ago
lib,zebra,bgpd: Remove nexthop_same_no_recurse()
The functions nexthop_same() does not check the resolved
nexthops so I don't think this function is even needed
anymore.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
lib: Add labels_cmp to nexthop_cmp
Add labels comparison to nexthop_cmp() as well.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
lib,zebra,bgpd,pbrd: Compare nexthops without labels
Allow label ignoring when comparing nexthops. Specifically,
add another functon nexthop_same_no_labels() that shares
a path with nexthop_same() but doesn't check labels.

rib_delete() needs to ignore labels in this case.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
lib: Explicitly cmp gateway and source based on AF
Refactor the gatway and source nexthop comparision into a
common code path that compares them explicitly based on
their address family.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
lib: Make labels_match function use labels_cmp
Update the nexthop_labels_match() function to use
nexthop_labels_cmp().

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
lib: nexthop-cmp return if gateways don't match
Fix the cmp check the so that it returns the result
if the gateways don't match.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
lib: Add nexthop labels cmp functions
Add a function to compare nexthop labels.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
lib: Update nexthop_same to use nexthop_cmp
Simplify the code a bit by making nexthop_same()
just use nexthop_cmp() internally.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
7766 passed
FRR › FRR › #1987 11 months ago
zebra:        Remove unused sockaddr variable
This variable does nothing, removing it.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
6583 passed
FRR › FRR › #1378 1 year ago
zebra: Check for netlink message next fault
NLMSG_NEXT decrements the buffer length (status) by
the header msg length (nlmsg_len) everytime its called.
If nlmsg_len isn't accurate and set to be larger than
what it should represent, it will cause status to
decrement passed 0. This makes NLMSG_NEXT return a
pointer that references an inaccessible address.
When that is passed to NLMSG_OK, it segfaults.

Add a check to verify that there is still something to read
before we try to.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
7200 passed
You have insufficient permissions to see all of the builds.