libnetwork/d/overlay: use netip types more #50090
Merged
+226
−248
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
Make the SetMatrix key's type generic so that e.g. netip.Addr values can be used as matrix keys. Signed-off-by: Cory Snider <csnider@mirantis.com>
The netip types are really useful for tracking state in the overlay driver as they are hashable, unlike net.IP and friends, making them directly useable as map keys. Converting between netip and net types is fairly trivial, but fewer conversions is more ergonomic. The NetworkDB entries for the overlay peer table encode the IP addresses as strings. We need to parse them to some representation before processing them further. Parse directly into netip types and pass those values around to cut down on the number of conversions needed. The peerDB needs to marshal the keys and entries to structs of hashable values to be able to insert them into the SetMatrix. Use netip.Addr in peerEntry so that peerEntry values can be directly inserted into the SetMatrix without conversions. Use a hashable struct type as the SetMatrix key to avoid having to marshal the whole struct to a string and parse it back out. Use netip.Addr as the map key for the driver's encryption map so the values do not need to be converted to and from strings. Change the encryption configuration methods to take netip types so the peerDB code can pass netip values directly. Signed-off-by: Cory Snider <csnider@mirantis.com>
6715ce4 to
d188df0
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors various parts of the overlay driver code to use netip types instead of net types, improving hashability and reducing conversions. Key changes include updating SetMatrix generic parameters, modifying peerDB methods to work with netip types, and revising encryption and network functions to operate on the new types.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| libnetwork/service.go | Update of SetMatrix type parameters in the service struct |
| libnetwork/network.go | Changes in setmatrix usage and conversions to netip types |
| libnetwork/internal/setmatrix/setmatrix*.go | Update of SetMatrix generics across insertion, deletion, etc. |
| libnetwork/drivers/overlay/*.go | Comprehensive conversion of net and IP-related logic to netip |
Comments suppressed due to low confidence (4)
libnetwork/service.go:60
- Ensure that updating the generic parameters for SetMatrix in the service struct aligns with its usage elsewhere in the module, and that this change does not break any implicit assumptions about key and value types.
ipToEndpoint setmatrix.SetMatrix[string, string]
libnetwork/drivers/overlay/peerdb.go:129
- Confirm that the ipmac type used as a key correctly implements equality and hashing semantics to prevent unexpected behavior in map operations.
b, i := pMap.mp.Insert(pKey, pEntry)
libnetwork/drivers/overlay/ov_network.go:616
- Review the logic in getSubnetforIP to ensure that comparing prefix mask lengths using Bits() is sufficient for matching subnets and that the Contains method is applied correctly.
func (n *network) getSubnetforIP(ip netip.Prefix) *subnet {
libnetwork/drivers/overlay/encryption.go:175
- Review the conversion of netip.Addr to slices via AsSlice(), ensuring compatibility for both IPv4 and IPv6 addresses during SPI computation.
spis := &spi{buildSPI(advIP.AsSlice(), remoteIP.AsSlice(), k.tag), buildSPI(remoteIP.AsSlice(), advIP.AsSlice(), k.tag)}
|
Looked from my phone, but this looks great! |
robmry
approved these changes
May 28, 2025
vvoland
approved these changes
May 29, 2025
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.
- What I did
- How I did it
The netip types are really useful for tracking state in the overlay driver as they are hashable, unlike net.IP and friends, making them directly useable as map keys. Converting between netip and net types is fairly trivial, but fewer conversions is more ergonomic.
The NetworkDB entries for the overlay peer table encode the IP addresses as strings. We need to parse them to some representation before processing them further. Parse directly into netip types and pass those values around to cut down on the number of conversions needed.
The peerDB needs to marshal the keys and entries to structs of hashable values to be able to insert them into the SetMatrix. Use netip.Addr in peerEntry so that peerEntry values can be directly inserted into the SetMatrix without conversions. Use a hashable struct type as the SetMatrix key to avoid having to marshal the whole struct to a string and parse it back out.
Use netip.Addr as the map key for the driver's encryption map so the values do not need to be converted to and from strings. Change the encryption configuration methods to take netip types so the peerDB code can pass netip values directly.
- How to verify it
docker service create --mode global --network my-overlay -p 80:8080 nginxdemos/hello:plain-text- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)