This patch just introduces the callback mechanism for the
resilient nexthop changes so that upper level daemons
can take advantage of the change. This does nothing
at this point but just call some code.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Firstly, *keep no change* for `hash_get()` with NULL
`alloc_func`.
Only focus on cases with non-NULL `alloc_func` of
`hash_get()`.
Since `hash_get()` with non-NULL `alloc_func` parameter
shall not fail, just ignore the returned value of it.
The returned value must not be NULL.
So in this case, remove the unnecessary checking NULL
or not for the returned value and add `void` in front
of it.
Importantly, also *keep no change* for the two cases with
non-NULL `alloc_func` -
1) Use `assert(<returned_data> == <searching_data>)` to
ensure it is a created node, not a found node.
Refer to `isis_vertex_queue_insert()` of isisd, there
are many examples of this case in isid.
2) Use `<returned_data> != <searching_data>` to judge it
is a found node, then free <searching_data>.
Refer to `aspath_intern()` of bgpd, there are many
examples of this case in bgpd.
Here, <returned_data> is the returned value from `hash_get()`,
and <searching_data> is the data, which is to be put into
hash table.
Signed-off-by: anlan_cs <vic.lan@pica8.com>
When adding a nhg to a route map, make sure to specify the `family`
of the rm by looking at the contents of the nhg. Installation in the
kernel (for DSCP rules in particular) relies on this being specified in
the netlink message.
Signed-off-by: Wesley Coakley <wcoakley@nvidia.com>
Signed-off-by: Stephen Worley <sworley@nvidia.com>
When handling the addition of a nhg which would saturate the range of
tables PBR is configured to install into, handle this *before* a
possible call to pbr_nhgc_alloc during hash query
Signed-off-by: Wesley Coakley <wcoakley@nvidia.com>
Represent installed ("allocated") NHG tables with a hash keyed by table
ID. Replaces a pre-allocated array of booleans that implemented this
functionality before.
+ PBR table range > 65535 is fixed (was OOB access before :))
+ Pre-compute next available ID to save time when only checking
if all tables are allocated
Signed-off-by: Wesley Coakley <wcoakley@nvidia.com>
Back when I put this together in 2015, ISO C11 was still reasonably new
and we couldn't require it just yet. Without ISO C11, there is no
"good" way (only bad hacks) to require a semicolon after a macro that
ends with a function definition. And if you added one anyway, you'd get
"spurious semicolon" warnings on some compilers...
With C11, `_Static_assert()` at the end of a macro will make it so that
the semicolon is properly required, consumed, and not warned about.
Consistently requiring semicolons after "file-level" macros matches
Linux kernel coding style and helps some editors against mis-syntax'ing
these macros.
Signed-off-by: David Lamparter <equinox@diac24.net>
Neither tabs nor newlines are acceptable in syslog messages. They also
break line-based parsing of file logs.
Signed-off-by: David Lamparter <equinox@diac24.net>
Only handle an interface update in the nexthop tracking code
if the nexthop in question was set with an interface to point
out of. If the nexthop is GW only, the interface update could
be unrelated but have overlapping address space. Let that be
handled elsewhere.
Ex)
```
5.5.5.0/30 dev dummyDoof proto kernel scope link src 5.5.5.1
5.5.5.0/24 dev goofDummy proto kernel scope link src 5.5.5.1
[root@alfred frr-2]# ip ro show table 10000
default via 5.5.5.2 dev dummyDoof proto pbr metric 20
[root@alfred frr-2]# ip link set goofDummy down
[root@alfred frr-2]# ip ro show table 10000
[root@alfred frr-2]# ip link set goofDummy up
[root@alfred frr-2]# ip ro show table 10000
```
Signed-off-by: Stephen Worley <sworley@nvidia.com>
I am not even sure what the goal of this code was in any
way shape fashion or form. But since it's pbr_nht.c
I as the original author should know... But I don't.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
the pnhi data structure can receive either a interface or a
nhr data structure. Ensure that we don't crash.
CID -> 1500586
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When an interface goes up/down we need to pay attention to this
in PBR. In the past we were relying *only* on the nht events
but this is not sufficient for cases where an interface is flapping
up and down. If this is happening it could be happening fast enough
that zebra is not sending nht events because they are consolidated
into a single event from it's perspective and that is the right thing
to do. This commit will allow us to back out commit:
0aaa722883
As that commit introduced extra processing in zebra that is actually
causing issues in other places. The problem that commit was trying
to solve should have always been handled in pbrd instead of making
zebra do work that is unnatural to it's actual flow.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Given a received nexthop update, only send down an update to the
relevant nexthop group. Avoid sending down superfluous updates
Signed-off-by: Wesley Coakley <wcoakley@nvidia.com>
The pnhc->nexthop was a pointer copy. Causing issues
with the ability to move pointers around for the
different pnhc since the pnhc mirrored the nexthop
caches. When we received a vrf change if we shared
pointers it was impossible to know if we had
already updated the code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We had multiple pnhc cache entries with the same nexthop
pointer. This causes some large amount of confusion.
Fixup the code to handle this situation better.
Ticket: CM-31044
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
PBR needs the ability to allow ephermeal interfaces( bonds,
vrfs, dummy, bridges, etc ) to be destroyed and then
recreated and at the same time keep track of them and
rebuild state as appropriate when we get a change.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Currently when a vrf is deleted than added back in PBR was
not going through and touching up all the data structures
that needed to be massaged to allow it to start working again.
This includes:
a) Search through the nexthop groups to find any nexthop
that references the old nexthop id and set it right again.
b) Search through the nexthop cache for nht and reset
those nexthops to the right vrf as well as re-register
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Cleanup the marking of a nhc as installed/removed based on table
route installation.
We were not even handling the removal state at all.
We saw some timing issues with the routes being installed/removed
multiple times and then never resending the pbr map due to bad states
on the nhgc.
Dont worry about checking if its already marked installed before
scheduling the policy walk. We have a check in `pbr_send_map()`
to ensure we dont try to resend a map sequence already installed.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Withdraw route from RIB if we detect the NHG is not valid
anymore. We were seeing an issue where we would leave a route
in zebra's RIB if it was recursive despite it being removed
from the kernel due to an interface going down.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Revise new `show pbr` keys to be consistent with existing
json in other daemons
target->nexthop
id->tableId (where relevant)
isValid->valid
isInstalled->installed
Signed-off-by: Wesley Coakley <wcoakley@cumulusnetworks.com>
The new json output for the `show pbr` directives return arrays instead
of associative arrays, which are more meaningful in this context
Signed-off-by: Wesley Coakley <wcoakley@cumulusnetworks.com>
Increased the verbosity of the json keys and flattened the returned
structure by removing superfluous keys.
Signed-off-by: Wesley Coakley <wcoakley@cumulusnetworks.com>
Implement the ability to replace any existing `set *` or
`match` with another one or adding more config without having
to first delete the original config already there.
Before, we needed to constantly execute a `no` command for everything
to remove the rule before making changes to it. With this
patch, you can replace configs on individual sequences much
easier.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Actually delete the allocated pbr_nhg_cache object we just
released.
Found via memory leak:
==3078405== 136 bytes in 1 blocks are definitely lost in loss record 8,282 of 8,802
==3078405== at 0x483BB1A: calloc (vg_replace_malloc.c:762)
==3078405== by 0x48E35E8: qcalloc (memory.c:110)
==3078405== by 0x40EBA7: pbr_nhgc_alloc (pbr_nht.c:194)
==3078405== by 0x48CC0EB: hash_get (hash.c:148)
==3078405== by 0x40F825: pbr_nht_add_individual_nexthop (pbr_nht.c:534)
==3078405== by 0x409853: pbr_map_nexthop_magic (pbr_vty.c:400)
==3078405== by 0x4093F1: pbr_map_nexthop (pbr_vty_clippy.c:417)
==3078405== by 0x48ACF72: cmd_execute_command_real (command.c:1073)
==3078405== by 0x48ACB3B: cmd_execute_command (command.c:1133)
==3078405== by 0x48AD063: cmd_execute (command.c:1288)
==3078405== by 0x493D8EE: vty_command (vty.c:526)
==3078405== by 0x493D397: vty_execute (vty.c:1293)
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
It's been a year search and destroy.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
`set vrf NAME` allows the pbr map to point to an arbitrary vrf table.
`set vrf unchanged` will use the interface's vrf for table lookup.
Further, add functionality for pbr to respond to interface events
such as interface vrf changes & interface creation/deletion.
Ex)
ubuntu_nh# show pbr map
pbr-map TEST valid: 1
Seq: 1 rule: 300 Installed: 3(1) Reason: Valid
SRC Match: 3.3.3.3/32
VRF Unchanged (use interface vrf)
pbr-map TEST2 valid: 1
Seq: 2 rule: 301 Installed: 3(2) Reason: Valid
SRC Match: 4.4.4.4/32
VRF Lookup: vrf-red
root@ubuntu_nh:/home# ip rule show
0: from all lookup local
300: from 3.3.3.3 iif dummy2 lookup main
300: from 3.3.3.3 iif dummyVRF lookup 1111
301: from 4.4.4.4 iif dummy1 lookup 1111
301: from 4.4.4.4 iif dummy3 lookup 1111
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com-
Reduce the api for deleting nexthops and the containing
group to just one call rather than having a special case
and handling it separately.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
The pbr->nhg callback is used exclusively for individual nexthops
set through `set nexthop`. If an actuall "tracked" nexthop_group is
used, only the `pbrms->nhgrp_name` is set. Thus this delete does
nothing.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Don't bother tracking ipv6 link locals to determine if a map
should be installed. Every interface has a route of `fe80::/64`
so its just going to return the arbitrarily first one it finds
when it resolves it and hands it back to us.
Instead, just track the interface we specify along with it.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
The code should be looking for both v4 and v6 nexthop types
instead of v4 nexthop types 2 times.
Found by Coverity SA
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
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>
The creation of a nexthop group results in a callback with
just the nexthop group name. At this point in time we
do not have any nexthop information so there is nothing to
install.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Add a file that exposes functions which modify nexthop groups.
Nexthop groups are techincally immutable but there are a
few special cases where we need direct access to add/remove
nexthops after the group has been made. This file provides a
way to expose those functions in a way that makes it clear
this is a private/hidden api.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
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>
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>
It doesn't make much sense for a hash function to modify its argument,
so const the hash input.
BGP does it in a couple places, those cast away the const. Not great but
not any worse than it was.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
