1 files changed, 75 insertions, 96 deletions

diff --git a/sys/netgraph/ng_pptpgre.c b/sys/netgraph/ng_pptpgre.c
index 945d779bfc4d..6059406ddab6 100644
--- a/sys/netgraph/ng_pptpgre.c
+++ b/sys/netgraph/ng_pptpgre.c
@@ -116,14 +116,16 @@ struct greheader {
 #define PPTP_MAX_PAYLOAD	(0xffff - sizeof(struct greheader) - 8)
 
 /* All times are scaled by this (PPTP_TIME_SCALE time units = 1 sec.) */
-#define PPTP_TIME_SCALE		1000
+#define PPTP_TIME_SCALE		1000			/* milliseconds */
 typedef u_int32_t		pptptime_t;
 
 /* Acknowledgment timeout parameters and functions */
-#define PPTP_XMIT_WIN		8			/* max xmit window */
-#define PPTP_MIN_RTT		(PPTP_TIME_SCALE / 10)	/* 1/10 second */
+#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_MAX_TIMEOUT	(10 * PPTP_TIME_SCALE)	/* 10 seconds */
 
+/* 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 */
 #define PPTP_ACK_CHI(x) 	((x) << 2)	/* chi = 4 */
@@ -137,11 +139,11 @@ struct ng_pptpgre_ackp {
 	int32_t			rtt;		/* round trip time estimate */
 	int32_t			dev;		/* deviation estimate */
 	u_int16_t		xmitWin;	/* size of xmit window */
-	u_char			sackTimerRunning;/* send ack timer is running */
-	u_char			rackTimerRunning;/* recv ack timer is running */
-	u_int32_t		winAck;		/* seq when xmitWin will grow */
 	struct callout_handle	sackTimer;	/* send ack timer */
 	struct callout_handle	rackTimer;	/* recv ack timer */
+	node_p			*sackTimerPtr;	/* send ack timer pointer */
+	node_p			*rackTimerPtr;	/* recv ack timer pointer */
+	u_int32_t		winAck;		/* seq when xmitWin will grow */
 	pptptime_t		timeSent[PPTP_XMIT_WIN];
 };
 
@@ -178,8 +180,6 @@ 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_recv_ack_timer(node_p node);
-static void	ng_pptpgre_stop_send_ack_timer(node_p node);
-static void	ng_pptpgre_stop_recv_ack_timer(node_p node);
 static void	ng_pptpgre_recv_ack_timeout(void *arg);
 static void	ng_pptpgre_send_ack_timeout(void *arg);
 static void	ng_pptpgre_reset(node_p node);
@@ -419,7 +419,7 @@ ng_pptpgre_rmnode(node_p node)
 {
 	const priv_p priv = node->private;
 
-	/* Cancel timers */
+	/* Reset node */
 	ng_pptpgre_reset(node);
 
 	/* Take down netgraph node */
@@ -513,7 +513,7 @@ ng_pptpgre_xmit(node_p node, struct mbuf *m, meta_p meta)
 		gre->hasAck = 1;
 		priv->xmitAck = priv->recvSeq;
 		gre->data[gre->hasSeq] = htonl(priv->xmitAck);
-		ng_pptpgre_stop_send_ack_timer(node);
+		a->sackTimerPtr = NULL;
 	}
 
 	/* Prepend GRE header to outgoing frame */
@@ -638,11 +638,17 @@ bad:
 		if (diff < 0)
 			diff = -diff;
 		a->dev += PPTP_ACK_BETA(diff - a->dev);
-		a->ato = a->rtt + (u_int) (PPTP_ACK_CHI(a->dev));
+		a->ato = a->rtt + PPTP_ACK_CHI(a->dev);
 		if (a->ato > PPTP_MAX_TIMEOUT)
 			a->ato = PPTP_MAX_TIMEOUT;
+		if (a->ato < PPTP_MIN_TIMEOUT)
+			a->ato = PPTP_MIN_TIMEOUT;
+
+		/* Shift packet transmit times in our transmit window */
 		ovbcopy(a->timeSent + index + 1, a->timeSent,
 		    sizeof(*a->timeSent) * (PPTP_XMIT_WIN - (index + 1)));
+
+		/* If we sent an entire window, increase window size by one */
 		if (PPTP_SEQ_DIFF(ack, a->winAck) >= 0
 		    && a->xmitWin < PPTP_XMIT_WIN) {
 			a->xmitWin++;
@@ -670,20 +676,19 @@ badAck:
 		priv->recvSeq = seq;
 
 		/* We need to acknowledge this packet; do it soon... */
-		if (!a->sackTimerRunning) {
-			long ackTimeout;
+		if (a->sackTimerPtr == NULL) {
+			long maxWait;
 
 			/* Take half of the estimated round trip time */
-			ackTimeout = (a->rtt >> 1);
+			maxWait = (a->rtt >> 1);
 
 			/* If too soon, just send one right now */
 			if (!priv->conf.enableDelayedAck)
 				ng_pptpgre_xmit(node, NULL, NULL);
 			else {			/* send the ack later */
-				if (ackTimeout > PPTP_MAX_ACK_DELAY)
-					ackTimeout = PPTP_MAX_ACK_DELAY;
-				ng_pptpgre_start_send_ack_timer(node,
-				    ackTimeout);
+				if (maxWait > PPTP_MAX_ACK_DELAY)
+					maxWait = PPTP_MAX_ACK_DELAY;
+				ng_pptpgre_start_send_ack_timer(node, maxWait);
 			}
 		}
 
@@ -718,9 +723,8 @@ ng_pptpgre_start_recv_ack_timer(node_p node)
 	struct ng_pptpgre_ackp *const a = &priv->ackp;
 	int remain;
 
-	/* Stop current recv ack timer, if any */
-	if (a->rackTimerRunning)
-		ng_pptpgre_stop_recv_ack_timer(node);
+	/* "Stop" current recv ack timer, if any */
+	a->rackTimerPtr = NULL;
 
 	/* Are we waiting for an acknowlegement? */
 	if (priv->recvAck == priv->xmitSeq)
@@ -732,28 +736,14 @@ ng_pptpgre_start_recv_ack_timer(node_p node)
 	if (remain < 0)
 		remain = 0;
 
-	/* Start timer */
-	a->rackTimer = timeout(ng_pptpgre_recv_ack_timeout,
-	    node, remain * hz / PPTP_TIME_SCALE);
+	/* Start new timer */
+	MALLOC(a->rackTimerPtr, node_p *, sizeof(node_p), M_NETGRAPH, M_NOWAIT);
+	if (a->rackTimerPtr == NULL)
+		return;			/* XXX potential hang here */
+	*a->rackTimerPtr = node;	/* insures the correct timeout event */
 	node->refs++;
-	a->rackTimerRunning = 1;
-}
-
-/*
- * Stop the recv ack timer, if running.
- */
-static void
-ng_pptpgre_stop_recv_ack_timer(node_p node)
-{
-	const priv_p priv = node->private;
-	struct ng_pptpgre_ackp *const a = &priv->ackp;
-
-	if (a->rackTimerRunning) {
-		untimeout(ng_pptpgre_recv_ack_timeout, node, a->rackTimer);
-		KASSERT(node->refs > 1, ("%s: no refs", __FUNCTION__));
-		ng_unref(node);
-		a->rackTimerRunning = 0;
-	}
+	a->rackTimer = timeout(ng_pptpgre_recv_ack_timeout,
+	    a->rackTimerPtr, remain * hz / PPTP_TIME_SCALE);
 }
 
 /*
@@ -765,22 +755,24 @@ static void
 ng_pptpgre_recv_ack_timeout(void *arg)
 {
 	int s = splnet();
-	const node_p node = arg;
+	const node_p node = *((node_p *)arg);
 	const priv_p priv = node->private;
 	struct ng_pptpgre_ackp *const a = &priv->ackp;
 
-	/* Avoid shutdown race condition */
-	if ((node->flags & NG_INVALID) != 0) {
+	/* This complicated stuff is needed to avoid race conditions */
+	FREE(arg, M_NETGRAPH);
+	KASSERT(node->refs > 0, ("%s: no refs", __FUNCTION__));
+	if ((node->flags & NG_INVALID) != 0) {	/* shutdown race condition */
 		ng_unref(node);
 		splx(s);
 		return;
 	}
-
-	/* Release timer reference */
-	KASSERT(a->rackTimerRunning, ("%s: !rackTimer", __FUNCTION__));
-	a->rackTimerRunning = 0;
-	KASSERT(node->refs > 1, ("%s: no refs", __FUNCTION__));
 	ng_unref(node);
+	if (arg != a->rackTimerPtr) {	/* timer stopped race condition */
+		splx(s);
+		return;
+	}
+	a->rackTimerPtr = NULL;
 
 	/* Update adaptive timeout stuff */
 	priv->stats.recvAckTimeouts++;
@@ -788,15 +780,13 @@ ng_pptpgre_recv_ack_timeout(void *arg)
 	a->ato = a->rtt + PPTP_ACK_CHI(a->dev);
 	if (a->ato > PPTP_MAX_TIMEOUT)
 		a->ato = PPTP_MAX_TIMEOUT;
-	priv->recvAck++;			/* assume packet was lost */
-	a->winAck = priv->recvAck + a->xmitWin;	/* reset win expand time */
-	ovbcopy(a->timeSent + 1, a->timeSent,	/* shift xmit window times */
-	    sizeof(*a->timeSent) * (PPTP_XMIT_WIN - 1));
-	a->xmitWin = (a->xmitWin + 1) / 2;	/* shrink transmit window */
+	if (a->ato < PPTP_MIN_TIMEOUT)
+		a->ato = PPTP_MIN_TIMEOUT;
 
-	/* Restart timer if there are any more outstanding frames */
-	if (priv->recvAck != priv->xmitSeq)
-		ng_pptpgre_start_recv_ack_timer(node);
+	/* Reset ack and sliding window */
+	priv->recvAck = priv->xmitSeq;		/* pretend we got the ack */
+	a->xmitWin = (a->xmitWin + 1) / 2;	/* shrink transmit window */
+	a->winAck = priv->recvAck + a->xmitWin;	/* reset win expand time */
 	splx(s);
 }
 
@@ -810,28 +800,15 @@ ng_pptpgre_start_send_ack_timer(node_p node, long ackTimeout)
 	const priv_p priv = node->private;
 	struct ng_pptpgre_ackp *const a = &priv->ackp;
 
-	KASSERT(!a->sackTimerRunning, ("%s: sackTimer", __FUNCTION__));
-	a->sackTimer = timeout(ng_pptpgre_send_ack_timeout,
-	    node, ackTimeout * hz / PPTP_TIME_SCALE);
+	/* Start new timer */
+	KASSERT(a->sackTimerPtr == NULL, ("%s: sackTimer", __FUNCTION__));
+	MALLOC(a->sackTimerPtr, node_p *, sizeof(node_p), M_NETGRAPH, M_NOWAIT);
+	if (a->sackTimerPtr == NULL)
+		return;			/* XXX potential hang here */
+	*a->sackTimerPtr = node;
 	node->refs++;
-	a->sackTimerRunning = 1;
-}
-
-/*
- * Stop the send ack timer, if running.
- */
-static void
-ng_pptpgre_stop_send_ack_timer(node_p node)
-{
-	const priv_p priv = node->private;
-	struct ng_pptpgre_ackp *const a = &priv->ackp;
-
-	if (a->sackTimerRunning) {
-		untimeout(ng_pptpgre_send_ack_timeout, node, a->sackTimer);
-		KASSERT(node->refs > 1, ("%s: no refs", __FUNCTION__));
-		ng_unref(node);
-		a->sackTimerRunning = 0;
-	}
+	a->sackTimer = timeout(ng_pptpgre_send_ack_timeout,
+	    a->sackTimerPtr, ackTimeout * hz / PPTP_TIME_SCALE);
 }
 
 /*
@@ -844,22 +821,24 @@ static void
 ng_pptpgre_send_ack_timeout(void *arg)
 {
 	int s = splnet();
-	const node_p node = arg;
+	const node_p node = *((node_p *)arg);
 	const priv_p priv = node->private;
 	struct ng_pptpgre_ackp *const a = &priv->ackp;
 
-	/* Avoid shutdown race condition */
-	if ((node->flags & NG_INVALID) != 0) {
+	/* This complicated stuff is needed to avoid race conditions */
+	FREE(arg, M_NETGRAPH);
+	KASSERT(node->refs > 0, ("%s: no refs", __FUNCTION__));
+	if ((node->flags & NG_INVALID) != 0) {	/* shutdown race condition */
 		ng_unref(node);
 		splx(s);
 		return;
 	}
-
-	/* Release timer reference */
-	KASSERT(a->sackTimerRunning, ("%s: !sackTimer", __FUNCTION__));
-	a->sackTimerRunning = 0;
-	KASSERT(node->refs > 1, ("%s: no refs", __FUNCTION__));
 	ng_unref(node);
+	if (a->sackTimerPtr != arg) {	/* timer stopped race condition */
+		splx(s);
+		return;
+	}
+	a->sackTimerPtr = NULL;
 
 	/* Send a frame with an ack but no payload */
   	ng_pptpgre_xmit(node, NULL, NULL);
@@ -886,17 +865,17 @@ ng_pptpgre_reset(node_p node)
 		a->rtt = PPTP_MIN_RTT;
 	a->dev = 0;
 	a->xmitWin = (priv->conf.recvWin + 1) / 2;
-	if (a->xmitWin < 1)
-		a->xmitWin = 1;
+	if (a->xmitWin < 2)		/* often the first packet is lost */
+		a->xmitWin = 2;		/*   because the peer isn't ready */
 	if (a->xmitWin > PPTP_XMIT_WIN)
 		a->xmitWin = PPTP_XMIT_WIN;
 	a->winAck = a->xmitWin;
 
 	/* Reset sequence numbers */
-	priv->recvSeq = 0;
-	priv->recvAck = 0;
-	priv->xmitSeq = 0;
-	priv->xmitAck = 0;
+	priv->recvSeq = ~0;
+	priv->recvAck = ~0;
+	priv->xmitSeq = ~0;
+	priv->xmitAck = ~0;
 
 	/* Reset start time */
 	getmicrouptime(&priv->startTime);
@@ -904,9 +883,9 @@ ng_pptpgre_reset(node_p node)
 	/* Reset stats */
 	bzero(&priv->stats, sizeof(priv->stats));
 
-	/* Stop timers */
-	ng_pptpgre_stop_send_ack_timer(node);
-	ng_pptpgre_stop_recv_ack_timer(node);
+	/* "Stop" timers */
+	a->sackTimerPtr = NULL;
+	a->rackTimerPtr = NULL;
 }
 
 /*