Commit Graph

33559 Commits

Author SHA1 Message Date
Donald Sharp
c937582491 zebra: Prevent protodown_rc from going Bzonkas
The code that handles the protodown_rc setting for
VRRP interfaces in zebra is sending a interface
to be set into a protodown state *before* the
interface has been learned by the kernel.  Resulting
in crashes when the data plane sends the ctx back
to us saying hey man you are uncool.

Additionally change the protodown code to refuse
to send any protodown_rc codes *until* the interface
has actually been learned about from the kernel.

Ticket: 3582375
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-21 15:49:09 -04:00
Donatas Abraitis
451fb24b17
Merge pull request #8790 from donaldsharp/peer_connection
Peer connection
2023-08-21 20:22:53 +03:00
Donald Sharp
ff4c767a31
Merge pull request #14241 from opensourcerouting/fix/software_version_capability_handling_len
bgpd: Check the length of the rcv software version
2023-08-21 09:33:18 -04:00
Donald Sharp
c2b0d6a08f
Merge pull request #14245 from opensourcerouting/fix/check_if_the_first_byte_is_not_null_orf
bgpd: Don't read the first byte of ORF header if we are ahead of stream
2023-08-21 09:32:32 -04:00
Donatas Abraitis
ccc3c6c9db
Merge pull request #14244 from donaldsharp/static_simple
tests: static_simple gives up after 3 seconds
2023-08-21 11:53:45 +03:00
Keelan10
0629ad01d4 lib: Clear Computed Path Pointer to Destination on Clean
This commit ensures proper cleanup by clearing the `algo->pdst` pointer if it points to a path that is being deleted.
It addresses memory leaks by freeing memory held by `algo->pdst` that might not have been released during the cleanup of processed paths.

The ASan leak log for reference:

```
Direct leak of 96 byte(s) in 1 object(s) allocated from:
    #0 0x7fbffcec9a37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7fbffca67a81 in qcalloc ../lib/memory.c:105
    #2 0x7fbffc9d1a54 in cpath_new ../lib/cspf.c:44
    #3 0x7fbffc9d2829 in cspf_init ../lib/cspf.c:256
    #4 0x7fbffc9d295d in cspf_init_v4 ../lib/cspf.c:287
    #5 0x5601dcd34d3f in show_sharp_cspf_magic ../sharpd/sharp_vty.c:1262
    #6 0x5601dcd2c2be in show_sharp_cspf sharpd/sharp_vty_clippy.c:1869
    #7 0x7fbffc9afd61 in cmd_execute_command_real ../lib/command.c:993
    #8 0x7fbffc9b00ee in cmd_execute_command ../lib/command.c:1052
    #9 0x7fbffc9b0dc0 in cmd_execute ../lib/command.c:1218
    #10 0x7fbffcb611c7 in vty_command ../lib/vty.c:591
    #11 0x7fbffcb660ac in vty_execute ../lib/vty.c:1354
    #12 0x7fbffcb6c4aa in vtysh_read ../lib/vty.c:2362
    #13 0x7fbffcb51324 in event_call ../lib/event.c:1979
    #14 0x7fbffca3b872 in frr_run ../lib/libfrr.c:1213
    #15 0x5601dcd11c6f in main ../sharpd/sharp_main.c:177
    #16 0x7fbffc5ffd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

Indirect leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x7fbffcec9a37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7fbffca67a81 in qcalloc ../lib/memory.c:105
    #2 0x7fbffca3c108 in list_new ../lib/linklist.c:49
    #3 0x7fbffc9d1acc in cpath_new ../lib/cspf.c:47
    #4 0x7fbffc9d2829 in cspf_init ../lib/cspf.c:256
    #5 0x7fbffc9d295d in cspf_init_v4 ../lib/cspf.c:287
    #6 0x5601dcd34d3f in show_sharp_cspf_magic ../sharpd/sharp_vty.c:1262
    #7 0x5601dcd2c2be in show_sharp_cspf sharpd/sharp_vty_clippy.c:1869
    #8 0x7fbffc9afd61 in cmd_execute_command_real ../lib/command.c:993
    #9 0x7fbffc9b00ee in cmd_execute_command ../lib/command.c:1052
    #10 0x7fbffc9b0dc0 in cmd_execute ../lib/command.c:1218
    #11 0x7fbffcb611c7 in vty_command ../lib/vty.c:591
    #12 0x7fbffcb660ac in vty_execute ../lib/vty.c:1354
    #13 0x7fbffcb6c4aa in vtysh_read ../lib/vty.c:2362
    #14 0x7fbffcb51324 in event_call ../lib/event.c:1979
    #15 0x7fbffca3b872 in frr_run ../lib/libfrr.c:1213
    #16 0x5601dcd11c6f in main ../sharpd/sharp_main.c:177
    #17 0x7fbffc5ffd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
2023-08-21 07:36:39 +04:00
Donald Sharp
9f4c654c59 tests: static_simple gives up after 3 seconds
Under heavy system load we can see that the static_simple
test is giving up too early in this micronet run:

8-17 15:00:27,105 DEBUG: topo: Waiting for [0.1]s as initial delay
2023-08-17 15:00:27,206 DEBUG: r1: cmd_status("/bin/bash -c 'ip -4 route show'")
2023-08-17 15:00:28,209 DEBUG: r1:
	stdout: 101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1
2023-08-17 15:00:28,209 DEBUG: topo: checking kernel routing table:
101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1

2023-08-17 15:00:28,210  INFO: topo: Function raised exception: Failed to find
  '10.0.0.0/8(?: nhid [0-9]+)? via 101.0.0.2 dev r1-eth0 proto (static|196) metric 20'
   in
  '101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1
  '
assert None
 +  where None = <function search at 0x7f405b7bb0a0>('10.0.0.0/8(?: nhid [0-9]+)? via 101.0.0.2 dev r1-eth0 proto (static|196) metric 20', '101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1 \n')
 +    where <function search at 0x7f405b7bb0a0> = re.search
2023-08-17 15:00:28,210 DEBUG: topo: Sleeping 2s until next retry with 3.0 retry time left
2023-08-17 15:00:30,211 DEBUG: r1: cmd_status("/bin/bash -c 'ip -4 route show'")
2023-08-17 15:00:31,703 DEBUG: r1:
	stdout: 101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1
2023-08-17 15:00:31,703 DEBUG: topo: checking kernel routing table:
101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1

2023-08-17 15:00:31,704  INFO: topo: Function raised exception: Failed to find
  '10.0.0.0/8(?: nhid [0-9]+)? via 101.0.0.2 dev r1-eth0 proto (static|196) metric 20'
   in
  '101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1
  '
assert None
 +  where None = <function search at 0x7f405b7bb0a0>('10.0.0.0/8(?: nhid [0-9]+)? via 101.0.0.2 dev r1-eth0 proto (static|196) metric 20', '101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1 \n')
 +    where <function search at 0x7f405b7bb0a0> = re.search
2023-08-17 15:00:31,704  INFO: topo: Retry timeout of 3s reached
2023-08-17 15:00:31,704  INFO: topo: Spawn collection of support bundle for r1
2023-08-17 15:00:31,704 DEBUG: r1: cmd_status("/bin/bash -c 'mkdir -p /tmp/topotests/static_simple.test_static_simple/r1/support_bundles/test_static_cli'")
2023-08-17 15:00:31,710 DEBUG: r1: popen("/usr/lib/frr/generate_support_bundle.py --log-dir=/tmp/topotests/static_simple.test_static_simple/r1/support_bundles/test_static_cli")
2023-08-17 15:00:31,711 DEBUG: topo: Waiting on support bundle for r1
2023-08-17 15:00:31,751 DEBUG: topo: RETRY DIAG: [failure] Sleeping 2s until next retry with 2.2 retry time left - too see if timeout was too short
2023-08-17 15:00:33,751 DEBUG: r1: cmd_status("/bin/bash -c 'ip -4 route show'")
2023-08-17 15:00:35,137 DEBUG: r1:
	stdout: 10.0.0.0/8 nhid 12 via 101.0.0.2 dev r1-eth0 proto 196 metric 20...
2023-08-17 15:00:35,137 DEBUG: topo: checking kernel routing table:
10.0.0.0/8 nhid 12 via 101.0.0.2 dev r1-eth0 proto 196 metric 20
101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1

2023-08-17 15:00:35,137 DEBUG: topo: Function returned None
2023-08-17 15:00:35,138  WARN: topo: RETRY DIAGNOSTIC: SUCCEED after FAILED with requested timeout of 3.0s; however, succeeded in 8.0s, investigate timeout timing
2023-08-17 15:00:35,138  INFO: topo: Function raised exception: Failed to find
  '10.0.0.0/8(?: nhid [0-9]+)? via 101.0.0.2 dev r1-eth0 proto (static|196) metric 20'
   in
  '101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1
  '
assert None
 +  where None = <function search at 0x7f405b7bb0a0>('10.0.0.0/8(?: nhid [0-9]+)? via 101.0.0.2 dev r1-eth0 proto (static|196) metric 20', '101.0.0.0/24 dev r1-eth0 proto kernel scope link src 101.0.0.1 \n')
 +    where <function search at 0x7f405b7bb0a0> = re.search
2023-08-17 15:00:35,138 DEBUG: topo: RETRY DIAG: [failure] Sleeping 2s until next retry with 0.2 retry time left - too see if timeout was too short
2023-08-17 15:00:37,139 DEBUG: r1: cmd_status("/bin/bash -c 'ip -4 route show'")
2023-08-17 15:00:37,247 DEBUG: r1:
	stdout: 10.0.0.0/8 nhid 12 via 101.0.0.2 dev r1-eth0 proto 196 metric 20...

Of course it works in the extra couple of times it tries but the test still fails.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-20 18:43:48 -04:00
Donatas Abraitis
767aaa3a80 bgpd: Do not explicitly print MAXTTL value for ebgp-multihop vty output
1. Create /etc/frr/frr.conf
```
frr version 7.5
frr defaults traditional
hostname centos8.localdomain
no ip forwarding
no ipv6 forwarding
service integrated-vtysh-config
line vty
router bgp 4250001000
  neighbor 192.168.122.207 remote-as 65512
  neighbor 192.168.122.207 ebgp-multihop
```

2. Start FRR
`# systemctl start frr
`
3. Show running configuration. Note that FRR explicitly set and shows the default TTL (225)

```
Building configuration...

Current configuration:
!
frr version 7.5
frr defaults traditional
hostname centos8.localdomain
no ip forwarding
no ipv6 forwarding
service integrated-vtysh-config
!
router bgp 4250001000
 neighbor 192.168.122.207 remote-as 65512
 neighbor 192.168.122.207 ebgp-multihop 255
!
line vty
!
end
```
4. Copy initial frr.conf to frr.conf.new (no changes)
`# cp /etc/frr/frr.conf /root/frr.conf.new
`
5. Run frr-reload.sh:

```
$ /usr/lib/frr/frr-reload.py --test  /root/frr.conf.new
2023-08-20 20:15:48,050  INFO: Called via "Namespace(bindir='/usr/bin', confdir='/etc/frr', daemon='', debug=False, filename='/root/frr.conf.new', input=None, log_level='info', overwrite=False, pathspace=None, reload=False, rundir='/var/run/frr', stdout=False, test=True, vty_socket=None)"
2023-08-20 20:15:48,050  INFO: Loading Config object from file /root/frr.conf.new
2023-08-20 20:15:48,124  INFO: Loading Config object from vtysh show running

Lines To Delete
===============
router bgp 4250001000
 no neighbor 192.168.122.207 ebgp-multihop 255

Lines To Add
============
router bgp 4250001000
 neighbor 192.168.122.207 ebgp-multihop
```

Closes https://github.com/FRRouting/frr/issues/14242

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-21 00:03:24 +03:00
Donatas Abraitis
9b855a692e bgpd: Don't read the first byte of ORF header if we are ahead of stream
Reported-by: Iggy Frankovic iggyfran@amazon.com
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-20 23:22:00 +03:00
Donatas Abraitis
19ad3e2770
Merge pull request #14226 from Keelan10/fix-pbrd-leak
pbrd: Correct Handling of Sequence Deletion
2023-08-20 22:32:21 +03:00
Donatas Abraitis
b4d09af919 bgpd: Check the length of the rcv software version
Make sure we don't exceed the maximum of BGP_MAX_SOFT_VERSION.

The Capability Length SHOULD be no greater than 64.

Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-20 21:48:36 +03:00
Donatas Abraitis
f96201e104 bgpd: Make sure we have enough data to read two bytes when validating AIGP
Found when fuzzing:

```
==3470861==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xffff77801ef7 at pc 0xaaaaba7b3dbc bp 0xffffcff0e760 sp 0xffffcff0df50
READ of size 2 at 0xffff77801ef7 thread T0
    0 0xaaaaba7b3db8 in __asan_memcpy (/home/ubuntu/frr_8_5_2/frr_8_5_2_fuzz_clang/bgpd/bgpd+0x363db8) (BuildId: cc710a2356e31c7f4e4a17595b54de82145a6e21)
    1 0xaaaaba81a8ac in ptr_get_be16 /home/ubuntu/frr_8_5_2/frr_8_5_2_fuzz_clang/./lib/stream.h:399:2
    2 0xaaaaba819f2c in bgp_attr_aigp_valid /home/ubuntu/frr_8_5_2/frr_8_5_2_fuzz_clang/bgpd/bgp_attr.c:504:3
    3 0xaaaaba808c20 in bgp_attr_aigp /home/ubuntu/frr_8_5_2/frr_8_5_2_fuzz_clang/bgpd/bgp_attr.c:3275:7
    4 0xaaaaba7ff4e0 in bgp_attr_parse /home/ubuntu/frr_8_5_2/frr_8_5_2_fuzz_clang/bgpd/bgp_attr.c:3678:10
```

Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-20 21:26:00 +03:00
Keelan10
c09013e33a pbrd: Correct Handling of Sequence Deletion
This commit ensures that sequence data
and associated structures are correctly deleted to prevent memory leaks

The ASan leak log for reference:
```
Direct leak of 432 byte(s) in 1 object(s) allocated from:
    #0 0x7f911ebaba37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7f911e749a4e in qcalloc ../lib/memory.c:105
    #2 0x564fd444b2d3 in pbrms_get ../pbrd/pbr_map.c:527
    #3 0x564fd443a82d in pbr_map ../pbrd/pbr_vty.c:90
    #4 0x7f911e691d61 in cmd_execute_command_real ../lib/command.c:993
    #5 0x7f911e6920ee in cmd_execute_command ../lib/command.c:1052
    #6 0x7f911e692dc0 in cmd_execute ../lib/command.c:1218
    #7 0x7f911e843197 in vty_command ../lib/vty.c:591
    #8 0x7f911e84807c in vty_execute ../lib/vty.c:1354
    #9 0x7f911e84e47a in vtysh_read ../lib/vty.c:2362
    #10 0x7f911e8332f4 in event_call ../lib/event.c:1979
    #11 0x7f911e71d828 in frr_run ../lib/libfrr.c:1213
    #12 0x564fd4425795 in main ../pbrd/pbr_main.c:168
    #13 0x7f911e2e1d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
2023-08-20 07:07:36 +04:00
Donald Sharp
899427b515
Merge pull request #14216 from LabNConsulting/ziemba-coverity-pbr-230816
pbrd: address coverity issues reported 230815
2023-08-19 16:17:14 -04:00
Donald Sharp
fa8e49c79e
Merge pull request #14238 from Keelan10/ospf-leak-fix
ospfd: Delete `q_space->vertex_list` on No Backup Path
2023-08-19 16:12:08 -04:00
Donald Sharp
f3b381352f
Merge pull request #14236 from Keelan10/bgpd-memleak
bgpd: Free memory in set_aspath_exclude_access_list
2023-08-19 16:11:30 -04:00
Keelan10
64e0a47b2c ospfd: Delete q_space->vertex_list on No Backup Path
In scenarios where no backup paths are available, ensure proper
memory management by deleting `q_space->vertex_list`. This prevents
memory leaks.

The ASan leak log for reference:

```
Direct leak of 80 byte(s) in 2 object(s) allocated from:
    #0 0x7fcf8c70aa37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x7fcf8c2a8a45 in qcalloc ../lib/memory.c:105
    #2 0x7fcf8c27d0cc in list_new ../lib/linklist.c:49
    #3 0x55d6e8385e35 in ospf_spf_init ../ospfd/ospf_spf.c:540
    #4 0x55d6e838c30d in ospf_spf_calculate ../ospfd/ospf_spf.c:1736
    #5 0x55d6e83933cf in ospf_ti_lfa_generate_q_spaces ../ospfd/ospf_ti_lfa.c:673
    #6 0x55d6e8394214 in ospf_ti_lfa_generate_p_space ../ospfd/ospf_ti_lfa.c:812
    #7 0x55d6e8394c63 in ospf_ti_lfa_generate_p_spaces ../ospfd/ospf_ti_lfa.c:923
    #8 0x55d6e8396390 in ospf_ti_lfa_compute ../ospfd/ospf_ti_lfa.c:1101
    #9 0x55d6e838ca48 in ospf_spf_calculate_area ../ospfd/ospf_spf.c:1811
    #10 0x55d6e838cd73 in ospf_spf_calculate_areas ../ospfd/ospf_spf.c:1840
    #11 0x55d6e838cfb0 in ospf_spf_calculate_schedule_worker ../ospfd/ospf_spf.c:1871
    #12 0x7fcf8c3922e4 in event_call ../lib/event.c:1979
    #13 0x7fcf8c27c828 in frr_run ../lib/libfrr.c:1213
    #14 0x55d6e82eeb6d in main ../ospfd/ospf_main.c:249
    #15 0x7fcf8bd59d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
2023-08-19 18:38:14 +04:00
Keelan10
411cb8a827 bgpd: Free memory in set_aspath_exclude_access_list
Properly free the dynamically allocated memory held by `str` after its use.
The change also maintains the return value of `nb_cli_apply_changes` by using `ret` variable.

The ASan leak log for reference:

```
Direct leak of 55 byte(s) in 2 object(s) allocated from:
    #0 0x7f16f285f867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7f16f23fda11 in qmalloc ../lib/memory.c:100
    #2 0x7f16f23a01a0 in frrstr_join ../lib/frrstr.c:89
    #3 0x7f16f23418c7 in argv_concat ../lib/command.c:183
    #4 0x55aba24731f2 in set_aspath_exclude_access_list_magic ../bgpd/bgp_routemap.c:6327
    #5 0x55aba2455cf4 in set_aspath_exclude_access_list bgpd/bgp_routemap_clippy.c:836
    #6 0x7f16f2345d61 in cmd_execute_command_real ../lib/command.c:993
    #7 0x7f16f23460ee in cmd_execute_command ../lib/command.c:1052
    #8 0x7f16f2346dc0 in cmd_execute ../lib/command.c:1218
    #9 0x7f16f24f7197 in vty_command ../lib/vty.c:591
    #10 0x7f16f24fc07c in vty_execute ../lib/vty.c:1354
    #11 0x7f16f250247a in vtysh_read ../lib/vty.c:2362
    #12 0x7f16f24e72f4 in event_call ../lib/event.c:1979
    #13 0x7f16f23d1828 in frr_run ../lib/libfrr.c:1213
    #14 0x55aba2269e52 in main ../bgpd/bgp_main.c:510
    #15 0x7f16f1dbfd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
2023-08-19 14:00:17 +04:00
G. Paul Ziemba
5cde1e89f0 pbrd: address 230815 coverity: pbr_vty.c vrf_name
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
2023-08-18 11:19:05 -07:00
G. Paul Ziemba
eb3929b4fa pbrd: address 230815 coverity: pbr_vty.c pbrms
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
2023-08-18 11:14:25 -07:00
G. Paul Ziemba
2e6c879e99 pbrd: address 230815 coverity: pbr_vty.c pend/strtoul
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
2023-08-18 11:13:20 -07:00
G. Paul Ziemba
6182675e7e pbrd: address 230815 coverity: r.action.flags reordering
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
2023-08-18 11:11:17 -07:00
Mark Stapp
852e24d7a4
Merge pull request #14223 from donaldsharp/interface_fies
zebra: Fix crashes in interface change
2023-08-18 11:56:20 -04:00
Donald Sharp
05c2d8a200 bgpd: Separate out mtype for peer and connection
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
419c5b4ef0 bgpd: Cleanup bgp_start declarations
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
26ad36e097 bgpd: Convert FSM to use struct peer_connection
The BGP FSM was using the peer as the unit of work
but the FSM is connection focused.  So let's switch
it over to using that.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
3e5a31b24e bgpd: Convert struct peer_connection to dynamically allocated
As part of the conversion to a `struct peer_connection` it will
be desirable to have 2 pointers one for when we open a connection
and one for when we receive a connection.  Start this actual
conversion over to this in `struct peer`.  If this sounds confusing
take a look at the bgp state machine for connections and how
it resolves the processing of this router opening -vs- this
router receiving an open.  At some point in time the state
machine decides that we are keeping one of the two connections.

Future commits will allow us to untangle the peer/doppelganger
duality with this abstraction.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
5d52756735 bgpd: Move t_process_packet and t_process_packet_error to connection
The t_process_packet thread events should be managed by the connection.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
e20c23fa5b bgpd: Move status and ostatus to struct peer_connection
The status and ostatus are a function of the `struct peer_connection`
move it into that data structure.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
71d72c4998 bgpd: READ and WRITE flags are a part of the connection
Move PEER_THREAD_WRITES_ON and PEER_THREAD_READS_ON to
be a part of the `struct peer_connection` since this is
a connection oriented bit of data.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
c528b3b153 bgpd: Move t_write and t_read into struct peer_connection
Move the peer->t_write and peer->t_read into `struct peer_connection`
as that these are properties of the connection.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
P# Please enter the commit message for your changes. Lines starting
2023-08-18 09:29:04 -04:00
Donald Sharp
ccb51e8266 bgpd: Convert bgp_io.c to take struct peer_connection
bgp_io.c is clearly connection oriented so let's convert
it over to using `struct peer_connection`

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
84d1abd3d9 bgpd: Add peer backpointer to struct peer_connection
We will need the peer backpointer for a `struct peer_connection`
Let's add it in.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
e27bf2b9bd bgpd: Create a _new function for struct peer_connection
Nothing fancy here allow us to create the needed buffers
in an abstract way.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
3b2d89b0a3 bgpd: Create destructor function for struct peer_connection
Create a destructor function to free up memory associated
with the io buffers.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donald Sharp
1f32eb30d9 bgpd: Start abstraction of struct peer_connection
BGP tracks connections based upon the peer.  But the problem
with this is that the doppelganger structure for it is being
created.  This has introduced a bunch of fragileness in that
the peer exists independently of the connections to it.

The whole point of the doppelganger structure was to allow
BGP to both accept and initiate tcp connections and then
when we get one to a `good` state we collapse into the
appropriate one.  The problem with this is that having
2 peer structures for this creates a situation where
we have to make sure we are configing the `right` one
and also make sure that we collapse the two independent
peer structures into 1 acting peer.  This makes no sense
let's abstract out the peer into having 2 connection
one for incoming connections and one for outgoing connections
then we can easily collapse down without having to do crazy
stuff.  In addition people adding new features don't need
to have to go touch a million places in the code.

This is the start of this abstraction.  In this commit
we'll just pull out the fd and input/output buffers
into a connection data structure.  Future commits
will abstract further.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-18 09:29:04 -04:00
Donatas Abraitis
1b52af80fd
Merge pull request #14224 from Keelan10/fix-bgpd-leak
bgpd: Free memory in set_aspath_replace_access_list
2023-08-18 10:21:50 +03:00
Donatas Abraitis
d02fae5836 zebra: Show NHT resolve via default status on/off
```
donatas-laptop# show ip nht
VRF default:
 Resolve via default: on
192.168.10.123
 resolved via connected
 is directly connected, wlp82s0 (vrf default)
 Client list: bgp(fd 21)
donatas-laptop# show ip nht json
{
  "default":{
    "ipv4":{
      "resolveViaDefault":true,
      "192.168.10.123":{
        "nhtConnected":false,
        "clientList":[
          {
            "protocol":"bgp",
            "socket":21,
            "protocolFiltered":"none"
          }
        ],
        "nexthops":[
          {
            "flags":3,
            "fib":true,
            "directlyConnected":true,
            "interfaceIndex":3,
            "interfaceName":"wlp82s0",
            "vrf":"default",
            "active":true
          }
        ],
        "resolvedProtocol":"connected"
      }
    }
  }
}
donatas-laptop# show ip nht vrf all

VRF default:
 Resolve via default: on
192.168.10.123
 resolved via connected
 is directly connected, wlp82s0 (vrf default)
 Client list: bgp(fd 21)
donatas-laptop# show ip nht vrf all json
{
  "default":{
    "ipv4":{
      "resolveViaDefault":true,
      "192.168.10.123":{
        "nhtConnected":false,
        "clientList":[
          {
            "protocol":"bgp",
            "socket":21,
            "protocolFiltered":"none"
          }
        ],
        "nexthops":[
          {
            "flags":3,
            "fib":true,
            "directlyConnected":true,
            "interfaceIndex":3,
            "interfaceName":"wlp82s0",
            "vrf":"default",
            "active":true
          }
        ],
        "resolvedProtocol":"connected"
      }
    }
  }
}
donatas-laptop#
```

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-17 21:45:55 +03:00
Donatas Abraitis
62196fbd19 zebra: Enable nht resolve-via-default by default for traditional profile
Lots of questions raising regarding unresolved nht, I think it's time to
relax this and make it a default ON.

Here is an example list of issues when `nht resolvia-via-default` solved
the problem:

https://github.com/FRRouting/frr/issues/3241
https://github.com/FRRouting/frr/issues/7420
https://github.com/FRRouting/frr/issues/3474
https://github.com/FRRouting/frr/issues/5023
https://github.com/FRRouting/frr/issues/6504
https://github.com/FRRouting/frr/issues/6680
https://github.com/FRRouting/frr/issues/7049
https://github.com/FRRouting/frr/issues/7862
https://github.com/FRRouting/frr/issues/7999
https://github.com/FRRouting/frr/issues/13215
https://github.com/FRRouting/frr/issues/14098

TL;DR;

The BGP session does not come up if using multihop sessions and/or the peer(nexthop)
is not accessible from the RIB, but only via default route. This is even valid for
iBGP, and not only for eBGP peering. Adding a static /32, /128 route for the peer
would do the trick, but it's a workaround.

If the route has a nexthop marked as invalid, most likely this is due to it can't
be resolved from the current RIB, but only via default route.

For instance, Cisco allows this by default (can't find even a knob to turn it
off or I'm blind).

For eBGP sessions it might be also combined with `disable-ebgp-connected-route-check`.

Some people asked if this could be a default, also for instance MetalLB is adding
this by default for all the configs it generates.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-17 21:45:55 +03:00
Keelan10
c60dc2a285 bgpd: Free memory in set_aspath_replace_access_list
Properly free the dynamically allocated memory held by `str` after its use.
The change also maintains the return value of `nb_cli_apply_changes` by using 'ret' variable.

The ASan leak log for reference:

```
***********************************************************************************
Address Sanitizer Error detected in bgp_set_aspath_replace.test_bgp_set_aspath_replace/r1.asan.bgpd.11586

=================================================================
==11586==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 92 byte(s) in 3 object(s) allocated from:
    #0 0x7f4e2951db40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7f4e28f19ea2 in qmalloc lib/memory.c:100
    #2 0x7f4e28edbb08 in frrstr_join lib/frrstr.c:89
    #3 0x7f4e28e9a601 in argv_concat lib/command.c:183
    #4 0x56519adf8413 in set_aspath_replace_access_list_magic bgpd/bgp_routemap.c:6174
    #5 0x56519adf8942 in set_aspath_replace_access_list bgpd/bgp_routemap_clippy.c:683
    #6 0x7f4e28e9d548 in cmd_execute_command_real lib/command.c:993
    #7 0x7f4e28e9da0c in cmd_execute_command lib/command.c:1051
    #8 0x7f4e28e9de8b in cmd_execute lib/command.c:1218
    #9 0x7f4e28fc4f1c in vty_command lib/vty.c:591
    #10 0x7f4e28fc53c7 in vty_execute lib/vty.c:1354
    #11 0x7f4e28fcdc8d in vtysh_read lib/vty.c:2362
    #12 0x7f4e28fb8c8b in event_call lib/event.c:1979
    #13 0x7f4e28efd445 in frr_run lib/libfrr.c:1213
    #14 0x56519ac85d81 in main bgpd/bgp_main.c:510
    #15 0x7f4e27f40c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 92 byte(s) leaked in 3 allocation(s).
***********************************************************************************

***********************************************************************************
Address Sanitizer Error detected in bgp_set_aspath_exclude.test_bgp_set_aspath_exclude/r1.asan.bgpd.10385

=================================================================
==10385==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 55 byte(s) in 2 object(s) allocated from:
    #0 0x7f6814fdab40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7f68149d6ea2 in qmalloc lib/memory.c:100
    #2 0x7f6814998b08 in frrstr_join lib/frrstr.c:89
    #3 0x7f6814957601 in argv_concat lib/command.c:183
    #4 0x5570e05117a1 in set_aspath_exclude_access_list_magic bgpd/bgp_routemap.c:6327
    #5 0x5570e05119da in set_aspath_exclude_access_list bgpd/bgp_routemap_clippy.c:836
    #6 0x7f681495a548 in cmd_execute_command_real lib/command.c:993
    #7 0x7f681495aa0c in cmd_execute_command lib/command.c:1051
    #8 0x7f681495ae8b in cmd_execute lib/command.c:1218
    #9 0x7f6814a81f1c in vty_command lib/vty.c:591
    #10 0x7f6814a823c7 in vty_execute lib/vty.c:1354
    #11 0x7f6814a8ac8d in vtysh_read lib/vty.c:2362
    #12 0x7f6814a75c8b in event_call lib/event.c:1979
    #13 0x7f68149ba445 in frr_run lib/libfrr.c:1213
    #14 0x5570e03a0d81 in main bgpd/bgp_main.c:510
    #15 0x7f68139fdc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 55 byte(s) leaked in 2 allocation(s).
***********************************************************************************
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
2023-08-17 20:42:11 +04:00
Mark Stapp
d50812edb0
Merge pull request #14218 from Pdoijode/pdoijode/frr-bgp-nexthop-find-fix
bgpd: Set ifindex to find the correct nexthop
2023-08-17 09:56:36 -04:00
Donald Sharp
6349e49645 zebra: Fix crashes in interface change
Upon some internal testing some crashes were found.  This fixes
the several crashes and normalizes the code to be closer in
it's execution pre and post changes to use the data plane.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-17 09:43:06 -04:00
Pooja Jagadeesh Doijode
e06293c395 bgpd: Set ifindex to find the correct nexthop
Problem:
    On GR helper, paths learnt from an interface based peer were linked
    to bnc with ifindex=0. During restart of GR peer, BGP (unnumbered)
    session (with GR restarter peer) goes down on GR helper but the routes
    are retained. Later, when BGP receives an interface up event, it
    will process all the paths associated with BNC whose ifindex matches the
    ifindex of the interface for which UP event is received. However, paths
    associated with bnc that has ifindex=0 were not being reinstalled since
    ifindex=0 doesn't match ifindex of any interfaces. This results in
    BGP routes not being reinstalled in zebra and kernel.

Fix:
    For paths learnt from an interface based peer, set the
    ifindex to peer's interface ifindex so that correct
    peer based nexthop can be found and linked to the path.

Signed-off-by: Donald Sharp sharpd@nvidia.com
Signed-off-by: Pooja Jagadeesh Doijode <pdoijode@nvidia.com>
2023-08-16 15:27:38 -07:00
Renato Westphal
c88ff642c4 ospf6d: introduce OSPFv3 Cryptographic Protocol ID constant
Create a constant OSPFV3_CRYPTO_PROTO_ID to replace the hard-coded
Cryptographic Protocol ID in the OSPFv3 authentication trailer
code. This enhances code clarity and maintainability.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
2023-08-16 15:58:42 -03:00
Renato Westphal
8a23a83eb6 ospf6d: fix interoperability issue in auth trailer digest computation
Ensure the OSPFv3 Cryptographic Protocol ID is encoded in network
byte order when appending it to the authentication key. This solves
interoperability issues with other implementations such as BIRD
and IOS-XR.

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
2023-08-16 15:58:42 -03:00
G. Paul Ziemba
d04cf80525 pbrd: add advisory flag PBR_ACTION_DROP
PBR configuration may specify "set nexthop blackhole" which,
    for linux dataplanes, is implemented as a table with a blackhole
    route.

    Other dataplanes might implement this action as an explicit
    packet-filtering "drop" action instead of a route. This new flag
    PBR_ACTION_DROP is now set when a rule has "set nexthop blackhole"
    as an aid to other dataplanes.

Signed-off-by: G. Paul Ziemba <paulz@labn.net>
2023-08-16 07:08:49 -07:00
Donald Sharp
bd6a00e8f7
Merge pull request #14181 from opensourcerouting/fix/bgpd_labeled_unicast_set_explicit_null
bgpd: Assign explicit-null for default-originate according to the AFI
2023-08-16 09:25:49 -04:00
Donald Sharp
fce2afe1aa
Merge pull request #14204 from opensourcerouting/fix/clear_bgp
lib: Lower precedence for ASNUM_TKN when using together with IPV4/IPV6_TKN
2023-08-16 09:25:23 -04:00
Donald Sharp
1f348e5c13
Merge pull request #14213 from opensourcerouting/fix/cli_descriptions_bgp_confederation
bgpd: Fix CLI descriptions for `bgp confederation identifier`
2023-08-16 09:24:35 -04:00
Donatas Abraitis
83a2d5ba69
Merge pull request #13623 from Keelan10/zebra-leak-fix
zebra: Delete the 'mbr_zifs' list in the if_zebra_delete_hook function
2023-08-16 11:35:24 +03:00