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>
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>
