Trigger regression tests - #1
Closed
riccardo-negri wants to merge 5 commits into
Closed
Conversation
…ern() This commit will perform the following changes: 1. Re-enable the prerequisite check in `bgp_attr_munge_as4path()` to return `ERR` when `AS4_PATH` is present but `AS_PATH` is absent. 2. Add a null guard in `aspath_unintern()` to prevent a potential null-dereference when calling the function with null aspath. There is no behaviour change for well-formed BGP sessions. (cherry picked from commit 641f23f)
- bgp_parse_update_msg: unintern partially-parsed attr components (aspath, community, ecommunity, lcommunity) when bgp_attr_parse() returns an error, preventing leaks on malformed UPDATE messages - bgp_attr_parse: use goto cleanup label so as4_path is always uninterned on all early-return error paths inside the parse loop - bgp_ls_nlri_parse: initialize tlv_type to 0 to prevent undefined behaviour when the inner TLV loop does not execute, which could cause phantom RIB insertions - bgp_ls_nlri_parse: free attr_hdr and attr_aux when cdada_map_insert_replace fails to avoid leaking both allocations (cherry picked from commit 1a192e4)
cdada_map_insert() silently ignores updates to existing keys (returns CDADA_E_EXISTS) leaving the old mapping in place permanently. When a router reassigns its internal VRF IDs, both the old and new mappings coexist in vrf_name_map indefinitely. Consider the following before/after state on the router: Before: ID_1 -> NAME_A, ID_X -> NAME_B After: ID_1 -> NAME_B, ID_2 -> NAME_A The router sends both new option records. ID_2 -> NAME_A is a fresh key and inserts correctly. ID_1 -> NAME_B hits an existing key and the update is silently dropped. nfacctd then permanently holds: ID_1 -> NAME_A (stale — should be NAME_B) ID_2 -> NAME_A (correct) All flows arriving with ID_1 are enriched with NAME_A instead of NAME_B, causing flows from one customer to appear under another customer's VRF context in the output. The daemon retains this wrong state until restarted, which explains why a restart temporarily resolves the issue. Replace cdada_map_insert() with cdada_map_insert_replace(), which atomically updates existing keys and returns the old pointer for freeing, handling both insert and update in a single call. (cherry picked from commit 7b8ef26)
…entries on ID reassignment Same issue as 7b8ef26: cdada_map_insert() silently ignores updates to existing keys (returns CDADA_E_EXISTS), leaving stale mappings in place permanently. Replace cdada_map_insert() with cdada_map_insert_replace() for in_rd_map, out_rd_map and iface_name_map, freeing any displaced entry. Each RD map now owns an independent heap-allocated copy of the RD value so replacements on one map cannot dangle the other. The intermediate mpls_vpn_rd heap allocation is replaced with a stack variable since it is only needed as a temporary parse buffer. (cherry picked from commit 83fa10d)
Test that nfacctd correctly updates vrf_name_map, in_rd_map, out_rd_map, and iface_name_map when a Cisco router reassigns its internal IDs via new IPFIX Options Data records. Covers the stale-entry bugs fixed in 7b8ef26 and 83fa10d, where cdada_map_insert() silently dropped updates to existing keys, causing flows to be permanently enriched with the wrong VRF name, MPLS VPN RD, and interface name after an ID reassignment. (cherry picked from commit bf9da1d)
rodonile
marked this pull request as ready for review
August 3, 2026 15:33
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
1 participant
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.
No description provided.