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

["Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>]

[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]

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.