From 54782cbf70cf988fa2e3bed4217b5f670824729f Mon Sep 17 00:00:00 2001 From: TheLastRar Date: Fri, 12 Jul 2024 18:21:47 +0100 Subject: [PATCH] DEV9: Amend ICMP_Session comments --- .../Sessions/ICMP_Session/ICMP_Session.cpp | 217 ++++++++++-------- .../DEV9/Sessions/ICMP_Session/ICMP_Session.h | 4 +- 2 files changed, 122 insertions(+), 99 deletions(-) diff --git a/pcsx2/DEV9/Sessions/ICMP_Session/ICMP_Session.cpp b/pcsx2/DEV9/Sessions/ICMP_Session/ICMP_Session.cpp index 6195177fd1..f2f67e7153 100644 --- a/pcsx2/DEV9/Sessions/ICMP_Session/ICMP_Session.cpp +++ b/pcsx2/DEV9/Sessions/ICMP_Session/ICMP_Session.cpp @@ -36,36 +36,38 @@ using namespace PacketReader::IP::ICMP; using namespace std::chrono_literals; -/* Ping is kindof annoying to do crossplatform - All platforms restrict raw sockets - - Windows provides an api for ICMP - ICMP_ECHO_REPLY should always be used, ignore ICMP_ECHO_REPLY32 - IP_OPTION_INFORMATION should always be used, ignore IP_OPTION_INFORMATION32 - - Linux - We have access to raw sockets via CAP_NET_RAW (for pcap) - However we may be missing that cap on some builds - Linux has socket(PF_INET, SOCK_DGRAM, IPPROTO_ICMP), used similar to raw sockets but for ICMP only - Auto filters responses - Requires net.ipv4.ping_group_range sysctl, default off on a lot of distros - Timeouts reported via sock_extended_err control messages (with IP_RECVERR socket option set) - - Mac - Raw sockets restricted - Mac has socket(PF_INET, SOCK_DGRAM, IPPROTO_ICMP) - No restriction to using it - Implementation differs, is more versatile than linux - Does not auto filter responses - Timeouts reported as a normal packet - - FreeBSD - Raw sockets restricted - No unprivilaged ICMP sockets - Timeouts reported as a normal packet?? - - Ping cli - Present for all platforms, but command args differ +/* + * Ping is kindof annoying to do crossplatform + * All platforms restrict raw sockets + * + * Windows provides an api for ICMP + * ICMP_ECHO_REPLY should always be used, ignore ICMP_ECHO_REPLY32 + * IP_OPTION_INFORMATION should always be used, ignore IP_OPTION_INFORMATION32 + * + * Linux + * We have access to raw sockets via CAP_NET_RAW (for pcap) + * However we may be missing that cap on some builds + * Also hava socket(PF_INET, SOCK_DGRAM, IPPROTO_ICMP), used similarly to raw sockets, but for ICMP only + * Auto filters responses + * Requires net.ipv4.ping_group_range sysctl, default off on a lot of distros + * Timeouts reported via sock_extended_err control messages (with IP_RECVERR socket option set) + * + * Mac + * Raw sockets restricted + * Mac has socket(PF_INET, SOCK_DGRAM, IPPROTO_ICMP) + * No restriction to using it with ICMP_ECHO + * Implementation differs, is more versatile than linux + * Does not auto filter responses + * Timeouts reported as a normal packet + * + * FreeBSD + * Raw sockets restricted + * No unprivilaged ICMP sockets + * Timeouts reported as a normal packet?? + * + * Ping cli + * Present for all platforms, but command args differ + * Not used here */ namespace Sessions @@ -96,28 +98,34 @@ namespace Sessions return; } - //Allocate return buffer - //Documentation says + 8 to allow for an ICMP error message - //In testing, ICMP_ECHO_REPLY structure itself was returned with data set to null + /* + * Allocate response buffer + * Documentation says + 8 to allow for an ICMP error message + * In testing, ICMP_ECHO_REPLY structure itself was returned with data set to null + */ icmpResponseBufferLen = sizeof(ICMP_ECHO_REPLY) + requestSize + 8; icmpResponseBuffer = std::make_unique(icmpResponseBufferLen); #elif defined(__POSIX__) { + /* + * Allocate response buffer + * Size needed depends on which socket protocol (ICMP or raw) we use aswell as os + */ switch (icmpConnectionKind) { - //Two different methods for raw/icmp sockets between the unix OSes - //Play it safe and only enable when we know which of the two methods we use + // Two different methods for raw/icmp sockets between the Unix OSes + // Play it safe and only enable when we know which of the two methods we use #if defined(ICMP_SOCKETS_LINUX) || defined(ICMP_SOCKETS_BSD) case (PingType::ICMP): icmpSocket = socket(AF_INET, SOCK_DGRAM, IPPROTO_ICMP); if (icmpSocket != -1) { #if defined(ICMP_SOCKETS_LINUX) - //Space for ICMP header, as MSG_ERRQUEUE returns what we sent - //An extra +8 required sometimes, for some reason? + // Only need space for ICMP header, as MSG_ERRQUEUE returns data we sent + // Testing found an extra + 8 was required sometimes, for some reason? icmpResponseBufferLen = 8 + requestSize + 8; #elif defined(ICMP_SOCKETS_BSD) - //Returned IP Header, ICMP Header & either data or failed ICMP packet + // Returned IP Header, ICMP Header & either data or failed ICMP packet icmpResponseBufferLen = 20 + 8 + std::max(20 + 8, requestSize); #endif break; @@ -133,10 +141,10 @@ namespace Sessions if (icmpSocket != -1) { #if defined(ICMP_SOCKETS_LINUX) - //We get packet + header + // We get IP packet + ICMP header icmpResponseBufferLen = 20 + 8 + requestSize; #elif defined(ICMP_SOCKETS_BSD) - //As above, but we will also directly receive error ICMP messages + // As above, but we will also directly receive error ICMP messages icmpResponseBufferLen = 20 + 8 + std::max(20 + 8, requestSize); #endif break; @@ -171,21 +179,21 @@ namespace Sessions #endif } - //Note, we can finish reading but have no data + // Returned PingResult.data is only valid when PingResult.type is 0 ICMP_Session::PingResult* ICMP_Session::Ping::Recv() { #ifdef _WIN32 if (WaitForSingleObject(icmpEvent, 0) == WAIT_OBJECT_0) { ResetEvent(icmpEvent); - //Prep buffer for reasing + [[maybe_unused]] int count = IcmpParseReplies(icmpResponseBuffer.get(), icmpResponseBufferLen); pxAssert(count == 1); // Rely on implicit object creation ICMP_ECHO_REPLY* pingRet = reinterpret_cast(icmpResponseBuffer.get()); - //Map status to ICMP type/code + // Map status to ICMP type/code switch (pingRet->Status) { case (IP_SUCCESS): @@ -212,21 +220,23 @@ namespace Sessions result.type = 3; result.code = 4; break; - case (IP_BAD_ROUTE): //Bad source route + case (IP_BAD_ROUTE): // Bad source route result.type = 3; result.code = 5; break; case (IP_BAD_DESTINATION): - //I think this could be either - //Destination network unknown - //or - //Destination host unknown - //Use host unknown + /* + * I think this could be mapped to either + * Destination network unknown + * or + * Destination host unknown + * Lets map to host unknown + */ result.type = 3; result.code = 7; break; case (IP_REQ_TIMED_OUT): - //Return nothing + // Return nothing result.type = -2; result.code = 0; break; @@ -243,7 +253,7 @@ namespace Sessions result.code = 0; break; - //Unexpected Errors + // Unexpected errors case (IP_BUF_TOO_SMALL): case (IP_NO_RESOURCES): case (IP_BAD_OPTION): @@ -326,9 +336,9 @@ namespace Sessions iov.iov_len = icmpResponseBufferLen; #if defined(ICMP_SOCKETS_LINUX) - //Needs to hold cmsghdr + sock_extended_err + sockaddr_in - //for ICMP error responses (total 44 bytes) - //Unknown for other types of error + // Needs to hold cmsghdr + sock_extended_err + sockaddr_in + // for ICMP error responses, this is a total of 44 bytes + // Unknown size needed for other error types std::byte cbuff[64]{}; #endif @@ -341,6 +351,10 @@ namespace Sessions #if defined(ICMP_SOCKETS_LINUX) ret = recvmsg(icmpSocket, &msg, MSG_DONTWAIT); #elif defined(ICMP_SOCKETS_BSD) + // Why not use MSG_DONTWAIT? + // We can get rid of the select logic above if we do use it + // Might be doing it for extra error checking?? + // maybe we can just make the socket non-blocking ret = recvmsg(icmpSocket, &msg, 0); #endif @@ -381,14 +395,17 @@ namespace Sessions if (msg.msg_flags & MSG_CTRUNC) Console.Error("DEV9: ICMP: RecvMsg Control Truncated"); + // On Linux, ICMP errors are stored in control messages retrieved using MSG_ERRQUEUE sock_extended_err* exErrorPtr = nullptr; cmsghdr* cmsg; - /* Receive auxiliary data in msgh */ + // Search though control messages, taking the latest mesage + // We should only have at most 1 message for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; cmsg = CMSG_NXTHDR(&msg, cmsg)) { if (cmsg->cmsg_level == SOL_IP && cmsg->cmsg_type == IP_RECVERR) { + pxAssert(!exErrorPtr); exErrorPtr = reinterpret_cast(CMSG_DATA(cmsg)); continue; } @@ -404,6 +421,7 @@ namespace Sessions sock_extended_err exError; std::memcpy(&exError, exErrorPtr, sizeof(exError)); + // Process the error if (exError.ee_origin == SO_EE_ORIGIN_ICMP) { result.type = exError.ee_type; @@ -449,9 +467,9 @@ namespace Sessions offset = headerLength; #ifdef __APPLE__ - //Apple (old BSD)'s raw IP sockets implementation converts the ip_len field to host byte order - //and additionally subtracts the header length. - //https://www.unix.com/man-page/mojave/4/ip/ + // Apple (old BSD)'s raw IP sockets implementation converts the ip_len field to host byte order + // and additionally subtracts the header length. + // https://www.unix.com/man-page/mojave/4/ip/ length = ipHeader->ip_len; #else length = ntohs(ipHeader->ip_len) - headerLength; @@ -469,7 +487,7 @@ namespace Sessions if (icmp.type == 0) { - //Check if response is to us + // Check if response is for us if (icmpConnectionKind == PingType::RAW) { ICMP_HeaderDataIdentifier headerData(icmp.headerData); @@ -477,26 +495,28 @@ namespace Sessions return nullptr; } - //While icmp (and its PayloadPtr) will be destroyed when leaving this function - //the data pointed to it persist in icmpResponseBuffer + // While icmp (and its PayloadPtr) will be destroyed when leaving this function + // the data it points to persists in icmpResponseBuffer result.dataLength = icmpPayload->GetLength(); result.data = icmpPayload->data; return &result; } #if defined(ICMP_SOCKETS_BSD) + // On BSD/Mac, ICMP errors are returned as normal packets else if (icmp.type == 3 || icmp.type == 4 || icmp.type == 5 || icmp.type == 11) { - //Check if response is to us - //We need to extract the sent header + // Extract the packet the ICMP message is responding to IP_Packet ipPacket(icmpPayload->data, icmpPayload->GetLength(), true); IP_PayloadPtr* ipPayload = static_cast(ipPacket.GetPayload()); + // TODO, validate proto? ICMP_Packet retIcmp(ipPayload->data, ipPayload->GetLength()); + // Check if response is for us ICMP_HeaderDataIdentifier headerData(icmp.headerData); if (headerData.identifier != icmpId) return nullptr; - //This response is for us + // This response is for us return &result; } #endif @@ -506,7 +526,7 @@ namespace Sessions Console.Error("DEV9: ICMP: Unexpected packet"); pxAssert(false); #endif - //Assume not for us + // Assume not for us return nullptr; } } @@ -522,7 +542,7 @@ namespace Sessions bool ICMP_Session::Ping::Send(IP_Address parAdapterIP, IP_Address parDestIP, int parTimeToLive, PayloadPtr* parPayload) { #ifdef _WIN32 - //Documentation is incorrect, IP_OPTION_INFORMATION is to be used regardless of platform + // Documentation is incorrect, IP_OPTION_INFORMATION is to be used regardless of platform IP_OPTION_INFORMATION ipInfo{}; ipInfo.Ttl = parTimeToLive; DWORD ret; @@ -533,8 +553,8 @@ namespace Sessions ret = IcmpSendEcho2(icmpFile, icmpEvent, nullptr, nullptr, parDestIP.integer, parPayload->data, parPayload->GetLength(), &ipInfo, icmpResponseBuffer.get(), icmpResponseBufferLen, static_cast(std::chrono::duration_cast(ICMP_TIMEOUT).count())); - //Documentation states that IcmpSendEcho2 returns ERROR_IO_PENDING - //However, it actually returns zero, with the error set to ERROR_IO_PENDING + // Documentation states that IcmpSendEcho2 returns ERROR_IO_PENDING + // However, it actually returns zero, with the error set to ERROR_IO_PENDING if (ret == 0) ret = GetLastError(); @@ -553,8 +573,8 @@ namespace Sessions { icmpDeathClockStart = std::chrono::steady_clock::now(); - //broadcast and multicast mignt need extra setsockopts - //I don't think any game will broadcast/multicast ping + // Broadcast and multicast might need extra setsockopts calls + // I don't think any game will do a broadcast/multicast ping if (parAdapterIP.integer != 0) { @@ -594,7 +614,7 @@ namespace Sessions #if defined(ICMP_SOCKETS_LINUX) if (icmpConnectionKind == PingType::ICMP) { - //We get assigned a port + // We get assigned a port/Id sockaddr_in endpoint{}; socklen_t endpointsize = sizeof(endpoint); if (getsockname(icmpSocket, reinterpret_cast(&endpoint), &endpointsize) == -1) @@ -610,7 +630,7 @@ namespace Sessions else #endif { - //Use time, in ms, as id + // Use time, in ms, as id icmpId = static_cast(std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count()); } @@ -618,7 +638,7 @@ namespace Sessions icmp.type = 8; icmp.code = 0; - //We only send one icmp packet per identifier + // We only send one icmp packet per identifier ICMP_HeaderDataIdentifier headerData(icmpId, 1); headerData.WriteHeaderData(icmp.headerData); @@ -689,11 +709,11 @@ namespace Sessions if (pingRet != nullptr) { Ping* ping = pings[i]; - //Remove ping from list and unlock + // Remove ping from list and unlock mutex pings.erase(pings.begin() + i); lock.unlock(); - //Create return ICMP packet + // Create return ICMP packet std::optional ret; if (pingRet->type >= 0) { @@ -705,21 +725,21 @@ namespace Sessions } else { - //We will copy the original packet back here - //Allocate fullsize buffer + // Copy the original packet into the returned ICMP packet + // Allocate fullsize buffer u8* temp = new u8[ping->originalPacket->GetLength()]; - //Allocate data + // Allocate returned ICMP payload int responseSize = ping->originalPacket->GetHeaderLength() + 8; data = new PayloadData(responseSize); - //Write packet back into bytes + // Write packet into buffer int offset = 0; ping->originalPacket->WriteBytes(temp, &offset); - //Copy only needed bytes + // Copy only needed bytes memcpy(data->data.get(), temp, responseSize); - //cleanup + // Cleanup delete[] temp; } @@ -735,7 +755,7 @@ namespace Sessions else DevCon.WriteLn("DEV9: ICMP: ICMP timeout"); - //free ping + // Free ping delete ping; if (--open == 0) @@ -744,7 +764,7 @@ namespace Sessions if (ret.has_value()) DevCon.WriteLn("DEV9: ICMP: Return Ping"); - //Return packet + // Return packet return ret; } } @@ -760,7 +780,7 @@ namespace Sessions return false; } - //Note, expects caller to set ipTimeToLive before calling + // Expects caller to set ipTimeToLive before calling bool ICMP_Session::Send(PacketReader::IP::IP_Payload* payload, IP_Packet* packet) { IP_PayloadPtr* ipPayload = static_cast(payload); @@ -770,16 +790,19 @@ namespace Sessions switch (icmp.type) { - case 3: //Port Closed + case 3: // Port Closed switch (icmp.code) { case 3: { Console.Error("DEV9: ICMP: Received Packet Rejected, Port Closed"); - //RE:Outbreak Hackfix - //TODO, check if still needed - + /* + * RE:Outbreak Hackfix + * ICMP port closed messages has an extra 4 bytes of padding before the packet copy + * this can be tested by trying to connect without using the resurrection server DNS + * turbo mode may be needed to trigger the bug, depending on the DNS server's latency + */ std::unique_ptr retPkt; if ((icmpPayload->data[0] & 0xF0) == (4 << 4)) retPkt = std::make_unique(icmpPayload->data, icmpPayload->GetLength(), true); @@ -803,12 +826,12 @@ namespace Sessions { case static_cast(IP_Type::TCP): case static_cast(IP_Type::UDP): - //Read ports directly from the payload - //both UDP and TCP have the same locations for ports + // Read ports directly from the payload + // both UDP and TCP have the same locations for ports IP_PayloadPtr* payload = static_cast(retPkt->GetPayload()); int offset = 0; - NetLib::ReadUInt16(payload->data, &offset, &srvPort); //src - NetLib::ReadUInt16(payload->data, &offset, &ps2Port); //dst + NetLib::ReadUInt16(payload->data, &offset, &srvPort); // src + NetLib::ReadUInt16(payload->data, &offset, &ps2Port); // dst } ConnectionKey Key{}; @@ -817,7 +840,7 @@ namespace Sessions Key.ps2Port = ps2Port; Key.srvPort = srvPort; - //is from Normal Port? + // Is from Normal Port? BaseSession* s = nullptr; connections->TryGetValue(Key, &s); @@ -828,7 +851,7 @@ namespace Sessions break; } - //Is from Fixed Port? + // Is from Fixed Port? Key.ip = {}; Key.srvPort = 0; connections->TryGetValue(Key, &s); @@ -846,7 +869,7 @@ namespace Sessions Console.Error("DEV9: ICMP: Unsupported ICMP Code For Destination Unreachable %d", icmp.code); } break; - case 8: //Echo + case 8: // Echo { DevCon.WriteLn("DEV9: ICMP: Send Ping"); open++; @@ -871,7 +894,7 @@ namespace Sessions memcpy(ping->headerData, icmp.headerData, 4); - //Need to copy IP_Packet, original is stack allocated + // Need to copy IP_Packet, original is stack allocated ping->originalPacket = std::make_unique(*packet); { @@ -897,7 +920,7 @@ namespace Sessions { std::scoped_lock lock(ping_mutex); - //Cleanup + // Cleanup for (size_t i = 0; i < pings.size(); i++) delete pings[i]; } diff --git a/pcsx2/DEV9/Sessions/ICMP_Session/ICMP_Session.h b/pcsx2/DEV9/Sessions/ICMP_Session/ICMP_Session.h index 9f0d39114e..e827c3b4af 100644 --- a/pcsx2/DEV9/Sessions/ICMP_Session/ICMP_Session.h +++ b/pcsx2/DEV9/Sessions/ICMP_Session/ICMP_Session.h @@ -47,14 +47,14 @@ namespace Sessions static PingType icmpConnectionKind; - //Sockets + // Sockets int icmpSocket{-1}; std::chrono::steady_clock::time_point icmpDeathClockStart; u16 icmpId; #endif - //Return buffers + // Return buffers PingResult result{}; int icmpResponseBufferLen{0}; std::unique_ptr icmpResponseBuffer;