Two changes for debug:
1. Add a field to indicate its vrf for nexthop. When the interface changes
vrf, we can't easily know the vrf of this nexthop according to current log.
2. Add a field to indicate operation type. We can't know whether to add or
remove route according to current log.
Before:
```
zebra_nhg_increment_ref: nhe 0x555623eb82c0 (76[if 6]) 0 => 1
zebra_interface_nhg_reinstall install nhe 75[77.75.1.75 if 6] nh type 3 flags 0x1
Route 77.75.1.0/24(8) queued for processing into sub-queue Early Route Processing
Route 77.75.1.0/24(8) queued for processing into sub-queue Early Route Processing
```
After:
```
zebra_nhg_increment_ref: nhe 0x555623eb82c0 (76[if 6 vrfid 9]) 0 => 1
zebra_interface_nhg_reinstall install nhe 75[77.75.1.75 if 6 vrfid 8] nh type 3 flags 0x1
Route 77.75.1.0/24(8) (add) queued for processing into sub-queue Early Route Processing
Route 77.75.1.0/24(8) (delete) queued for processing into sub-queue Early Route Processing
```
Donald Sharp [Mon, 24 Jul 2023 00:30:47 +0000 (20:30 -0400)]
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.
Quentin Young [Mon, 24 Jul 2023 23:01:51 +0000 (19:01 -0400)]
tests: fix strncpy warning
GCC/clang warns about using strncpy in such a way that it does not copy
the null byte of a string; as implemented it was fine, but to fix the
warning, just use strlcat which was purpose made for the task being
accomplished here.
Signed-off-by: Quentin Young <qlyoung@qlyoung.net>
Donald Sharp [Mon, 24 Jul 2023 14:33:21 +0000 (10:33 -0400)]
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.
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.
zebra: fix nhg out of sync between zebra and kernel
PR#13413 introduces reinstall mechanism, but there is problem with the route
leak scenario.
With route leak configuration: ( `x1` and `x2` are binded to `vrf1` )
```
vrf vrf2
ip route 75.75.75.75/32 77.75.1.75 nexthop-vrf vrf1
ip route 75.75.75.75/32 77.75.2.75 nexthop-vrf vrf1
exit-vrf
```
Firstly, all are ok. But after `x1` is set down and up ( The interval
between the down and up operations should be less than 180 seconds. ) ,
`x1` is lost from the nexthop group:
```
anlan# ip nexthop
id 121 group 122/123 proto zebra
id 122 via 77.75.1.75 dev x1 scope link proto zebra
id 123 via 77.75.2.75 dev x2 scope link proto zebra
anlan# ip route show table 2
75.75.75.75 nhid 121 proto 196 metric 20
nexthop via 77.75.1.75 dev x1 weight 1
nexthop via 77.75.2.75 dev x2 weight 1
anlan# ip link set dev x1 down
anlan# ip link set dev x1 up
anlan# ip route show table 2 <- Wrong, one nexthop lost from group
75.75.75.75 nhid 121 via 77.75.2.75 dev x2 proto 196 metric 20
anlan# ip nexthop
id 121 group 123 proto zebra
id 122 via 77.75.1.75 dev x1 scope link proto zebra
id 123 via 77.75.2.75 dev x2 scope link proto zebra
anlan# show ip route vrf vrf2 <- Still ok
VRF vrf2:
S>* 75.75.75.75/32 [1/0] via 77.75.1.75, x1 (vrf vrf1), weight 1, 00:00:05
* via 77.75.2.75, x2 (vrf vrf1), weight 1, 00:00:05
```
From the impact on kernel:
The `nh->type` of `id 122` is *always* `NEXTHOP_TYPE_IPV4` in the route leak
case. Then, `nexthop_is_ifindex_type()` introduced by commit `5bb877` always
returns `false`, so its dependents can't be reinstalled. After `x1` is down,
there is only `id 123` in the group of `id 121`. So, Finally `id 121` remains
unchanged after `x1` is up, i.e., `id 122` is not added to the group even it is
reinstalled itself.
From the impact on zebra:
The `show ip route vrf vrf2` is still ok because the `id`s are reused/reinstalled
successfully within 180 seconds after `x1` is down and up. The group of `id 121`
is with old `NEXTHOP_GROUP_INSTALLED` flag, and it is still the group of `id 122`
and `id 123` as before.
In this way, kernel and zebra have become out of sync.
The `nh->type` of `id 122` should be adjusted to `NEXTHOP_TYPE_IPV4_IFINDEX`
after nexthop resolved. This commit is for doing this to make that reinstall
mechanism work.
Currently, json output of show ip route command are no pretty format.
This is an extremely expensive operation at high scale
(with high number of routes with many paths).
Donald Sharp [Fri, 21 Jul 2023 17:10:03 +0000 (13:10 -0400)]
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.
G. Paul Ziemba [Wed, 19 Jul 2023 14:58:02 +0000 (07:58 -0700)]
pbrd: add vlan filters pcp/vlan-id/vlan-flags; ip-protocol any (pbr feature)
Subset: feature in PBR
New PBR rule fields:
match ip-protocol (was only tcp|udp, now any value in /etc/protocols)
match pcp (0-7)
match vlan (1-4094)
match vlan (tagged|untagged|untagged-or-zero)
Filter flags
Add filter_bm (flags) field internally to indicate which
filter fields should be considered active. Bit definitions
as in lib/pbr.h.
This commit uses only the PBR_FILTER_PCP bit, but other
fields will be added in future commits. (Fixes bug related
to determining set/not-set state of pcp filter)
Shift vlan filter flags to lib/pbr.h
Changes by:
Josh Werner <joshuawerner@mitre.org>
Eli Baum <ebaum@mitre.org>
G. Paul Ziemba <paulz@labn.net>
zebra:fix a zebra crash issue caused by mac change
When the MAC address of the neighbor changes, a possible crash issue may occur.
In the zebra_evpn_local_neigh_update function, the value of old_zmac (n->mac) will be updated to the new MAC address when the neighbor's MAC address changes.
The pointer to the memory that this pointer points to may be released in the zebra_evpn_local_neigh_deref_mac function. This will cause old_zmac to become a dangling pointer. Accessing this dangling pointer in the zebra_evpn_ip_inherit_dad_from_mac function below will cause the zebra process to crash.
Here is the backtrace:
(gdb) bt
0 0x00007fc12c5f1fbf in raise () from /lib/x86_64-linux-gnu/libpthread.so.0
1 0x00007fc12d52e19c in core_handler (signo=11, siginfo=0x7ffda1fd1570, context=<optimized out>) at lib/sigevent.c:262
2 <signal handler called>
3 zebra_evpn_ip_inherit_dad_from_mac (zvrf=<optimized out>, old_zmac=0x5579ac3ca520, new_zmac=0x5579aba82f80, nbr=0x5579abd65ec0) at zebra/ze
4 0x00005579aa8dbf6d in zebra_evpn_local_neigh_update (zevpn=0x5579abb81440, ifp=ifp@entry=0x5579ab8a1640, ip=ip@entry=0x7ffda1fd1b40, macadd
local_inactive=local_inactive@entry=253, dp_static=false) at zebra/zebra_evpn_neigh.c:1729
5 0x00005579aa9190a9 in zebra_vxlan_handle_kernel_neigh_update (ifp=ifp@entry=0x5579ab8a1640, link_if=link_if@entry=0x5579abd14f90, ip=ip@ent
is_ext=is_ext@entry=false, is_router=<optimized out>, local_inactive=false, dp_static=false) at zebra/zebra_vxlan.c:3791
6 0x00005579aa8b3048 in netlink_ipneigh_change (h=0x7ffda1fd1d50, len=<optimized out>, ns_id=<optimized out>) at zebra/rt_netlink.c:3649
7 0x00005579aa8ac667 in netlink_parse_info (filter=filter@entry=0x5579aa8ab630 <netlink_information_fetch>, nl=nl@entry=0x5579ab5861e8, zns=z
startup=startup@entry=0) at zebra/kernel_netlink.c:965
8 0x00005579aa8ac8c8 in kernel_read (thread=<optimized out>) at zebra/kernel_netlink.c:402
9 0x00007fc12d53e60b in thread_call (thread=thread@entry=0x7ffda1fd9fd0) at lib/thread.c:1834
10 0x00007fc12d4fba78 in frr_run (master=0x5579ab3a1740) at lib/libfrr.c:1155
11 0x00005579aa89c6e3 in main (argc=11, argv=0x7ffda1fda3c8) at zebra/main.c:485
(gdb) f 3
3 zebra_evpn_ip_inherit_dad_from_mac (zvrf=<optimized out>, old_zmac=0x5579ac3ca520, new_zmac=0x5579aba82f80, nbr=0x5579abd65ec0) at zebra/ze
1230 zebra/zebra_evpn_neigh.c: No such file or directory.
(gdb) p *old_zmac
Cannot access memory at address 0x5579ac3ca520
(gdb)
To fix this issue, the ZEBRA_MAC_DUPLICATE flag should be retrieved before old_zmac is released and used in the zebra_evpn_ip_inherit_dad_from_mac function.
tests: Update join state in verify_upstream_iif API
When JoinState is not passed to API it is expected to
be in Joined state, there was a minor bug in API, where
it was printng JoinState as None, which is default value
in API. Updated value to print Joined when verification
fails.
When route_next return node, it has lock the node. if return or break loop, should unlock node. Signed-off-by: guozhongfeng <guozhongfeng.gzf@alibaba-inc.com>
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)
```
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
```
Abhishek N R [Thu, 13 Jul 2023 09:54:27 +0000 (02:54 -0700)]
pim6d: Fixing core while running MLD conformance test.
While running MLD conformance test 9.2 core is getting generated.
Test setps:
1. ANVL: Listen (for upto <GeneralQueryRecvWaitTime> seconds) on <AIface-0>.
2. DUT: Send MLD General Query Message.
3. ANVL: Send MLD Report Message to <DIface-0> containing:
• IPv6 Source Address field set to link-local IPv6 Address of HOST-1
• IPv6 Destination Address field set to <McastAddrGroup>
• MLD Multicast Address field set to <McastAddrGroup>.
4. ANVL: Wait for <ProcessTime> seconds for DUT to process and add <Mcas- tAddrGroup> to its Multicast Address list.
5. ANVL: Send MLD General Query Message to <DIface-0> containing:
• IPv6 Source Address field set to link-local IPv6 Address of RTR-1 which is numerically less than the link-local IPv6 unicast address of <DIface-0>
• IPv6 Destination Address field set to link-scope all-nodes multicast address.
6. ANVL: Send MLD Multicast-Address-Specific Query Message to <DIface-0> containing:
• IPv6 Source Address field set to link-local IPv6 Address of RTR-1
• IPv6 Destination Address field set to <McastAddrGroup>
• MLD Multicast Address field set to <McastAddrGroup>
• MLD Maximum Response Delay field value set to 0.
7. ANVL: Verify that the Maximum Response Delay timer for <McastAd- drGroup> is set to zero.
While running above test, when group specific query is received we start gm_t_sg_expire timer.
Once this timer expires, we clear the corresponding entry.
During this sg->state was still set to JOIN. This happened because receiver went down without sending leave.
Added a condition to update the sg->state before starting the timer.
If receiver goes down without sending leave we will update sg->state to GM_SG_JOIN_EXPIRING or GM_SG_NOPRUNE_EXPIRING based on previous state.
If we receive a join then sg->state will be refreshed and will be updated to JOIN state.
Fixes: #13387 Signed-off-by: Abhishek N R <abnr@vmware.com>
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.
Zhiyuan Wan [Mon, 3 Apr 2023 08:21:15 +0000 (16:21 +0800)]
ospfd: Support show intra-area network type in 'show ip ospf route' command
User can now use 'show ip ospf route detail' command to distinguish
intra-area stub network and transit network.
Transit network will be displayed as 'N T prefix ...'.
NOTICE: Json output format has been changed, intra-area transit networks
will have a new attribute 'transit' and value is 'true'.
And 'adv' (means advertise router) change to 'advertisedRouter'.
Example output:
bsp-debianrt-exp1# show ip ospf route detail
Codes: N - network T - transitive
IA - inter-area E - external route
D - destination R - router
============ OSPF network routing table ============
N T 10.0.0.0/24 [32] area: 0.0.0.0
via 192.168.124.67, ens192
adv 10.0.0.5
N 10.0.30.0/24 [33] area: 0.0.0.0
via 192.168.124.67, ens192
adv 10.0.0.5
...
Donald Sharp [Mon, 17 Jul 2023 14:00:32 +0000 (10:00 -0400)]
zebra: Further handle route replace semantics
When an upper level protocol is installing a route X that needs to be
route replaced and at the same time the same or another protocol installs a
different route that depends on route X for nexthop resolution can leave
us with a state where the route is not accepted because zebra is still
really early in the route replace semantics ( route X is still on the work
Queue to be processed ) then the dependent route would not be installed.
This came up in the bgp_default_originate test cases frequently.
Further extendd the ROUTE_ENTR_ROUTE_REPLACING flag to cover this case
as well. This has come up because the early route processing queueing
that was implemented late last year.
Christian Hopps [Sat, 15 Jul 2023 04:04:05 +0000 (00:04 -0400)]
tests: backtraces/cores now fail tests
Previously we just raised an error for the test file with possibly all tests
passing. Now we fail any test that produces new backtraces or cores. This
just works a lot better with analysis and even CI.
Also be less verbose to the console after failure runs, just show error
level and above logs. The log files still capture all levels (DEBUG).
Christian Hopps [Fri, 14 Jul 2023 22:21:55 +0000 (18:21 -0400)]
vtysh: track and fix file-lock use in the workaround from 2004
There's a workaround in the code from a bug from back in 2004, it ends
and re-enters config mode anytime an `exit` is done from a level below
the top-level config node (e.g., from a `router isis` node). We need to
re-enter config mode with or without a lock according to how we actually
entered it to begin with.
Christian Hopps [Fri, 14 Jul 2023 20:13:54 +0000 (16:13 -0400)]
lib: mgmtd: only clear pending for the in-progress command
The lock/unlocks are being done short-circuit so they are never pending;
however, the handling of the unlock notification was always resuming the command
if pending was set. In all cases pending is set for another command. For example
implicit commit locks then when notified its done unlocks which was clearing the
set-config pending flag and resuming that command incorrectly.
Donald Sharp [Fri, 14 Jul 2023 16:14:20 +0000 (12:14 -0400)]
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.
projects / mirror / frr.git / log