This commit removes various parts of the bgpd implementation code which
are unused/useless, e.g. unused functions, unused variable
initializations, unused structs, ...
Signed-off-by: Pascal Mathis <mail@pascalmathis.com>
The warning given by PVS-Studio is related to per-element overflow (there is
no real overflow, because of how elements are mapped in the union). This
same warning is typically reported by Coverity, too.
Signed-off-by: F. Aragon <paco@voltanet.io>
In order to make EVPN behavior work without special casing
the code, bring the evpn commands under HAVE_CUMULUS into
the fold.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This commit fixes the issue mentioned in #2419, which is caused by a
double-free. The problem of the current implementation is that
*bgp_input_modifier* already frees the passed attributes under specific
circumstances, which can then lead to a double-free as *bgp_attr_undup*
does not check if the attributes are set to NULL.
As it is not transparent to the function caller if the attributes get
freed or not and the similar function *bgp_output_modifier* also does
not flush the passed attributes, the line has been removed altogether.
All callers of *bgp_input_modifier* already deal by themself with
freeing/flushing/unduping BGP attributes, so it is safe to remove.
Signed-off-by: Pascal Mathis <mail@pascalmathis.com>
When evpn configured wiht route-map with vni which is not
configured. Upon receiving evpn routes (i.e Type-2, Type-3),
route-map match will be triggered. Since there is no l2vni
exists in db, some of the member fields in bgp_info (i.e.
dummy_info_extra) are passed uninitialized to evpn filter match cb.
This results in inaccessible memory causes crash.
Fix is to memset the bgp_info prior to passing to evpn filter cb.
In evpn vni filter cb, ensure to have NULL check for member filed
of the bgp_info.
memset bgp_info at few places where it is passed to route_match.
Ticket:CM-21335
Reviewed By:
Testing Done:
Configure route-map with not configured l2vni
Simulate to learn l2vpn type-2, 3 route
Restart frr.service with below config
address-family l2vpn evpn
neighbor fear route-map EVPN_VNI out
route-map EVPN_VNI deny 10
match evpn vni 140010
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
The bgp_info_extra_free code was the correct place to free
up data associated with the bgp_info pointer when we are
deleting the bgp_info node.
Additionally, if we have a parent pointer, we may not have a net
pointer. So make sure we do.
Finally clean up the bgp_info_extra_free code so it is a bit
easier to read. Use variables instead of multiple level
of casting.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
With a new version of clang 6.0, the compiler is detecting more
issues where we may be possibly be truncating the output string.
Fix by increasing the size of the output string to make the compiler
happy.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
These two functions are functionally the same, except
bgp_aggregate_route is meant to handle the addition and
deletion of routes, while aggregate_add is meant for all of them.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The aggregated route was being sent in updates to peers every
time a route changed that we were aggregating. Modify
the code such that we only send aggregated route updates
if we actually have something different to tell the peer.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The function bgp_aggregate_delete function was forward
declared and not static. Move it so we can clean that
up.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This is a transitional commit, to get us where we want to go.
Seperate out the install/removal of the aggregate route from
the bgp_aggregate_delete and bgp_aggregate_route functions.
In the future we'll write a bit of code to determine if the
aggregate add has actually changed any information we care
about.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We were allowing useless aggregation commands (/32 and /128).
These were being silently accepted and nvgenned and then
just ignored.
When a user enters a value that should be rejected tell
them and reject.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Make bgp_aggregate_route easier to read. It was indented so many
levels that it was extremely hard to figure out what it was doing.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The #define AGGREGATE_NEXTHOP_CHECK has not been used
for a very very long time. Since this is effectively
dead code, let's remove it.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The safi passed in to short-circuit the aggregate lookup
adds code complexity and little speed improvements for
the case where we actually may have aggregates configured!
Since bgp_table_top_nolock() actually tells us if there
are any aggregates installed and safely returns if there
is nothing to do, trust it. As that we know for those
safi's were we don't want to have, we dissallow the
creation via the cli anyways.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The bgp_aggregate_set/unset functions are only called from the cli
invocations which control what AFI/SAFI we are looking at. Tests
for safi are unimportant.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
There exists cases where we will attempt to hard delete
the bgp instance( say a `no router bgp` is issued )
when we have vrf route leaking. If we do have this
lock the bgp instance of the originator and do not
let it be deleted out from under us until we are
finished processing.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Issue Found by Coverity Scan. When we call peer_clear we
are checking the return code in every other call. Add
a bit of extra code to notice the failure and note it.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Added improved error message text to other places that could also
encounter the same condition. In testing found that in certain
case, duplicate error messages were previously issued. This fix
also removes the duplicates.
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
There exists code paths where the rn was being used after free.
This eliminates these code paths.
Fixes: CM-21019
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This commit tries to adapt a similar codeflow within the `show bgp [afi]
[safi] neighbor <neighbor> advertised-routes` command compared to its
`received-routes` and `filtered-routes` opponents. Some branching code
has been restructured to achieve this.
Additionally, this commit fixes a memory leak within `received-routes`
(and `filtered-routes`, although the issue has been present before the
previous commit!) where the previous implementation forgot to
deduplicate the BGP attributes.
When a user called `<...> received-routes route-map <RM-TEST>` and that
routemap changed any AS path or community parameters, the duplicated
memory for these parameters was never freed. This has been fixed by
ensuring to call `bgp_attr_undup()` accordingly.
Signed-off-by: Pascal Mathis <mail@pascalmathis.com>
This commit changes the behavior of `show bgp [afi] [safi] neighbor
<neighbor> received-routes [json]` to return all received prefixes
instead of filtering rejected/denied prefixes.
Compared to Cisco and Juniper products, this is the usual way how this
command is supposed to work, as `show bgp [afi] [safi] neighbor
<neighbor> routes` will already return all accepted prefixes.
Additionally, the new command `show bgp [afi] [safi] neighbor <neighbor>
filtered-routes` has been added, which returns a list of all prefixes
that got filtered away, so it can be roughly described as a subset of
"received prefixes - accepted prefixes".
As the already available `filtered_count` variable inside
`show_adj_route` has not been used before, the last output line
summarizing the amount of prefixes found was extended to also mention
the amount of filtered prefixes if present.
Signed-off-by: Pascal Mathis <mail@pascalmathis.com>
The current implementation of building JSON output is greatly different
for large communities compared to standard communities. This is mainly
noticeable by the missing 'list' attribute, which usually offers an
array of all communities present on a BGP route.
This commit adds the missing functionality of properly returning a
'list' attribute in JSON output and also tries a similar approach like
the standard communities are using to implement this feature.
Additionally, the 'format' specifier has been completely removed from
large communities string/JSON rendering, as the official RFC8092 specifies that
there is only one canonical representation:
> The canonical representation of BGP Large Communities is three
> separate unsigned integers in decimal notation in the following
> order: Global Administrator, Local Data 1, Local Data 2. Numbers
> MUST NOT contain leading zeros; a zero value MUST be represented with
> a single zero. Each number is separated from the next by a single
> colon. For example: 64496:4294967295:2, 64496:0:0.
As the 'format' specifier has not been used/checked and only one
canonical representation exists per today, there was no reason to keep
the 'format' parameter in the function signature.
Last but not least, the struct attribute 'community_entry.config' is no
longer being used for large communities and instead 'lcommunity_str' is
being called to maintain a similar approach to standard communities.
As a side effect, this also fixed a memory leak inside 'community_entry_free'
which did not free the allocated memory for the 'config' attribute when
dealing with a large community.
Signed-off-by: Pascal Mathis <mail@pascalmathis.com>
Imported routes in a VRF routing table have a reference to their parent
route entry which resides in the EVPN or IPVPN routing table. Ensure that
this reference uses appropriate locking so that the parent entry doesn't
get freed prematurely.
Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
(cherry picked from commit 13cb6b22ba9d558b1b4a1e8752f63f13242462a7)
Conflicts:
bgpd/bgp_mplsvpn.c
Ticket: CM-20471
Testing Done:
a) Ran vrf_route_leak tests without fix and hit crash, ran twice with fix
and did not see the crash.
b) Ran evpn-smoke and ensured there were no new failures.
In the FRR implementation of EVPN,
eBGP leaf-spine peering for EVPN is fully supported by allowing
the next hop to be propagated and not rewritten at each hop.
There are other changes also related to route import to facilitate this.
However, propagating the next hop is not correct in some cases.
Specifically, if the DC is comprised of multiple PODs
with distinct intra-POD and inter-POD VxLAN tunnels,
EVPN routes received from an adjacent POD by a border/exit leaf
must be propagated into the local POD with the next hop rewritten (to self).
Signed-off-by: Mitesh Kanjariya <mitesh@cumulusnetworks.com>
We install type-5 routes as ipv4/ipv6 unicast routes in the vrf table.
along with these routes, we also install the RMAC
and the nexthop Neigh entries.
There might be scenarios were the bestpath has changed and
we are now pointing to a new nexthop with a different RMAC.
As per BGP logic, we just send an update for the route and the nexthop
is replaced. However, this causes problem because the RMAC and neigh entry
corresponding to the previous nexthop are still lingering in the system.
We need to clear those entries for proper functoning.
Signed-off-by: Mitesh Kanjariya <mitesh@cumulusnetworks.com>
A newly added ipv4/ipv6 route in BGP RIB might be advertised as type-5 EVPN route.
The user might have configured a route-map for advertising type-5 routes.
We need to apply this route-map while advertising ipv4/ipv6 routes as type-5.
Signed-off-by: Mitesh Kanjariya <mitesh@cumulusnetworks.com>
