From de921082177f868c93d42f6b0d2a4370913aa4ae Mon Sep 17 00:00:00 2001 From: Natale Patriciello Date: Tue, 23 Feb 2016 09:47:50 +0100 Subject: [PATCH] internet: (fixes #1783) BytesInFlight conforms to RFC 4898 Thanks to Alexander Krotov for a first version of this patch Thanks to Lynne Salameh for testing --- RELEASE_NOTES | 1 + src/internet/model/tcp-socket-base.cc | 35 ++++++++++++++++-- src/internet/model/tcp-socket-base.h | 1 + ...ns3tcp-loss-NewReno1-response-vectors.pcap | Bin 16216 -> 16216 bytes ...ns3tcp-loss-NewReno2-response-vectors.pcap | Bin 16296 -> 16296 bytes ...ns3tcp-loss-NewReno3-response-vectors.pcap | Bin 16376 -> 16376 bytes ...ns3tcp-loss-NewReno4-response-vectors.pcap | Bin 16456 -> 16456 bytes .../ns3tcp-state1-response-vectors.pcap | Bin 9976 -> 9976 bytes .../ns3tcp-state8-response-vectors.pcap | Bin 536 -> 500 bytes 9 files changed, 33 insertions(+), 4 deletions(-) diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 0f1fad154..8ad8aa32f 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -58,6 +58,7 @@ Bugs fixed - Bug 1205 - EDCA is incorrectly modelled as DCF - Bug 1571 - TCP zero-window and flow control window updates by the receiver - Bug 1761 - Rounding with olsr::EmfToSeconds +- Bug 1783 - BytesInFlight value fixed - Bug 1954 - Serialized size of wifi-net-device differ for TX and RX trace - Bug 1999 - PointToPointRemoteChannel invokes PointToPointChannel constructor - Bug 2003 - Missing DSSS short PLCP preamble diff --git a/src/internet/model/tcp-socket-base.cc b/src/internet/model/tcp-socket-base.cc index de6bfc917..6fd6e9111 100644 --- a/src/internet/model/tcp-socket-base.cc +++ b/src/internet/model/tcp-socket-base.cc @@ -299,6 +299,7 @@ TcpSocketBase::TcpSocketBase (void) m_recover (0), // Set to the initial sequence number m_retxThresh (3), m_limitedTx (false), + m_retransOut (0), m_congestionControl (0), m_isFirstPartialAck (true) { @@ -368,6 +369,7 @@ TcpSocketBase::TcpSocketBase (const TcpSocketBase& sock) m_recover (sock.m_recover), m_retxThresh (sock.m_retxThresh), m_limitedTx (sock.m_limitedTx), + m_retransOut (sock.m_retransOut), m_isFirstPartialAck (sock.m_isFirstPartialAck), m_txTrace (sock.m_txTrace), m_rxTrace (sock.m_rxTrace) @@ -1517,6 +1519,7 @@ TcpSocketBase::ReceivedAck (Ptr packet, const TcpHeader& tcpHeader) m_tcb->m_congState = TcpSocketState::CA_OPEN; m_congestionControl->PktsAcked (m_tcb, segsAcked, m_lastRtt); m_dupAckCount = 0; + m_retransOut = 0; NS_LOG_DEBUG ("DISORDER -> OPEN"); } @@ -1550,6 +1553,7 @@ TcpSocketBase::ReceivedAck (Ptr packet, const TcpHeader& tcpHeader) callCongestionControl = false; // No congestion control on cWnd show be invoked m_dupAckCount -= segsAcked; // Update the dupAckCount + m_retransOut--; // at least one retransmission has reached the other side m_txBuffer->DiscardUpTo (ackNumber); //Bug 1850: retransmit before newack DoRetransmit (); // Assume the next seq is lost. Retransmit lost packet @@ -1579,6 +1583,7 @@ TcpSocketBase::ReceivedAck (Ptr packet, const TcpHeader& tcpHeader) BytesInFlight () + m_tcb->m_segmentSize); m_isFirstPartialAck = true; m_dupAckCount = 0; + m_retransOut = 0; /* This FULL ACK acknowledge the fact that one segment has been * previously lost and now successfully received. All others have @@ -1600,6 +1605,7 @@ TcpSocketBase::ReceivedAck (Ptr packet, const TcpHeader& tcpHeader) m_isFirstPartialAck = true; m_congestionControl->PktsAcked (m_tcb, segsAcked, m_lastRtt); m_dupAckCount = 0; + m_retransOut = 0; m_tcb->m_congState = TcpSocketState::CA_OPEN; NS_LOG_DEBUG ("LOSS -> OPEN"); } @@ -2580,7 +2586,26 @@ uint32_t TcpSocketBase::BytesInFlight () { NS_LOG_FUNCTION (this); - uint32_t bytesInFlight = m_highTxMark.Get () - m_txBuffer->HeadSequence (); + // Previous (see bug 1783): + // uint32_t bytesInFlight = m_highTxMark.Get () - m_txBuffer->HeadSequence (); + // RFC 4898 page 23 + // PipeSize=SND.NXT-SND.UNA+(retransmits-dupacks)*CurMSS + + // flightSize == UnAckDataCount (), but we avoid the call to save log lines + uint32_t flightSize = m_nextTxSequence.Get () - m_txBuffer->HeadSequence (); + uint32_t duplicatedSize; + uint32_t bytesInFlight; + + if (m_retransOut > m_dupAckCount) + { + duplicatedSize = (m_retransOut - m_dupAckCount)*m_tcb->m_segmentSize; + bytesInFlight = flightSize + duplicatedSize; + } + else + { + duplicatedSize = (m_dupAckCount - m_retransOut)*m_tcb->m_segmentSize; + bytesInFlight = duplicatedSize > flightSize ? 0 : flightSize - duplicatedSize; + } // m_bytesInFlight is traced; avoid useless assignments which would fire // fruitlessly the callback @@ -2914,9 +2939,6 @@ TcpSocketBase::Retransmit () * are not able to retransmit anything because of local congestion. */ - m_nextTxSequence = m_txBuffer->HeadSequence (); // Restart from highest Ack - m_dupAckCount = 0; - if (m_tcb->m_congState != TcpSocketState::CA_LOSS) { m_tcb->m_congState = TcpSocketState::CA_LOSS; @@ -2924,6 +2946,9 @@ TcpSocketBase::Retransmit () m_tcb->m_cWnd = m_tcb->m_segmentSize; } + m_nextTxSequence = m_txBuffer->HeadSequence (); // Restart from highest Ack + m_dupAckCount = 0; + NS_LOG_DEBUG ("RTO. Reset cwnd to " << m_tcb->m_cWnd << ", ssthresh to " << m_tcb->m_ssThresh << ", restart from seqnum " << m_nextTxSequence); DoRetransmit (); // Retransmit the packet @@ -2971,6 +2996,8 @@ TcpSocketBase::DoRetransmit () // Retransmit a data packet: Call SendDataPacket uint32_t sz = SendDataPacket (m_txBuffer->HeadSequence (), m_tcb->m_segmentSize, true); + ++m_retransOut; + // In case of RTO, advance m_nextTxSequence m_nextTxSequence = std::max (m_nextTxSequence.Get (), m_txBuffer->HeadSequence () + sz); diff --git a/src/internet/model/tcp-socket-base.h b/src/internet/model/tcp-socket-base.h index d08e85f1d..173e4548a 100644 --- a/src/internet/model/tcp-socket-base.h +++ b/src/internet/model/tcp-socket-base.h @@ -967,6 +967,7 @@ protected: SequenceNumber32 m_recover; //!< Previous highest Tx seqnum for fast recovery uint32_t m_retxThresh; //!< Fast Retransmit threshold bool m_limitedTx; //!< perform limited transmit + uint32_t m_retransOut; //!< Number of retransmission in this window // Transmission Control Block Ptr m_tcb; //!< Congestion control informations diff --git a/src/test/ns3tcp/response-vectors/ns3tcp-loss-NewReno1-response-vectors.pcap b/src/test/ns3tcp/response-vectors/ns3tcp-loss-NewReno1-response-vectors.pcap index 6efab600ac332e18a3e4f09359150b490b1fbfbb..2a4200d1f77895d3af5fc3403274110b0358b8d6 100644 GIT binary patch delta 1471 zcmYk*K}ef-9LDjOx2Z9Swuzc1QCn@bCb4O&tu;;R8_HHt!^`L`C_N0iVf8wO%~;tE z0~fbJc{Zqq9=4NJIySa4SJ|xtb?iK~D@?|QCYNn6NDYO3Yu>|7pXUvK^1Q#_`@WlZ zHt&2j>CuWOpuESF+Z)gZ<}l(lwTM$#!N)k_3&`U$bq~ie*%;6g&f-39;#i}pXj4E7 zxP~PR9SJDaWNH!%=sp_Iwr%_feO%{p5jU`ck)x(|{q$opKtIl+$DdMnb5P^>7B1QD z;}fpO0x7k$1hs%Su@s=MmHuY_ztyh~n=`857Tyc`6>G_87jI+om|yESi{aLn(%;Hl z_!y%h`h%usa2(fg2`9tMkGTl*AEO5su!1EVX`?^H-mqZ1jk{dO!=~}#?RrgzEhXET zKg^sMiO`EvIC-3YTtQD8^PHd`bGU?yxQ`VaYd4MgZ#c_;Kbq$^J2Pihi8L$O-yz3f(?s{^LB7`P z)UutQr=64qQ={evErqVoKgisex-3JDx4cgMWap1BI@NZicIN60r>5=v?*~q;+WEnkPI-rGXKwuB)P$Y=4nxa! j{xNE(YUkV64Q0*R`9$7O(at+x7%H2V(Y3=nmFf5o2|&s~ delta 1421 zcmX}sUr3u*7{~GGYomEh8WT-S;-A(((U{oI>Z)n7zF`**qr8Q!URcNO+~$N01`{;0 z-fS2CVQk|EL+agTp<0A73UkU}g$-QXuGT^{lx;Vo=+&Tw+TQGo3A_571Lw{2JI{GY z;b!6Hx6@|j@cWuEuW8!!D{orb#z?KDnp(eRZ~}Mm4w`OD&u|h)+18_WqkZkt#-Y(_Si@2EKPeUkK4F| zHFcEtrK&3&4oAgzZjJe}|F744S&i2P_1u@zI*$5NdV;t7rSstquZn(KBR;a!TUy0s zO!~=!ix_Vp8|E4;!}rjk-3D8+da?w_TJO_)fsE$x2i!wfFk{TN7#a)O3N`q&5zJ^B zPjDNvjTub@=tU!~!y|l#sSvFTQXg0Guxva`@7U+UmT~a*8SRE`!#C+vZNyfxkzPc| zi91+8vx!_Go&x7^6GLIYp5r!-Hq)~R^>G4^nk{4Y>l!6m>{A!8XTL`Kxb>AwO|7=> za4y$MPR!#-6ZbzyPMpEVxP!@N>Yt}>3-xgiH!;*o{qywi&swG1Y~z<|w^qs)alX&3 zM0>Wnk~hr$;nqTXR#UA(9k!=*591vvRhD5q-DMj~*Bx3ZTg2PHJCx|QHP_BNbdw(sG1S2v#F#zI z;Wp;6rjtJOaQ`TMz+@MFz{M`+Af7Q?LFOQC8$Vix*6_o34IN|j3q$FIZ5(VHTESP% zDizBndTkZ>eyKO3Shr6HIEOtKm@k~iP|T-aaT>!HGsaTkjNZIx8&@lwx{LqzIQ0T= zzv0x_OSW-i!KpR$tUGm#_kVLL-N$=Bb!r74y;iMapRL6Nb9jmTxQqon#8@x)@8?}H zhkKaE&;{;4KwofXz%rr%m+lU5ud6P-z|Y=sY3yb0_qj`J`1E_1j&b8Jm(rJQ<1Lq| z74-F+D&qVnrV@kX=%fB1_3;p6m#9BfGKX{Q^SFnRBsu!2pG;}0pBcb=XkJNG8_d)F z*ZEJll099|=G$tprwC`3rRXX$K6K1o#9&YidSYDqDdz46*&TM_)(L#A0 z_{pRFbXE^8GpDaGcZ19+euR1a86#Iq&S1k38F30ti>yNd-LTTCN(Qun<9K3ajF#)n HNT%gKYJ$Lq diff --git a/src/test/ns3tcp/response-vectors/ns3tcp-loss-NewReno2-response-vectors.pcap b/src/test/ns3tcp/response-vectors/ns3tcp-loss-NewReno2-response-vectors.pcap index 0ef7f7702a22c4ba7266616eaa0f2804ac0366e0..23f0c2c8e74b824ececf92eb62ad241aaf744303 100644 GIT binary patch delta 1415 zcmXZbPe|Km9LMpHCt@07+eG~}Ms0P~Hh-$Mt=jlY{e?mcQTd(b!CHD5JE&!c!3LTZ z%3y;S>0x{}aC)tfWw1XmSe9NlunIeEOjE`v?2zfHY-N&O_NK|{^?ky_@AG`W&y(V# z;-haD11bl?$^{(x+rrw#d5pF>D&jI8V+FIpcN7RZO0|bo!aUY-A7|Pf#ZHE`i7R-B zJ7}MD6gd@E0n1oL|LL$2r}#eR@ECV78zT3#qYYecTEQCYK*-j|VM`16E0%Du!%{uO z_dCfCJ6gm|EaM^iyU6coSmST^a4;62>C* z-9S$2U zdtEbpC#YnfYhHX5)C2sy6x3dys~Y{U-)=P?k7w;|kB<7M-_%nNJxUKW>X%PES|6C!tszT?1GYXx`>lor{EKxJ$1Ye( zoVWGCMN2vS33stGW+^(z8Dg9T{c+BM^B5R%%t6^`cE~l4Ul?r-xjN?e`Vccngfx92 zqYeB5YxsB5$%`4Kk|C{O>{3VvxPtLmMmxBQkzw8fE@F2)qcSex{76VY#xo{0ef zZGDe9^i6U9IET>`Zw;66U##FQ$5LR@ZuRl@4%)n$ot%DCKOXaHt6AT?=hacOe)Wl0 zX}huK%dfp!Z`SvI_v(4G4tb1{>BgR)V@3~}^?TnL?WJ8aP&Mkg(eOV0=Jn|*t|fhH zn{v(6oKJW0%o{9gBY`D zJYdno@^J}+#M2Iy&BVkVupZPzBP;P@JXFwwQIifX9?%TP65p|nr`PlJ_cY(%^Z9;% z`5XBgpJjZtN4J#q%`4*zs_3)Y!f2zFw=t+JE?@!kxY%U%sPVcso2bFzBgZj9PB`kIj=Rmz0qpnHM*XtTSexqJ@tDZz%4M&K- zllX8GxA7Pw9mL;d&5K_(Xtm2#KFa;OsEbaw)#Xm=cUv=l-KU*y*Zi6Dsq>6$Zh!96 zE9m~>)0byxBj=)L$q%zw#sVh0$iK(YdtKz;!n9$ z!&>jHocZBnzxI05|DAv5d%qHWmG$&re!W#)Ka4bMr!TE5{4aja(H&f_TEs*4(SGjG z!#m*`7W%EZ`j1B^{jN&AWHLbS`p5*g@faiL$Yju(U%#o*>Y!_G?$>A!pJ}aCV#rm7 z=eZ$f3wP1mPfvy!bzH7m#60_Gj4Ux5v*z!KI^B)ADi4rJoNNY}iFisY_z9Nq2$zNe zN+(igWayMu6R!E~`BU0UxZ39Tne!<{h68#ZGx#%ZVQeHNZ!DlUaRDDx?HFa2*uOlQ zGB;mkCPta{IQ1`(2QFe6H!+#u{Y^o}x5w7DOxQ|oQVL7wRHRkXq+@0n7x=0pJlFcQuzzh~~3!_hw pO_Eu_1uS45eU=%)MQhD#g&J*G*9`n#ql2omDc5w&F$<~ge*tux(>eeE diff --git a/src/test/ns3tcp/response-vectors/ns3tcp-loss-NewReno3-response-vectors.pcap b/src/test/ns3tcp/response-vectors/ns3tcp-loss-NewReno3-response-vectors.pcap index a8c1e9dce1be23e769758e111d6ac43878c8da7c..8c049edaab3a4fe5a072680cbc911c8390967802 100644 GIT binary patch delta 1385 zcmYk*PfQed7{>99v&$~Cfb0MREV9V|Wf%V;>N3DgOte`rz0+KuCLO4+8O^@qlnLq3yC>`iuX==r^G;6wwL@ll37{;TnmT(FS(Q8`8oeE166|ORsmhw1{MSO{)m9COi*ObGx zDoeXKdDzlIm8%up!br8H)M4I_v$%xoSipF-t89#V)sEiAY^~y5%a$2)^e0Zlcz+Fb zaR!SeM=k1c&spxF@o%+dUDisiC8vgRb(F_KT}+!bp0ajKJ++<|@iy+@i;|6Xo-*|@ z-Kg`7Cy_EO*ZF!`Z^^VN+dyobY@|G{U=bs6V%HM~bGU@N^{#R8V!6@{{zvEV;L~z# zH2BIk5x>#ZBFZP|a3>YhNEf3_$Mg{A@m-f^y!s@pbhmFj_$;isZbr-V&2CT0UREOMX$EhXe1U7+ z8+$xOjdKfs*5 zn4aJ~9t?QK`0qgtI=)f)Pf+*pmzI#W@$+m*=|SJwckm}t}lAu%!0$XLA11X(;7CF$_uVTZ0H#&@8nSD!!0_xt;PKTk_{ zOLxD@S~Xw(pqzC>Su3Ce%h3)-ypBBHfO5EiR)eEW@1imdj!B(*w9(*dz9FD8_Y0Wz zIV$1_CVTLDB9#|nd_A%NBeky;U?-eJIZ3N+19sBA?;#q zb4Zro)@zvYJLbiSDsA{(Z8lTaPd}IrI4a>f&ikp41+-h}2lKcZpni*EZhl^)Xsc`f zc*kg|)m5y8`mK(ZF%mpC7j(??qS1EH)qaa#7eYyeTm8C;S^NTb@Hs97{YteZ&7JKR zbid72DM-BEfHK`l*dQ>sm?yA7^m3I2W15C70KSH0F!^5^f&D<@$ z-8^T4#S1!(xC*xil!-DT+=@D8e9_WBQP&*jEzNegX6+kG_bWc>aJ9pGb|)*0FoI4- z7zybJA7H%8)-zn`vg>QAs;a&o@M*6rrF$JAMY?T0#$5OLvtK{;X}ddh_TKxy_;gzN zJ@v0ouk@VXe;RJoMo&ruogtNb>@(B7wr+F1-piUh84HHH=&6r0V7|{WzV2#0>LW%y z0mb`S8?N*_W_Ybyd;P5G$7*>8T=UUMwO+@UTWYjD;Ht=b)*xf+WhNE3aFy#a9%A|; zvFjrixQr*b(@*^&;ygfo+{B|H$NcnVm~e$9*!Bm4k8!+!lfk~G0tkD{Zl zdH9A$OZeW09_@{~THty6<)k+87MAdPoR1UBv7`!d>W|R}ZdEK};u7b-!aQ&tVm^lULY3-oq1ojP^D1F&;5vl%TOvAr?g!HEz2|-3 zE8l7?-)j6JV`^OeCoZniIJ2h$yn;_dz$KmMj{drEV*t7wO(uP}>yxZT0^4&`jF#|5ln z31deb9b%-Dat_yH0XMOR=26NXd2JrkoOiH@@lIa{n8P*K)~}sOt#;Y!KAKb&-^Wf*!@rHyTrm0M!Ko*`o_I)!b)@n+T>j5;@&9r@$DY!N4OvJxQ{zH(@Xts@?frR zrQ0{=e{WH9G%#MhY|&ygP^{0;ry+sWYA)VF8EFyQ<+0^v*Kh3$F6`9`4{z7@uUnW|%)-#R{%t zbc+4LEY8jHJ=V3Rsmpm`n!FzU$5lMUDyGhnH$i?}#u}E7s!h_ zEMf%@Fgn9`$w*j>`1Lzs-NpHpuwLS; p@4~v`2S)41mhRxs>z1C@O(X;3qlcDmQZI^|spYx@D>0dg zhgyxv^3lkj#CTW?!;6QRU~fxUgf8A@f-D|PGy;C2zdzuSTzk|Lo zj|W)BSUczJbhL*#jC62b%%T}~jEN`bl@5E_=?Ln6*gf@m*fl;IX;iX{{yKwN?{c+> z&u|M95m)o!kctud!b7Z})$JJjZKjsHJr%mB&w6^vR8!P5ek+=KqvqF9?u++kkFDWI zSQ)&9+qjFz*wG!4a@F3+xy{*fL8lbr|;kYKA^|7 z=h;64>ghk*|NnWpKc&C;U%8(RlwA^s{>re z*ch3pnYiR?h4qKHhd*H?7F2w~RTgifKOWRRE@Jm2S;BX5-U{lwN!PgfWs^=OQ;i0* z?%r=wdMc$)txnyaO6pgOTuj-T87CK)ZGC_n_ynsse2GjI+pMrnltuk58pPlDm~+Aeu{5%hWo-}tl+{ddAZCSuz*!OK>Hfs z+%$8jd59~lt?SGO^BB3p959Q~Ip%{o^j~ETxQJ$goM0wF9%6WbG#JkV~4ZC1*D|LSATcm7EL5-6k4lp|Ax;CYiwcuV5lsW{4GXi2 GA#DY|L?q?_ diff --git a/src/test/ns3tcp/response-vectors/ns3tcp-state8-response-vectors.pcap b/src/test/ns3tcp/response-vectors/ns3tcp-state8-response-vectors.pcap index 06f617f33cd6e506422b97c6dcefdf1fd83efb7d..daed0da6e9fc818af509eefb6a0b0e531769b376 100644 GIT binary patch delta 7 OcmbQi@`ZWB7e)XL5Cb^? delta 44 tcmeyuJcDJ!7e*CE1_lQ9zYGi_KpKP(FdhzIU=Z#CGMNGd8h{i70|4F?2i*Vw