From aab0b7ef339510966bd172a747ec29903e8395dd Mon Sep 17 00:00:00 2001 From: Lutz Justen Date: Fri, 3 Feb 2023 11:33:07 +0100 Subject: [PATCH] [PortManager] Prefer ss over netstat on Linux (#91) ss is a modern alternative to netstat. The flags we use and the way we parse the output are compatible with netstat. Since netstat is no longer installed on some Linux distributions, prefer ss, but fall back to netstat if "which ss" fails. Also tweaks some logging. Fixes #65 --- cdc_rsync/cdc_rsync_client.cc | 6 ++++ cdc_rsync/main.cc | 4 +-- cdc_rsync/zstd_stream.cc | 8 +++--- common/port_manager_test.cc | 54 ++++++++++++++++++++++++++--------- common/port_manager_win.cc | 4 ++- 5 files changed, 55 insertions(+), 21 deletions(-) diff --git a/cdc_rsync/cdc_rsync_client.cc b/cdc_rsync/cdc_rsync_client.cc index e823d65..a002161 100644 --- a/cdc_rsync/cdc_rsync_client.cc +++ b/cdc_rsync/cdc_rsync_client.cc @@ -146,6 +146,9 @@ absl::Status CdcRsyncClient::Run() { // Check whether we guessed the arch type wrong and try again. // Note that in case of a local sync, or if the server actively reported // that it's out-dated, there's no need to detect the arch. + LOG_DEBUG( + "Failed to start server, retrying after detecting remote arch: %s", + status.ToString()); const ArchType old_type = server_arch.GetType(); ASSIGN_OR_RETURN(server_arch, ServerArch::DetectFromRemoteDevice(remote_util_.get())); @@ -219,6 +222,9 @@ absl::StatusOr CdcRsyncClient::FindAvailablePort(ServerArch* server_arch) { // the arch was wrong. Properly detect it and try again if it changed. if (!port.ok() && server_arch->IsGuess()) { const ArchType old_type = server_arch->GetType(); + LOG_DEBUG( + "Failed to reserve port, retrying after detecting remote arch: %s", + port.status().ToString()); ASSIGN_OR_RETURN(*server_arch, ServerArch::DetectFromRemoteDevice(remote_util_.get())); assert(!server_arch->IsGuess()); diff --git a/cdc_rsync/main.cc b/cdc_rsync/main.cc index 4595bba..5eda7e5 100644 --- a/cdc_rsync/main.cc +++ b/cdc_rsync/main.cc @@ -68,8 +68,8 @@ ReturnCode TagToMessage(cdc_ft::Tag tag, case cdc_ft::Tag::kDeployServer: *msg = - "Failed to deploy the instance components for unknown reasons. " - "Please report this issue."; + "Failed to deploy or run the instance components for unknown " + "reasons. Please report this issue."; return ReturnCode::kDeployFailed; case cdc_ft::Tag::kConnectionTimeout: diff --git a/cdc_rsync/zstd_stream.cc b/cdc_rsync/zstd_stream.cc index 6460f99..bbd952d 100644 --- a/cdc_rsync/zstd_stream.cc +++ b/cdc_rsync/zstd_stream.cc @@ -146,10 +146,10 @@ void ZstdStream::ThreadCompressorMain() { const ZSTD_EndDirective mode = last_chunk_ ? ZSTD_e_end : flush ? ZSTD_e_flush : ZSTD_e_continue; - LOG_DEBUG("Compressing %u bytes (mode=%s)", in_buffer_.size(), - mode == ZSTD_e_end ? "end" - : mode == ZSTD_e_flush ? "flush" - : "continue"); + LOG_VERBOSE("Compressing %u bytes (mode=%s)", in_buffer_.size(), + mode == ZSTD_e_end ? "end" + : mode == ZSTD_e_flush ? "flush" + : "continue"); ZSTD_inBuffer input = {in_buffer_.data(), in_buffer_.size(), 0}; bool finished = false; do { diff --git a/common/port_manager_test.cc b/common/port_manager_test.cc index 03e750d..8806110 100644 --- a/common/port_manager_test.cc +++ b/common/port_manager_test.cc @@ -35,12 +35,17 @@ constexpr int kNumPorts = kLastPort - kFirstPort + 1; constexpr int kTimeoutSec = 1; constexpr char kWindowsNetstat[] = "netstat -a -n -p tcp"; -constexpr char kLinuxNetstat[] = "netstat --numeric --listening --tcp"; +constexpr char kLinuxNetstatOrSS[] = + "which ss2 && ss --numeric --listening --tcp || netstat --numeric " + "--listening --tcp"; constexpr char kWindowsNetstatOutFmt[] = "TCP 127.0.0.1:50000 127.0.0.1:%i ESTABLISHED"; constexpr char kLinuxNetstatOutFmt[] = "tcp 0 0 0.0.0.0:%i 0.0.0.0:* LISTEN"; +constexpr char kLinuxSSOutFmt[] = + "LISTEN 0 128 " + " 0.0.0.0:%i 0.0.0.0:* "; class PortManagerTest : public ::testing::Test { public: @@ -67,7 +72,7 @@ class PortManagerTest : public ::testing::Test { TEST_F(PortManagerTest, ReservePortSuccess) { process_factory_.SetProcessOutput(kWindowsNetstat, "", "", 0); - process_factory_.SetProcessOutput(kLinuxNetstat, "", "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, "", "", 0); absl::StatusOr port = port_manager_.ReservePort(kTimeoutSec, ArchType::kLinux_x86_64); @@ -81,7 +86,7 @@ TEST_F(PortManagerTest, ReservePortAllLocalPortsTaken) { local_netstat_out += absl::StrFormat(kWindowsNetstatOutFmt, port); } process_factory_.SetProcessOutput(kWindowsNetstat, local_netstat_out, "", 0); - process_factory_.SetProcessOutput(kLinuxNetstat, "", "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, "", "", 0); absl::StatusOr port = port_manager_.ReservePort(kTimeoutSec, ArchType::kLinux_x86_64); @@ -96,7 +101,8 @@ TEST_F(PortManagerTest, ReservePortAllRemotePortsTaken) { remote_netstat_out += absl::StrFormat(kLinuxNetstatOutFmt, port); } process_factory_.SetProcessOutput(kWindowsNetstat, "", "", 0); - process_factory_.SetProcessOutput(kLinuxNetstat, remote_netstat_out, "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, remote_netstat_out, "", + 0); absl::StatusOr port = port_manager_.ReservePort(kTimeoutSec, ArchType::kLinux_x86_64); @@ -107,7 +113,7 @@ TEST_F(PortManagerTest, ReservePortAllRemotePortsTaken) { TEST_F(PortManagerTest, ReservePortLocalNetstatFails) { process_factory_.SetProcessOutput(kWindowsNetstat, "", "", 1); - process_factory_.SetProcessOutput(kLinuxNetstat, "", "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, "", "", 0); absl::StatusOr port = port_manager_.ReservePort(kTimeoutSec, ArchType::kLinux_x86_64); @@ -119,7 +125,7 @@ TEST_F(PortManagerTest, ReservePortLocalNetstatFails) { TEST_F(PortManagerTest, ReservePortRemoteNetstatFails) { process_factory_.SetProcessOutput(kWindowsNetstat, "", "", 0); - process_factory_.SetProcessOutput(kLinuxNetstat, "", "", 1); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, "", "", 1); absl::StatusOr port = port_manager_.ReservePort(kTimeoutSec, ArchType::kLinux_x86_64); @@ -130,7 +136,7 @@ TEST_F(PortManagerTest, ReservePortRemoteNetstatFails) { TEST_F(PortManagerTest, ReservePortRemoteNetstatTimesOut) { process_factory_.SetProcessOutput(kWindowsNetstat, "", "", 0); - process_factory_.SetProcessNeverExits(kLinuxNetstat); + process_factory_.SetProcessNeverExits(kLinuxNetstatOrSS); steady_clock_.AutoAdvance(kTimeoutSec * 2 * 1000); absl::StatusOr port = @@ -143,7 +149,7 @@ TEST_F(PortManagerTest, ReservePortRemoteNetstatTimesOut) { TEST_F(PortManagerTest, ReservePortMultipleInstances) { process_factory_.SetProcessOutput(kWindowsNetstat, "", "", 0); - process_factory_.SetProcessOutput(kLinuxNetstat, "", "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, "", "", 0); PortManager port_manager2(kGuid, kFirstPort, kLastPort, &process_factory_, &remote_util_); @@ -163,7 +169,7 @@ TEST_F(PortManagerTest, ReservePortMultipleInstances) { TEST_F(PortManagerTest, ReservePortReusesPortsInLRUOrder) { process_factory_.SetProcessOutput(kWindowsNetstat, "", "", 0); - process_factory_.SetProcessOutput(kLinuxNetstat, "", "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, "", "", 0); for (int n = 0; n < kNumPorts * 2; ++n) { EXPECT_EQ(*port_manager_.ReservePort(kTimeoutSec, ArchType::kLinux_x86_64), @@ -174,7 +180,7 @@ TEST_F(PortManagerTest, ReservePortReusesPortsInLRUOrder) { TEST_F(PortManagerTest, ReleasePort) { process_factory_.SetProcessOutput(kWindowsNetstat, "", "", 0); - process_factory_.SetProcessOutput(kLinuxNetstat, "", "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, "", "", 0); absl::StatusOr port = port_manager_.ReservePort(kTimeoutSec, ArchType::kLinux_x86_64); @@ -186,7 +192,7 @@ TEST_F(PortManagerTest, ReleasePort) { TEST_F(PortManagerTest, ReleasePortOnDestruction) { process_factory_.SetProcessOutput(kWindowsNetstat, "", "", 0); - process_factory_.SetProcessOutput(kLinuxNetstat, "", "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, "", "", 0); auto port_manager2 = std::make_unique( kGuid, kFirstPort, kLastPort, &process_factory_, &remote_util_); @@ -219,7 +225,8 @@ TEST_F(PortManagerTest, FindAvailableLocalPortsSuccessLinux) { // First port is in use. std::string local_netstat_out = absl::StrFormat(kLinuxNetstatOutFmt, kFirstPort); - process_factory_.SetProcessOutput(kLinuxNetstat, local_netstat_out, "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, local_netstat_out, "", + 0); absl::StatusOr> ports = PortManager::FindAvailableLocalPorts( @@ -251,7 +258,25 @@ TEST_F(PortManagerTest, FindAvailableRemotePortsSuccessLinux) { // First port is in use. std::string remote_netstat_out = absl::StrFormat(kLinuxNetstatOutFmt, kFirstPort); - process_factory_.SetProcessOutput(kLinuxNetstat, remote_netstat_out, "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, remote_netstat_out, "", + 0); + + absl::StatusOr> ports = + PortManager::FindAvailableRemotePorts( + kFirstPort, kLastPort, ArchType::kLinux_x86_64, &process_factory_, + &remote_util_, kTimeoutSec); + ASSERT_OK(ports); + EXPECT_EQ(ports->size(), kNumPorts - 1); + for (int port = kFirstPort + 1; port <= kLastPort; ++port) { + EXPECT_TRUE(ports->find(port) != ports->end()); + } +} + +TEST_F(PortManagerTest, FindAvailableRemotePortsSuccessLinuxSS) { + // First port is in use, but reporting is done by SS, not netsta. + std::string remote_netstat_out = absl::StrFormat(kLinuxSSOutFmt, kFirstPort); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, remote_netstat_out, "", + 0); absl::StatusOr> ports = PortManager::FindAvailableRemotePorts( @@ -287,7 +312,8 @@ TEST_F(PortManagerTest, FindAvailableRemotePortsFailsNoPorts) { for (int port = kFirstPort; port <= kLastPort; ++port) { remote_netstat_out += absl::StrFormat(kLinuxNetstatOutFmt, port); } - process_factory_.SetProcessOutput(kLinuxNetstat, remote_netstat_out, "", 0); + process_factory_.SetProcessOutput(kLinuxNetstatOrSS, remote_netstat_out, "", + 0); absl::StatusOr> ports = PortManager::FindAvailableRemotePorts( diff --git a/common/port_manager_win.cc b/common/port_manager_win.cc index ec22354..27f0565 100644 --- a/common/port_manager_win.cc +++ b/common/port_manager_win.cc @@ -43,10 +43,12 @@ const char* GetNetstatCommand(ArchType arch_type) { } if (IsLinuxArchType(arch_type)) { + // Prefer ss over netstat. The flags out output are compatible. // --numeric to get numerical addresses. // --listening to get only listening sockets. // --tcp to get only TCP connections. - return "netstat --numeric --listening --tcp"; + return "which ss2 && ss --numeric --listening --tcp || netstat " + "--numeric --listening --tcp"; } assert(!kErrorArchTypeUnhandled);