nat: Use iptrie instead of custom LPM implementation #230
Conversation
| dpdk = { workspace = true } | ||
| iptrie = { workspace = true } |
There was a problem hiding this comment.
Same here. Nothing wrong, you'll have to deal with it when merge/rebase.
Raising this because I'd actually merge #211 first for the reason mentioned below.
There was a problem hiding this comment.
If it's the exact same change for this line, I expect git/GitHub is able to silently discard this chunk when rebasing, so it shouldn't create a merge conflict, whichever PR gets in first.
| for key in keys { | ||
| if let Some(val) = &node.value { | ||
| best_match = Some(val.clone()); | ||
| fn insert(&mut self, addr: &IpAddr, len: &u8, value: String) -> Result<(), NatError> { |
There was a problem hiding this comment.
This is absolutely fine. However, in #211, I added an enum Prefix that works for both IPv4 and IPv6. I would use that type here so that:
- the same implementation works also for IPv6 (with minor tweaks like using trie_ipv4 or trie_ipv6)
- we don't leak the Ipv4Prefix, Ipv6Prefix type (specific to the iptrie) outside of the "scope" where they are absolutely needed. The Prefix uses "standard" types.
- we have a common type to represent prefixes throughout.
There was a problem hiding this comment.
Oh, sorry, now I realize that you added also IPv6.
Up to you then to use Prefix.
There was a problem hiding this comment.
I'll take a look, thanks.
Note also that iptrie might support both IPv4 and IPv6 in a single map in the near future.
Replace our PrefixTrie implementation with the one from the iptrie crate. We get some added complexity due to the fact that (as far as I can tell, and in spite of what the crate's README says) there seems to be no way to store a type that represents both IPv4 and IPv6 prefixes; so we end up with two distinct maps. We wrap these into a struct PrefixTrie (distinct from the one we had so far: it no longer implements the map), to avoid dealing with IP versions in the implmementation of the PifTable and the GlobalContext. We also have to implement the Debug trait for the PrefixTrie, given that the LPM map we use does not provide it. PrefixTrie's insert() function will eventually be reworked, when we change the definition of the endpoints to use CIDR objects rather than a combination of address + prefix length. Signed-off-by: Quentin Monnet <[email protected]>
qmonnet
force-pushed
the
pr/qmonnet/nat-iptrie
branch
from
36a29c8 to
b7720bc
Compare
Replace our PrefixTrie implementation with the one from the iptrie crate.
We get some added complexity due to the fact that (as far as I can tell, and in spite of what the crate's README says) there seems to be no way to store a type that represents both IPv4 and IPv6 prefixes; so we end up with two distinct maps. We wrap these into a struct PrefixTrie (distinct from the one we had so far: it no longer implements the map), to avoid dealing with IP versions in the implmementation of the PifTable and the GlobalContext. We also have to implement the Debug trait for the PrefixTrie, given that the LPM map we use does not provide it.
PrefixTrie's insert() function will eventually be reworked, when we change the definition of the endpoints to use CIDR objects rather than a combination of address + prefix length.
Note: iptrie is also introduced in #211.