Commit Graph

6531 Commits

Author SHA1 Message Date
Trey Aspelund
3880b4ece4 bgpd: prefix match for advertised/received-routes
This introduces the option for a user to lookup one specific prefix in
the advertised-routes or received-routes table of a peer.

Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
2023-02-08 21:06:10 +00:00
Trey Aspelund
aa9bf57eb8 bgpd: add 'detail' option to bestpath-routes
Introduce 'detail' keyword for 'show bgp <afi> <safi> bestpath-routes'.

Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
2023-02-08 20:55:00 +00:00
Donatas Abraitis
e7765b81ce
Merge pull request #12761 from anlancs/fix/bgpd-crash-evpn-vni-both-rts
bgpd: fix use-after-free crash for evpn
2023-02-08 10:07:07 +02:00
Donatas Abraitis
96475dfde9
Merge pull request #12707 from donaldsharp/missed_enums
Missed enums
2023-02-07 22:22:27 +02:00
Russ White
0b1a6ca322
Merge pull request #12748 from opensourcerouting/fix/route_map_vpn_import
tests: Check if route-map vpn import basic funtionality works fine
2023-02-07 11:40:03 -05:00
Russ White
24caf3069b
Merge pull request #12720 from opensourcerouting/fix/ecommunity_ipv6_missing_token
bgpd: ecommunity_token_rt6 is not handled
2023-02-07 09:21:07 -05:00
Donatas Abraitis
4ee5265aa7 bgpd: Add more context in logs about from where to where routes are leaked
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-02-06 18:44:11 +02:00
Donatas Abraitis
1aa2c93e2c bgpd: Drop struct bgp from vpn_leak_to_vrf_withdraw()
Not used at all, just drop it.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-02-06 18:44:11 +02:00
anlan_cs
432ff4b036 bgpd: fix use-after-free crash for evpn
```
anlan(config-router-af)# vni 33
anlan(config-router-af-vni)# route-target both 44:55
anlan(config-router-af-vni)# no route-target both 44:55
vtysh: error reading from bgpd: Resource temporarily unavailable (11)Warning: closing connection to bgpd because of an I/O error!
```

When `bgp_evpn_vni_rt_cmd` deals with "both" type, it wrongly created
only one node ( should be two nodes ) for lists of both `vpn->import_rtl` and
`vpn->export_rtl`.  At this time, the two lists are already wrong.

In `no route-target both RT`, it will free the single node from lists of both
`vpn->import_rtl` and `vpn->export_rtl`.  After freed from `vpn->import_rtl`,
it is "use-after-free" at the time of freeing it from `vpn->export_rtl`.
It causes crash sometimes, or other unexpected behaviours.

This issue is introduced by commit `3b7e8d`, which have adjusted both
`bgp_evpn_vni_rt_cmd` and `bgp_evpn_vrf_rt_cmd`.

Since `bgp_evpn_vrf_rt_cmd/no_bgp_evpn_vrf_rt_cmd` works well again
unintentionally with commit `7022da`, only `bgp_evpn_vni_rt_cmd` needs to
modify - add two nodes for "both" type and some explicit comments for this
special case of "both" type.

Signed-off-by: anlan_cs <vic.lan@pica8.com>
2023-02-06 21:39:22 +08:00
Donald Sharp
f1b1efdefc bgpd: Don't try to recursively hold peer io mutex
BGP was modified in a0b937de42
to grab the peer->io_mtx before validating the header to ensure
that the input Queue was not being modified by anyone else at that
moment in time.  Unfortunately validate_header can detect a problem
and attempt to relock the mutex, which deadlocks.  This deadlock in
the bgp_io pthread is the lone deadlock at first, eventually though
bgp attempts to write another packet to the peer( say when the
it's time to send the next packet ) and the main pthread of bgpd
becomes deadlocked and then the whole bgpd process is stuck at that
point in time leaving us dead in the water.

The point of locking the mutex earlier was to ensure that the input
Queue wasn't being modified by anyone else, (Say reading off it )
as that we wanted to ensure that we don't hold more packets then necessary.

Let's grab the mutex long enough to look at the input Q size, this
ensure that we have room and then we can validate_header and do the right
thing from there.  We'll need to lock the mutex when we actually move it
into the input Q as well.

Fixes: #12725
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-02-02 16:39:01 -05:00
Russ White
73fb874e0a
Merge pull request #12731 from donaldsharp/remove_pretty_print
lib, bgpd: Add ability to specify that some json output should not be…
2023-02-02 15:44:45 -05:00
Donald Sharp
2d4460de6f bgpd: Convert evpn output to not pretty print json
Commit: 3cdb03fba7
changed the vty_json output to not be pretty printing.
The previous commit in the tree added vty_json_no_pretty
let's use that instead

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-02-02 10:42:21 -05:00
Donald Sharp
d7c6467ba2 lib, bgpd: Add ability to specify that some json output should not be pretty
Initial commit: 23b2a7ef52
changed the json output of `show bgp <afi> <safi> json` to
not have pretty print because when under a situation where
there are a bunch of routes with a large scale ecmp show
output was taking forever and this commit cut 2 minutes out
of vtysh run time.

Subusequent commit: f4ec52f7cc
changed this back.

When upgrading to latest version the long run time was noticed
due to testing.  Let's add back this functionality such that
FRR can have reduced run times with vtysh when it's really
needed.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-02-02 10:28:19 -05:00
Donatas Abraitis
b1d7888f9b bgpd: Set attr to NULL when passing NLRI_UPDATE with treat-as-withdraw
Before this patch, we always passed `struct attr` for NLRI_UPDATE, but if we
have a situation with treat-as-withdraw (for example: malformed attribute, or
using a command like `neighbor path-attribute treat-as-withdraw`) the route
MUST be withdrawn form the BGP table.

Hence, we MUST pass attr as NULL, in this case we already have this check
under NLRI_ATTR_ARG() macro, just reuse it properly.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-02-01 23:07:56 +02:00
Donald Sharp
33303f08bf
Merge pull request #12713 from opensourcerouting/fix/json_naming_deprecation
*: Drop deprecated incorrect JSON fields with wrong naming
2023-02-01 15:39:18 -05:00
David Lamparter
e3a7fc5021 bgpd/rfapi: add missing printf attribute
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
2023-02-01 03:00:22 +01:00
Donald Sharp
125ef88d29 bgpd: ecommunity_token_rt6 is not handled
The function ecommunity_str2com_internal appears to want to handle
the ecommunity_token_rt6 enum but skips over it.  Commit
9a659715df tried to add this but I really
don't see how this is going to behave correctly.  Add the
ecommunity_token_rt6 case to the switch statement so it is handled
appropriately?

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-01-31 21:04:42 +02:00
Donald Sharp
58cf0823bf bgpd: Add missing enum's to case statement
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-01-31 12:29:08 -05:00
Donald Sharp
29763bc910 bgpd: ecommunity_token_rt6 is not handled
The function ecommunity_str2com_internal appears to want to handle
the ecommunity_token_rt6 enum but skips over it.  Commit
9a659715df tried to add this but I really
don't see how this is going to behave correctly.  Add the
ecommunity_token_rt6 case to the switch statement so it is handled
appropriately?

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-01-31 12:29:08 -05:00
Donald Sharp
ea768492f1
Merge pull request #12695 from opensourcerouting/format-warnings
build: `-Wformat-nonliteral -Wformat-security`
2023-01-31 09:01:32 -05:00
Donald Sharp
d829de9178
Merge pull request #12710 from opensourcerouting/fix/reset_fqdn_capability_on_before_handling_open
bgpd: Vanish FQDN capability hostname/domainname before handling new BGP OPEN
2023-01-31 08:46:03 -05:00
Donatas Abraitis
1ce23106eb *: Drop deprecated incorrect JSON fields with wrong naming
Deprecation cycle already passed.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-01-31 10:18:28 +02:00
Donatas Abraitis
d188b08f30 bgpd: Vanish FQDN capability hostname/domainname before handling new BGP OPEN
Before this, if the peer disables sending FQDN capability, the old hostname
still (STALE) exists and is misleading in the outputs of `show bgp ...`.

Especially when using with `bgp default show-hostname`, etc.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-01-30 23:34:41 +02:00
Donatas Abraitis
17ff4f6367 bgpd: Free peer's hostname (aka FQDN capability stuff)
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-01-30 23:22:58 +02:00
Donald Sharp
367b458cb4 bgpd: bgp_update and bgp_withdraw never return failures
These two functions always return 0.  As such any and all
tests against this make no sense.  Remove the return 0
to a void and follow the chain, logically, to remove all
the dead code.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-01-30 16:02:23 -05:00
Trey Aspelund
4dabdde32a bgpd: move tunnel-ip comparison into handler
Moves the old/new IP comparison into handle_tunnel_ip_change instead of
expecting the caller to do the check on their own.
Also changes handle_tunnel_ip_change to return void since it only ever
returned 0 in all cases.

Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
2023-01-27 11:12:14 -05:00
Trey Aspelund
826c3f6db3 bgpd: only unimport routes if tunnel-ip changes
When processing a new local VNI, we were always walking the global EVPN
table to look for routes that needed to be removed due to a martian
nexthop change (specifically a tunnel-ip change).
Since the martian TIP table is global (all VNIs) + the walk is also in
the global table (all VNIs), we can trust that any new TIP from any VNI
would result in routes getting removed from the global table and
unimported from all live (L2)VNIs.
i.e.
The only time this update is actionable is if we are adding/removing an
IP from the martian TIP table, and we do not need to walk the table for
normal refcount adjustments.

Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
2023-01-27 11:11:44 -05:00
David Lamparter
0e4e879b40 bgpd: fix silly format string SNAFU
Someone thought vty_out accepts a list of strings.  It does not.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
2023-01-27 12:01:20 +01:00
David Lamparter
e678b143a9 bgpd: fix format string mess in AS-path printing
This was done *very* weirdly.  Make it slightly less so.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
2023-01-27 12:01:20 +01:00
David Lamparter
c84e518709 *: no-warn pragmas for non-const format strings
We do use non-constant/literal format strings in a few places for more
or less valid reasons;  put `ignored "-Wformat-nonliteral"` around those
so we can have the warning enabled for everywhere else.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
2023-01-27 12:01:20 +01:00
David Lamparter
0f9de11a11 *: apply proper format string attributes
So that we get warnings about broken format strings.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
2023-01-27 12:01:17 +01:00
Donatas Abraitis
88bcd9f7ae
Merge pull request #12684 from chiragshah6/fdev2
bgpd: evpn route detail json display non prett
2023-01-25 10:30:10 +02:00
Russ White
9b1b028cc2
Merge pull request #12682 from opensourcerouting/time-cs
*: fix time truncation in many places
2023-01-24 10:51:44 -05:00
Russ White
b25695f630
Merge pull request #12678 from opensourcerouting/fix/missing_no_form_for_path_attribute_discard
bgpd: Add missing `no` form for `neighbor path-attribute discard` cmd
2023-01-24 10:45:03 -05:00
Russ White
95e5cc2319
Merge pull request #12647 from anlancs/fix/bgpd-type-2
bgpd: cosmetic changes for debug
2023-01-24 10:13:22 -05:00
Philippe Guibert
9795e9f234 bgpd: fix when route-map changes the link local nexthop for vpnv6
This fix updates the nexthop length of a bgp update to be
transmitted to a remote peer. Before the previous commit,
the ipv6 nexthop length was internally set to 32 bytes which
was not correct, as it should be 48 bytes which is conform
to the vpnv6 encoding format.

However, without the previous match, even if internally, the
nexthop length was set to 32, the real nexthop length was set
to 48 bytes, and everything was operating ok.

Now, if we use the following route-map, and attach it to
outgoing for vpnv6 address family, then we have a malformed
packet detected, and the peering breaks.

 > route-map rmap permit 1
 > set ipv6 next-hop global 5:5::3:6
 > set ipv6 next-hop local fe80:55::333:222

Maintain the mp_nexthop_len to 48 bytes if it was already set
to 48 previously.

Fixes: 35ac9b53f2 ("bgpd: fix vpnv6 nexthop encoding")

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2023-01-24 08:04:27 +01:00
Chirag Shah
3cdb03fba7 bgpd: evpn route detail json display non prett
For BGP evpn route table detail json to use
non pretty form of display.

Problem:
In scaled evpn route table detail json dump
occupies high resources (CPU + memory) of the system.
In high scale evpn route dump using pretty form
hogs CPU for a while which can trigger watchfrr
to kill bgpd.

Solution:
Avoid pretty JSON print for detail version dump

Signed-off-by: Chirag Shah <chirag@nvidia.com>
2023-01-23 22:25:12 -08:00
Philippe Guibert
f7a0eb6a17 bgpd: encode properly vpnv6 nexthop
This change updates the nexthop attribute length
accordingly to the safi used. Actually, with the
previous commit, the length calculated was not
aligned with the real nexthop length. Such packet
received by remote peer was malformed, and this
was resulting in breaking vpnv6 peering.

Fix this by updating appropriately the real
nexthop length.

Fixes: 35ac9b53f2 ("bgpd: fix vpnv6 nexthop encoding")

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2023-01-23 16:43:51 +01:00
Donatas Abraitis
b986d7f41a bgpd: Add missing no form for neighbor path-attribute discard cmd
Just forgot this _somehow_ :)

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-01-22 22:17:39 +02:00
Rafael Zalamena
23acae3b87 bgpd: fix time truncation due to parameter type
Use the correct function parameters type to avoid truncation and other
signal issues.

Found by Coverity Scan (CID 1519802)

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
2023-01-20 16:20:56 -03:00
Donatas Abraitis
e9dbc60ee2
Merge pull request #12666 from donaldsharp/bgp_outq_limit
Bgp outq limit
2023-01-20 11:59:34 +02:00
Donatas Abraitis
9070c493a1
Merge pull request #12664 from pguibert6WIND/vpnv6_encoding_nexthop
bgpd: fix vpnv6 nexthop encoding
2023-01-20 11:45:51 +02:00
Donatas Abraitis
f5c8073ddd
Merge pull request #12669 from proelbtn/fix-accept-own-rd-check
bgpd: Skip RD match if ACCEPT_OWN excended community is not attached
2023-01-20 11:12:47 +02:00
Philippe Guibert
35ac9b53f2 bgpd: fix vpnv6 nexthop encoding
In ipv6 vpn, when the global and the local ipv6 address are received,
when re-transmitting the bgp ipv6 update, the nexthop attribute
length must still be 48 bytes.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2023-01-20 08:22:20 +01:00
Ryoga Saito
dd2d28ea99 bgpd: Skip RD match if ACCEPT_OWN is not enabled
RFC7611 introduces new extended community ACCEPT_OWN and is already
implemented for FRR in the previous PR. However, this PR broke
compatibility about importing VPN routes.

Let's consider the following situation. There are 2 routers and these
routers connects with iBGP session. These routers have two VRF, vrf10
and vrf20, and RD 0:10, 0:20 is configured as the route distinguisher
of vrf10 and vrf20 respectively.

    +- R1 --------+    +- R2 --------+
    | +---------+ |    | +---------+ |
    | |  VRF10  | |    | |  VRF10  | |
    | | RD 0:10 +--------+ RD 0:10 | |
    | +---------+ |    | +---------+ |
    | +---------+ |    | +---------+ |
    | |  VRF20  +--------+  VRF20  | |
    | | RD 0:20 | |    | | RD 0:20 | |
    | +---------+ |    | +---------+ |
    +-------------+    +-------------+

In this situation, the VPN routes from R1's VRF10 should be imported to
R2's VRF10 and the VPN routes from R2's VRF10 should be imported to R2's
VRF20. However, the current implementation of ACCEPT_OWN will always
reject routes if the RD of VPN routes are matched with the RD of VRF.

Similar issues will happen in local VRF2VRF route leaks. In such cases,
the route reaked from VRF10 should be imported to VRF20. However, the
current implementation of ACCEPT_OWN will not permit them.

    +- R1 ---------------------+
    |      +------------+      |
    | +----v----+  +----v----+ |
    | |  VRF10  |  |  VRF20  | |
    | | RD 0:10 |  | RD 0:10 | |
    | +---------+  +---------+ |
    +--------------------------+

So, this commit add additional condition in RD match. If the route
doesn't have ACCEPT_OWN extended community, source VRF check will be
skipped.

[RFC7611]: https://datatracker.ietf.org/doc/html/rfc7611

Signed-off-by: Ryoga Saito <ryoga.saito@linecorp.com>
2023-01-20 15:28:39 +09:00
Donald Sharp
2d13ec0a50 bgpd: Input Queue does not belong under router bgp
The input queue limit does not belong under router bgp.  This
is a dev escape and should just be removed.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-01-19 11:49:30 -05:00
Donald Sharp
963b7ee448 bgpd: Limit peer output queue length like input queue length
Consider this scenario:

Lots of peers with a bunch of route information that is changing
fast.  One of the peers happens to be really slow for whatever
reason.  The way the output queue is filled is that bgpd puts
64 packets at a time and then reschedules itself to send more
in the future.  Now suppose that peer has hit it's input Queue
limit and is slow.  As such bgp will continue to add data to
the output Queue, irrelevant if the other side is receiving
this data.

Let's limit the Output Queue to the same limit as the Input
Queue.  This should prevent bgp eating up large amounts of
memory as stream data when under severe network trauma.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-01-19 11:48:01 -05:00
Donatas Abraitis
cfd01fc0ac Revert "bgpd: optimal router reflection cli and fsm changes"
This reverts commit 70cd87ca02.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-01-17 18:15:28 +02:00
Donatas Abraitis
1ea57af264 Revert "bgpd, ospfd: BGP ORR CI warning fixes"
This reverts commit d6b2761134.
2023-01-17 18:10:04 +02:00
Donatas Abraitis
af7e7dbec5 Revert "bgpd: fix for crash when no neighbor A.B.C.D remote-as AS_NUM with orr config"
This reverts commit 5fcf01c9ae.
2023-01-17 18:07:46 +02:00