Further refine the previous commit to store the hash value in
both the `struct community_list` as well as the `struct rmap_community`
structures. This allows us to know a priori what our hash value
is. This change cuts another couple of seconds of convergence
off to ~55 seconds and further reduces cpu load of bgp:
16 40061.706 433732 92 330102 129 1242965 RWTEX TOTAL
Down from ~43 seconds previously.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This commit is the last missing piece to complete BGP LU support in bgpd. To this moment, bgpd (and zebra) supported auto label assignment only for prefixes leaked from VRFs to vpn and for MPLS SR prefixes. This adds auto label assignment to other routes types in bgpd. The following enhancements have been made:
* bgp_route.c:bgp_process_main_one() now sets implicit-null local_label to all local, aggregate and redistributed routes.
* bgp_route.c:bgp_process_main_one() now will request a label from the label pool for any prefix that loses the label for some reason (for example, when the static label assignment config is removed)
* bgp_label.c:bgp_reg_dereg_for_label() now requests labels from label pool for routes which have no associated label index
* zebra_mpls.c:zebra_mpls_fec_register() now expects both label and label_index from the calling function, one of which must be set to MPLS_INVALID_LABEL or MPLS_INVALID_LABEL_INDEX, based on this it will decide how to register the provided FEC.
Signed-off-by: Anton Degtyarev <anton@cumulusnetworks.com>
Default vrf name has been changed in show route. Because the default vrf
name can be configured in zebra, the default vrf name in bgp is changed.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
if default vrf is not Default, then nexthop vrf name returned may be
"Default", which is not the correct name of default vrf. change it
accordingly.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Add some code that will reject local mac's from
being installed and add some code that will cause
a rescan when we have a local mac change.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com.
When you have configured soft reconfiguration inbound
for evpn allow it to notice and send in the evpn data
as appropriate.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The adj_out data structure is a linked list of adjacencies
1 per update group. In a large scale env where we are
not using peer groups, this list lookup starts to become
rather costly. Convert to a better data structure for this.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
1. "show bgp ipv4/ipv6 [json]"
2. "show bgp ipv4/ipv6 neighbor <peer> routes [json]"
3. "show bgp ipv4/ipv6 neighbors <peer> advertised-routes [json]"
In the above show commands, when a BGP path is displayed, we do not display the
local preference if it is EBGP route. Route calculation assumes the default
local preference. But, we can change the default local preference using
configuration in FRR. In this case, user should know the default local
preference value that is being used in the route calculation. Thus, adding a
new field 'default local preferece' in the show commands where a BGP path is
displayed.
When a BGP path is displayed in the above show commands, as-path does not
include the local AS. So, user has to execute another show command to display
the local-AS. To avoid this, adding a new field local-AS to above show commands.
Signed-off-by: Ameya Dharkar <adharkar@vmware.com>
The bgp_distance_set_node_info and bgp_distance_get_node_info
function names were slightly backwards lets fix them up
to bgp_node_get_bgp_distance_info and bgp_node_set_bgp_distance_info
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The bgp_static_set_node_info and bgp_static_get_node_info
function names were slightly backwards rename to
bgp_node_get_bgp_static_info and bgp_node_set_bgp_static_info
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The bgp_aggregate_set_node_info and bgp_aggregate_get_node_info
functions names were slightly backwards, rename to
bgp_node_get_bgp_aggregate_info and bgp_node_set_bgp_aggregate_info
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The bgp_info 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>
The motivation for this patch is to address a concerning behavior of
tx-addpath-bestpath-per-AS. Prior to this patch, all paths' TX ID was
pre-determined as the path was received from a peer. However, this meant
that any time the path selected as best from an AS changed, bgpd had no
choice but to withdraw the previous best path, and advertise the new
best-path under a new TX ID. This could cause significant network
disruption, especially for the subset of prefixes coming from only one
AS that were also communicated over a bestpath-per-AS session.
The patch's general approach is best illustrated by
txaddpath_update_ids. After a bestpath run (required for best-per-AS to
know what will and will not be sent as addpaths) ID numbers will be
stripped from paths that no longer need to be sent, and held in a pool.
Then, paths that will be sent as addpaths and do not already have ID
numbers will allocate new ID numbers, pulling first from that pool.
Finally, anything left in the pool will be returned to the allocator.
In order for this to work, ID numbers had to be split by strategy. The
tx-addpath-All strategy would keep every ID number "in use" constantly,
preventing IDs from being transferred to different paths. Rather than
create two variables for ID, this patch create a more generic array that
will easily enable more addpath strategies to be implemented. The
previously described ID manipulations will happen per addpath strategy,
and will only be run for strategies that are enabled on at least one
peer.
Finally, the ID numbers are allocated from an allocator that tracks per
AFI/SAFI/Addpath Strategy which IDs are in use. Though it would be very
improbable, there was the possibility with the free-running counter
approach for rollover to cause two paths on the same prefix to get
assigned the same TX ID. As remote as the possibility is, we prefer to
not leave it to chance.
This ID re-use method is not perfect. In some cases you could still get
withdraw-then-add behaviors where not strictly necessary. In the case of
bestpath-per-AS this requires one AS to advertise a prefix for the first
time, then a second AS withdraws that prefix, all within the space of an
already pending MRAI timer. In those situations a withdraw-then-add is
more forgivable, and fixing it would probably require a much more
significant effort, as IDs would need to be moved to ADVs instead of
paths.
Signed-off-by Mitchell Skiba <mskiba@amazon.com>
Same sequence number handling is specified by RFC 7432 -
[
If two (or more) PEs advertise the same MAC address with the same
sequence number but different Ethernet segment identifiers, a PE that
receives these routes selects the route advertised by the PE with the
lowest IP address as the best route.
If the PE is the originator of the MAC route and it receives the same
MAC address with the same sequence number that it generated, it will
compare its own IP address with the IP address of the remote PE and
will select the lowest IP. If its own route is not the best one, it
will withdraw the route.
]
To implement that specification this commit uses nexthop IP as a tie
breaker between two paths of equal seq number with lower IP winning.
Now if a local path already exists with the same sequence number but higher
(local-VTEP) IP it is evicted (deleted and withdrawn from the peers) and
the winning new remote path is installed in zebra. This is existing code
and handled implicitly via evpn_route_select_install.
If a local path is rxed from zebra with the same sequence as the
current remote winner it is rejected (not installed in the bgp
routing tables) and zebra is asked to re-install the older/remote winner.
This is a race condition that can only happen if bgp's add and zebra's add
cross paths. Additional handling has been added in this commit via
evpn_cleanup_local_non_best_route to take care of the race condition.
Ticket: CM-22674
Reviewed By: CCR-7937
Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
Problems were reported with the name of the default vrf and the
default bgp instance being different, creating confusion. This
fix changes both to "default" for consistency.
Ticket: CM-21791
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Reviewed-by: CCR-7658
Testing: manual testing and automated tests before pushing
community_free, lcommunity_free and ecommunity_free are similar type of functions. Most of the places, these three are called together. The signature of community_free is different from other two functions. Modified the community_free API signature to align with other two functions to avoid any confusion. There is no functionality impact with this and this is just to avoid any confusion.
Testing: manual testing and show commands
Signed-off-by: Sri Mohana Singamsetty msingamsetty@vmware.com
as prefix is opaque for flowspec, and json needs to have a non empty
full of meaning value in prefix, the proposal is to encode the
displayable form of flowspec entry.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Add the ability to aggregate routes to handle
extended communities. Make the actions similiar
to what we do for normal communities.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
1. "show bgp ipv4 json"
- Added "network" field which displays a prefix in 'prefix/prefixlen' format.
2. "show bgp ipv6 json"
- Added "network" field which displays a prefix in 'prefix/prefixlen' format.
- JSON does not have "prefix", "prefixLen" fields which are present in IPv4
command. Added these fields as they are useful.
3. "show bgp ipv4/ipv6 neighbor <neighbor_addr> advertised-routes json"
- Added "network" field.
4. "show bgp ipv4/ipv6 summary json"
- Added "pfxSnt" for peers. This count is obtained from corresponding
update_subgroup.
5. "show bgp neighbor json"
- Added "sentPrefixCounter"
Signed-off-by: Ameya Dharkar <adharkar@vmware.org>
Do a straight conversion of `struct bgp_info` to `struct bgp_path_info`.
This commit will setup the rename of variables as well.
This is being done because `struct bgp_info` is not descriptive
of what this data actually is. It is path information for routes
that we keep to build the actual routes nexthops plus some extra
information.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The bgp->peerhash is a secretive bit of data that we use
to quickly lookup data about peers. Unfortunately
since we had not way to look at it, we had no way
of knowing if it had gotten in or out of sync.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
1. "show bgp ipv4 json"
- Corresponding CLI has "network" field which displays a prefix in
'prefix/prefixlen' format. Added this "network" field to JSON as well.
- Following fields have different names in JSON and CLI.
CLI JSON
metric med
locPrf localPref
path aspath
Added fields "metric", "locPrf" and "path" in JSON for CLI/JSON
consistency. Older JSON fields med, localPref, aspath will be
deprecated in future.
2. "show bgp ipv6 json"
- Similar changes as "show bgp ipv4 json"
- JSON does not have "prefix", "prefixLen" fields which are present in IPv4
command. Added these fields as they are useful.
3. "show bgp ipv4/ipv6 neighbor <neighbor_addr> advertised-routes json"
- Added "network" field.
- Added locPrf, path fields for CLI/JSON consistency. localPref, aspath will
be deprecated in future.
4. "show bgp ipv4/ipv6 summary json"
- Added "pfxRcd" for CLI/JSON consistency.
"prefixReceivedCount" will be deprecated in future.
- Added "pfxSnt" for peers. This count is obtalned from corresponding
update_subgroup. This needed a fix in the code where we copy fields
for a split update_subgroup from the parent update_subgrp.
New subgrp should inherit subgrp->scount(Count of advertized prefixes)
of the parent subgrp.
5. "show bgp neighbor json"
- Added "sentPrefixCounter"
6. "show bgp ipv4/ipv6 <prefix> json"
- Added "metric" field for CLI/JSON consistency.
"med" will be deprecated in future.
Signed-off-by: Ameya Dharkar <adharkar@vmware.org>
When this description code was added, it was all dead code since none of
the bools that checked if the communities were present were ever changed
from 0.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
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>
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>
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>
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>
When the origin changed we must honor and update the aggregate
to the peer. This code adds a bit of code to the bgp_aggregate_info_same
code to see if the origin has changed and to indicate that it has.
Fixes: #2993
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Problem reported that some bgp and ospf json commands did not return
any json output at all if the bgp/ospf instance did not exist.
Additionally, some bgp and ospf json commands did not return any json
output if the instance existed but no neighbors were defined. This
fix makes these commands more consistent in returning empty braces for
json output and issue a message if not using json output. Additionally,
made the flag "use_json" a bool to make it consistent since previously,
it had been defined as an int, char, u_char, and bool at various places.
Ticket: CM-21040
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Coded as part of #2684 and most code written while participating at
BornHack@2018.
bgp_route.c: Changes regarding adding explanations for the IANA
well-known communities added in #2684
Signed-off-by: Christoffer <netravnen@gmail.com>
Several zlog_warns were being used to tell the end
user that bgp had detected a bug. These all look like information
added during development that can be noted as debugs or logged
as an error situation.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
A warn with a backtrace does not need another warn
to file an issue with Quagga, so just remove it.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Root Cause: In the function bgp_show_table(), we are creating a
json object and a json array with the same name as “json_paths”.
First it will create a json object variable "json_paths" pointing
to the memory allocated for the json object. Then it will create
a json array for each bap node rn (if rn->info is available) with
the same name as json_paths. Because of this, json_paths which was
pointing to the memory allocated for the json object earlier, now
will be overwritten with the memory allocated for the json array.
As per the existing code, at the end of each iteration loop of bgp
node, it will deallocate the memory used by the json array and
assigned NULL to the variable json_paths. Since we don’t have the
pointer pointing to the memory allocated for json object, will be
not able to de-allocate the memory, which is a memory leak here.
Fix: Removing this json object since it is never getting used in
this function.
Testing: Reproduced the memory leak with valgrind.
With the fix, memory leak gets resolved and checked with valgrind.
Signed-off-by: Sarita Patra saritap@vmware.com
Make the wart slightly less bad... also there is still a possible write
after free here. This needs to be fixed again, properly, by some
structure changes.
Signed-off-by: David Lamparter <equinox@diac24.net>
Avoid memory leak in bgp flowspec list.
Usage of bool parameter instead of int, to handle the number of entries
PBR.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
There exists a few places where actual debugs were being
displayed as warns. Convert them over to debugs and
guard as appropriate.
Signed-off-by: Donald Sharp <sharpd@cumulsunetworks.com>
Classful networking has been obsolete for ages and there is currently an
inconsistency between `show ip route` and `show bgp`, where the first
one always displays the CIDR mask while the second one hides classful
network masks.
This commit adjusts the behavior of `show bgp` to always show the CIDR
mask for a route, even when it is classful.
Signed-off-by: Pascal Mathis <mail@pascalmathis.com>
There is no need to check for failure of a ALLOC call
as that any failure to do so will result in a assert
happening. So we can safely remove all of this code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Avoid displaying default configured local preference as
part of bgp route's detail output.
Local preference is for iBGP learnt route's. The value could be
default (100) or configured value (via routemap or local pref config cmd).
show bgp afi safi (brief output) does not display,
if the local pref attribute is not set.
Similarly, show bgp afi safi detail route output should display
if the the attribute is set, and should not display configured value.
This way both output would be consistent.
The configured local preference can be seen via running-config.
Ticket:CM-12769
Reviewed By:
Testing Done:
eBGP output:
show bgp ipv4 45.0.3.0/24
BGP routing table entry for 45.0.3.0/24
Paths: (1 available, best #1, table default)
Advertised to non peer-group peers:
MSP1(uplink-1) MSP2(uplink-2)
Local
0.0.0.0 from 0.0.0.0 (27.0.0.9)
Origin incomplete, metric 0, weight 32768, valid,sourced, bestpath-from-AS Local, best
AddPath ID: RX 0, TX 7
Last update: Thu Jul 26 02:10:02 2018
iBGP output:
show bgp ipv4 unicast 6.0.0.16/32
BGP routing table entry for 6.0.0.16/32
Paths: (1 available, best #1, table default)
Not advertised to any peer
Local
6.0.0.16 (metric 20) from tor-12(6.0.0.16) (6.0.0.16)
Origin incomplete, metric 0, localpref 100, valid, internal, bestpath-from-AS Local, best
AddPath ID: RX 0, TX 13
Last update: Thu Jul 26 05:26:18 2018
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
The bi->net pointer that is being unlocked had a commit
that removed the `bi->net = NULL;` recently. This code
was preventing a use after free crash being experienced
in other code paths. While commit 37e679629f was fixing
a different code path crash.
Make the parent->net pointer aware it may be locked/freed
from multiple places and to not NULL the pointer to it
unless we have actually freed the data.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
There is a default BGP VPN and BGP VRF instance in L3VPN configuration.
The routes are imported and exported between BGP VPN and BGP VRF.
Suppose there is one route in BGP VRF and exported to BGP VPN.
In BGP VPN there is bgp_info struct with bgp_info_extra struct has parent pointer pointing to the bgp_info of BGP VRF.
We take the lock for bgp_node and bgp_info of BGP VRF in the context of BGP VPN.
bgp_info has a back pointer to bgp_node via net.
Now when we have done "no rd vpn" in BGP VRF then in bgp_info_extra_free we have to free the parent resources.
In this context only unlocking is required. It should not set the BGP VRF (bgp_info->net) to NULL.
Signed-off-by: vishaldhingra vdhingra@vmware.com
because the IP destination criterium may match several entries, the show
command may return more than one entry.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Because one flowspec entry can create 1-N bgp pbr entries, the list is
now updated and visible. Also, because the bgp_extra structure is used,
this list is flushed when the bgp_extra structure is deleted.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
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>
