- Changed sequence numbers processing to avoid incorrect timeout waiting

when one of links is inactive and have stale sequence number. To avoid
this sequence numbers of all links are getting updated on every
successful packet reassembling.
- ng_ppp_bump_mseq function created to simplify code.
- ng_ppp_frag_drop function separated from ng_ppp_frag_process to
simplify code.

Reviewed by:	archie
Approved by:	glebius (mentor)

1 files changed, 65 insertions, 66 deletions

diff --git a/sys/netgraph/ng_ppp.c b/sys/netgraph/ng_ppp.c
index a9eba07..0cdc0d7 100644
--- a/sys/netgraph/ng_ppp.c
+++ b/sys/netgraph/ng_ppp.c
@@ -304,6 +304,8 @@ static int	ng_ppp_link_xmit(node_p node, item_p item, uint16_t proto,
 static int	ng_ppp_bypass(node_p node, item_p item, uint16_t proto,
 		    uint16_t linkNum);
 
+static void	ng_ppp_bump_mseq(node_p node, int32_t new_mseq);
+static int	ng_ppp_frag_drop(node_p node);
 static int	ng_ppp_check_packet(node_p node);
 static void	ng_ppp_get_packet(node_p node, struct mbuf **mp);
 static int	ng_ppp_frag_process(node_p node);
@@ -1527,6 +1529,28 @@ ng_ppp_mp_recv(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
  ************************************************************************/
 
 /*
+ * If new mseq > current then set it and update all active links
+ */
+static void
+ng_ppp_bump_mseq(node_p node, int32_t new_mseq)
+{
+	const priv_p priv = NG_NODE_PRIVATE(node);
+	int i;
+	
+	if (MP_RECV_SEQ_DIFF(priv, priv->mseq, new_mseq) < 0) {
+		priv->mseq = new_mseq;
+		for (i = 0; i < priv->numActiveLinks; i++) {
+			struct ng_ppp_link *const alink =
+			    &priv->links[priv->activeLinks[i]];
+
+			if (MP_RECV_SEQ_DIFF(priv,
+			    alink->seq, new_mseq) < 0)
+				alink->seq = new_mseq;
+		}
+	}
+}
+
+/*
  * Examine our list of fragments, and determine if there is a
  * complete and deliverable packet at the head of the list.
  * Return 1 if so, zero otherwise.
@@ -1587,8 +1611,11 @@ ng_ppp_get_packet(node_p node, struct mbuf **mp)
 		}
 		while (tail->m_next != NULL)
 			tail = tail->m_next;
-		if (qent->last)
+		if (qent->last) {
 			qnext = NULL;
+			/* Bump MSEQ if necessary */
+			ng_ppp_bump_mseq(node, qent->seq);
+		}
 		FREE(qent, M_NETGRAPH_PPP);
 		priv->qlen--;
 	}
@@ -1647,52 +1674,17 @@ ng_ppp_frag_trim(node_p node)
 }
 
 /*
- * Run the queue, restoring the queue invariants
+ * Drop fragments on queue overflow.
+ * Returns 1 if fragments were removed, zero otherwise.
  */
 static int
-ng_ppp_frag_process(node_p node)
+ng_ppp_frag_drop(node_p node)
 {
 	const priv_p priv = NG_NODE_PRIVATE(node);
-	struct mbuf *m;
-	item_p item;
-	uint16_t proto;
-
-	/* Deliver any deliverable packets */
-	while (ng_ppp_check_packet(node)) {
-		ng_ppp_get_packet(node, &m);
-		if ((m = ng_ppp_cutproto(m, &proto)) == NULL)
-			continue;
-		if (!PROT_VALID(proto)) {
-			priv->bundleStats.badProtos++;
-			NG_FREE_M(m);
-			continue;
-		}
-		if ((item = ng_package_data(m, NG_NOFLAGS)) != NULL)
-			ng_ppp_crypt_recv(node, item, proto,
-				NG_PPP_BUNDLE_LINKNUM);
-	}
-
-	/* Delete dead fragments and try again */
-	if (ng_ppp_frag_trim(node)) {
-		while (ng_ppp_check_packet(node)) {
-			ng_ppp_get_packet(node, &m);
-			if ((m = ng_ppp_cutproto(m, &proto)) == NULL)
-				continue;
-			if (!PROT_VALID(proto)) {
-				priv->bundleStats.badProtos++;
-				NG_FREE_M(m);
-				continue;
-			}
-			if ((item = ng_package_data(m, NG_NOFLAGS)) != NULL)
-				ng_ppp_crypt_recv(node, item, proto,
-					NG_PPP_BUNDLE_LINKNUM);
-		}
-	}
 
 	/* Check queue length */
 	if (priv->qlen > MP_MAX_QUEUE_LEN) {
 		struct ng_ppp_frag *qent;
-		int i;
 
 		/* Get oldest fragment */
 		KASSERT(!TAILQ_EMPTY(&priv->frags),
@@ -1700,17 +1692,7 @@ ng_ppp_frag_process(node_p node)
 		qent = TAILQ_FIRST(&priv->frags);
 
 		/* Bump MSEQ if necessary */
-		if (MP_RECV_SEQ_DIFF(priv, priv->mseq, qent->seq) < 0) {
-			priv->mseq = qent->seq;
-			for (i = 0; i < priv->numActiveLinks; i++) {
-				struct ng_ppp_link *const alink =
-				    &priv->links[priv->activeLinks[i]];
-
-				if (MP_RECV_SEQ_DIFF(priv,
-				    alink->seq, priv->mseq) < 0)
-					alink->seq = priv->mseq;
-			}
-		}
+		ng_ppp_bump_mseq(node, qent->seq);
 
 		/* Drop it */
 		priv->bundleStats.dropFragments++;
@@ -1719,9 +1701,39 @@ ng_ppp_frag_process(node_p node)
 		FREE(qent, M_NETGRAPH_PPP);
 		priv->qlen--;
 
-		/* Process queue again */
-		return ng_ppp_frag_process(node);
+		return (1);
 	}
+	return (0);
+}
+
+/*
+ * Run the queue, restoring the queue invariants
+ */
+static int
+ng_ppp_frag_process(node_p node)
+{
+	const priv_p priv = NG_NODE_PRIVATE(node);
+	struct mbuf *m;
+	item_p item;
+	uint16_t proto;
+
+	do {
+		/* Deliver any deliverable packets */
+		while (ng_ppp_check_packet(node)) {
+			ng_ppp_get_packet(node, &m);
+			if ((m = ng_ppp_cutproto(m, &proto)) == NULL)
+				continue;
+			if (!PROT_VALID(proto)) {
+				priv->bundleStats.badProtos++;
+				NG_FREE_M(m);
+				continue;
+			}
+			if ((item = ng_package_data(m, NG_NOFLAGS)) != NULL)
+				ng_ppp_crypt_recv(node, item, proto,
+					NG_PPP_BUNDLE_LINKNUM);
+		}
+	  /* Delete dead fragments and try again */
+	} while (ng_ppp_frag_trim(node) || ng_ppp_frag_drop(node));
 
 	/* Done */
 	return (0);
@@ -1746,7 +1758,7 @@ ng_ppp_frag_checkstale(node_p node)
 	struct ng_ppp_frag *qent, *beg, *end;
 	struct timeval now, age;
 	struct mbuf *m;
-	int i, seq;
+	int seq;
 	item_p item;
 	int endseq;
 	uint16_t proto;
@@ -1802,19 +1814,6 @@ ng_ppp_frag_checkstale(node_p node)
 		endseq = end->seq;
 		ng_ppp_get_packet(node, &m);
 
-		/* Bump MSEQ if necessary */
-		if (MP_RECV_SEQ_DIFF(priv, priv->mseq, endseq) < 0) {
-			priv->mseq = endseq;
-			for (i = 0; i < priv->numActiveLinks; i++) {
-				struct ng_ppp_link *const alink =
-				    &priv->links[priv->activeLinks[i]];
-
-				if (MP_RECV_SEQ_DIFF(priv,
-				    alink->seq, priv->mseq) < 0)
-					alink->seq = priv->mseq;
-			}
-		}
-
 		if ((m = ng_ppp_cutproto(m, &proto)) == NULL)
 			continue;
 		if (!PROT_VALID(proto)) {