netdev - Re: RE: A Linux TCP SACK Question

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:	Tue, 8 Apr 2008 17:59:06 +0300 (EEST)
From:	"Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi>
To:	Wenji Wu <wenji@...l.gov>
cc:	'Sangtae Ha' <sangtae.ha@...il.com>,
	'John Heffner' <johnwheffner@...il.com>,
	'Netdev' <netdev@...r.kernel.org>
Subject: Re: RE: A Linux TCP SACK Question

On Tue, 8 Apr 2008, Wenji Wu wrote:

> With 3 DupACKs, tcp_ack() calls tcp_fastretrans_alert(), and which in 
> turn calls tcp_xmit_retransmit_queue().

Yeah. It should.

> Within tcp_xmit_retransmit_queue(), there is a line of code that would 
> cause the problem above:
> 
> ......................................................................................................
>  /* Forward retransmissions are possible only during Recovery. */
> 1999        if (icsk->icsk_ca_state != TCP_CA_Recovery)
> 2000                return;
> 
> 2001
> 2002        /* No forward retransmissions in Reno are possible. */
> 2003        if (tcp_is_reno(tp))
> 2004                return;
> 
> .....................................................................................................
> 
> if you look at "tcp_is_reno", you would see that with SACK off, Reno 
> does not do retransmit, it will return!!!

Your analysis is missing something important here, there are two loops 
there :-). One for retransmitting assumed lost segments that's above those 
lines you quoted! The other below is for non-lost marked similar to what 
is specified by RFC3517's Rule 3 for NextSeg, which definately won't apply 
for newreno nor should be executed.

> Really do not understand why these two lines of code exist there!!!
>
> Also, this code still in 2.6.25.

Sure, but there's nothing wrong with them! 2.6.24 just is currently broken 
if you have TSO+NewReno because it won't do the correct lost marking which 
is a necessary preparation step for the loop above that, too bad as I just 
figured that out one/two days ago so there's no fix yet available :-).

> > In which kernel version this dump comes from? 2.6.24 newreno is 
> > crippled 
> > with TSO as was recently discovered, ie., it won't mark lost super 
> > skbs 
> > at head and thus won't retransmit them. Also 2.6.25-rcs are still 
> > broken 
> > (though they'll transmit too much, I'll not go detail in here), DaveM 
> > now 
> > has the fix for 2.6.25-rcs in net-2.6.
> 
> The dumped file is from 2.6.24. 2.6.25's is similiar.

It's a bit hard for me to believe, considering what the last weeks debug 
has revealed about internals of it. Have you checked it from the dumps or 
from the overall results, a similarity in the latter could be due to 
other factors related to the differences in reordering detection between 
NewReno/SACK.

> > In addition, in tcp_init_metrics():
> > 
> > 	if (dst_metric(dst, RTAX_REORDERING) &&
> >             tp->reordering != dst_metric(dst, RTAX_REORDERING)) {
> >                 tcp_disable_fack(tp);
> >                 tp->reordering = dst_metric(dst, RTAX_REORDERING);
> >         }
> 
> Good to know this, thanks

...There might be some bug which causes it to get skipped under some 
circumstances though (which I haven't yet remembered to fix). I don't 
remember too well anymore, probably some goto which caused skipping most 
of what's in there.

-- 
 i.
--
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