lkml.org 
[lkml]   [1999]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: take 2 -- [PATCH] real fix found? (Re: timestamps, tcp_paws_discard and out of order packets/acks (fwd))
On Fri, 23 Jul 1999, David S. Miller wrote:

> 1) RFC1323 describes the check wrong, as stated by the comment.

Agreed, but the fix was worse.

> 2) The rules stated by all RFCs and working drafts do not allow
> for best utilization of ACK feedback when ACKs are massively
> reordered. This is what my PAWS workaround is trying to
> accomplish.

We may not want to update the rtt estimate as frequently as we can as the
skew that small packets put on the rtt for small links is quite
substantial. (See below for my idea.)

> And the best response I got was "oh that bug, it hurts my head each
> time I think about it" So Linux will perform well in these cases
> and nobody else will because aparently even the RFC authors
> themselves do not care. Note that what Linux does makes the PAWS
> check rules for pure-ACKs orthogonal to the out of order data
> packet cases.

Ahhh, now I understand the motivation for the change. Below is my
suggested fix against 2.2.11-1. I've split the original function of
ts_recent into ts_recent and ts_paws. ts_paws now serves as the cutoff
timestamp in order to perform PAWS, whereas ts_recent is the most recently
seen timestamp. This change allows PAWS to be performed against pure ACK
packets now too, so PAWS is actually useful! ;-)

> 3) Audrey found some other RTT estimation problems which are being
> dealt with too, and these fixes have security implications as well.
> (before you could quite easily make a connection believe that the
> RTT was some nearly infinite value, effectively making the
> connection useless)

Security wise, I've left the window check in place for ts_recent updates,
so attacks won't be any easier.

Underestimating RTT is something I'm worried about quite a bit on the link
in question. Since ACKs are getting through before packets, and data
packets are getting sent out of order, we've now got a factor reducing the
rtt estimate that *shouldn't* be reduced. 20 byte packets get through in
~36ms, whereas 720 byte packets are ~200ms, and 1440 byte packets take
~370ms (via ping). This is modelled by rtt = A + B * sizeof(payload)
almost perfectly. Could we perhaps adopt something like this in our rtt
estimator?

-ben

--
Hi! I'm Signature Virus 99! Copy me into your .signature and join the fun!


diff -ur clean/2.2.11-1/include/net/sock.h 2.2.10/include/net/sock.h
--- clean/2.2.11-1/include/net/sock.h Tue May 11 13:36:44 1999
+++ 2.2.10/include/net/sock.h Fri Jul 23 23:00:17 1999
@@ -286,7 +286,8 @@
__u32 rcv_tsval; /* Time stamp value */
__u32 rcv_tsecr; /* Time stamp echo reply */
__u32 ts_recent; /* Time stamp to echo next */
- __u32 ts_recent_stamp;/* Time we stored ts_recent (for aging) */
+ __u32 ts_paws; /* Time stamp for PAWS discards */
+ __u32 ts_paws_stamp; /* Time we stored ts_paws (for aging) */
int num_sacks; /* Number of SACK blocks */
struct tcp_sack_block selective_acks[4]; /* The SACKS themselves*/

diff -ur clean/2.2.11-1/net/ipv4/tcp_input.c 2.2.10/net/ipv4/tcp_input.c
--- clean/2.2.11-1/net/ipv4/tcp_input.c Fri Jul 23 20:41:15 1999
+++ 2.2.10/net/ipv4/tcp_input.c Fri Jul 23 23:00:17 1999
@@ -214,7 +214,7 @@
__u32 start_seq, __u32 end_seq)
{
/* From draft-ietf-tcplw-high-performance: the correct
- * test is last_ack_sent <= end_seq.
+ * test is last_ack_sent <= seq <= window_right.
* (RFC1323 stated last_ack_sent < end_seq.)
*
* HOWEVER: The current check contradicts the draft statements.
@@ -225,14 +225,20 @@
*/
if (!before(end_seq, tp->last_ack_sent - sk->rcvbuf) &&
!after(start_seq, tp->rcv_wup + tp->rcv_wnd)) {
- /* PAWS bug workaround wrt. ACK frames, the PAWS discard
- * extra check below makes sure this can only happen
- * for pure ACK frames. -DaveM
- */
- if((s32)(tp->rcv_tsval - tp->ts_recent) >= 0) {
- tp->ts_recent = tp->rcv_tsval;
- tp->ts_recent_stamp = tcp_time_stamp;
- }
+ tp->ts_recent = tp->rcv_tsval;
+ }
+
+ /* PAWS update if start_seq < last_ack_seq <= end_seq
+ * I've separated the timestamp used for PAWS discards
+ * from ts_recent, as otherwise we don't handle reordered
+ * packets very well. By updating the discard timestamp
+ * only when the left side of the window advances, we
+ * will accept out of order packets, and protect ourselves
+ * from wrapped windows. -ben (19990723)
+ */
+ if (between(tp->last_ack_sent + 1, start_seq, end_seq)) {
+ tp->ts_paws = tp->rcv_tsval;
+ tp->ts_paws_stamp = tcp_time_stamp;
}
}

@@ -240,11 +246,9 @@

extern __inline__ int tcp_paws_discard(struct tcp_opt *tp, struct tcphdr *th, unsigned len)
{
- /* ts_recent must be younger than 24 days */
- return (((s32)(tcp_time_stamp - tp->ts_recent_stamp) >= PAWS_24DAYS) ||
- (((s32)(tp->rcv_tsval - tp->ts_recent) < 0) &&
- /* Sorry, PAWS as specified is broken wrt. pure-ACKs -DaveM */
- (len != (th->doff * 4))));
+ /* ts_paws must be younger than 24 days */
+ return ((s32)(tcp_time_stamp - tp->ts_paws_stamp) >= PAWS_24DAYS) ||
+ ((s32)(tp->rcv_tsval - tp->ts_paws) < 0);
}


@@ -2188,8 +2192,8 @@
} else
tp->tcp_header_len = sizeof(struct tcphdr);
if (tp->saw_tstamp) {
- tp->ts_recent = tp->rcv_tsval;
- tp->ts_recent_stamp = tcp_time_stamp;
+ tp->ts_paws = tp->ts_recent = tp->rcv_tsval;
+ tp->ts_paws_stamp = tcp_time_stamp;
}

/* Can't be earlier, doff would be wrong. */
@@ -2212,8 +2216,8 @@
tcp_set_state(sk, TCP_SYN_RECV);
tcp_parse_options(sk, th, tp, 0);
if (tp->saw_tstamp) {
- tp->ts_recent = tp->rcv_tsval;
- tp->ts_recent_stamp = tcp_time_stamp;
+ tp->ts_paws = tp->ts_recent = tp->rcv_tsval;
+ tp->ts_paws_stamp = tcp_time_stamp;
}

tp->rcv_nxt = TCP_SKB_CB(skb)->seq + 1;
diff -ur clean/2.2.11-1/net/ipv4/tcp_ipv4.c 2.2.10/net/ipv4/tcp_ipv4.c
--- clean/2.2.11-1/net/ipv4/tcp_ipv4.c Fri Jul 23 20:41:15 1999
+++ 2.2.10/net/ipv4/tcp_ipv4.c Fri Jul 23 23:00:17 1999
@@ -1428,8 +1428,8 @@
newtp->window_clamp = min(newtp->window_clamp,65535);
}
if (newtp->tstamp_ok) {
- newtp->ts_recent = req->ts_recent;
- newtp->ts_recent_stamp = tcp_time_stamp;
+ newtp->ts_paws = newtp->ts_recent = req->ts_recent;
+ newtp->ts_paws_stamp = tcp_time_stamp;
newtp->tcp_header_len = sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED;
} else {
newtp->tcp_header_len = sizeof(struct tcphdr);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:53    [W:0.107 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site