netdev - Re: [PATCH net 5/5] tcp: send real dupACKs by locking advertized window for non-SACK flows

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]

Message-ID: <alpine.DEB.2.20.1803072229380.30307@whs-18.cs.helsinki.fi>
Date:   Wed, 7 Mar 2018 23:39:50 +0200 (EET)
From:   Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>
To:     Eric Dumazet <eric.dumazet@...il.com>
cc:     Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net 5/5] tcp: send real dupACKs by locking advertized
 window for non-SACK flows

On Wed, 7 Mar 2018, Eric Dumazet wrote:

> > Currently non-SACK senders cannot identify almost any duplicate ACKs 
> > because the window keeps updating for almost all ACKs. As a result, 
> > non-SACK senders end up doing loss recovery only with RTO. RTO
> > recovery 
> > without SACK is quite annoying because it potentially sends 
> > large number of unnecessary rexmits.
> 
> I get that, but switching from "always" to "never" sounds dangerous.
> 
> May I suggest you refine your patch, to maybe allow win reductions, in
> a logarithmic fashion maybe ?
> 
> This way, when you send 1000 DUPACK, only few of them would actually
> have to lower the window, and 99% of them would be considered as DUPACK
> by these prehistoric TCP stacks.

The problem I'm trying to fix is due to window increasing (of course, 
dupacks could also be masked because of decrease but that's something
I've never seen to occur in practice) so I tried to make you happy and 
change my fix to do "never increase, always decrease". However, it turns 
out that even pre-fixed code implements "never decrease" not "always 
decrease" like you thought:

static u16 tcp_select_window(struct sock *sk)
{
	...
	/* Never shrink the offered window */
        if (new_win < cur_win) {
		...
		new_win = ALIGN(cur_win, 1 << tp->rx_opt.rcv_wscale);

...Thus, I don't see my fix making the case you're worried about any
worse, AFAICT.


-- 
 i.