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  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 18 Jul 2020 18:43:24 +0800
From:   hujunwei <hujunwei4@...wei.com>
To:     Neal Cardwell <ncardwell@...gle.com>
CC:     Eric Dumazet <edumazet@...gle.com>,
        David Miller <davem@...emloft.net>,
        Alexey Kuznetsov <kuznet@....inr.ac.ru>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Netdev <netdev@...r.kernel.org>, <wangxiaogang3@...wei.com>,
        <jinyiting@...wei.com>, <xuhanbing@...wei.com>,
        <zhengshaoyu@...wei.com>, Yuchung Cheng <ycheng@...gle.com>,
        Ilpo Jarvinen <ilpo.jarvinen@...helsinki.fi>
Subject: Re: [PATCH net-next] tcp: Optimize the recovery of tcp when lack of
 SACK


On 2020/7/17 22:44, Neal Cardwell wrote:
> On Fri, Jul 17, 2020 at 7:43 AM hujunwei <hujunwei4@...wei.com> wrote:
>>
>> From: Junwei Hu <hujunwei4@...wei.com>
>>
>> In the document of RFC2582(https://tools.ietf.org/html/rfc2582)
>> introduced two separate scenarios for tcp congestion control:
> 
> Can you please elaborate on how the sender is able to distinguish
> between the two scenarios, after your patch?
> 
> It seems to me that with this proposed patch, there is the risk of
> spurious fast recoveries due to 3 dupacks in the second second
> scenario (the sender unnecessarily retransmitted three packets below
> "send_high"). Can you please post a packetdrill test to demonstrate
> that with this patch the TCP sender does not spuriously enter fast
> recovery in such a scenario?
> 
Hi neal,
Thanks for you quick reply!
What I want to says is when these three numbers: snd_una, high_seq and
snd_nxt are the same, that means all data outstanding
when the Loss state began have successfully been acknowledged.
So the sender is time to exits to the Open state.
I'm not sure whether my understanding is correct.

>> This patch enhance the TCP congestion control algorithm for lack
>> of SACK.
> 
> You describe this as an enhancement. Can you please elaborate on the
> drawback/downside of staying in CA_Loss in this case you are
> describing (where you used kprobes to find that TCP stayed in CA_Loss
> state when high_seq was equal to snd_nxt)?
> 
I tried, but I can't reproduce it by packetdrill. This problem appeared
in our production environment. Here is part of the trace message:

First ack:
#tcp_ack: (tcp_ack+0x0/0x920) skb_tcp_seq=0x1dc21196 skb_tcp_ack_seq=0x9d5e4bcc(3427491485)
	packets_out=4 retrans_out=1 sacked_out=0 lost_out=4 snd_nxt=3427491485
	snd_una=3427485917 high_seq=3427491485 reordering=1 mss_cache=1392
	icsk_ca_state=4 sack_ok=0 undo_retrans=1 snd_cwnd=1

#tcp_fastretrans_alert: (tcp_fastretrans_alert+0x0/0x7b0) prior_snd_una=3427485917
	num_dupack=0 packets_out=0 retrans_out=0 sacked_out=0 lost_out=0
	snd_nxt=3427491485 snd_una=3427491485 high_seq=3427491485 reordering=1
	mss_cache=1392 icsk_ca_state=4 sack_ok=0 undo_retrans=1 snd_cwnd=1

As we can see by func tcp_fastretrans_alert icsk_ca_state remains CA_Loss (4),
and the numbers: snd_nxt, snd_una and high_seq are the same.

first dup ack:
#tcp_ack: (tcp_ack+0x0/0x920) skb_tcp_seq=0x1dc21196 skb_tcp_ack_seq=0x9d5e4bcc(3427491485)
	packets_out=2 retrans_out=0 sacked_out=0 lost_out=0 snd_nxt=3427494269
	snd_una=3427491485 high_seq=3427491485 reordering=1 mss_cache=1392
	icsk_ca_state=4 sack_ok=0 undo_retrans=1 snd_cwnd=2

#tcp_fastretrans_alert: (tcp_fastretrans_alert+0x0/0x7b0) num_dupack=1 packets_out=2
	retrans_out=0 sacked_out=0 lost_out=0 snd_nxt=3427494269 snd_una=3427491485
	high_seq=3427491485 reordering=1 icsk_ca_state=4 sack_ok=0 undo_retrans=1 snd_cwnd=2

second dup ack:
#tcp_ack: (tcp_ack+0x0/0x920) skb_tcp_seq=0x1dc21196 skb_tcp_ack_seq=0x9d5e4bcc(3427491485)
	packets_out=4 retrans_out=0 sacked_out=0 lost_out=0 snd_nxt=3427497053
	snd_una=3427491485 high_seq=3427491485 reordering=1 mss_cache=1392
	icsk_ca_state=4 sack_ok=0 undo_retrans=1 snd_cwnd=4

So, I really hope someone can answer whether my understanding is correct.

> To deal with sequence number wrap-around, sequence number comparisons
> in TCP need to use the before() and after() helpers, rather than
> comparison operators. Here it seems the patch should use after()
> rather than >. However,  I think the larger concern is the concern
> mentioned above.
> 
If this patch is useful, I will modify this.

Regards Junwei

Powered by blists - more mailing lists