Skip to content

Commit

Permalink
Fix packet fragmentation
Browse files Browse the repository at this point in the history
  • Loading branch information
Extremelyd1 committed Jul 30, 2024
1 parent 61acfee commit d1f4441
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 21 deletions.
5 changes: 3 additions & 2 deletions HKMP/Networking/Client/UdpNetClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,11 @@ private void ReceiveData(CancellationToken token) {
EndPoint endPoint = new IPEndPoint(IPAddress.Any, 0);

while (!token.IsCancellationRequested) {
var numReceived = 0;
var buffer = new byte[MaxUdpPacketSize];

try {
UdpSocket.ReceiveFrom(
numReceived = UdpSocket.ReceiveFrom(
buffer,
SocketFlags.None,
ref endPoint
Expand All @@ -88,7 +89,7 @@ ref endPoint
Logger.Error($"UDP Socket exception:\n{e}");
}

var packets = PacketManager.HandleReceivedData(buffer, ref _leftoverData);
var packets = PacketManager.HandleReceivedData(buffer, numReceived, ref _leftoverData);

_onReceive?.Invoke(packets);
}
Expand Down
23 changes: 15 additions & 8 deletions HKMP/Networking/Packet/PacketManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -479,10 +479,19 @@ Action<T, IPacketData> handler
/// <summary>
/// Handle received data and leftover data and store subsequent leftover data again.
/// </summary>
/// <param name="receivedData">Byte array of received data.</param>
/// <param name="buffer">Byte array of the buffer containing received data. The entirety of the array does not
/// necessarily contain received bytes. Rather, the first <paramref name="numReceived"/> bytes are received data.
/// </param>
/// <param name="numReceived">Number of received bytes in the buffer.</param>
/// <param name="leftoverData">Reference byte array that should be filled with leftover data.</param>
/// <returns>A list of packets that were constructed from the received data.</returns>
public static List<Packet> HandleReceivedData(byte[] receivedData, ref byte[] leftoverData) {
public static List<Packet> HandleReceivedData(byte[] buffer, int numReceived, ref byte[] leftoverData) {
// Make a new byte array exactly the length of number of received bytes and fill it
var receivedData = new byte[numReceived];
for (var i = 0; i < numReceived; i++) {
receivedData[i] = buffer[i];
}

var currentData = receivedData;

// Check whether we have leftover data from the previous read, and concatenate the two byte arrays
Expand Down Expand Up @@ -520,8 +529,7 @@ private static List<Packet> ByteArrayToPackets(byte[] data, ref byte[] leftover)

// The only break from this loop is when there is no new packet to be read
do {
// If there is still an int (4 bytes) to read in the data,
// it represents the next packet's length
// If there is still 2 bytes to read in the data, it represents the next packet's length
var packetLength = 0;
var unreadDataLength = data.Length - readIndex;
if (unreadDataLength > 1) {
Expand All @@ -534,15 +542,14 @@ private static List<Packet> ByteArrayToPackets(byte[] data, ref byte[] leftover)
break;
}

// Check whether our given data array actually contains
// the same number of bytes as the packet length
// Check whether our given data array actually contains the same number of bytes as the packet length
if (data.Length - readIndex < packetLength) {
// There is not enough bytes in the data array to fill the requested packet with
// So we put everything, including the packet length ushort (2 bytes) into the leftover byte array
leftover = new byte[unreadDataLength];
for (var i = 0; i < unreadDataLength; i++) {
// Make sure to index data 2 bytes earlier, since we incremented
// when we read the packet length ushort
// Make sure to index data 2 bytes earlier, since we incremented when we read the packet
// length ushort
leftover[i] = data[readIndex - 2 + i];
}

Expand Down
25 changes: 14 additions & 11 deletions HKMP/Networking/Server/NetServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@ internal class NetServer : INetServer {
/// </summary>
private readonly PacketManager _packetManager;

/// <summary>
/// Object to lock asynchronous access when dealing with clients.
/// </summary>
private readonly object _clientLock = new object();

/// <summary>
/// Dictionary mapping client IDs to net server clients.
/// </summary>
Expand Down Expand Up @@ -155,11 +150,12 @@ private void ReceiveData(CancellationToken token) {
EndPoint endPoint = new IPEndPoint(IPAddress.Any, 0);

while (!token.IsCancellationRequested) {
var numReceived = 0;
var buffer = new byte[MaxUdpPacketSize];

try {
// This will block until data is available
_udpSocket.ReceiveFrom(
numReceived = _udpSocket.ReceiveFrom(
buffer,
SocketFlags.None,
ref endPoint
Expand All @@ -169,7 +165,8 @@ ref endPoint
}

_receivedQueue.Enqueue(new ReceivedData {
Data = buffer,
Buffer = buffer,
NumReceived = numReceived,
EndPoint = endPoint as IPEndPoint
});
_processingWaitHandle.Set();
Expand All @@ -192,7 +189,8 @@ private void StartProcessing(CancellationToken token) {

while (_receivedQueue.TryDequeue(out var receivedData)) {
var packets = PacketManager.HandleReceivedData(
receivedData.Data,
receivedData.Buffer,
receivedData.NumReceived,
ref _leftoverData
);

Expand Down Expand Up @@ -529,12 +527,17 @@ Func<TPacketId, IPacketData> packetInstantiator
/// </summary>
internal class ReceivedData {
/// <summary>
/// Byte array of received data.
/// Byte array of the buffer containing received data.
/// </summary>
public byte[] Buffer { get; init; }

/// <summary>
/// The number of bytes in the buffer that were received. The rest of the buffer is empty.
/// </summary>
public byte[] Data { get; set; }
public int NumReceived { get; init; }

/// <summary>
/// The IP end-point of the client from which we received the data.
/// </summary>
public IPEndPoint EndPoint { get; set; }
public IPEndPoint EndPoint { get; init; }
}
31 changes: 31 additions & 0 deletions HKMP/Networking/UdpUpdateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ internal abstract class UdpUpdateManager<TOutgoing, TPacketId> : UdpUpdateManage
/// </summary>
private const int ConnectionTimeout = 5000;

/// <summary>
/// The MTU (maximum transfer unit) to use to send packets with. If the length of a packet exceeds this, we break
/// it up into smaller packets before sending. This ensures that we control the breaking of packets in most
/// cases and do not rely on smaller network devices for the breaking up as this could impact performance.
/// </summary>
private const int PacketMtu = 1400;

/// <summary>
/// The number of sequence numbers to store in the received queue to construct ack fields with and
/// to check against resent data.
Expand Down Expand Up @@ -234,6 +241,30 @@ private void CreateAndSendUpdatePacket() {
// Increase (and potentially wrap) the current local sequence number
_localSequence++;

// Check if the packet exceeds (usual) MTU and break it up if so
if (packet.Length > PacketMtu) {
// Get the original packet's bytes as an array
var byteArray= packet.ToArray();

// Keep track of the index in the original array for copying
var index = 0;
// While we have not reached the end of the original array yet with the index
while (index < byteArray.Length) {
// Take the minimum of what's left to copy in the original array and the max MTU
var length = System.Math.Min(byteArray.Length - index, PacketMtu);
// Create a new array that is this calculated length
var newBytes = new byte[length];
// Copy over the length of bytes starting from index into the new array
Array.Copy(byteArray, index, newBytes, 0, length);

SendPacket(new Packet.Packet(newBytes));

index += length;
}

return;
}

SendPacket(packet);
}

Expand Down

0 comments on commit d1f4441

Please sign in to comment.