From 4463034235ff875f59966a61261dc904a92004dc Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Sat, 23 Oct 2010 14:51:56 +0100 Subject: [PATCH] rx: More improvments to RTT calculation Move the decision about whether a packet contributes to the peer's rount trip time into the CalculateRoundTripTime function, and improve the criteria used. Previously, we only computed the RTT if we had not retransmitted. This is bad, because it means that places where we have backed off in order to retransmit never actually lengthen the RTT, and so the RTT is kept artificially low, and we see a large number of retransmits. Instead, use the serial of the ACK packet to determine which transmission is being acknowledged, and if it is the first, or the last, transmission use the appropriate sent time to calculate the RTT. If we have no serial in the ACK (for a delayed ack, for example), or if the serial doesn't match (where a single acknowledgement is soft acking a number of packets), fall back to only using the ack if the packet has not be retransmitted. Also, avoid multiple counting of packets which have arrived as part of a jumbogram by only permitting the last packet in a jumbogram to contribute to the RTT. This avoids giving the RTT of jumbograms more weight than those of normal packets - doing so would pull down the RTT, as it in effect favours packets which have not be retransmitted. Reviewed-by: Derrick Brashear Reviewed-by: Jeffrey Altman Tested-by: Jeffrey Altman (cherry picked from commit 290495fab1b2a8f1dc842cb2dd6de2d9922169c6) Change-Id: I13fef02ad3c456614cd71227e6a0ae8b4a3f5c72 Reviewed-on: http://gerrit.openafs.org/3127 Tested-by: Derrick Brashear --- src/rx/rx.c | 51 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/src/rx/rx.c b/src/rx/rx.c index b5ed392a1..54591224a 100644 --- a/src/rx/rx.c +++ b/src/rx/rx.c @@ -123,7 +123,7 @@ int (*swapNameProgram) (PROCESS, const char *, char *) = 0; /* Local static routines */ static void rxi_DestroyConnectionNoLock(struct rx_connection *conn); -static void rxi_ComputeRoundTripTime(struct rx_packet *, struct clock *, +static void rxi_ComputeRoundTripTime(struct rx_packet *, struct rx_ackPacket *, struct rx_peer *, struct clock *); #ifdef RX_ENABLE_LOCKS @@ -4009,11 +4009,8 @@ rxi_ReceiveAckPacket(struct rx_call *call, struct rx_packet *np, if (!(tp->flags & RX_PKTFLAG_ACKED)) { newAckCount++; - if (ap->reason != RX_ACK_DELAY && - clock_Eq(&tp->timeSent, &tp->firstSent)) { - rxi_ComputeRoundTripTime(tp, &tp->timeSent, call->conn->peer, - &now); - } + + rxi_ComputeRoundTripTime(tp, ap, call->conn->peer, &now); } #ifdef ADAPT_WINDOW @@ -4090,11 +4087,7 @@ rxi_ReceiveAckPacket(struct rx_call *call, struct rx_packet *np, newAckCount++; tp->flags |= RX_PKTFLAG_ACKED; - if (ap->reason != RX_ACK_DELAY && - clock_Eq(&tp->timeSent, &tp->firstSent)) { - rxi_ComputeRoundTripTime(tp, &tp->timeSent, - call->conn->peer, &now); - } + rxi_ComputeRoundTripTime(tp, ap, call->conn->peer, &now); #ifdef ADAPT_WINDOW rxi_ComputeRate(call->conn->peer, call, tp, np, ap->reason); @@ -6427,12 +6420,44 @@ rxi_ChallengeOn(struct rx_connection *conn) /* sentp and/or peer may be null */ static void rxi_ComputeRoundTripTime(struct rx_packet *p, - struct clock *sentp, + struct rx_ackPacket *ack, struct rx_peer *peer, struct clock *now) { - struct clock thisRtt, *rttp = &thisRtt; + struct clock thisRtt, *sentp, *rttp = &thisRtt; int rtt_timeout; + int serial; + + /* If the ACK is delayed, then do nothing */ + if (ack->reason == RX_ACK_DELAY) + return; + + /* On the wire, jumbograms are a single UDP packet. We shouldn't count + * their RTT multiple times, so only include the RTT of the last packet + * in a jumbogram */ + if (p->flags & RX_JUMBO_PACKET) + return; + + /* Use the serial number to determine which transmission the ACK is for, + * and set the sent time to match this. If we have no serial number, then + * only use the ACK for RTT calculations if the packet has not been + * retransmitted + */ + + serial = ntohl(ack->serial); + if (serial) { + if (serial == p->header.serial) { + sentp = &p->timeSent; + } else if (serial == p->firstSerial) { + sentp = &p->firstSent; + } else + return; + } else { + if (clock_Eq(&p->timeSent, &p->firstSent)) { + sentp = &p->firstSent; + } else + return; + } thisRtt = *now; -- 2.39.5