If we have soft inbound enabled, we should see how the route looks like
before it was modified by a route-map/prefix-list.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
1. bgp coredump is observed when we delete default bgp instance
when we have multi-vrf; and route-leaking is enabled between
default, non-default vrfs.
Removing default router bgp when routes leaked between non-default vrfs.
- Routes are leaked from VRF-A to VRF-B
- VPN table is created with auto RD/RT in default instance.
- Default instance is deleted, we try to unimport the routes from all VRFs
- non-default VRF schedules a work-queue to process deleted routes.
- Meanwhile default bgp instance clears VPN tables and free the route
entries as well, which are still referenced by non-default VRFs which
have imported routes.
- When work queue process starts to delete imported route in VRF-A it cores
as it accesses freed memory.
- Whenever we delete bgp in default vrf, we skip deleting routes in the vpn
table, import and export lists.
- The default hidden bgp instance will not be listed in any of the show
commands.
- Whenever we create new default instance, handle it with AS number change
i.e. old hidden default bgp's AS number is updated and also changing
local_as for all peers.
2. A default instance is created with ASN of the vrf with the import
statement.
This may not be the ASN desired for the default table
- First problem with current behavior.
Define two vrfs with different ASNs and then add import between.
starting without any bgp config (no default instance)
A default instance is created with ASN of the vrf with the import
statement.
This may not be the ASN desired for the default table
- Second related problem. Start with a default instance and a vrf in a
different ASN. Do an import statement in the vrf for a bgp vrf instance
not yet defined and it auto-creates that bgp/vrf instance and it inherits
the ASN of the importing vrf
- Handle bgp instances with different ASNs and handle ASN for auto created
BGP instance
Signed-off-by: Kantesh Mundaragi <kmundaragi@vmware.com>
Add counters for redistributed routes, and local aggregates to the
output of "show ip bgp statistics".
Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
The check for an equivalent bgp pointer makes no sense
in the context of the workqueue as that we have a
work queue per bgp process, as such the bgp pointer
will always be the same as the pqnode.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
There is a need to be able to process certain bgp
routes earlier than others. Especially when there
is major trauma going on in the network. Start
the ability for this to happen.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Ticket: #4060069
show bgp vrf afi unicast statistics json output is not return in json
format for non exists vrf.
Fix:
Json output is formatted for non exists vrf cases.
Command supported:
```
show bgp vrf <VRFNAME> ipv4/ipv6 unicast statistics json
show bgp vrf <VRFNAME> l2vpn evpn statistics json
```
Before Fix:
```
leaf11#
leaf11# show bgp vrf test ipv4 unicast statistics json
View/Vrf test is unknown
leaf11#
leaf11#
leaf11# show bgp vrf test ipv6 unicast statistics json
View/Vrf test is unknown
leaf11#
leaf11#
leaf11# show bgp vrf default1 l2vpn evpn statistics json
View/Vrf default1 is unknown
leaf11#
```
After Fix:
```
leaf11#
leaf11# show bgp vrf test ipv4 unicast statistics json
{
"warning":"View/Vrf is unknown"
}
leaf11#
leaf11#
leaf11# show bgp vrf test ipv6 unicast statistics json
{
"warning":"View/Vrf is unknown"
}
leaf11#
leaf11# show bgp vrf default1 l2vpn evpn statistics json
{
"warning":"View/Vrf is unknown"
}
leaf11#
```
Ticket: #4060069
Signed-off-by: Sindhu Parvathi Gopinathan's <sgopinathan@nvidia.com>
With lots of update-groups, subgroups, this could be very tricky and the timer
is spawned even if it's totally unnecessary (default-originate is not enabled).
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
evpn has a concept of `local` tables where the evpn routes
are actually converted into underlying routes/neighbor
table entries( or vice versa ). Then this local route
is propagated to the global evpn l2vpn table and sent
to the peers. Certain show commands in evpn look
operate on the local table but make the output look
like the data has not been sent to the peer. This
is confusing for the operator. Modify the code
such that local tables get a `Local BGP table not advertised`
in the place where the code talks about whom has received
the data or not.
Example:
torm11# show bgp l2vpn evpn route vni 1000 mac 8a:a1:cc:73:a3:ac ip 45.0.0.5
BGP routing table entry for [2]:[0]:[48]:[8a:a1:cc:73:a3:ac]:[32]:[45.0.0.5]
Paths: (2 available, best #2)
Local BGP table not advertised
Route [2]:[0]:[48]:[8a:a1:cc:73:a3:ac]:[32]:[45.0.0.5] VNI 1000
Imported from 192.168.100.18:2:[2]:[0]:[48]:[8a:a1:cc:73:a3:ac]:[32]:[45.0.0.5], VNI 1000
65101 65005
192.168.100.18(leaf2) from leaf2(192.168.5.1) (192.168.100.14)
Origin IGP, valid, external
Extended Community: RT:65005:1000 ET:8
Last update: Thu Mar 21 14:29:04 2024
Route [2]:[0]:[48]:[8a:a1:cc:73:a3:ac]:[32]:[45.0.0.5] VNI 1000
Imported from 192.168.100.18:2:[2]:[0]:[48]:[8a:a1:cc:73:a3:ac]:[32]:[45.0.0.5], VNI 1000
65101 65005
192.168.100.18(leaf1) from leaf1(192.168.1.1) (192.168.100.13)
Origin IGP, valid, external, bestpath-from-AS 65101, best (Router ID)
Extended Community: RT:65005:1000 ET:8
Last update: Thu Mar 21 14:29:04 2024
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
If we expand the truth (A || B) to "(A && B) || (A && !B) || (!A && B)"
so that we can isolate the case (!A && B), we then add the additional
check (C) to ensure that original route actually has a link-local hext-hop
Signed-off-by: Richard Cunningham <29760295+cunningr@users.noreply.github.com>
Fix static-analyser warnings with BGP labels:
> $ scan-build make -j12
> bgpd/bgp_updgrp_packet.c:819:10: warning: Access to field 'extra' results in a dereference of a null pointer (loaded from variable 'path') [core.NullDereference]
> ? &path->extra->labels->label[0]
> ^~~~~~~~~
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
When parsing EVPN NLRIs, and an error occurred, do no forget to free the memory.
Fixes: 4ace11d010 ("bgpd: Move evpn_overlay to a pointer")
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
A crash happens when executing the following command:
> ubuntu2204hwe# conf
> ubuntu2204hwe(config)# router bgp 65500
> ubuntu2204hwe(config-router)# !
> ubuntu2204hwe(config-router)# address-family ipv4 unicast
> ubuntu2204hwe(config-router-af)# sid vpn export auto
> ubuntu2204hwe(config-router-af)# exit-address-family
> ubuntu2204hwe(config-router)# !
> ubuntu2204hwe(config-router)# address-family ipv4 vpn
> ubuntu2204hwe(config-router-af)# network 4.4.4.4/32 rd 55:55 label 556
> ubuntu2204hwe(config-router-af)# network 5.5.5.5/32 rd 662:33 label 232
> ubuntu2204hwe(config-router-af)# exit-address-family
> ubuntu2204hwe(config-router)# exit
> ubuntu2204hwe(config)# !
> ubuntu2204hwe(config)# no router bgp
The crash analysis indicates a memory item has been freed.
> #6 0x000076066a629c15 in mt_count_free (mt=0x56b57be85e00 <MTYPE_BGP_NAME>, ptr=0x60200038b4f0)
> at lib/memory.c:73
> #7 mt_count_free (ptr=0x60200038b4f0, mt=0x56b57be85e00 <MTYPE_BGP_NAME>) at lib/memory.c:69
> #8 qfree (mt=mt@entry=0x56b57be85e00 <MTYPE_BGP_NAME>, ptr=0x60200038b4f0) at lib/memory.c:129
> #9 0x000056b57bb09ce9 in bgp_free (bgp=<optimized out>) at bgpd/bgpd.c:4120
> #10 0x000056b57bb0aa73 in bgp_unlock (bgp=<optimized out>) at ./bgpd/bgpd.h:2513
> #11 peer_free (peer=0x62a000000200) at bgpd/bgpd.c:1313
> #12 0x000056b57bb0aca8 in peer_unlock_with_caller (name=<optimized out>, peer=<optimized out>)
> at bgpd/bgpd.c:1344
> #13 0x000076066a6dbb2c in event_call (thread=thread@entry=0x7ffc8cae1d60) at lib/event.c:2011
> #14 0x000076066a60aa88 in frr_run (master=0x613000000040) at lib/libfrr.c:1214
> #15 0x000056b57b8b2c44 in main (argc=<optimized out>, argv=<optimized out>) at bgpd/bgp_main.c:543
Actually, the BGP_NAME item has not been used at allocation for
static->prd_pretty, and this results in reaching 0 quicker at bgp
deletion.
Fix this by reassigning MTYPE_BGP_NAME to prd_pretty.
Fixes: 16600df2c4 ("bgpd: fix show run of network route-distinguisher")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Instead of using 3 uint8_t variables under struct attr, let's use a single
uint8_t as the flags. Saving 2-bytes. Not a big deal, but it's even easier to
track EVPN-related flags/variables.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Introduce BGP-wide flags to denote if BGP has started gracefully
and GR is in progress or not. Use this for setting of the R-bit in
the GR capability, and not a timer which is set for any new
instance creation. Mark graceful restart is complete when the
deferred path selection has been done and route sync with zebra as
well as deferred EOR advertisement has been initiated.
Introduce a function to check on F-bit setting rather than just
base it on configuration.
Subsequent commits will extend these functionalities.
Signed-off-by: Vivek Venkatraman <vivek@nvidia.com>
RFC 8212 defines leak prevention for eBGP peers, but BGP-OAD defines a new
peering type One Administrative Domain (OAD), where multiple ASNs could be used
inside a single administrative domain. OAD allows sending non-transitive attributes,
so this prevention should be relaxed too.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Under a setup where two BGP prefixes are available from multiple sources,
if one of the two prefixes is recursive over the other BGP prefix, then
it will not be considered as multipath. The below output shows the two
prefixes 192.0.2.24/32 and 192.0.2.21/32. The 192.0.2.[5,6,8] are the
known IP addresses visible from the IGP.
> # show bgp ipv4 192.0.2.24/32
> *>i 192.0.2.24/32 192.0.2.21 0 100 0 i
> * i 192.0.2.21 0 100 0 i
> * i 192.0.2.21 0 100 0 i
> # show bgp ipv4 192.0.2.21/32
> *>i 192.0.2.21/32 192.0.2.5 0 100 0 i
> *=i 192.0.2.6 0 100 0 i
> *=i 192.0.2.8 0 100 0 i
The bgp best selection algorithm refuses to consider the paths to
'192.0.2.24/32' as multipath, whereas the BGP paths which use the
BGP peer as nexthop are considered multipath.
> ... has the same nexthop as the bestpath, skip it ...
Previously, this condition has been added to prevent ZEBRA from
installing routes with same nexthop:
> Here you can see the two paths with nexthop 210.2.2.2
> superm-redxp-05# show ip route 2.23.24.192/28
> Routing entry for 2.23.24.192/28
> Known via "bgp", distance 20, metric 0, best
> Last update 00:32:12 ago
> * 210.2.2.2, via swp3
> * 210.2.0.2, via swp1
> * 210.2.1.2, via swp2
> * 210.2.2.2, via swp3
> [..]
But today, ZEBRA knows how to handle it. When receiving incoming routes,
nexthop groups are used. At creation, duplicated nexthops are
identified, and will not be installed. The below output illustrate the
duplicate paths to 172.16.0.200 received by an other peer.
> r1# show ip route 172.18.1.100 nexthop-group
> Routing entry for 172.18.1.100/32
> Known via "bgp", distance 200, metric 0, best
> Last update 00:03:03 ago
> Nexthop Group ID: 75757580
> 172.16.0.200 (recursive), weight 1
> * 172.31.0.3, via r1-eth1, label 16055, weight 1
> * 172.31.2.4, via r1-eth2, label 16055, weight 1
> * 172.31.0.3, via r1-eth1, label 16006, weight 1
> * 172.31.2.4, via r1-eth2, label 16006, weight 1
> * 172.31.8.7, via r1-eth4, label 16008, weight 1
> 172.16.0.200 (duplicate nexthop removed) (recursive), weight 1
> 172.31.0.3, via r1-eth1 (duplicate nexthop removed), label 16055, weight 1
> 172.31.2.4, via r1-eth2 (duplicate nexthop removed), label 16055, weight 1
> 172.31.0.3, via r1-eth1 (duplicate nexthop removed), label 16006, weight 1
> 172.31.2.4, via r1-eth2 (duplicate nexthop removed), label 16006, weight 1
> 172.31.8.7, via r1-eth4 (duplicate nexthop removed), label 16008, weight 1
Fix this by proposing to let ZEBRA handle this duplicate decision.
Fixes: 7dc9d4e4e3 ("bgp may add multiple path entries with the same nexthop")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This commit addresses an issue that happens when using bgp
labeled unicast peering with a rr client, with a received prefix
which is the local ip address of the bgp session.
When using bgp ipv4 labeled session, the local prefix is
received by a peer, and finds out that the proposed prefix
and its next-hop are the same. To avoid a route loop locally,
no nexthop entry is referenced for that prefix, and the route
will not be selected.
As it has been done for ipv4-unicast, apply the following fix
for labeled address families: when the received peer is
a route reflector, the prefix has to be selected, even if the
route can not be installed locally.
Fixes: f874552557 ("bgpd: authorise to select bgp self peer prefix on rr case")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Signed-off-by: Dmytro Shytyi <dmytro.shytyi@6wind.com>
In a BGP L3VPN context using ADJ-RIB-IN (ie. enabled with
'soft-reconfiguration inbound'), after applying a deny route-map and
removing it, the remote MPLS label information is lost. As a result, BGP
is unable to re-install the related routes in the RIB.
For example,
> router bgp 65500
> [..]
> neighbor 192.0.2.2 remote-as 65501
> address-family ipv4 vpn
> neighbor 192.0.2.2 activate
> neighbor 192.0.2.2 soft-reconfiguration inbound
The 192.168.0.0/24 prefix has a remote label value of 102 in the BGP
RIB.
> # show bgp ipv4 vpn 192.168.0.0/24
> BGP routing table entry for 444:1:192.168.0.0/24, version 2
> [..]
> 192.168.0.0 from 192.0.2.2
> Origin incomplete, metric 0, valid, external, best (First path received)
> Extended Community: RT:52:100
> Remote label: 102
A route-map now filter all incoming BGP updates:
> route-map rmap deny 1
> router bgp 65500
> address-family ipv4 vpn
> neighbor 192.0.2.2 route-map rmap in
The prefix is now filtered:
> # show bgp ipv4 vpn 192.168.0.0/24
> #
The route-map is detached:
> router bgp 65500
> address-family ipv4 vpn
> no neighbor 192.168.0.1 route-map rmap in
The BGP RIB entry is present but the remote label is lost:
> # show bgp ipv4 vpn 192.168.0.0/24
> BGP routing table entry for 444:1:192.168.0.0/24, version 2
> [..]
> 192.168.0.0 from 192.0.2.2
> Origin incomplete, metric 0, valid, external, best (First path received)
> Extended Community: RT:52:100
The reason for the loose is that labels are stored within struct attr ->
struct extra -> struct bgp_labels but not in the struct bgp_adj_in.
Reference the bgp_labels pointer in struct bgp_adj_in and use its values
when doing a soft reconfiguration of the BGP table.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
In route_vty_out_detail(), tag_buf stores a string representation of
the VNI label.
Rename tag_buf to vni_buf for clarity and rework the code a little bit
to prepare the following commits.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Add bgp_path_info_labels_same() to compare labels with labels from
path_info. Remove labels_same() that was used for mplsvpn only.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
In bgp_update(), path_info *new has just been created and has void
labels. bgp_labels_same() is always false.
Do not compare previous labels before setting them.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
No need to init labels at extra allocation. num_labels is the number
of set labels in label[] and is initialized to 0 by default.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
The handling of MPLS labels in BGP faces an issue due to the way labels
are stored in memory. They are stored in bgp_path_info but not in
bgp_adj_in and bgp_adj_out structures. As a consequence, some
configuration changes result in losing labels or even a bgpd crash. For
example, when retrieving routes from the Adj-RIB-in table
("soft-reconfiguration inbound" enabled), labels are missing.
bgp_path_info stores the MPLS labels, as shown below:
> struct bgp_path_info {
> struct bgp_path_info_extra *extra;
> [...]
> struct bgp_path_info_extra {
> mpls_label_t label[BGP_MAX_LABELS];
> uint32_t num_labels;
> [...]
To solve those issues, a solution would be to set label data to the
bgp_adj_in and bgp_adj_out structures in addition to the
bgp_path_info_extra structure. The idea is to reference a common label
pointer in all these three structures. And to store the data in a hash
list in order to save memory.
However, an issue in the code prevents us from setting clean data
without a rework. The extra->num_labels field, which is intended to
indicate the number of labels in extra->label[], is not reliably checked
or set. The code often incorrectly assumes that if the extra pointer is
present, then a label must also be present, leading to direct access to
extra->label[] without verifying extra->num_labels. This assumption
usually works because extra->label[0] is set to MPLS_INVALID_LABEL when
a new bgp_path_info_extra is created, but it is technically incorrect.
Cleanup the label code by setting num_labels each time values are set in
extra->label[] and checking extra->num_labels before accessing the
labels.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
As part of backpressure changes, there is a bug where immediate withdraw
is to be sent for evpn imported type-5 prefix to clear the nh neigh and
RMAC entry.
Fixing this by sending withdraw immediately to keep it inline with the
code today
Ticket: #3905571
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>
This fixes the crash:
```
==14759== Invalid read of size 8
==14759== at 0x31032B: bgp_reuselist_del (bgp_damp.c:51)
==14759== by 0x310392: bgp_damp_info_unclaim (bgp_damp.c:69)
==14759== by 0x310CD6: bgp_damp_info_free (bgp_damp.c:387)
==14759== by 0x311016: bgp_reuse_timer (bgp_damp.c:230)
==14759== by 0x4F227CC: thread_call (thread.c:2008)
==14759== by 0x4EDB7D7: frr_run (libfrr.c:1216)
==14759== by 0x1EF748: main (bgp_main.c:525)
==14759== Address 0x48 is not stack'd, malloc'd or (recently) free'd
==14759==
==14759==
==14759== Process terminating with default action of signal 11 (SIGSEGV)
==14759== at 0x59CC7F5: raise (raise.c:46)
==14759== by 0x4F10CEB: core_handler (sigevent.c:261)
==14759== by 0x59CC97F: ??? (in /lib/x86_64-linux-gnu/libpthread-2.27.so)
==14759== by 0x31032A: bgp_reuselist_del (bgp_damp.c:51)
==14759== by 0x310392: bgp_damp_info_unclaim (bgp_damp.c:69)
==14759== by 0x310CD6: bgp_damp_info_free (bgp_damp.c:387)
==14759== by 0x311016: bgp_reuse_timer (bgp_damp.c:230)
==14759== by 0x4F227CC: thread_call (thread.c:2008)
==14759== by 0x4EDB7D7: frr_run (libfrr.c:1216)
==14759== by 0x1EF748: main (bgp_main.c:525)
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
bgp_damp_config, afi and safi are never used.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
This causes a crash using `clear ip bgp dampening <prefix>`.
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Description:
clear ip bgp dampening was not triggering the route
calculation for the prefix, Due to this prefix are not install in
RIB(Zebra) and not adv to neighbor
Problem Description/Summary :
clear ip bgp dampening was not triggering the route
calculation for the prefix, Due to this prefix are not install in
RIB(Zebra) and not adv to neighbor
Fix: When clear ip bgp dampening, route are put for route-calculation as
that it is install in the Zebra and adv to neighbor.
Signed-off-by: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
Changes implement dampening profiles for peers and peer groups. This is
achieved by introducing the possibility to have multible existing
dampening configurations with their own sets of parameters and lists of
associated paths.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Signed-off-by: David Schweizer <dschweizer@opensourcerouting.org>
There is no support for option 8, as per RFC7854.
Add the 64 bit counter in the peer structure.
Add the missing per peer statistic.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When a peer has no IPv6 global address to send as nexthop, it sends the
IPv6 link-local instead as global. "show bgp ipv6 json" displays the
same address in global and link-local scopes.
> "nexthops": [
> {
> "ip": "fe80::a495:38ff:fea6:6ea3",
> "afi": "ipv6",
> "scope": "global",
> "used": true
> },
> {
> "ip": "fe80::a495:38ff:fea6:6ea3",
> "afi": "ipv6",
> "scope": "link-local"
> }
> ]
However, "used" key is set on the global nexthop but not in link-local.
It is correct but it makes difficult to test JSON to expect the usage of
a link-local. The contrary is also correct.
Set "used" key on the link-local nexhop instead to facilitate the tests.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
The usage of the `bgp bestpath med missing-as-worst` command
was being accepted and applied during bestpath, but during output
of the routes affected by this it would not give any indication
that this was happening or what med value was being used.
Fixes: #15718
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Current changes deals with EVPN routes installation to zebra.
In evpn_route_select_install() we invoke evpn_zebra_install/uninstall
which sends zclient_send_message().
This is a continuation of code changes (similar to
ccfe452763) but to handle evpn part
of the code.
Ticket: #3390099
Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>
Currently bgp_path_info's are stored in reverse order
received. Sort them by the best path ordering.
This will allow for optimizations in the future on
how multipath is done.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This will allow a consistency of approach to adding/removing
pi's to from the workqueue for processing as well as properly
handling the dest->info pi list more appropriately.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Add a new flag BGP_PATH_UNSORTED to keep track
of sorted -vs- unsorted path_info's. Add some
ability to the system to understand when that
flag is set.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Add a debug function to allow developers to dump flags
associated with a bgp_path_info in a human readable format.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Currently BGP attempts to send route change information
to it's peers *before* the route is installed into zebra.
This creates a bug in suppress-fib-pending in the following
scenario:
a) bgp suppress-fib-pending and bgp has a route with
2 way ecmp.
b) bgp receives a route withdraw from peer 1. BGP
will send the route to zebra and mark the route as
FIB_INSTALL_PENDING.
c) bgp receives a route withdraw from peer 2. BGP
will see the route has the FIB_INSTALL_PENDING and
not send the withdrawal of the route to the peer.
bgp will then send the route deletion to zebra and
clean up the bgp_path_info's.
At this point BGP is stuck where it has not sent
a route withdrawal to downstream peers.
Let's modify the code in bgp_process_main_one to
send the route notification to zebra first before
attempting to announce the route. The route withdrawal
will remove the FIB_INSTALL_PENDING flag from the dest
and this will allow group_announce_route to believe
it can send the route withdrawal.
For the master branch this is ok because the recent
backpressure commits are in place and nothing is going
to change from an ordering perspective in that regards.
Ostensibly this fix is also for operators of Sonic and
will be backported to the 8.5 branch as well. This will
change the order of the send to peers to be after the
zebra installation but sonic users are using suppress-fib-pending
anyways so updates won't go out until rib ack has been
received anyways.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
BGP is now keeping a list of dests with the dest having a pointer
to the bgp_path_info that it will be working on.
1) When bgp receives a prefix, process it, add the bgp_dest of the
prefix into the new Fifo list if not present, update the flags (Ex:
earlier if the prefix was advertised and now it is a withdrawn),
increment the ref_count and DO NOT advertise the install/withdraw
to zebra yet.
2) Schedule an event to wake up to invoke the new function which will
walk the list one by one and installs/withdraws the routes into zebra.
a) if BUFFER_EMPTY, process the next item on the list
b) if BUFFER_PENDING, bail out and the callback in
zclient_flush_data() will invoke the same function when BUFFER_EMPTY
Changes
- rename old bgp_zebra_announce to bgp_zebra_announce_actual
- rename old bgp_zebra_withdrw to bgp_zebra_withdraw_actual
- Handle new fifo list cleanup in bgp_exit()
- New funcs: bgp_handle_route_announcements_to_zebra() and
bgp_zebra_route_install()
- Define a callback function to invoke
bgp_handle_route_announcements_to_zebra() when BUFFER_EMPTY in
zclient_flush_data()
The current change deals with bgp installing routes via
bgp_process_main_one()
Ticket: #3390099
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>
Since installing/withdrawing routes into zebra is going to be changed
around to be dest based in a list,
- Retrieve the afi/safi to use based upon the dest's afi/safi
instead of passing it in.
- Prefix is known by the dest. Remove this arg as well
Ticket: #3390099
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>
When running `show ip bgp` command, the 'route short status' and
'network' columns do not have white-space between them.
Old show:
Network Next Hop Metric LocPrf Weight Path
*>i1.1.1.1/32 10.1.12.111 0 100 0 i
New show:
Network Next Hop Metric LocPrf Weight Path
*>i 1.1.1.1/32 10.1.12.111 0 100 0 i
Added white-space to enhance readability between them.
Signed-off-by: Cassiano Campes <cassiano.campes@venkonetworks.com>
currently:
when as-path-loop-detection is set on a peer-group.
members of the peer-group are not using that functionnality.
analysis:
the as-path-loop-detection, is not using the peer's flags
related framework.
fix:
use the peer's flag framework for as-path-loop-detection.
Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
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>
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>
This for loop has no chance of removing entries so there is no
need to do a bit of complicated code to handle the case where
an entry can be removed.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Move mp_nexthop_prefer_global boolean attribute to nh_flags. It does
not currently save memory because of the packing.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Locally leaked routes remain active after the nexthop VRF interface goes
down.
Update route leaking when the loopback or a VRF interface state change is
received from zebra.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
When a BGP flowspec peering stops, the BGP RIB entries for IPv6
flowspec entries are removed, but not the ZEBRA RIB IPv6 entries.
Actually, when calling bgp_zebra_withdraw() function call, only
the AFI_IP parameter is passed to the bgp_pbr_update_entry() function
in charge of the Flowspec add/delete in zebra. Fix this by passing
the AFI parameter to the bgp_zebra_withdraw() function.
Note that using topotest does not show up the problem as the
flowspec driver code is not present and was refused. Without that,
routes are not installed, and can not be uninstalled.
Fixes: 529efa2346 ("bgpd: allow flowspec entries to be announced to zebra")
Link: https://github.com/FRRouting/frr/pull/2025
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
By default, iBGP and eBGP-OAD peers exchange RPKI extended community by default.
Add a command to disable sending RPKI extended community if needed.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Structure size of bgp_path_info_extra when compiled
with vnc is 184 bytes. Reduce this size to 72 bytes
when compiled w/ vnc but not necessarily turned
on vnc.
With 2 full bgp feeds this saves aproximately 100mb
when compiling with vnc and not using vnc.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Without this change when we change the route-map, we never reinstall the route
if the route-map has changed.
We checked only some attributes like aspath, communities, large-communities,
extended-communities, but ignoring the rest of attributes.
With this change, let's check if the route-map has changed.
bgp_route_map_process_update() is triggered on route-map change, and we set
`changed` to true, which treats aggregated route as not the same as it was before.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Comparing pointers is not the appropriate way to know
if the label values are the same or not. Perform a
memcmp call instead is better.
Fixes: 8ba7105057 ("bgpd: fix valgrind flagged errors")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
