And also do not crash when we do `clear ip bgp ...`.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit 35b7ce08a8)
When we receive a default route from a peer and we originate default route
using `neighbor default-originate`, we do not track of struct attr we use,
and when we do `no neighbor default-originate` we withdraw our generated
default route, but we announce default-route from the peer.
After we do this, we unintern aspath (which was used for default-originate),
BUT it was used also for peer's default route we received.
And here we have a use-after-free crash, because bgp_process_main_one()
reaps old paths that are marked as BGP_PATH_REMOVED with aspath->refcnt > 0,
but here it's 0.
```
0 0x55c24bbcd022 in aspath_key_make bgpd/bgp_aspath.c:2070
1 0x55c24b8f1140 in attrhash_key_make bgpd/bgp_attr.c:777
2 0x7f52322e66c9 in hash_release lib/hash.c:220
3 0x55c24b8f6017 in bgp_attr_unintern bgpd/bgp_attr.c:1271
4 0x55c24ba0acaa in bgp_path_info_free_with_caller bgpd/bgp_route.c:283
5 0x55c24ba0a7de in bgp_path_info_unlock bgpd/bgp_route.c:309
6 0x55c24ba0af6d in bgp_path_info_reap bgpd/bgp_route.c:426
7 0x55c24ba17b9a in bgp_process_main_one bgpd/bgp_route.c:3333
8 0x55c24ba18a1d in bgp_process_wq bgpd/bgp_route.c:3425
9 0x7f52323c2cd5 in work_queue_run lib/workqueue.c:282
10 0x7f52323aab92 in thread_call lib/thread.c:2006
11 0x7f5232300dc7 in frr_run lib/libfrr.c:1198
12 0x55c24b8ea792 in main bgpd/bgp_main.c:520
13 0x7f5231c3a082 in __libc_start_main ../csu/libc-start.c:308
14 0x55c24b8ef0bd in _start (/usr/lib/frr/bgpd+0x2c90bd)
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit e9340ff429)
Seems just a missed one because at `goto error` we release dnode.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit ecf82aa41e)
Filter out keys in JSON output with "grep -v" does not work when JSON
does not use the pretty format.
Use native python code to filter out keys.
Fixes: 6c13bd5744 ("topotests: fix bgp_vpnv4_noretain")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit a8d6faa986)
```
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>
(cherry picked from commit 432ff4b036)
The first argument of `access_list_lookup` and `prefix_list_lookup`
should be `AFI_` constants instead of `AF_` constants.
Signed-off-by: Yuxiang Zhu <vfreex@gmail.com>
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>
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>
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>
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>
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>
