netdev - Re: [PATCH] tcp: fixing TLP's FIN recovery

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 08 Jun 2014 09:41:46 +0200
From:	Per Hurtig <per.hurtig@....se>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	netdev@...r.kernel.org, anna.brunstrom@....se,
	mohammad.rajiullah@....se, ncardwell@...gle.com,
	nanditad@...gle.com, sergei.shtylyov@...entembedded.com
Subject: Re: [PATCH] tcp: fixing TLP's FIN recovery



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@....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 ;)
>
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.

>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html