From 9556fecbe2de298969769b005623cd19a513434c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Jul 2015 17:10:22 -0700 Subject: [PATCH 01/21] initial changes to make _nodeSocket a udt::Socket --- assignment-client/src/audio/AudioMixer.cpp | 3 - domain-server/src/DomainServer.cpp | 2 +- libraries/networking/src/LimitedNodeList.cpp | 55 +--- libraries/networking/src/LimitedNodeList.h | 12 +- libraries/networking/src/NLPacket.cpp | 49 +++- libraries/networking/src/NLPacket.h | 11 +- libraries/networking/src/PacketReceiver.cpp | 265 +++++++++---------- libraries/networking/src/PacketReceiver.h | 5 +- libraries/networking/src/udt/Packet.cpp | 18 +- libraries/networking/src/udt/Socket.cpp | 68 +++++ libraries/networking/src/udt/Socket.h | 55 ++++ 11 files changed, 333 insertions(+), 210 deletions(-) create mode 100644 libraries/networking/src/udt/Socket.cpp create mode 100644 libraries/networking/src/udt/Socket.h diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index b68332210b..aa8e405d34 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -652,9 +652,6 @@ void AudioMixer::run() { auto nodeList = DependencyManager::get(); - // we do not want this event loop to be the handler for UDP datagrams, so disconnect - disconnect(&nodeList->getNodeSocket(), 0, this, 0); - nodeList->addNodeTypeToInterestSet(NodeType::Agent); nodeList->linkedDataCreateCallback = [](Node* node) { diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index c13de0449e..4532180ca7 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -259,7 +259,7 @@ void DomainServer::setupNodeListAndAssignments(const QUuid& sessionUUID) { auto nodeList = DependencyManager::set(domainServerPort, domainServerDTLSPort); // no matter the local port, save it to shared mem so that local assignment clients can ask what it is - nodeList->putLocalPortIntoSharedMemory(DOMAIN_SERVER_LOCAL_PORT_SMEM_KEY, this, nodeList->getNodeSocket().localPort()); + nodeList->putLocalPortIntoSharedMemory(DOMAIN_SERVER_LOCAL_PORT_SMEM_KEY, this, nodeList->getSocketLocalPort()); // store our local http ports in shared memory quint16 localHttpPort = DOMAIN_SERVER_HTTP_PORT; diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index fdb3461c1f..c36931fa9e 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -82,7 +82,7 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short } const int LARGER_BUFFER_SIZE = 1048576; - changeSocketBufferSizes(LARGER_BUFFER_SIZE); + _nodeSocket.setBufferSizes(LARGER_BUFFER_SIZE); // check for local socket updates every so often const int LOCAL_SOCKET_UPDATE_INTERVAL_MSECS = 5 * 1000; @@ -96,10 +96,10 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short // check the local socket right now updateLocalSockAddr(); - - // TODO: Create a new thread, and move PacketReceiver to it - - connect(&_nodeSocket, &QUdpSocket::readyRead, _packetReceiver, &PacketReceiver::processDatagrams); + + // set &PacketReceiver::handleVerifiedPacket as the verified packet function for the udt::Socket + using std::placeholders::_1; + _nodeSocket.setVerifiedPacketFunction(std::bind(&PacketReceiver::handleVerifiedPacket, _packetReceiver, _1)); _packetStatTimer.start(); @@ -149,32 +149,6 @@ QUdpSocket& LimitedNodeList::getDTLSSocket() { return *_dtlsSocket; } -void LimitedNodeList::changeSocketBufferSizes(int numBytes) { - for (int i = 0; i < 2; i++) { - QAbstractSocket::SocketOption bufferOpt; - QString bufferTypeString; - if (i == 0) { - bufferOpt = QAbstractSocket::SendBufferSizeSocketOption; - bufferTypeString = "send"; - - } else { - bufferOpt = QAbstractSocket::ReceiveBufferSizeSocketOption; - bufferTypeString = "receive"; - } - int oldBufferSize = _nodeSocket.socketOption(bufferOpt).toInt(); - if (oldBufferSize < numBytes) { - int newBufferSize = _nodeSocket.socketOption(bufferOpt).toInt(); - - qCDebug(networking) << "Changed socket" << bufferTypeString << "buffer size from" << oldBufferSize << "to" - << newBufferSize << "bytes"; - } else { - // don't make the buffer smaller - qCDebug(networking) << "Did not change socket" << bufferTypeString << "buffer size from" << oldBufferSize - << "since it is larger than desired size of" << numBytes; - } - } -} - bool LimitedNodeList::packetSourceAndHashMatch(const NLPacket& packet, SharedNodePointer& matchingNode) { if (NON_SOURCED_PACKETS.contains(packet.getType())) { @@ -255,12 +229,7 @@ qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, const HifiSock ++_numCollectedPackets; _numCollectedBytes += datagram.size(); - qint64 bytesWritten = _nodeSocket.writeDatagram(datagram, - destinationSockAddr.getAddress(), destinationSockAddr.getPort()); - - if (bytesWritten < 0) { - qCDebug(networking) << "ERROR in writeDatagram:" << _nodeSocket.error() << "-" << _nodeSocket.errorString(); - } + qint64 bytesWritten = _nodeSocket.writeDatagram(datagram, destinationSockAddr); return bytesWritten; } @@ -571,7 +540,7 @@ void LimitedNodeList::sendSTUNRequest() { ++_numInitialSTUNRequests; } - unsigned char stunRequestPacket[NUM_BYTES_STUN_HEADER]; + char stunRequestPacket[NUM_BYTES_STUN_HEADER]; int packetIndex = 0; @@ -597,15 +566,7 @@ void LimitedNodeList::sendSTUNRequest() { flagTimeForConnectionStep(ConnectionStep::SendSTUNRequest); - _nodeSocket.writeDatagram((char*) stunRequestPacket, sizeof(stunRequestPacket), - _stunSockAddr.getAddress(), _stunSockAddr.getPort()); -} - -void LimitedNodeList::rebindNodeSocket() { - quint16 oldPort = _nodeSocket.localPort(); - - _nodeSocket.close(); - _nodeSocket.bind(QHostAddress::AnyIPv4, oldPort); + _nodeSocket.writeDatagram(stunRequestPacket, _stunSockAddr); } bool LimitedNodeList::processSTUNResponse(QSharedPointer packet) { diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 33d490c960..8a4230be04 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -37,9 +37,10 @@ #include "DomainHandler.h" #include "Node.h" #include "NLPacket.h" -#include "udt/PacketHeaders.h" #include "PacketReceiver.h" #include "NLPacketList.h" +#include "udt/PacketHeaders.h" +#include "udt/Socket.h" #include "UUIDHasher.h" const quint64 NODE_SILENCE_THRESHOLD_MSECS = 2 * 1000; @@ -109,9 +110,8 @@ public: bool getThisNodeCanRez() const { return _thisNodeCanRez; } void setThisNodeCanRez(bool canRez); - - void rebindNodeSocket(); - QUdpSocket& getNodeSocket() { return _nodeSocket; } + + quint16 getSocketLocalPort() const { return _nodeSocket.localPort(); } QUdpSocket& getDTLSSocket(); bool packetSourceAndHashMatch(const NLPacket& packet, SharedNodePointer& matchingNode); @@ -256,8 +256,6 @@ protected: PacketSequenceNumber getNextSequenceNumberForPacket(const QUuid& nodeUUID, PacketType::Value packetType); - void changeSocketBufferSizes(int numBytes); - void handleNodeKill(const SharedNodePointer& node); void stopInitialSTUNUpdate(bool success); @@ -272,7 +270,7 @@ protected: QUuid _sessionUUID; NodeHash _nodeHash; QReadWriteLock _nodeMutex; - QUdpSocket _nodeSocket; + udt::Socket _nodeSocket; QUdpSocket* _dtlsSocket; HifiSockAddr _localSockAddr; HifiSockAddr _publicSockAddr; diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 7a6503dbc3..7ccd33a1ed 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -63,6 +63,14 @@ std::unique_ptr NLPacket::fromReceivedPacket(std::unique_ptr dat } +std::unique_ptr NLPacket::fromBase(std::unique_ptr packet) { + // Fail with null packet + Q_ASSERT(packet); + + // call our constructor to create an NLPacket from this Packet + return std::unique_ptr(new NLPacket(std::move(packet))); +} + std::unique_ptr NLPacket::createCopy(const NLPacket& other) { return std::unique_ptr(new NLPacket(other)); } @@ -81,24 +89,59 @@ NLPacket::NLPacket(PacketType::Value type) : adjustPayloadStartAndCapacity(); } -NLPacket::NLPacket(const NLPacket& other) : Packet(other) { +NLPacket::NLPacket(std::unique_ptr packet) : + Packet(*packet) +{ + adjustPayloadStartAndCapacity(_payloadSize > 0); + readSourceID(); + readVerificationHash(); +} + +NLPacket::NLPacket(const NLPacket& other) : Packet(other) { + *this = other; +} + +NLPacket& NLPacket::operator=(const NLPacket& other) { + Packet::operator=(other); + + _sourceID = other._sourceID; + _verificationHash = other._verificationHash; + + return *this; } NLPacket::NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr) : Packet(std::move(data), size, senderSockAddr) { adjustPayloadStartAndCapacity(); - _payloadSize = _payloadCapacity; readSourceID(); readVerificationHash(); } -void NLPacket::adjustPayloadStartAndCapacity() { +NLPacket::NLPacket(NLPacket&& other) : + Packet(other) +{ + *this = std::move(other); +} + +NLPacket& NLPacket::operator=(NLPacket&& other) { + _sourceID = std::move(other._sourceID); + _verificationHash = std::move(other._verificationHash); + + return *this; +} + + +void NLPacket::adjustPayloadStartAndCapacity(bool shouldDecreasePayloadSize) { qint64 headerSize = localHeaderSize(_type); _payloadStart += headerSize; _payloadCapacity -= headerSize; + + if (shouldDecreasePayloadSize) { + _payloadSize -= headerSize; + } } void NLPacket::readSourceID() { diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index 669278ed65..eab0fa3a99 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -22,6 +22,8 @@ public: static std::unique_ptr create(PacketType::Value type, qint64 size = -1); static std::unique_ptr fromReceivedPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr); + static std::unique_ptr fromBase(std::unique_ptr packet); + // Provided for convenience, try to limit use static std::unique_ptr createCopy(const NLPacket& other); @@ -41,12 +43,19 @@ public: protected: - void adjustPayloadStartAndCapacity(); + void adjustPayloadStartAndCapacity(bool shouldDecreasePayloadSize = false); NLPacket(PacketType::Value type); NLPacket(PacketType::Value type, qint64 size); + NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr); + NLPacket(std::unique_ptr packet); + NLPacket(const NLPacket& other); + NLPacket& operator=(const NLPacket& other); + + NLPacket(NLPacket&& other); + NLPacket& operator=(NLPacket&& other); void readSourceID(); void readVerificationHash(); diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index f59be35dc5..a97b212355 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -234,147 +234,134 @@ bool PacketReceiver::packetVersionMatch(const NLPacket& packet) { } } -void PacketReceiver::processDatagrams() { - //PerformanceWarning warn(Menu::getInstance()->isOptionChecked(MenuOption::PipelineWarnings), - //"PacketReceiver::processDatagrams()"); - +void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { + + // if we're supposed to drop this packet then break out here + if (_shouldDropPackets) { + return; + } + auto nodeList = DependencyManager::get(); - - while (nodeList && nodeList->getNodeSocket().hasPendingDatagrams()) { - // setup a buffer to read the packet into - int packetSizeWithHeader = nodeList->getNodeSocket().pendingDatagramSize(); - std::unique_ptr buffer = std::unique_ptr(new char[packetSizeWithHeader]); - - // if we're supposed to drop this packet then break out here - if (_shouldDropPackets) { - break; - } - - // setup a HifiSockAddr to read into - HifiSockAddr senderSockAddr; - - // pull the datagram - nodeList->getNodeSocket().readDatagram(buffer.get(), packetSizeWithHeader, - senderSockAddr.getAddressPointer(), senderSockAddr.getPortPointer()); - - // setup an NLPacket from the data we just read - auto packet = NLPacket::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, senderSockAddr); + + // setup a HifiSockAddr to read into + HifiSockAddr senderSockAddr; + + // setup an NLPacket from the data we just read + auto nlPacket = NLPacket::fromBase(std::move(packet)); + + _inPacketCount++; + _inByteCount += nlPacket->getDataSize(); + + SharedNodePointer matchingNode; + if (!nlPacket->getSourceID().isNull()) { + matchingNode = nodeList->nodeWithUUID(nlPacket->getSourceID()); - _inPacketCount++; - _inByteCount += packetSizeWithHeader; - - if (packetVersionMatch(*packet)) { - - SharedNodePointer matchingNode; - if (nodeList->packetSourceAndHashMatch(*packet, matchingNode)) { - - if (matchingNode) { - // No matter if this packet is handled or not, we update the timestamp for the last time we heard - // from this sending node - matchingNode->setLastHeardMicrostamp(usecTimestampNow()); - } - - _packetListenerLock.lock(); - - bool listenerIsDead = false; - - auto it = _packetListenerMap.find(packet->getType()); - - if (it != _packetListenerMap.end()) { - - auto listener = it.value(); - - if (listener.first) { - - bool success = false; - - // check if this is a directly connected listener - _directConnectSetMutex.lock(); - - Qt::ConnectionType connectionType = - _directlyConnectedObjects.contains(listener.first) ? Qt::DirectConnection : Qt::AutoConnection; - - _directConnectSetMutex.unlock(); - - PacketType::Value packetType = packet->getType(); - - if (matchingNode) { - // if this was a sequence numbered packet we should store the last seq number for - // a packet of this type for this node - if (SEQUENCE_NUMBERED_PACKETS.contains(packet->getType())) { - matchingNode->setLastSequenceNumberForPacketType(packet->readSequenceNumber(), packet->getType()); - } - - emit dataReceived(matchingNode->getType(), packet->getDataSize()); - QMetaMethod metaMethod = listener.second; - - static const QByteArray QSHAREDPOINTER_NODE_NORMALIZED = QMetaObject::normalizedType("QSharedPointer"); - static const QByteArray SHARED_NODE_NORMALIZED = QMetaObject::normalizedType("SharedNodePointer"); - - // one final check on the QPointer before we go to invoke - if (listener.first) { - if (metaMethod.parameterTypes().contains(SHARED_NODE_NORMALIZED)) { - success = metaMethod.invoke(listener.first, - connectionType, - Q_ARG(QSharedPointer, - QSharedPointer(packet.release())), - Q_ARG(SharedNodePointer, matchingNode)); - - } else if (metaMethod.parameterTypes().contains(QSHAREDPOINTER_NODE_NORMALIZED)) { - success = metaMethod.invoke(listener.first, - connectionType, - Q_ARG(QSharedPointer, - QSharedPointer(packet.release())), - Q_ARG(QSharedPointer, matchingNode)); - - } else { - success = metaMethod.invoke(listener.first, - connectionType, - Q_ARG(QSharedPointer, - QSharedPointer(packet.release()))); - } - } else { - listenerIsDead = true; - } - - } else { - emit dataReceived(NodeType::Unassigned, packet->getDataSize()); - - success = listener.second.invoke(listener.first, - Q_ARG(QSharedPointer, QSharedPointer(packet.release()))); - } - - if (!success) { - qDebug().nospace() << "Error delivering packet " << packetType - << " (" << qPrintable(nameForPacketType(packetType)) << ") to listener " - << listener.first << "::" << qPrintable(listener.second.methodSignature()); - } - - } else { - listenerIsDead = true; - } - - if (listenerIsDead) { - qDebug().nospace() << "Listener for packet" << packet->getType() - << " (" << qPrintable(nameForPacketType(packet->getType())) << ")" - << " has been destroyed. Removing from listener map."; - it = _packetListenerMap.erase(it); - - // if it exists, remove the listener from _directlyConnectedObjects - _directConnectSetMutex.lock(); - _directlyConnectedObjects.remove(listener.first); - _directConnectSetMutex.unlock(); - } - - } else { - qWarning() << "No listener found for packet type " << nameForPacketType(packet->getType()); - - // insert a dummy listener so we don't print this again - _packetListenerMap.insert(packet->getType(), { nullptr, QMetaMethod() }); - } - - _packetListenerLock.unlock(); - } + if (matchingNode) { + // No matter if this packet is handled or not, we update the timestamp for the last time we heard + // from this sending node + matchingNode->setLastHeardMicrostamp(usecTimestampNow()); } } + + _packetListenerLock.lock(); + + bool listenerIsDead = false; + + auto it = _packetListenerMap.find(packet->getType()); + + if (it != _packetListenerMap.end()) { + + auto listener = it.value(); + + if (listener.first) { + + bool success = false; + + // check if this is a directly connected listener + _directConnectSetMutex.lock(); + + Qt::ConnectionType connectionType = + _directlyConnectedObjects.contains(listener.first) ? Qt::DirectConnection : Qt::AutoConnection; + + _directConnectSetMutex.unlock(); + + PacketType::Value packetType = packet->getType(); + + if (matchingNode) { + // if this was a sequence numbered packet we should store the last seq number for + // a packet of this type for this node + if (SEQUENCE_NUMBERED_PACKETS.contains(packet->getType())) { + matchingNode->setLastSequenceNumberForPacketType(packet->readSequenceNumber(), packet->getType()); + } + + emit dataReceived(matchingNode->getType(), packet->getDataSize()); + QMetaMethod metaMethod = listener.second; + + static const QByteArray QSHAREDPOINTER_NODE_NORMALIZED = QMetaObject::normalizedType("QSharedPointer"); + static const QByteArray SHARED_NODE_NORMALIZED = QMetaObject::normalizedType("SharedNodePointer"); + + // one final check on the QPointer before we go to invoke + if (listener.first) { + if (metaMethod.parameterTypes().contains(SHARED_NODE_NORMALIZED)) { + success = metaMethod.invoke(listener.first, + connectionType, + Q_ARG(QSharedPointer, + QSharedPointer(nlPacket.release())), + Q_ARG(SharedNodePointer, matchingNode)); + + } else if (metaMethod.parameterTypes().contains(QSHAREDPOINTER_NODE_NORMALIZED)) { + success = metaMethod.invoke(listener.first, + connectionType, + Q_ARG(QSharedPointer, + QSharedPointer(nlPacket.release())), + Q_ARG(QSharedPointer, matchingNode)); + + } else { + success = metaMethod.invoke(listener.first, + connectionType, + Q_ARG(QSharedPointer, + QSharedPointer(nlPacket.release()))); + } + } else { + listenerIsDead = true; + } + + } else { + emit dataReceived(NodeType::Unassigned, packet->getDataSize()); + + success = listener.second.invoke(listener.first, + Q_ARG(QSharedPointer, QSharedPointer(nlPacket.release()))); + } + + if (!success) { + qDebug().nospace() << "Error delivering packet " << packetType + << " (" << qPrintable(nameForPacketType(packetType)) << ") to listener " + << listener.first << "::" << qPrintable(listener.second.methodSignature()); + } + + } else { + listenerIsDead = true; + } + + if (listenerIsDead) { + qDebug().nospace() << "Listener for packet" << packet->getType() + << " (" << qPrintable(nameForPacketType(packet->getType())) << ")" + << " has been destroyed. Removing from listener map."; + it = _packetListenerMap.erase(it); + + // if it exists, remove the listener from _directlyConnectedObjects + _directConnectSetMutex.lock(); + _directlyConnectedObjects.remove(listener.first); + _directConnectSetMutex.unlock(); + } + + } else { + qWarning() << "No listener found for packet type " << nameForPacketType(packet->getType()); + + // insert a dummy listener so we don't print this again + _packetListenerMap.insert(packet->getType(), { nullptr, QMetaMethod() }); + } + + _packetListenerLock.unlock(); + } diff --git a/libraries/networking/src/PacketReceiver.h b/libraries/networking/src/PacketReceiver.h index 9fdccdfddf..ac38749a88 100644 --- a/libraries/networking/src/PacketReceiver.h +++ b/libraries/networking/src/PacketReceiver.h @@ -44,9 +44,8 @@ public: bool registerListenerForTypes(const QSet& types, QObject* listener, const char* slot); bool registerListener(PacketType::Value type, QObject* listener, const char* slot); void unregisterListener(QObject* listener); - -public slots: - void processDatagrams(); + + void handleVerifiedPacket(std::unique_ptr packet); signals: void dataReceived(quint8 channelType, int bytes); diff --git a/libraries/networking/src/udt/Packet.cpp b/libraries/networking/src/udt/Packet.cpp index 02a44c4a4f..05b5f3fe50 100644 --- a/libraries/networking/src/udt/Packet.cpp +++ b/libraries/networking/src/udt/Packet.cpp @@ -98,12 +98,6 @@ Packet::Packet(const Packet& other) : QIODevice() { *this = other; - - if (other.isOpen()) { - this->open(other.openMode()); - } - - this->seek(other.pos()); } Packet& Packet::operator=(const Packet& other) { @@ -117,6 +111,12 @@ Packet& Packet::operator=(const Packet& other) { _payloadCapacity = other._payloadCapacity; _payloadSize = other._payloadSize; + + if (other.isOpen() && !isOpen()) { + open(other.openMode()); + } + + seek(other.pos()); return *this; } @@ -135,6 +135,12 @@ Packet& Packet::operator=(Packet&& other) { _payloadCapacity = other._payloadCapacity; _payloadSize = other._payloadSize; + + if (other.isOpen() && !isOpen()) { + open(other.openMode()); + } + + seek(other.pos()); return *this; } diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp new file mode 100644 index 0000000000..2e1e3481f9 --- /dev/null +++ b/libraries/networking/src/udt/Socket.cpp @@ -0,0 +1,68 @@ +// +// Socket.cpp +// libraries/networking/src/udt +// +// Created by Stephen Birarda on 2015-07-20. +// Copyright 2015 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "Socket.h" + +#include "../NetworkLogging.h" + +using namespace udt; + +Socket::Socket(QObject* parent) : + QObject(parent) +{ + +} + +void Socket::rebind() { + quint16 oldPort = _udpSocket.localPort(); + + _udpSocket.close(); + _udpSocket.bind(QHostAddress::AnyIPv4, oldPort); +} + +void Socket::setBufferSizes(int numBytes) { + for (int i = 0; i < 2; i++) { + QAbstractSocket::SocketOption bufferOpt; + QString bufferTypeString; + + if (i == 0) { + bufferOpt = QAbstractSocket::SendBufferSizeSocketOption; + bufferTypeString = "send"; + + } else { + bufferOpt = QAbstractSocket::ReceiveBufferSizeSocketOption; + bufferTypeString = "receive"; + } + + int oldBufferSize = _udpSocket.socketOption(bufferOpt).toInt(); + + if (oldBufferSize < numBytes) { + int newBufferSize = _udpSocket.socketOption(bufferOpt).toInt(); + + qCDebug(networking) << "Changed socket" << bufferTypeString << "buffer size from" << oldBufferSize << "to" + << newBufferSize << "bytes"; + } else { + // don't make the buffer smaller + qCDebug(networking) << "Did not change socket" << bufferTypeString << "buffer size from" << oldBufferSize + << "since it is larger than desired size of" << numBytes; + } + } +} + +qint64 Socket::writeDatagram(const QByteArray& datagram, const HifiSockAddr& sockAddr) { + qint64 bytesWritten = _udpSocket.writeDatagram(datagram, sockAddr.getAddress(), sockAddr.getPort()); + + if (bytesWritten < 0) { + qCDebug(networking) << "ERROR in writeDatagram:" << _udpSocket.error() << "-" << _udpSocket.errorString(); + } + + return bytesWritten; +} diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h new file mode 100644 index 0000000000..23ad75db12 --- /dev/null +++ b/libraries/networking/src/udt/Socket.h @@ -0,0 +1,55 @@ +// +// Socket.h +// libraries/networking/src/udt +// +// Created by Stephen Birarda on 2015-07-20. +// Copyright 2015 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#pragma once + +#ifndef hifi_Socket_h +#define hifi_Socket_h + +#include + +#include +#include + +#include "../HifiSockAddr.h" +#include "Packet.h" + +namespace udt { + +using VerifiedPacketFunction = std::function)>; + +class Socket : public QObject { + Q_OBJECT +public: + Socket(QObject* object = 0); + + quint16 localPort() const { return _udpSocket.localPort(); } + + qint64 writeDatagram(const char* data, qint64 size, const HifiSockAddr& sockAddr) + { return writeDatagram(QByteArray::fromRawData(data, size), sockAddr); } + qint64 writeDatagram(const QByteArray& datagram, const HifiSockAddr& sockAddr); + + void bind(const QHostAddress& address, quint16 port = 0) { _udpSocket.bind(address, port); } + void rebind(); + + void setVerifiedPacketFunction(VerifiedPacketFunction verifiedPacketFunction) + { _verifiedPacketFunction = verifiedPacketFunction; } + + void setBufferSizes(int numBytes); +private: + QUdpSocket _udpSocket { this }; + VerifiedPacketFunction _verifiedPacketFunction; +}; + +} + + +#endif // hifi_Socket_h From 796dfee687b1799035a77ff480146d26f1162911 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Jul 2015 17:28:04 -0700 Subject: [PATCH 02/21] udt namespace fixes and NLPacket operator fixes --- ice-server/src/IceServer.cpp | 6 +++--- ice-server/src/IceServer.h | 2 +- libraries/networking/src/NLPacket.cpp | 11 ++++++++--- libraries/networking/src/NLPacket.h | 2 +- libraries/networking/src/NLPacketList.cpp | 2 +- libraries/networking/src/NLPacketList.h | 4 ++-- libraries/networking/src/PacketReceiver.cpp | 2 +- libraries/networking/src/PacketReceiver.h | 2 +- libraries/networking/src/udt/Packet.cpp | 2 ++ libraries/networking/src/udt/Packet.h | 6 +++++- libraries/networking/src/udt/PacketList.cpp | 2 ++ libraries/networking/src/udt/PacketList.h | 8 +++++++- libraries/networking/src/udt/Socket.cpp | 22 ++++++++++++++++++++- libraries/networking/src/udt/Socket.h | 4 ++++ 14 files changed, 59 insertions(+), 16 deletions(-) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index f56fe9202f..1a08426081 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -55,7 +55,7 @@ void IceServer::processDatagrams() { _serverSocket.readDatagram(buffer.get(), packetSizeWithHeader, sendingSockAddr.getAddressPointer(), sendingSockAddr.getPortPointer()); - auto packet = Packet::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, sendingSockAddr); + auto packet = udt::Packet::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, sendingSockAddr); PacketType::Value packetType = packet->getType(); @@ -100,7 +100,7 @@ void IceServer::processDatagrams() { } } -SharedNetworkPeer IceServer::addOrUpdateHeartbeatingPeer(Packet& packet) { +SharedNetworkPeer IceServer::addOrUpdateHeartbeatingPeer(udt::Packet& packet) { // pull the UUID, public and private sock addrs for this peer QUuid senderUUID; @@ -133,7 +133,7 @@ SharedNetworkPeer IceServer::addOrUpdateHeartbeatingPeer(Packet& packet) { } void IceServer::sendPeerInformationPacket(const NetworkPeer& peer, const HifiSockAddr* destinationSockAddr) { - auto peerPacket = Packet::create(PacketType::ICEServerPeerInformation); + auto peerPacket = udt::Packet::create(PacketType::ICEServerPeerInformation); // get the byte array for this peer peerPacket->write(peer.toByteArray()); diff --git a/ice-server/src/IceServer.h b/ice-server/src/IceServer.h index 7820ae2e22..e91dc4e064 100644 --- a/ice-server/src/IceServer.h +++ b/ice-server/src/IceServer.h @@ -33,7 +33,7 @@ private slots: void clearInactivePeers(); private: - SharedNetworkPeer addOrUpdateHeartbeatingPeer(Packet& incomingPacket); + SharedNetworkPeer addOrUpdateHeartbeatingPeer(udt::Packet& incomingPacket); void sendPeerInformationPacket(const NetworkPeer& peer, const HifiSockAddr* destinationSockAddr); QUuid _id; diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 7ccd33a1ed..5f24810d0b 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -99,12 +99,13 @@ NLPacket::NLPacket(std::unique_ptr packet) : } NLPacket::NLPacket(const NLPacket& other) : Packet(other) { - *this = other; + _sourceID = other._sourceID; + _verificationHash = other._verificationHash; } NLPacket& NLPacket::operator=(const NLPacket& other) { Packet::operator=(other); - + _sourceID = other._sourceID; _verificationHash = other._verificationHash; @@ -123,10 +124,14 @@ NLPacket::NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& NLPacket::NLPacket(NLPacket&& other) : Packet(other) { - *this = std::move(other); + _sourceID = std::move(other._sourceID); + _verificationHash = std::move(other._verificationHash); } NLPacket& NLPacket::operator=(NLPacket&& other) { + + Packet::operator=(other); + _sourceID = std::move(other._sourceID); _verificationHash = std::move(other._verificationHash); diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index eab0fa3a99..aaf6c5df75 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -16,7 +16,7 @@ #include "udt/Packet.h" -class NLPacket : public Packet { +class NLPacket : public udt::Packet { Q_OBJECT public: static std::unique_ptr create(PacketType::Value type, qint64 size = -1); diff --git a/libraries/networking/src/NLPacketList.cpp b/libraries/networking/src/NLPacketList.cpp index 8c794cf5f8..6b7b53e8e9 100644 --- a/libraries/networking/src/NLPacketList.cpp +++ b/libraries/networking/src/NLPacketList.cpp @@ -19,7 +19,7 @@ NLPacketList::NLPacketList(PacketType::Value packetType, QByteArray extendedHead } -std::unique_ptr NLPacketList::createPacket() { +std::unique_ptr NLPacketList::createPacket() { return NLPacket::create(getType()); } diff --git a/libraries/networking/src/NLPacketList.h b/libraries/networking/src/NLPacketList.h index 28fbde9112..33a8316f95 100644 --- a/libraries/networking/src/NLPacketList.h +++ b/libraries/networking/src/NLPacketList.h @@ -14,7 +14,7 @@ #include "udt/PacketList.h" -class NLPacketList : public PacketList { +class NLPacketList : public udt::PacketList { public: NLPacketList(PacketType::Value packetType, QByteArray extendedHeader = QByteArray()); @@ -22,7 +22,7 @@ private: NLPacketList(const NLPacketList& other) = delete; NLPacketList& operator=(const NLPacketList& other) = delete; - virtual std::unique_ptr createPacket(); + virtual std::unique_ptr createPacket(); }; #endif // hifi_PacketList_h diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index a97b212355..eacdff9704 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -234,7 +234,7 @@ bool PacketReceiver::packetVersionMatch(const NLPacket& packet) { } } -void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { +void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { // if we're supposed to drop this packet then break out here if (_shouldDropPackets) { diff --git a/libraries/networking/src/PacketReceiver.h b/libraries/networking/src/PacketReceiver.h index ac38749a88..9851521250 100644 --- a/libraries/networking/src/PacketReceiver.h +++ b/libraries/networking/src/PacketReceiver.h @@ -45,7 +45,7 @@ public: bool registerListener(PacketType::Value type, QObject* listener, const char* slot); void unregisterListener(QObject* listener); - void handleVerifiedPacket(std::unique_ptr packet); + void handleVerifiedPacket(std::unique_ptr packet); signals: void dataReceived(quint8 channelType, int bytes); diff --git a/libraries/networking/src/udt/Packet.cpp b/libraries/networking/src/udt/Packet.cpp index 05b5f3fe50..b8c38f3b09 100644 --- a/libraries/networking/src/udt/Packet.cpp +++ b/libraries/networking/src/udt/Packet.cpp @@ -11,6 +11,8 @@ #include "Packet.h" +using namespace udt; + const qint64 Packet::PACKET_WRITE_ERROR = -1; qint64 Packet::localHeaderSize(PacketType::Value type) { diff --git a/libraries/networking/src/udt/Packet.h b/libraries/networking/src/udt/Packet.h index b4c53b8165..96e8f8c3d4 100644 --- a/libraries/networking/src/udt/Packet.h +++ b/libraries/networking/src/udt/Packet.h @@ -1,6 +1,6 @@ // // Packet.h -// libraries/networking/src +// libraries/networking/src/udt // // Created by Clement on 7/2/15. // Copyright 2015 High Fidelity, Inc. @@ -19,6 +19,8 @@ #include "../HifiSockAddr.h" #include "PacketHeaders.h" +namespace udt { + class Packet : public QIODevice { Q_OBJECT public: @@ -131,5 +133,7 @@ template qint64 Packet::writePrimitive(const T& data) { static_assert(!std::is_pointer::value, "T must not be a pointer"); return QIODevice::write(reinterpret_cast(&data), sizeof(T)); } + +} // namespace udt #endif // hifi_Packet_h diff --git a/libraries/networking/src/udt/PacketList.cpp b/libraries/networking/src/udt/PacketList.cpp index 5dcacfa9b1..47545eb910 100644 --- a/libraries/networking/src/udt/PacketList.cpp +++ b/libraries/networking/src/udt/PacketList.cpp @@ -15,6 +15,8 @@ #include "Packet.h" +using namespace udt; + PacketList::PacketList(PacketType::Value packetType, QByteArray extendedHeader) : _packetType(packetType), _extendedHeader(extendedHeader) diff --git a/libraries/networking/src/udt/PacketList.h b/libraries/networking/src/udt/PacketList.h index 30288dcaab..3d1166149c 100644 --- a/libraries/networking/src/udt/PacketList.h +++ b/libraries/networking/src/udt/PacketList.h @@ -18,6 +18,10 @@ #include "PacketHeaders.h" +class LimitedNodeList; + +namespace udt { + class Packet; class PacketList : public QIODevice { @@ -42,7 +46,7 @@ protected: virtual qint64 readData(char* data, qint64 maxSize) { return 0; } private: - friend class LimitedNodeList; + friend class ::LimitedNodeList; PacketList(const PacketList& other) = delete; PacketList& operator=(const PacketList& other) = delete; @@ -82,5 +86,7 @@ template std::unique_ptr PacketList::takeFront() { _packets.pop_front(); return std::unique_ptr(dynamic_cast(packet.release())); } + +} #endif // hifi_PacketList_h diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 2e1e3481f9..8f766594c6 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -18,7 +18,7 @@ using namespace udt; Socket::Socket(QObject* parent) : QObject(parent) { - + connect(&_udpSocket, &QUdpSocket::readyRead, this, &Socket::readPendingDatagrams); } void Socket::rebind() { @@ -66,3 +66,23 @@ qint64 Socket::writeDatagram(const QByteArray& datagram, const HifiSockAddr& soc return bytesWritten; } + +void Socket::readPendingDatagrams() { + while (_udpSocket.hasPendingDatagrams()) { + // setup a HifiSockAddr to read into + HifiSockAddr senderSockAddr; + + // setup a buffer to read the packet into + int packetSizeWithHeader = _udpSocket.pendingDatagramSize(); + std::unique_ptr buffer = std::unique_ptr(new char[packetSizeWithHeader]); + + // pull the datagram + _udpSocket.readDatagram(buffer.get(), packetSizeWithHeader, + senderSockAddr.getAddressPointer(), senderSockAddr.getPortPointer()); + + // setup a Packet from the data we just read + auto packet = Packet::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, senderSockAddr); + + + } +} diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 23ad75db12..3ab568c952 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -44,6 +44,10 @@ public: { _verifiedPacketFunction = verifiedPacketFunction; } void setBufferSizes(int numBytes); + +private slots: + void readPendingDatagrams(); + private: QUdpSocket _udpSocket { this }; VerifiedPacketFunction _verifiedPacketFunction; From cea8e01a012517ad09fd3aca2328dd42574d019d Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Jul 2015 17:35:16 -0700 Subject: [PATCH 03/21] add a verify packet operator to verify packets --- libraries/networking/src/LimitedNodeList.cpp | 2 +- libraries/networking/src/udt/Socket.cpp | 7 ++++++- libraries/networking/src/udt/Socket.h | 11 +++++++---- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index c36931fa9e..b5c152d194 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -99,7 +99,7 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short // set &PacketReceiver::handleVerifiedPacket as the verified packet function for the udt::Socket using std::placeholders::_1; - _nodeSocket.setVerifiedPacketFunction(std::bind(&PacketReceiver::handleVerifiedPacket, _packetReceiver, _1)); + _nodeSocket.setVerifiedPacketCallback(std::bind(&PacketReceiver::handleVerifiedPacket, _packetReceiver, _1)); _packetStatTimer.start(); diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 8f766594c6..8facac5cd4 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -83,6 +83,11 @@ void Socket::readPendingDatagrams() { // setup a Packet from the data we just read auto packet = Packet::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, senderSockAddr); - + // call our verification operator to see if this packet is verified + if (!_verifyPacketOperator || _verifyPacketOperator(*packet)) { + + // call the verified packet callback to let it handle this packet + return _verifiedPacketCallback(std::move(packet)); + } } } diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 3ab568c952..4d98667c3a 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -24,7 +24,8 @@ namespace udt { -using VerifiedPacketFunction = std::function)>; +using VerifyPacketOperator = std::function; +using VerifiedPacketCallback = std::function)>; class Socket : public QObject { Q_OBJECT @@ -40,8 +41,9 @@ public: void bind(const QHostAddress& address, quint16 port = 0) { _udpSocket.bind(address, port); } void rebind(); - void setVerifiedPacketFunction(VerifiedPacketFunction verifiedPacketFunction) - { _verifiedPacketFunction = verifiedPacketFunction; } + void setVerifyPacketOperator(VerifyPacketOperator verifyPacketOperator) { _verifyPacketOperator = verifyPacketOperator; } + void setVerifiedPacketCallback(VerifiedPacketCallback verifiedPacketCallback) + { _verifiedPacketCallback = verifiedPacketCallback; } void setBufferSizes(int numBytes); @@ -50,7 +52,8 @@ private slots: private: QUdpSocket _udpSocket { this }; - VerifiedPacketFunction _verifiedPacketFunction; + VerifyPacketOperator _verifyPacketOperator; + VerifiedPacketCallback _verifiedPacketCallback; }; } From 3901dbae33731060b1c0d8589886d38e90880115 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Jul 2015 11:06:25 -0700 Subject: [PATCH 04/21] fix for broken move in NLPacket/Packet --- libraries/networking/src/LimitedNodeList.cpp | 2 +- libraries/networking/src/NLPacket.cpp | 6 +++--- libraries/networking/src/PacketReceiver.cpp | 16 ++++++++-------- libraries/networking/src/udt/Packet.cpp | 2 ++ libraries/networking/src/udt/Socket.cpp | 1 + 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index b5c152d194..94e8b97779 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -566,7 +566,7 @@ void LimitedNodeList::sendSTUNRequest() { flagTimeForConnectionStep(ConnectionStep::SendSTUNRequest); - _nodeSocket.writeDatagram(stunRequestPacket, _stunSockAddr); + _nodeSocket.writeDatagram(QByteArray::fromRawData(stunRequestPacket, sizeof(stunRequestPacket)), _stunSockAddr); } bool LimitedNodeList::processSTUNResponse(QSharedPointer packet) { diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 5f24810d0b..3a3fe47397 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -90,7 +90,7 @@ NLPacket::NLPacket(PacketType::Value type) : } NLPacket::NLPacket(std::unique_ptr packet) : - Packet(*packet) + Packet(std::move(*packet.release())) { adjustPayloadStartAndCapacity(_payloadSize > 0); @@ -122,7 +122,7 @@ NLPacket::NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& } NLPacket::NLPacket(NLPacket&& other) : - Packet(other) + Packet(other) { _sourceID = std::move(other._sourceID); _verificationHash = std::move(other._verificationHash); @@ -130,7 +130,7 @@ NLPacket::NLPacket(NLPacket&& other) : NLPacket& NLPacket::operator=(NLPacket&& other) { - Packet::operator=(other); + Packet::operator=(std::move(other)); _sourceID = std::move(other._sourceID); _verificationHash = std::move(other._verificationHash); diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index eacdff9704..70dad7b1a6 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -267,7 +267,7 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { bool listenerIsDead = false; - auto it = _packetListenerMap.find(packet->getType()); + auto it = _packetListenerMap.find(nlPacket->getType()); if (it != _packetListenerMap.end()) { @@ -285,16 +285,16 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { _directConnectSetMutex.unlock(); - PacketType::Value packetType = packet->getType(); + PacketType::Value packetType = nlPacket->getType(); if (matchingNode) { // if this was a sequence numbered packet we should store the last seq number for // a packet of this type for this node - if (SEQUENCE_NUMBERED_PACKETS.contains(packet->getType())) { + if (SEQUENCE_NUMBERED_PACKETS.contains(nlPacket->getType())) { matchingNode->setLastSequenceNumberForPacketType(packet->readSequenceNumber(), packet->getType()); } - emit dataReceived(matchingNode->getType(), packet->getDataSize()); + emit dataReceived(matchingNode->getType(), nlPacket->getDataSize()); QMetaMethod metaMethod = listener.second; static const QByteArray QSHAREDPOINTER_NODE_NORMALIZED = QMetaObject::normalizedType("QSharedPointer"); @@ -327,7 +327,7 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { } } else { - emit dataReceived(NodeType::Unassigned, packet->getDataSize()); + emit dataReceived(NodeType::Unassigned, nlPacket->getDataSize()); success = listener.second.invoke(listener.first, Q_ARG(QSharedPointer, QSharedPointer(nlPacket.release()))); @@ -344,8 +344,8 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { } if (listenerIsDead) { - qDebug().nospace() << "Listener for packet" << packet->getType() - << " (" << qPrintable(nameForPacketType(packet->getType())) << ")" + qDebug().nospace() << "Listener for packet" << nlPacket->getType() + << " (" << qPrintable(nameForPacketType(nlPacket->getType())) << ")" << " has been destroyed. Removing from listener map."; it = _packetListenerMap.erase(it); @@ -356,7 +356,7 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { } } else { - qWarning() << "No listener found for packet type " << nameForPacketType(packet->getType()); + qWarning() << "No listener found for packet type " << nameForPacketType(nlPacket->getType()); // insert a dummy listener so we don't print this again _packetListenerMap.insert(packet->getType(), { nullptr, QMetaMethod() }); diff --git a/libraries/networking/src/udt/Packet.cpp b/libraries/networking/src/udt/Packet.cpp index b8c38f3b09..45c6a6f75f 100644 --- a/libraries/networking/src/udt/Packet.cpp +++ b/libraries/networking/src/udt/Packet.cpp @@ -138,6 +138,8 @@ Packet& Packet::operator=(Packet&& other) { _payloadSize = other._payloadSize; + _senderSockAddr = std::move(other._senderSockAddr); + if (other.isOpen() && !isOpen()) { open(other.openMode()); } diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 8facac5cd4..661c27bef5 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -58,6 +58,7 @@ void Socket::setBufferSizes(int numBytes) { } qint64 Socket::writeDatagram(const QByteArray& datagram, const HifiSockAddr& sockAddr) { + qint64 bytesWritten = _udpSocket.writeDatagram(datagram, sockAddr.getAddress(), sockAddr.getPort()); if (bytesWritten < 0) { From ed6867e1a01e945301f48c71d772ab57ae2bd843 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Jul 2015 11:47:06 -0700 Subject: [PATCH 05/21] add a method to LNL to verify packets --- libraries/networking/src/LimitedNodeList.cpp | 73 +++++++++++++++++--- libraries/networking/src/LimitedNodeList.h | 7 +- libraries/networking/src/NLPacket.cpp | 56 +++++++-------- libraries/networking/src/NLPacket.h | 11 ++- libraries/networking/src/PacketReceiver.cpp | 42 ----------- libraries/networking/src/PacketReceiver.h | 3 - 6 files changed, 99 insertions(+), 93 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 94e8b97779..821cd5e47b 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -101,6 +101,9 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short using std::placeholders::_1; _nodeSocket.setVerifiedPacketCallback(std::bind(&PacketReceiver::handleVerifiedPacket, _packetReceiver, _1)); + // set our isPacketVerified method as the verify operator for the udt::Socket + _nodeSocket.setVerifyPacketOperator(std::bind(&LimitedNodeList::isPacketVerified, this, _1)); + _packetStatTimer.start(); // make sure we handle STUN response packets @@ -149,27 +152,77 @@ QUdpSocket& LimitedNodeList::getDTLSSocket() { return *_dtlsSocket; } -bool LimitedNodeList::packetSourceAndHashMatch(const NLPacket& packet, SharedNodePointer& matchingNode) { +bool LimitedNodeList::isPacketVerified(const udt::Packet& packet) { + return packetVersionMatch(packet) && packetSourceAndHashMatch(packet); +} + +bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { + + if (packet.getVersion() != versionForPacketType(packet.getType())) { + + static QMultiHash sourcedVersionDebugSuppressMap; + static QMultiHash versionDebugSuppressMap; + + bool hasBeenOutput = false; + QString senderString; + + if (NON_SOURCED_PACKETS.contains(packet.getType())) { + const HifiSockAddr& senderSockAddr = packet.getSenderSockAddr(); + hasBeenOutput = versionDebugSuppressMap.contains(senderSockAddr, packet.getType()); + + if (!hasBeenOutput) { + versionDebugSuppressMap.insert(senderSockAddr, packet.getType()); + senderString = QString("%1:%2").arg(senderSockAddr.getAddress().toString()).arg(senderSockAddr.getPort()); + } + } else { + QUuid sourceID = QUuid::fromRfc4122(QByteArray::fromRawData(packet.getPayload(), NUM_BYTES_RFC4122_UUID)); + + hasBeenOutput = sourcedVersionDebugSuppressMap.contains(sourceID, packet.getType()); + + if (!hasBeenOutput) { + sourcedVersionDebugSuppressMap.insert(sourceID, packet.getType()); + senderString = uuidStringWithoutCurlyBraces(sourceID.toString()); + } + } + + if (!hasBeenOutput) { + qCDebug(networking) << "Packet version mismatch on" << packet.getType() << "- Sender" + << senderString << "sent" << qPrintable(QString::number(packet.getVersion())) << "but" + << qPrintable(QString::number(versionForPacketType(packet.getType()))) << "expected."; + + emit packetVersionMismatch(packet.getType()); + } + + return false; + } else { + return true; + } +} + +bool LimitedNodeList::packetSourceAndHashMatch(const udt::Packet& packet) { if (NON_SOURCED_PACKETS.contains(packet.getType())) { return true; } else { + QUuid sourceID = NLPacket::sourceIDInHeader(packet); // figure out which node this is from - matchingNode = nodeWithUUID(packet.getSourceID()); + SharedNodePointer matchingNode = nodeWithUUID(sourceID); if (matchingNode) { if (!NON_VERIFIED_PACKETS.contains(packet.getType())) { + + QByteArray packetHeaderHash = NLPacket::verificationHashInHeader(packet); + QByteArray expectedHash = NLPacket::hashForPacketAndSecret(packet, matchingNode->getConnectionSecret()); + // check if the md5 hash in the header matches the hash we would expect - if (packet.getVerificationHash() != packet.payloadHashWithConnectionUUID(matchingNode->getConnectionSecret())) { + if (packetHeaderHash != expectedHash) { static QMultiMap hashDebugSuppressMap; - const QUuid& senderID = packet.getSourceID(); + if (!hashDebugSuppressMap.contains(sourceID, packet.getType())) { + qCDebug(networking) << "Packet hash mismatch on" << packet.getType() << "- Sender" << sourceID; - if (!hashDebugSuppressMap.contains(senderID, packet.getType())) { - qCDebug(networking) << "Packet hash mismatch on" << packet.getType() << "- Sender" << senderID; - - hashDebugSuppressMap.insert(senderID, packet.getType()); + hashDebugSuppressMap.insert(sourceID, packet.getType()); } return false; @@ -183,7 +236,7 @@ bool LimitedNodeList::packetSourceAndHashMatch(const NLPacket& packet, SharedNod = LogHandler::getInstance().addRepeatedMessageRegex("Packet of type \\d+ \\([\\sa-zA-Z]+\\) received from unknown node with UUID"); qCDebug(networking) << "Packet of type" << packet.getType() << "(" << qPrintable(nameForPacketType(packet.getType())) << ")" - << "received from unknown node with UUID" << qPrintable(uuidStringWithoutCurlyBraces(packet.getSourceID())); + << "received from unknown node with UUID" << qPrintable(uuidStringWithoutCurlyBraces(sourceID)); } } @@ -215,7 +268,7 @@ qint64 LimitedNodeList::writePacket(const NLPacket& packet, const HifiSockAddr& if (!connectionSecret.isNull() && !NON_SOURCED_PACKETS.contains(packet.getType()) && !NON_VERIFIED_PACKETS.contains(packet.getType())) { - const_cast(packet).writeVerificationHash(packet.payloadHashWithConnectionUUID(connectionSecret)); + const_cast(packet).writeVerificationHashGivenSecret(connectionSecret); } emit dataSent(NodeType::Unassigned, packet.getDataSize()); diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 8a4230be04..f05ba4b0dc 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -114,8 +114,6 @@ public: quint16 getSocketLocalPort() const { return _nodeSocket.localPort(); } QUdpSocket& getDTLSSocket(); - bool packetSourceAndHashMatch(const NLPacket& packet, SharedNodePointer& matchingNode); - PacketReceiver& getPacketReceiver() { return *_packetReceiver; } qint64 sendUnreliablePacket(const NLPacket& packet, const Node& destinationNode); @@ -233,6 +231,7 @@ public slots: signals: void dataSent(quint8 channelType, int bytes); + void packetVersionMismatch(PacketType::Value type); void uuidChanged(const QUuid& ownerUUID, const QUuid& oldUUID); void nodeAdded(SharedNodePointer); @@ -255,6 +254,10 @@ protected: qint64 writeDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr); PacketSequenceNumber getNextSequenceNumberForPacket(const QUuid& nodeUUID, PacketType::Value packetType); + + bool isPacketVerified(const udt::Packet& packet); + bool packetVersionMatch(const udt::Packet& packet); + bool packetSourceAndHashMatch(const udt::Packet& packet); void handleNodeKill(const SharedNodePointer& node); diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 3a3fe47397..f9964c8eb1 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -95,19 +95,16 @@ NLPacket::NLPacket(std::unique_ptr packet) : adjustPayloadStartAndCapacity(_payloadSize > 0); readSourceID(); - readVerificationHash(); } NLPacket::NLPacket(const NLPacket& other) : Packet(other) { _sourceID = other._sourceID; - _verificationHash = other._verificationHash; } NLPacket& NLPacket::operator=(const NLPacket& other) { Packet::operator=(other); _sourceID = other._sourceID; - _verificationHash = other._verificationHash; return *this; } @@ -118,14 +115,12 @@ NLPacket::NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& adjustPayloadStartAndCapacity(); readSourceID(); - readVerificationHash(); } NLPacket::NLPacket(NLPacket&& other) : Packet(other) { _sourceID = std::move(other._sourceID); - _verificationHash = std::move(other._verificationHash); } NLPacket& NLPacket::operator=(NLPacket&& other) { @@ -133,11 +128,32 @@ NLPacket& NLPacket::operator=(NLPacket&& other) { Packet::operator=(std::move(other)); _sourceID = std::move(other._sourceID); - _verificationHash = std::move(other._verificationHash); return *this; } +QUuid NLPacket::sourceIDInHeader(const udt::Packet& packet) { + int offset = packet.localHeaderSize(); + return QUuid::fromRfc4122(QByteArray::fromRawData(packet.getData() + offset, NUM_BYTES_RFC4122_UUID)); +} + +QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) { + int offset = packet.localHeaderSize() + NUM_BYTES_RFC4122_UUID; + return QByteArray(packet.getData() + offset, NUM_BYTES_MD5_HASH); +} + +QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret) { + QCryptographicHash hash(QCryptographicHash::Md5); + + int offset = packet.localHeaderSize() + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; + + // add the packet payload and the connection UUID + hash.addData(packet.getData(), packet.getDataSize() - offset); + hash.addData(connectionSecret.toRfc4122()); + + // return the hash + return hash.result(); +} void NLPacket::adjustPayloadStartAndCapacity(bool shouldDecreasePayloadSize) { qint64 headerSize = localHeaderSize(_type); @@ -151,15 +167,7 @@ void NLPacket::adjustPayloadStartAndCapacity(bool shouldDecreasePayloadSize) { void NLPacket::readSourceID() { if (!NON_SOURCED_PACKETS.contains(_type)) { - auto offset = Packet::localHeaderSize(); - _sourceID = QUuid::fromRfc4122(QByteArray::fromRawData(_packet.get() + offset, NUM_BYTES_RFC4122_UUID)); - } -} - -void NLPacket::readVerificationHash() { - if (!NON_SOURCED_PACKETS.contains(_type) && !NON_VERIFIED_PACKETS.contains(_type)) { - auto offset = Packet::localHeaderSize() + NUM_BYTES_RFC4122_UUID; - _verificationHash = QByteArray(_packet.get() + offset, NUM_BYTES_MD5_HASH); + _sourceID = sourceIDInHeader(*this); } } @@ -172,22 +180,10 @@ void NLPacket::writeSourceID(const QUuid& sourceID) { _sourceID = sourceID; } -void NLPacket::writeVerificationHash(const QByteArray& verificationHash) { +void NLPacket::writeVerificationHashGivenSecret(const QUuid& connectionSecret) { Q_ASSERT(!NON_SOURCED_PACKETS.contains(_type) && !NON_VERIFIED_PACKETS.contains(_type)); - + auto offset = Packet::localHeaderSize() + NUM_BYTES_RFC4122_UUID; + QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret); memcpy(_packet.get() + offset, verificationHash.data(), verificationHash.size()); - - _verificationHash = verificationHash; -} - -QByteArray NLPacket::payloadHashWithConnectionUUID(const QUuid& connectionUUID) const { - QCryptographicHash hash(QCryptographicHash::Md5); - - // add the packet payload and the connection UUID - hash.addData(_payloadStart, _payloadSize); - hash.addData(connectionUUID.toRfc4122()); - - // return the hash - return hash.result(); } diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index aaf6c5df75..9b2b78c3f9 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -26,6 +26,10 @@ public: // Provided for convenience, try to limit use static std::unique_ptr createCopy(const NLPacket& other); + + static QUuid sourceIDInHeader(const udt::Packet& packet); + static QByteArray verificationHashInHeader(const udt::Packet& packet); + static QByteArray hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret); static qint64 localHeaderSize(PacketType::Value type); static qint64 maxPayloadSize(PacketType::Value type); @@ -34,12 +38,9 @@ public: virtual qint64 localHeaderSize() const; // Current level's header size const QUuid& getSourceID() const { return _sourceID; } - const QByteArray& getVerificationHash() const { return _verificationHash; } void writeSourceID(const QUuid& sourceID); - void writeVerificationHash(const QByteArray& verificationHash); - - QByteArray payloadHashWithConnectionUUID(const QUuid& connectionUUID) const; + void writeVerificationHashGivenSecret(const QUuid& connectionSecret); protected: @@ -58,10 +59,8 @@ protected: NLPacket& operator=(NLPacket&& other); void readSourceID(); - void readVerificationHash(); QUuid _sourceID; - QByteArray _verificationHash; }; #endif // hifi_NLPacket_h diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 70dad7b1a6..16ee775afa 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -192,48 +192,6 @@ void PacketReceiver::unregisterListener(QObject* listener) { _directConnectSetMutex.unlock(); } -bool PacketReceiver::packetVersionMatch(const NLPacket& packet) { - - if (packet.getVersion() != versionForPacketType(packet.getType()) - && packet.getType() != PacketType::StunResponse) { - - static QMultiHash sourcedVersionDebugSuppressMap; - static QMultiHash versionDebugSuppressMap; - - bool hasBeenOutput = false; - QString senderString; - - if (NON_SOURCED_PACKETS.contains(packet.getType())) { - const HifiSockAddr& senderSockAddr = packet.getSenderSockAddr(); - hasBeenOutput = versionDebugSuppressMap.contains(senderSockAddr, packet.getType()); - - if (!hasBeenOutput) { - versionDebugSuppressMap.insert(senderSockAddr, packet.getType()); - senderString = QString("%1:%2").arg(senderSockAddr.getAddress().toString()).arg(senderSockAddr.getPort()); - } - } else { - hasBeenOutput = sourcedVersionDebugSuppressMap.contains(packet.getSourceID(), packet.getType()); - - if (!hasBeenOutput) { - sourcedVersionDebugSuppressMap.insert(packet.getSourceID(), packet.getType()); - senderString = uuidStringWithoutCurlyBraces(packet.getSourceID().toString()); - } - } - - if (!hasBeenOutput) { - qCDebug(networking) << "Packet version mismatch on" << packet.getType() << "- Sender" - << senderString << "sent" << qPrintable(QString::number(packet.getVersion())) << "but" - << qPrintable(QString::number(versionForPacketType(packet.getType()))) << "expected."; - - emit packetVersionMismatch(packet.getType()); - } - - return false; - } else { - return true; - } -} - void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { // if we're supposed to drop this packet then break out here diff --git a/libraries/networking/src/PacketReceiver.h b/libraries/networking/src/PacketReceiver.h index 9851521250..25bf9656be 100644 --- a/libraries/networking/src/PacketReceiver.h +++ b/libraries/networking/src/PacketReceiver.h @@ -49,15 +49,12 @@ public: signals: void dataReceived(quint8 channelType, int bytes); - void packetVersionMismatch(PacketType::Value type); private: // these are brutal hacks for now - ideally GenericThread / ReceivedPacketProcessor // should be changed to have a true event loop and be able to handle our QMetaMethod::invoke void registerDirectListenerForTypes(const QSet& types, QObject* listener, const char* slot); void registerDirectListener(PacketType::Value type, QObject* listener, const char* slot); - - bool packetVersionMatch(const NLPacket& packet); QMetaMethod matchingMethodForListener(PacketType::Value type, QObject* object, const char* slot) const; void registerVerifiedListener(PacketType::Value type, QObject* listener, const QMetaMethod& slot); From a61c04aa92e4d5d9f6095aed6a4054181a7cc9f9 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Jul 2015 12:00:16 -0700 Subject: [PATCH 06/21] allow socket owner to specify unfiltered packets --- interface/src/Application.cpp | 3 +-- libraries/networking/src/LimitedNodeList.cpp | 1 + libraries/networking/src/LimitedNodeList.h | 2 ++ libraries/networking/src/udt/Socket.cpp | 2 +- libraries/networking/src/udt/Socket.h | 4 ++++ 5 files changed, 9 insertions(+), 3 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index f0dcdd1a8a..e920c760ba 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -452,8 +452,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : connect(nodeList.data(), &NodeList::uuidChanged, _myAvatar, &MyAvatar::setSessionUUID); connect(nodeList.data(), &NodeList::uuidChanged, this, &Application::setSessionUUID); connect(nodeList.data(), &NodeList::limitOfSilentDomainCheckInsReached, nodeList.data(), &NodeList::reset); - connect(&nodeList->getPacketReceiver(), &PacketReceiver::packetVersionMismatch, - this, &Application::notifyPacketVersionMismatch); + connect(&nodeList.data(), &NodeList::packetVersionMismatch, this, &Application::notifyPacketVersionMismatch); // connect to appropriate slots on AccountManager AccountManager& accountManager = AccountManager::getInstance(); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 821cd5e47b..4d8b7e48a3 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -710,6 +710,7 @@ void LimitedNodeList::startSTUNPublicSocketUpdate() { // if we don't know the STUN IP yet we need to have ourselves be called once it is known if (_stunSockAddr.getAddress().isNull()) { connect(&_stunSockAddr, &HifiSockAddr::lookupCompleted, this, &LimitedNodeList::startSTUNPublicSocketUpdate); + connect(&_stunSockAddr, &HifiSockAddr::lookupCompleted, this, &LimitedNodeList::addSTUNSockAddrToUnfiltered); // in case we just completely fail to lookup the stun socket - add a 10s timeout that will trigger the fail case const quint64 STUN_DNS_LOOKUP_TIMEOUT_MSECS = 10 * 1000; diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index f05ba4b0dc..ebfe77cc8b 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -310,9 +310,11 @@ protected: functor(it); } } + private slots: void flagTimeForConnectionStep(ConnectionStep connectionStep, quint64 timestamp); void possiblyTimeoutSTUNAddressLookup(); + void addSTUNSockAddrToUnfiltered() { _nodeSocket.addUnfilteredSockAddr(_stunSockAddr); } // called once STUN socket known }; #endif // hifi_LimitedNodeList_h diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 661c27bef5..0192e306c3 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -85,7 +85,7 @@ void Socket::readPendingDatagrams() { auto packet = Packet::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, senderSockAddr); // call our verification operator to see if this packet is verified - if (!_verifyPacketOperator || _verifyPacketOperator(*packet)) { + if (_unfilteredSockAddrs.contains(senderSockAddr) || !_verifyPacketOperator || _verifyPacketOperator(*packet)) { // call the verified packet callback to let it handle this packet return _verifiedPacketCallback(std::move(packet)); diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 4d98667c3a..36f11213ff 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -46,6 +46,8 @@ public: { _verifiedPacketCallback = verifiedPacketCallback; } void setBufferSizes(int numBytes); + + void addUnfilteredSockAddr(const HifiSockAddr& senderSockAddr) { _unfilteredSockAddrs.insert(senderSockAddr); } private slots: void readPendingDatagrams(); @@ -54,6 +56,8 @@ private: QUdpSocket _udpSocket { this }; VerifyPacketOperator _verifyPacketOperator; VerifiedPacketCallback _verifiedPacketCallback; + + QSet _unfilteredSockAddrs; }; } From da761d4c952d280d335462a8d90b8bacf1046307 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Jul 2015 12:12:15 -0700 Subject: [PATCH 07/21] fix static methods to handle header read/write --- libraries/networking/src/NLPacket.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index f9964c8eb1..fdfb94f107 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -133,19 +133,19 @@ NLPacket& NLPacket::operator=(NLPacket&& other) { } QUuid NLPacket::sourceIDInHeader(const udt::Packet& packet) { - int offset = packet.localHeaderSize(); + int offset = packet.Packet::localHeaderSize(); return QUuid::fromRfc4122(QByteArray::fromRawData(packet.getData() + offset, NUM_BYTES_RFC4122_UUID)); } QByteArray NLPacket::verificationHashInHeader(const udt::Packet& packet) { - int offset = packet.localHeaderSize() + NUM_BYTES_RFC4122_UUID; + int offset = packet.Packet::localHeaderSize() + NUM_BYTES_RFC4122_UUID; return QByteArray(packet.getData() + offset, NUM_BYTES_MD5_HASH); } QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUuid& connectionSecret) { QCryptographicHash hash(QCryptographicHash::Md5); - int offset = packet.localHeaderSize() + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; + int offset = packet.Packet::localHeaderSize() + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; // add the packet payload and the connection UUID hash.addData(packet.getData(), packet.getDataSize() - offset); @@ -185,5 +185,6 @@ void NLPacket::writeVerificationHashGivenSecret(const QUuid& connectionSecret) { auto offset = Packet::localHeaderSize() + NUM_BYTES_RFC4122_UUID; QByteArray verificationHash = hashForPacketAndSecret(*this, connectionSecret); + memcpy(_packet.get() + offset, verificationHash.data(), verificationHash.size()); } From 4604bc5a3a3e02fd5398bd2351d86397e13fdc69 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Jul 2015 12:27:27 -0700 Subject: [PATCH 08/21] leverage udt::Socket in ice-server --- ice-server/src/IceServer.cpp | 95 +++++++++----------- ice-server/src/IceServer.h | 7 +- libraries/networking/src/LimitedNodeList.cpp | 10 +-- libraries/networking/src/LimitedNodeList.h | 2 +- libraries/networking/src/udt/Socket.cpp | 4 + libraries/networking/src/udt/Socket.h | 2 + 6 files changed, 57 insertions(+), 63 deletions(-) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index 1d38802f4e..fb7433aa10 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -34,8 +34,9 @@ IceServer::IceServer(int argc, char* argv[]) : qDebug() << "monitoring http endpoint is listening on " << ICE_SERVER_MONITORING_PORT; _serverSocket.bind(QHostAddress::AnyIPv4, ICE_SERVER_DEFAULT_PORT); - // call our process datagrams slot when the UDP socket has packets ready - connect(&_serverSocket, &QUdpSocket::readyRead, this, &IceServer::processDatagrams); + // set processPacket as the verified packet callback for the udt::Socket + using std::placeholders::_1; + _serverSocket.setVerifiedPacketCallback(std::bind(&IceServer::processPacket, this, _1)); // setup our timer to clear inactive peers QTimer* inactivePeerTimer = new QTimer(this); @@ -44,58 +45,45 @@ IceServer::IceServer(int argc, char* argv[]) : } -void IceServer::processDatagrams() { - HifiSockAddr sendingSockAddr; - - while (_serverSocket.hasPendingDatagrams()) { - // setup a buffer to read the packet into - int packetSizeWithHeader = _serverSocket.pendingDatagramSize(); - std::unique_ptr buffer = std::unique_ptr(new char[packetSizeWithHeader]); - - _serverSocket.readDatagram(buffer.get(), packetSizeWithHeader, - sendingSockAddr.getAddressPointer(), sendingSockAddr.getPortPointer()); +void IceServer::processPacket(std::unique_ptr packet) { + PacketType::Value packetType = packet->getType(); + + if (packetType == PacketType::ICEServerHeartbeat) { + SharedNetworkPeer peer = addOrUpdateHeartbeatingPeer(*packet); - auto packet = udt::Packet::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, sendingSockAddr); - - PacketType::Value packetType = packet->getType(); - - if (packetType == PacketType::ICEServerHeartbeat) { - SharedNetworkPeer peer = addOrUpdateHeartbeatingPeer(*packet); - - // so that we can send packets to the heartbeating peer when we need, we need to activate a socket now - peer->activateMatchingOrNewSymmetricSocket(sendingSockAddr); - } else if (packetType == PacketType::ICEServerQuery) { - QDataStream heartbeatStream(packet.get()); - - // this is a node hoping to connect to a heartbeating peer - do we have the heartbeating peer? - QUuid senderUUID; - heartbeatStream >> senderUUID; - - // pull the public and private sock addrs for this peer - HifiSockAddr publicSocket, localSocket; - heartbeatStream >> publicSocket >> localSocket; - - // check if this node also included a UUID that they would like to connect to - QUuid connectRequestID; - heartbeatStream >> connectRequestID; + // so that we can send packets to the heartbeating peer when we need, we need to activate a socket now + peer->activateMatchingOrNewSymmetricSocket(packet->getSenderSockAddr()); + } else if (packetType == PacketType::ICEServerQuery) { + QDataStream heartbeatStream(packet.get()); + + // this is a node hoping to connect to a heartbeating peer - do we have the heartbeating peer? + QUuid senderUUID; + heartbeatStream >> senderUUID; + + // pull the public and private sock addrs for this peer + HifiSockAddr publicSocket, localSocket; + heartbeatStream >> publicSocket >> localSocket; + + // check if this node also included a UUID that they would like to connect to + QUuid connectRequestID; + heartbeatStream >> connectRequestID; + + SharedNetworkPeer matchingPeer = _activePeers.value(connectRequestID); + + if (matchingPeer) { - SharedNetworkPeer matchingPeer = _activePeers.value(connectRequestID); - - if (matchingPeer) { - - qDebug() << "Sending information for peer" << connectRequestID << "to peer" << senderUUID; - - // we have the peer they want to connect to - send them pack the information for that peer - sendPeerInformationPacket(*(matchingPeer.data()), &sendingSockAddr); - - // we also need to send them to the active peer they are hoping to connect to - // create a dummy peer object we can pass to sendPeerInformationPacket - - NetworkPeer dummyPeer(senderUUID, publicSocket, localSocket); - sendPeerInformationPacket(dummyPeer, matchingPeer->getActiveSocket()); - } else { - qDebug() << "Peer" << senderUUID << "asked for" << connectRequestID << "but no matching peer found"; - } + qDebug() << "Sending information for peer" << connectRequestID << "to peer" << senderUUID; + + // we have the peer they want to connect to - send them pack the information for that peer + sendPeerInformationPacket(*(matchingPeer.data()), &packet->getSenderSockAddr()); + + // we also need to send them to the active peer they are hoping to connect to + // create a dummy peer object we can pass to sendPeerInformationPacket + + NetworkPeer dummyPeer(senderUUID, publicSocket, localSocket); + sendPeerInformationPacket(dummyPeer, matchingPeer->getActiveSocket()); + } else { + qDebug() << "Peer" << senderUUID << "asked for" << connectRequestID << "but no matching peer found"; } } } @@ -139,8 +127,7 @@ void IceServer::sendPeerInformationPacket(const NetworkPeer& peer, const HifiSoc peerPacket->write(peer.toByteArray()); // write the current packet - _serverSocket.writeDatagram(peerPacket->getData(), peerPacket->getDataSize(), - destinationSockAddr->getAddress(), destinationSockAddr->getPort()); + _serverSocket.writeUnreliablePacket(*peerPacket, *destinationSockAddr); } void IceServer::clearInactivePeers() { diff --git a/ice-server/src/IceServer.h b/ice-server/src/IceServer.h index e91dc4e064..092ece6c3f 100644 --- a/ice-server/src/IceServer.h +++ b/ice-server/src/IceServer.h @@ -20,6 +20,7 @@ #include #include #include +#include typedef QHash NetworkPeerHash; @@ -29,15 +30,15 @@ public: IceServer(int argc, char* argv[]); bool handleHTTPRequest(HTTPConnection* connection, const QUrl& url, bool skipSubHandler = false); private slots: - void processDatagrams(); void clearInactivePeers(); private: - + void processPacket(std::unique_ptr packet); + SharedNetworkPeer addOrUpdateHeartbeatingPeer(udt::Packet& incomingPacket); void sendPeerInformationPacket(const NetworkPeer& peer, const HifiSockAddr* destinationSockAddr); QUuid _id; - QUdpSocket _serverSocket; + udt::Socket _serverSocket; NetworkPeerHash _activePeers; HTTPManager _httpManager; }; diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 4d8b7e48a3..45deb80c16 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -97,7 +97,7 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short // check the local socket right now updateLocalSockAddr(); - // set &PacketReceiver::handleVerifiedPacket as the verified packet function for the udt::Socket + // set &PacketReceiver::handleVerifiedPacket as the verified packet callback for the udt::Socket using std::placeholders::_1; _nodeSocket.setVerifiedPacketCallback(std::bind(&PacketReceiver::handleVerifiedPacket, _packetReceiver, _1)); @@ -273,16 +273,16 @@ qint64 LimitedNodeList::writePacket(const NLPacket& packet, const HifiSockAddr& emit dataSent(NodeType::Unassigned, packet.getDataSize()); - return writeDatagram(QByteArray::fromRawData(packet.getData(), packet.getDataSize()), destinationSockAddr); + return writePacketAndCollectStats(packet, destinationSockAddr); } -qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr) { +qint64 LimitedNodeList::writePacketAndCollectStats(const NLPacket& packet, const HifiSockAddr& destinationSockAddr) { // XXX can BandwidthRecorder be used for this? // stat collection for packets ++_numCollectedPackets; - _numCollectedBytes += datagram.size(); + _numCollectedBytes += packet.getDataSize(); - qint64 bytesWritten = _nodeSocket.writeDatagram(datagram, destinationSockAddr); + qint64 bytesWritten = _nodeSocket.writeUnreliablePacket(packet, destinationSockAddr); return bytesWritten; } diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index ebfe77cc8b..67dc707815 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -251,7 +251,7 @@ protected: qint64 writePacket(const NLPacket& packet, const Node& destinationNode); qint64 writePacket(const NLPacket& packet, const HifiSockAddr& destinationSockAddr, const QUuid& connectionSecret = QUuid()); - qint64 writeDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr); + qint64 writePacketAndCollectStats(const NLPacket& packet, const HifiSockAddr& destinationSockAddr); PacketSequenceNumber getNextSequenceNumberForPacket(const QUuid& nodeUUID, PacketType::Value packetType); diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 0192e306c3..33ad4159c2 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -57,6 +57,10 @@ void Socket::setBufferSizes(int numBytes) { } } +qint64 Socket::writeUnreliablePacket(const Packet& packet, const HifiSockAddr& sockAddr) { + return writeDatagram(packet.getData(), packet.getDataSize(), sockAddr); +} + qint64 Socket::writeDatagram(const QByteArray& datagram, const HifiSockAddr& sockAddr) { qint64 bytesWritten = _udpSocket.writeDatagram(datagram, sockAddr.getAddress(), sockAddr.getPort()); diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 36f11213ff..6997986e91 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -34,6 +34,8 @@ public: quint16 localPort() const { return _udpSocket.localPort(); } + qint64 writeUnreliablePacket(const Packet& packet, const HifiSockAddr& sockAddr); + qint64 writeDatagram(const char* data, qint64 size, const HifiSockAddr& sockAddr) { return writeDatagram(QByteArray::fromRawData(data, size), sockAddr); } qint64 writeDatagram(const QByteArray& datagram, const HifiSockAddr& sockAddr); From 086e4efe4490ed6a19ef472ba8b5fc81d0c3e666 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Jul 2015 13:03:39 -0700 Subject: [PATCH 09/21] verify packet versions at ice-server --- ice-server/src/IceServer.cpp | 14 ++++++++++++ ice-server/src/IceServer.h | 1 + interface/src/Application.cpp | 2 +- libraries/networking/src/NetworkPeer.cpp | 28 ++++++++++++++---------- libraries/networking/src/udt/Socket.h | 2 +- 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index fb7433aa10..e86ed0b4ea 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -37,6 +37,9 @@ IceServer::IceServer(int argc, char* argv[]) : // set processPacket as the verified packet callback for the udt::Socket using std::placeholders::_1; _serverSocket.setVerifiedPacketCallback(std::bind(&IceServer::processPacket, this, _1)); + + // set packetVersionMatch as the verify packet operator for the udt::Socket + _serverSocket.setVerifyPacketOperator(std::bind(&IceServer::packetVersionMatch, this, _1)); // setup our timer to clear inactive peers QTimer* inactivePeerTimer = new QTimer(this); @@ -45,6 +48,17 @@ IceServer::IceServer(int argc, char* argv[]) : } +bool IceServer::packetVersionMatch(const udt::Packet& packet) { + if (packet.getVersion() == versionForPacketType(packet.getType())) { + return true; + } else { + qDebug() << "Packet version mismatch for packet" << packet.getType() + << "(" << nameForPacketType(packet.getType()) << ") from" << packet.getSenderSockAddr(); + + return false; + } +} + void IceServer::processPacket(std::unique_ptr packet) { PacketType::Value packetType = packet->getType(); diff --git a/ice-server/src/IceServer.h b/ice-server/src/IceServer.h index 092ece6c3f..3ab6df9044 100644 --- a/ice-server/src/IceServer.h +++ b/ice-server/src/IceServer.h @@ -32,6 +32,7 @@ public: private slots: void clearInactivePeers(); private: + bool packetVersionMatch(const udt::Packet& packet); void processPacket(std::unique_ptr packet); SharedNetworkPeer addOrUpdateHeartbeatingPeer(udt::Packet& incomingPacket); diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index e920c760ba..652d12ade0 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -452,7 +452,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer &startup_time) : connect(nodeList.data(), &NodeList::uuidChanged, _myAvatar, &MyAvatar::setSessionUUID); connect(nodeList.data(), &NodeList::uuidChanged, this, &Application::setSessionUUID); connect(nodeList.data(), &NodeList::limitOfSilentDomainCheckInsReached, nodeList.data(), &NodeList::reset); - connect(&nodeList.data(), &NodeList::packetVersionMismatch, this, &Application::notifyPacketVersionMismatch); + connect(nodeList.data(), &NodeList::packetVersionMismatch, this, &Application::notifyPacketVersionMismatch); // connect to appropriate slots on AccountManager AccountManager& accountManager = AccountManager::getInstance(); diff --git a/libraries/networking/src/NetworkPeer.cpp b/libraries/networking/src/NetworkPeer.cpp index 398d4106c1..28f19d4f60 100644 --- a/libraries/networking/src/NetworkPeer.cpp +++ b/libraries/networking/src/NetworkPeer.cpp @@ -54,12 +54,14 @@ void NetworkPeer::setPublicSocket(const HifiSockAddr& publicSocket) { // if the active socket was the public socket then reset it to NULL _activeSocket = NULL; } - - if (!_publicSocket.isNull()) { - qCDebug(networking) << "Public socket change for node" << *this; - } + + bool wasOldSocketNull = _publicSocket.isNull(); _publicSocket = publicSocket; + + if (!wasOldSocketNull) { + qCDebug(networking) << "Public socket change for node" << *this; + } } } @@ -69,12 +71,14 @@ void NetworkPeer::setLocalSocket(const HifiSockAddr& localSocket) { // if the active socket was the local socket then reset it to NULL _activeSocket = NULL; } + + bool wasOldSocketNull = _localSocket.isNull(); + + _localSocket = localSocket; - if (!_localSocket.isNull()) { + if (!wasOldSocketNull) { qCDebug(networking) << "Local socket change for node" << *this; } - - _localSocket = localSocket; } } @@ -84,12 +88,14 @@ void NetworkPeer::setSymmetricSocket(const HifiSockAddr& symmetricSocket) { // if the active socket was the symmetric socket then reset it to NULL _activeSocket = NULL; } - - if (!_symmetricSocket.isNull()) { + + bool wasOldSocketNull = _symmetricSocket.isNull(); + + _symmetricSocket = symmetricSocket; + + if (!wasOldSocketNull) { qCDebug(networking) << "Symmetric socket change for node" << *this; } - - _symmetricSocket = symmetricSocket; } } diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 6997986e91..2ebeb179bc 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -24,7 +24,7 @@ namespace udt { -using VerifyPacketOperator = std::function; +using VerifyPacketOperator = std::function; using VerifiedPacketCallback = std::function)>; class Socket : public QObject { From 30225ba3c1d0829ff57c185b6dc3a3ee17503bda Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Jul 2015 13:05:38 -0700 Subject: [PATCH 10/21] fix spacing for multi-line QDebug in PacketReceiver --- libraries/networking/src/PacketReceiver.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 16ee775afa..502563489b 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -293,8 +293,8 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { if (!success) { qDebug().nospace() << "Error delivering packet " << packetType - << " (" << qPrintable(nameForPacketType(packetType)) << ") to listener " - << listener.first << "::" << qPrintable(listener.second.methodSignature()); + << " (" << qPrintable(nameForPacketType(packetType)) << ") to listener " + << listener.first << "::" << qPrintable(listener.second.methodSignature()); } } else { @@ -303,8 +303,8 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { if (listenerIsDead) { qDebug().nospace() << "Listener for packet" << nlPacket->getType() - << " (" << qPrintable(nameForPacketType(nlPacket->getType())) << ")" - << " has been destroyed. Removing from listener map."; + << " (" << qPrintable(nameForPacketType(nlPacket->getType())) << ")" + << " has been destroyed. Removing from listener map."; it = _packetListenerMap.erase(it); // if it exists, remove the listener from _directlyConnectedObjects From 240f53ddd2dfc4ae19c0f4805c8e79dcbf209980 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 09:51:13 -0700 Subject: [PATCH 11/21] bunch up the NLPacket constructors --- libraries/networking/src/NLPacket.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index 9b2b78c3f9..aad6e6d072 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -48,13 +48,10 @@ protected: NLPacket(PacketType::Value type); NLPacket(PacketType::Value type, qint64 size); - NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr); NLPacket(std::unique_ptr packet); - NLPacket(const NLPacket& other); NLPacket& operator=(const NLPacket& other); - NLPacket(NLPacket&& other); NLPacket& operator=(NLPacket&& other); From 7f53eda0d99cc5df1b053d56e98798e6f622abc3 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 10:01:24 -0700 Subject: [PATCH 12/21] naming changes to udt::Socket handlers --- ice-server/src/IceServer.cpp | 4 ++-- libraries/networking/src/LimitedNodeList.cpp | 4 ++-- libraries/networking/src/udt/Socket.cpp | 9 +++++---- libraries/networking/src/udt/Socket.h | 13 ++++++------- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index e86ed0b4ea..c4dd2cf532 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -36,10 +36,10 @@ IceServer::IceServer(int argc, char* argv[]) : // set processPacket as the verified packet callback for the udt::Socket using std::placeholders::_1; - _serverSocket.setVerifiedPacketCallback(std::bind(&IceServer::processPacket, this, _1)); + _serverSocket.setVerifiedPacketHandler(std::bind(&IceServer::processPacket, this, _1)); // set packetVersionMatch as the verify packet operator for the udt::Socket - _serverSocket.setVerifyPacketOperator(std::bind(&IceServer::packetVersionMatch, this, _1)); + _serverSocket.setPacketVerificationHandler(std::bind(&IceServer::packetVersionMatch, this, _1)); // setup our timer to clear inactive peers QTimer* inactivePeerTimer = new QTimer(this); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 45deb80c16..e6b0b27712 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -99,10 +99,10 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short // set &PacketReceiver::handleVerifiedPacket as the verified packet callback for the udt::Socket using std::placeholders::_1; - _nodeSocket.setVerifiedPacketCallback(std::bind(&PacketReceiver::handleVerifiedPacket, _packetReceiver, _1)); + _nodeSocket.setVerifiedPacketHandler(std::bind(&PacketReceiver::handleVerifiedPacket, _packetReceiver, _1)); // set our isPacketVerified method as the verify operator for the udt::Socket - _nodeSocket.setVerifyPacketOperator(std::bind(&LimitedNodeList::isPacketVerified, this, _1)); + _nodeSocket.setPacketVerificationHandler(std::bind(&LimitedNodeList::isPacketVerified, this, _1)); _packetStatTimer.start(); diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 33ad4159c2..685689f4e6 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -89,10 +89,11 @@ void Socket::readPendingDatagrams() { auto packet = Packet::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, senderSockAddr); // call our verification operator to see if this packet is verified - if (_unfilteredSockAddrs.contains(senderSockAddr) || !_verifyPacketOperator || _verifyPacketOperator(*packet)) { - - // call the verified packet callback to let it handle this packet - return _verifiedPacketCallback(std::move(packet)); + if (_unfilteredSockAddrs.contains(senderSockAddr) || !_packetVerificationHandler || _packetVerificationHandler(*packet)) { + if (_verifiedPacketHandler) { + // call the verified packet callback to let it handle this packet + return _verifiedPacketHandler(std::move(packet)); + } } } } diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 2ebeb179bc..fcade487a3 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -24,8 +24,8 @@ namespace udt { -using VerifyPacketOperator = std::function; -using VerifiedPacketCallback = std::function)>; +using PacketVerificationHandler = std::function; +using VerifiedPacketHandler = std::function)>; class Socket : public QObject { Q_OBJECT @@ -43,9 +43,8 @@ public: void bind(const QHostAddress& address, quint16 port = 0) { _udpSocket.bind(address, port); } void rebind(); - void setVerifyPacketOperator(VerifyPacketOperator verifyPacketOperator) { _verifyPacketOperator = verifyPacketOperator; } - void setVerifiedPacketCallback(VerifiedPacketCallback verifiedPacketCallback) - { _verifiedPacketCallback = verifiedPacketCallback; } + void setPacketVerificationHandler(PacketVerificationHandler handler) { _packetVerificationHandler = handler; } + void setVerifiedPacketHandler(VerifiedPacketHandler handler) { _verifiedPacketHandler = handler; } void setBufferSizes(int numBytes); @@ -56,8 +55,8 @@ private slots: private: QUdpSocket _udpSocket { this }; - VerifyPacketOperator _verifyPacketOperator; - VerifiedPacketCallback _verifiedPacketCallback; + PacketVerificationHandler _packetVerificationHandler; + VerifiedPacketHandler _verifiedPacketHandler; QSet _unfilteredSockAddrs; }; From fb7130120ac2a25d0147778e6dde9c51c74d9d1d Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 10:16:26 -0700 Subject: [PATCH 13/21] another rename to Socket callbacks --- ice-server/src/IceServer.cpp | 4 ++-- libraries/networking/src/LimitedNodeList.cpp | 4 ++-- libraries/networking/src/udt/Socket.cpp | 6 +++--- libraries/networking/src/udt/Socket.h | 12 ++++++------ 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index c4dd2cf532..5da906a1f3 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -36,10 +36,10 @@ IceServer::IceServer(int argc, char* argv[]) : // set processPacket as the verified packet callback for the udt::Socket using std::placeholders::_1; - _serverSocket.setVerifiedPacketHandler(std::bind(&IceServer::processPacket, this, _1)); + _serverSocket.setPacketHandler(std::bind(&IceServer::processPacket, this, _1)); // set packetVersionMatch as the verify packet operator for the udt::Socket - _serverSocket.setPacketVerificationHandler(std::bind(&IceServer::packetVersionMatch, this, _1)); + _serverSocket.setPacketFilterOperator(std::bind(&IceServer::packetVersionMatch, this, _1)); // setup our timer to clear inactive peers QTimer* inactivePeerTimer = new QTimer(this); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index e6b0b27712..648fb7f2a0 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -99,10 +99,10 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short // set &PacketReceiver::handleVerifiedPacket as the verified packet callback for the udt::Socket using std::placeholders::_1; - _nodeSocket.setVerifiedPacketHandler(std::bind(&PacketReceiver::handleVerifiedPacket, _packetReceiver, _1)); + _nodeSocket.setPacketHandler(std::bind(&PacketReceiver::handleVerifiedPacket, _packetReceiver, _1)); // set our isPacketVerified method as the verify operator for the udt::Socket - _nodeSocket.setPacketVerificationHandler(std::bind(&LimitedNodeList::isPacketVerified, this, _1)); + _nodeSocket.setPacketFilterOperator(std::bind(&LimitedNodeList::isPacketVerified, this, _1)); _packetStatTimer.start(); diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 685689f4e6..ea4340065a 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -89,10 +89,10 @@ void Socket::readPendingDatagrams() { auto packet = Packet::fromReceivedPacket(std::move(buffer), packetSizeWithHeader, senderSockAddr); // call our verification operator to see if this packet is verified - if (_unfilteredSockAddrs.contains(senderSockAddr) || !_packetVerificationHandler || _packetVerificationHandler(*packet)) { - if (_verifiedPacketHandler) { + if (_unfilteredSockAddrs.contains(senderSockAddr) || !_packetFilterOperator || _packetFilterOperator(*packet)) { + if (_packetHandler) { // call the verified packet callback to let it handle this packet - return _verifiedPacketHandler(std::move(packet)); + return _packetHandler(std::move(packet)); } } } diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index fcade487a3..5043616dec 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -24,8 +24,8 @@ namespace udt { -using PacketVerificationHandler = std::function; -using VerifiedPacketHandler = std::function)>; +using PacketFilterOperator = std::function; +using PacketHandler = std::function)>; class Socket : public QObject { Q_OBJECT @@ -43,8 +43,8 @@ public: void bind(const QHostAddress& address, quint16 port = 0) { _udpSocket.bind(address, port); } void rebind(); - void setPacketVerificationHandler(PacketVerificationHandler handler) { _packetVerificationHandler = handler; } - void setVerifiedPacketHandler(VerifiedPacketHandler handler) { _verifiedPacketHandler = handler; } + void setPacketFilterOperator(PacketFilterOperator filterOperator) { _packetFilterOperator = filterOperator; } + void setPacketHandler(PacketHandler handler) { _packetHandler = handler; } void setBufferSizes(int numBytes); @@ -55,8 +55,8 @@ private slots: private: QUdpSocket _udpSocket { this }; - PacketVerificationHandler _packetVerificationHandler; - VerifiedPacketHandler _verifiedPacketHandler; + PacketFilterOperator _packetFilterOperator; + PacketHandler _packetHandler; QSet _unfilteredSockAddrs; }; From 1386aa57da4a1c5994e5aa9027b7b91e8ea065b7 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 11:02:45 -0700 Subject: [PATCH 14/21] repair to NLPacket creation from Packet and char* --- libraries/networking/src/LimitedNodeList.cpp | 2 +- libraries/networking/src/NLPacket.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 648fb7f2a0..1aee0e2da4 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -619,7 +619,7 @@ void LimitedNodeList::sendSTUNRequest() { flagTimeForConnectionStep(ConnectionStep::SendSTUNRequest); - _nodeSocket.writeDatagram(QByteArray::fromRawData(stunRequestPacket, sizeof(stunRequestPacket)), _stunSockAddr); + _nodeSocket.writeDatagram(stunRequestPacket, sizeof(stunRequestPacket), _stunSockAddr); } bool LimitedNodeList::processSTUNResponse(QSharedPointer packet) { diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index fdfb94f107..1af8dd0019 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -112,7 +112,7 @@ NLPacket& NLPacket::operator=(const NLPacket& other) { NLPacket::NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr) : Packet(std::move(data), size, senderSockAddr) { - adjustPayloadStartAndCapacity(); + adjustPayloadStartAndCapacity(_payloadSize > 0); readSourceID(); } @@ -148,7 +148,7 @@ QByteArray NLPacket::hashForPacketAndSecret(const udt::Packet& packet, const QUu int offset = packet.Packet::localHeaderSize() + NUM_BYTES_RFC4122_UUID + NUM_BYTES_MD5_HASH; // add the packet payload and the connection UUID - hash.addData(packet.getData(), packet.getDataSize() - offset); + hash.addData(packet.getData() + offset, packet.getDataSize() - offset); hash.addData(connectionSecret.toRfc4122()); // return the hash From 8bf507dd243876f20a2545d4538a3fe586d31c54 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 11:06:13 -0700 Subject: [PATCH 15/21] add a sanity check for data NLPacket ctor --- libraries/networking/src/NLPacket.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 1af8dd0019..ad90659ffe 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -112,6 +112,9 @@ NLPacket& NLPacket::operator=(const NLPacket& other) { NLPacket::NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr) : Packet(std::move(data), size, senderSockAddr) { + // sanity check before we decrease the payloadSize with the payloadCapacity + Q_ASSERT(_payloadSize == _payloadCapacity); + adjustPayloadStartAndCapacity(_payloadSize > 0); readSourceID(); From 3fbda89f1d77ba156308c5fa252eeeaa54e373ef Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 11:17:50 -0700 Subject: [PATCH 16/21] copy version in Packet move ctor/assignment --- interface/src/octree/OctreePacketProcessor.cpp | 4 ++-- libraries/networking/src/udt/Packet.cpp | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp index 7bb94323b7..386ebacc6d 100644 --- a/interface/src/octree/OctreePacketProcessor.cpp +++ b/interface/src/octree/OctreePacketProcessor.cpp @@ -77,8 +77,8 @@ void OctreePacketProcessor::processPacket(QSharedPointer packet, Share const QUuid& senderUUID = packet->getSourceID(); if (!versionDebugSuppressMap.contains(senderUUID, packetType)) { - qDebug() << "Packet version mismatch on" << packetType << "- Sender" - << senderUUID << "sent" << (int) packetType << "but" + qDebug() << "OctreePacketProcessor - piggyback packet version mismatch on" << packetType << "- Sender" + << senderUUID << "sent" << (int) packet->getVersion() << "but" << (int) versionForPacketType(packetType) << "expected."; emit packetVersionMismatch(); diff --git a/libraries/networking/src/udt/Packet.cpp b/libraries/networking/src/udt/Packet.cpp index 45c6a6f75f..91f9661b9d 100644 --- a/libraries/networking/src/udt/Packet.cpp +++ b/libraries/networking/src/udt/Packet.cpp @@ -104,6 +104,7 @@ Packet::Packet(const Packet& other) : Packet& Packet::operator=(const Packet& other) { _type = other._type; + _version = other._version; _packetSize = other._packetSize; _packet = std::unique_ptr(new char[_packetSize]); @@ -129,6 +130,7 @@ Packet::Packet(Packet&& other) { Packet& Packet::operator=(Packet&& other) { _type = other._type; + _version = other._version; _packetSize = other._packetSize; _packet = std::move(other._packet); From 12e684bea1ebf45811f3332002b793709ac3c2bb Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 11:21:31 -0700 Subject: [PATCH 17/21] fix identation for NLPacket move ctor --- libraries/networking/src/NLPacket.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index ad90659ffe..fb2561486a 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -121,7 +121,7 @@ NLPacket::NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& } NLPacket::NLPacket(NLPacket&& other) : - Packet(other) + Packet(other) { _sourceID = std::move(other._sourceID); } From 4a11bdc22e6862b291dd77d957253ffef95c66c9 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 11:26:28 -0700 Subject: [PATCH 18/21] indentation and spacing fixes --- libraries/networking/src/PacketReceiver.cpp | 2 +- libraries/networking/src/udt/Socket.h | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 502563489b..ce89b60af6 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -239,7 +239,7 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { _directConnectSetMutex.lock(); Qt::ConnectionType connectionType = - _directlyConnectedObjects.contains(listener.first) ? Qt::DirectConnection : Qt::AutoConnection; + _directlyConnectedObjects.contains(listener.first) ? Qt::DirectConnection : Qt::AutoConnection; _directConnectSetMutex.unlock(); diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 5043616dec..e8d6e5c6fa 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -61,7 +61,6 @@ private: QSet _unfilteredSockAddrs; }; -} - +} // namespace udt #endif // hifi_Socket_h From 023f3b81346dd9b9915b24d2aab27b3c523c38a9 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 11:34:30 -0700 Subject: [PATCH 19/21] fix sequence number read in PacketReceiver --- libraries/networking/src/PacketReceiver.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index ce89b60af6..5dfd35f70d 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -249,7 +249,7 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { // if this was a sequence numbered packet we should store the last seq number for // a packet of this type for this node if (SEQUENCE_NUMBERED_PACKETS.contains(nlPacket->getType())) { - matchingNode->setLastSequenceNumberForPacketType(packet->readSequenceNumber(), packet->getType()); + matchingNode->setLastSequenceNumberForPacketType(nlPacket->readSequenceNumber(), nlPacket->getType()); } emit dataReceived(matchingNode->getType(), nlPacket->getDataSize()); From 5bed3502ef16d3955d3167fc26f81cca482ed593 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 14:15:17 -0700 Subject: [PATCH 20/21] fix order of ctors and assignment operators --- libraries/networking/src/NLPacket.cpp | 16 ++++++++-------- libraries/networking/src/NLPacket.h | 3 ++- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index fb2561486a..39dc296f85 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -101,14 +101,6 @@ NLPacket::NLPacket(const NLPacket& other) : Packet(other) { _sourceID = other._sourceID; } -NLPacket& NLPacket::operator=(const NLPacket& other) { - Packet::operator=(other); - - _sourceID = other._sourceID; - - return *this; -} - NLPacket::NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr) : Packet(std::move(data), size, senderSockAddr) { @@ -126,6 +118,14 @@ NLPacket::NLPacket(NLPacket&& other) : _sourceID = std::move(other._sourceID); } +NLPacket& NLPacket::operator=(const NLPacket& other) { + Packet::operator=(other); + + _sourceID = other._sourceID; + + return *this; +} + NLPacket& NLPacket::operator=(NLPacket&& other) { Packet::operator=(std::move(other)); diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index aad6e6d072..57c099764a 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -51,8 +51,9 @@ protected: NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr); NLPacket(std::unique_ptr packet); NLPacket(const NLPacket& other); - NLPacket& operator=(const NLPacket& other); NLPacket(NLPacket&& other); + + NLPacket& operator=(const NLPacket& other); NLPacket& operator=(NLPacket&& other); void readSourceID(); From 2c965eee9101599fb390fa2391df8722ccb65e24 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 22 Jul 2015 14:42:44 -0700 Subject: [PATCH 21/21] fix ugly deref of matchingPeer --- ice-server/src/IceServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ice-server/src/IceServer.cpp b/ice-server/src/IceServer.cpp index 5da906a1f3..0545e0d274 100644 --- a/ice-server/src/IceServer.cpp +++ b/ice-server/src/IceServer.cpp @@ -89,7 +89,7 @@ void IceServer::processPacket(std::unique_ptr packet) { qDebug() << "Sending information for peer" << connectRequestID << "to peer" << senderUUID; // we have the peer they want to connect to - send them pack the information for that peer - sendPeerInformationPacket(*(matchingPeer.data()), &packet->getSenderSockAddr()); + sendPeerInformationPacket(*matchingPeer, &packet->getSenderSockAddr()); // we also need to send them to the active peer they are hoping to connect to // create a dummy peer object we can pass to sendPeerInformationPacket