Followup to previous commit..

- It's worthwhile to use untimeout(9), even though we must still protect
  against "false" timeouts, because most of the time it saves having to
  handle a dummy timeout event.
- Slight tweaks to the delayed ACK algorithm paramters.

1 files changed, 40 insertions, 20 deletions

diff --git a/sys/netgraph/ng_pptpgre.c b/sys/netgraph/ng_pptpgre.c
index 3a361f7..feee658 100644
--- a/sys/netgraph/ng_pptpgre.c
+++ b/sys/netgraph/ng_pptpgre.c
@@ -121,9 +121,16 @@ typedef u_int64_t		pptptime_t;
 /* Acknowledgment timeout parameters and functions */
 #define PPTP_XMIT_WIN		16			/* max xmit window */
 #define PPTP_MIN_RTT		(PPTP_TIME_SCALE / 10)	/* 100 milliseconds */
-#define PPTP_MIN_TIMEOUT	(PPTP_TIME_SCALE / 100)	/* 10 milliseconds */
+#define PPTP_MIN_TIMEOUT	(PPTP_TIME_SCALE / 500)	/* 2 milliseconds */
 #define PPTP_MAX_TIMEOUT	(10 * PPTP_TIME_SCALE)	/* 10 seconds */
 
+/* When we recieve a packet, we wait to see if there's an outgoing packet
+   we can piggy-back the ACK off of. These parameters determine the mimimum
+   and maxmimum length of time we're willing to wait in order to do that.
+   These have no effect unless "enableDelayedAck" is turned on. */
+#define PPTP_MIN_ACK_DELAY	(PPTP_TIME_SCALE / 500)	/* 2 milliseconds */
+#define PPTP_MAX_ACK_DELAY	(PPTP_TIME_SCALE / 2)	/* 500 milliseconds */
+
 /* See RFC 2637 section 4.4 */
 #define PPTP_ACK_ALPHA(x)	((x) >> 3)	/* alpha = 0.125 */
 #define PPTP_ACK_BETA(x)	((x) >> 2)	/* beta = 0.25 */
@@ -145,16 +152,11 @@ struct ng_pptpgre_ackp {
 	u_int32_t		winAck;		/* seq when xmitWin will grow */
 	pptptime_t		timeSent[PPTP_XMIT_WIN];
 #ifdef DEBUG_RAT
-	pptptime_t		timerStart;
-	pptptime_t		timerLength;
+	pptptime_t		timerStart;	/* when rackTimer started */
+	pptptime_t		timerLength;	/* rackTimer duration */
 #endif
 };
 
-/* When we recieve a packet, we wait to see if there's an outgoing packet
-   we can piggy-back the ACK off of.  These parameters determine the mimimum
-   and maxmimum length of time we're willing to wait in order to do that. */
-#define PPTP_MAX_ACK_DELAY	((int) (0.25 * PPTP_TIME_SCALE))
-
 /* Node private data */
 struct ng_pptpgre_private {
 	hook_p			upper;		/* hook to upper layers */
@@ -181,7 +183,7 @@ static ng_disconnect_t	ng_pptpgre_disconnect;
 /* Helper functions */
 static int	ng_pptpgre_xmit(node_p node, struct mbuf *m, meta_p meta);
 static int	ng_pptpgre_recv(node_p node, struct mbuf *m, meta_p meta);
-static void	ng_pptpgre_start_send_ack_timer(node_p node, long ackTimeout);
+static void	ng_pptpgre_start_send_ack_timer(node_p node, int ackTimeout);
 static void	ng_pptpgre_start_recv_ack_timer(node_p node);
 static void	ng_pptpgre_recv_ack_timeout(void *arg);
 static void	ng_pptpgre_send_ack_timeout(void *arg);
@@ -515,7 +517,11 @@ ng_pptpgre_xmit(node_p node, struct mbuf *m, meta_p meta)
 		gre->hasAck = 1;
 		gre->data[gre->hasSeq] = htonl(priv->recvSeq);
 		priv->xmitAck = priv->recvSeq;
-		a->sackTimerPtr = NULL;
+		if (a->sackTimerPtr != NULL) {
+			untimeout(ng_pptpgre_send_ack_timeout,
+			    a->sackTimerPtr, a->sackTimer);
+			a->sackTimerPtr = NULL;
+		}
 	}
 
 	/* Prepend GRE header to outgoing frame */
@@ -547,7 +553,7 @@ ng_pptpgre_xmit(node_p node, struct mbuf *m, meta_p meta)
 	/* Deliver packet */
 	NG_SEND_DATA(error, priv->lower, m, meta);
 
-	/* Start send ACK timer */
+	/* Start receive ACK timer if data was sent and not already running */
 	if (error == 0 && gre->hasSeq && priv->xmitSeq == priv->recvAck + 1)
 		ng_pptpgre_start_recv_ack_timer(node);
 	return (error);
@@ -667,7 +673,11 @@ bad:
 		}
 
 		/* Stop/(re)start receive ACK timer as necessary */
-		a->rackTimerPtr = NULL;
+		if (a->rackTimerPtr != NULL) {
+			untimeout(ng_pptpgre_recv_ack_timeout,
+			    a->rackTimerPtr, a->rackTimer);
+			a->rackTimerPtr = NULL;
+		}
 		if (priv->recvAck != priv->xmitSeq)
 			ng_pptpgre_start_recv_ack_timer(node);
 	}
@@ -690,13 +700,14 @@ badAck:
 
 		/* We need to acknowledge this packet; do it soon... */
 		if (a->sackTimerPtr == NULL) {
-			long maxWait;
+			int maxWait;
 
-			/* Take half of the estimated round trip time */
-			maxWait = (a->rtt >> 1);
+			/* Take 1/4 of the estimated round trip time */
+			maxWait = (a->rtt >> 2);
 
 			/* If delayed ACK is disabled, send it now */
-			if (!priv->conf.enableDelayedAck || maxWait == 0)
+			if (!priv->conf.enableDelayedAck
+			    || maxWait < PPTP_MIN_ACK_DELAY)
 				ng_pptpgre_xmit(node, NULL, NULL);
 			else {			/* send the ack later */
 				if (maxWait > PPTP_MAX_ACK_DELAY)
@@ -738,6 +749,7 @@ ng_pptpgre_start_recv_ack_timer(node_p node)
 
 	/* Compute how long until oldest unack'd packet times out,
 	   and reset the timer to that time. */
+	KASSERT(a->rackTimerPtr == NULL, ("%s: rackTimer", __FUNCTION__));
 	remain = (a->timeSent[0] + a->ato) - ng_pptpgre_time(node);
 	if (remain < 0)
 		remain = 0;
@@ -817,7 +829,7 @@ ng_pptpgre_recv_ack_timeout(void *arg)
  * already running.
  */
 static void
-ng_pptpgre_start_send_ack_timer(node_p node, long ackTimeout)
+ng_pptpgre_start_send_ack_timer(node_p node, int ackTimeout)
 {
 	const priv_p priv = node->private;
 	struct ng_pptpgre_ackp *const a = &priv->ackp;
@@ -911,9 +923,17 @@ ng_pptpgre_reset(node_p node)
 	/* Reset stats */
 	bzero(&priv->stats, sizeof(priv->stats));
 
-	/* "Stop" timers */
-	a->sackTimerPtr = NULL;
-	a->rackTimerPtr = NULL;
+	/* Stop timers */
+	if (a->sackTimerPtr != NULL) {
+		untimeout(ng_pptpgre_send_ack_timeout,
+		    a->sackTimerPtr, a->sackTimer);
+		a->sackTimerPtr = NULL;
+	}
+	if (a->rackTimerPtr != NULL) {
+		untimeout(ng_pptpgre_recv_ack_timeout,
+		    a->rackTimerPtr, a->rackTimer);
+		a->rackTimerPtr = NULL;
+	}
 }
 
 /*