Hi, Andrey Thanks for provide the log file. I think your point is right, that the server is not really reduce the window size, since it could still ACK the whole data to 199866. I searched the IETF and finally found the RFC7323, it’s an updated version of TCP window scale option, and discussed this problem in section 2.4 as “Window Retraction”. We will evaluate this RFC and provide a patch to fix the problem. Once I have a patch I will let you know and maybe you can help to test it. Thanks again for helping to address the problem. Best Regards, Siyuan From: atepin@kraftway.ru [mailto:atepin@kraftway.ru] Sent: 2017年4月17日 22:41 To: Fu, Siyuan ; edk2-devel@lists.01.org Cc: Ye, Ting ; Tian, Feng ; Wu, Jiaxin Subject: Re: [PATCH 2/2] NetworkPkg/TcpDxe: Fix unconditional window shrinking Hello, Siyuan Sorry for providing incomplete log. I think missing packets is not the case here since all packets reached client. I made another log and I am attaching it to this email (full wireshark log from start of transmission). I am also attaching log from TcpDxe driver (added debug prints in TcpInput function as input and TcpTransmitSegment as output). This log actually contains two cases: (1) in frame 145 (line 450 in TcpDxe log) advertised window reaches zero size, but after window update in frame 152 transmission continues without deadlocks; (2) in frame 221 (line 698 in TcpDxe log) advertised window reaches zero again, but after window update we have a deadlock starting from frame 241. Please notice how packets actually processed by TcpDxe driver. According to TcpDxe log server starts to shrink window in line 615 (frame 198). This seems to be shrinking, so i was wrong, thinking there was no shrinking at all, server indeed moves its Right edge to the left. TcpDxe driver catches this and prints message in line 615, though, no actual data shrinking happens here since Right > SndNxt. Same goes for next incoming packets (lines 616 - 627). At this point client have stopped sending data (line 611, frame 197) at SEQ 198418 (did he wait until he can send full MSS segment? which is 1448). In next packets (lines 629 - 636) server opens its window again (moves Right edge to the right) and we have Right=199866 here. After receiving Right=199866 from server client can now send 1448 bytes of payload and he does (frame 208, line 638). Starting from line 641 we have only input packets from server. Next packet forces client to move SndNxt to the left since client have just sent Seq:198418, End:199866 packet and server advertised Ack:156426, Wnd:43264, Right:199690. This packets ACKs old segment and advertises quite big window. Should client use values from this packet to reduce its SndNxt? Anyway, server shrinks Right to the value of 199642 but then it opens Right to the value of 199866 in line 700 with ACK=199866. Does this mean that in general Right didn't move (yes, technically it moved left but then it restored to the original right)? Server without doubt accepted all data. So maybe the bug here is to use 'intermediate' values from server to adjust SndNxt? Maybe we should calculate usable window right before sending data in TcpOutput? Regards, Andrey On 17.04.2017 05:41, Fu, Siyuan wrote: Hi, Andrey For the question 1, that why client send out seq 590994, I don’t think it’s client side problem. We can see the server is moving the right edge if its receive window start from the beginning: the frame 4490, right edge is 590954, then it comes to 590946 (farme 4491), 590770 (frame 4492), 590754(frame 4492)... Please note that these packets have not been received by the client side. I think why client send out frame 4496 with seq number to 590994, is because the server did advise a receive window for 590994 before frame 4490, and not shown in your dump log. Later the client send out frame 4500, which seq number is 590754, this is actually the right edge advised by the server’s frame 4494. This means the client just received the frame 4494 after it send out the frame 4496, and this is the frame which really shrink the receive and make a negative useable in client side. [cid:image002.png@01D2B839.0DE0F920] I have check the new dump file, this time the server has never advise a right window edge on left of client’s SndNxt, that’s why the client won’t meet the “negative useable window” this time, not because your patch fixed the problem. Best Regards, Siyuan From: atepin@kraftway.ru [mailto:atepin@kraftway.ru] Sent: 2017年4月14日 21:32 To: Fu, Siyuan ; edk2-devel@lists.01.org Cc: Ye, Ting ; Tian, Feng ; Wu, Jiaxin Subject: Re: [PATCH 2/2] NetworkPkg/TcpDxe: Fix unconditional window shrinking Hello, Siyuan, Thank you for your reply. I created a bug#486 in bugzilla for this case. And answering your questions: 1. In frame 4496 client sends data starting from 589546 with len of 1448 and ending with 590994 seq number. Previously offered window from server was in frame 4495 Wnd=5632 ACK=585202 and the right window edge 585202+5632=590834. So the client should have sent 590834-589546=1288 bytes instead. So why the server accepted all 1448 bytes? I can only assume, that maybe during transmission of frame 4496 from client to server the application's process on server removed some data from server's receive buffer. But at this point it is clear that this situation was lead to by invalid behavior of the client. 2. While in most cases the deadlock occurs after recovering from ZeroWindow, in some rare cases it is not. I believe it has something to do with how far the 'distance' between Snd.Una and Snd.Nxt at the time server offers zero window. But it never occurs (at least after dozens of ties) with proposed values for usable window calculation. So it seems patch fixes the problem here (though it is not fully correct too, see below). Some discussion about why TCB's values should be used: the proposed change if (TCP_SEQ_LT (Tcb->SndUna + Tcb->SndWnd, Tcb->SndNxt)) is wrong too. At the point of this check in TcpInput function we have Tcb->SndUna == Seg->Ack (it was assigned previously in this function). I believe, TCB's value should be used and not the new SEG's value, because we calculate the usable window here for the segments which has already been sent. So we need to use untouched TCB's values and calculate usable window (UsableWnd = Tcb->SndUna + Tcb->SndWnd - Tcb->SndNxt) at TcpInput function start. And later in window update section of TcpInput function we need to preform check if (UsableWnd < 0) { instead of if (TCP_SEQ_LT (Right, Tcb->SndNxt)) { Bug does not occur with described change. Below I provide wireshark's dump of tcp transmission with implemented change: Frame 2834: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 118946, Ack: 1, Len: 1448 Sequence number: 118946 (relative sequence number) [Next sequence number: 120394 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 28960] Frame 2835: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 120394, Ack: 1, Len: 1448 Sequence number: 120394 (relative sequence number) [Next sequence number: 121842 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 30408] Frame 2836: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 121842, Ack: 1, Len: 1448 Sequence number: 121842 (relative sequence number) [Next sequence number: 123290 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 31856] Frame 2837: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 97226, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 97226 (relative ack number) Window size value: 141 [Calculated window size: 36096] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2813] [The RTT to ACK the segment was: 0.005748897 seconds] [iRTT: 0.046641494 seconds] Frame 2838: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 100122, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 100122 (relative ack number) Window size value: 130 [Calculated window size: 33280] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2815] [The RTT to ACK the segment was: 0.006319882 seconds] [iRTT: 0.046641494 seconds] Frame 2839: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 103018, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 103018 (relative ack number) Window size value: 118 [Calculated window size: 30208] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2817] [The RTT to ACK the segment was: 0.006863979 seconds] [iRTT: 0.046641494 seconds] Frame 2840: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 123290, Ack: 1, Len: 1448 Sequence number: 123290 (relative sequence number) [Next sequence number: 124738 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 21720] Frame 2841: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 124738, Ack: 1, Len: 1448 Sequence number: 124738 (relative sequence number) [Next sequence number: 126186 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 23168] Frame 2842: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 126186, Ack: 1, Len: 1448 Sequence number: 126186 (relative sequence number) [Next sequence number: 127634 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 24616] Frame 2843: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 127634, Ack: 1, Len: 1448 Sequence number: 127634 (relative sequence number) [Next sequence number: 129082 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 26064] Frame 2844: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 129082, Ack: 1, Len: 1448 Sequence number: 129082 (relative sequence number) [Next sequence number: 130530 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 27512] Frame 2845: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 130530, Ack: 1, Len: 1448 Sequence number: 130530 (relative sequence number) [Next sequence number: 131978 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 28960] Frame 2846: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 105914, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 105914 (relative ack number) Window size value: 107 [Calculated window size: 27392] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2823] [The RTT to ACK the segment was: 0.006541883 seconds] [iRTT: 0.046641494 seconds] Frame 2847: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 105914, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 105914 (relative ack number) Window size value: 107 [Calculated window size: 27392] [Window size scaling factor: 256] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [TCP Analysis Flags] [This is a TCP duplicate ack] [Duplicate ACK #: 1] [Duplicate to the ACK in frame: 2846] [Expert Info (Note/Sequence): Duplicate ACK (#1)] Frame 2848: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 108810, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 108810 (relative ack number) Window size value: 96 [Calculated window size: 24576] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2825] [The RTT to ACK the segment was: 0.007126692 seconds] [iRTT: 0.046641494 seconds] Frame 2849: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 114602, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 114602 (relative ack number) Window size value: 73 [Calculated window size: 18688] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2829] [The RTT to ACK the segment was: 0.007430487 seconds] [iRTT: 0.046641494 seconds] Frame 2850: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 121842, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 121842 (relative ack number) Window size value: 45 [Calculated window size: 11520] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2835] [The RTT to ACK the segment was: 0.008185283 seconds] [iRTT: 0.046641494 seconds] Frame 2851: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 126186, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 126186 (relative ack number) Window size value: 28 [Calculated window size: 7168] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2841] [The RTT to ACK the segment was: 0.005921180 seconds] [iRTT: 0.046641494 seconds] Frame 2852: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 130530, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 130530 (relative ack number) Window size value: 11 [Calculated window size: 2816] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2844] [The RTT to ACK the segment was: 0.006290025 seconds] [iRTT: 0.046641494 seconds] Frame 2853: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 131978, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 131978 (relative ack number) Window size value: 5 [Calculated window size: 1280] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2845] [The RTT to ACK the segment was: 0.006883619 seconds] [iRTT: 0.046641494 seconds] Frame 2872: 1346 bytes on wire (10768 bits), 1346 bytes captured (10768 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 131978, Ack: 1, Len: 1280 Sequence number: 131978 (relative sequence number) [Next sequence number: 133258 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 1280] [TCP Analysis Flags] [Expert Info (Warn/Sequence): TCP window specified by the receiver is now completely full] Frame 2873: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 133258, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 133258 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] [SEQ/ACK analysis] [This is an ACK to the segment in frame: 2872] [The RTT to ACK the segment was: 0.028816305 seconds] [iRTT: 0.046641494 seconds] [TCP Analysis Flags] [Expert Info (Warn/Sequence): TCP Zero Window segment] Frame 2895: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 133257, Ack: 1, Len: 0 Sequence number: 133257 (relative sequence number) Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [TCP Analysis Flags] [Expert Info (Note/Sequence): TCP keep-alive segment] Frame 2896: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 133258, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 133258 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [TCP Analysis Flags] [Expert Info (Warn/Sequence): TCP Zero Window segment] Frame 2929: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 133257, Ack: 1, Len: 0 Sequence number: 133257 (relative sequence number) Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [TCP Analysis Flags] [Expert Info (Note/Sequence): TCP keep-alive segment] Frame 2930: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 133258, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 133258 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [TCP Analysis Flags] [Expert Info (Warn/Sequence): TCP Zero Window segment] Frame 2957: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 133257, Ack: 1, Len: 0 Sequence number: 133257 (relative sequence number) Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [TCP Analysis Flags] [Expert Info (Note/Sequence): TCP keep-alive segment] Frame 2958: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 133258, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 133258 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [TCP Analysis Flags] [Expert Info (Warn/Sequence): TCP Zero Window segment] Frame 2969: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1838 (1838), Seq: 1, Ack: 133258, Len: 0 Sequence number: 1 (relative sequence number) Acknowledgment number: 133258 (relative ack number) Window size value: 260 [Calculated window size: 66560] [Window size scaling factor: 256] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [TCP Analysis Flags] [Expert Info (Chat/Sequence): TCP window update] Frame 2970: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1838 (1838), Dst Port: 80 (80), Seq: 133258, Ack: 1, Len: 1448 Sequence number: 133258 (relative sequence number) [Next sequence number: 134706 (relative sequence number)] Acknowledgment number: 1 (relative ack number) Window size value: 32768 [Calculated window size: 2097152] [Window size scaling factor: 64] [SEQ/ACK analysis] [iRTT: 0.046641494 seconds] [Bytes in flight: 1448] Regards, Andrey On 14.04.2017 12:00, Fu, Siyuan wrote: Hi, Andrey I’d like to compare the original code logic and your suggested change: Original: Right = Seg->Ack + Seg->Wnd; ... if (TCP_SEQ_LT (Right, Tcb->SndNxt)) { <- check if “Seg->Ack + Seg->Wnd – SndNxt < 0” Change to: if (TCP_SEQ_LT (Tcb->SndUna + Tcb->SndWnd, Tcb->SndNxt)) <- check if “Tcb->SndUna + Tcb->SndWnd – SndNxt < 0” For each received segment, TCP driver will set SND.UNA <- SEG.ACK, and SND.WND <- SEG.WND. So the original code is always checking the server advised Right edge and adjust the SndNxt, before apply the new right edge to the Tcb, then the useable window would never become a negative value. While your patch use the TCB’s SndUna and SndWnd, which means it will accept a negative window for the first time, then adjust it when receiving a new segment. If the TCP client received a series of segment that continuous shrink the receive window, just like the frame 4492-4497, I think above 2 method will get same result, that the SndNxt will move to the left, and send out zero window probe segment. Then I have 2 question here: 1. The client send out frame 4496 with data Seq to 590994, which already beyond the server’s current receive window (this means the server has advised the right edge large than 590994 in previous packet, but not list here??). But server did acked all the data in frame 4498, this is the first thing that looks very strange to me. 2. The server re-open the receive window in frame 4520, with ACK=590994. As I mentioned above, the SndNxt has already been moved to the left of 590994, no matter we choose which method to adjust the SndNxt, so this frame will always be ignored and become a deadlock. So, if the problem is solved by the patch, something must be wrong in my above understanding. Do you also have a sample file to show how the patch could solve the dead lock, especially for the packets after server re-open the receive windows? Best Regards, Siyuan From: Fu, Siyuan Sent: 2017年4月14日 9:04 To: atepin@kraftway.ru; edk2-devel@lists.01.org Cc: Ye, Ting ; Tian, Feng ; Wu, Jiaxin Subject: RE: [PATCH 2/2] NetworkPkg/TcpDxe: Fix unconditional window shrinking Hi, Andrey Thanks, that explains my confusion to the patch. Per my understanding, to against the useable windows to be a negative window means the code should check the value before assign the new right edge, and reject that change, not to correct it after the useable windows already becomes a negative one. Could you please help to file a bug in edk2 Bugzilla so we can easily trace it? https://tianocore.acgmultimedia.com/ Best Regards, Siyuan From: atepin@kraftway.ru [mailto:atepin@kraftway.ru] Sent: 2017年4月13日 22:33 To: Fu, Siyuan >; edk2-devel@lists.01.org Cc: Ye, Ting >; Tian, Feng >; Wu, Jiaxin > Subject: Re: [PATCH 2/2] NetworkPkg/TcpDxe: Fix unconditional window shrinking Hello, Siyuan If usable window becomes negative (UsableWnd < 0), we adjust SndNxt (Tcb->SndNxt = Right) and thus, our usable window becomes non-negative (UsableWnd = Tcb->SndUna + Tcb->SndWnd - Right). In other words, we are fixing usable window if it becomes negative and this way we provide robustness. Examining this code again I realized, that I was wrong about bug here. I was thinking the bug here was not checking usable window, but in line if (TCP_SEQ_LT (Right, Tcb->SndNxt)) { // <---- this line - Tcb->SndNxt = Right; + UsableWnd = Tcb->SndUna + Tcb->SndWnd - Tcb->SndNxt; + if (UsableWnd < 0) { + Tcb->SndNxt = Right; + } usable window is actually being checked. So the bug here is checking it against wrong values. Variable Right is assigned here // // Update window info // if (TCP_SEQ_LT (Tcb->SndWl1, Seg->Seq) || ((Tcb->SndWl1 == Seg->Seq) && TCP_SEQ_LEQ (Tcb->SndWl2, Seg->Ack))) { Right = Seg->Ack + Seg->Wnd; // <---- this seems to be wrong (see rfc1122 p.99) using values from Seg structure. I am not sure about other checks where variable Right occurs, but in line if (TCP_SEQ_LT (Right, Tcb->SndNxt)) it should be changed to if (TCP_SEQ_LT (Tcb->SndUna + Tcb->SndWnd, Tcb->SndNxt)) Regards, Andrey On 12.04.2017 13:48, Fu, Siyuan wrote: Hi, Andrey Thanks for providing the detail data and now I understand the problem. The TCP driver was designed and implemented according RFC793, so I think it doesn't fully comply with the subsequent update ECR. While I still have question about your patch, the RFC1122 said that the sending TCP must be robust against the window shrinking, per my understanding, this means we should guarantee the useable windows always be a positive number. But in your patch it actually doing the opposite thing, that make sure the UsableWnd always a negative number. That's why I'm confusing when I see the patch first time. if (TCP_SEQ_LT (Right, Tcb->SndNxt)) { - Tcb->SndNxt = Right; + UsableWnd = Tcb->SndUna + Tcb->SndWnd - Tcb->SndNxt; + if (UsableWnd < 0) { <----- why "< 0" here ?? + Tcb->SndNxt = Right; + } Best Regards, Siyuan -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of atepin@kraftway.ru Sent: 2017年4月6日 23:34 To: Fu, Siyuan ; edk2-devel@lists.01.org Cc: Ye, Ting ; Tian, Feng ; Wu, Jiaxin Subject: Re: [edk2] [PATCH 2/2] NetworkPkg/TcpDxe: Fix unconditional window shrinking here Hello, Siyuan, I experienced actual deadlock while sending huge amounts of data from uefi to some server (tried Apache and IIS). At some point of transmission server starts to reduce its window size all the way to zero window size. After window update on server (when window size restores back to non-zero value) client starts to drop all packets from server because of future ACK (Seg.Ack > Tcb.SndNxt). This supposedly happened because uefi client mistakenly moved SndNxt to the left one or multiple times. According to RFC 1122 (section 4.2.2.16), when server shrinks rcv buffer, usable window becomes negative. Below is a part of wireshark's dump of tcp transmission (with non-important parts removed). In frames 4490-4496 server reduces window size from 24576 to 5623, while ACKing relatively old packets (from 566378 to 585202). Client then sends packet with SEQ 589546 (frame 4496). In frame 4497 server ACKs segment from frame 4489 and in next frame it ACKs segment from frame 4496 with ACK=590994 (which means he accepted all 1448 data bytes from 4496 frame?). But on all zero window size packets (frames 4498-4515) client replies with SEQ 590754 and after window update (frame 4516) we have a deadlock starting from frame 4517. --- Frame 4489: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 588098, Ack: 26, Len: 1448 Sequence number: 588098 (relative sequence number) [Next sequence number: 589546 (relative sequence number)] Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] TCP segment data (1448 bytes) Frame 4490: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 566378, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 566378 (relative ack number) Window size value: 96 [Calculated window size: 24576] [Window size scaling factor: 256] Frame 4491: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 570722, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 570722 (relative ack number) Window size value: 79 [Calculated window size: 20224] [Window size scaling factor: 256] Frame 4492: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 573618, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 573618 (relative ack number) Window size value: 67 [Calculated window size: 17152] [Window size scaling factor: 256] Frame 4493: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Con4500trol Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 577962, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 577962 (relative ack number) Window size value: 50 [Calculated window size: 12800] [Window size scaling factor: 256] Frame 4494: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 582306, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 582306 (relative ack number) Window size value: 33 [Calculated window size: 8448] [Window size scaling factor: 256] Frame 4495: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 585202, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 585202 (relative ack number) Window size value: 22 [Calculated window size: 5632] [Window size scaling factor: 256] Frame 4496: 1514 bytes on wire (12112 bits), 1514 bytes captured (12112 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 589546, Ack: 26, Len: 1448 Sequence number: 589546 (relative sequence number) [Next sequence number: 590994 (relative sequence number)] Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] TCP segment data (1448 bytes) Frame 4497: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 589546, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 589546 (relative ack number) Window size value: 5 [Calculated window size: 1280] [Window size scaling factor: 256] Frame 4498: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] Frame 4500: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 590754, Ack: 26, Len: 0 Sequence number: 590754 (relative sequence number) Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] Frame 4501: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] Frame 4502: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 590754, Ack: 26, Len: 0 Sequence number: 590754 (relative sequence number) Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] Frame 4503: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] Frame 4504: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 590754, Ack: 26, Len: 0 Sequence number: 590754 (relative sequence number) Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] Frame 4505: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] Frame 4506: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 590754, Ack: 26, Len: 0 Sequence number: 590754 (relative sequence number) Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] Frame 4507: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] Frame 4508: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 590754, Ack: 26, Len: 0 Sequence number: 590754 (relative sequence number) Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] Frame 4509: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] Frame 4510: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 590754, Ack: 26, Len: 0 Sequence number: 590754 (relative sequence number) Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] Frame 4511: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] Frame 4512: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 590754, Ack: 26, Len: 0 Sequence number: 590754 (relative sequence number) Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] Frame 4513: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] Frame 4514: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 590754, Ack: 26, Len: 0 Sequence number: 590754 (relative sequence number) Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] Frame 4515: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 0 [Calculated window size: 0] [Window size scaling factor: 256] Frame 4516: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 260 [Calculated window size: 66560] [Window size scaling factor: 256] Frame 4517: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 590754, Ack: 26, Len: 0 Sequence number: 590754 (relative sequence number) Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] Frame 4518: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 1452 (1452), Dst Port: 80 (80), Seq: 590754, Ack: 26, Len: 0 Sequence number: 590754 (relative sequence number) Acknowledgment number: 26 (relative ack number) Window size value: 32767 [Calculated window size: 2097088] [Window size scaling factor: 64] Frame 4520: 66 bytes on wire (528 bits), 66 bytes captured (528 bits) on interface 0 Transmission Control Protocol, Src Port: 80 (80), Dst Port: 1452 (1452), Seq: 26, Ack: 590994, Len: 0 Sequence number: 26 (relative sequence number) Acknowledgment number: 590994 (relative ack number) Window size value: 260 [Calculated window size: 66560] [Window size scaling factor: 256] Regards, Andrey On 05.04.2017 12:47, Fu, Siyuan wrote: Hi, Andrey When the client received such a segment that moves the right edge of send window back to the left said, the client actually have no idea about whether the server is "shrinking the window" or just "reducing window size". Actually, if the server reduced the advertised windows size too much, which takes back his previous permission of sending a number of bytes, from the client side, the server is indeed shrink the receive window. This is discouraged by RFC793, while it says the client must prepare for the peer to do such kind of thing. Back to the original code, move the SndNxt to the "Right" will let client to retransmit some bytes of data (from Right to SndNxt), but I don't see a condition that it will cause deadlock. While if we adopt this patch, the SndNxt is unchanged when we got a positive usable window, and in the meanwhile, if server did shrink the window and dropped the data from Right to SndNxt, this piece of data won't be retransmit by the client anymore, this is actually a deadlock. So please provide more details if you did observe a deadlock, maybe an example would help to understand the problem. Thanks, Siyuan -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of atepin@kraftway.ru Sent: 2017年3月28日 15:20 To: edk2-devel@lists.01.org Subject: [edk2] [PATCH 2/2] NetworkPkg/TcpDxe: Fix unconditional window shrinking Moving Right window edge to the left on sender side without additional checks leads to the situation when sender assumes the receiver shrunk its rcv buffer, when, in fact, it only reduced window size. This is a TCP deadlock situation. Receiver ACKs proper segment, while sender discards it for future ACK. Add check for negative usable window to prevent erroneous window shrinking. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Andrey Tepin --- NetworkPkg/TcpDxe/TcpInput.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/NetworkPkg/TcpDxe/TcpInput.c b/NetworkPkg/TcpDxe/TcpInput.c index 04c8a82..11b3eb8 100644 --- a/NetworkPkg/TcpDxe/TcpInput.c +++ b/NetworkPkg/TcpDxe/TcpInput.c @@ -738,6 +738,7 @@ TcpInput ( TCP_SEQNO Right; TCP_SEQNO Urg; UINT16 Checksum; + INT32 UsableWnd; ASSERT ((Version == IP_VERSION_4) || (Version == IP_VERSION_6)); @@ -1307,7 +1308,10 @@ TcpInput ( if (TCP_SEQ_LT (Right, Tcb->SndNxt)) { - Tcb->SndNxt = Right; + UsableWnd = Tcb->SndUna + Tcb->SndWnd - Tcb->SndNxt; + if (UsableWnd < 0) { + Tcb->SndNxt = Right; + } if (Right == Tcb->SndUna) { -- 1.9.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel