diff --git a/CHANGELOG.md b/CHANGELOG.md index 564090d5c9..161beb1625 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ### NEXT +- `PortManager`: Replace `uint64_t` hash token with exact-tuple `PortRangeKey` ([PR #1812](https://github.com/versatica/mediasoup/pull/1812), by @999purple999 and @penguinol). + ### 3.20.1 - Node: Make all public methods/getters that return an object/array, return a clone of that object/array ([PR #1811](https://github.com/versatica/mediasoup/pull/1811)). @@ -1196,7 +1198,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi ### 3.5.9 - `libwebrtc`: Apply patch by @sspanak and @Ivaka to avoid crash. Related issue: #357. -- `PortManager.cpp`: Do not use `UV_UDP_RECVMMSG` in Windows due to a bug in `libuv` 1.37.0. +- `PortManager`: Do not use `UV_UDP_RECVMMSG` in Windows due to a bug in `libuv` 1.37.0. - Update Node deps. ### 3.5.8 @@ -1290,7 +1292,7 @@ Migrate `npm-scripts.js` to `npm-scripts.mjs` (ES Module) ([PR #1093](https://gi ### 3.4.7 -- `PortManager.cpp`: Do not limit the number of failed `bind()` attempts to 20 since it does not work well in scenarios that launch tons of `Workers` with same port range. Instead iterate all ports in the range given to the Worker. +- `PortManager`: Do not limit the number of failed `bind()` attempts to 20 since it does not work well in scenarios that launch tons of `Workers` with same port range. Instead iterate all ports in the range given to the Worker. - Do not copy `catch.hpp` into `test/include/` but make the GYP `mediasoup-worker-test` target include the corresponding folder in `deps/catch`. ### 3.4.6 diff --git a/worker/include/RTC/PortManager.hpp b/worker/include/RTC/PortManager.hpp index f6976970b3..893b7a123d 100644 --- a/worker/include/RTC/PortManager.hpp +++ b/worker/include/RTC/PortManager.hpp @@ -3,8 +3,10 @@ #include "common.hpp" #include "RTC/Transport.hpp" +#include "Utils.hpp" #include #include +#include #include #include @@ -12,13 +14,75 @@ namespace RTC { class PortManager { - private: + public: enum class Protocol : uint8_t { UDP = 1, TCP }; + /** + * Opaque-ish key identifying one (protocol, bind address, port range) + * tuple. Issued by Bind*() and consumed by Unbind(). Callers store it as + * an opaque token; equality and hashing are exact-tuple based, so distinct + * tuples never collide regardless of how close their numeric + * representations are. + */ + class PortRangeKey + { + private: + friend class PortManager; + friend struct PortRangeKeyHash; + + public: + PortRangeKey() = default; + PortRangeKey( + Protocol protocol, const sockaddr_storage& bindAddr, uint16_t minPort, uint16_t maxPort); + + bool operator==(const PortRangeKey& other) const noexcept; + bool operator!=(const PortRangeKey& other) const noexcept + { + return !(*this == other); + } + + public: + Protocol GetProtocol() const + { + return this->protocol; + } + const sockaddr_storage& GetSockaddrStorage() const + { + return this->bindAddr; + } + uint16_t GetMinPort() const + { + return this->minPort; + } + uint16_t GetMaxPort() const + { + return this->maxPort; + } + + private: + Protocol protocol{ Protocol::UDP }; + sockaddr_storage bindAddr{}; + uint16_t minPort{ 0u }; + uint16_t maxPort{ 0u }; + }; + + struct PortRangeKeyHash + { + /** + * Hash function. Uses ankerl::unordered_dense per-field hashes combined + * with the standard boost-style `hash_combine` seed mixer. We deliberately + * do NOT hash the raw `sockaddr_storage` bytes (padding is + * caller-controlled and would cause structurally-equal keys to hash + * differently); instead we hash the same fields that `operator==` + * inspects. + */ + size_t operator()(const PortRangeKey& key) const noexcept; + }; + private: struct PortRange { @@ -42,9 +106,9 @@ namespace RTC uint16_t minPort, uint16_t maxPort, RTC::Transport::SocketFlags& flags, - uint64_t& hash) + PortRangeKey& key) { - return reinterpret_cast(Bind(Protocol::UDP, ip, minPort, maxPort, flags, hash)); + return reinterpret_cast(Bind(Protocol::UDP, ip, minPort, maxPort, flags, key)); } static uv_tcp_t* BindTcp(std::string& ip, uint16_t port, RTC::Transport::SocketFlags& flags) { @@ -55,11 +119,11 @@ namespace RTC uint16_t minPort, uint16_t maxPort, RTC::Transport::SocketFlags& flags, - uint64_t& hash) + PortRangeKey& key) { - return reinterpret_cast(Bind(Protocol::TCP, ip, minPort, maxPort, flags, hash)); + return reinterpret_cast(Bind(Protocol::TCP, ip, minPort, maxPort, flags, key)); } - static void Unbind(uint64_t hash, uint16_t port); + static void Unbind(const PortRangeKey& key, uint16_t port); void Dump(int indentation = 0) const; private: @@ -71,15 +135,31 @@ namespace RTC uint16_t minPort, uint16_t maxPort, RTC::Transport::SocketFlags& flags, - uint64_t& hash); - static uint64_t GeneratePortRangeHash( - Protocol protocol, sockaddr_storage* bindAddr, uint16_t minPort, uint16_t maxPort); - static PortRange& GetOrCreatePortRange(uint64_t hash, uint16_t minPort, uint16_t maxPort); + PortRangeKey& key); + static PortRange& GetOrCreatePortRange(const PortRangeKey& key, uint16_t minPort, uint16_t maxPort); static uint8_t ConvertSocketFlags(RTC::Transport::SocketFlags& flags, Protocol protocol, int family); private: - static thread_local ankerl::unordered_dense::map mapPortRanges; + static thread_local ankerl::unordered_dense::map mapPortRanges; }; + + /** + * For Catch2 to print it nicely. + */ + inline std::ostream& operator<<(std::ostream& os, const PortManager::PortRangeKey& k) + { + const std::string protocolStr = (k.GetProtocol() == PortManager::Protocol::UDP) ? "udp" : "tcp"; + + int family; + uint16_t port; + std::string ip; + auto* storage = const_cast(std::addressof(k.GetSockaddrStorage())); + + Utils::IP::GetAddressInfo(reinterpret_cast(storage), family, ip, port); + + return os << "{protocol:" << protocolStr << ", family:" << family << ", ip:" << ip + << ", minPort:" << k.GetMinPort() << ", maxPort:" << k.GetMaxPort() << "}"; + } } // namespace RTC #endif diff --git a/worker/include/RTC/TcpServer.hpp b/worker/include/RTC/TcpServer.hpp index 08c8bdbf5d..b63f777a74 100644 --- a/worker/include/RTC/TcpServer.hpp +++ b/worker/include/RTC/TcpServer.hpp @@ -4,6 +4,7 @@ #include "common.hpp" #include "handles/TcpConnectionHandle.hpp" #include "handles/TcpServerHandle.hpp" +#include "RTC/PortManager.hpp" #include "RTC/TcpConnection.hpp" #include "RTC/Transport.hpp" #include @@ -37,7 +38,7 @@ namespace RTC uint16_t minPort, uint16_t maxPort, RTC::Transport::SocketFlags& flags, - uint64_t& portRangeHash); + RTC::PortManager::PortRangeKey& portRangeKey); ~TcpServer() override; /* Pure virtual methods inherited from ::TcpServerHandle. */ @@ -50,7 +51,7 @@ namespace RTC Listener* listener{ nullptr }; RTC::TcpConnection::Listener* connListener{ nullptr }; bool fixedPort{ false }; - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey{}; }; } // namespace RTC diff --git a/worker/include/RTC/UdpSocket.hpp b/worker/include/RTC/UdpSocket.hpp index d995ee251b..a1e21c56fa 100644 --- a/worker/include/RTC/UdpSocket.hpp +++ b/worker/include/RTC/UdpSocket.hpp @@ -3,6 +3,7 @@ #include "common.hpp" #include "handles/UdpSocketHandle.hpp" +#include "RTC/PortManager.hpp" #include "RTC/Transport.hpp" #include @@ -33,7 +34,7 @@ namespace RTC uint16_t minPort, uint16_t maxPort, RTC::Transport::SocketFlags& flags, - uint64_t& portRangeHash); + RTC::PortManager::PortRangeKey& portRangeKey); ~UdpSocket() override; /* Pure virtual methods inherited from ::UdpSocketHandle. */ @@ -45,7 +46,7 @@ namespace RTC // Passed by argument. Listener* listener{ nullptr }; bool fixedPort{ false }; - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey{}; }; } // namespace RTC diff --git a/worker/meson.build b/worker/meson.build index 798f1ad512..ee1f1ec8ec 100644 --- a/worker/meson.build +++ b/worker/meson.build @@ -451,6 +451,7 @@ test_sources = [ 'test/src/testHelpers.cpp', 'test/src/RTC/TestKeyFrameRequestManager.cpp', 'test/src/RTC/TestNackGenerator.cpp', + 'test/src/RTC/TestPortManager.cpp', 'test/src/RTC/TestRateCalculator.cpp', 'test/src/RTC/TestRtpEncodingParameters.cpp', 'test/src/RTC/TestSeqManager.cpp', diff --git a/worker/src/RTC/PipeTransport.cpp b/worker/src/RTC/PipeTransport.cpp index 31d058ab08..b4723e9829 100644 --- a/worker/src/RTC/PipeTransport.cpp +++ b/worker/src/RTC/PipeTransport.cpp @@ -4,6 +4,7 @@ #include "RTC/PipeTransport.hpp" #include "Logger.hpp" #include "MediaSoupErrors.hpp" +#include "RTC/PortManager.hpp" #include "RTC/SCTP/packet/Packet.hpp" #include "Settings.hpp" #include "Utils.hpp" @@ -72,7 +73,7 @@ namespace RTC { if (this->listenInfo.portRange.min != 0 && this->listenInfo.portRange.max != 0) { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey; this->udpSocket = new RTC::UdpSocket( this, @@ -80,7 +81,7 @@ namespace RTC this->listenInfo.portRange.min, this->listenInfo.portRange.max, this->listenInfo.flags, - portRangeHash); + portRangeKey); } else if (this->listenInfo.port != 0) { @@ -92,7 +93,7 @@ namespace RTC // required. else { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey; this->udpSocket = new RTC::UdpSocket( this, @@ -100,7 +101,7 @@ namespace RTC Settings::configuration.rtcMinPort, Settings::configuration.rtcMaxPort, this->listenInfo.flags, - portRangeHash); + portRangeKey); } if (this->listenInfo.sendBufferSize != 0) diff --git a/worker/src/RTC/PlainTransport.cpp b/worker/src/RTC/PlainTransport.cpp index d35927eb7b..76d2021185 100644 --- a/worker/src/RTC/PlainTransport.cpp +++ b/worker/src/RTC/PlainTransport.cpp @@ -4,6 +4,7 @@ #include "RTC/PlainTransport.hpp" #include "Logger.hpp" #include "MediaSoupErrors.hpp" +#include "RTC/PortManager.hpp" #include "RTC/SCTP/packet/Packet.hpp" #include "Settings.hpp" #include "Utils.hpp" @@ -151,7 +152,7 @@ namespace RTC { if (this->listenInfo.portRange.min != 0 && this->listenInfo.portRange.max != 0) { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey; this->udpSocket = new RTC::UdpSocket( this, @@ -159,7 +160,7 @@ namespace RTC this->listenInfo.portRange.min, this->listenInfo.portRange.max, this->listenInfo.flags, - portRangeHash); + portRangeKey); } else if (this->listenInfo.port != 0) { @@ -171,7 +172,7 @@ namespace RTC // required. else { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey; this->udpSocket = new RTC::UdpSocket( this, @@ -179,7 +180,7 @@ namespace RTC Settings::configuration.rtcMinPort, Settings::configuration.rtcMaxPort, this->listenInfo.flags, - portRangeHash); + portRangeKey); } if (this->listenInfo.sendBufferSize != 0) @@ -198,7 +199,7 @@ namespace RTC { if (this->rtcpListenInfo.portRange.min != 0 && this->rtcpListenInfo.portRange.max != 0) { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey; this->rtcpUdpSocket = new RTC::UdpSocket( this, @@ -206,7 +207,7 @@ namespace RTC this->rtcpListenInfo.portRange.min, this->rtcpListenInfo.portRange.max, this->rtcpListenInfo.flags, - portRangeHash); + portRangeKey); } else if (this->rtcpListenInfo.port != 0) { @@ -218,7 +219,7 @@ namespace RTC // required. else { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey; this->rtcpUdpSocket = new RTC::UdpSocket( this, @@ -226,7 +227,7 @@ namespace RTC Settings::configuration.rtcMinPort, Settings::configuration.rtcMaxPort, this->rtcpListenInfo.flags, - portRangeHash); + portRangeKey); } if (this->rtcpListenInfo.sendBufferSize != 0) diff --git a/worker/src/RTC/PortManager.cpp b/worker/src/RTC/PortManager.cpp index f960ea4ceb..5f09634dee 100644 --- a/worker/src/RTC/PortManager.cpp +++ b/worker/src/RTC/PortManager.cpp @@ -6,7 +6,8 @@ #include "Logger.hpp" #include "MediaSoupErrors.hpp" #include "Utils.hpp" -#include // std:make_tuple() +#include // std::memcmp(), std::memset() +#include // std::make_tuple() /* Static methods for UV callbacks. */ @@ -31,9 +32,11 @@ namespace RTC { /* Class variables. */ - thread_local ankerl::unordered_dense::map PortManager::mapPortRanges; + thread_local ankerl::unordered_dense:: + map + PortManager::mapPortRanges; - /* Class methods. */ + /* PortManager class methods. */ uv_handle_t* PortManager::Bind( Protocol protocol, std::string& ip, uint16_t port, RTC::Transport::SocketFlags& flags) @@ -71,7 +74,8 @@ namespace RTC { case AF_INET: { - err = uv_ip4_addr(ip.c_str(), 0, reinterpret_cast(&bindAddr)); + err = uv_ip4_addr( + ip.c_str(), 0, reinterpret_cast(std::addressof(bindAddr))); if (err != 0) { @@ -83,7 +87,8 @@ namespace RTC case AF_INET6: { - err = uv_ip6_addr(ip.c_str(), 0, reinterpret_cast(&bindAddr)); + err = uv_ip6_addr( + ip.c_str(), 0, reinterpret_cast(std::addressof(bindAddr))); if (err != 0) { @@ -105,14 +110,14 @@ namespace RTC { case AF_INET: { - (reinterpret_cast(&bindAddr))->sin_port = htons(port); + (reinterpret_cast(std::addressof(bindAddr)))->sin_port = htons(port); break; } case AF_INET6: { - (reinterpret_cast(&bindAddr))->sin6_port = htons(port); + (reinterpret_cast(std::addressof(bindAddr)))->sin6_port = htons(port); break; } @@ -175,7 +180,7 @@ namespace RTC { err = uv_udp_bind( reinterpret_cast(uvHandle), - reinterpret_cast(&bindAddr), + reinterpret_cast(std::addressof(bindAddr)), bitFlags); if (err != 0) @@ -198,7 +203,7 @@ namespace RTC { err = uv_tcp_bind( reinterpret_cast(uvHandle), - reinterpret_cast(&bindAddr), + reinterpret_cast(std::addressof(bindAddr)), bitFlags); if (err != 0) @@ -250,7 +255,7 @@ namespace RTC uint16_t minPort, uint16_t maxPort, RTC::Transport::SocketFlags& flags, - uint64_t& hash) + PortRangeKey& key) { MS_TRACE(); @@ -288,7 +293,8 @@ namespace RTC { case AF_INET: { - err = uv_ip4_addr(ip.c_str(), 0, reinterpret_cast(&bindAddr)); + err = uv_ip4_addr( + ip.c_str(), 0, reinterpret_cast(std::addressof(bindAddr))); if (err != 0) { @@ -300,7 +306,8 @@ namespace RTC case AF_INET6: { - err = uv_ip6_addr(ip.c_str(), 0, reinterpret_cast(&bindAddr)); + err = uv_ip6_addr( + ip.c_str(), 0, reinterpret_cast(std::addressof(bindAddr))); if (err != 0) { @@ -317,9 +324,9 @@ namespace RTC } } - hash = GeneratePortRangeHash(protocol, std::addressof(bindAddr), minPort, maxPort); + key = PortRangeKey(protocol, bindAddr, minPort, maxPort); - auto& portRange = PortManager::GetOrCreatePortRange(hash, minPort, maxPort); + auto& portRange = PortManager::GetOrCreatePortRange(key, minPort, maxPort); const size_t numPorts = portRange.ports.size(); const size_t numAttempts = numPorts; size_t attempt{ 0u }; @@ -385,14 +392,14 @@ namespace RTC { case AF_INET: { - (reinterpret_cast(&bindAddr))->sin_port = htons(port); + (reinterpret_cast(std::addressof(bindAddr)))->sin_port = htons(port); break; } case AF_INET6: { - (reinterpret_cast(&bindAddr))->sin6_port = htons(port); + (reinterpret_cast(std::addressof(bindAddr)))->sin6_port = htons(port); break; } @@ -455,7 +462,7 @@ namespace RTC { err = uv_udp_bind( reinterpret_cast(uvHandle), - reinterpret_cast(&bindAddr), + reinterpret_cast(std::addressof(bindAddr)), bitFlags); if (err != 0) @@ -477,7 +484,7 @@ namespace RTC { err = uv_tcp_bind( reinterpret_cast(uvHandle), - reinterpret_cast(&bindAddr), + reinterpret_cast(std::addressof(bindAddr)), bitFlags); if (err != 0) @@ -595,16 +602,19 @@ namespace RTC return uvHandle; } - void PortManager::Unbind(uint64_t hash, uint16_t port) + void PortManager::Unbind(const PortRangeKey& key, uint16_t port) { MS_TRACE(); - auto it = PortManager::mapPortRanges.find(hash); + auto it = PortManager::mapPortRanges.find(key); // This should not happen. if (it == PortManager::mapPortRanges.end()) { - MS_ERROR("hash %" PRIu64 " doesn't exist in the map", hash); + MS_ERROR( + "port range key [minPort:%" PRIu16 ", maxPort:%" PRIu16 "] doesn't exist in the map", + key.minPort, + key.maxPort); return; } @@ -633,13 +643,15 @@ namespace RTC { MS_DUMP_CLEAN(indentation, ""); - for (auto& kv : PortManager::mapPortRanges) + for (const auto& kv : PortManager::mapPortRanges) { - auto hash = kv.first; - auto portRange = kv.second; + const auto& key = kv.first; + const auto& portRange = kv.second; + const char* protocolStr = (key.protocol == Protocol::UDP) ? "udp" : "tcp"; MS_DUMP_CLEAN(indentation + 1, ""); - MS_DUMP_CLEAN(indentation + 1, " hash: %" PRIu64, hash); + MS_DUMP_CLEAN(indentation + 1, " protocol: %s", protocolStr); + MS_DUMP_CLEAN(indentation + 1, " family: %d", key.bindAddr.ss_family); MS_DUMP_CLEAN(indentation + 1, " minPort: %" PRIu16, portRange.minPort); MS_DUMP_CLEAN(indentation + 1, " maxPort: %zu", portRange.minPort + portRange.ports.size() - 1); MS_DUMP_CLEAN(indentation + 1, " numUsedPorts: %" PRIu16, portRange.numUsedPorts); @@ -649,95 +661,14 @@ namespace RTC MS_DUMP_CLEAN(indentation, ""); } - /* - * Hash for IPv4. - * - * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 - * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - * | MIN PORT | MAX PORT | - * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - * | IP | IP >> 2 |F|P| - * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - * - * Hash for IPv6. - * - * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 - * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - * | MIN PORT | MAX PORT | - * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - * |IP[0] ^ IP[1] ^ IP[2] ^ IP[3] | same >> 2 |F|P| - * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - */ - uint64_t PortManager::GeneratePortRangeHash( - Protocol protocol, sockaddr_storage* bindAddr, uint16_t minPort, uint16_t maxPort) - { - MS_TRACE(); - - uint64_t hash{ 0u }; - - switch (bindAddr->ss_family) - { - case AF_INET: - { - auto* bindAddrIn = reinterpret_cast(bindAddr); - - // We want it in network order. - const uint64_t address = bindAddrIn->sin_addr.s_addr; - - hash |= static_cast(minPort) << 48; - hash |= static_cast(maxPort) << 32; - hash |= (address >> 2) << 2; - hash |= 0x0000; // AF_INET. - - break; - } - - case AF_INET6: - { - auto* bindAddrIn6 = reinterpret_cast(bindAddr); - auto* a = reinterpret_cast(std::addressof(bindAddrIn6->sin6_addr)); - - const auto address = a[0] ^ a[1] ^ a[2] ^ a[3]; - - hash |= static_cast(minPort) << 48; - hash |= static_cast(maxPort) << 32; - hash |= static_cast(address) << 16; - hash |= (static_cast(address) >> 2) << 2; - hash |= 0x0002; // AF_INET6. - - break; - } - - // This cannot happen. - default: - { - MS_THROW_ERROR("unknown IP family"); - } - } - - // Override least significant bit with protocol information: - // - If UDP, start with 0. - // - If TCP, start with 1. - if (protocol == Protocol::UDP) - { - hash |= 0x0000; - } - else - { - hash |= 0x0001; - } - - return hash; - } - PortManager::PortRange& PortManager::GetOrCreatePortRange( - uint64_t hash, uint16_t minPort, uint16_t maxPort) + const PortRangeKey& key, uint16_t minPort, uint16_t maxPort) { MS_TRACE(); - auto it = PortManager::mapPortRanges.find(hash); + const auto it = PortManager::mapPortRanges.find(key); - // If the hash is already handled, return its port range. + // If the key is already handled, return its port range. if (it != PortManager::mapPortRanges.end()) { auto& portRange = it->second; @@ -747,10 +678,10 @@ namespace RTC const uint16_t numPorts = maxPort - minPort + 1; - // Emplace a new vector filled with numPorts false values, meaning that + // Emplace a new vector filled with `numPorts` false values, meaning that // all ports are available. - auto pair = PortManager::mapPortRanges.emplace( - std::piecewise_construct, std::make_tuple(hash), std::make_tuple(numPorts, minPort)); + const auto pair = PortManager::mapPortRanges.emplace( + std::piecewise_construct, std::forward_as_tuple(key), std::forward_as_tuple(numPorts, minPort)); // pair.first is an iterator to the inserted value. auto& portRange = pair.first->second; @@ -764,7 +695,7 @@ namespace RTC uint8_t bitFlags{ 0b00000000 }; - // Ignore ipv6Only in IPv4, otherwise libuv will throw. + // Ignore `ipv6Only` in IPv4, otherwise libuv will throw. if (flags.ipv6Only && family == AF_INET6) { switch (protocol) @@ -785,7 +716,7 @@ namespace RTC } } - // Ignore udpReusePort in TCP, otherwise libuv will throw. + // Ignore `udpReusePort` in TCP, otherwise libuv will throw. if (flags.udpReusePort && protocol == Protocol::UDP) { bitFlags |= UV_UDP_REUSEADDR; @@ -793,4 +724,133 @@ namespace RTC return bitFlags; } + + /* PortRangeKey instance methods. */ + + PortManager::PortRangeKey::PortRangeKey( + Protocol protocol, const sockaddr_storage& bindAddr, uint16_t minPort, uint16_t maxPort) + : protocol(protocol), bindAddr(bindAddr), minPort(minPort), maxPort(maxPort) + { + MS_TRACE(); + + // `sockaddr_storage` is padded; the unused tail bytes are caller-controlled. + // `operator==` inspects only the meaningful address bytes (`sin_addr` / + // `sin6_addr`) so padding does not affect equality, and the hash function + // hashes the same exact fields, so two structurally-equal keys always + // produce the same hash regardless of how the caller zero-initialized. + } + + bool PortManager::PortRangeKey::operator==(const PortRangeKey& other) const noexcept + { + MS_TRACE(); + + if (this->protocol != other.protocol) + { + return false; + } + else if (this->minPort != other.minPort) + { + return false; + } + else if (this->maxPort != other.maxPort) + { + return false; + } + else if (this->bindAddr.ss_family != other.bindAddr.ss_family) + { + return false; + } + + switch (this->bindAddr.ss_family) + { + case AF_INET: + { + const auto* a = reinterpret_cast(std::addressof(this->bindAddr)); + const auto* b = reinterpret_cast(std::addressof(other.bindAddr)); + + return a->sin_addr.s_addr == b->sin_addr.s_addr; + } + + case AF_INET6: + { + const auto* a = reinterpret_cast(std::addressof(this->bindAddr)); + const auto* b = reinterpret_cast(std::addressof(other.bindAddr)); + + return std::memcmp( + std::addressof(a->sin6_addr), std::addressof(b->sin6_addr), sizeof(in6_addr)) == 0; + } + + default: + { + // Unknown family; treat as not equal to avoid accidental merge. + return false; + } + } + } + + /* PortRangeKeyHash instance methods. */ + + size_t PortManager::PortRangeKeyHash::operator()(const PortRangeKey& key) const noexcept + { + MS_TRACE(); + + const auto protocolBits = static_cast(key.protocol); + const auto familyBits = static_cast(key.bindAddr.ss_family); + + auto hashCombine = [](size_t& seed, size_t value) + { + seed ^= value + 0x9e3779b9 + (seed << 6) + (seed >> 2); + }; + + size_t seed = 0; + + switch (key.bindAddr.ss_family) + { + case AF_INET: + { + const auto* in = reinterpret_cast(std::addressof(key.bindAddr)); + + hashCombine(seed, ankerl::unordered_dense::hash{}(protocolBits)); + hashCombine(seed, ankerl::unordered_dense::hash{}(familyBits)); + hashCombine(seed, ankerl::unordered_dense::hash{}(in->sin_addr.s_addr)); + hashCombine(seed, ankerl::unordered_dense::hash{}(key.minPort)); + hashCombine(seed, ankerl::unordered_dense::hash{}(key.maxPort)); + + break; + } + + case AF_INET6: + { + const auto* in6 = reinterpret_cast(std::addressof(key.bindAddr)); + const auto* addr = in6->sin6_addr.s6_addr; + + uint64_t hi; + uint64_t lo; + + std::memcpy(std::addressof(hi), addr, sizeof(uint64_t)); + std::memcpy(std::addressof(lo), addr + sizeof(uint64_t), sizeof(uint64_t)); + + hashCombine(seed, ankerl::unordered_dense::hash{}(protocolBits)); + hashCombine(seed, ankerl::unordered_dense::hash{}(familyBits)); + hashCombine(seed, ankerl::unordered_dense::hash{}(hi)); + hashCombine(seed, ankerl::unordered_dense::hash{}(lo)); + hashCombine(seed, ankerl::unordered_dense::hash{}(key.minPort)); + hashCombine(seed, ankerl::unordered_dense::hash{}(key.maxPort)); + + break; + } + + default: + { + hashCombine(seed, ankerl::unordered_dense::hash{}(protocolBits)); + hashCombine(seed, ankerl::unordered_dense::hash{}(familyBits)); + hashCombine(seed, ankerl::unordered_dense::hash{}(key.minPort)); + hashCombine(seed, ankerl::unordered_dense::hash{}(key.maxPort)); + + break; + } + } + + return seed; + } } // namespace RTC diff --git a/worker/src/RTC/TcpServer.cpp b/worker/src/RTC/TcpServer.cpp index f123faa77b..eb88151547 100644 --- a/worker/src/RTC/TcpServer.cpp +++ b/worker/src/RTC/TcpServer.cpp @@ -36,16 +36,16 @@ namespace RTC uint16_t minPort, uint16_t maxPort, RTC::Transport::SocketFlags& flags, - uint64_t& portRangeHash) + RTC::PortManager::PortRangeKey& portRangeKey) : // This may throw. ::TcpServerHandle::TcpServerHandle( - RTC::PortManager::BindTcp(ip, minPort, maxPort, flags, portRangeHash)), + RTC::PortManager::BindTcp(ip, minPort, maxPort, flags, portRangeKey)), listener(listener), connListener(connListener) { MS_TRACE(); - this->portRangeHash = portRangeHash; + this->portRangeKey = portRangeKey; } TcpServer::~TcpServer() @@ -54,7 +54,7 @@ namespace RTC if (!this->fixedPort) { - RTC::PortManager::Unbind(this->portRangeHash, this->localPort); + RTC::PortManager::Unbind(this->portRangeKey, this->localPort); } } diff --git a/worker/src/RTC/UdpSocket.cpp b/worker/src/RTC/UdpSocket.cpp index fa4b14f2d7..748cc38805 100644 --- a/worker/src/RTC/UdpSocket.cpp +++ b/worker/src/RTC/UdpSocket.cpp @@ -26,15 +26,15 @@ namespace RTC uint16_t minPort, uint16_t maxPort, RTC::Transport::SocketFlags& flags, - uint64_t& portRangeHash) + RTC::PortManager::PortRangeKey& portRangeKey) : // This may throw. ::UdpSocketHandle::UdpSocketHandle( - RTC::PortManager::BindUdp(ip, minPort, maxPort, flags, portRangeHash)), + RTC::PortManager::BindUdp(ip, minPort, maxPort, flags, portRangeKey)), listener(listener) { MS_TRACE(); - this->portRangeHash = portRangeHash; + this->portRangeKey = portRangeKey; } UdpSocket::~UdpSocket() @@ -43,7 +43,7 @@ namespace RTC if (!this->fixedPort) { - RTC::PortManager::Unbind(this->portRangeHash, this->localPort); + RTC::PortManager::Unbind(this->portRangeKey, this->localPort); } } diff --git a/worker/src/RTC/WebRtcServer.cpp b/worker/src/RTC/WebRtcServer.cpp index 38cbf3ebdb..27f192acc5 100644 --- a/worker/src/RTC/WebRtcServer.cpp +++ b/worker/src/RTC/WebRtcServer.cpp @@ -96,7 +96,7 @@ namespace RTC if (listenInfo->portRange()->min() != 0 && listenInfo->portRange()->max() != 0) { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey{}; udpSocket = new RTC::UdpSocket( this, @@ -104,7 +104,7 @@ namespace RTC listenInfo->portRange()->min(), listenInfo->portRange()->max(), flags, - portRangeHash); + portRangeKey); } else if (listenInfo->port() != 0) { @@ -115,7 +115,7 @@ namespace RTC // required. else { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey{}; udpSocket = new RTC::UdpSocket( this, @@ -123,7 +123,7 @@ namespace RTC Settings::configuration.rtcMinPort, Settings::configuration.rtcMaxPort, flags, - portRangeHash); + portRangeKey); } this->udpSocketOrTcpServers.emplace_back( @@ -152,7 +152,7 @@ namespace RTC if (listenInfo->portRange()->min() != 0 && listenInfo->portRange()->max() != 0) { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey{}; tcpServer = new RTC::TcpServer( this, @@ -161,7 +161,7 @@ namespace RTC listenInfo->portRange()->min(), listenInfo->portRange()->max(), flags, - portRangeHash); + portRangeKey); } else if (listenInfo->port() != 0) { @@ -172,7 +172,7 @@ namespace RTC // required. else { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey{}; tcpServer = new RTC::TcpServer( this, @@ -181,7 +181,7 @@ namespace RTC Settings::configuration.rtcMinPort, Settings::configuration.rtcMaxPort, flags, - portRangeHash); + portRangeKey); } this->udpSocketOrTcpServers.emplace_back( diff --git a/worker/src/RTC/WebRtcTransport.cpp b/worker/src/RTC/WebRtcTransport.cpp index 0384415578..1e3a55c997 100644 --- a/worker/src/RTC/WebRtcTransport.cpp +++ b/worker/src/RTC/WebRtcTransport.cpp @@ -5,6 +5,7 @@ #include "FBS/webRtcTransport.h" #include "Logger.hpp" #include "MediaSoupErrors.hpp" +#include "RTC/PortManager.hpp" #include "Settings.hpp" #include "Utils.hpp" #include // std::pow() @@ -81,7 +82,7 @@ namespace RTC if (listenInfo->portRange()->min() != 0 && listenInfo->portRange()->max() != 0) { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey; udpSocket = new RTC::UdpSocket( this, @@ -89,7 +90,7 @@ namespace RTC listenInfo->portRange()->min(), listenInfo->portRange()->max(), flags, - portRangeHash); + portRangeKey); } else if (listenInfo->port() != 0) { @@ -100,7 +101,7 @@ namespace RTC // required. else { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey; udpSocket = new RTC::UdpSocket( this, @@ -108,7 +109,7 @@ namespace RTC Settings::configuration.rtcMinPort, Settings::configuration.rtcMaxPort, flags, - portRangeHash); + portRangeKey); } this->udpSockets[udpSocket] = announcedAddress; @@ -151,7 +152,7 @@ namespace RTC if (listenInfo->portRange()->min() != 0 && listenInfo->portRange()->max() != 0) { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey{}; tcpServer = new RTC::TcpServer( this, @@ -160,7 +161,7 @@ namespace RTC listenInfo->portRange()->min(), listenInfo->portRange()->max(), flags, - portRangeHash); + portRangeKey); } else if (listenInfo->port() != 0) { @@ -171,7 +172,7 @@ namespace RTC // required. else { - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey{}; tcpServer = new RTC::TcpServer( this, @@ -180,7 +181,7 @@ namespace RTC Settings::configuration.rtcMinPort, Settings::configuration.rtcMaxPort, flags, - portRangeHash); + portRangeKey); } this->tcpServers[tcpServer] = announcedAddress; diff --git a/worker/test/src/RTC/TestNackGenerator.cpp b/worker/test/src/RTC/TestNackGenerator.cpp index 4fad4fcb78..271ed8953e 100644 --- a/worker/test/src/RTC/TestNackGenerator.cpp +++ b/worker/test/src/RTC/TestNackGenerator.cpp @@ -7,7 +7,7 @@ #include #include -SCENARIO("NACK generator", "[rtp][rtcp][nack]") +SCENARIO("NackGenerator generator", "[rtp][rtcp][nack]") { constexpr unsigned int SendNackDelay{ 0u }; // In ms. diff --git a/worker/test/src/RTC/TestPortManager.cpp b/worker/test/src/RTC/TestPortManager.cpp new file mode 100644 index 0000000000..c402779a2e --- /dev/null +++ b/worker/test/src/RTC/TestPortManager.cpp @@ -0,0 +1,126 @@ +#include "common.hpp" +#include "RTC/PortManager.hpp" +#include +#include + +// Pre-fix issue #1805: The legacy `GeneratePortRangeHash()` mangled the IPv4 +// address with `(address >> 2) << 2`, so any two IPv4 addresses in the same +// /30 block produced the same `uint64_t` hash. The downstream +// `mapPortRanges.find(hash)` then treated them as the same PortRange and +// merged unrelated bindings. This scenario locks down the post-fix +// behavior: distinct tuples produce distinct keys, equal tuples produce equal +// keys. +SCENARIO("PortManager", "[rtc][portmanager]") +{ + // Helper: build an IPv4 `sockaddr_storage` from a dotted-quad string + port=0. + auto makeV4 = [](const char* dottedQuad) + { + sockaddr_storage ss{}; + auto* in = reinterpret_cast(std::addressof(ss)); + + in->sin_family = AF_INET; + in->sin_port = 0; + + REQUIRE(inet_pton(AF_INET, dottedQuad, &in->sin_addr) == 1); + + return ss; + }; + + // Helper: build an IPv6 `sockaddr_storage` from a textual address + port=0. + auto makeV6 = [](const char* literal) + { + sockaddr_storage ss{}; + auto* in6 = reinterpret_cast(std::addressof(ss)); + + in6->sin6_family = AF_INET6; + in6->sin6_port = 0; + + REQUIRE(inet_pton(AF_INET6, literal, &in6->sin6_addr) == 1); + + return ss; + }; + + SECTION("identical tuples compare equal and hash equal") + { + const auto addr = makeV4("192.168.1.10"); + + const RTC::PortManager::PortRangeKey ka(RTC::PortManager::Protocol::UDP, addr, 40000u, 40099u); + const RTC::PortManager::PortRangeKey kb(RTC::PortManager::Protocol::UDP, addr, 40000u, 40099u); + + REQUIRE(ka == kb); + REQUIRE(RTC::PortManager::PortRangeKeyHash{}(ka) == RTC::PortManager::PortRangeKeyHash{}(kb)); + } + + SECTION("IPv4 addresses in the same /30 are NOT collapsed") + { + // 192.168.1.0 / .1 / .2 / .3 all live in 192.168.1.0/30. The old + // GeneratePortRangeHash dropped the bottom two bits of the address, + // merging all four into one bucket. + const auto a0 = makeV4("192.168.1.0"); + const auto a1 = makeV4("192.168.1.1"); + const auto a2 = makeV4("192.168.1.2"); + const auto a3 = makeV4("192.168.1.3"); + + const RTC::PortManager::PortRangeKey k0(RTC::PortManager::Protocol::UDP, a0, 40000u, 40099u); + const RTC::PortManager::PortRangeKey k1(RTC::PortManager::Protocol::UDP, a1, 40000u, 40099u); + const RTC::PortManager::PortRangeKey k2(RTC::PortManager::Protocol::UDP, a2, 40000u, 40099u); + const RTC::PortManager::PortRangeKey k3(RTC::PortManager::Protocol::UDP, a3, 40000u, 40099u); + + REQUIRE(k0 != k1); + REQUIRE(k0 != k2); + REQUIRE(k0 != k3); + REQUIRE(k1 != k2); + REQUIRE(k1 != k3); + REQUIRE(k2 != k3); + } + + // The old IPv6 hash folded `a[0] ^ a[1] ^ a[2] ^ a[3]` (4 x uint32_t) into + // 32 bits. Any two IPv6 addresses where the four 32-bit words XOR to the + // same value collided. Easiest collision constructor: swap two words. + // ::1 = 0000:0000:0000:0000:0000:0000:0000:0001 XOR-folds the same as + // ::1:0:0 (just word reorder). + SECTION("IPv6 addresses that XOR-fold to the same value are NOT collapsed") + { + const auto a = makeV6("::1"); + const auto b = makeV6("1::1:0:0:0"); + + const RTC::PortManager::PortRangeKey ka(RTC::PortManager::Protocol::UDP, a, 40000u, 40099u); + const RTC::PortManager::PortRangeKey kb(RTC::PortManager::Protocol::UDP, b, 40000u, 40099u); + + REQUIRE(ka != kb); + } + + SECTION("protocol differentiates the key (UDP/TCP on same address+range)") + { + const auto addr = makeV4("10.0.0.1"); + + const RTC::PortManager::PortRangeKey udp(RTC::PortManager::Protocol::UDP, addr, 40000u, 40099u); + const RTC::PortManager::PortRangeKey tcp(RTC::PortManager::Protocol::TCP, addr, 40000u, 40099u); + + REQUIRE(udp != tcp); + } + + SECTION("port-range bounds differentiate the key") + { + const auto addr = makeV4("10.0.0.1"); + + const RTC::PortManager::PortRangeKey ka(RTC::PortManager::Protocol::UDP, addr, 40000u, 40099u); + const RTC::PortManager::PortRangeKey kb(RTC::PortManager::Protocol::UDP, addr, 40000u, 40100u); + const RTC::PortManager::PortRangeKey kc(RTC::PortManager::Protocol::UDP, addr, 40001u, 40099u); + + REQUIRE(ka != kb); + REQUIRE(ka != kc); + REQUIRE(kb != kc); + } + + SECTION("family differentiates the key") + { + const auto v4 = makeV4("0.0.0.0"); + const auto v6 = makeV6("::"); + + const RTC::PortManager::PortRangeKey k4(RTC::PortManager::Protocol::UDP, v4, 40000u, 40099u); + const RTC::PortManager::PortRangeKey k6(RTC::PortManager::Protocol::UDP, v6, 40000u, 40099u); + + REQUIRE(k4 != k6); + } +} diff --git a/worker/test/src/RTC/TestTransportTuple.cpp b/worker/test/src/RTC/TestTransportTuple.cpp index ea67b4262c..bf2f6a7170 100644 --- a/worker/test/src/RTC/TestTransportTuple.cpp +++ b/worker/test/src/RTC/TestTransportTuple.cpp @@ -1,4 +1,5 @@ #include "common.hpp" +#include "RTC/PortManager.hpp" #include "RTC/Transport.hpp" #include "RTC/TransportTuple.hpp" #include "RTC/UdpSocket.hpp" @@ -24,9 +25,9 @@ SCENARIO("TransportTuple", "[transport-tuple]") { UdpSocketListener listener; auto flags = RTC::Transport::SocketFlags{ .ipv6Only = false, .udpReusePort = false }; - uint64_t portRangeHash{ 0u }; + RTC::PortManager::PortRangeKey portRangeKey{}; auto* udpSocket = new RTC::UdpSocket( - std::addressof(listener), const_cast(ip), minPort, maxPort, flags, portRangeHash); + std::addressof(listener), const_cast(ip), minPort, maxPort, flags, portRangeKey); return std::unique_ptr(udpSocket); };