From 7017cf84376eea677b6ba352e7c9ded4eabdce19 Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Wed, 12 Jun 2019 15:24:43 -0700 Subject: [PATCH 1/4] reinitialize winsock on silent domain checkin --- libraries/networking/src/udt/Socket.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 9871935853..7fd697a674 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -76,6 +76,10 @@ void Socket::rebind() { void Socket::rebind(quint16 localPort) { _udpSocket.close(); +#ifdef WIN32 + WSAData ws; + WSAStartup(MAKEWORD(2, 2), &ws); +#endif bind(QHostAddress::AnyIPv4, localPort); } From 3d2b1dbf2bf2a3e310f22a132a86011d5f4cf1ce Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Thu, 13 Jun 2019 17:25:35 -0700 Subject: [PATCH 2/4] When missing domain checkins, drop packets not destined for DS --- libraries/networking/src/DomainHandler.cpp | 9 +++++++-- libraries/networking/src/LimitedNodeList.cpp | 7 +++++++ libraries/networking/src/LimitedNodeList.h | 4 ++++ libraries/networking/src/NodeList.cpp | 1 + 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/DomainHandler.cpp b/libraries/networking/src/DomainHandler.cpp index b442463ac8..5f8aceca35 100644 --- a/libraries/networking/src/DomainHandler.cpp +++ b/libraries/networking/src/DomainHandler.cpp @@ -566,9 +566,14 @@ bool DomainHandler::checkInPacketTimeout() { qCDebug(networking_ice) << "Silent domain checkins:" << _checkInPacketsSinceLastReply; } - if (_checkInPacketsSinceLastReply > MAX_SILENT_DOMAIN_SERVER_CHECK_INS) { + auto nodeList = DependencyManager::get(); - auto nodeList = DependencyManager::get(); + if(_checkInPacketsSinceLastReply > 2) { + qCDebug(networking_ice) << _checkInPacketsSinceLastReply << "seconds since last domain list request, squelching traffic"; + nodeList->setDropOutgoingNodeTraffic(true); + } + + if (_checkInPacketsSinceLastReply > MAX_SILENT_DOMAIN_SERVER_CHECK_INS) { // we haven't heard back from DS in MAX_SILENT_DOMAIN_SERVER_CHECK_INS // so emit our signal that says that diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 9f4eb39013..6faaca219c 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -409,6 +409,13 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiS Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket", "Trying to send a reliable packet unreliably."); + if(_dropOutgoingNodeTraffic) { + auto destinationNode = findNodeWithAddr(sockAddr); + if (!destinationNode.isNull() && (destinationNode->getType() != NodeType::DomainServer)) { + return ERROR_SENDING_PACKET_BYTES; + } + } + fillPacketHeader(packet, hmacAuth); return _nodeSocket.writePacket(packet, sockAddr); diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index bd5e6bd013..f9f6bf3b3e 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -335,6 +335,8 @@ public: float getInboundKbps() const { return _inboundKbps; } float getOutboundKbps() const { return _outboundKbps; } + void setDropOutgoingNodeTraffic(bool squelchOutgoingNodeTraffic) { _dropOutgoingNodeTraffic = squelchOutgoingNodeTraffic; } + const std::set SOLO_NODE_TYPES = { NodeType::AvatarMixer, NodeType::AudioMixer, @@ -493,6 +495,8 @@ private: int _outboundPPS { 0 }; float _inboundKbps { 0.0f }; float _outboundKbps { 0.0f }; + + bool _dropOutgoingNodeTraffic { false }; }; #endif // hifi_LimitedNodeList_h diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 86b33bbe20..39b8fa29a9 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -710,6 +710,7 @@ void NodeList::processDomainServerList(QSharedPointer message) // this is a packet from the domain server, reset the count of un-replied check-ins _domainHandler.clearPendingCheckins(); + setDropOutgoingNodeTraffic(false); // emit our signal so listeners know we just heard from the DS emit receivedDomainServerList(); From 5b6911efa46452b98de667cf18612729767df23d Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Fri, 14 Jun 2019 10:45:18 -0700 Subject: [PATCH 3/4] log username with "haven't heard from" messages remove rebind on silent domain checkin --- domain-server/src/DomainServer.cpp | 7 ++++++- libraries/networking/src/NodeList.cpp | 5 ----- libraries/networking/src/udt/Socket.cpp | 4 ---- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 44887599d3..5dd98c2ae7 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -1735,7 +1735,12 @@ void DomainServer::nodePingMonitor() { nodeList->eachNode([now](const SharedNodePointer& node) { quint64 lastHeard = now - node->getLastHeardMicrostamp(); if (lastHeard > 2 * USECS_PER_SECOND) { - qCDebug(domain_server) << "Haven't heard from " << node->getPublicSocket() << " in " << lastHeard / USECS_PER_MSEC << " msec"; + QString username; + DomainServerNodeData* nodeData = static_cast(node->getLinkedData()); + if(nodeData) { + username = nodeData->getUsername(); + } + qCDebug(domain_server) << "Haven't heard from " << node->getPublicSocket() << username << " in " << lastHeard / USECS_PER_MSEC << " msec"; } }); } diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 39b8fa29a9..b43f9f88e1 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -450,13 +450,8 @@ void NodeList::sendDomainServerCheckIn() { // Send duplicate check-ins in the exponentially increasing sequence 1, 1, 2, 4, ... static const int MAX_CHECKINS_TOGETHER = 20; - static const int REBIND_CHECKIN_COUNT = 2; int outstandingCheckins = _domainHandler.getCheckInPacketsSinceLastReply(); - if (outstandingCheckins > REBIND_CHECKIN_COUNT) { - _nodeSocket.rebind(); - } - int checkinCount = outstandingCheckins > 1 ? std::pow(2, outstandingCheckins - 2) : 1; checkinCount = std::min(checkinCount, MAX_CHECKINS_TOGETHER); for (int i = 1; i < checkinCount; ++i) { diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index a3e9a43503..fc6d2cbe2a 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -76,10 +76,6 @@ void Socket::rebind() { void Socket::rebind(quint16 localPort) { _udpSocket.close(); -#ifdef WIN32 - WSAData ws; - WSAStartup(MAKEWORD(2, 2), &ws); -#endif bind(QHostAddress::AnyIPv4, localPort); } From 1b31b8cff8066a1e102c18e1b9b6d669b44f7093 Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Fri, 14 Jun 2019 13:23:33 -0700 Subject: [PATCH 4/4] CR fixes --- domain-server/src/DomainServer.cpp | 2 +- libraries/networking/src/DomainHandler.cpp | 4 +++- libraries/networking/src/LimitedNodeList.cpp | 7 +++++-- libraries/networking/src/NodeList.cpp | 11 ----------- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 5dd98c2ae7..358b05222c 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -1737,7 +1737,7 @@ void DomainServer::nodePingMonitor() { if (lastHeard > 2 * USECS_PER_SECOND) { QString username; DomainServerNodeData* nodeData = static_cast(node->getLinkedData()); - if(nodeData) { + if (nodeData) { username = nodeData->getUsername(); } qCDebug(domain_server) << "Haven't heard from " << node->getPublicSocket() << username << " in " << lastHeard / USECS_PER_MSEC << " msec"; diff --git a/libraries/networking/src/DomainHandler.cpp b/libraries/networking/src/DomainHandler.cpp index 5f8aceca35..3512b02d11 100644 --- a/libraries/networking/src/DomainHandler.cpp +++ b/libraries/networking/src/DomainHandler.cpp @@ -559,6 +559,8 @@ void DomainHandler::processDomainServerConnectionDeniedPacket(QSharedPointer(); - if(_checkInPacketsSinceLastReply > 2) { + if (_checkInPacketsSinceLastReply > SILENT_DOMAIN_TRAFFIC_DROP_MIN) { qCDebug(networking_ice) << _checkInPacketsSinceLastReply << "seconds since last domain list request, squelching traffic"; nodeList->setDropOutgoingNodeTraffic(true); } diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 6faaca219c..8fefe5820c 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -409,9 +409,12 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiS Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket", "Trying to send a reliable packet unreliably."); - if(_dropOutgoingNodeTraffic) { + if (_dropOutgoingNodeTraffic) { auto destinationNode = findNodeWithAddr(sockAddr); - if (!destinationNode.isNull() && (destinationNode->getType() != NodeType::DomainServer)) { + + // findNodeWithAddr returns null for the address of the domain server + if (!destinationNode.isNull()) { + // This only suppresses individual unreliable packets, not unreliable packet lists return ERROR_SENDING_PACKET_BYTES; } } diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 7dd9ee21b7..3d367bc761 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -453,17 +453,6 @@ void NodeList::sendDomainServerCheckIn() { static const int MAX_CHECKINS_TOGETHER = 20; int outstandingCheckins = _domainHandler.getCheckInPacketsSinceLastReply(); - /* - static const int WARNING_CHECKIN_COUNT = 2; - if (outstandingCheckins > WARNING_CHECKIN_COUNT) { - // We may be headed for a disconnect, as we've written two DomainListRequests without getting anything back. - // In some cases, we've found that nothing is going out on the wire despite not getting any errors from - // sendPacket => writeDatagram, below. In at least some such cases, we've found that the DomainDisconnectRequest - // does go through, so let's at least try to mix it up with a different safe packet. - // TODO: send ICEPing, and later on tell the other nodes to shut up for a moment. - - }*/ - int checkinCount = outstandingCheckins > 1 ? std::pow(2, outstandingCheckins - 2) : 1; checkinCount = std::min(checkinCount, MAX_CHECKINS_TOGETHER); for (int i = 1; i < checkinCount; ++i) {