All of lore.kernel.org
 help / color / mirror / Atom feed
From: Per Hurtig <per.hurtig@kau.se>
To: Nandita Dukkipati <nanditad@google.com>
Cc: "Eric Dumazet" <eric.dumazet@gmail.com>,
	Netdev <netdev@vger.kernel.org>,
	"Anna Brunström" <anna.brunstrom@kau.se>,
	mohammad.rajiullah@kau.se, "Neal Cardwell" <ncardwell@google.com>,
	"Sergei Shtylyov" <sergei.shtylyov@cogentembedded.com>
Subject: Re: [PATCH] tcp: fixing TLP's FIN recovery
Date: Mon, 09 Jun 2014 15:13:32 +0200	[thread overview]
Message-ID: <5395B2FC.3070205@kau.se> (raw)
In-Reply-To: <CAB_+Fg5=Z0wfaAZgUMXKttHKx9WoVLQv5ZqDeUQJmGA0LJTwPg@mail.gmail.com>

See inline,

On 2014-06-09 09:02, Nandita Dukkipati wrote:
> On Sun, Jun 8, 2014 at 12:41 AM, Per Hurtig <per.hurtig@kau.se> wrote:
>>
>>
>> On sön  8 jun 2014 04:58:25, Eric Dumazet wrote:
>>>
>>> On Sat, 2014-06-07 at 16:34 +0200, Per Hurtig wrote:
>>>>
>>>> Fix to a problem observed when losing a FIN segment that does not
>>>> contain data.  In such situations, TLP is unable to recover from
>>>> *any* tail loss and instead adds at least PTO ms to the
>>>> retransmission process, i.e., RTO = RTO + PTO.
>>>>
>>>> Signed-off-by: Per Hurtig <per.hurtig@kau.se>
>>>> ---
>>>>    net/ipv4/tcp_output.c | 6 ++++--
>>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>>>> index d463c35..6573765 100644
>>>> --- a/net/ipv4/tcp_output.c
>>>> +++ b/net/ipv4/tcp_output.c
>>>> @@ -2130,8 +2130,10 @@ void tcp_send_loss_probe(struct sock *sk)
>>>>          if (WARN_ON(!skb || !tcp_skb_pcount(skb)))
>>>>                  goto rearm_timer;
>>>>
>>>> -       /* Probe with zero data doesn't trigger fast recovery. */
>>>> -       if (skb->len > 0)
>>>> +       /* Probe with zero data doesn't trigger fast recovery, if FIN
>>>> +        * flag is not set.
>>>> +        */
>>>> +       if ((skb->len > 0) || (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN))
>>>>                  err = __tcp_retransmit_skb(sk, skb);
>>>>
>>>>          /* Record snd_nxt for loss detection. */
>>>
>>>
>>>
>>> You know, I believe the test was exactly to avoid sending data less FIN
>>> packets.
>>>
>>> If you write :
>>>
>>>       if (A  || !A)
>>>
>>> Better remove the condition, completely ;)
>>>

After looking more closely, I see that we only enter this part when the 
FIN flag is set on an otherwise empty segment. I guess I was distracted 
by the comment that suggested a more general scenario than the actual 
one, a bit confusing ;)

>> Obviously, but I don't think that FINs are the only segments
>> who are targeted by this condition (or targeted at all given
>> the implications of this statement). Furthermore, the comment above
>> the if statement would probably have mentioned FINs explicity
>> and not zero sized segments in general if this were the case.
>>
>>
>>
>>>
>>> Nandita, why FIN packet wont trigger fast retransnmits ?
>>>
>>
>> They do, that's the whole thing with this patch.
>>
>>
>>> It sounds like if the timer is the issue you want to fix, you might
>>> simply rearm a timer with RTO-PTO instead of RTO ?
>>>
>>>
>> No I want to enable TLP for tail loss where an empty FIN is involved,
>> this does not work now.
>
> I understand the tail loss case you want to solve - essentially when a
> tail loss occurs that involves data segments as well as that of an
> empty FIN. However, have you verified that re-sending an empty FIN
> triggers fast recovery? I would be surprised if it did, because I
> think the sender needs to receive a SACK of at least 1-byte of data
> before sender can trigger FACK based fast recovery.
>
Yes, it needs a SACK that covers one "sequence number", which a FIN
does. I don't see why it shouldn't generate a SACK? See below for some 
packet dumps.

> If you have verified that a pure FIN does indeed trigger recovery, can
> you tell me what part of the code makes that happen?
>

Scenario:
Eleven segments are sent back-to-back (ten data and one empty FIN), the 
last three segments (the FIN + two others) are dropped.

Other relevant info: RTT of 20ms.

Transfer time of the entire flow (from the receiver's point of view):
TCP w TLP: 324ms
TCP w modified TLP: 122ms


Detailed TLP behavior:
The entire transfer including retransmissions takes approx 324ms. The 
retransmissions are conducted in frames 23 and 25.

Sender-side packet trace:
1   0.000000     10.0.1.1 -> 10.0.2.1     TCP 74 36713 > search-agent 
[SYN] Seq=0 Win=29200 Len=0 MSS=1460 SACK_PERM=1 TSval=1171292524 
TSecr=0 WS=128
2   0.000020     10.0.2.1 -> 10.0.1.1     TCP 74 search-agent > 36713 
[SYN, ACK] Seq=0 Ack=1 Win=28960 Len=0 MSS=1460 SACK_PERM=1 
TSval=1171296150 TSecr=1171292524 WS=128
3   0.019818     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[ACK] Seq=1 Ack=1 Win=29312 Len=0 TSval=1171292529 TSecr=1171296150
4   0.019854     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 36713 
[PSH, ACK] Seq=1 Ack=1 Win=29056 Len=1448 TSval=1171296155 
TSecr=1171292529[Packet size limited during capture]
5   0.019864     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 36713 
[PSH, ACK] Seq=1449 Ack=1 Win=29056 Len=1448 TSval=1171296155 
TSecr=1171292529[Packet size limited during capture]
6   0.019868     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 36713 
[PSH, ACK] Seq=2897 Ack=1 Win=29056 Len=1448 TSval=1171296155 
TSecr=1171292529[Packet size limited during capture]
7   0.019871     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 36713 
[PSH, ACK] Seq=4345 Ack=1 Win=29056 Len=1448 TSval=1171296155 
TSecr=1171292529[Packet size limited during capture]
8   0.019875     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 36713 
[PSH, ACK] Seq=5793 Ack=1 Win=29056 Len=1448 TSval=1171296155 
TSecr=1171292529[Packet size limited during capture]
9   0.019878     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 36713 
[PSH, ACK] Seq=7241 Ack=1 Win=29056 Len=1448 TSval=1171296155 
TSecr=1171292529[Packet size limited during capture]
10   0.019881     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 36713 
[PSH, ACK] Seq=8689 Ack=1 Win=29056 Len=1448 TSval=1171296155 
TSecr=1171292529[Packet size limited during capture]
11   0.019922     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 36713 
[PSH, ACK] Seq=10137 Ack=1 Win=29056 Len=1448 TSval=1171296155 
TSecr=1171292529[Packet size limited during capture]
12   0.019929     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 36713 
[PSH, ACK] Seq=11585 Ack=1 Win=29056 Len=1448 TSval=1171296155 
TSecr=1171292529[Packet size limited during capture]
13   0.019930     10.0.2.1 -> 10.0.1.1     TCP 1513 search-agent > 36713 
[PSH, ACK] Seq=13033 Ack=1 Win=29056 Len=1447 TSval=1171296155 
TSecr=1171292529[Packet size limited during capture]
14   0.019971     10.0.2.1 -> 10.0.1.1     TCP 66 search-agent > 36713 
[FIN, ACK] Seq=14480 Ack=1 Win=29056 Len=0 TSval=1171296155 TSecr=1171292529
15   0.039635     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[ACK] Seq=1 Ack=1449 Win=32128 Len=0 TSval=1171292534 TSecr=1171296155
16   0.039643     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[ACK] Seq=1 Ack=2897 Win=35072 Len=0 TSval=1171292534 TSecr=1171296155
17   0.039646     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[ACK] Seq=1 Ack=4345 Win=37888 Len=0 TSval=1171292534 TSecr=1171296155
18   0.039650     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[ACK] Seq=1 Ack=5793 Win=40832 Len=0 TSval=1171292534 TSecr=1171296155
19   0.039653     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[ACK] Seq=1 Ack=7241 Win=43520 Len=0 TSval=1171292534 TSecr=1171296155
20   0.039655     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[ACK] Seq=1 Ack=8689 Win=43520 Len=0 TSval=1171292534 TSecr=1171296155
21   0.039657     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[ACK] Seq=1 Ack=10137 Win=43520 Len=0 TSval=1171292534 TSecr=1171296155
22   0.039660     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[ACK] Seq=1 Ack=11585 Win=43520 Len=0 TSval=1171292534 TSecr=1171296155
23   0.283780     10.0.2.1 -> 10.0.1.1     TCP 1514 [TCP Retransmission] 
search-agent > 36713 [PSH, ACK] Seq=11585 Ack=1 Win=29056 Len=1448 
TSval=1171296221 TSecr=1171292534[Packet size limited during capture]
24   0.303267     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[ACK] Seq=1 Ack=13033 Win=43520 Len=0 TSval=1171292600 TSecr=1171296221
25   0.303276     10.0.2.1 -> 10.0.1.1     TCP 1513 [TCP Retransmission] 
search-agent > 36713 [FIN, PSH, ACK] Seq=13033 Ack=1 Win=29056 Len=1447 
TSval=1171296225 TSecr=1171292600[Packet size limited during capture]
26   0.324085     10.0.1.1 -> 10.0.2.1     TCP 66 36713 > search-agent 
[FIN, ACK] Seq=1 Ack=14481 Win=43520 Len=0 TSval=1171292605 TSecr=1171296225
27   0.324093     10.0.2.1 -> 10.0.1.1     TCP 66 search-agent > 36713 
[ACK] Seq=14481 Ack=2 Win=29056 Len=0 TSval=1171296231 TSecr=1171292605

----

Modified TLP behavior:
The entire transfer including retransmissions takes approx 122ms.

The TLP probe is sent in frame 23 below, and you can see in frame 24 
below that a SACK covering one sequence number is returned from the 
receiver and used to trigger retransmissions of the other lost segments.

Sender-side packet trace:
1   0.000000     10.0.1.1 -> 10.0.2.1     TCP 74 37730 > search-agent 
[SYN] Seq=0 Win=29200 Len=0 MSS=1460 SACK_PERM=1 TSval=1194757582 
TSecr=0 WS=128
2   0.000021     10.0.2.1 -> 10.0.1.1     TCP 74 search-agent > 37730 
[SYN, ACK] Seq=0 Ack=1 Win=28960 Len=0 MSS=1460 SACK_PERM=1 TSval=222654 
TSecr=1194757582 WS=128
3   0.020765     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[ACK] Seq=1 Ack=1 Win=29312 Len=0 TSval=1194757587 TSecr=222654
4   0.020800     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 37730 
[PSH, ACK] Seq=1 Ack=1 Win=29056 Len=1448 TSval=222659 
TSecr=1194757587[Packet size limited during capture]
5   0.020810     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 37730 
[PSH, ACK] Seq=1449 Ack=1 Win=29056 Len=1448 TSval=222659 
TSecr=1194757587[Packet size limited during capture]
6   0.020814     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 37730 
[PSH, ACK] Seq=2897 Ack=1 Win=29056 Len=1448 TSval=222659 
TSecr=1194757587[Packet size limited during capture]
7   0.020818     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 37730 
[PSH, ACK] Seq=4345 Ack=1 Win=29056 Len=1448 TSval=222659 
TSecr=1194757587[Packet size limited during capture]
8   0.020821     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 37730 
[PSH, ACK] Seq=5793 Ack=1 Win=29056 Len=1448 TSval=222659 
TSecr=1194757587[Packet size limited during capture]
9   0.020824     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 37730 
[PSH, ACK] Seq=7241 Ack=1 Win=29056 Len=1448 TSval=222659 
TSecr=1194757587[Packet size limited during capture]
10   0.020827     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 37730 
[PSH, ACK] Seq=8689 Ack=1 Win=29056 Len=1448 TSval=222659 
TSecr=1194757587[Packet size limited during capture]
11   0.020870     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 37730 
[PSH, ACK] Seq=10137 Ack=1 Win=29056 Len=1448 TSval=222659 
TSecr=1194757587[Packet size limited during capture]
12   0.020877     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 37730 
[PSH, ACK] Seq=11585 Ack=1 Win=29056 Len=1448 TSval=222659 
TSecr=1194757587[Packet size limited during capture]
13   0.020879     10.0.2.1 -> 10.0.1.1     TCP 1514 search-agent > 37730 
[PSH, ACK] Seq=13033 Ack=1 Win=29056 Len=1448 TSval=222659 
TSecr=1194757587[Packet size limited during capture]
14   0.020918     10.0.2.1 -> 10.0.1.1     TCP 66 search-agent > 37730 
[FIN, ACK] Seq=14481 Ack=1 Win=29056 Len=0 TSval=222659 TSecr=1194757587
15   0.040583     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[ACK] Seq=1 Ack=1449 Win=32128 Len=0 TSval=1194757593 TSecr=222659
16   0.040591     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[ACK] Seq=1 Ack=2897 Win=35072 Len=0 TSval=1194757593 TSecr=222659
17   0.040594     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[ACK] Seq=1 Ack=4345 Win=37888 Len=0 TSval=1194757593 TSecr=222659
18   0.040597     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[ACK] Seq=1 Ack=5793 Win=40832 Len=0 TSval=1194757593 TSecr=222659
19   0.040599     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[ACK] Seq=1 Ack=7241 Win=43520 Len=0 TSval=1194757593 TSecr=222659
20   0.040601     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[ACK] Seq=1 Ack=8689 Win=43520 Len=0 TSval=1194757593 TSecr=222659
21   0.040604     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[ACK] Seq=1 Ack=10137 Win=43520 Len=0 TSval=1194757593 TSecr=222659
22   0.040606     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[ACK] Seq=1 Ack=11585 Win=43520 Len=0 TSval=1194757593 TSecr=222659
23   0.078751     10.0.2.1 -> 10.0.1.1     TCP 66 [TCP Retransmission] 
search-agent > 37730 [FIN, ACK] Seq=14481 Ack=1 Win=29056 Len=0 
TSval=222674 TSecr=1194757593
24   0.098093     10.0.1.1 -> 10.0.2.1     TCP 78 [TCP Dup ACK 22#1] 
37730 > search-agent [ACK] Seq=1 Ack=11585 Win=43520 Len=0 
TSval=1194757607 TSecr=222659 SLE=14481 SRE=14482
25   0.102752     10.0.2.1 -> 10.0.1.1     TCP 1514 [TCP Retransmission] 
search-agent > 37730 [PSH, ACK] Seq=11585 Ack=1 Win=29056 Len=1448 
TSval=222680 TSecr=1194757607[Packet size limited during capture]
26   0.102757     10.0.2.1 -> 10.0.1.1     TCP 1514 [TCP Retransmission] 
search-agent > 37730 [PSH, ACK] Seq=13033 Ack=1 Win=29056 Len=1448 
TSval=222680 TSecr=1194757607[Packet size limited during capture]
27   0.121854     10.0.1.1 -> 10.0.2.1     TCP 78 37730 > search-agent 
[ACK] Seq=1 Ack=13033 Win=43520 Len=0 TSval=1194757613 TSecr=222680 
SLE=14481 SRE=14482
28   0.121862     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[ACK] Seq=1 Ack=14482 Win=43520 Len=0 TSval=1194757613 TSecr=222680
29   0.121868     10.0.1.1 -> 10.0.2.1     TCP 66 37730 > search-agent 
[FIN, ACK] Seq=1 Ack=14482 Win=43520 Len=0 TSval=1194757613 TSecr=222680
30   0.121875     10.0.2.1 -> 10.0.1.1     TCP 66 search-agent > 37730 
[ACK] Seq=14482 Ack=2 Win=29056 Len=0 TSval=222684 TSecr=1194757613

----

Thanks,
Per

> Nandita
>

  reply	other threads:[~2014-06-09 13:21 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-06 18:46 tcp: fixing TLP's FIN recovery Per Hurtig
2014-06-06 19:07 ` Eric Dumazet
2014-06-07 11:10   ` [PATCH] " Per Hurtig
2014-06-07 13:56     ` Sergei Shtylyov
2014-06-07 14:34       ` Per Hurtig
2014-06-08  2:58         ` Eric Dumazet
2014-06-08  7:41           ` Per Hurtig
2014-06-08 16:35             ` Eric Dumazet
2014-06-09  7:04               ` Nandita Dukkipati
2014-06-09  7:02             ` Nandita Dukkipati
2014-06-09 13:13               ` Per Hurtig [this message]
2014-06-09 14:33                 ` Eric Dumazet
2014-06-09 14:39                   ` Eric Dumazet
2014-06-09 14:42                   ` Per Hurtig
2014-06-09 15:04                     ` Eric Dumazet
2014-06-09 15:56                       ` Per Hurtig
2014-06-09 16:15                         ` Eric Dumazet
2014-06-09 16:24                           ` Eric Dumazet
2014-06-09 18:33                             ` Eric Dumazet
2014-06-12 14:21               ` Weiping Pan
2014-06-12 14:32                 ` Eric Dumazet
2014-06-12 15:08                   ` [PATCH v2 1/1] " Per Hurtig
2014-06-12 15:28                     ` Eric Dumazet
2014-06-12 17:36                       ` Nandita Dukkipati
2014-06-12 17:46                         ` Neal Cardwell
2014-06-12 18:06                     ` David Miller
2014-10-07 15:03                       ` Josh Hunt
2014-10-07 20:17                         ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5395B2FC.3070205@kau.se \
    --to=per.hurtig@kau.se \
    --cc=anna.brunstrom@kau.se \
    --cc=eric.dumazet@gmail.com \
    --cc=mohammad.rajiullah@kau.se \
    --cc=nanditad@google.com \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=sergei.shtylyov@cogentembedded.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.