Skip to content

Conversation

@camshaft
Copy link
Contributor

Description of changes:

Currently, the secret map control socket binds to an ipv4 address. This means that if a peer connects with ipv6 it's unable to receive secret control messages. This change fixes the control sockets to try binding to a ipv6 address before ipv4.

Testing:

I was able to verify this fixes issues with ipv6 deployments. That being said, we should probably add more ipv6 coverage in our tests. Unfortunately I don't have a ton of time to do that as part of this PR so it will have to come later.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@camshaft camshaft marked this pull request as ready for review October 24, 2025 17:37
@camshaft camshaft requested a review from a team as a code owner October 24, 2025 17:37
fn send_control_packet(&self, dst: &SocketAddr, buffer: &mut [u8]) {
match self.control_socket.send_to(buffer, dst) {
let Some(control_socket) = self.control_socket.as_ref() else {
return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It'd be good to wire this up to an event -- otherwise I suspect it'll be hard to debug why we're missing sends in production, should we fail to create the control socket for some reason. Probably also an event at map initialization time (bool field on the existing event?) noting that happened, with =1 being the negative state (failed to create) for easy alarming.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good call - though it'll need to be a follow-up change as I haven't had a ton of time :)

@boquan-fang boquan-fang enabled auto-merge (squash) November 3, 2025 19:30
@boquan-fang boquan-fang merged commit f7c2ac0 into aws:main Nov 3, 2025
120 checks passed
@camshaft camshaft deleted the camshaft/dc-control-socket branch November 3, 2025 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants