From ddb302d03487b07b6a8127576677223174357e20 Mon Sep 17 00:00:00 2001 From: dave Date: Sun, 2 Oct 2022 16:26:05 +0100 Subject: [PATCH 1/2] tcp: avoid stall with larger MSS If the connection requests a large MSS > 16k, for example to exploit a large MTU, then writes block because available space is believed to be 0. Consider UTX.available: ``` let available t = let a = Int32.sub t.max_size t.bufbytes in match a < (Int32.of_int (Window.tx_mss t.wnd)) with | true -> 0l | false -> a ``` Initially max_size = 16k (hardcoded in flow.ml) and bufbytes = 0, so a = 16k (meaning 16k space is free in the buffer). If the free space (a) is less than an MSS, we return 0 and the connection stalls. I think the assumption is that the UTX can buffer at least 2*MSS worth of data so that when the free space (a) is less than an MSS, the buffer already contains an MSS worth of data to transmit to make progress. Bump the user buffer size to 128k which is 2x a 64k MSS (where 64k is the max value of the 16-bit MSS option). This might need rethinking if we support segmentation offload because we might see even bigger segments. Signed-off-by: David Scott --- src/tcp/flow.ml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/tcp/flow.ml b/src/tcp/flow.ml index a96e2400..cf2d40ae 100644 --- a/src/tcp/flow.ml +++ b/src/tcp/flow.ml @@ -20,6 +20,9 @@ open Lwt.Infix let src = Logs.Src.create "tcp.pcb" ~doc:"Mirage TCP PCB module" module Log = (val Logs.src_log src : Logs.LOG) +(* MSS options are 16 bites, so the max value is 64k *) +let max_mss = Int32.of_int (64 * 1024) + module Make(Ip: Tcpip.Ip.S)(Time:Mirage_time.S)(Clock:Mirage_clock.MCLOCK)(Random:Mirage_random.S) = struct @@ -360,8 +363,9 @@ struct in (* Set up ACK module *) let ack = ACK.t ~send_ack ~last:(Sequence.succ rx_isn) in - (* The user application transmit buffer *) - let utx = UTX.create ~wnd ~txq ~max_size:16384l in + (* The user application transmit buffer. Ensure we are always allowed to write + an MSS of data into it before `available` returns 0. *) + let utx = UTX.create ~wnd ~txq ~max_size:(Int32.mul 2l max_mss) in let rxq = RXS.create ~rx_data ~ack ~wnd ~state ~tx_ack in (* Set up the keepalive state if requested *) let keepalive = match keepalive with From 657b7c87ea8fd4c19279f87ed9ffec19e597bcb7 Mon Sep 17 00:00:00 2001 From: Calascibetta Romain Date: Fri, 10 Feb 2023 11:16:52 +0100 Subject: [PATCH 2/2] Directly use int32 values instead of Int32.of_int (@avsm) --- src/tcp/flow.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tcp/flow.ml b/src/tcp/flow.ml index cf2d40ae..01fe0d89 100644 --- a/src/tcp/flow.ml +++ b/src/tcp/flow.ml @@ -21,7 +21,7 @@ let src = Logs.Src.create "tcp.pcb" ~doc:"Mirage TCP PCB module" module Log = (val Logs.src_log src : Logs.LOG) (* MSS options are 16 bites, so the max value is 64k *) -let max_mss = Int32.of_int (64 * 1024) +let max_mss = Int32.mul 64l 1024l module Make(Ip: Tcpip.Ip.S)(Time:Mirage_time.S)(Clock:Mirage_clock.MCLOCK)(Random:Mirage_random.S) = struct