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

endpoint: correctly log IPv6 addresses #24255

Merged
merged 1 commit into from
Mar 9, 2023

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Mar 8, 2023

Reference: f3a4c4d#r103598930
Fixes: f3a4c4d ("endpoint: convert Endpoint from addressing.CiliumIPv{4,6} to netip.Addr")
Reported-by: Joe Stringer joe@cilium.io

Reference: f3a4c4d#r103598930
Fixes: f3a4c4d ("endpoint: convert Endpoint from addressing.CiliumIPv{4,6} to netip.Addr")
Reported-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser added release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Mar 8, 2023
@tklauser tklauser requested a review from a team as a code owner March 8, 2023 22:10
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.13.1 Mar 8, 2023
tklauser referenced this pull request Mar 8, 2023
A netip.Addr always uses 24 bytes, regardless of the address type. An
addressing.CiliumIPv4 uses 28 bytes (24 bytes for the slice header and 4
bytes for the address). An addressing.CiliumIPv6 uses 56 bytes (24 bytes
for the slice header and 32 bytes for the address). Also see [1] for
more details on further advantages of the netip types in terms of
performance.

[1] https://tailscale.com/blog/netaddr-new-ip-type-for-go/

This change thus slightly reduces the in-memory size of the Endpoint
type and also allows to drop the custom addressing types in favor of Go
standard library types with corresponding helper functions and methods.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
Copy link
Contributor

@tommyp1ckles tommyp1ckles left a comment

Choose a reason for hiding this comment

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

seems very reasonable

@tklauser
Copy link
Member Author

tklauser commented Mar 9, 2023

/test

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Thanks!

@joestringer joestringer merged commit 54ee633 into master Mar 9, 2023
@joestringer joestringer deleted the pr/tklauser/endpoint-addr-log-fix branch March 9, 2023 16:25
@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 Mar 9, 2023
@nebril nebril added this to Needs backport from master in 1.13.2 Mar 15, 2023
@nebril nebril removed this from Needs backport from master in 1.13.1 Mar 15, 2023
@NikAleksandrov NikAleksandrov mentioned this pull request Mar 23, 2023
29 tasks
@NikAleksandrov NikAleksandrov added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Mar 23, 2023
@jibi jibi added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Apr 7, 2023
@gentoo-root gentoo-root moved this from Needs backport from master to Backport done to v1.13 in 1.13.2 Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. 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
No open projects
1.13.2
Backport done to v1.13
Status: Released
Development

Successfully merging this pull request may close these issues.

None yet

5 participants