Customer has this valgrind trace:
Direct leak of 2829120 byte(s) in 70728 object(s) allocated from:
0 in community_new ../bgpd/bgp_community.c:39
1 in community_uniq_sort ../bgpd/bgp_community.c:170
2 in route_set_community ../bgpd/bgp_routemap.c:2342
3 in route_map_apply_ext ../lib/routemap.c:2673
4 in subgroup_announce_check ../bgpd/bgp_route.c:2367
5 in subgroup_process_announce_selected ../bgpd/bgp_route.c:2914
6 in group_announce_route_walkcb ../bgpd/bgp_updgrp_adv.c:199
7 in hash_walk ../lib/hash.c:285
8 in update_group_af_walk ../bgpd/bgp_updgrp.c:2061
9 in group_announce_route ../bgpd/bgp_updgrp_adv.c:1059
10 in bgp_process_main_one ../bgpd/bgp_route.c:3221
11 in bgp_process_wq ../bgpd/bgp_route.c:3221
12 in work_queue_run ../lib/workqueue.c:282
The above leak detected by valgrind was from a screenshot so I copied it
by hand. Any mistakes in line numbers are purely from my transcription.
Additionally this is against a slightly modified 8.5.1 version of FRR.
Code inspection of 8.5.1 -vs- latest master shows the same problem
exists. Code should be able to be followed from there to here.
What is happening:
There is a route-map being applied that modifes the outgoing community
to a peer. This is saved in the attr copy created in
subgroup_process_announce_selected. This community pointer is not
interned. So the community->refcount is still 0. Normally when
a prefix is announced, the attr and the prefix are placed on a
adjency out structure where the attribute is interned. This will
cause the community to be saved in the community hash list as well.
In a non-normal operation when the decision to send is aborted after
the route-map application, the attribute is just dropped and the
pointer to the community is just dropped too, leading to situations
where the memory is leaked. The usage of bgp suppress-fib would
would be a case where the community is caused to be leaked.
Additionally the previous commit where an unsuppress-map is used
to modify the outgoing attribute but since unsuppress-map was
not considered part of outgoing policy the attribute would be dropped as
well. This pointer drop also extends to any dynamically allocated
memory saved by the attribute pointer that was not interned yet as well.
So let's modify the return case where the decision is made to
not send the prefix to the peer to always just flush the attribute
to ensure memory is not leaked.
Fixes: #15459
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Currently in subgroup_default_originate the attr.aspath
is set in bgp_attr_default_set, which hashs the aspath
and creates a refcount for it. If this is a withdraw
the subgroup_announce_check and bgp_adj_out_set_subgroup
is called which will intern the attribute. This will
cause the the attr.aspath to be set to a new value
finally at the bottom of the function it intentionally
uninterns the aspath which is not the one that was
created for this function. This reduces the other
aspath's refcount by 1 and if a clear bgp * is issued
fast enough the aspath for that will be removed
and the system will crash.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The capability should be untouched, and send 0 (unlimited) instead.
Otherwise, we miss the capability and things are broken later until the
session reset.
Fixes: 72f0e06824 ("bgpd: Implement Paths-Limit capability")
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
It's very annoying when testing and instead of looking for true/false, you
have to check if the field exists.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
A route and its nexthop might belong to different VRFs. Therefore, we need
both the bgp and bgp_nexthop pointers.
Fixes: 8d51fafdcb ("bgpd: Drop bgp_static_update_safi() function")
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
I noticed that ospf6d always had a linked list memory leak.
Tracking it down shows that frr_fini() shuts down the memory
system and prints out memory not cleaned up. eigrpd, mgmtd
and ospf6d all called cleanup functions after frr_fini leaving
memory leaked that was not really leaked.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Recent commit: 6b2554b94a
Exposed, via Address Sanitation, that memory was being
leaked. Unfortunately the CI system did not catch this.
Two pieces of memory were being lost: The zserv client
data structure as well as anything on the client->gr_info_queue.
Clean these up.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Before this patch after command
'no neighbor <A.B.C.D|X:X::X:X|WORD> bfd profile [BFDPROF]'
has always been created bfd-config for neighbor.
Signed-off-by: Dmitrii Turlupov <turlupov@zts.ru>
When actions and notification are defined as descendants of other nodes,
they are not getting initialized, because the iterator skips them. Fix
the iterator to include them when traversing the schema.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
The new `%w99d` fixed-width modifier still needs work in `frr-format`,
which unfortunately is not as trivial as `%b` was.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This adds specific width length modifiers in the form of wN and wfN
(where N is 8, 16, 32, or 64) which allow printing intN_t and
int_fastN_t without resorting to casts or PRI macros.
Reviewed by: imp, emaste
Differential Revision: https://reviews.freebsd.org/D41725
FRR changes only include printf(), scanf/strtol are not locally
implemented in FRR. Also added "(void) 0" to empty "else ..." to
avoid a compiler warning.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from FreeBSD commit bce0bef3c6abab92c7ac8cc23b7cc632a382721e)
This way, full functionality of `zlog_*` is available.
Having `fatal()` be wrappers around `assertf()` also means we get
backtraces, which is not the case for a plain `exit(1)`.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This is primarily intended for ldpd with its split-process architecture.
The LDE/LDPE subprocesses currently lose the extended zlog
functionality.
The zlog_live target already encapsulates all necessary bits for vtysh.
Reuse it for a relay function to be used in the main ldpd process.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
When using dynamic capabilities, do not forget to unset advertised capabilities.
Otherwise, it's kept as advertised.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
If `bgp default software-version-capability` is enabled, allow unsetting this
for a single neighbor also.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Alpine Linux gets this with 3.19:
This is already installed with `pytest` via apk package manager.
```
15 78.20 error: externally-managed-environment
15 78.20
15 78.20 × This environment is externally managed
15 78.20 ╰─>
15 78.20 The system-wide python installation should be maintained using the system
15 78.20 package manager (apk) only.
15 78.20
15 78.20 If the package in question is not packaged already (and hence installable via
15 78.20 "apk add py3-somepackage"), please consider installing it inside a virtual
15 78.20 environment, e.g.:
15 78.20
15 78.20 python3 -m venv /path/to/venv
15 78.20 . /path/to/venv/bin/activate
15 78.20 pip install mypackage
15 78.20
15 78.20 To exit the virtual environment, run:
15 78.20
15 78.20 deactivate
15 78.20
15 78.20 The virtual environment is not deleted, and can be re-entered by re-sourcing
15 78.20 the activate file.
15 78.20
15 78.20 To automatically manage virtual environments, consider using pipx (from the
15 78.20 pipx package).
15 78.20
15 78.20 note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
This is happening for Alpine Linux.
```
26 64.59 ./lib/sigevent.h:23:18: error: unknown type name 'sig_atomic_t'
26 64.59 23 | volatile sig_atomic_t caught; /* private member */
26 64.59 | ^~~~~~~~~~~~
26 64.60 In file included from ./lib/libfrr.h:12,
26 64.60 from ./lib/vty.h:28,
26 64.60 from ./lib/command.h:11,
26 64.60 from ./lib/debug.h:11,
26 64.60 from ./mgmtd/mgmt.h:12,
26 64.60 from mgmtd/mgmt_history.c:14:
26 64.60 ./lib/sigevent.h:23:18: error: unknown type name 'sig_atomic_t'
26 64.60 23 | volatile sig_atomic_t caught; /* private member */
26 64.60 | ^~~~~~~~~~~~
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
