MTU fixes/improvements

Lowered default MTU for IPv4 to 508. I don't want to have to deal with any more issues from somebody with too shit of a network.

Default MTU can now be specified separately for IPv6, as it permits a higher default (1232).

Fixed MTU detection not setting its state to InProgress correctly. This means that every network thread heartbeat it would send ANOTHER MTU expansion attempt while existing ones are already in flight, and this would keep stacking up until MTU detection was finished.

Fixed ExpandMTUFailAttempts not being respected for delay-failure (it was only checked by immediate transmission failure).

Lidgren.Network/NetConnection.MTU.cs

@@ -2,7 +2,7 @@
 
 namespace Lidgren.Network
 {
-	public partial class NetConnection  
+	public partial class NetConnection
 	{
 		private enum ExpandMTUStatus
 		{
@@ -25,7 +25,8 @@ private enum ExpandMTUStatus
 		internal int m_currentMTU;
 
 		/// <summary>
-		/// Gets the current MTU in bytes. If PeerConfiguration.AutoExpandMTU is false, this will be PeerConfiguration.MaximumTransmissionUnit.
+		/// Gets the current MTU in bytes. If <see cref="NetPeerConfiguration.AutoExpandMTU"/> is false,
+		/// this will be <see cref="NetPeerConfiguration.MaximumTransmissionUnit"/> or <see cref="NetPeerConfiguration.MaximumTransmissionUnitV6"/>.
 		/// </summary>
 		public int CurrentMTU { get { return m_currentMTU; } }
 
@@ -34,7 +35,7 @@ internal void InitExpandMTU(double now)
 			m_lastSentMTUAttemptTime = now + m_peerConfiguration.m_expandMTUFrequency + 1.5f + m_averageRoundtripTime; // wait a tiny bit before starting to expand mtu
 			m_largestSuccessfulMTU = 512;
 			m_smallestFailedMTU = -1;
-			m_currentMTU = m_peerConfiguration.MaximumTransmissionUnit;
+			m_currentMTU = m_peerConfiguration.MTUForEndPoint(m_remoteEndPoint);
 		}
 
 		private void MTUExpansionHeartbeat(double now)
@@ -51,14 +52,17 @@ private void MTUExpansionHeartbeat(double now)
 				}
 
 				// begin expansion
+				m_expandMTUStatus = ExpandMTUStatus.InProgress;
+				// m_peer.LogDebug("Doing initial MTU expand");
 				ExpandMTU(now);
 				return;
 			}
 
 			if (now > m_lastSentMTUAttemptTime + m_peerConfiguration.ExpandMTUFrequency)
 			{
+				// m_peer.LogDebug("MTU attempt failed");
 				m_mtuAttemptFails++;
-				if (m_mtuAttemptFails == 3)
+				if (m_mtuAttemptFails >= m_peerConfiguration.ExpandMTUFailAttempts)
 				{
 					FinalizeMTU(m_currentMTU);
 					return;

Lidgren.Network/NetConnection.cs

@@ -97,7 +97,7 @@ internal NetConnection(NetPeer peer, NetEndPoint remoteEndPoint)
 			m_queuedIncomingAcks = new NetQueue<NetTuple<NetMessageType, int>>(4);
 			m_statistics = new NetConnectionStatistics(this);
 			m_averageRoundtripTime = -1.0f;
-			m_currentMTU = m_peerConfiguration.MaximumTransmissionUnit;
+			m_currentMTU = m_peerConfiguration.MTUForEndPoint(m_remoteEndPoint);
 		}
 
 		/// <summary>

Lidgren.Network/NetPeer.Discovery.cs

@@ -62,8 +62,9 @@ public void SendDiscoveryResponse(NetOutgoingMessage msg, NetEndPoint recipient)
 			else if (msg.m_isSent)
 				throw new NetException("Message has already been sent!");
 
-			if (msg.LengthBytes >= m_configuration.MaximumTransmissionUnit)
-				throw new NetException("Cannot send discovery message larger than MTU (currently " + m_configuration.MaximumTransmissionUnit + " bytes)");
+			var mtu = m_configuration.MTUForEndPoint(recipient);
+			if (msg.LengthBytes >= mtu)
+				throw new NetException("Cannot send discovery message larger than MTU (currently " + mtu + " bytes)");
 
 			msg.m_messageType = NetMessageType.DiscoveryResponse;
 			Interlocked.Increment(ref msg.m_recyclingCount);

Lidgren.Network/NetPeer.Send.cs

@@ -159,16 +159,18 @@ public void SendUnconnectedMessage(NetOutgoingMessage msg, string host, int port
 				throw new ArgumentNullException("host");
 			if (msg.m_isSent)
 				throw new NetException("This message has already been sent! Use NetPeer.SendMessage() to send to multiple recipients efficiently");
-			if (msg.LengthBytes > m_configuration.MaximumTransmissionUnit)
-				throw new NetException("Unconnected messages too long! Must be shorter than NetConfiguration.MaximumTransmissionUnit (currently " + m_configuration.MaximumTransmissionUnit + ")");
-
-			msg.m_isSent = true;
-			msg.m_messageType = NetMessageType.Unconnected;
 
 			var adr = NetUtility.Resolve(host);
 			if (adr == null)
 				throw new NetException("Failed to resolve " + host);
 
+			var mtu = m_configuration.MTUForAddress(adr);
+			if (msg.LengthBytes > mtu)
+				throw new NetException("Unconnected messages too long! Must be shorter than NetConfiguration.MaximumTransmissionUnit (whichever is appropriate, currently " + mtu + ")");
+
+			msg.m_isSent = true;
+			msg.m_messageType = NetMessageType.Unconnected;
+
 			Interlocked.Increment(ref msg.m_recyclingCount);
 			m_unsentUnconnectedMessages.Enqueue(new NetTuple<NetEndPoint, NetOutgoingMessage>(new NetEndPoint(adr, port), msg));
 		}
@@ -184,8 +186,10 @@ public void SendUnconnectedMessage(NetOutgoingMessage msg, NetEndPoint recipient
 				throw new ArgumentNullException("recipient");
 			if (msg.m_isSent)
 				throw new NetException("This message has already been sent! Use NetPeer.SendMessage() to send to multiple recipients efficiently");
-			if (msg.LengthBytes > m_configuration.MaximumTransmissionUnit)
-				throw new NetException("Unconnected messages too long! Must be shorter than NetConfiguration.MaximumTransmissionUnit (currently " + m_configuration.MaximumTransmissionUnit + ")");
+
+			var mtu = m_configuration.MTUForEndPoint(recipient);
+			if (msg.LengthBytes > mtu)
+				throw new NetException("Unconnected messages too long! Must be shorter than NetConfiguration.MaximumTransmissionUnit (currently " + mtu + ")");
 
 			msg.m_messageType = NetMessageType.Unconnected;
 			msg.m_isSent = true;
@@ -207,8 +211,16 @@ public void SendUnconnectedMessage(NetOutgoingMessage msg, IList<NetEndPoint> re
 				throw new NetException("recipients must contain at least one item");
 			if (msg.m_isSent)
 				throw new NetException("This message has already been sent! Use NetPeer.SendMessage() to send to multiple recipients efficiently");
-			if (msg.LengthBytes > m_configuration.MaximumTransmissionUnit)
-				throw new NetException("Unconnected messages too long! Must be shorter than NetConfiguration.MaximumTransmissionUnit (currently " + m_configuration.MaximumTransmissionUnit + ")");
+
+			// TODO: Avoid this extra allocating loop in the case that this isn't a dual-stack socket.
+			var minimumMTU = int.MaxValue;
+			foreach (var ep in recipients)
+			{
+				minimumMTU = Math.Min(minimumMTU, m_configuration.MTUForEndPoint(ep));
+			}
+
+			if (msg.LengthBytes > minimumMTU)
+				throw new NetException("Unconnected messages too long! Must be shorter than NetConfiguration.MaximumTransmissionUnit (currently " + minimumMTU + ")");
 
 			msg.m_messageType = NetMessageType.Unconnected;
 			msg.m_isSent = true;

Lidgren.Network/NetPeerConfiguration.cs

@@ -19,6 +19,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 */
 using System;
 using System.Net;
+using System.Net.Sockets;
 
 namespace Lidgren.Network
 {
@@ -27,25 +28,29 @@ namespace Lidgren.Network
 	/// </summary>
 	public sealed class NetPeerConfiguration
 	{
-		// Default MTU value used to be a slightly conservative number based off 1500 IP-MTU: 1408 
-		// This was too aggressive and caused connection troubles for some people (CGNAT + VPN lowered MTU too much)
-		// I wasn't able to find good numbers on what "practical" MTU numbers are, but:
-		// * QUIC requires an UDP-payload MTU of 1200. https://datatracker.ietf.org/doc/html/rfc9000#section-14
-		// * I've read at least one forum post by somebody claiming IP-MTU does get as low as 1260
-		// As such, I've lowered this number to 1200.
-		// 
-		// I considered lowering it further to like 506 (extremely conservative, technically IP-spec-minimum)
-		// But decided against it, because in practice that never makes sense I think.
-		// Furthermore, MTU is not purely a performance thing.
-		// Some message types like hail and unreliable are locked to MTU: lowering it too much can cause trouble.
-
-		/// <summary>
-		/// Default MTU value in bytes
+		// Default MTU value used to be pretty high. I've slowly had to reduce it over the years due to continuing
+		// reports of "yet another person with an even shittier network" having issues.
+		// The default values are now the spec-minimum values.
+		// IPv4: 576 (spec minimum) - 60 (max IPv4 header) - 8 (UDP header) = 508
+		// IPv6: 1280 (spec minimum) - 40 (IPv6 header) - 8 (UDP header) = 1232
+
+		/// <summary>
+		/// Default MTU value for IPv4 connections, in bytes.
+		/// </summary>
+		/// <remarks>
+		/// <para>
+		/// Note that lidgren headers (5 bytes) are not included here; since it's part of the "mtu payload".
+		/// </para>
+		/// </remarks>
+		public const int kDefaultMTU = 508;
+
+		/// <summary>
+		/// Default MTU value for IPv6 connections, in bytes.
 		/// </summary>
 		/// <remarks>
 		/// Note that lidgren headers (5 bytes) are not included here; since it's part of the "mtu payload"
 		/// </remarks>
-		public const int kDefaultMTU = 1200;
+		public const int kDefaultMTUV6 = 1232;
 
 		private const string c_isLockedMessage = "You may not modify the NetPeerConfiguration after it has been used to initialize a NetPeer";
 
@@ -83,6 +88,7 @@ public sealed class NetPeerConfiguration
 
 		// MTU
 		internal int m_maximumTransmissionUnit;
+		internal int m_maximumTransmissionUnitV6;
 		internal bool m_autoExpandMTU;
 		internal float m_expandMTUFrequency;
 		internal int m_expandMTUFailAttempts;
@@ -124,6 +130,7 @@ public NetPeerConfiguration(string appIdentifier)
 			m_suppressUnreliableUnorderedAcks = false;
 
 			m_maximumTransmissionUnit = kDefaultMTU;
+			m_maximumTransmissionUnitV6 = kDefaultMTUV6;
 			m_autoExpandMTU = false;
 			m_expandMTUFrequency = 2.0f;
 			m_expandMTUFailAttempts = 5;
@@ -223,8 +230,10 @@ public int MaximumConnections
 		}
 
 		/// <summary>
-		/// Gets or sets the maximum amount of bytes to send in a single packet, excluding ip, udp and lidgren headers. Cannot be changed once NetPeer is initialized.
+		/// Gets or sets the maximum amount of bytes to send in a single packet for IPv4 connections, excluding IPv4 and UDP headers.
+		/// Cannot be changed once NetPeer is initialized.
 		/// </summary>
+		/// <seealso cref="MaximumTransmissionUnitV6"/>
 		public int MaximumTransmissionUnit
 		{
 			get { return m_maximumTransmissionUnit; }
@@ -238,6 +247,24 @@ public int MaximumTransmissionUnit
 			}
 		}
 
+		/// <summary>
+		/// Gets or sets the maximum amount of bytes to send in a single packet for IPv6 connections, excluding IPv6 and UDP headers.
+		/// Cannot be changed once NetPeer is initialized.
+		/// </summary>
+		/// <seealso cref="MaximumTransmissionUnit"/>
+		public int MaximumTransmissionUnitV6
+		{
+			get { return m_maximumTransmissionUnitV6; }
+			set
+			{
+				if (m_isLocked)
+					throw new NetException(c_isLockedMessage);
+				if (value < 1 || value >= ((ushort.MaxValue + 1) / 8))
+					throw new NetException("MaximumTransmissionUnitV6 must be between 1 and " + (((ushort.MaxValue + 1) / 8) - 1) + " bytes");
+				m_maximumTransmissionUnitV6 = value;
+			}
+		}
+
 		/// <summary>
 		/// Gets or sets the default capacity in bytes when NetPeer.CreateMessage() is called without argument
 		/// </summary>
@@ -538,6 +565,16 @@ public NetPeerConfiguration Clone()
 			retval.m_isLocked = false;
 			return retval;
 		}
+
+		internal int MTUForAddress(IPAddress address)
+		{
+			if (address.AddressFamily == AddressFamily.InterNetworkV6)
+				return address.IsIPv4MappedToIPv6 ? MaximumTransmissionUnit : MaximumTransmissionUnitV6;
+
+			return MaximumTransmissionUnit;
+		}
+
+		internal int MTUForEndPoint(IPEndPoint endPoint) => MTUForAddress(endPoint.Address);
 	}
 
 	/// <summary>

RELEASE-NOTES.md

@@ -7,6 +7,10 @@
 - Updated dependencies
 - More and improved doc comments.
 - Fix callback-based `NetUtility.ResolveAsync` functions never running their callback. (bug introduced in 0.1.0)
+- Default MTU for IPv4 has been dropped to 508 to avoid issues for certain people.
+- A separate MTU value is now used for IPv6, as it permits a higher default. You can set it with `NetPeerConfiguration.MaximumTransmissionUnitV6`
+- Fixed MTU expansion not setting internal state correctly and spamming network packets.
+- Fixed `NetPeerConfiguration.ExpandMTUFailAttempts` not being respected completely.
 
 ## 0.2.7