Commit Graph

1030 Commits

Author SHA1 Message Date
Donatas Abraitis
4c60e50f7f
Merge pull request #6191 from NaveenThanikachalam/ibgp_connected
bgpd: Enforce self-next-hop check in next-hop update.
2020-04-21 23:16:03 +03:00
Donatas Abraitis
0355b41d84 bgpd: Do not discard an UPDATE if the global nexthop is set to ::
When we receive an UPDATE with MP_NEXTHOP len as 32 bytes, we shouldn't
check if the global (1st) nexthop is unspecified.

Peering between bird and FRRouting we receive from Bird something like:
```
rcvd UPDATE w/ attr: , origin i, mp_nexthop ::(fe80::a00:27ff:fe09:f8a3)
```
The link-local (2nd) nexthop is valid and validated later in the code.

Before it was marked:
```
IPv6 unicast -- DENIED due to: martian or self next-hop;
```

After it's a valid prefix:
```
spine1-debian-9# show bgp
BGP table version is 0, local router ID is 2.2.2.2, vrf id 0
Default local pref 100, local AS 65002
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete

   Network          Next Hop            Metric LocPrf Weight Path
   2a02:4780::/64   fe80::a00:27ff:fe09:f8a3
                                                           0 65001 i

Displayed  1 routes and 1 total paths
```

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-04-20 18:59:15 +03:00
Donatas Abraitis
7f972cd8dc bgpd: Use true/false for reject_as_sets
Just remove MACROS and use true/false.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-04-20 12:59:52 +03:00
Donald Sharp
8b1e4f30ba
Merge pull request #6164 from ton31337/feature/rfc8212_enabled_traditional_profile
bgpd: Enable rfc8212 by default except datacenter profile
2020-04-18 15:06:04 -04:00
Donatas Abraitis
eb91f8d6d7 bgpd: Add a sanitify check for bgp_nexthop_cache against NULL
In real world sometimes happens that bgp_nexthop_cache is NULL. Avoid
segfaulting when using `show [ip] bgp ...` CLI commands.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-04-16 16:13:01 +03:00
Donatas Abraitis
2ba93fd65b bgpd: Show hostname in show [ip] bgp ... only if nexthop is connected
The problem is when using kinda such topologies:
(192.168.1.1/32) r1 <-- eBGP --> r2 <-- iBGP --> r3

Looking at r3's nexthop for 192.168.1.1/32 we have it as r2, but really
it MUST be r1.

Checking if the nexthop is connected solves the problem even for cases
when route-reflectors are used.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-04-16 10:36:59 +03:00
Donald Sharp
b9ba7ed533
Merge pull request #5812 from pguibert6WIND/bgp_stats_all
Bgp stats all
2020-04-14 14:36:21 -04:00
Donatas Abraitis
1d3fdccfe1 bgpd: Enable rfc8212 by default except datacenter profile
Some competitive vendors like Cisco, Bird, OpenBGPD,
Nokia already have this by default enabled.

The list is here: https://github.com/bgp/RFC8212

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-04-14 16:01:46 +03:00
Donatas Abraitis
2dbe3fa97b bgpd: Replace 0 to false for bool assignment in bgp_update_martian_nexthop()
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-04-13 20:39:31 +03:00
Mark Stapp
f9dfa64797
Merge pull request #6209 from donaldsharp/true_false
bgpd: bools use `true/false` not `TRUE/FALSE`
2020-04-13 12:17:19 -04:00
Donald Sharp
cded3b7232 bgpd: bools use true/false not TRUE/FALSE
Who knows where these values were coming from.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
2020-04-13 08:08:48 -04:00
Naveen Thanikachalam
e7cbe5e599 bgpd: Force self-next-hop check in next-hop update.
Problem Description:
=====================
+--+                                            +--+
|R1|-(192.201.202.1)----iBGP----(192.201.202.2)-|R2|
+--+                                            +--+

Routes on R2:
=============
S>* 202.202.202.202/32 [1/0] via 192.201.78.1, ens256, 00:40:48
Where, the next-hop network, 192.201.78.0/24, is a directly connected network address.
C>* 192.201.78.0/24 is directly connected, ens256, 00:40:48

Configurations on R1:
=====================
!
router bgp 201
 bgp router-id 192.168.0.1
 neighbor 192.201.202.2 remote-as 201
!

Configurations on R2:
=====================
!
ip route 202.202.202.202/32 192.201.78.1
!
router bgp 201
 bgp router-id 192.168.0.2
 neighbor 192.201.202.1 remote-as 201
 !
 address-family ipv4 unicast
  redistribute static
 exit-address-family
!

Step-1:
=======
R1 receives the route 202.202.202.202/32 from R2.
R1 installs the route in its BGP RIB.

Step-2:
=======
On R1, a connected interface address is added.
The address is the same as the next-hop of the BGP route received from R2 (192.201.78.1).

Point of Failure:
=================
R1 resolves the BGP route even though the route's next-hop is its own connected address.
Even though this appears to be a misconfiguration it would still be better to safeguard the code against it.

Fix:
====
When BGP receives a connected route from Zebra, it processes the
routes for the next-hop update.
While doing so, BGP must ignore routes whose next-hop address matches
the address of the connected route for which Zebra sent the next-hop update
message.

Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>
2020-04-11 07:26:33 -07:00
Quentin Young
293d5cb3c0
Merge pull request #6176 from NaveenThanikachalam/memleaks
bgpd: Fixes for memory leaks.
2020-04-10 13:55:52 -04:00
Naveen Thanikachalam
74a630b606 bgpd: Fixes for memory leaks.
This commit addresses the memory leaks when certain BGP JSON
show commands are executed

Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>
2020-04-08 20:27:49 -07:00
vivek
3b0c17e1d4 bgpd: Trigger EVPN type-5 injection upon link-bandwidth change
Ensure that upon a link-bandwidth change - for e.g., due to change in
the number of multipaths - EVPN type-5 route injection is triggered.
In the absence of this, the proper link-bandwidth is not updated in
EVPN type-5 routes originated by the router.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
2020-04-08 19:12:09 -07:00
Donatas Abraitis
c4efd0f423 *: Do not cast to the same type
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-04-08 17:15:06 +03:00
Philippe Guibert
9ab0cf5830 bgpd: take into account code style recommendations.
take into account polychaeta tips ono code style.
also, take into account miscellaneous code style recommandations like
braces usage.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2020-04-08 08:56:52 +02:00
Sri Mohana Singamsetty
70ecc066e7
Merge pull request #6105 from vivek-cumulus/bgp_link_bandwidth_unequal_cost_multipath
Unequal cost multipath (a.ka. weighted ECMP) with BGP link-bandwidth
2020-04-05 11:41:42 -07:00
Sri Mohana Singamsetty
dba3453515
Merge pull request #6130 from ton31337/fix/remove_some_redundant_attributes_from_json
bgpd: Remove deprecated JSON fields for `show bgp ... json`
2020-04-02 16:17:24 -07:00
Quentin Young
49e5a4a0b8 bgpd: #if ENABLE_BGP_VNC -> #ifdef ENABLE_BGP_VNC
This macro is undefined if vnc is disabled, and while it defaults to 0,
this is still wrong and causes issues with -Werror

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
2020-04-01 15:05:26 -04:00
Donatas Abraitis
0fbac0b478 bgpd: Remove deprecated JSON fields for show bgp ... json
med --------> metric
localPref --> locPrf
aspath -----> path

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-04-01 17:02:30 +03:00
Philippe Guibert
1471864374 bgpd: add show bgp l2vpn evpn statistics [json] support
add show bgp l2vpn evpn statistics [json] support.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2020-03-31 14:38:15 +02:00
Philippe Guibert
6c9d22e223 bgpd: review the hierarchy for bgp statistics in json format
- each statistics is encapsulated into concatenated "<afi><safi>" value.
- the json encoding for floating and double values is using json api
double api. this change is done for bgp statistics.
- the lines over 80 characters have been handled.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2020-03-31 14:38:15 +02:00
Philippe Guibert
4265b26111 bgpd: new vty command to dump all bgp per vrf statistics
this command is a shortcut to facilitate the extraction of statistics
for all afi/safi related to one bgp instance.
the command is: show bgp [vrf XX] statistics-all [json]

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2020-03-31 14:38:15 +02:00
Philippe Guibert
b9f4d96f23 bgpd: permit to get statistics for other bgp safis
safis that use a route distinguisher in bgp tables, and as such
introduce a two level hierarchy on the bgp table, must be made available
to statistics too.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2020-03-31 14:38:15 +02:00
Philippe Guibert
893cccd057 bgpd: add json support for show bgp statistics command
add json support for show bgp statistics command.
The title of the stats entry is aggregated without spaces.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2020-03-31 14:38:15 +02:00
vivek
f7e1c681f4 bgpd: Implement options for link bandwidth handling
Support configurable options to control how link bandwidth is handled
by the receiver. The default behavior is to automatically honor the
link bandwidths received and use it to perform a weighted ECMP BUT only
if all paths in the multipath have associated link bandwidth; if one or
more paths do not have link bandwidth, normal ECMP is performed among
the multipaths. This behavior is as recommended by
https://tools.ietf.org/html/draft-ietf-idr-link-bandwidth.

The additional options available are to (a) completely ignore any link
bandwidth (i.e., weighted ECMP is effectively disabled), (b) skip paths
in the multipath which do not have link bandwidth and perform weighted
ECMP among the other paths (if at least some paths have the bandwidth)
or (c) use a default weight (value chosen is 1) for the paths which
do not have link bandwidth.

The command syntax is
bgp bestpath bandwidth <ignore|skip-missing|default-weight-for-missing>

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
2020-03-30 20:12:31 -07:00
vivek
7b651a321e bgpd: Announce cumulative link bandwidth to EBGP peers
When announcing ourselves as the next hop (e.g., to EBGP peers), if the
best path has the link bandwidth extended community and it is transitive,
change the value of the link bandwidth to the cumulative downstream
bandwidth (sum of the link bandwidths of all our multipaths) as this
makes the most sense. It is also implied by
https://tools.ietf.org/html/draft-mohanty-bess-ebgp-dmz. Of course, do
not override the link bandwidth if it has been specified by policy.

Note: Transitive extended communities will be automatically passed along
to EBGP peers; this commit is updating the value that is announced to
something that is the most appropriate.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by:   Don Slice <dslice@cumulusnetworks.com>
2020-03-30 20:12:31 -07:00
vivek
b1875e656c bgpd: Additional options for generating link bandwidth
Implement the code to handle the other route-map options to generate
the link bandwidth, namely, to use the cumulative bandwidth or to
base this on the number of multipaths. In the latter case, a reference
bandwidth is internally chosen - the implementation uses a value of
1 Gbps.

These additional options mean that the prefix may need to be advertised
if there is a link bandwidth change, which is a new criteria. Define a
new path (change) flag to support this and implement the advertisement.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by:   Don Slice <dslice@cumulusnetworks.com>
2020-03-30 20:12:31 -07:00
Sri Mohana Singamsetty
0298bb01bb
Merge pull request #6085 from donaldsharp/bgp_node_get_prefix
Bgp node get prefix
2020-03-26 19:07:36 -07:00
Donald Sharp
b54892e0ea bgpd: Convert users of rn->p to use accessor function
Add new function `bgp_node_get_prefix()` and modify
the bgp code base to use it.

This is prep work for the struct bgp_dest rework.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
2020-03-26 16:25:16 -04:00
Donald Sharp
5f040085ba lib, bgpd: Another round of struct const prefix cleanup
Cleanup another set of functions that need to respect the
const'ness of a prefix.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
2020-03-26 16:22:00 -04:00
Donatas Abraitis
87c8213108 bgpd: Show that prefix is malformed if aggregated by 0
Show if this malformed under `show [ip] bgp <prefix>`:
 ```
eva# sh ip bgp 103.79.124.0/22
BGP routing table entry for 103.79.124.0/22
Paths: (1 available, best #1, table default)
  Advertised to non peer-group peers:
  192.168.201.136
  64539 15096 6939 7545 7545 136001, (aggregated by 0(malformed) 0.0.0.0)
    192.168.201.136 from 192.168.201.136 (192.168.201.136)
      Origin IGP, valid, external, best (First path received)
      Last update: Thu Mar 26 10:02:07 2020
```

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-03-26 16:06:34 +02:00
Donald Sharp
42984e1bd4
Merge pull request #6087 from opensourcerouting/log-kill-tabs
*: remove tabs and linefeeds from log messages
2020-03-25 06:30:38 -04:00
David Lamparter
63efca0e95 *: remove line breaks from log messages
Line break at the end of the message is implicit for zlog_* and flog_*,
don't put it in the string.  Mid-message line breaks are currently
unsupported.  (LF is "end of message" in syslog.)

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
2020-03-24 19:43:18 +01:00
Donald Sharp
26a3ffd60e bgpd, lib, ripngd: Add agg_node_get_prefix
Modify code to use lookup function agg_node_get_prefix()
as the abstraction layer.  When we rework bgp_node to
bgp_dest this will allow us to greatly limit the amount
of work needed to do that.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
2020-03-24 07:51:41 -04:00
Donald Sharp
5a1ae2c237 bgpd: Rework code to use const struct prefix
Future work needs the ability to specify a
const struct prefix value.  Iterate into
bgp a bit to get this started.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
2020-03-24 07:51:41 -04:00
Donald Sharp
cb9f254c01 bgpd: Make bgp_debug_bestpath take a struct bgp_node
Defer the grabbing of the prefix for as long as is possible.
This is a long term rework of how we access the `struct bgp_node`
to only use accessor functions.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
2020-03-24 07:33:13 -04:00
Sri Mohana Singamsetty
865a8f8611
Merge pull request #6073 from donaldsharp/is_default
More `const struct prefix` work
2020-03-23 10:54:33 -07:00
Donald Sharp
b8685f9bea bgpd: Add some const struct prefix for a couple more functions
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
2020-03-23 08:10:55 -04:00
Donald Sharp
bd494ec5ed bgpd: More const struct prefix work
Modify more code to use `const struct prefix` throughout
bgp.  This is all prep work for adding an accessor function
for bgp_node to get the prefix and reduce all the places that
code needs to be touched when we get that work done.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
2020-03-22 14:50:46 -04:00
Donatas Abraitis
3dc339cdc2 bgpd: Convert lots of int type functions to bool/void
Some were converted to bool, where true/false status is needed.
Converted to void only those, where the return status was only false or true.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-03-21 14:59:18 +02:00
vivek
e34291b86a bgpd: Allow EVPN advertise route-map to modify attributes
Ensure that the EVPN advertise route-map is applied on a copy of the
original path_info and associated attribute, so that if the route-map
has SET clauses, they can operate properly. This closely follows
the model already in use in other route-map application code.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by:   Don Slice <dslice@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
2020-03-19 14:21:23 -07:00
vivek
d69a76ac1a bgpd: Reverse route-map check for consistency
Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
2020-03-19 14:21:23 -07:00
Donatas Abraitis
5910f7f1b0
Merge pull request #6022 from vivek-cumulus/refine_multiaccess_check
bgpd: Refine multiaccess check for next hop resetting
2020-03-18 10:47:27 +02:00
Donatas Abraitis
974ac286f1
Merge pull request #6013 from donaldsharp/bgp_reason_it
bgpd: Fix certain code paths that reset reason code
2020-03-18 10:37:02 +02:00
vivek
a3b7253990 bgpd: Refine multiaccess check for next hop resetting
A BGP update-group is dynamically created to group together a set of peers
such that any BGP updates can be formed just once for the entire group and
only the next hop attribute may need to be modified when the update is sent
out to each peer in the group. The update formation code attempts to
determine as much as possible if the next hop will be set to our own IP
address for every peer in the group. This helps to avoid additional checks
at the point of sending the update (which happens on a per-peer basis) and
also because some other attributes may/could vary depending on whether the
next hop is set to our own IP or not. Resetting the next hop to our own IP
address is the most common behavior for EBGP peerings in the absence of
other user-configured or internal (e.g., for l2vpn/evpn) settings and
peerings on a shared subnet.

The code had a flaw in the multiaccess check to see if there are peers in
the update group which are on a shared subnet as the next hop of the path
being announced - the source peer could itself be in the same update group
and cause the check to give an incorrect result. Modify the check to skip
the source peer so that the check is more accurate.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by:   Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by:   Don Slice <dslice@cumulusnetworks.com>
2020-03-17 19:59:52 -07:00
Donald Sharp
19ea4cec4e bgpd: Fix certain code paths that reset reason code
The bgp reason code was being reset in bgp_best_selection
by rerunning bgp_path_info_cmp multiple times under certain
receiving patterns of data from peers.

This is the debugs that show this issue:
2020/03/16 19:17:22.523780 BGP: 2001:20:1:1::6 rcvd UPDATE w/ attr: nexthop 20.1.1.6, origin i, metric 600, community 1000:1006, path 20
2020/03/16 19:17:22.523819 BGP: 2001:20:1:1::6 rcvd 20.10.0.6/32 IPv4 unicast
2020/03/16 19:17:22.556168 BGP: 20.1.1.6 rcvd UPDATE w/ attr: nexthop 20.1.1.6, origin i, metric 500, community 1000:1006, path 20
2020/03/16 19:17:22.556209 BGP: 20.1.1.6 rcvd 20.10.0.6/32 IPv4 unicast
2020/03/16 19:17:22.572358 BGP: bgp_process_main_one: p=20.10.0.6/32 afi=IPv4, safi=unicast start
2020/03/16 19:17:22.572408 BGP: 20.10.0.6/32: Comparing path 2001:20:1:1::6 flags 0x410 with path 20.1.1.6 flags 0x410
2020/03/16 19:17:22.572415 BGP: 20.10.0.6/32: path 2001:20:1:1::6 loses to path 20.1.1.6 due to MED 600 > 500
2020/03/16 19:17:22.572422 BGP: 20.10.0.6/32: path 20.1.1.6 is the bestpath from AS 20
2020/03/16 19:17:22.572429 BGP: 20.10.0.6/32: path 20.1.1.6 is the initial bestpath
2020/03/16 19:17:22.572435 BGP: bgp_best_selection: pi 0x5627187c66c0 dmed
2020/03/16 19:17:22.572441 BGP: 20.10.0.6/32: After path selection, newbest is path 20.1.1.6 oldbest was NONE
2020/03/16 19:17:22.572447 BGP: 20.10.0.6/32: path 20.1.1.6 is the bestpath, add to the multipath list
2020/03/16 19:17:22.572453 BGP: 20.10.0.6/32: path 2001:20:1:1::6 has the same nexthop as the bestpath, skip it
2020/03/16 19:17:22.572460 BGP: 20.10.0.6/32: starting mpath update, newbest 20.1.1.6 num candidates 1 old-mpath-count 0 old-cum-bw u0
2020/03/16 19:17:22.572466 BGP: 20.10.0.6/32: comparing candidate 20.1.1.6 with existing mpath NONE
2020/03/16 19:17:22.572473 BGP: 20.10.0.6/32: New mpath count (incl newbest) 1 mpath-change NO all_paths_lb 0 cum_bw u0

Effectively if BGP receives 2 paths it could end up running bgp_path_info_cmp multiple times
and in some situations overwrite the reason selected the first time through.

In this example path selection is run and the MED is the reason for the choice.
Then in bgp_best_selection is run again this time clearing new_select
to NULL before calling path selection for the first time. This second
call into path selection resets the reason, since it is only passing in one
path.  So save the last reason selected and restore in this case.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
2020-03-17 15:48:17 -04:00
Russ White
047315df42
Merge pull request #5954 from ton31337/feature/rfc7607
bgpd: Proscribe the use of AS 0 (zero)
2020-03-17 10:27:35 -04:00
Donatas Abraitis
33d022bcf6 bgpd: Proscribe the use of AS 0 (zero)
Implements https://tools.ietf.org/html/rfc7607

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-03-17 13:31:23 +02:00