From c71134c4ab9839ff55beb3c17a3b9d920a7be47d Mon Sep 17 00:00:00 2001 From: Sepalani Date: Sat, 3 Feb 2024 14:52:04 +0400 Subject: [PATCH 1/4] BBA/HLE: Check queue overrun --- Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp | 14 +++++++++++--- Source/Core/Core/HW/EXI/EXI_DeviceEthernet.h | 1 + 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp b/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp index 733053b6a6..47044c4c9e 100644 --- a/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp +++ b/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp @@ -126,6 +126,13 @@ void CEXIETHERNET::BuiltInBBAInterface::WriteToQueue(const std::vector& data const u8 next_write_index = (m_queue_write + 1) & 15; if (next_write_index != m_queue_read) m_queue_write = next_write_index; + else + WARN_LOG_FMT(SP1, "BBA queue overrun, data might be lost"); +} + +bool CEXIETHERNET::BuiltInBBAInterface::WillQueueOverrun() const +{ + return ((m_queue_write + 1) & 15) == m_queue_read; } void CEXIETHERNET::BuiltInBBAInterface::PollData(std::size_t* datasize) @@ -140,13 +147,14 @@ void CEXIETHERNET::BuiltInBBAInterface::PollData(std::size_t* datasize) { for (auto& tcp_buf : net_ref.tcp_buffers) { + if (WillQueueOverrun()) + break; if (!tcp_buf.used || (GetTickCountStd() - tcp_buf.tick) <= 1000) continue; - tcp_buf.tick = GetTickCountStd(); // Timed out packet, resend - if (((m_queue_write + 1) & 15) != m_queue_read) - WriteToQueue(tcp_buf.data); + tcp_buf.tick = GetTickCountStd(); + WriteToQueue(tcp_buf.data); } } diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.h b/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.h index 094487c9e9..e432056038 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.h +++ b/Source/Core/Core/HW/EXI/EXI_DeviceEthernet.h @@ -463,6 +463,7 @@ private: static void ReadThreadHandler(BuiltInBBAInterface* self); #endif void WriteToQueue(const std::vector& data); + bool WillQueueOverrun() const; void PollData(std::size_t* datasize); std::optional> TryGetDataFromSocket(StackRef* ref); From 05ed1a2faeb2da062a9900b059c8c3afb8057f61 Mon Sep 17 00:00:00 2001 From: Sepalani Date: Sat, 3 Feb 2024 15:14:01 +0400 Subject: [PATCH 2/4] BBA/HLE: Poll as much data as possible --- Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp | 32 ++++++++++++++++++------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp b/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp index 47044c4c9e..e29d1f9188 100644 --- a/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp +++ b/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp @@ -159,13 +159,27 @@ void CEXIETHERNET::BuiltInBBAInterface::PollData(std::size_t* datasize) } // Check for connection data - if (*datasize != 0) - continue; - const auto socket_data = TryGetDataFromSocket(&net_ref); - if (socket_data.has_value()) + if (*datasize == 0) { - *datasize = socket_data->size(); - std::memcpy(m_eth_ref->mRecvBuffer.get(), socket_data->data(), *datasize); + // Send it to the network buffer if empty + const auto socket_data = TryGetDataFromSocket(&net_ref); + if (socket_data.has_value()) + { + *datasize = socket_data->size(); + std::memcpy(m_eth_ref->mRecvBuffer.get(), socket_data->data(), *datasize); + } + } + else if (!WillQueueOverrun()) + { + // Otherwise, enqueue it + const auto socket_data = TryGetDataFromSocket(&net_ref); + if (socket_data.has_value()) + WriteToQueue(*socket_data); + } + else + { + WARN_LOG_FMT(SP1, "BBA queue might overrun, can't poll more data"); + return; } } } @@ -232,7 +246,7 @@ void CEXIETHERNET::BuiltInBBAInterface::HandleDHCP(const Common::UDPPacket& pack std::optional> CEXIETHERNET::BuiltInBBAInterface::TryGetDataFromSocket(StackRef* ref) { - size_t datasize = 0; // Set by socket.receive using a non-const reference + std::size_t datasize = 0; // Set by socket.receive using a non-const reference unsigned short remote_port; switch (ref->type) @@ -380,7 +394,7 @@ void CEXIETHERNET::BuiltInBBAInterface::HandleTCPFrame(const Common::TCPPacket& { // only if contain data if (static_cast(this_seq - ref->ack_num) >= 0 && - data.size() >= static_cast(size)) + data.size() >= static_cast(size)) { ref->tcp_socket.send(data.data(), size); ref->ack_num += size; @@ -671,7 +685,7 @@ void CEXIETHERNET::BuiltInBBAInterface::ReadThreadHandler(CEXIETHERNET::BuiltInB if (!self->m_read_enabled.IsSet()) continue; - size_t datasize = 0; + std::size_t datasize = 0; u8 wp = self->m_eth_ref->page_ptr(BBA_RWP); const u8 rp = self->m_eth_ref->page_ptr(BBA_RRP); From 73aa8c50a28070e9617fbcfe48229fb801fd1864 Mon Sep 17 00:00:00 2001 From: Sepalani Date: Sat, 3 Feb 2024 15:20:29 +0400 Subject: [PATCH 3/4] BBA/HLE: Sleep when no data is received --- Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp b/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp index e29d1f9188..67c60d35d5 100644 --- a/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp +++ b/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp @@ -678,14 +678,17 @@ bool CEXIETHERNET::BuiltInBBAInterface::SendFrame(const u8* frame, u32 size) void CEXIETHERNET::BuiltInBBAInterface::ReadThreadHandler(CEXIETHERNET::BuiltInBBAInterface* self) { + std::size_t datasize = 0; while (!self->m_read_thread_shutdown.IsSet()) { - // make thread less cpu hungry - std::this_thread::sleep_for(std::chrono::milliseconds(1)); + if (datasize == 0) + { + // Make thread less CPU hungry + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } if (!self->m_read_enabled.IsSet()) continue; - std::size_t datasize = 0; u8 wp = self->m_eth_ref->page_ptr(BBA_RWP); const u8 rp = self->m_eth_ref->page_ptr(BBA_RRP); @@ -711,6 +714,10 @@ void CEXIETHERNET::BuiltInBBAInterface::ReadThreadHandler(CEXIETHERNET::BuiltInB self->m_queue_read++; self->m_queue_read &= 15; } + else + { + datasize = 0; + } // Check network stack references self->PollData(&datasize); From c0fd59adfd3b823c202c76c8faf1cd7fab57250e Mon Sep 17 00:00:00 2001 From: Sepalani Date: Sat, 3 Feb 2024 21:06:08 +0400 Subject: [PATCH 4/4] BBA/HLE: Handle connection error --- Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp | 26 +++++++++++++++++++------ Source/Core/Core/HW/EXI/BBA/BuiltIn.h | 6 +++--- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp b/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp index 67c60d35d5..802f64813a 100644 --- a/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp +++ b/Source/Core/Core/HW/EXI/BBA/BuiltIn.cpp @@ -269,8 +269,24 @@ CEXIETHERNET::BuiltInBBAInterface::TryGetDataFromSocket(StackRef* ref) } case IPPROTO_TCP: - if (!ref->tcp_socket.Connected(ref)) + switch (ref->tcp_socket.Connected(ref)) + { + case BbaTcpSocket::ConnectingState::Error: + { + // Create the resulting RST ACK packet + const Common::TCPPacket result(ref->bba_mac, ref->my_mac, ref->from, ref->to, ref->seq_num, + ref->ack_num, TCP_FLAG_RST | TCP_FLAG_ACK); + WriteToQueue(result.Build()); + ref->ip = 0; + ref->tcp_socket.disconnect(); + [[fallthrough]]; + } + case BbaTcpSocket::ConnectingState::None: + case BbaTcpSocket::ConnectingState::Connecting: return std::nullopt; + case BbaTcpSocket::ConnectingState::Connected: + break; + } sf::Socket::Status st = sf::Socket::Status::Done; TcpBuffer* tcp_buffer = nullptr; @@ -804,14 +820,11 @@ sf::Socket::Status BbaTcpSocket::GetSockName(sockaddr_in* addr) const return sf::Socket::Status::Done; } -bool BbaTcpSocket::Connected(StackRef* ref) +BbaTcpSocket::ConnectingState BbaTcpSocket::Connected(StackRef* ref) { // Called by ReadThreadHandler's TryGetDataFromSocket - // TODO: properly handle error state switch (m_connecting_state) { - case ConnectingState::Connected: - return true; case ConnectingState::Connecting: { const int fd = getHandle(); @@ -841,6 +854,7 @@ bool BbaTcpSocket::Connected(StackRef* ref) if (getsockopt(fd, SOL_SOCKET, SO_ERROR, reinterpret_cast(&error), &len) != 0) { ERROR_LOG_FMT(SP1, "Failed to get BBA socket error state: {}", Common::StrNetworkError()); + m_connecting_state = ConnectingState::Error; break; } @@ -885,7 +899,7 @@ bool BbaTcpSocket::Connected(StackRef* ref) default: break; } - return false; + return m_connecting_state; } BbaUdpSocket::BbaUdpSocket() = default; diff --git a/Source/Core/Core/HW/EXI/BBA/BuiltIn.h b/Source/Core/Core/HW/EXI/BBA/BuiltIn.h index 5cef003450..3901cbe038 100644 --- a/Source/Core/Core/HW/EXI/BBA/BuiltIn.h +++ b/Source/Core/Core/HW/EXI/BBA/BuiltIn.h @@ -48,9 +48,6 @@ public: sf::Socket::Status GetPeerName(sockaddr_in* addr) const; sf::Socket::Status GetSockName(sockaddr_in* addr) const; - bool Connected(StackRef* ref); - -private: enum class ConnectingState { None, @@ -59,6 +56,9 @@ private: Error }; + ConnectingState Connected(StackRef* ref); + +private: ConnectingState m_connecting_state = ConnectingState::None; };