Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fqdn: avoid converting from netip.Addr to net.IP and back #29625

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Dec 5, 2023

Change (*DNSCache).Lookup and (*DNSCache).LookupByRegex to return netip.Addr. This avoids a back-and-forth conversion to net.IP and allows to drop some now unused helper functions in pkg/ip.

See commits for details.

Towards: #24246

@tklauser tklauser added kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. labels Dec 5, 2023
@tklauser tklauser requested review from a team as code owners December 5, 2023 10:57
@tklauser
Copy link
Member Author

tklauser commented Dec 5, 2023

/test

fqdn.DNSCache keeps its IP addresses as netip.Addr internally already.
The (*DNSCache).Lookup and (*DNSCache.LookupByRegex methods currently
return a []net.IP and map[string][]net.IP but their callers directly
convert the IPs in the return values to netip.Addr again. Avoid this
back-and-forth conversion and let the methods return netip.Addr types
directly.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
The last remaining users of KeepUniqueIPs were converted to use
KeepUniqueAddrs. Remove the now unused function.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
SortedIPListsAreEqual no longer has any callers outside of pkg/ip.
Inline the use into the only caller and avoid a second slice length
comparision while at it.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser force-pushed the pr/tklauser/fqdn-netip-no-back-and-forth branch from 08942e7 to f466edb Compare December 5, 2023 11:13
@tklauser
Copy link
Member Author

tklauser commented Dec 5, 2023

/test

@tklauser tklauser added the kind/tech-debt Technical debt label Dec 5, 2023
Copy link
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks! 💯

@tklauser tklauser added this pull request to the merge queue Dec 6, 2023
@tklauser tklauser removed the request for review from danehans December 6, 2023 12:12
Merged via the queue into main with commit 61b9a21 Dec 6, 2023
203 checks passed
@tklauser tklauser deleted the pr/tklauser/fqdn-netip-no-back-and-forth branch December 6, 2023 12:19
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 6, 2023
@tklauser tklauser added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Dec 12, 2023
@giorio94 giorio94 mentioned this pull request Dec 13, 2023
10 tasks
@giorio94 giorio94 added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Dec 13, 2023
@giorio94
Copy link
Member

Dropped the backport labels based on #29863 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup This includes no functional changes. kind/tech-debt Technical debt ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants