bgpd: bgp_connected_delete needs to ensure dest is still there

Again coverity believes that dest could be freed by a call
into bgp_dest_unlock_node, and it can if the lock count
is wrong.  Let's fix that assumption for coverity

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: bgp_cleanup_routes ensure dest is not freed

The bgp_cleanup_routes function holds the lock for dest
while walking it.  Ensure that coverity understands this
proposition.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: bgp_evpn_es_route_del_all should not free dest until after looping

Again the dest pointer should be still locked by the table walk.  Ensure
that coverity is happy that this is not happening.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: bgp_evpn_mh_route_delete should ensure dest is still usable

Again coverity believes that dest may be freed but it should not
be because of how locking is done.  Make coverity happy.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: delete_vin_type2_route may free dest

The dest pointer may be freed( but should not be
due to locking ).  Let's ensure that this assumption
is true and make coverity happy.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: delete_evpn_route ensure that dest is not freed before usage

There exist two spots in this function where the dest could be
freed, but is not due to locking, but coverity thinks it might
so let's make the function happy.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: evpn_cleanup_local_non_best_route could free dest

But never really does due to locking, but since it can
we need to treat it like it does and ensure that FRR
is not making a mistake, by using memory after it
has been freed.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: bgp_clear_adj_in|remove dest may be freed

dest will not be freed due to lock but coverity does not know
that.  Give it a hint.  This change includes modifying bgp_dest_unlock_node
to return the dest pointer so that we can determine if we should
continue working on dest or not with an assert.  Since this
is lock based we should be ok.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

Merge pull request #14377 from mjstapp/nl_rule_valid_action

zebra: ignore iprule requests for unsupported actions

zebra: ignore iprule requests for unsupported actions

Only attempt to install in netlink iprules that include supported
actions; ignore requests with actions that aren't supported by
netlink.

Signed-off-by: Mark Stapp <mjs@labn.net>

Merge pull request #14360 from opensourcerouting/fix/as_path_no_prepend

bgpd: Fix `no set as-path prepend` command for BGP

Merge pull request #14358 from donaldsharp/tc_possible_crash

zebra: Prevent Null pointer deref

Merge pull request #14357 from donaldsharp/no_need_anymore

bgpd: Fix `no set as-path prepend` command for BGP

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #14261 from Keelan10/ospfd-memleak

ospfd: fix area range memory leak

Merge pull request #14225 from opensourcerouting/bug/topostest_grpc

topotest: Command using wrong python version

zebra: Prevent Null pointer deref

If the kernel sends us bad data then the kind_str
will be NULL and a later strcmp operation will
cause a crash.

As a note: If the kernel is not sending us properly
formated netlink messages then we got bigger problems
than zebra crashing.  But at least let's prevent zebra
from crashing.

Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donald Sharp <sharpd@nvidia.com>

Merge pull request #14344 from opensourcerouting/freebsd-printf-sync-20230903

lib/printf: sync with FreeBSD for ISO C23 enhancements

Merge pull request #14349 from opensourcerouting/fix/update_doc_for_docker

doc: Reuse $TAG for git checkout when building Docker images

*: Remove unused configure option

The `-disable-rr-semantics` or `--enable-rr-senamtics` configure
option is never used.  Let's just remove it.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

Merge pull request #14285 from opensourcerouting/feature/graceful_restart_dynamic_capability

bgpd: Handle Graceful Restart capability using dynamic capabilities

Merge pull request #14341 from opensourcerouting/fix/bgpd_BGP_ATTR_PMSI_TUNNEL_treat_as_withdraw

bgpd: AS4_PATH and PMSI tunnel attributes handling by RFC 7606

Merge pull request #14318 from raja-rajasekar/frr_dev1

zebra: Fix zebra crash when replacing NHE during shutdown

Merge pull request #14234 from Pdoijode/pdoijode/frr-bgp-nexthop-find-fix-1

bgpd: set ifindex only for v6 nexthops and nexthops that match peer's LL

ospfd: fix area range memory leak

Addressed a memory leak in OSPF by fixing the improper deallocation of
area range nodes when removed from the table. Introducing a new function,
`ospf_range_table_node_destroy` for proper node cleanup, resolved the issue.

The ASan leak log for reference:

```
Direct leak of 56 byte(s) in 2 object(s) allocated from:
    #0 0x7faf661d1d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
    #1 0x7faf65bce1e9 in qcalloc lib/memory.c:105
    #2 0x55a66e0b61cd in ospf_area_range_new ospfd/ospf_abr.c:43
    #3 0x55a66e0b61cd in ospf_area_range_set ospfd/ospf_abr.c:195
    #4 0x55a66e07f2eb in ospf_area_range ospfd/ospf_vty.c:631
    #5 0x7faf65b51548 in cmd_execute_command_real lib/command.c:993
    #6 0x7faf65b51f79 in cmd_execute_command_strict lib/command.c:1102
    #7 0x7faf65b51fd8 in command_config_read_one_line lib/command.c:1262
    #8 0x7faf65b522bf in config_from_file lib/command.c:1315
    #9 0x7faf65c832df in vty_read_file lib/vty.c:2605
    #10 0x7faf65c83409 in vty_read_config lib/vty.c:2851
    #11 0x7faf65bb0341 in frr_config_read_in lib/libfrr.c:977
    #12 0x7faf65c6cceb in event_call lib/event.c:1979
    #13 0x7faf65bb1488 in frr_run lib/libfrr.c:1213
    #14 0x55a66dfb28c4 in main ospfd/ospf_main.c:249
    #15 0x7faf651c9c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

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

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>

doc: Reuse $TAG for git checkout when building Docker images

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #14346 from opensourcerouting/frr-format-gcc-13

tools/gcc-plugins: fix for GCC 13

Merge pull request #14340 from mjstapp/txqlen_info

lib,zebra: add tx queuelen to interface struct

tools/gcc-plugins: fix for GCC 13

As usual, new GCC version, new small random changes in the API.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

lib/printf: update README

Make a note that `%b` was picked up.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

tests: exercise `%b` printfrr format specifier

Added by ISO C23 / N2630, implementation imported from FreeBSD.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

tools/gcc-plugins: allow ISO C23 `%b`

It behaves exactly like `%x`, so adding it is a single byte diff.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

lib/printf: Implement N2630.

This adds formatted input/output of binary integer numbers to the
printf(), scanf(), and strtol() families, including their wide-character
counterparts.

Reviewed by: imp, emaste
Differential Revision: https://reviews.freebsd.org/D41511

FRR changes only include printf(), scanf/strtol are not locally
implemented in FRR.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from FreeBSD commit d9dc1603d6e48cca84cad3ebe859129131b8387c)

lib/printf: Remove $FreeBSD$: two-line .h pattern

Remove /^\s*\*\n \*\s+\$FreeBSD\$$\n/

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from FreeBSD commit b3e7694832e81d7a904a10f525f8797b753bf0d3)

lib/printf: drop "All rights reserved" from Foundation copyrights

This has already been done for most files that have the Foundation as
the only listed copyright holder.  Do it now for files that list
multiple copyright holders, but have the Foundation copyright in its own
section.

Sponsored by: The FreeBSD Foundation

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from FreeBSD commit 5b5fa75acff11d871d0c90045f8c1a58fed85365)

bgpd: Treat as4-path (17) attribute as withdraw if malformed

rfc7606 defines:

Attributes 17 (AS4_PATH), 18 (AS4_AGGREGATOR), 22 (PMSI_TUNNEL), 23 (Tunnel
   Encapsulation Attribute), 26 (AIGP), 27 (PE Distinguisher Labels),
   and 29 (BGP-LS Attribute) do have error handling consistent with
   Section 8 and thus are not further discussed herein.

Section 8 defines:

The "treat-as-withdraw" approach is generally
   preferred and the "session reset" approach is discouraged.
For any malformed attribute that is handled by the "attribute
   discard" instead of the "treat-as-withdraw" approach, it is critical
   to consider the potential impact of doing so.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

lib,zebra: add tx queuelen to interface struct

Add the txqlen attribute to the common interface struct. Capture
the value in zebra, and distribute it through the interface lib
module's zapi messaging.

Signed-off-by: Mark Stapp <mjs@labn.net>

bgpd: Treat PMSI tunnel attribute as withdrawn if malformed

https://datatracker.ietf.org/doc/html/rfc6514#page-10 states:

A router that supports the PMSI Tunnel attribute considers this
   attribute to be malformed if either (a) it contains an undefined
   tunnel type in the Tunnel Type field of the attribute, or (b) the
   router cannot parse the Tunnel Identifier field of the attribute as a
   tunnel identifier of the tunnel types specified in the Tunnel Type
   field of the attribute.

When a router that receives a BGP Update that contains the PMSI
   Tunnel attribute with its Partial bit set determines that the
   attribute is malformed, the router SHOULD treat this Update as though
   all the routes contained in this Update had been withdrawn.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #14294 from donaldsharp/pim_crashola

pimd: Prevent crash when receiving register message when the RP() is …

Merge pull request #14327 from ryonkn/fix_show_bgp_rpki_notfound

bgpd: Fix `show bgp all rpki notfound`

bgpd: Fix `show bgp all rpki notfound`

The command "show bgp all rpki notfound" includes not only RPKI
notfound routes but also RPKI valid and invalid routes in its results.

Fix the code to display only RPKI notfound routes.

Old output:
```
frr# show bgp all rpki notfound

For address family: IPv4 Unicast
BGP table version is 0, local router ID is 10.0.0.1, vrf id 0
Default local pref 100, local AS 64512
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

    Network          Next Hop            Metric LocPrf Weight Path
N   x.x.x.0/18       a.a.a.a                       100      0 64513 i
V   y.y.y.0/19       a.a.a.a                       200      0 64513 i
I   z.z.z.0/16       a.a.a.a                        10      0 64513 i

Displayed  3 routes and 3 total paths
```

New output:
```
frr# show bgp all rpki notfound

For address family: IPv4 Unicast
BGP table version is 0, local router ID is 10.0.0.1, vrf id 0
Default local pref 100, local AS 64512
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

    Network          Next Hop            Metric LocPrf Weight Path
N   x.x.x.0/18       a.a.a.a                       100      0 64513 i

Displayed  1 routes and 3 total paths
```

Signed-off-by: Ryo Nakano <ryo.z.nakano@gmail.com>

Merge pull request #14325 from donaldsharp/peerhash_take_two

bgpd: Add peers back to peer hash when peer_xfer_conn fails

Merge pull request #14323 from donaldsharp/name_pretty

bgpd: When using `show bgp peerhash` don't display (NULL)

bgpd: Add peers back to peer hash when peer_xfer_conn fails

It was noticed that occassionally peering failed in a testbed
upon investigation it was found that the peer was not in the
peer hash and we saw these failure messages:

Aug 25 21:31:15 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: %NOTIFICATION: sent to neighbor 2001:cafe:1ead:4::4 4/0 (Hold Timer Expired) 0 bytes
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 100663299] Can't get remote address and port: Transport endpoint is not connected
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 100663299] %bgp_getsockname() failed for  peer 2001:cafe:1ead:4::4 fd 27 (from_peer fd -1)
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 33554464] %Neighbor failed in xfer_conn

root@doca-hbn-service-bf3-s06-1-ipmi:/var/log/hbn/frr# vtysh -c 'show bgp peerhash' | grep 2001:cafe:1ead:4::4
root@doca-hbn-service-bf3-s06-1-ipmi:/var/log/hbn/frr#

Upon looking at the code the peer_xfer_conn function can fail
and the bgp_establish code will then return before adding the
peer back to the peerhash.

This is only part of the failure.  The peer also appears to
be in a state where it is no longer initiating connection attempts
but that will be another commited fix when we figure that one out.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: When using `show bgp peerhash` don't display (NULL)

Fix up the output to not display a (NULL) output for the bgp name

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

Merge pull request #14322 from opensourcerouting/fix/revert_6f8c927b03c454aa309b84cefccc4faa31e0c03f

Revert "bgpd: Add peers back to peer hash when peer_xfer_conn fails"

Revert "bgpd: Add peers back to peer hash when peer_xfer_conn fails"

peer is NULL, but we pass it to hash_get().

This reverts commit 6f8c927b03c454aa309b84cefccc4faa31e0c03f.

pimd: Prevent crash when receiving register message when the RP() is unknown

When receiving a register message for a Group, that the group has no
associated RP specified.  Prevent a crash from happening.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

Merge pull request #14282 from pguibert6WIND/fix_redistribute_table_flush

bgpd: fix redistribute table command after bgp restarts

zebra: Fix zebra crash when replacing NHE during shutdown

During replace of a NHE from upper proto in zebra_nhg_proto_add(),
 - rib_handle_nhg_replace() is invoked with old NHE where we walk all
   RNs/REs & replace the re->nhe whose address points to old NHE.
 - In this walk, if prev re->nhe refcnt is decremented to 0, we free up
   the memory which the old NHE is pointing to.
Later in zebra_nhg_proto_add(), we end up accessing this freed memory
and crash.

Logs:
1380766 2023/08/16 22:34:11.994671 ZEBRA: [WDEB1-93HCZ] zebra_nhg_decrement_ref: nhe 0x56091d890840 (70312519[2756/2762/2810]) 2 => 1
1380773 2023/08/16 22:34:11.994678 ZEBRA: [WDEB1-93HCZ] zebra_nhg_decrement_ref: nhe 0x56091d890840 (70312519[2756/2762/2810]) 1 => 0
1380777 2023/08/16 22:34:11.994844 ZEBRA: [JE46R-G2NEE] zebra_nhg_release: nhe 0x56091d890840 (70312519[2756/2762/2810])
1380778 2023/08/16 22:34:11.994849 ZEBRA: [SCDBM-4H062] zebra_nhg_free: nhe 0x56091d890840 (70312519[2756/2762/2810]), refcnt 0
1380782 2023/08/16 22:34:11.995000 ZEBRA: [SCDBM-4H062] zebra_nhg_free: nhe 0x56091d890840 (0[]), refcnt 0
1380783 2023/08/16 22:34:11.995011 ZEBRA: lib/memory.c:84: mt_count_free(): assertion (mt->n_alloc) failed

Backtrace:
0  0x00007f833f5f48eb in raise () from /lib/x86_64-linux-gnu/libc.so.6
1  0x00007f833f5df535 in abort () from /lib/x86_64-linux-gnu/libc.so.6
2  0x00007f833f636648 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
3  0x00007f833f63cd6a in ?? () from /lib/x86_64-linux-gnu/libc.so.6
4  0x00007f833f63cfb4 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
5  0x00007f833f63fbc8 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
6  0x00007f833f64172a in malloc () from /lib/x86_64-linux-gnu/libc.so.6
7  0x00007f833f6c3fd2 in backtrace_symbols () from /lib/x86_64-linux-gnu/libc.so.6
8  0x00007f833f9013fc in zlog_backtrace_sigsafe (priority=priority@entry=2, program_counter=program_counter@entry=0x7f833f5f48eb <raise+267>) at lib/log.c:222
9  0x00007f833f901593 in zlog_signal (signo=signo@entry=6, action=action@entry=0x7f833f988ee8 "aborting...", siginfo_v=siginfo_v@entry=0x7ffee1ce4a30,
    program_counter=program_counter@entry=0x7f833f5f48eb <raise+267>) at lib/log.c:154
10 0x00007f833f92dbd1 in core_handler (signo=6, siginfo=0x7ffee1ce4a30, context=<optimized out>) at lib/sigevent.c:254
11 <signal handler called>
12 0x00007f833f5f48eb in raise () from /lib/x86_64-linux-gnu/libc.so.6
13 0x00007f833f5df535 in abort () from /lib/x86_64-linux-gnu/libc.so.6
14 0x00007f833f958f96 in _zlog_assert_failed (xref=xref@entry=0x7f833f9e4080 <_xref.10705>, extra=extra@entry=0x0) at lib/zlog.c:680
15 0x00007f833f905400 in mt_count_free (mt=0x7f833fa02800 <MTYPE_NH_LABEL>, ptr=0x51) at lib/memory.c:84
16 mt_count_free (ptr=0x51, mt=0x7f833fa02800 <MTYPE_NH_LABEL>) at lib/memory.c:80
17 qfree (mt=0x7f833fa02800 <MTYPE_NH_LABEL>, ptr=0x51) at lib/memory.c:140
18 0x00007f833f90799c in nexthop_del_labels (nexthop=nexthop@entry=0x56091d776640) at lib/nexthop.c:563
19 0x00007f833f907b91 in nexthop_free (nexthop=0x56091d776640) at lib/nexthop.c:393
20 0x00007f833f907be8 in nexthops_free (nexthop=<optimized out>) at lib/nexthop.c:408
21 0x000056091c21aa76 in zebra_nhg_free_members (nhe=0x56091d890840) at zebra/zebra_nhg.c:1628
22 zebra_nhg_free (nhe=0x56091d890840) at zebra/zebra_nhg.c:1628
23 0x000056091c21bab2 in zebra_nhg_proto_add (id=<optimized out>, type=9, instance=<optimized out>, session=0, nhg=nhg@entry=0x56091d7da028, afi=afi@entry=AFI_UNSPEC)
    at zebra/zebra_nhg.c:3532
24 0x000056091c22bc4e in process_subq_nhg (lnode=0x56091d88c540) at zebra/zebra_rib.c:2689
25 process_subq (qindex=META_QUEUE_NHG, subq=0x56091d24cea0) at zebra/zebra_rib.c:3290
26 meta_queue_process (dummy=<optimized out>, data=0x56091d24d4c0) at zebra/zebra_rib.c:3343
27 0x00007f833f9492c8 in work_queue_run (thread=0x7ffee1ce55a0) at lib/workqueue.c:285
28 0x00007f833f93f60d in thread_call (thread=thread@entry=0x7ffee1ce55a0) at lib/thread.c:2008
29 0x00007f833f8f9888 in frr_run (master=0x56091d068660) at lib/libfrr.c:1223
30 0x000056091c1b8366 in main (argc=12, argv=0x7ffee1ce5988) at zebra/main.c:551

Issue: 3492162

Ticket# 3492162

Signed-off-by: Chirag Shah <chirag@nvidia.com>
Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>

Merge pull request #14304 from donaldsharp/ospf_use_after_flumple

ospfd: Prevent use after free( and crash of ospf ) when no router ospf

Merge pull request #14302 from donaldsharp/pim_pim_pim_pim

pimd: When receiving a packet be more careful with length in pim_pim_…

Merge pull request #14301 from donaldsharp/bgp_lost_hash

bgpd: Add peers back to peer hash when peer_xfer_conn fails

Merge pull request #14118 from GaladrielZhao/master

bgpd: Convert from struct bgp_node to struct bgp_dest

ospfd: Prevent use after free( and crash of ospf ) when no router ospf

Consider this config:

router ospf
  redistribute kernel

Then you issue:

no router ospf

ospf will crash with a use after free.

The problem is that the event's associated with the
ospf pointer were shut off then the ospf_external_delete
was called which rescheduled the event.  Let's just move
event deletion to the end of the no router ospf.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Print a hostname also for GR logs under dynamic capability

Just to be consistent with other zlog_ stuff for dynamic capabilities.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd: Make sure we have enough data to read restart time and flags for GR cap

Just a safety check to avoid out of bound reading.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

tests: Check if GR settings can be changed via BGP dynamic capabilities

restart-time and/or notification support.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

doc: GR restart time, notifcation flag can be changed via BGP dynamic cap

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd: Handle Graceful-Restart capability with dynamic capability

Graceful-Restart restart time is exchanged using OPEN messages. In order to
reduce restart time before doing an actual graceful restart, it might be useful
to increase the time, but this is not possible without resetting the session.

With this change, it's possible to send dynamic capability with a new value, and
GR will respect a new reset time value.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

topotest: Command using wrong python version

Changing command from python to python3.

Signed-off-by: Rodrigo Nardi <rnardi@netdef.org>

Merge pull request #14300 from opensourcerouting/fix/set_role_as_undefined_when_capability_unset

bgpd: Unset role when receiving UNSET action for dynamic capability

pimd: When receiving a packet be more careful with length in pim_pim_packet

a) If the length passed is the header length then it is possible that
assignment of data will happen without data actually existing.

b) Just move the assignment to after we ensure that the pim packet
received is the minimum possible length that can be received.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

Merge pull request #14284 from opensourcerouting/fix/bgp_dynamic_capability_zlog

bgpd: Use zlog_err and not zlog_info when we have an error for dynamic capability

bgpd: Add peers back to peer hash when peer_xfer_conn fails

It was noticed that occassionally peering failed in a testbed
upon investigation it was found that the peer was not in the
peer hash and we saw these failure messages:

Aug 25 21:31:15 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: %NOTIFICATION: sent to neighbor 2001:cafe:1ead:4::4 4/0 (Hold Timer Expired) 0 bytes
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 100663299] Can't get remote address and port: Transport endpoint is not connected
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 100663299] %bgp_getsockname() failed for  peer 2001:cafe:1ead:4::4 fd 27 (from_peer fd -1)
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 33554464] %Neighbor failed in xfer_conn

root@doca-hbn-service-bf3-s06-1-ipmi:/var/log/hbn/frr# vtysh -c 'show bgp peerhash' | grep 2001:cafe:1ead:4::4
root@doca-hbn-service-bf3-s06-1-ipmi:/var/log/hbn/frr#

Upon looking at the code the peer_xfer_conn function can fail
and the bgp_establish code will then return before adding the
peer back to the peerhash.

This is only part of the failure.  The peer also appears to
be in a state where it is no longer initiating connection attempts
but that will be another commited fix when we figure that one out.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: Unset role when receiving UNSET action for dynamic capability

Capability was unset, but forgot to unset the role.

Fixes: 5ad080d37a26d72b56ecd0b796593bb7fc3aa6ad ("bgpd: Handle Role capability via dynamic capabilities for SET/UNSET properly")
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #14288 from opensourcerouting/fix/warn_the_user_if_keepalive_was_changed

bgpd: Add a warning for the operator that keepalive was changed

Merge pull request #14290 from opensourcerouting/fix/bgpd_encap_tunnel_handling

bgpd: Use treat-as-withdraw for tunnel encapsulation attribute

bgpd: Use zlog_err and not zlog_info when we have an error for dynamic capability

Also change the outputs a bit to be consistent and more detailed.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

bgpd: Use treat-as-withdraw for tunnel encapsulation attribute

Before this path we used session reset method, which is discouraged by rfc7606.

Handle this as rfc requires.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #14243 from opensourcerouting/fix/bgpd_ebgp_multihop_vty_out

bgpd: Do not explicitly print MAXTTL value for ebgp-multihop vty output

bgpd: Add a warning for the operator that keepalive was changed

```
donatas-pc(config-router)# timers bgp 8 12
% keeplive value 8 is larger than 1/3 of the holdtime, setting to 4
donatas-pc(config-router)# do sh run | include timers bgp
 timers bgp 4 12
donatas-pc(config-router)#
```

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

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #14283 from opensourcerouting/fix/dynamic_capability_role_unset

bgpd: Handle Role capability via dynamic capabilities for SET/UNSET properly

Merge pull request #14274 from opensourcerouting/fix/staticd_with_blackhole_nexthop_handling

staticd: Accept full blackhole typed keywords for ip_route_cmd

Merge pull request #14262 from Keelan10/ospfd6-memleak

ospf6d: Free Newly Created LSA when Non-Self-Originated Grace LSA is …

Merge pull request #14252 from LabNConsulting/ziemba-pbr-bugfix-match-dscp-numeric

pbrd: fix dscp field value computation

Merge pull request #14257 from Keelan10/lib-memleak

lib: Fix memory leaks in LS Update Functions

bgpd: fix redistribute table command after bgp restarts

When the BGP 'redistribute table' command is used for a given route
table, and BGP configuration is flushed and rebuilt, the redistribution
does not work.

Actually, when flushing the BGP configuration with the 'no router bgp'
command, the BGP redistribute entries related to the 'redistribute table'
entries are not flushed. Actually, at BGP deletion, the table number is
not given as parameter in bgp_redistribute_unset() function, and the
redistribution entry is not removed in zebra.
Fix this by adding some code to flush all the redistribute table
instances.

Fixes: 7c8ff89e9346 ("Multi-Instance OSPF Summary")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>

bgpd: Handle Role capability via dynamic capabilities for SET/UNSET properly

It was missed to handle UNSET Role capability using dynamic capabilities.

Also move length check before actually handling Role capability.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #14276 from donaldsharp/bgp_fsm_problemos

Bgp fsm problemos

Merge pull request #14256 from rodecker/rt-table-id

zebra: Make main routing table (RT_TABLE_MAIN) configurable

Merge pull request #14264 from lkClare/master_0823

bgpd: fix bug in a place about label validation

bgpd: Prevent use after free

When bgp_stop finishes and it deletes the peer it is sending
back a return code stating that the peer was deleted, but
the code was operating like it was not deleted and continued
to access the data structure.  Fix.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: bgp_event_update switch to a switch

The return code from a event handling perspective
is an enum.  Let's intentionally make it a switch
so that all cases are ensured to be covered now
and in the future.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

bgpd: bgp_event_update mixes enum's with a non-enum

Straighten out the code to not mix the two.  Especially
since bgp was assigning non enum values to the enum.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

staticd: Accept full blackhole typed keywords for ip_route_cmd

Before this patch we allow entering next-hop interface address as any string.

Like, we can type: `ip route 10.10.10.10/32 bla`, but this will create a blackhole
route instead of using an interface `bla`.

The same is with reject.

After the patch:

```
$ vtysh -c 'con' -c 'ip route 10.10.10.100/32 bla'
ERROR: SET_CONFIG request failed, Error: nexthop interface name must be (reject, blackhole)

$ ip link show dev bla
472: bla: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc fq_codel state DOWN mode DEFAULT group default qlen 1000
    link/ether fa:45:bd:f1:f8:f0 brd ff:ff:ff:ff:ff:ff

$ vtysh -c 'sh run | include ip route'
$ vtysh -c 'con' -c 'ip route 10.10.10.100/32 blac'
$ vtysh -c 'sh run | include ip route'
ip route 10.10.10.100/32 blackhole

$ vtysh -c 'con' -c 'no ip route 10.10.10.100/32 blac'
$ vtysh -c 'sh run | include ip route'
$ vtysh -c 'con' -c 'ip route 10.10.10.100/32 blackhole'
$ vtysh -c 'sh run | include ip route'
ip route 10.10.10.100/32 blackhole

$ vtysh -c 'con' -c 'no ip route 10.10.10.100/32 blackhole'
$ vtysh -c 'sh run | include ip route'
$ vtysh -c 'con' -c 'ip route 10.10.10.100/32 Null0'
$ vtysh -c 'sh run | include ip route'
ip route 10.10.10.100/32 Null0

$ vtysh -c 'con' -c 'no ip route 10.10.10.100/32 Null0'
$ vtysh -c 'sh run | include ip route'
$
```

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #14253 from bigbanglwb/frr_master

configure.ac: fix protobuf config

Merge pull request #14260 from opensourcerouting/fix/do_not_process_nlri_if_attribute_len_is_0

bgpd: Do not process NLRIs if the attribute length is zero

Merge pull request #14232 from opensourcerouting/fix/aigp_validation_bytes

bgpd: Make sure we have enough data to read two bytes when validating AIGP

bgpd: fix bug in a place about label validation

Shouldn't validate the label after 'decode_label'. If we validate
the label after 'decode_label', even the 'MPLS_INVALID_LABEL' will
be valid then.

Signed-off-by: Valerian_He <1826906282@qq.com>

configure.ac: fix protobuf config

  Bug description: frr_init load zebra_fpm.so error. Zebra can't
  find function `zfpm_protobuf_encode_route` in symbol table.

  Bug trigger condition ( CI have this set ):
    ./configure --enable-protobuf=no --enable-fpm=yes
    /usr/lib/frr/zebra -M fpm

  Cause: Macro `HAVE_PROTOBUF` and compile condition variable
  `HAVE_PROTOBUF`  in `configure.ac ` is not consistent. When
  configure `disable-protobuf`, compile condition variable
  `HAVE_PROTOBUF` is 0, but the macro is 1. It leads to zebra
  load protobuf module, but protobuf module is not linked.

  Fix: add a same condition statement to the macro define.

Signed-off-by: 乐倚 <lwb406054@alibaba-inc.com>

Merge pull request #14255 from gromit1811/gromit1811-patch-1

debian: Don't install sample dplane plugin

Merge pull request #14251 from donaldsharp/protodown_rc_issue

zebra: Prevent protodown_rc from going Bzonkas

ospf6d: Free Newly Created LSA when Non-Self-Originated Grace LSA is Discarded

The newly created LSA `new` is now properly freed to prevent memory leaks when
a non-self-originated Grace LSA which is not in LSDB is received.

The ASan leak log for reference:

```
Direct leak of 400 byte(s) in 2 object(s) allocated from:
    #0 0x7f70e984bd28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
    #1 0x7f70e92481c5 in qcalloc lib/memory.c:105
    #2 0x55b35068c975 in ospf6_lsa_alloc ospf6d/ospf6_lsa.c:710
    #3 0x55b35068c9f9 in ospf6_lsa_create ospf6d/ospf6_lsa.c:725
    #4 0x55b35065ab2c in ospf6_receive_lsa ospf6d/ospf6_flood.c:912
    #5 0x55b3506a1413 in ospf6_lsupdate_recv ospf6d/ospf6_message.c:1621
    #6 0x55b3506a1413 in ospf6_read_helper ospf6d/ospf6_message.c:1896
    #7 0x55b3506a1413 in ospf6_receive ospf6d/ospf6_message.c:1925
    #8 0x7f70e92e6ccb in event_call lib/event.c:1979
    #9 0x7f70e922b488 in frr_run lib/libfrr.c:1213
    #10 0x55b35064345e in main ospf6d/ospf6_main.c:250
    #11 0x7f70e8843c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

Indirect leak of 72 byte(s) in 2 object(s) allocated from:
    #0 0x7f70e984bb40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
    #1 0x7f70e9247ee5 in qmalloc lib/memory.c:100
    #2 0x55b35068c987 in ospf6_lsa_alloc ospf6d/ospf6_lsa.c:711
    #3 0x55b35068c9f9 in ospf6_lsa_create ospf6d/ospf6_lsa.c:725
    #4 0x55b35065ab2c in ospf6_receive_lsa ospf6d/ospf6_flood.c:912
    #5 0x55b3506a1413 in ospf6_lsupdate_recv ospf6d/ospf6_message.c:1621
    #6 0x55b3506a1413 in ospf6_read_helper ospf6d/ospf6_message.c:1896
    #7 0x55b3506a1413 in ospf6_receive ospf6d/ospf6_message.c:1925
    #8 0x7f70e92e6ccb in event_call lib/event.c:1979
    #9 0x7f70e922b488 in frr_run lib/libfrr.c:1213
    #10 0x55b35064345e in main ospf6d/ospf6_main.c:250
    #11 0x7f70e8843c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: 472 byte(s) leaked in 4 allocation(s).
```

Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>

bgpd: Do not process NLRIs if the attribute length is zero

```
3  0x00007f423aa42476 in __GI_raise (sig=sig@entry=11) at ../sysdeps/posix/raise.c:26
4  0x00007f423aef9740 in core_handler (signo=11, siginfo=0x7fffc414deb0, context=<optimized out>) at lib/sigevent.c:246
5  <signal handler called>
6  0x0000564dea2fc71e in route_set_aspath_prepend (rule=0x564debd66d50, prefix=0x7fffc414ea30, object=0x7fffc414e400)
    at bgpd/bgp_routemap.c:2258
7  0x00007f423aeec7e0 in route_map_apply_ext (map=<optimized out>, prefix=prefix@entry=0x7fffc414ea30,
    match_object=match_object@entry=0x7fffc414e400, set_object=set_object@entry=0x7fffc414e400, pref=pref@entry=0x0) at lib/routemap.c:2690
8  0x0000564dea2d277e in bgp_input_modifier (peer=peer@entry=0x7f4238f59010, p=p@entry=0x7fffc414ea30, attr=attr@entry=0x7fffc414e770,
    afi=afi@entry=AFI_IP, safi=safi@entry=SAFI_UNICAST, rmap_name=rmap_name@entry=0x0, label=0x0, num_labels=0, dest=0x564debdd5130)
    at bgpd/bgp_route.c:1772
9  0x0000564dea2df762 in bgp_update (peer=peer@entry=0x7f4238f59010, p=p@entry=0x7fffc414ea30, addpath_id=addpath_id@entry=0,
    attr=0x7fffc414eb50, afi=afi@entry=AFI_IP, safi=<optimized out>, safi@entry=SAFI_UNICAST, type=9, sub_type=0, prd=0x0, label=0x0,
    num_labels=0, soft_reconfig=0, evpn=0x0) at bgpd/bgp_route.c:4374
10 0x0000564dea2e2047 in bgp_nlri_parse_ip (peer=0x7f4238f59010, attr=attr@entry=0x7fffc414eb50, packet=0x7fffc414eaf0)
    at bgpd/bgp_route.c:6249
11 0x0000564dea2c5a58 in bgp_nlri_parse (peer=peer@entry=0x7f4238f59010, attr=attr@entry=0x7fffc414eb50,
    packet=packet@entry=0x7fffc414eaf0, mp_withdraw=mp_withdraw@entry=false) at bgpd/bgp_packet.c:339
12 0x0000564dea2c5d66 in bgp_update_receive (peer=peer@entry=0x7f4238f59010, size=size@entry=109) at bgpd/bgp_packet.c:2024
13 0x0000564dea2c901d in bgp_process_packet (thread=<optimized out>) at bgpd/bgp_packet.c:2933
14 0x00007f423af0bf71 in event_call (thread=thread@entry=0x7fffc414ee40) at lib/event.c:1995
15 0x00007f423aebb198 in frr_run (master=0x564deb73c670) at lib/libfrr.c:1213
16 0x0000564dea261b83 in main (argc=<optimized out>, argv=<optimized out>) at bgpd/bgp_main.c:505
```

With the configuration:

```
frr version 9.1-dev-MyOwnFRRVersion
frr defaults traditional
hostname ip-172-31-13-140
log file /tmp/debug.log
log syslog
service integrated-vtysh-config
!
debug bgp keepalives
debug bgp neighbor-events
debug bgp updates in
debug bgp updates out
!
router bgp 100
 bgp router-id 9.9.9.9
 no bgp ebgp-requires-policy
 bgp bestpath aigp
 neighbor 172.31.2.47 remote-as 200
 !
 address-family ipv4 unicast
  neighbor 172.31.2.47 default-originate
  neighbor 172.31.2.47 route-map RM_IN in
 exit-address-family
exit
!
route-map RM_IN permit 10
 set as-path prepend 200
exit
!
```

The issue is that we try to process NLRIs even if the attribute length is 0.

Later bgp_update() will handle route-maps and a crash occurs because all the
attributes are NULL, including aspath, where we dereference.

According to the RFC 4271:

A value of 0 indicates that neither the Network Layer
         Reachability Information field nor the Path Attribute field is
         present in this UPDATE message.

But with a fuzzed UPDATE message this can be faked. I think it's reasonable
to skip processing NLRIs if both update_len and attribute_len are 0.

Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

Merge pull request #14217 from LabNConsulting/ziemba-pbr-action-drop

pbrd: add advisory flag PBR_ACTION_DROP

Merge pull request #14214 from opensourcerouting/fix/handle_rfc7606_attr_len_remaining_data

bgpd: Treat-as-withdraw attribute if remaining data is not enough

Merge pull request #14142 from zays26/fix/zebra-assert

zebra: fix assert in process_subq_route

Merge pull request #14146 from opensourcerouting/ospfv3-auth-interop

OSPFv3 authentication trailer interop

zebra: Make main routing table (RT_TABLE_MAIN) configurable

Signed-off-by: Martin Pels <mpels@ripe.net>