LPM fixes - #188
Merged
Merged
Conversation
Non-unique LPM entries keep objects after the head in a slice. Updating one of those objects in a write transaction currently changes the object visible through an older read snapshot before the write is committed. Aborting the write does not restore the snapshot. Add a regression test that holds an old read transaction while updating a non-head object under the same LPM key and verifies that the old value remains visible. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
LookupExact returns an lpmEntry by value, but the tail slice still shares its backing array with the entry in the committed trie. lpmEntry.upsert updates that array in place when replacing a tail object and may also reuse it while inserting or reordering objects. This violates StateDB snapshot isolation and can expose uncommitted writes to readers. Clone the tail before every path that mutates it. The updated entry then owns its backing array before it is inserted into the transactional trie. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
Lookup treats reaching the end of the query prefix as a match even when the current compressed node is more specific than the query. It can therefore return a prefix that does not contain the query. If the current node is imaginary, the same early return also discards a valid less specific ancestor. Add regression cases for both outcomes: a query ending inside a real compressed node and a query ending exactly at an imaginary node below a default route. Update the script fixture so a /16 lookup expects its containing /8 instead of a more-specific /32. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
Ending the query inside a compressed node only proves that the query is a prefix of that node; it does not make the more-specific node an LPM candidate. Likewise, an exact imaginary node carries no value and must not hide the closest real ancestor found during traversal. Return the current node only when its prefix length equals the query length and the node is real. Otherwise stop traversal and use the saved closest ancestor. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
An object may contribute multiple keys to a non-unique LPM index. Prefix and lower-bound iteration currently visits each matching trie entry independently, so an object whose broader and narrower keys both match is returned more than once. Add a regression test that indexes one object under an 8-bit and a 16-bit prefix and requires both Prefix and LowerBound to emit it once. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
LPM iterator adaptation previously discarded the primary keys stored in lpmEntry and yielded every object from every matching prefix. It therefore had no way to recognize the same object reached through multiple secondary keys. Mark adapters for non-unique indexes as deduplicating and keep a visited set keyed by primary key while iterating. Unique indexes retain the allocation-free path. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
NetIPPrefixIndex embeds IPv4 keys in the IPv4-mapped portion of the IPv6 key space. This preserves equivalence between native IPv4 and mapped IPv6 forms, but it also places every IPv4 key below broad IPv6 prefixes. An IPv6 default prefix can therefore satisfy an unrelated IPv4 lookup. Add a regression test with a mapped IPv4 subnet and an IPv6 default. Both native and mapped queries for the subnet must find the canonical IPv4 entry, an ordinary IPv6 query must find the IPv6 default, and an unrelated IPv4 query must not fall back to that IPv6 default. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
Encoding both families as 16-byte addresses makes broad IPv6 prefixes match IPv4 queries. Separating every mapped address as IPv6 would avoid that cross-family match, but would also discard the existing and useful aliasing between native IPv4 and IPv4-mapped IPv6 forms. First canonicalize prefixes wholly contained in the IPv4-mapped IPv6 range by unmapping their address and subtracting the 96-bit mapped prefix. Then prefix each canonical key with an address-family discriminator and include those bits in the LPM length. Address queries use the same encoder, so native and mapped IPv4 forms share a key while ordinary IPv6 remains in a separate trie namespace. IPv6 prefixes broader than the mapped range remain IPv6 because they have no equivalent IPv4 prefix. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
NetIPPrefixIndex and LPMIndex implement Indexer and are accepted by NewTable as primary indexes when marked unique. Their primary mutation methods are intentionally unsupported, so the first table insertion panics instead of returning a construction-time error. Add regression coverage requiring NewTable to reject both LPM index implementations when supplied as the primary index. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
LPM indexes only implement secondary-index reindexing; their primary insert, modify, and delete operations panic. Allowing one through NewTable defers a configuration error until the first write and crashes the caller. Mark the LPM index implementations as secondary-only and have NewTable return ErrPrimaryIndexNotSupported when such an index is selected as the primary index. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
PrefixLen is uint16, but rounding a bit length to bytes currently adds seven before widening the value. Lengths from 65529 through 65535 overflow that addition; encoding the maximum length consequently emits only the two-byte length suffix, and decoding accepts that truncated key. Add a regression test for the maximum PrefixLen that verifies the full 8192-byte payload, final-byte masking, and rejection of a truncated key. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
The byte-length calculations in encoding, decoding, and trie validation perform prefixLen + 7 as uint16. The addition wraps near the maximum PrefixLen before the result is converted to int, producing malformed keys and ineffective validation. Centralize the calculation in lpmDataLen and convert prefixLen to int before rounding. Use it consistently in EncodeLPMKey, DecodeLPMKey, and the validation path. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
|
Contributor
Author
|
cc @immanuwell |
derailed
approved these changes
Aug 26, 2026
EncodeLPMKey accepts caller-provided byte slices and prefix lengths, but currently panics when the data is too short. Generic LPM queries and string parsers can receive malformed input and should be able to report it without terminating the process. Return an error from EncodeLPMKey and propagate it through generic query and string conversion paths. Keep panic-on-error behavior in netip helpers and object indexing callbacks, where address sizes or existing interfaces make an encoding failure an internal invariant violation. Propagate the otherwise impossible error while constructing compressed trie nodes through Insert's existing error result. AIL:3 Signed-off-by: Jussi Maki <jussi@isovalent.com>
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
2 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.
See commits for detailed descriptions.