Stateless NAT: Simplify TrieValue, improve error handling in IpList #663
Conversation
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
Functions get_public_trie_value() and get_private_trie_value() are identical but shouldn't be. It makes more sense to have them distinct, and then consistently assign the original and target ranges to the variables with similar names when retrieving the values on the NAT processor's side. Signed-off-by: Quentin Monnet <[email protected]>
Instead of holding the full BTreeSet<Prefix> for the original prefixes for the mapping, just store the prefix in use with its offset in the list. This is more efficient in terms of memory usage, and saves us from recomputing the offset of this prefix at runtime when computing the IP address mapping. Signed-off-by: Quentin Monnet <[email protected]>
We have a few calls to unreachable!() in the IpList logics, where we compute and map the offsets from and to IP addresses within lists of prefixes for stateless NAT. Those unreachable!() have been reached in the past. We also have assumptions that the offsets or IP addresses that we pass to the functions are always contained within the lists or prefixes that we process, which is only true as far as we don't have any bug in the logics. Instead of these, let's introduce proper error handling for the module. When something's wrong, we raise it to the caller. This should avoid crashing the whole dataplane (you never know) and should help debug in case of bugs. The errors propagate to the NAT translation functions, where we stop translating the address when we fail to compute the mapping. We need to improve error handling in there as well, and to drop the packet when we face issues with the logics of the processing during NAT, but this is not covered by this commit. Signed-off-by: Quentin Monnet <[email protected]>
mvachhar
requested changes
Jul 2, 2025
nat/src/stateless/iplist.rs
Outdated
|
|
||
| // We should probably return an error in this case, but at the moment we don't | ||
| let offset = ipls.addr_offset_in_prefix(&addr_v4("1.2.0.0")); | ||
| assert_eq!(offset.offset, 65536); |
There was a problem hiding this comment.
I also suggest a fuzzer where we generate a prefix as a u128 or u32 and mask off the low bits. Then the fuzzer generates a random u32 or u128 and we | in the lower bits with the prefix. This forms our address. We know the offset is the |'ed in lower bits. So then we can call the addr_offset_in_prefix and compare to the known offset. Let me know if you want some help writing this.
| offset: 3 * 65536 + 256 * (128 + 72) + 1, | ||
| ip_version: IpVersion::V4 | ||
| }), | ||
| addr_v4("2.5.72.1") |
There was a problem hiding this comment.
Again, no V6? Also, can we write a fuzzer easily for this as well? I'm less certain of the exact logic for the prefix offset. The addr offset is super clear.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The main motivation of this PR is to reduce the memory footpring and the complexity of the address mapping for stateless NAT, by reducing the amount of information stored in
struct TrieValue.Please refer to individual commit descriptions for details.