From dc6479d051e08661619b8cf6798fb11700c9b079 Mon Sep 17 00:00:00 2001 From: Aviram Hassan Date: Fri, 25 Oct 2024 15:14:57 +0300 Subject: [PATCH 1/4] Added experimental disable_reuseaddr to bypass the issue --- changelog.d/2819.fixed.md | 1 + mirrord-schema.json | 8 ++++++++ mirrord/config/src/experimental.rs | 11 +++++++++++ mirrord/layer/src/socket/ops.rs | 17 ++++++++++++++++- 4 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 changelog.d/2819.fixed.md diff --git a/changelog.d/2819.fixed.md b/changelog.d/2819.fixed.md new file mode 100644 index 00000000000..0bc5bc8320c --- /dev/null +++ b/changelog.d/2819.fixed.md @@ -0,0 +1 @@ +Added experimental disable_reuseaddr to bypass the issue \ No newline at end of file diff --git a/mirrord-schema.json b/mirrord-schema.json index 0410f006563..da6463b0443 100644 --- a/mirrord-schema.json +++ b/mirrord-schema.json @@ -756,6 +756,14 @@ "description": "mirrord Experimental features. This shouldn't be used unless someone from MetalBear/mirrord tells you to.", "type": "object", "properties": { + "disable_reuseaddr": { + "title": "_experimental_ disable_reuseaddr {#experimental-disable_reuseaddr}", + "description": "Disables the `SO_REUSEADDR` socket option on sockets that mirrord steals/mirrors. on macOS the application can use the same address many times but then we don't steal it correctly. This probably should be on by default but we want to gradual roll it out. https://github.com/metalbear-co/mirrord/issues/2819 This option applies only on macOS.", + "type": [ + "boolean", + "null" + ] + }, "enable_exec_hooks_linux": { "title": "_experimental_ enable_exec_hooks_linux {#experimental-enable_exec_hooks_linux}", "description": "Enables exec hooks on Linux. Enable Linux hooks can fix issues when the application shares sockets with child commands (e.g Python web servers with reload), but the feature is not stable and may cause other issues.", diff --git a/mirrord/config/src/experimental.rs b/mirrord/config/src/experimental.rs index 6cf31e3d28e..0755f5d23eb 100644 --- a/mirrord/config/src/experimental.rs +++ b/mirrord/config/src/experimental.rs @@ -42,6 +42,16 @@ pub struct ExperimentalConfig { /// Enables `getifaddrs` hook that removes IPv6 interfaces from the list returned by libc. #[config(default = false)] pub hide_ipv6_interfaces: bool, + + /// ### _experimental_ disable_reuseaddr {#experimental-disable_reuseaddr} + /// + /// Disables the `SO_REUSEADDR` socket option on sockets that mirrord steals/mirrors. + /// on macOS the application can use the same address many times but then we don't steal it + /// correctly. This probably should be on by default but we want to gradual roll it out. + /// https://github.com/metalbear-co/mirrord/issues/2819 + /// This option applies only on macOS. + #[config(default = false)] + pub disable_reuseaddr: bool, } impl CollectAnalytics for &ExperimentalConfig { @@ -51,5 +61,6 @@ impl CollectAnalytics for &ExperimentalConfig { analytics.add("trust_any_certificate", self.trust_any_certificate); analytics.add("enable_exec_hooks_linux", self.enable_exec_hooks_linux); analytics.add("hide_ipv6_interfaces", self.hide_ipv6_interfaces); + analytics.add("disable_reuseaddr", self.disable_reuseaddr); } } diff --git a/mirrord/layer/src/socket/ops.rs b/mirrord/layer/src/socket/ops.rs index e46502ca939..89fe06fd26d 100644 --- a/mirrord/layer/src/socket/ops.rs +++ b/mirrord/layer/src/socket/ops.rs @@ -217,7 +217,6 @@ pub(super) fn bind( let requested_address = SocketAddr::try_from_raw(raw_address, address_length)?; let requested_port = requested_address.port(); let incoming_config = crate::setup().incoming_config(); - let mut socket = { SOCKETS .lock()? @@ -275,6 +274,22 @@ pub(super) fn bind( Err(HookError::AddressAlreadyBound(requested_address))?; } + #[cfg(target_os = "macos")] + unsafe { + let experimental = crate::setup().experimental(); + if experimental.disable_reuseaddr { + tracing::debug!("here!!"); + let socket = socket2::Socket::from_raw_fd(sockfd); + if let Err(e) = socket.set_reuse_address(false) { + tracing::debug!(?e, "Failed to set SO_REUSEADDR to false"); + } + if let Err(e) = socket.set_reuse_port(false) { + tracing::debug!(?e, "Failed to set SE_REUSEPORT to false"); + } + // get ownership of fd back + let _ = socket.into_raw_fd(); + } + } // Try to bind a port from listen ports, if no configuration // try to bind the requested port, if not available get a random port // if there's configuration and binding fails with the requested port From d667c57bd39a4075925d2d021499fec5a5d6e5b0 Mon Sep 17 00:00:00 2001 From: Aviram Hassan Date: Fri, 25 Oct 2024 16:26:34 +0300 Subject: [PATCH 2/4] Cr --- mirrord-schema.json | 2 +- mirrord/config/src/experimental.rs | 4 ++-- mirrord/layer/src/socket/ops.rs | 15 ++++++++------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/mirrord-schema.json b/mirrord-schema.json index da6463b0443..a3fef55b7a7 100644 --- a/mirrord-schema.json +++ b/mirrord-schema.json @@ -758,7 +758,7 @@ "properties": { "disable_reuseaddr": { "title": "_experimental_ disable_reuseaddr {#experimental-disable_reuseaddr}", - "description": "Disables the `SO_REUSEADDR` socket option on sockets that mirrord steals/mirrors. on macOS the application can use the same address many times but then we don't steal it correctly. This probably should be on by default but we want to gradual roll it out. https://github.com/metalbear-co/mirrord/issues/2819 This option applies only on macOS.", + "description": "Disables the `SO_REUSEADDR` socket option on sockets that mirrord steals/mirrors. On macOS the application can use the same address many times but then we don't steal it correctly. This probably should be on by default but we want to gradually roll it out. https://github.com/metalbear-co/mirrord/issues/2819 This option applies only on macOS.", "type": [ "boolean", "null" diff --git a/mirrord/config/src/experimental.rs b/mirrord/config/src/experimental.rs index 0755f5d23eb..2f66e771b52 100644 --- a/mirrord/config/src/experimental.rs +++ b/mirrord/config/src/experimental.rs @@ -46,8 +46,8 @@ pub struct ExperimentalConfig { /// ### _experimental_ disable_reuseaddr {#experimental-disable_reuseaddr} /// /// Disables the `SO_REUSEADDR` socket option on sockets that mirrord steals/mirrors. - /// on macOS the application can use the same address many times but then we don't steal it - /// correctly. This probably should be on by default but we want to gradual roll it out. + /// On macOS the application can use the same address many times but then we don't steal it + /// correctly. This probably should be on by default but we want to gradually roll it out. /// https://github.com/metalbear-co/mirrord/issues/2819 /// This option applies only on macOS. #[config(default = false)] diff --git a/mirrord/layer/src/socket/ops.rs b/mirrord/layer/src/socket/ops.rs index 89fe06fd26d..4a7be9ef1cb 100644 --- a/mirrord/layer/src/socket/ops.rs +++ b/mirrord/layer/src/socket/ops.rs @@ -275,19 +275,20 @@ pub(super) fn bind( } #[cfg(target_os = "macos")] - unsafe { + { let experimental = crate::setup().experimental(); if experimental.disable_reuseaddr { - tracing::debug!("here!!"); - let socket = socket2::Socket::from_raw_fd(sockfd); - if let Err(e) = socket.set_reuse_address(false) { + let fd = unsafe { BorrowedFd::borrow_raw(sockfd) }; + if let Err(e) = + nix::sys::socket::setsockopt(&fd, nix::sys::socket::sockopt::ReuseAddr, &false) + { tracing::debug!(?e, "Failed to set SO_REUSEADDR to false"); } - if let Err(e) = socket.set_reuse_port(false) { + if let Err(e) = + nix::sys::socket::setsockopt(&fd, nix::sys::socket::sockopt::ReusePort, &false) + { tracing::debug!(?e, "Failed to set SE_REUSEPORT to false"); } - // get ownership of fd back - let _ = socket.into_raw_fd(); } } // Try to bind a port from listen ports, if no configuration From 441d0700a4f6e1c47e398c9c3c46e14e27fad5e6 Mon Sep 17 00:00:00 2001 From: Aviram Hassan Date: Sat, 26 Oct 2024 09:16:22 +0300 Subject: [PATCH 3/4] nit --- mirrord/config/src/experimental.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mirrord/config/src/experimental.rs b/mirrord/config/src/experimental.rs index 2f66e771b52..a226af6793e 100644 --- a/mirrord/config/src/experimental.rs +++ b/mirrord/config/src/experimental.rs @@ -48,7 +48,7 @@ pub struct ExperimentalConfig { /// Disables the `SO_REUSEADDR` socket option on sockets that mirrord steals/mirrors. /// On macOS the application can use the same address many times but then we don't steal it /// correctly. This probably should be on by default but we want to gradually roll it out. - /// https://github.com/metalbear-co/mirrord/issues/2819 + /// /// This option applies only on macOS. #[config(default = false)] pub disable_reuseaddr: bool, From c82a69ba93f10e0c387b8507b39eca88d8b14ec1 Mon Sep 17 00:00:00 2001 From: Aviram Hassan Date: Sun, 27 Oct 2024 09:38:10 +0200 Subject: [PATCH 4/4] .. --- mirrord-schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mirrord-schema.json b/mirrord-schema.json index a2dda84dfd1..4e7278f0869 100644 --- a/mirrord-schema.json +++ b/mirrord-schema.json @@ -758,7 +758,7 @@ "properties": { "disable_reuseaddr": { "title": "_experimental_ disable_reuseaddr {#experimental-disable_reuseaddr}", - "description": "Disables the `SO_REUSEADDR` socket option on sockets that mirrord steals/mirrors. On macOS the application can use the same address many times but then we don't steal it correctly. This probably should be on by default but we want to gradually roll it out. https://github.com/metalbear-co/mirrord/issues/2819 This option applies only on macOS.", + "description": "Disables the `SO_REUSEADDR` socket option on sockets that mirrord steals/mirrors. On macOS the application can use the same address many times but then we don't steal it correctly. This probably should be on by default but we want to gradually roll it out. This option applies only on macOS.", "type": [ "boolean", "null"