Commit Graph

6965 Commits

Author SHA1 Message Date
Donatas Abraitis
ceea81be77
Merge pull request #14139 from donaldsharp/v6_v4_nexthops
V6 v4 nexthops
2023-08-06 20:11:19 +03:00
Donald Sharp
e30a10cff5
Merge pull request #14140 from opensourcerouting/fix/handle_more_capabilities_via_dynamic_capability
bgpd: Handle software version capability dynamicaly
2023-08-04 11:26:43 -04:00
Donald Sharp
b036c510f2
Merge pull request #14145 from opensourcerouting/fix/handling_cluster_transit
bgpd: Some cleanup and consistency "improvements"
2023-08-04 07:14:55 -04:00
Donatas Abraitis
dd58cd4d38 bgpd: Use SET_FLAG when setting AIGP attribute flag
Just reuse an existing more-readable code.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-03 22:54:54 +03:00
Donatas Abraitis
fa2749f58e bgpd: Handle srv6 attributes the same way as others using setters/getters
To be consistent and error-safe.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-03 22:53:21 +03:00
Donatas Abraitis
312b8c02a6 bgpd: Handle encap attributes the same way as others using setters/getters
To be consistent and error-safe.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-03 22:52:09 +03:00
Donatas Abraitis
09b4537755 bgpd: Handle transit attributes the same way as others using setters/getters
To be consistent and error-safe.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-03 22:48:40 +03:00
Donatas Abraitis
0a0137da85 bgpd: Handle cluster attribute the same way as others using setters/getters
To be consistent and error-safe.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-03 22:44:17 +03:00
Donatas Abraitis
7636bcc765 bgpd: Check if we have such a peer before handling software capability
Do not pass NULL for peer_established(), just in case.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-03 22:00:27 +03:00
Donatas Abraitis
b2b47bb4a9 bgpd: Remove deprecated COMMUNITY_INTERNET community
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-03 17:12:19 +03:00
Donatas Abraitis
f3279abe13 bgpd: Add all other capabilities for dynamic handling (placeholders)
Gonna be covered later with further PRs. Now adding them to avoid compiler
errors due to uncovered switch/cases.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-03 17:08:33 +03:00
Donatas Abraitis
bf11a9eb25 bgpd: Handle software version capability dynamicaly
We have dynamic capability support, but it handles only MP capability.

With this change, we can enable software version capability dynamicaly, without
resetting the session.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-03 17:08:33 +03:00
Donald Sharp
0435b31bb8 bgpd: Allow bgp to specify if it will allow v6 routing with v4 nexthops
Add a `--v6-with-v4-nexthop` cli to bgp to allow it to peer with
neighbors in the configuration where the interface has no v6 addresses
at all and there is a v4 address that is usable as a v4 address
embedded in a v6 address.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-03 08:25:20 -04:00
Donald Sharp
95002ded3e bgpd: Do not allow a peer to come up on v6 if we have no ability to route
Modify bgp to not allow a v6 peer to come up if the v6 afi is negotiated
and the outgoing interface has no v6 address as well as zebra does
not support the v6 with v4 nexthop capabilities that some dataplanes
allow.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-03 08:25:20 -04:00
Donald Sharp
052debc3ee bgpd: Have bgp notice the zebra ability to use v6_with_v4_nexthops
Store the data.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-08-03 08:25:20 -04:00
Donald Sharp
7415f1e120
Merge pull request #14129 from samanvithab/bgpd_frr_fix
bgpd: Fix for session reset issue caused by malformed core attributes  in update message
2023-08-02 13:48:14 -04:00
Samanvitha B Bhargav
70ff940fd1 bgpd: Fix session reset issue caused by malformed core attributes
RCA:
On encountering any attribute error for core attributes in update message,
the error handling is set to 'treat as withdraw' and
further parsing of the remaining attributes is skipped.
But the stream pointer is not being correctly adjusted to
point to the next NLRI field skipping the rest of the attributes.
This leads to incorrect parsing of the NLRI field,
which causes BGP session to reset.

Fix:
The stream pointer offset is rightly adjusted to point to the NLRI field correctly
when the malformed attribute is encountered and remaining attribute parsing is skipped.

Signed-off-by: Samanvitha B Bhargav <bsamanvitha@vmware.com>
2023-08-01 23:17:19 -07:00
Donatas Abraitis
27dbf81a73 lib: Do not use time_t as a special Lua encoder/decoder
This is purely an integer (long long/long), and causes issues for 32-bit systems.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-08-01 14:08:25 +03:00
Farid Mihoub
902a8d1fd3 bgpd: add set extended-comm-list <> delete command
Signed-off-by: Farid Mihoub <farid.mihoub@6wind.com>
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2023-07-31 11:52:36 +02:00
Trey Aspelund
62a452c47f bgpd: skip reset when removing dup update-source
When 'no neighbor .. update-source' is issued for a regular peer, that
peer is always reset.  This is unnecessary if the peer is a member of a
peer-group and it inherits an identical update-source, so let's skip
the reset/Notification for that condition.

Config:
------------
router bgp 1
 neighbor PG peer-group
 neighbor PG remote-as internal
 neighbor PG update-source 100.64.0.3
 neighbor 192.168.122.99 peer-group PG
 neighbor 192.168.122.99 update-source 100.64.0.3

Before:
------------
ub20-2(config-router)# do show ip bgp sum | include .99
192.168.122.99  4          1        36        34        0    0    0 00:00:17            0        0 N/A
ub20-2(config-router)# do show ip bgp neighbors 192.168.122.99 | include Local host
Local host: 100.64.0.3, Local port: 46083
ub20-2(config-router)# no neighbor 192.168.122.99 update-source
ub20-2(config-router)# do show ip bgp sum | include .99
192.168.122.99  4          1        36        35        0    0    0 00:00:01         Idle        0 N/A
ub20-2(config-router)# do show ip bgp neighbors 192.168.122.99 | include Local host
Local host: 100.64.0.3, Local port: 39847

After:
------------
ub20-2(config-router)# do show ip bgp sum | include .99
192.168.122.99  4          1         3         3        0    0    0 00:00:20            0        0 N/A
ub20-2(config-router)# do show ip bgp neighbors 192.168.122.99 | include Local host
Local host: 100.64.0.3, Local port: 39415
ub20-2(config-router)# no neighbor 192.168.122.99 update-source
ub20-2(config-router)# do show ip bgp sum | include .99
192.168.122.99  4          1         3         3        0    0    0 00:00:28            0        0 N/A
ub20-2(config-router)# do show ip bgp neighbors 192.168.122.99 | include Local host
Local host: 100.64.0.3, Local port: 39415

Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
2023-07-28 16:34:55 +00:00
mobash-rasool
49f0484113
Merge pull request #14064 from donaldsharp/pim_cleanup
Cleanup from examining gcov runs
2023-07-26 21:33:29 +05:30
Russ White
3f043d027f
Merge pull request #14050 from LabNConsulting/ziemba-pbr-zapi-common
pbrd: 2/3 zapi PBR common encode/decode
2023-07-25 10:55:50 -04:00
Russ White
0095023cc4
Merge pull request #14081 from donaldsharp/bgp_ringbuf_cleanup
Bgp ringbuf cleanup
2023-07-25 10:24:12 -04:00
Mark Stapp
d2c0d224b9
Merge pull request #14079 from donaldsharp/last_reset_cause_slimming
bgpd: The last_reset_cause in the peer structure is too large
2023-07-25 09:25:43 -04:00
Donald Sharp
73b66bed83 bgpd: The last_reset_cause in the peer structure is too large
The last_reset_cause is a plain old BGP_MAX_PACKET_SIZE buffer
that is really enlarging the peer data structure.  Let's just
copy the stream that failed and only allocate how ever much
the packet size actually was.  While it's likely that we have
a reset reason, the packet typically is not going to be 65k
in size.  Let's save space.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-07-24 22:41:14 -04:00
Sindhu Parvathi Gopinathan
e784a60b17 bgpd: non pretty json output for neighbor routes
Currently, json output of show BGP commands are no pretty format.

This is an extremely expensive operation for huge scale (lots of
routes with lots of paths).

BGP json non-pretty commands support added:

```
show bgp neighbors <nbr-id> advertised-routes json
show bgp neighbors <nbr-id> received-routes json
show bgp neighbors <nbr-id> advertised-routes detail json
show bgp neighbors <nbr-id> received-routes detail json
```

Ticket:#3513256
Issue:3513256

Testing: UT done

Signed-off-by: Sindhu Parvathi Gopinathan's <sgopinathan@nvidia.com>
2023-07-24 10:50:45 -07:00
Donald Sharp
fe1c72a573 bgpd: Reduce size of ibuf_work ringbuf
The ringbuf is 650k in size.  This is obscenely large and
in practical experimentation FRR never even approaches
that size at all.  Let's reduce this to 1.5 max packet sizes.

If a BGP_MAX_PACKET_SIZE packet is ever received having a bit
of extra space ensures that we can read at least 1 packet.

This also will significantly reduce memory usage when the
operator has a lot of peers.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-07-24 10:41:00 -04:00
Donald Sharp
102bad0a9b bgpd: With io limit allow parsing to continue even if memory is low
Commit: a0b937de42

Introduced the idea of a input Q packet limit.  Say you read in
635000 bytes of data and the input Q is already at it's limit
(currently 1000) then when bgp_process_reads runs it will
assert because there is less then a BGP_MAX_PACKET_SIZE in ibuf_work.

Don't assert as that it's irrelevant.  Even if we can't read a full packet
in let's let the whole system keep working as that as the input Q length
comes down we will start pulling down the ibuf_work and it will be ok.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-07-24 10:41:00 -04:00
Donald Sharp
bdc1762405 bgpd: Replace peer->ibuf_scratch
The peer->ibuf_scratch was allocating 65535 * 10 bytes
for scratch space to hold data incoming from a read
from a peer.  When you have 4k peers this is 262,1400,000
or 262 mb of data.  Which is crazy large.  Especially
since the i/o pthread is reading per peer without
any chance of having the data interfere with other reads.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-07-21 13:10:03 -04:00
Donald Sharp
4215e8011b bgpd: Remove withdraw_low from system, it is never used
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-07-21 12:46:31 -04:00
Donald Sharp
c81d6d4d5f bgpd: Remove peer->sync array
It is never used.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-07-21 12:41:35 -04:00
Donald Sharp
acf4defcd8 bgpd: Remove peer->obuf_work
This is never used.  Free up another 65k of stream data
never used per peer.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-07-21 12:30:20 -04:00
Donald Sharp
b157af0ac1 bgpd: Remove peer->scratch
This was only ever being allocated and de-allocated.
Let's save 65k per peer

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-07-21 12:14:59 -04:00
Donald Sharp
cc66dff0a3 bgpd: Cleanup bgp_zebra_announce_default to be cleaner
Over time the bgp_zebra_announce_default function has gotten
slightly convoluted, clean it up so it's easier to read

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-07-21 07:31:04 -04:00
G. Paul Ziemba
580a98b798 lib: zapi PBR common encode/decode
bgpd, pbrd: use common pbr encoder
    zebra: use common pbr decoder
    tests: pbr_topo1: check more filter fields

    Purpose:
	1. Reduce likelihood of zapi format mismatches when adding
	   PBR fields due to multiple parallel encoder implementations
	2. Encourage common PBR structure usage among various daemons
	3. Reduce coding errors via explicit per-field enable flags

Signed-off-by: G. Paul Ziemba <paulz@labn.net>
2023-07-20 08:10:45 -07:00
Donald Sharp
1b1d256f03
Merge pull request #14026 from LabNConsulting/pbr-add-vlan-filters
pbrd: 1/3: add vty support for vlan filtering and send to zebra
2023-07-20 08:01:24 -04:00
G. Paul Ziemba
dbade07e0e pbrd: add vlan filters pcp/vlan-id/vlan-flags; ip-protocol any (zapi)
Subset: ZAPI changes to send the new data

    Also adds filter_bm field; currently for PBR_FILTER_PCP, but in the
    future to be used for all of the filter fields.

    Changes by:
	Josh Werner <joshuawerner@mitre.org>
	Eli Baum <ebaum@mitre.org>
	G. Paul Ziemba <paulz@labn.net>

Signed-off-by: G. Paul Ziemba <paulz@labn.net>
2023-07-19 08:14:49 -07:00
Donatas Abraitis
5a69379e81
Merge pull request #13951 from fdumontet6WIND/as_path_exclude_reg
bgpd: add set as-path exclude acl-list command
2023-07-19 16:35:33 +03:00
Francois Dumontet
4685db418e bgpd: add set as-path exclude acl-list command
A route-map applied on incoming BGP updates is not able
to exclude the unwanted as segments, based on an AS path
access-list.

The below configuration illustrates the case:

router bgp 65001

address-family ipv4 unicast
 neighbor 192.168.1.2 route-map rule_2 in
exit-address-family

bgp as-path access-list RULE permit ^65

route-map rule_2 permit 10
 set as-path exclude as-path-access-list RULE

```
BGP routing table entry for 10.10.10.10/32, version 13
Paths: (1 available, best #1, table default)
  Advertised to non peer-group peers:
  192.168.10.65
  65000 1 2 3 123
    192.168.10.65 from 192.168.10.65 (10.10.10.11)
      Origin IGP, metric 0, valid, external, best (First path received)
```

After:

```
do show ip bgp 10.10.10.10/32
BGP routing table entry for 10.10.10.10/32, version 15
    Paths: (1 available, best #1, table default)
      Advertised to non peer-group peers:
      192.168.10.65
      2 3 123
        192.168.10.65 from 192.168.10.65 (10.10.10.11)
          Origin IGP, metric 0, valid, external, best (First path
received)
```

Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
2023-07-19 10:57:48 +02:00
Donatas Abraitis
721eba0047 bgpd: Initialize bgp_nht_ifp_table_handle() bools
Assuming field 'ifindex_ipv6_ll' is not equal to field 'ifindex', then
nhop_found is just a garbage, let's avoid that.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-07-18 20:45:47 +03:00
Jafar Al-Gharaibeh
975b8e5e9c
Merge pull request #13918 from rameshabhinay/evpn_memleak
bgpd: fix bgp evpn cli memory leaks.
2023-07-18 10:59:20 -05:00
Donald Sharp
4a986f20d7
Merge pull request #14041 from opensourcerouting/fix/memory_leak_bgp_redistribute_add
bgpd: Do not try to redistribute routes if we are shutting down
2023-07-18 10:55:22 -04:00
Russ White
3d91ef63c1
Merge pull request #14015 from opensourcerouting/fix/bgpd_refactor_bgp_update_withdraw_safi
bgpd: Refactor a bit bgp_{update,withdraw} functions
2023-07-18 09:42:07 -04:00
Donald Sharp
37940809ad
Merge pull request #14037 from opensourcerouting/feature/enable_software_capability_for_datacenter_profile_by_default
bgpd: Enable software version capability for datacenter profile
2023-07-18 09:19:19 -04:00
Russ White
8593341164
Merge pull request #13868 from zice312963205/zly_mapflag
bgpd:Fixing a conflict issue between rmap_in_change_flag and rmap_out…
2023-07-18 09:14:50 -04:00
Russ White
204bbf1b3c
Merge pull request #13974 from pguibert6WIND/bgp_nht_other_if_cases_2
Bgp nht other if cases 2
2023-07-18 09:10:50 -04:00
Donatas Abraitis
bddf5c13b2 bgpd: Do not try to redistribute routes if we are shutting down
When switching `router bgp`, `no router bgp` and doing redistributions, we should
ignore this action, otherwise memory leak happens:

```
Indirect leak of 400 byte(s) in 2 object(s) allocated from:
    0 0x7f81b36b3a06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
    1 0x7f81b327bd2e in qcalloc lib/memory.c:105
    2 0x55f301d28628 in bgp_node_create bgpd/bgp_table.c:92
    3 0x7f81b3309d0b in route_node_new lib/table.c:52
    4 0x7f81b3309d0b in route_node_set lib/table.c:61
    5 0x7f81b330be0a in route_node_get lib/table.c:319
    6 0x55f301ce89df in bgp_redistribute_add bgpd/bgp_route.c:8907
    7 0x55f301dac182 in zebra_read_route bgpd/bgp_zebra.c:593
    8 0x7f81b334dcd7 in zclient_read lib/zclient.c:4179
    9 0x7f81b331d702 in event_call lib/event.c:1995
    10 0x7f81b325d597 in frr_run lib/libfrr.c:1213
    11 0x55f301b94b12 in main bgpd/bgp_main.c:505
    12 0x7f81b2b57082 in __libc_start_main ../csu/libc-start.c:308
```

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-07-18 13:28:01 +03:00
Abhinay Ramesh
56b895c172 bgpd: fix bgp evpn cli memory leaks.
problem:
In CLI config codeflow there are memory leaks in failure scenario

Fix:
Code changes are done to free ecommunity

Signed-off-by: Abhinay Ramesh <rabhinay@vmware.com>
2023-07-18 10:15:19 +00:00
Donatas Abraitis
30db544508 bgpd: Send software-version capability by default
Useful to have it for datacenter profile only, disabled for traditional.

If the peer is not established or established, but has no description set,
we will show the FRR version instead, which is kinda handy to have instead of
nothing.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-07-18 09:42:48 +03:00
Donald Sharp
2ba2c284ba bgpd: Prevent use after free
When running bgp_always_compare_med, I am frequently seeing a crash
After running with valgrind I am seeing this and a invalid write
immediately after this as well.

==311743== Invalid read of size 2
==311743==    at 0x4992421: route_map_counter_decrement (routemap.c:3308)
==311743==    by 0x35664D: peer_route_map_unset (bgpd.c:7259)
==311743==    by 0x306546: peer_route_map_unset_vty (bgp_vty.c:8037)
==311743==    by 0x3066AC: no_neighbor_route_map (bgp_vty.c:8081)
==311743==    by 0x49078DE: cmd_execute_command_real (command.c:990)
==311743==    by 0x4907A63: cmd_execute_command (command.c:1050)
==311743==    by 0x490801F: cmd_execute (command.c:1217)
==311743==    by 0x49C5535: vty_command (vty.c:551)
==311743==    by 0x49C7459: vty_execute (vty.c:1314)
==311743==    by 0x49C97D1: vtysh_read (vty.c:2223)
==311743==    by 0x49BE5E2: event_call (event.c:1995)
==311743==    by 0x494786C: frr_run (libfrr.c:1204)
==311743==    by 0x1F7655: main (bgp_main.c:505)
==311743==  Address 0x9ec2180 is 64 bytes inside a block of size 120 free'd
==311743==    at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==311743==    by 0x495A1BA: qfree (memory.c:130)
==311743==    by 0x498D412: route_map_free_map (routemap.c:748)
==311743==    by 0x498D176: route_map_add (routemap.c:672)
==311743==    by 0x498D79B: route_map_get (routemap.c:857)
==311743==    by 0x499C256: lib_route_map_create (routemap_northbound.c:102)
==311743==    by 0x49702D8: nb_callback_create (northbound.c:1234)
==311743==    by 0x497107F: nb_callback_configuration (northbound.c:1578)
==311743==    by 0x4971693: nb_transaction_process (northbound.c:1709)
==311743==    by 0x496FCF4: nb_candidate_commit_apply (northbound.c:1103)
==311743==    by 0x496FE4E: nb_candidate_commit (northbound.c:1136)
==311743==    by 0x497798F: nb_cli_classic_commit (northbound_cli.c:49)
==311743==    by 0x4977B4F: nb_cli_pending_commit_check (northbound_cli.c:88)
==311743==    by 0x49078C1: cmd_execute_command_real (command.c:987)
==311743==    by 0x4907B44: cmd_execute_command (command.c:1068)
==311743==    by 0x490801F: cmd_execute (command.c:1217)
==311743==    by 0x49C5535: vty_command (vty.c:551)
==311743==    by 0x49C7459: vty_execute (vty.c:1314)
==311743==    by 0x49C97D1: vtysh_read (vty.c:2223)
==311743==    by 0x49BE5E2: event_call (event.c:1995)
==311743==    by 0x494786C: frr_run (libfrr.c:1204)
==311743==    by 0x1F7655: main (bgp_main.c:505)
==311743==  Block was alloc'd at
==311743==    at 0x484DA83: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==311743==    by 0x495A068: qcalloc (memory.c:105)
==311743==    by 0x498D0C8: route_map_new (routemap.c:646)
==311743==    by 0x498D128: route_map_add (routemap.c:658)
==311743==    by 0x498D79B: route_map_get (routemap.c:857)
==311743==    by 0x499C256: lib_route_map_create (routemap_northbound.c:102)
==311743==    by 0x49702D8: nb_callback_create (northbound.c:1234)
==311743==    by 0x497107F: nb_callback_configuration (northbound.c:1578)
==311743==    by 0x4971693: nb_transaction_process (northbound.c:1709)
==311743==    by 0x496FCF4: nb_candidate_commit_apply (northbound.c:1103)
==311743==    by 0x496FE4E: nb_candidate_commit (northbound.c:1136)
==311743==    by 0x497798F: nb_cli_classic_commit (northbound_cli.c:49)
==311743==    by 0x4977B4F: nb_cli_pending_commit_check (northbound_cli.c:88)
==311743==    by 0x49078C1: cmd_execute_command_real (command.c:987)
==311743==    by 0x4907B44: cmd_execute_command (command.c:1068)
==311743==    by 0x490801F: cmd_execute (command.c:1217)
==311743==    by 0x49C5535: vty_command (vty.c:551)
==311743==    by 0x49C7459: vty_execute (vty.c:1314)
==311743==    by 0x49C97D1: vtysh_read (vty.c:2223)
==311743==    by 0x49BE5E2: event_call (event.c:1995)
==311743==    by 0x494786C: frr_run (libfrr.c:1204)

Effectively the route_map that is being stored has been freed already
but we have not cleaned up properly yet.  Go through and clean the
code up by ensuring that the pointer actually exists instead of trusting
it does when doing the decrement operation.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-07-14 12:16:38 -04:00