Coverity points a copy-paste error in the Red-Black tree implementation. The
RB tree code is based on the OpenBSD implementation, so at first glance, it
is a strong point for thinking twice before touching anything.
Details:
The code is an augmented RB tree implementation [1], which adds to RB trees
the possibility of using a callback on every node update for updating per-node
associated metainformation. The bug is clear once checking other places where
the callback is called.
Impact:
- FRR: no impact, because the "augmented" capability is not being used.
- OpenBSD [2]: it seems there is no impact, at least in the 'src' repository.
Additional observations:
- If the "augmented" capability is not used, the code could run faster (at
every operation on a node the callback is checked for not being NULL). May
be branch prediction could be enough for those extra operations being
negligible on most processors in use.
Christian Franke [Fri, 28 Sep 2018 17:32:38 +0000 (19:32 +0200)]
doc: Use `mv -f` in Makefile
Sphinx always runs, even in the `make install` stage. When `make install`
is run as root and then another `make` is run by a nonprivileged user,
some versions of `mv` prompt like this:
Don Slice [Fri, 28 Sep 2018 15:55:39 +0000 (15:55 +0000)]
bgpd: solve issue entering aggregate twice
Problem reported that frr-relaod.py was not installing an aggregate
properly. Problem was actually that frr-reload.py does the command
twice, and the second time the aggregate command was entered, it would
appear in the config but the aggregate was removed from the bgp table
and not advertised to peers. Solved by noticing when an aggregate
was marked for deletion (info_invalid) and allowing the re-entry if
the old one was being removed.
Ticket: CM-22509 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Don Slice [Thu, 27 Sep 2018 16:51:59 +0000 (16:51 +0000)]
bgpd: enable aggregation in evpn
Problem encountered where using the aggregate-address command in an
evpn environment did not work properly. Depending on the order of
actions, the aggregate may not be created or removed when either the
commands were issued or routes come and go.
Ticket: CM-20585 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Donald Sharp [Tue, 7 Nov 2017 14:14:32 +0000 (09:14 -0500)]
bgpd: Add lua match command
Please note this is a Proof of Concept and not actually something
that is ready to commit at this point. The file tools/lua.scr
contains some documentation on how we expect it to work currently.
Additionally not all bgp values have been hooked up into the
ability to lua script yet.
There is still significant work to be done here:
1) Add the ability to pass in more data and to adjust the return values
as appropriate.
To set it up:
1) copy tools/lua.scr into /etc/frr (or whereever the config
directory is )
2) Create a route-map match command:
!
router bgp 55
neighbor 10.50.11.116 remote-as external
!
address-family ipv4 unicast
neighbor 10.50.11.116 route-map TEST in
exit-address-family
!
route-map TEST permit 10
match command mooey
!
3) In the lua.scr file make sure that you have a function
named 'mooey' ( as the above example does ):
In bgp if we have not configured bgp we were ignoring
interface based callbacks. Leading to states where
we may not be processing interface information.
Leading to states where we do not actually keep
ifp data. As an example:
Suppose vrf A and vrf B. A has interface swp1.
At the same time we only have a `router bgp 9 vrf B`
When we received the callback for moving swp1
from vrf A to vrf B we were not processing the
move at all and BGP would not consider the interface
part of vrf B at all.
This commit makes bgp pay attention to interface
events irrelevant if bgp is using that vrf. This
is now consistent with how the lib/if* expects
to work and the rest of the daemons in FRR.
Signed-off-by: Donald Sharp <sharpd@cumulsnetworks.com>
Donald Sharp [Thu, 20 Sep 2018 15:31:14 +0000 (11:31 -0400)]
watchfrr: Modify some stderr messages to zlog_warn
The stderr output is not being displayed as part of watchfrr invocation
in system startup. Specifically if the user has not properly sent
1 or more daemons to monitor. If the end-user is using tools/frr
this stderr is dropped( and systemd appears to drop stderr too? )
Modify the two stderr calls in this situation and use the zlog system.
Now I can clearly see an error message that tells me what has gone wrong.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
[DL: fixed typo]
Conditional code in netlink_macfdb_update() introduced in 2232a77c used
the 'dst_present' variable because not all cases were covered. Now it is
not necessary.
Issue 1: if the router ospf current configuration is "area 0.0.0.2
range 1.0.0.0/24 cost 23" and user try to configure "area 0.0.0.2
range 1.0.0.0/24 not-advertise", the existing o/p is "area 0.0.0.2
range 1.0.0.0/24 cost 23 not-advertise". The keywords "not-advertise"
& "cost" are multually exclusive, so they should not come together.
The vice versa way configuration is working fine.
Fix: When ospf area range "not-advertise", the cost should be initialized
to OSPF_AREA_RANGE_COST_UNSPEC.
Issue 2: if the router ospf current configuration "area 0.0.0.2 range
1.0.0.0/24 substitute 2.0.0.0/24" and user try to configure "area 0.0.0.2
range 1.0.0.0/24 not-advertise" the existing o/p is "area 0.0.0.2 range
1.0.0.0/24 not-advertise substitute 2.0.0.0/24". The keywords
"not-advertise" & "substiture" are multually exclusive, so they should
not come together. The vice versa way configuration is working fine.
Fix: When ospf area range "not-advertise" is configured,
ospf_area_range_substitute_unset() should be get called.
Issue 3: if the router ospf6 current configuration is "area 0.0.0.2
range 2001::/64 cost 23" and user try to configure "area 0.0.0.2 range
2001::/64 advertise", the existing o/p is area 0.0.0.2 range 2001::/64.
The keyword "cost 23" disappears.
Fix: When ospf area range "advertise" is configured and the range is not
NULL, the cost should not be modified.
ospfd: remove unnecessary housekeeping code when using linked lists
The head and tail pointers of linked lists should never be modified
manually, the linked list API guarantees that these pointers are always
valid and up-to-date.
Donald Sharp [Mon, 24 Sep 2018 19:12:36 +0000 (15:12 -0400)]
pimd: Fix several address sanitizer issues
This commit fixes two issues during pim shutdown.
1) The rp_info structure was being freed before the
outgoing notifications that depended on it's information
was sent out as part of shutdown.
2) The pim->upstream_list shutdown involved iterating
over the list via ALL_LIST_ELEMENTS. This typically
is enough but pim will auto delete child nodes as well
as itself when it goes away and they depend on it. As such
the node and nnode could possibly already have been freed.
So change the way we look at all the data in the upstream_list
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Mon, 24 Sep 2018 00:41:49 +0000 (20:41 -0400)]
eigrpd: Fix memory leaks and remove dead/unused functions
During shutdown we were not properly cleaning up some memory
as reported by valgrind. Additionally during cleanup operations
I noticed that there were some dead/unused functions remove/reduce.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 19 Sep 2018 12:20:37 +0000 (08:20 -0400)]
bgpd: Abstract bgp_nexthop_cache retrieving/setting from info pointer
The bgp_nexthop_cache data is stored as a void pointer in `struct bgp_node`.
Abstract retrieval of this data and setting of this data
into functions so that in the future we can move around
what is stored in bgp_node.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Mon, 30 Jul 2018 14:46:00 +0000 (10:46 -0400)]
bgpd: Abstract bgp_connected_ref retrieving/setting from info pointer
The bgp_connected_ref data is stored as a void pointer in `struct bgp_node`.
Abstract retrieval of this data and setting of this data
into functions so that in the future we can move around
what is stored in bgp_node.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Mon, 30 Jul 2018 14:30:41 +0000 (10:30 -0400)]
bgpd: Abstract bgp_static retrieving/setting from info pointer
The bgp_static data is stored as a void pointer in `struct bgp_node`.
Abstract retrieval of this data and setting of this data
into functions so that in the future we can move around
what is stored in bgp_node.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Mon, 30 Jul 2018 14:29:28 +0000 (10:29 -0400)]
bgpd: Abstract distance retrieving/setting from info pointer
The bgp_distance data is stored as a void pointer in `struct bgp_node`.
Abstract retrieval of this data and setting of this data
into functions so that in the future we can move around
what is stored in bgp_node.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Mon, 30 Jul 2018 12:50:47 +0000 (08:50 -0400)]
bgpd: Abstract aggregate retrieving/setting from info pointer
The aggregate data is stored as a void pointer in `struct bgp_node`.
Abstract retrieval of this data and setting of this data
into functions so that in the future we can move around
what is stored in bgp_node.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Sat, 22 Sep 2018 16:43:32 +0000 (12:43 -0400)]
eigrpd: Fix eigrp crash on shut of a interface
The eigrp interface structure was storing a pointer to the
connected interface structure and on shutdown of an interface
this would cause zebra to call eigrp back with a shutdown of
that interface, as part of that operation the connected interface
structure is being deleted, but eigrp was keeping a pointer to
the connected structure. At the same time we were keeping the address
of the connected structure and this is all we need, so keep a copy
of that data and use that instead.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Fri, 21 Sep 2018 20:02:34 +0000 (16:02 -0400)]
bgpd: Replace refcount with list of ifp names
Track the refcount a bit differently as that it is possible
to get into situations where we have multiple calls for the
same ifc. So let's just keep a list of the ifc's off of
each `struct bgp_addr` and then keep the hash entry based
upon list count or not.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Fri, 21 Sep 2018 19:50:47 +0000 (15:50 -0400)]
bgpd: Make `struct bgp_addr` a private data structure
The `struct bgp_addr` is not needed for anything other than
the address hash. Isolate this data structure so that it
is not polluting up the name space.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Fri, 21 Sep 2018 15:41:46 +0000 (11:41 -0400)]
pimd: blackhole traffic when we are not DR for Stream received
Suppose we have a bridge with a host and two routers attached
to it.
r1 r2
| |
--------
|
host
host is sending traffic.
r1 and r2 are pim neighbors and r2 is the DR.
Both r1 and r2 will receive data from the stream up the pim
kernel socket. r1 will notice that it is not the DR and
stop processing in pim. This code adds a bit more code to blackhole
the route when r1 detects it is not the DR in this scenario.
This is being done because the kernel is both keeping state and
sending data to the pim process to continue processing this.
Additionally if we happen to be running this on a asic, then
blackholing the route in the asic can save a significant amount
of cpu time handling this situation.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Fri, 21 Sep 2018 14:28:57 +0000 (10:28 -0400)]
pimd: Add a debug to tell why we are not creating upsteam state
When we decide we are not the right pim process to add upstream state
for the igmp state received, notice this in a debug to make life
easier to debug.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
lib: Trigger callbacks to re-evaluate route-map clauses when "call" CLI is executed.
When the "call" CLI is executed from with-in a route-map that is already in use,
there is a need to get the route-map clients to re-evalute the clauses defined
by both the parent route-map, as well as the child route-map.
The existing callbacks, add_hook() and delete_hook() can be used by the lib to
inform the clients when the "call" is configured and unconfigured.
There were checks for null pointer after being dereferenced. Checks have
been removed (we've discussed the no need of adding assert()'s because
of similar code not requiring them).
Mark Stapp [Wed, 12 Sep 2018 18:59:57 +0000 (14:59 -0400)]
zebra: Start abstraction of zebra_dplane_info for context passing
Reduce or eliminate use of global zebra_ns structs in
a couple of netlink/kernel code paths, so that those paths
can potentially be made asynch eventually.
Signed-off-by: Mark Stapp <mjs@voltanet.io> Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
projects / matthieu / frr.git / log