Bfd singlehop linklocal defer install - #22743
Draft
sougatahitcs wants to merge 101 commits into
Draft
Conversation
A single-hop BFD session whose destination is an IPv6 link-local address requires an outgoing interface to be usable: the link-local peer/local pair is not globally unique, so the interface (ifindex) is the only key that distinguishes such a session in bfdd. When a box has many single-hop unnumbered neighbours towards the same remote box (e.g. one BGP peer per VLAN sub-interface), every BFD session derives the identical link-local src/dst. If the client (bgpd) installs the session before the egress interface is known - which happens transiently during peer re-add, when the address setter fires while peer->nexthop.ifp is still NULL - the REGISTER/DEREGISTER is sent with no interface. All such interface-less sessions then collapse onto a single ifindex-less key in bfdd and tear each other down, leaving most BFD sessions stuck Down even though BGP is Established. Treat a single-hop IPv6 link-local session with no interface as not-yet-valid in _bfd_sess_valid(), so it is neither registered nor deregistered on the wire. Since _bfd_sess_remove() only deregisters an installed session, this also suppresses the bogus interface-less teardown. The client re-drives the update once the nexthop interface is resolved, and the session then installs with a unique per-interface key. Ticket: #5131052 Signed-off-by: Sougata Barik <sougatab@nvidia.com>
Adds tests/topotests/bgp_bfd_unnumbered_subif_churn, exercising many single-hop unnumbered eBGP+BFD peers over VLAN sub-interfaces that all share the same IPv6 link-local address (same parent MAC), across repeated BGP neighbor delete/re-add churn. - test_..._baseline: all peers Established and all BFD sessions Up on first bring-up. - test_..._churn_recovery: after each delete/re-add cycle every BFD session must return to Up. This FAILS without the _bfd_sess_valid() interface-less link-local guard (sessions collapse onto one ifindex-less key and stay Down) and PASSES with the fix. Ticket: #5131052 Signed-off-by: Sougata Barik <sougatab@nvidia.com>
Greptile SummaryThis PR fixes a long-standing BFD session collapse on boxes with many single-hop unnumbered IPv6 eBGP peers sharing the same link-local address (e.g. multiple VLAN sub-interfaces to the same remote). The root cause was that
Confidence Score: 3/5The core lib/bfd.c deferral logic is correct and well-guarded, but the Debian build configuration adds a hard dependency on LTTng without declaring the package in Build-Depends, which will break clean Debian package builds. The fix itself is sound — deferring link-local sessions without an interface prevents the key collapse in bfdd and the re-drive path is correct. However, debian/rules now passes --enable-lttng unconditionally while debian/control omits liblttng-ust-dev from Build-Depends; configure.ac calls AC_MSG_ERROR on a missing lttng-ust when the flag is explicitly set, so a clean Debian build chroot will abort at configure time. debian/rules and debian/control need to be updated together — the new --enable-lttng flag requires liblttng-ust-dev in Build-Depends. Important Files Changed
Sequence DiagramsequenceDiagram
participant BGP as bgpd (bgp_bfd.c)
participant BFD as lib/bfd.c
participant BFDD as bfdd (ptm_adapter.c)
Note over BGP: Peer created, nexthop.ifp == NULL
BGP->>BFD: bgp_peer_bfd_update_source()
BFD->>BFD: "_bfd_sess_valid(): link-local + ifnamelen==0"
BFD-->>BGP: return false (deferred, no REGISTER sent)
Note over BFDD: No interface-less REGISTER reaches bfdd
Note over BGP: Nexthop resolved, nexthop.ifp != NULL
BGP->>BFD: bgp_peer_bfd_update_source() [re-drive]
BFD->>BFD: bfd_sess_set_interface(ifname)
BFD->>BFD: "_bfd_sess_valid(): ifnamelen > 0 true"
BFD->>BFDD: ZEBRA_BFD_DEST_REGISTER (with ifindex)
BFDD-->>BFD: ZEBRA_INTERFACE_BFD_DEST_UPDATE (Up)
BFD-->>BGP: bfd status callback Up
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+92
to
93
| --enable-lttng \ | ||
| # end |
There was a problem hiding this comment.
--enable-lttng is now passed unconditionally to configure, but configure.ac calls AC_MSG_ERROR when --enable-lttng is specified and lttng-ust is not found (line 2154). Because liblttng-ust-dev is not present in debian/control's Build-Depends, a clean Debian package build (e.g. a pbuilder/sbuild chroot) will abort at the configure step with a hard error.
Prompt To Fix With AI
This is a comment left during a code review.
Path: debian/rules
Line: 92-93
Comment:
`--enable-lttng` is now passed unconditionally to `configure`, but `configure.ac` calls `AC_MSG_ERROR` when `--enable-lttng` is specified and `lttng-ust` is not found (line 2154). Because `liblttng-ust-dev` is not present in `debian/control`'s `Build-Depends`, a clean Debian package build (e.g. a pbuilder/sbuild chroot) will abort at the configure step with a hard error.
How can I resolve this? If you propose a fix, please make it concise.
sougatahitcs
force-pushed
the
bfd-singlehop-linklocal-defer-install
branch
from
July 24, 2026 10:34
d80e1fc to
872cce6
Compare
…figured Currently the addition of a static group to an interface depends on the ip address being properly configured. If the ip address is not there the corresponding vif will not be properly set up and causes a crash: 9 0x00007dcd2f44527e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26 10 0x00007dcd2f4288ff in __GI_abort () at ./stdlib/abort.c:79 11 0x00007dcd2fba07c0 in _zlog_assert_failed (xref=0x6369275b10e0 <_xref.17>, extra=0x6369275570f8 "trying to add OIF %s with VIF (%d)") at lib/zlog.c:801 12 0x00006369274ccd9e in pim_channel_add_oif (channel_oil=0x636940f4ff20, oif=0x636940f4fa40, proto_mask=1, caller=0x63692755d4c8 <__func__.12> "tib_sg_gm_join") at pimd/pim_oil.c:441 13 0x00006369274df7d1 in tib_sg_gm_join (pim=0x636940df20c0, sg=..., oif=0x636940f4fa40, oilp=0x636940f4f6c8) at pimd/pim_tib.c:373 14 0x00006369274a9e58 in static_group_new (ifp=0x636940f4fa40, group_addr=..., source_addr=...) at pimd/pim_iface.c:1516 15 0x00006369274aa4b1 in pim_if_static_group_add (ifp=0x636940f4fa40, group_addr=..., source_addr=...) at pimd/pim_iface.c:1670 16 0x00006369274c3668 in lib_interface_gmp_address_family_static_group_create (args=0x7ffc7ab30870) at pimd/pim_nb_config.c:5477 17 0x00007dcd2fb16c40 in nb_callback_create (context=0x636940f50c30, nb_node=0x636940ddea10, event=NB_EV_APPLY, dnode=0x636940f6c100, resource=0x636940f6a768, errmsg=0x7ffc7ab30eb0 "", errmsg_len=8192) at lib/northbound.c:1583 18 0x00007dcd2fb17d77 in nb_callback_configuration (context=0x636940f50c30, event=NB_EV_APPLY, change=0x636940f6a730, errmsg=0x7ffc7ab30eb0 "", errmsg_len=8192) at lib/northbound.c:2000 19 0x00007dcd2fb1838a in nb_transaction_process (event=NB_EV_APPLY, transaction=0x636940f50c30, errmsg=0x7ffc7ab30eb0 "", errmsg_len=8192) at lib/northbound.c:2133 20 0x00007dcd2fb16627 in nb_candidate_commit_apply (transaction=0x636940f50c30, save_transaction=true, transaction_id=0x0, errmsg=0x7ffc7ab30eb0 "", errmsg_len=8192) at lib/northbound.c:1445 21 0x00007dcd2fb167d6 in nb_candidate_commit (context=..., candidate=0x636940d48160, save_transaction=true, comment=0x0, transaction_id=0x0, errmsg=0x7ffc7ab30eb0 "", errmsg_len=8192) at lib/northbound.c:1485 22 0x00007dcd2fb1dbb0 in nb_cli_classic_commit (vty=0x636940f427f0) at lib/northbound_cli.c:59 23 0x00007dcd2fb1e20c in nb_cli_apply_changes_internal (vty=0x636940f427f0, xpath_base=0x7ffc7ab34f40 "/frr-interface:lib/interface[name='fhr-eth4']/frr-gmp:gmp/address-family[address-family='frr-routing:ipv4']/static-group[group-addr='225.1.3.4'][source-addr='0.0.0.0']", clear_pending=false) at lib/northbound_cli.c:197 24 0x00007dcd2fb1e3b0 in _nb_cli_apply_changes (vty=0x636940f427f0, xpath_base=0x7ffc7ab35390 "./frr-gmp:gmp/address-family[address-family='frr-routing:ipv4']/static-group[group-addr='225.1.3.4'][source-addr='0.0.0.0']", clear_pending=false) at lib/northbound_cli.c:230 25 0x00007dcd2fb1e4c3 in nb_cli_apply_changes (vty=0x636940f427f0, xpath_base_fmt=0x6369275703d8 "./frr-gmp:gmp/address-family[address-family='%s']/static-group[group-addr='%s'][source-addr='%s']") at lib/northbound_cli.c:246 26 0x0000636927521a94 in interface_ip_igmp_static_group_magic (self=0x6369275be5c0 <interface_ip_igmp_static_group_cmd>, vty=0x636940f427f0, argc=4, argv=0x636940f51740, no=0x0, grp=..., grp_str=0x636940f61ac0 "225.1.3.4", src=..., src_str=0x0) at pimd/pim_cmd.c:5419 27 0x0000636927509d4a in interface_ip_igmp_static_group (self=0x6369275be5c0 <interface_ip_igmp_static_group_cmd>, vty=0x636940f427f0, argc=4, argv=0x636940f51740) at ./pimd/pim_cmd_clippy.c:6527 28 0x00007dcd2fa96d80 in cmd_execute_command_real (vline=0x636940f616b0, vty=0x636940f427f0, cmd=0x0, up_level=0) at lib/command.c:1014 29 0x00007dcd2fa96ef9 in cmd_execute_command (vline=0x636940f616b0, vty=0x636940f427f0, cmd=0x0, vtysh=0) at lib/command.c:1073 30 0x00007dcd2fa974a6 in cmd_execute (vty=0x636940f427f0, cmd=0x636940f68120 ' ' <repeats 14 times>, "ip igmp static-group 225.1.3.4", matched=0x0, vtysh=0) at lib/command.c:1239 31 0x00007dcd2fb7b941 in vty_command (vty=0x636940f427f0, buf=0x636940f68120 ' ' <repeats 14 times>, "ip igmp static-group 225.1.3.4") at lib/vty.c:593 Config is this: interface fhr-eth4 ip address 10.20.2.253/16 ip pim ip pim passive ip igmp ip igmp static-group 225.1.3.4 Under heavy load the ip address has not been fully passed through zebra->kernel->zebra->pimd and the interface is not properly setup. Modify the code to only allow setup if the interface is fully ready. If not do not do so. When the interface comes cause the static groups to be handled properly at that point. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
CI is seeing this upon freeing an upstream variable:
ERROR: AddressSanitizer: heap-use-after-free on address 0x61300009617c at pc 0x557b9d225cac bp 0x7ffde36f7a90 sp 0x7ffde36f7a88
READ of size 4 at 0x61300009617c thread T0
0 0x557b9d225cab in pim_jp_groups_source_set_prune pimd/pim_join.c:573
1 0x557b9d225cab in pim_jp_send pimd/pim_join.c:975
2 0x557b9d2296b3 in pim_joinprune_send pimd/pim_join.c:1085
3 0x557b9d29655c in pim_zebra_update_all_interfaces pimd/pim_zebra.c:233
4 0x557b9d24ebb5 in pim_update_upstream_nh pimd/pim_nht.c:1033
5 0x557b9d25adb6 in pim_nexthop_update pimd/pim_nht.c:1957
6 0x7f895432b84e in zclient_nexthop_update lib/zclient.c:4758
7 0x7f89543251b0 in zclient_read lib/zclient.c:4888
8 0x7f89542f4893 in event_call lib/event.c:2744
9 0x7f8954210544 in frr_run lib/libfrr.c:1258
10 0x557b9d3111a0 in main pimd/pim_main.c:169
11 0x7f8953c1b249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
12 0x7f8953c1b304 in __libc_start_main_impl ../csu/libc-start.c:360
13 0x557b9d1de7f0 in _start (/usr/lib/frr/pimd+0x1487f0)
0x61300009617c is located 60 bytes inside of 352-byte region [0x613000096140,0x6130000962a0)
freed by thread T0 here:
0 0x7f89546b76a8 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
1 0x7f8954235bcb in qfree lib/memory.c:136
2 0x557b9d285ff3 in pim_upstream_del pimd/pim_upstream.c:287
3 0x557b9d285ca9 in pim_upstream_remove_children pimd/pim_upstream.c:71
4 0x557b9d285ca9 in pim_upstream_del pimd/pim_upstream.c:256
5 0x557b9d21d281 in pim_ifchannel_delete pimd/pim_ifchannel.c:190
6 0x557b9d208592 in clear_mroute pimd/pim_cmd_common.c:4481
7 0x557b9d2b630f in clear_ip_mroute_magic pimd/pim_cmd.c:1537
8 0x557b9d2b630f in clear_ip_mroute pimd/pim_cmd_clippy.c:86
9 0x7f89541a4d8e in cmd_execute_command_real lib/command.c:1014
10 0x7f89541a50ad in cmd_execute_command lib/command.c:1073
11 0x7f89541a55d9 in cmd_execute lib/command.c:1239
12 0x7f8954301b2c in vty_command lib/vty.c:593
13 0x7f8954302079 in vty_execute lib/vty.c:1356
14 0x7f89543090a7 in vtysh_read lib/vty.c:2302
15 0x7f89542f4893 in event_call lib/event.c:2744
16 0x7f8954210544 in frr_run lib/libfrr.c:1258
17 0x557b9d3111a0 in main pimd/pim_main.c:169
18 0x7f8953c1b249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
previously allocated by thread T0 here:
0 0x7f89546b83b7 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
1 0x7f89542352d8 in qcalloc lib/memory.c:111
2 0x557b9d28d1d0 in pim_upstream_new pimd/pim_upstream.c:1198
3 0x557b9d28d1d0 in pim_upstream_add pimd/pim_upstream.c:1417
4 0x557b9d22f9da in pim_mroute_msg_wholepkt pimd/pim_mroute.c:632
5 0x557b9d239243 in pim_mroute_msg pimd/pim_mroute.c:1448
6 0x557b9d23956c in mroute_read pimd/pim_mroute.c:1490
7 0x7f89542f4893 in event_call lib/event.c:2744
8 0x7f8954210544 in frr_run lib/libfrr.c:1258
9 0x557b9d3111a0 in main pimd/pim_main.c:169
10 0x7f8953c1b249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
Effectively when the pim_upstream is deleted it is leaving the upstream in the jp group send
hung off the interface. Add a bit of code on final deletion that prevents this from happening.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Introduces UPA (Unreachable Prefix Announcement) infrastructure per draft-ietf-idr-upa-02 Section 5.1: - EC_BGP_UPA error code - ECOMMUNITY_OPAQUE_SUBTYPE_UPA (0x09) for UPA Extended Community encoding - BGP_PATH_UPA, BGP_PATH_UPA_DROP path flags - PEER_FLAG_UPA_SEND peer capability flag - UPA fields in struct bgp and struct bgp_aggregate - Hash tables and function declarations for UPA tracking Signed-off-by: Patrice Brissette <pbrisset@cisco.com>
Adds functions to create, parse, display, and filter UPA Extended Communities: - bgp_upa_extcom_new() - create UPA ExtCom with flags and router-id - bgp_upa_extcom_parse() - parse received UPA ExtCom from wire format - bgp_upa_extcom_filter() - filter UPA ExtComs from communities - bgp_ecommunity_has_upa() - check if ExtCom has UPA - Display 'upa:' prefix in community string output Signed-off-by: Patrice Brissette <pbrisset@cisco.com>
Core route processing for UPA: - Set BGP_PATH_UPA flag when receiving routes with UPA Extended Community - UPA routes have lower priority than non-UPA routes in best-path selection - Filter UPA announcements to peers without PEER_FLAG_UPA_SEND capability - Strip UPA Extended Community from announcements to non-UPA peers User-facing CLI commands: - neighbor PEER upa - enable UPA announcements to peer - upa originate-all [ipv4|ipv6] [unicast|multicast] - enable global UPA - upa drop [ipv4|ipv6] [unicast|multicast] - set D-bit for global UPA - upa max-routes (1-4294967295) [ipv4|ipv6] [unicast|multicast] - limit global UPA - debug bgp upa - enable/disable UPA debug logging Signed-off-by: Patrice Brissette <pbrisset@cisco.com>
Announces UPA routes to FIB and handles update group optimization: - bgp_zebra_announce_eligible() determines nexthop type (unicast vs blackhole) - UPA debug logging in announcement functions - PEER_FLAG_UPA_SEND in PEER_UPDGRP_AF_FLAGS macro - UPA-only UPDATE batching (packet_is_upa variable) - Cleanup UPA hash when freeing BGP instance Signed-off-by: Patrice Brissette <pbrisset@cisco.com>
Documentation for UPA feature: - aggregate-address A.B.C.D/M upa [drop] [max-routes] syntax - neighbor PEER upa command documentation - Full 'Unreachable Prefix Announcement (UPA)' section with: - Feature overview and use cases - Configuration examples - Behavior description - Show command reference Signed-off-by: Patrice Brissette <pbrisset@cisco.com>
Comprehensive test coverage for UPA feature: - bgp_upa_extcom: Extended community encoding/decoding/display - bgp_upa_anycast: Anycast failover detection scenarios - bgp_upa_ipv6: IPv6 support and multicast scenarios - bgp_upa_multipeer: Multiple peer and aggregate configurations - Tests for max-routes limits, state transitions, show commands Signed-off-by: Patrice Brissette <pbrisset@cisco.com>
bmp monitor and bmp import-vrf-view mark the affected afi/safi for a new table sync on every live BMP session, but never woke the sessions' write loops. On a quiet session the sync only started once unrelated traffic bumped the pull-writer, and until then live route updates for the afi/safi were discarded against the pending sync. Bump the pull-writer right after requesting the sync. Signed-off-by: Anton Berezin <tobez@tobez.org>
BMP pre-policy route monitoring reads from the Adj-RIB-In, but bgpd only maintained it for peers with soft-reconfiguration inbound configured. Peers without it were silently absent from the pre-policy feed (FRRouting/frr issue FRRouting#10240). Make the Adj-RIB-In maintenance decision a proper predicate, bgp_adj_in_needed(): true when PEER_FLAG_SOFT_RECONFIG is set or when any registered consumer claims the peer through the new bgp_adj_in_needed hook. The BMP module registers a hook handler that claims every peer covered by pre-policy monitoring, either in the monitored bgp instance itself or through bmp import-vrf-view. bgpd core keeps no knowledge of BMP internals. With the Adj-RIB-In now populated for monitored peers, the pre-policy branch of bmp_wrqueue keys off the same predicate instead of the soft-reconfiguration flag, so it no longer skips peers whose adj-in is BMP-driven. show bgp neighbors PEER received-routes intentionally stays gated on the user-visible soft-reconfiguration flag. Signed-off-by: Anton Berezin <tobez@tobez.org>
Enabling pre-policy monitoring at runtime only covered routes received from that point on; established peers' existing routes stayed missing from the Adj-RIB-In. Repopulate it the same way enabling soft-reconfiguration inbound does: a route refresh towards each newly covered established peer (session reset for peers without the refresh capability). This applies to the bmp monitor command, to bmp import-vrf-view picking up an existing instance, and honours peers and instances already covered by another consumer. On the disable side (bmp monitor removal, import-vrf-view removal, bmp targets deletion, and also soft-reconfiguration unset), free the Adj-RIB-In memory unless bgp_adj_in_needed() says another consumer still needs it. With the Adj-RIB-In now guaranteed for every peer that pre-policy monitoring covers, the missing-adj-in warnings can no longer fire truthfully, and the configuration-time warning text (asking for soft-reconfiguration inbound) would be plain wrong; remove them. Drop the topotest subtest that asserted the now-removed peer-up warning. Signed-off-by: Anton Berezin <tobez@tobez.org>
bgpd now maintains the Adj-RIB-In for every peer covered by pre-policy monitoring; update the note accordingly. Signed-off-by: Anton Berezin <tobez@tobez.org>
Rework the no-soft-reconfiguration topotest from asserting the old limitation to asserting the full pre-policy feed: announcements appear as pre-policy updates with no fabricated withdrawals and no missing adj-in warning, a forced table sync dumps the routes before the pre-policy End-of-RIB, genuine withdrawals and a peer bounce are reflected, disabling pre-policy monitoring at runtime frees the Adj-RIB-In memory (observed through the MTYPE allocation counter) and re-enabling repopulates it via route refresh, while received-routes still requires the user-visible soft-reconfiguration flag. Signed-off-by: Anton Berezin <tobez@tobez.org>
The calling of setsockopt_ipv6_hoplimit was not being checked for failure and reported. Let's do so. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
On line 2231 of bgp_stop, the bgp variable has already been derefed in every path up to this point. No need for a null check. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
In bgp_ls_originate_bgp_link_internal the connection variable has already been derefed in all paths to the test for it being valid. In addition if the peer does not have a connection value something so terrible has gone wrong, there is no recovering from this. We will crash. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This function already has deref'ed bgp, no need to check to see if it is NULL. Additionally if you call this function with a NULL bgp pointer you get what you ask for. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The srv6_l3service variable can be null as that the accessor function can return a NULL. Although if this were to happen at this point, we are really really screwed. Let coverity know that this is not a problem. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Remove the redundant NULL check on this variable. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
No need to do this, it's impossible for bgp to be null, remove. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When a route-map or policy filter changes, updgrp_policy_update_walkcb() call subgroup_announce_route() which re-evaluates every route from scratch. If the computed attributes are unchanged, the normal adj-out duplicate suppression in subgroup_announce_table() already detects the match and skips the send. Setting SUBGRP_STATUS_FORCE_UPDATES bypasses that check and causes a spurious re-send of every route whose attrs happened not to change. The flag is still set for the two cases that legitimately require re-sending even when attrs are identical: - "clear bgp * soft out" (explicit operator request) - ROUTE-REFRESH (RFC 2918: peer requested the full table) Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
Add a topotest to verify that:
TC1: "clear bgp * soft out" forces UPDATE re-send even if attributes
are unchanged. This is an explicit operator request to re-advertise
all routes.
Verify: receivedPrefixDup increments on r2.
TC2: Change inbound prefix-list on r1, then "clear bgp * soft in" triggers
ROUTE-REFRESH (RFC 2918) to r2. Upon receiving ROUTE-REFRESH, r2
must re-advertise the full table.
Verify: receivedPrefixDup increments on r1 (for the existing prefix),
and the new prefix (10.0.0.22/32) is now accepted.
TC3: Changing outbound prefix-list should NOT cause duplicate UPDATE
re-send for routes whose attributes are unchanged. Only the newly
permitted prefix should be sent.
Verify: receivedPrefixDup does NOT increment on r2, and the new
prefix (10.0.0.11/32) is received.
Topology: r1 (AS 65001) -- r2 (AS 65002)
- r1 sources 10.0.0.1/32 and 10.0.0.11/32
- r2 sources 10.0.0.2/32 and 10.0.0.22/32
- r1 uses outbound prefix-list to advertise only 10.0.0.1/32 initially
- r1 uses inbound prefix-list to accept only 10.0.0.2/32 initially
Signed-off-by: Enke Chen <enchen@paloaltonetworks.com>
This returns -1 regardless of which nexthop has more segments. Sorted insertion produces non-deterministic order when the two nexthops have different "num_segs". Signed-off-by: anlan_cs <vic.lan@pica8.com>
Add some const to a couple of ospf neighbor apis. Signed-off-by: Mark Stapp <mjs@cisco.com>
The router-LSA can contain a series of link objects; those objects' sizes can vary, based on the presence or absence of TOS information. When iterating through the links in an LSA, use the correct size in computing the number of octets to advance. Reported-by: leidongzheng <2691605373@qq.com> Signed-off-by: Mark Stapp <mjs@cisco.com>
Currently, FRRouting does not support associating an MPLS label/ SRv6 segment to SR policies with open-source SDN controllers. It does support installation of Cisco binding labels, but the open-source SDN controllers do not use this properitary TLV. This patch adds support for RFC9604, which defines a TLV allowing a PCEP controller to indicate the MPLS label/SRv6 segment asociated with SR policies. The patch implements 2 of the 4 binding types: MPLS labels and SRv6 segments. MPLS Stack Entry and SRv6 Behavior are left as future work, but this patch should be a good starting point for adding bindings. This commit adds the TE-PATH-BINDING TLV structure to pceplib, and functions to encode/decode this new TLV. Signed-off-by: Anthony Doeraene <anthony.doeraene@hotmail.com>
Currently, FRRouting does not support associating an MPLS label/ SRv6 segment to SR policies with open-source SDN controllers. It does support installation of Cisco binding labels, but the open-source SDN controllers do not use this properitary TLV. This patch adds support for RFC9604, which defines a TLV allowing a PCEP controller to indicate the MPLS label/SRv6 segment asociated with SR policies. The patch implements 2 of the 4 binding types: MPLS labels and SRv6 segments. MPLS Stack Entry and SRv6 Behavior are left as future work, but this patch should be a good starting point for adding bindings. This commit adds tests for the encoding/decoding of the new TE-PATH-BINDING TLV. Signed-off-by: Anthony Doeraene <anthony.doeraene@hotmail.com>
Currently, FRRouting does not support associating an MPLS label/ SRv6 segment to SR policies with open-source SDN controllers. It does support installation of Cisco binding labels, but the open-source SDN controllers do not use this properitary TLV. This patch adds support for RFC9604, which defines a TLV allowing a PCEP controller to indicate the MPLS label/SRv6 segment asociated with SR policies. The patch implements 2 of the 4 binding types: MPLS labels and SRv6 segments. MPLS Stack Entry and SRv6 Behavior are left as future work, but this patch should be a good starting point for adding bindings. This commit adds the parsing of the TE-PATH-BINDING when creating a SR policy, and associate MPLS segment to the new policy. Signed-off-by: Anthony Doeraene <anthony.doeraene@hotmail.com>
When "no router bgp" deletes the default instance while its MPLS-VPN RIB
is non-empty, bgp_delete() keeps the instance alive as a hidden tombstone
(BGP_FLAG_INSTANCE_HIDDEN) and clears bm->bgp_evpn via bgp_set_evpn(NULL).
Reviving the instance with "router bgp <AS>" reuses the same struct bgp
through the hidden path of bgp_create(), which does "goto peer_init" and
skips the fresh-instance block that records the default as the EVPN
instance. bm->bgp_evpn therefore stays NULL and EVPN stops working on the
revived default: bgp_get_evpn() returns NULL, so "show bgp l2vpn evpn"
collapses to "{}" and EVPN route processing on the default is broken.
A plain delete/recreate (empty MPLS-VPN RIB, no tombstone) reclaims the
EVPN role because the non-hidden bgp_create() path assigns it; only the
hidden-revival path is inconsistent. advertise-all-vni would re-assign
bm->bgp_evpn and mask the problem, but setups using "advertise ipv4
unicast" (EVPN type-5) or importing EVPN routes without advertise-all-vni
remain broken after revival.
Re-record the revived default as the EVPN instance on the hidden path,
mirroring the assignment already done on the fresh-instance path.
Add bgp_hidden_evpn_revival which deletes a default instance with a
non-empty MPLS-VPN RIB, revives it with a new AS, and checks the revived
default is still the EVPN instance. The test fails without this fix.
Signed-off-by: Nageswara Soma <nsoma@cisco.com>
peer_errs_mtx is initialized in bgp_create() and destroyed only in bgp_free(). bgp_free() runs when the instance refcount reaches zero via the initial-reference bgp_unlock() in bgp_delete(). For a hidden tombstone (BGP_FLAG_INSTANCE_HIDDEN, not terminating) bgp_delete() takes the else branch and keeps that initial reference, so bgp_free() is never called and the mutex is left initialized while the instance is dormant. Reviving the instance re-enters bgp_create() on the same struct bgp and, without this guard, calls pthread_mutex_init() a second time on the still initialized mutex. Re-initializing a live mutex is undefined behavior per POSIX; on implementations where pthread_mutex_t is a heap-allocated handle (e.g. FreeBSD libthr) it also leaks the previously allocated mutex on every revival. Only initialize peer_errs_mtx on the fresh-instance path; the hidden revival path reuses the mutex that was never destroyed. Signed-off-by: Nageswara Soma <nsoma@cisco.com>
Adds observability for the BFD client library (lib/bfd.c) session lifecycle.
No functional change; every tracepoint degrades to (void)0 when HAVE_LTTNG is
not compiled in.
frr_libfrr provider (lib/libfrr_trace.h, lib/bfd.c)
- bfd_sess_send : one event per _bfd_sess_send() call
(ifname, vrf, family, dst, src,
lastev, installed-in, ZAPI command,
zclient rv, installed-out). Also fires
on the "not installed + DEREGISTER"
early-out (command=0) to expose silent
drops.
- bfd_sess_remove : one event per _bfd_sess_remove(),
with installed and whether a pending
installev event is cancelled.
- bfd_sess_defer_linklocal_without_intf : fired when _bfd_sess_valid() defers a
single-hop IPv6 link-local session
whose outgoing interface is not yet
known.
tools/frr_babeltrace.py : parsers/formatters for the frr_libfrr:bfd_sess_*
events.
debian/control : add liblttng-ust-dev to Build-Depends (required by
--enable-lttng; a clean pbuilder/sbuild build would
otherwise abort at configure).
debian/rules : add --enable-lttng so the frr_libfrr tracepoints are
built.
Ticket: #5131052
Signed-off-by: Sougata Barik <sougatab@nvidia.com>
Adds observability for the BGP BFD source-update path. No functional change;
every tracepoint degrades to (void)0 when the bgp LTTng provider is not compiled
in.
frr_bgp provider (bgpd/bgp_trace.h, bgpd/bgp_bfd.c, bgpd/bgp_network.c)
- bfd_update_source_enter : entry of bgp_peer_bfd_update_source(); shows which
peers hit that path with a still-NULL su_local.
- bgp_getsockname_result : bgp_getsockname() failure (bgp_zebra_nexthop_set()
failed) that leaves peer->su_local unset and drives
the race. Emitted only on the failure path to avoid
tracing every successful session establishment.
bgp_bfd.c also gains the bgpd/bgp_trace.h include required by frrtrace().
tools/frr_babeltrace.py : parsers for the frr_bgp:bfd_update_source_enter and
frr_bgp:bgp_getsockname_result events.
Ticket: #5131052
Signed-off-by: Sougata Barik <sougatab@nvidia.com>
sougatahitcs
force-pushed
the
bfd-singlehop-linklocal-defer-install
branch
from
July 24, 2026 10:45
872cce6 to
cadb677
Compare
Contributor
Author
Member
|
Should this be broken up into multiple pr's? |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
lib: defer BFD install for interface-less single-hop link-local sessions
Single-hop IPv6 link-local sessions require an outgoing interface.
A link-local (fe80::/10) peer/local pair is not globally unique: on a
box with many single-hop unnumbered neighbours (e.g. one BGP peer per
VLAN sub-interface towards the same remote box) every session shares
the identical link-local src/dst, so the interface (ifindex) is the
only key that distinguishes them in bfdd. Sending a REGISTER (or
DEREGISTER) before the egress interface is known collapses all of
those sessions onto a single ifindex-less key, and siblings then tear
each other down - leaving most BFD sessions stuck Down even though BGP
is Established. Treat such a session as not-yet-valid and defer the
install; the client re-drives us (bgp_peer_bfd_update_source) once the
nexthop interface is resolved.
Signed-off-by: Sougata Barik sougatab@nvidia.com