fix: Fixes send-path backpressure: consume IORingGroup 1.0.8, stop dropping packets silently (#2551)
## Summary Two related fixes on the outbound path: 1. Consume **IORingGroup 1.0.8**, which allows more than one send in flight per socket, and expose the two settings that go with it. 2. Stop `NetState.Send` silently discarding packets when the send buffer fills — including an out-of-bounds write reachable in that state. ## 1. Send-path stall (RIO) RIO reports send completion on **acknowledgement**, not on copy, so a completion cannot arrive sooner than one round trip. With one send in flight, `PostSend` refused to post again until the previous completion arrived — capping a connection at **one send per RTT** whenever it had data queued. Measured on a 50ms-RTT production shard: | | before | after | |---|---|---| | in-game latency, data flowing | **101–146 ms** | **48–51 ms** | | p95 | ~135 ms | 52.8 ms | | samples > 70 ms | 20 | **0** | The control that confirms the mechanism: server-side post→completion was **unchanged** at median 92ms across both runs. The ACK-binding is inherent to RIO and did not move; only its propagation into application latency did. Two things worth recording, because they explain why this went unnoticed: - As little as **6 bytes** of queued data held the gate shut, so it reproduced in empty areas, not just crowded ones. - The same measurement at loopback RTT is **microseconds**, so local testing could never surface it. New settings, both restart-time: - **`network.maxOutstandingSends`** (default 32) — sends in flight per connection. Honoured by RIO only; other backends complete sends on copy and report 1. Costs a request-queue and completion-queue slot per send, **not another buffer**, since every outstanding send addresses a different range of the same registered buffer. Worst-case added latency is roughly `completion RTT / value`. - **`network.sendBufferSize`** (default 256KB) — per-connection send buffer, coerced to a power of two of at least the platform allocation granularity. This is the lever for the disconnects below, and the per-connection memory ceiling. ## 2. Send buffer full `NetState.Send` had three failure modes once the buffer filled, none of them visible: | writable | behaviour | |---|---| | `0` | `GetSendBuffer` returned false → **packet dropped**, no log, no disconnect | | `4 … needed-1` | `Compress` returned 0 → `CommitWrite(0)` → **packet dropped** the same way | | `1 … 3` | `safeOutputLength = (nuint)output.Length - 4` **underflows** → hot-loop bounds check never trips → **writes past the span** | The first two leave a client connected while quietly missing game state, which is undiagnosable from either end. The third corrupts the in-flight region of the ring buffer, and is reachable precisely when a connection is congested, since callers only check for non-zero space. `Compress` now refuses an output too small to bound, and `Send` reports exhaustion instead of dropping — logging and disconnecting with **needed / writable / unacked / capacity**. Those numbers separate a slow client holding the buffer from a buffer genuinely too small for the shard, which is the case that warrants raising `network.sendBufferSize`. ## Testing `NetworkCompressionBoundsTests` covers the underflow using sentinel bytes around the output window. **Verified to fail without the guard** (4 failures from overwritten sentinels), confirming the out-of-bounds writes were real rather than theoretical. Full suites green: **788 Server.Tests**, **597 UOContent.Tests**, Release build clean against the published 1.0.8. ## Notes for reviewers - Upstream change: modernuo/IORingGroup#9. - The buffer-full path is now *loud* where it used to be silent. If a shard has been quietly dropping packets under load, this will surface as disconnects — that is the intended outcome, and the log line says which setting to raise. - Follow-up under discussion: promoting a connection to a larger buffer instead of disconnecting, which looks feasible on a live connection since buffers are referenced per-operation rather than bound to the request queue.
This commit is contained in:
parent
c909ed1f2f
commit
294dcd94a0
5 changed files with 186 additions and 9 deletions
|
|
@ -0,0 +1,86 @@
|
||||||
|
using System;
|
||||||
|
using Server.Network;
|
||||||
|
using Xunit;
|
||||||
|
|
||||||
|
namespace Server.Tests.Network;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Bounds behaviour of the Huffman compressor when the destination is too small.
|
||||||
|
///
|
||||||
|
/// This is reachable in production: NetState only checks that the send buffer has *some* writable
|
||||||
|
/// space before handing the remainder to Compress, so a nearly-full buffer can offer a span of one
|
||||||
|
/// to three bytes. The internal guard is computed as an unsigned <c>output.Length - 4</c>, which
|
||||||
|
/// underflows for those sizes and stops bounding the writes at all.
|
||||||
|
/// </summary>
|
||||||
|
public class NetworkCompressionBoundsTests
|
||||||
|
{
|
||||||
|
[Theory]
|
||||||
|
[InlineData(0)]
|
||||||
|
[InlineData(1)]
|
||||||
|
[InlineData(2)]
|
||||||
|
[InlineData(3)]
|
||||||
|
public void RefusesOutputTooSmallToBound(int outputSize)
|
||||||
|
{
|
||||||
|
var input = new byte[64];
|
||||||
|
Array.Fill(input, (byte)'A');
|
||||||
|
|
||||||
|
// Sentinel-filled backing array; only the middle window is offered to the compressor, so
|
||||||
|
// any write past the span shows up as a modified sentinel rather than silent corruption.
|
||||||
|
var backing = new byte[256];
|
||||||
|
Array.Fill(backing, (byte)0xCC);
|
||||||
|
|
||||||
|
const int windowStart = 64;
|
||||||
|
var output = backing.AsSpan(windowStart, outputSize);
|
||||||
|
|
||||||
|
var written = NetworkCompression.Compress(input, output);
|
||||||
|
|
||||||
|
Assert.Equal(0, written);
|
||||||
|
|
||||||
|
for (var i = 0; i < backing.Length; i++)
|
||||||
|
{
|
||||||
|
Assert.Equal(0xCC, backing[i]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void StillCompressesWhenOutputIsLargeEnough()
|
||||||
|
{
|
||||||
|
var input = new byte[64];
|
||||||
|
Array.Fill(input, (byte)'A');
|
||||||
|
|
||||||
|
var output = new byte[256];
|
||||||
|
|
||||||
|
var written = NetworkCompression.Compress(input, output);
|
||||||
|
|
||||||
|
Assert.True(written > 0);
|
||||||
|
Assert.True(written <= output.Length);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ReportsFailureRatherThanOverrunningATightOutput()
|
||||||
|
{
|
||||||
|
// Large input against a small-but-bounded output: the guard is well-defined here, so this
|
||||||
|
// must fail cleanly rather than write past the end.
|
||||||
|
var input = new byte[4096];
|
||||||
|
Array.Fill(input, (byte)'A');
|
||||||
|
|
||||||
|
var backing = new byte[256];
|
||||||
|
Array.Fill(backing, (byte)0xCC);
|
||||||
|
|
||||||
|
const int windowStart = 64;
|
||||||
|
const int windowSize = 16;
|
||||||
|
var output = backing.AsSpan(windowStart, windowSize);
|
||||||
|
|
||||||
|
NetworkCompression.Compress(input, output);
|
||||||
|
|
||||||
|
for (var i = 0; i < windowStart; i++)
|
||||||
|
{
|
||||||
|
Assert.Equal(0xCC, backing[i]);
|
||||||
|
}
|
||||||
|
|
||||||
|
for (var i = windowStart + windowSize; i < backing.Length; i++)
|
||||||
|
{
|
||||||
|
Assert.Equal(0xCC, backing[i]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -19,6 +19,7 @@ using System.Linq;
|
||||||
using System.Net;
|
using System.Net;
|
||||||
using System.Net.NetworkInformation;
|
using System.Net.NetworkInformation;
|
||||||
using System.Network;
|
using System.Network;
|
||||||
|
using System.Numerics;
|
||||||
|
|
||||||
namespace Server.Network;
|
namespace Server.Network;
|
||||||
|
|
||||||
|
|
@ -28,9 +29,10 @@ namespace Server.Network;
|
||||||
public partial class NetState
|
public partial class NetState
|
||||||
{
|
{
|
||||||
// Buffer sizes
|
// Buffer sizes
|
||||||
private const int RecvBufferSize = 1024 * 64; // 64KB recv buffers
|
private const int RecvBufferSize = 1024 * 64; // 64KB recv buffers
|
||||||
private const int SendBufferSize = 1024 * 256; // 256KB send buffers
|
private const int DefaultSendBufferSize = 1024 * 256; // 256KB send buffers
|
||||||
private const int MaxConnections = 4096; // Max concurrent connections
|
private const int MinSendBufferSize = 1024 * 64; // Platform allocation granularity
|
||||||
|
private const int MaxConnections = 4096; // Max concurrent connections
|
||||||
|
|
||||||
private static readonly Queue<NetState> _disposed = [];
|
private static readonly Queue<NetState> _disposed = [];
|
||||||
private static readonly TimeSpan ConnectingSocketIdleLimit = TimeSpan.FromMilliseconds(5000); // 5 seconds
|
private static readonly TimeSpan ConnectingSocketIdleLimit = TimeSpan.FromMilliseconds(5000); // 5 seconds
|
||||||
|
|
@ -41,7 +43,9 @@ public partial class NetState
|
||||||
// NetState storage indexed by RingSocket.Id
|
// NetState storage indexed by RingSocket.Id
|
||||||
private static readonly NetState[] _netStates = new NetState[MaxConnections];
|
private static readonly NetState[] _netStates = new NetState[MaxConnections];
|
||||||
|
|
||||||
// Events buffer for ProcessCompletions
|
// Events buffer for ProcessCompletions. Bounded by one event per peeked completion
|
||||||
|
// (maxSockets), doubled for headroom. Undersizing drops DataReceived events whose bytes were
|
||||||
|
// already committed, leaving them unparsed until the next recv completes.
|
||||||
private static readonly RingSocketEvent[] _events = new RingSocketEvent[MaxConnections * 2];
|
private static readonly RingSocketEvent[] _events = new RingSocketEvent[MaxConnections * 2];
|
||||||
|
|
||||||
// Listener management
|
// Listener management
|
||||||
|
|
@ -88,20 +92,61 @@ public partial class NetState
|
||||||
// Initialize IP rate limiter
|
// Initialize IP rate limiter
|
||||||
_ipRateLimiter = new IPRateLimiter(10, 10000, 1000, 2.0, 3_600_000, Core.ClosingTokenSource.Token);
|
_ipRateLimiter = new IPRateLimiter(10, 10000, 1000, 2.0, 3_600_000, Core.ClosingTokenSource.Token);
|
||||||
|
|
||||||
|
// Sends in flight per connection; honoured by RIO only (see IIORingGroup). Costs a
|
||||||
|
// request-queue and completion-queue slot per send, not another buffer. Worst-case added
|
||||||
|
// latency is roughly completion RTT / this value.
|
||||||
|
var maxOutstandingSends = ServerConfiguration.GetOrUpdateSetting("network.maxOutstandingSends", 32);
|
||||||
|
|
||||||
// Initialize IORingGroup
|
// Initialize IORingGroup
|
||||||
var ring = IORingGroup.Create(queueSize: MaxConnections * 2, maxConnections: MaxConnections);
|
var ring = IORingGroup.Create(
|
||||||
|
queueSize: MaxConnections * 2,
|
||||||
|
maxConnections: MaxConnections,
|
||||||
|
maxOutstandingSends: maxOutstandingSends
|
||||||
|
);
|
||||||
|
|
||||||
|
// Per-connection send buffer: the lever for "send buffer exhausted" disconnects, and the
|
||||||
|
// per-connection memory ceiling.
|
||||||
|
var sendBufferSize = GetSendBufferSize();
|
||||||
|
|
||||||
// Create socket manager which handles buffer pools and socket lifecycle
|
// Create socket manager which handles buffer pools and socket lifecycle
|
||||||
_socketManager = new RingSocketManager(
|
_socketManager = new RingSocketManager(
|
||||||
ring,
|
ring,
|
||||||
maxSockets: MaxConnections,
|
maxSockets: MaxConnections,
|
||||||
recvBufferSize: RecvBufferSize,
|
recvBufferSize: RecvBufferSize,
|
||||||
sendBufferSize: SendBufferSize,
|
sendBufferSize: sendBufferSize,
|
||||||
initialBufferSlabs: 8,
|
initialBufferSlabs: 8,
|
||||||
maxBufferSlabs: 32
|
maxBufferSlabs: 32
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Reads the configured send buffer size, coerced to a power of two of at least the platform
|
||||||
|
/// allocation granularity. IORingBuffer requires this and would otherwise throw at socket
|
||||||
|
/// creation rather than at startup.
|
||||||
|
/// </summary>
|
||||||
|
private static int GetSendBufferSize()
|
||||||
|
{
|
||||||
|
var configured = ServerConfiguration.GetOrUpdateSetting("network.sendBufferSize", DefaultSendBufferSize);
|
||||||
|
var size = Math.Max(MinSendBufferSize, configured);
|
||||||
|
|
||||||
|
if (!BitOperations.IsPow2(size))
|
||||||
|
{
|
||||||
|
size = (int)BitOperations.RoundUpToPowerOf2((uint)size);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (size != configured)
|
||||||
|
{
|
||||||
|
logger.Warning(
|
||||||
|
"network.sendBufferSize {Configured} is not a power of two of at least {Minimum}; using {Adjusted}",
|
||||||
|
configured,
|
||||||
|
MinSendBufferSize,
|
||||||
|
size
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
return size;
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Starts the network server on configured listening addresses.
|
/// Starts the network server on configured listening addresses.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
|
|
||||||
|
|
@ -444,17 +444,36 @@ public partial class NetState : IComparable<NetState>, IValueLinkListNode<NetSta
|
||||||
}
|
}
|
||||||
|
|
||||||
var length = span.Length;
|
var length = span.Length;
|
||||||
if (length <= 0 || !GetSendBuffer(out var buffer))
|
if (length <= 0)
|
||||||
{
|
{
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Never drop silently: the client would stay connected while missing game state.
|
||||||
|
if (!GetSendBuffer(out var buffer))
|
||||||
|
{
|
||||||
|
SendBufferExhausted(length, 0);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
// Apply encoding first (e.g., compression from UOContent)
|
// Apply encoding first (e.g., compression from UOContent)
|
||||||
if (CompressionEnabled)
|
if (CompressionEnabled)
|
||||||
{
|
{
|
||||||
length = NetworkCompression.Compress(span, buffer);
|
length = NetworkCompression.Compress(span, buffer);
|
||||||
|
|
||||||
|
// 0 means nothing was written, whether it did not fit or the input was too large.
|
||||||
|
if (length <= 0)
|
||||||
|
{
|
||||||
|
SendBufferExhausted(span.Length, buffer.Length);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
else if (span.Length > buffer.Length)
|
||||||
|
{
|
||||||
|
SendBufferExhausted(span.Length, buffer.Length);
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
|
|
@ -484,6 +503,31 @@ public partial class NetState : IComparable<NetState>, IValueLinkListNode<NetSta
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Handles a packet that cannot be placed in the send buffer.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// High unacked means a slow client holding the buffer; needed approaching capacity means the
|
||||||
|
/// buffer is too small for this shard and network.sendBufferSize should be raised.
|
||||||
|
/// </remarks>
|
||||||
|
private void SendBufferExhausted(int needed, int writable)
|
||||||
|
{
|
||||||
|
var sendBuffer = _socket?.SendBuffer;
|
||||||
|
var unacked = sendBuffer?.InFlightBytes ?? 0;
|
||||||
|
var capacity = sendBuffer?.PhysicalSize ?? 0;
|
||||||
|
|
||||||
|
logger.Warning(
|
||||||
|
"{NetState}: send buffer exhausted - needed {Needed} bytes, {Writable} writable, {Unacked} awaiting acknowledgement, {Capacity} capacity. Raise network.sendBufferSize (power of two) if this recurs on healthy connections.",
|
||||||
|
this,
|
||||||
|
needed,
|
||||||
|
writable,
|
||||||
|
unacked,
|
||||||
|
capacity
|
||||||
|
);
|
||||||
|
|
||||||
|
Disconnect($"Send buffer exhausted (needed {needed}, writable {writable}, unacked {unacked}, capacity {capacity})");
|
||||||
|
}
|
||||||
|
|
||||||
private void StartPacketLog()
|
private void StartPacketLog()
|
||||||
{
|
{
|
||||||
try
|
try
|
||||||
|
|
|
||||||
|
|
@ -73,7 +73,9 @@ public static class NetworkCompression
|
||||||
|
|
||||||
public static int Compress(ReadOnlySpan<byte> input, Span<byte> output)
|
public static int Compress(ReadOnlySpan<byte> input, Span<byte> output)
|
||||||
{
|
{
|
||||||
if (input.Length > DefiniteOverflow)
|
// output.Length < 4 underflows safeOutputLength below (nuint), defeating the hot loop's
|
||||||
|
// bounds check. Reachable whenever the send buffer is nearly full.
|
||||||
|
if (input.Length > DefiniteOverflow || output.Length < 4)
|
||||||
{
|
{
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -34,7 +34,7 @@
|
||||||
</Target>
|
</Target>
|
||||||
<ItemGroup>
|
<ItemGroup>
|
||||||
<ProjectReference Include="..\Logger\Logger.csproj" />
|
<ProjectReference Include="..\Logger\Logger.csproj" />
|
||||||
<PackageReference Include="IORingGroup" Version="1.0.7" />
|
<PackageReference Include="IORingGroup" Version="1.0.8" />
|
||||||
<PackageReference Include="CommunityToolkit.HighPerformance" Version="8.4.2" />
|
<PackageReference Include="CommunityToolkit.HighPerformance" Version="8.4.2" />
|
||||||
<PackageReference Include="LibDeflate.Bindings" Version="1.0.3" />
|
<PackageReference Include="LibDeflate.Bindings" Version="1.0.3" />
|
||||||
<PackageReference Include="System.IO.Hashing" Version="10.0.10" />
|
<PackageReference Include="System.IO.Hashing" Version="10.0.10" />
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue