Commit Graph

5612 Commits

Author SHA1 Message Date
Louis Scalbert
80444d30ce bgpd: fix peer-group with maximum-prefix-out
When setting maximum-prefix-out on peer-group, the applied value on
member is 0.

Fix usage of maximum-prefix-out on peer-group.

The peer_maximum_prefix_out_(un)set functions are derived from
peer_maximum_prefix_(un)set.

Fixes: fde246e835 ("bgpd: Add an option to limit outgoing prefixes")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
2022-01-26 16:49:31 +01:00
Louis Scalbert
bc03c622e1 bgpd: allow no neighbor X.X.X.X maximum-prefix-out [(1-4294967295)]
Specifying a number is not possible with command no neighbor X.X.X.X
maximum-prefix-out

> frr(config-router-af)# no neighbor 192.168.1.2 maximum-prefix-out 1
> % Unknown command: no neighbor 192.168.1.2 maximum-prefix-out 1

This patch allows it.

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
2022-01-26 16:38:14 +01:00
Louis Scalbert
d0bf49ecd5 bgpd: apply maximum-prefix-out without clearing the neighbor
Abstract:
- The command "neighbor PEER maximum-prefix-out NUMBER" cannot be applied
  without clearing the BGP neighbor.
- Apply the maximum-prefix-out value as soon as it is modified without
  clearing the neighbor.

subgroup_update_packet() and subgroup_withdraw_packet() respectively
manages the announcement and withdrawal BGP message to the peer.
subgrp->scount counter counts the number of sent prefixes.

Before the patch, the maximum out prefix limitation was applied in
subgroup_update_packet() in order that subgrp->scount never exceeds the
limit. Setting a limit inferior to the effective number of sent prefix
did not result in sending any withdrawal message to reduce the number of
sent prefixes. Without clearing the BGP neighbor, the limitation only
applied to the announcement of new prefixes when the limitation was
over.

With the patch, the limitation is checked in subgroup_announce_check().
The function is intended to say whether a prefix has to be announced in
regards to the prefix-list, route-map... Now when a maximum-prefix-out
value is changed/removed, the neighbor AFI/SAFI table is re-parsed in
the same way as for the application of route-map, prefix-lists...

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
2022-01-20 18:19:37 +01:00
Louis Scalbert
e02672605b bgpd: fix calculation of update-group hash with maximum-prefix-out
Take into account the maximum-prefix-out value when calculating the
update-group hash.

Fixes: fde246e835 ("bgpd: Add an option to limit outgoing prefixes")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
2022-01-20 18:19:37 +01:00
Rafael Zalamena
4e4c027803
Merge pull request #10183 from idryzhov/rework-vrf-rename
*: rework renaming the default VRF
2022-01-17 08:45:12 -03:00
Trey Aspelund
c1984955b7 bgpd: fix advertisedRoutes json key
'show bgp ... neighbor [routes|received-routes]' both incorrectly
used a json key of 'advertisedRoutes'.
This corrects the key to be 'receivedRoutes' for commands where
the displayed routes were received, not advertised.

before:
unet> r3 show ip bgp neigh 10.2.30.2 received-routes json | include Routes
  "advertisedRoutes":{

after:
ub18# show ip bgp neighbors enp1s0 received-routes json | include Routes
  "receivedRoutes":{
ub18# show ip bgp neighbors enp1s0 advertised-routes json | include Routes
  "advertisedRoutes":{

Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
2022-01-14 22:03:11 +00:00
Donald Sharp
25c44cf5de
Merge pull request #10339 from opensourcerouting/printfrr-20220114
lib: printfrr shenanigans
2022-01-14 14:15:03 -05:00
Donald Sharp
cce7c33396
Merge pull request #10335 from ton31337/fix/reduce_nesting_show_neighbor_bgp
bgpd: Reduce nesting for bgp_show_peer()
2022-01-14 08:11:48 -05:00
David Lamparter
54929fd38a *: use semicolon after printfrr_ext_autoreg_{p,d}
Mostly to make clang-format not format these to peak ugly.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
2022-01-14 13:33:57 +01:00
Igor Ryzhov
18461891d3
Merge pull request #10327 from ton31337/fix/reduce_nested_loops
*: Add FOREACH_AFI_SAFI_NSF(afi, safi) macro to reduce nesting
2022-01-13 20:09:50 +03:00
Donatas Abraitis
107115632a bgpd: Reduce nesting for bgp_show_peer()
It's hard to read sometimes or even add something more.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-13 17:23:03 +02:00
Donatas Abraitis
df8d723c5f *: Add FOREACH_AFI_SAFI_NSF(afi, safi) macro to reduce nesting
Used for graceful-restart mostly.

Especially for bgp_show_neighbor_graceful_restart_capability_per_afi_safi()

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-13 14:29:54 +02:00
Donatas Abraitis
48ebba0476 bgpd: Show Long-lived Graceful Restart timer remaining per prefix
```
exit1-debian-11# sh ip bgp 100.100.100.100/32
BGP routing table entry for 100.100.100.100/32, version 7
Paths: (2 available, best #2, table default)
  Advertised to non peer-group peers:
  home-spine1.donatas.net(192.168.0.2)
  65002, (stale)
    192.168.10.17 from donatas-pc(192.168.10.17) (0.0.0.0)
      Origin incomplete, valid, external
      Community: llgr-stale
      Last update: Thu Jan 13 08:58:08 2022
      Time until Long-lived stale route deleted: 18
  65001
    192.168.0.2 from home-spine1.donatas.net(192.168.0.2) (2.2.2.2)
      Origin incomplete, metric 0, valid, external, best (First path received)
      Last update: Thu Jan 13 08:57:56 2022
```

```
~# vtysh -c 'show ip bgp 100.100.100.100/32 json' | jq '."paths"[] | ."llgrSecondsRemaining"'
17
```

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-13 12:19:32 +02:00
Donatas Abraitis
8ac66010c5 bgpd: Avoid additional check for json output under show ip bgp route
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-13 10:34:04 +02:00
Igor Ryzhov
379effbf70
Merge pull request #10328 from ton31337/fix/vty_json 2022-01-13 09:45:57 +03:00
Donatas Abraitis
83fc30745d
Merge pull request #10266 from opensourcerouting/bgp-aggr-rm
bgpd: fix aggregate route unsuppression bug
2022-01-12 23:18:58 +02:00
Donatas Abraitis
66964cf6bf bgpd: Use vty_json() for bgp_print_dampening_parameters()
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-12 22:50:26 +02:00
Donatas Abraitis
4be03f305b
Merge pull request #10325 from donaldsharp/peer_conditional_adv_cleanup
bgpd: Remove unneeded loop over all peers
2022-01-12 22:15:49 +02:00
Donald Sharp
52979c3baa bgpd: Remove unneeded loop over all peers
The bgp_notify_conditional_adv_scanner function was/is looping
over all peers.  And only matching on the passed in peer,
based upon the subgroup.  As such we do not need to loop
over everything and just cut-to-the chase and just modify
the peer structure.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2022-01-12 09:53:15 -05:00
Igor Ryzhov
4d33086cad
Merge pull request #10317 from ton31337/fix/shutdown_msg
bgpd: Make sure we are playing with non-NULL for bgp shutdown message
2022-01-12 14:36:58 +03:00
Rafael Zalamena
92b175bd40 bgpd: fix aggregate route unsuppression bug
Unsuppress route part of the aggregation when route-map configuration
is removed before the aggregation itself.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
2022-01-11 13:44:54 -03:00
Russ White
ab6bff727a
Merge pull request #10235 from proelbtn/srv6-soft-reconf
SRv6 routes aren't inserted into data plane correctly with soft-reconfiguration
2022-01-11 11:03:34 -05:00
Donatas Abraitis
ed284e2338 bgpd: Make sure we are playing with non-NULL for bgp shutdown message
```
*** CID 1510738:  Null pointer dereferences  (FORWARD_NULL)
/bgpd/bgp_vty.c: 4243 in bgp_shutdown_msg_magic()
4237            if (msgstr && strlen(msgstr) > BGP_ADMIN_SHUTDOWN_MSG_LEN) {
4238                    vty_out(vty, "%% Shutdown message size exceeded %d\n",
4239                            BGP_ADMIN_SHUTDOWN_MSG_LEN);
4240                    return CMD_WARNING_CONFIG_FAILED;
4241            }
4242
>>>     CID 1510738:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing null pointer "msgstr" to "bgp_shutdown_enable", which dereferences it.
4243            bgp_shutdown_enable(bgp, msgstr);
4244            XFREE(MTYPE_TMP, msgstr);
4245
4246            return CMD_SUCCESS;
4247     }
4248
```

```
*** CID 1510737:  Null pointer dereferences  (REVERSE_INULL)
/bgpd/bgpd.c: 4344 in bgp_shutdown_enable()
4338                    /* continue, if peer is already in administrative shutdown. */
4339                    if (CHECK_FLAG(peer->flags, PEER_FLAG_SHUTDOWN))
4340                            continue;
4341
4342                    /* send a RFC 4486 notification message if necessary */
4343                    if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) {
>>>     CID 1510737:  Null pointer dereferences  (REVERSE_INULL)
>>>     Null-checking "msg" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
4344                            if (msg)
4345                                    bgp_notify_send_with_data(
4346                                            peer, BGP_NOTIFY_CEASE,
4347                                            BGP_NOTIFY_CEASE_ADMIN_SHUTDOWN, data,
4348                                            datalen + 1);
4349                            else
```

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-11 17:07:19 +02:00
Kantesh Mundaragi
641065d4fc bgpd: VRF-Lite fix to clear stale leaked routes
Description:
Change is intended for fixing the issue related to
clearing of stale leaked routes:
- Whenever BGP goes down,
  after bringing down tcp connection and renegotiating capabilities,
  once we reestablish connection,
  we are not handling clear of VRF leaked route in the bgp_clear_stale_route.

- While bgp is clearing stale routes,
  we need to handle withdraw of routes for VRF route-leaking.

Co-authored-by: Kantesh Mundaragi <kmundaragi@vmware.com>
Signed-off-by: Iqra Siddiqui <imujeebsiddi@vmware.com>
2022-01-08 10:21:10 -08:00
Donald Sharp
0edd8e4714
Merge pull request #10288 from ton31337/fix/rfc7300
bgpd: Add a warning if we try to use the last reserved ASN
2022-01-08 08:42:19 -05:00
Donatas Abraitis
202a171144 bgpd: Use correct encoding before printing shutdown msg
Using `bgp shutdown message MSG...`.

Length should be decoded from the first byte, but it's decoded from the data
instead.

Before:

```
%NOTIFICATION: sent to neighbor 192.168.0.2 6/2 (Cease/Administratively Shutdown) 70 bytes 5b 54 49 43 4b 45 54 2d 31 2d 31 34 33 38 33 36 37 33 39 30 5d 20 73
```

After:

```
%NOTIFICATION: sent to neighbor 192.168.0.2 6/2 (Cease/Administratively Shutdown) "[TICKET-1-1438367390] software upgrade; Expected downtime for 2 hours;"
```

On receiving side:

```
"[TICKET-1-1438367390] software upgrade; Expected downtime for 2 hours;"
```

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-07 22:35:38 +02:00
Donatas Abraitis
b776f48c36 bgpd: Limit shutdown message size to max 255 characters
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-07 22:35:38 +02:00
Donatas Abraitis
9f33eea39a bgpd: Increase administrative shutdown message size to 255
Extended BGP Administrative Shutdown Communication (rfc9003):

Basically, shutdown message size is increased to 255 from 128.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-07 22:35:38 +02:00
Donatas Abraitis
cc413e2ade bgpd: Add a warning if we try to use the last reserved ASN
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-07 22:35:04 +02:00
Donatas Abraitis
812a20dc57 bgpd: Deprecate DPA, ADVERTISER and RCID_PATH path attributes
rfc6938

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-06 17:10:31 +02:00
Donald Sharp
3d162a6950
Merge pull request #10284 from ton31337/fix/adjust_rfc4486
bgpd: Adjust symbolic names for cease notifications according to rfc4486
2022-01-06 07:49:00 -05:00
Donald Sharp
7747e99cf4
Merge pull request #10287 from ton31337/fix/rfc7196
bgpd: Increase maximum supress threshold for dampening to 50,000
2022-01-06 07:34:45 -05:00
Donatas Abraitis
dcbebfd3ff bgpd: Graceful Restart restart-time can be 0
Using with LLGR, this should be allowed setting GR restart-time timer to 0,
to immediately start LLGR timers.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-06 11:24:48 +02:00
Donatas Abraitis
a30fec23f8 bgpd: Increase maximum supress threshold for dampening to 50,000
rfc7196 recommends:

In addition, BGP implementations have an internal constant, which we
   will call the 'maximum penalty', and the current computed penalty may
   not exceed it.

Router Maximum Penalty:  The internal constant for the maximum
      penalty value MUST be raised to at least 50,000.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-06 10:09:05 +02:00
Donatas Abraitis
0ac7452334 bgpd: Adjust symbolic names for cease notifications according to rfc4486
The following subcodes are defined for the Cease NOTIFICATION
   message:

      Subcode     Symbolic Name

         1        Maximum Number of Prefixes Reached
         2        Administrative Shutdown
         3        Peer De-configured
         4        Administrative Reset
         5        Connection Rejected
         6        Other Configuration Change
         7        Connection Collision Resolution
         8        Out of Resources

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2022-01-06 10:07:41 +02:00
Donatas Abraitis
7f8a9a24a9 bgpd: Change default long-lived graceful restart stale timer to 0 seconds
That means the feature is off by default.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2021-12-28 16:08:00 +02:00
Donatas Abraitis
1479ed2fb3 bgpd: Implement LLGR helper mode
Tested between GoBGP and FRR (this commit).

```
┌───────────┐             ┌────────────┐
│           │             │            │
│ GoBGPD    │             │ FRRouting  │
│ (restart) │             │            │
│           │             │            │
└──────┬────┘             └───────┬────┘
       │                          │
       │                          │
       │                          │
       │     ┌───────────┐        │
       │     │           │        │
       │     │           │        │
       └─────┤ FRRouting ├────────┘
             │ (helper)  │
             │           │
             └───────────┘

// GoBGPD
% cat /etc/gobgp/config.toml
[global.config]
    as = 65002
    router-id = "2.2.2.2"
    port = 179

[[neighbors]]
    [neighbors.config]
        peer-as = 65001
        neighbor-address = "2a02🔤:123"
    [neighbors.graceful-restart.config]
        enabled = true
        restart-time = 3
        long-lived-enabled = true
    [[neighbors.afi-safis]]
        [neighbors.afi-safis.config]
            afi-safi-name = "ipv6-unicast"
        [neighbors.afi-safis.mp-graceful-restart.config]
            enabled = true
        [neighbors.afi-safis.long-lived-graceful-restart.config]
            enabled = true
            restart-time = 10
    [[neighbors.afi-safis]]
        [neighbors.afi-safis.config]
            afi-safi-name = "ipv4-unicast"
        [neighbors.afi-safis.mp-graceful-restart.config]
            enabled = true
        [neighbors.afi-safis.long-lived-graceful-restart.config]
            enabled = true
            restart-time = 20

% ./gobgp global rib add -a ipv6 2001:db8:4::/64
% ./gobgp global rib add -a ipv6 2001:db8:5::/64 community 65535:7
% ./gobgp global rib add -a ipv4 100.100.100.100/32
% ./gobgp global rib add -a ipv4 100.100.100.200/32 community 65535:7
```

1. When killing GoBGPD, graceful restart timer starts in FRR helper router;
2. When GR timer expires in helper router:
   a) LLGR_STALE community is attached to routes to be retained;
   b) Clear stale routes that have NO_LLGR community attached;
   c) Start LLGR timer per AFI/SAFI;
   d) Recompute bestpath and reannounce routes to peers;
   d) When LLGR timer expires, clear all routes on particular AFI/SAFI.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2021-12-28 16:07:59 +02:00
Donald Sharp
5086cc1c66
Merge pull request #10254 from ton31337/fix/typo
bgpd: Fix typo in bgp_aggr_community_hash_alloc()
2021-12-27 08:21:49 -05:00
Donatas Abraitis
f5827f3689 bgpd: Drop if 0 blocks
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2021-12-23 14:41:11 +02:00
Donatas Abraitis
1182f26489
Merge pull request #8494 from donaldsharp/wfi_failures
bgpd, tests: Add code to handle failed installations
2021-12-22 09:53:44 +02:00
Donatas Abraitis
69a211cb69 bgpd: Fix typo in bgp_aggr_community_hash_alloc()
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2021-12-21 21:18:17 +02:00
Igor Ryzhov
ac2cb9bf94 *: rework renaming the default VRF
Currently, it is possible to rename the default VRF either by passing
`-o` option to zebra or by creating a file in `/var/run/netns` and
binding it to `/proc/self/ns/net`.

In both cases, only zebra knows about the rename and other daemons learn
about it only after they connect to zebra. This is a problem, because
daemons may read their config before they connect to zebra. To handle
this rename after the config is read, we have some special code in every
single daemon, which is not very bad but not desirable in my opinion.
But things are getting worse when we need to handle this in northbound
layer as we have to manually rewrite the config nodes. This approach is
already hacky, but still works as every daemon handles its own NB
structures. But it is completely incompatible with the central
management daemon architecture we are aiming for, as mgmtd doesn't even
have a connection with zebra to learn from it. And it shouldn't have it,
because operational state changes should never affect configuration.

To solve the problem and simplify the code, I propose to expand the `-o`
option to all daemons. By using the startup option, we let daemons know
about the rename before they read their configs so we don't need any
special code to deal with it. There's an easy way to pass the option to
all daemons by using `frr_global_options` variable.

Unfortunately, the second way of renaming by creating a file in
`/var/run/netns` is incompatible with the new mgmtd architecture.
Theoretically, we could force daemons to read their configs only after
they connect to zebra, but it means adding even more code to handle a
very specific use-case. And anyway this won't work for mgmtd as it
doesn't have a connection with zebra. So I had to remove this option.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
2021-12-21 22:09:29 +03:00
Rafael Zalamena
8bd0d3b1db bgpd: fix aggregate route AS Path attribute
Always free the locally allocated attribute not the one we are using for
return. This fixes a memory leak and a crash when AS Path is set with
route-map.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
2021-12-21 10:48:18 -03:00
Donatas Abraitis
22472feef8 bgpd: No need to test if a thread is running for BGP_TIMER_OFF
Handles that inside the macro.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2021-12-21 10:57:07 +02:00
Donald Sharp
be785e356a bgpd, tests: Add code to handle failed installations
Currently the Wait for Install code ( bgp_suppress_fib ) does
not properly handle two states from zebra:  ROUTE_INSTALL_FAILED
and BETTER_ADMIN_DISTANCE_WON.  Pre this change the WFI code
would just never notify our peers about a route install failure
but more is needed.  In the ROUTE_INSTALL_FAILED and the
BETTER_ADMIN_DISTANCE_WON we need to notify our peers with
a withdrawal about the route, else we will continue to
draw traffic to us when we cannot legally do so.

Why is this needed?  In either case imagine that we've already
received a bgp route, installed it and sent to our peers.
In the Better admin distance won case, say a static route is installed
at this point in time we must stop advertising the route through
us since we are not installed.  As such a withdrawal must be sent.

In the ROUTE_INSTALL_FAILED case, the code was not properly handling
the situation where we have Route A, it was successfully installed
and then we received a update to Route A that was attempted to be
installed but failed.  In this case we also need to send a withdrawal

Finally update the bgp_suppress_fib topotest to test both of these
situations.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2021-12-17 13:28:56 -05:00
Ryoga Saito
3d1ae061a3 bgpd: delete NULL assignment in bgp_attr_hash_alloc
If soft-reconfiguration is enabled, bgp_adj_in_set will be called
from bgp_update and bgp_adj_in_set will call bgp_attr_intern to intern
attr pointer. If given attr isn't found in attrhash, hash_get will call
bgp_attr_hash_alloc to allocate new attr structure. In
bgp_attr_hash_alloc, NULL will be assigned to srv6_vpn field and
srv6_l3vpn field in origin attr pointer. attr->srv6_vpn and
attr->srv6_l3vpn are interned in bgp_attr_intern, so NULL assignment
isn't needed.

And, these fields are used later in bgp_update to set SRv6 information
to bgp_path_info. If bgp_attr_hash_alloc assign NULL to these fields,
SRv6 information will be lost and incorrect routes are inserted into
data-plane.

Signed-off-by: Ryoga Saito <contact@proelbtn.com>
2021-12-16 23:28:12 +09:00
Quentin Young
2e38d79e64
Merge pull request #10144 from ton31337/fix/bmp_memory_leaks 2021-12-06 00:00:27 -05:00
Mark Stapp
907707db48 bgpd: clearer safi handling for BGP-LU route updates
Don't hide the LABELED_UNICAST safi when processing route
updates; map it where necessary (to use the UNICAST table
for instance).

Signed-off-by: Mark Stapp <mstapp@nvidia.com>
2021-12-01 07:56:38 -05:00
Donatas Abraitis
e2144103f8
Merge pull request #9878 from pguibert6WIND/resolver_vrf
lib: resolver per vrf support
2021-12-01 08:12:33 +02:00
Russ White
f1f6716d4a
Merge pull request #9610 from iqras23/best_path
bgpd: VRF-Lite fix best path selection
2021-11-30 16:14:34 -05:00