Some patches to ppp which improve stability. I have been running a

ppp based on these patches for about 3 weeks with no downtime.

The original submitters comments:

Two features iijppp has over kernel ppp that I like are predictor1
compression and demand dialing.  Here are a few bug fixes.

I expanded the priority queueing scheme and discovered it was broken
due to the assignment at ip.c line 300.  All packets were being
queued at the same priority.

Fixing priority queueing broke predictor1 compression.  Packets
were compressed before being queued and predictor1 worked as long
as the packets were popped off the queue in the same order they
were pushed onto the queue.

There were a few byte order problems in IP header tests also.

There is a recursion problem in SendLqrReport().  LcpClose() is
called when "Too many echo packets are lost" which winds up in
SendLqrReport() again.  I believe the original intention was to
just stop the LQR timer with the call to StopLqr() but the side
effects hurt.

Submitted by:	John Capo <jc@irbs.com>

12 files changed, 85 insertions, 72 deletions

diff --git a/usr.sbin/ppp/chap.c b/usr.sbin/ppp/chap.c
index 42d9846..2d03ee9 100644
--- a/usr.sbin/ppp/chap.c
+++ b/usr.sbin/ppp/chap.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: chap.c,v 1.3 1995/05/30 03:50:28 rgrimes Exp $
+ * $Id: chap.c,v 1.4 1996/01/10 21:27:37 phk Exp $
  *
  *	TODO:
  */
@@ -64,7 +64,7 @@ int count;
   DumpBp(bp);
 #endif
   LogPrintf(LOG_LCP, "ChapOutput: %s\n", chapcodes[code]);
-  HdlcOutput(PRI_NORMAL, PROTO_CHAP, bp);
+  HdlcOutput(PRI_LINK, PROTO_CHAP, bp);
 }
 
 
diff --git a/usr.sbin/ppp/fsm.c b/usr.sbin/ppp/fsm.c
index 60f8484..afe7c78 100644
--- a/usr.sbin/ppp/fsm.c
+++ b/usr.sbin/ppp/fsm.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: fsm.c,v 1.4 1995/09/09 13:23:53 joerg Exp $
+ * $Id: fsm.c,v 1.5 1996/01/11 17:48:44 phk Exp $
  *
  *  TODO:
  *		o Refer loglevel for log output
@@ -87,7 +87,7 @@ int count;
 #ifdef DEBUG
   DumpBp(bp);
 #endif
-  HdlcOutput(PRI_NORMAL, fp->proto, bp);
+  HdlcOutput(PRI_LINK, fp->proto, bp);
 }
 
 void
diff --git a/usr.sbin/ppp/hdlc.h b/usr.sbin/ppp/hdlc.h
index 3da20ef..ca05b11 100644
--- a/usr.sbin/ppp/hdlc.h
+++ b/usr.sbin/ppp/hdlc.h
@@ -15,7 +15,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: hdlc.h,v 1.2 1995/02/26 12:17:31 amurai Exp $
+ * $Id: hdlc.h,v 1.3 1996/01/11 17:48:48 phk Exp $
  *
  *	TODO:
  */
@@ -45,9 +45,14 @@
 /*
  *  Output priority
  */
+/* PRI_NORMAL and PRI_FAST have meaning only on the IP queue.
+ * All IP frames have the same priority once they are compressed.
+ * IP frames stay on the IP queue till they can be sent on the
+ * link. They are compressed at that time.
+*/
 #define	PRI_NORMAL	0	/* Normal priority */
-#define	PRI_FAST	1	/* Fast (interructive) */
-#define	PRI_URGENT	2	/* Urgent (LQR packets) */
+#define	PRI_FAST	1	/* Fast (interractive) */
+#define	PRI_LINK	1	/* Urgent (LQR packets) */
 
 unsigned char EscMap[33];
 
diff --git a/usr.sbin/ppp/ip.c b/usr.sbin/ppp/ip.c
index 6be717a..239b5b6 100644
--- a/usr.sbin/ppp/ip.c
+++ b/usr.sbin/ppp/ip.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: ip.c,v 1.6 1996/01/10 21:27:50 phk Exp $
+ * $Id: ip.c,v 1.7 1996/01/11 17:48:49 phk Exp $
  *
  *	TODO:
  *		o Return ICMP message for filterd packet
@@ -80,11 +80,12 @@ RestartIdleTimer()
   }
 }
 
-static u_short interactive_ports[8] = {
-  0, 513, 0, 0, 0, 21, 0, 23,
+static u_short interactive_ports[32] = {
+  544, 513, 514,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,
+    0,   0,   0,   0,   0,  21,  22,  23,   0,   0,   0,   0,   0,   0,   0, 543,
 };
 
-#define	INTERACTIVE(p)	(interactive_ports[(p) & 7] == (p))
+#define	INTERACTIVE(p)	(interactive_ports[(p) & 0x1F] == (p))
 
 static char *TcpFlags[] = {
   "FIN", "SYN", "RST", "PSH", "ACK", "URG",
@@ -133,7 +134,7 @@ int direction;
       if (fp->action) {
          /* permit fragments on in and out filter */
          if ((direction == FL_IN || direction == FL_OUT) &&
-             (pip->ip_off & IP_OFFMASK) != 0) {
+             (ntohs(pip->ip_off) & IP_OFFMASK) != 0) {
               return(A_PERMIT);
          }
 #ifdef DEBUG
@@ -216,7 +217,7 @@ int code;
   if (pip->ip_p != IPPROTO_ICMP) {
     bp = mballoc(cnt, MB_IPIN);
     bcopy(ptr, MBUF_CTOP(bp), cnt);
-    SendPppFrame(PRI_URGENT, bp);
+    SendPppFrame(bp);
     RestartIdleTimer();
     ipOutOctets += cnt;
   }
@@ -268,7 +269,7 @@ int direction;
     th = (struct tcphdr *)ptop;
     if (pip->ip_tos == IPTOS_LOWDELAY)
       pri = PRI_FAST;
-    else if (pip->ip_off == 0) {
+    else if ((ntohs(pip->ip_off) & IP_OFFMASK) == 0) {
       if (INTERACTIVE(ntohs(th->th_sport)) || INTERACTIVE(ntohs(th->th_dport)))
 	 pri = PRI_FAST;
     }
@@ -296,8 +297,8 @@ int direction;
     }
     break;
   }
-  pri = FilterCheck(pip, direction);
-  if (pri & A_DENY) {
+  
+  if ((FilterCheck(pip, direction) & A_DENY)) {
 #ifdef DEBUG
     logprintf("blocked.\n");
 #endif
@@ -347,28 +348,7 @@ struct mbuf *bp;		/* IN: Pointer to IP pakcet */
   RestartIdleTimer();
 }
 
-void
-IpOutput(ptr, cnt)
-u_char *ptr;			/* IN: Pointer to IP packet */
-int cnt;			/* IN: Length of packet */
-{
-  struct mbuf *bp;
-  int pri;
-
-  if (IpcpFsm.state != ST_OPENED)
-    return;
-
-  pri = PacketCheck(ptr, cnt, FL_OUT);
-  if (pri >= 0) {
-    bp = mballoc(cnt, MB_IPIN);
-    bcopy(ptr, MBUF_CTOP(bp), cnt);
-    SendPppFrame(pri, bp);
-    RestartIdleTimer();
-    ipOutOctets += cnt;
-  }
-}
-
-static struct mqueue IpOutputQueues[PRI_URGENT+1];
+static struct mqueue IpOutputQueues[PRI_FAST+1];
 
 void
 IpEnqueue(pri, ptr, count)
@@ -388,7 +368,7 @@ IsIpEnqueued()
 {
   struct mqueue *queue;
   int    exist = FALSE;
-  for (queue = &IpOutputQueues[PRI_URGENT]; queue >= IpOutputQueues; queue--) {
+  for (queue = &IpOutputQueues[PRI_FAST]; queue >= IpOutputQueues; queue--) {
      if ( queue->qlen > 0 ) {
        exist = TRUE;
        break;
@@ -406,15 +386,16 @@ IpStartOutput()
 
   if (IpcpFsm.state != ST_OPENED)
     return;
-  pri = PRI_URGENT;
-  for (queue = &IpOutputQueues[PRI_URGENT]; queue >= IpOutputQueues; queue--) {
+  pri = PRI_FAST;
+  for (queue = &IpOutputQueues[PRI_FAST]; queue >= IpOutputQueues; queue--) {
     if (queue->top) {
       bp = Dequeue(queue);
       if (bp) {
 	cnt = plength(bp);
-	SendPppFrame(pri, bp);
+	SendPppFrame(bp);
 	RestartIdleTimer();
 	ipOutOctets += cnt;
+	break;
        }
     }
     pri--;
diff --git a/usr.sbin/ppp/lcp.c b/usr.sbin/ppp/lcp.c
index 32da0e32..bf91538 100644
--- a/usr.sbin/ppp/lcp.c
+++ b/usr.sbin/ppp/lcp.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: lcp.c,v 1.6 1995/09/17 16:14:47 amurai Exp $
+ * $Id: lcp.c,v 1.7 1996/01/11 17:48:51 phk Exp $
  *
  * TODO:
  *      o Validate magic number received from peer.
@@ -331,6 +331,7 @@ StopAllTimers()
   StopIdleTimer();
   StopTimer(&AuthPapInfo.authtimer);
   StopTimer(&AuthChapInfo.authtimer);
+  StopLqrTimer();
 }
 
 static void
@@ -376,7 +377,6 @@ struct fsm *fp;
 {
   LogPrintf(LOG_LCP, "%s: LayerDown\n", fp->name);
   StopAllTimers();
-  StopLqr( LQM_LQR );
   OsLinkdown();
   NewPhase(PHASE_TERMINATE);
 }
diff --git a/usr.sbin/ppp/lqr.c b/usr.sbin/ppp/lqr.c
index c6dde2e..da8a798 100644
--- a/usr.sbin/ppp/lqr.c
+++ b/usr.sbin/ppp/lqr.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: lqr.c,v 1.4 1995/05/30 03:50:44 rgrimes Exp $
+ * $Id: lqr.c,v 1.5 1996/01/11 17:48:52 phk Exp $
  *
  *	o LQR based on RFC1333
  *
@@ -108,19 +108,19 @@ SendLqrReport()
       /*
        * XXX: Should implement LQM strategy
        */
-      LogPrintf(LOG_PHASE, "** Too many ECHO packets are lost. **\n");
+      LogPrintf(LOG_PHASE, "** 1 Too many ECHO packets are lost. **\n");
+      lqmmethod = 0;   /* Prevent rcursion via LcpClose() */
       LcpClose();
-      Cleanup(EX_ERRDEAD);
     } else {
       bp = mballoc(sizeof(struct lqrdata), MB_LQR);
-      HdlcOutput(PRI_URGENT, PROTO_LQR, bp);
+      HdlcOutput(PRI_LINK, PROTO_LQR, bp);
       lqrsendcnt++;
     }
   } else if (lqmmethod & LQM_ECHO) {
     if (echoseq - gotseq > 5) {
-      LogPrintf(LOG_PHASE, "** Too many ECHO packets are lost. **\n");
+      LogPrintf(LOG_PHASE, "** 2 Too many ECHO packets are lost. **\n");
+      lqmmethod = 0;   /* Prevent rcursion via LcpClose() */
       LcpClose();
-      Cleanup(EX_ERRDEAD);
     } else
       SendEchoReq();
   }
@@ -214,6 +214,12 @@ StartLqm()
 }
 
 void
+StopLqrTimer(void)
+{
+    StopTimer(&LqrTimer);
+}
+
+void
 StopLqr(method)
 int method;
 {
diff --git a/usr.sbin/ppp/lqr.h b/usr.sbin/ppp/lqr.h
index f8ca555..30fe971 100644
--- a/usr.sbin/ppp/lqr.h
+++ b/usr.sbin/ppp/lqr.h
@@ -15,7 +15,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id:$
+ * $Id: lqr.h,v 1.2 1995/02/26 12:17:40 amurai Exp $
  *
  *	TODO:
  */
@@ -62,5 +62,6 @@ extern void LqrDump __P((char *, struct lqrdata *));
 extern void LqrChangeOrder __P((struct lqrdata *, struct lqrdata *));
 extern void StartLqm __P((void));
 extern void StopLqr __P((int));
+extern void StopLqrTimer __P((void));
 extern void RecvEchoLqr __P((struct mbuf *));
 #endif
diff --git a/usr.sbin/ppp/main.c b/usr.sbin/ppp/main.c
index c1c90be..cae46e1 100644
--- a/usr.sbin/ppp/main.c
+++ b/usr.sbin/ppp/main.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: main.c,v 1.12 1996/01/10 21:27:53 phk Exp $
+ * $Id: main.c,v 1.13 1996/01/11 17:48:52 phk Exp $
  *
  *	TODO:
  *		o Add commands for traffic summary, version display, etc.
@@ -638,8 +638,6 @@ DoLoop()
   dial_up = FALSE;			/* XXXX */
   tries = 0;
   for (;;) {
-    if ( modem )
-	IpStartOutput();
     FD_ZERO(&rfds); FD_ZERO(&wfds); FD_ZERO(&efds);
 
    /*
@@ -676,6 +674,12 @@ DoLoop()
       }
     }
     qlen = ModemQlen();
+
+    if (qlen == 0) {
+      IpStartOutput();
+      qlen = ModemQlen();
+    }
+
     if (modem) {
       FD_SET(modem, &rfds);
       FD_SET(modem, &efds);
diff --git a/usr.sbin/ppp/modem.c b/usr.sbin/ppp/modem.c
index 020da90..9e7808c 100644
--- a/usr.sbin/ppp/modem.c
+++ b/usr.sbin/ppp/modem.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: modem.c,v 1.10 1996/01/10 21:27:55 phk Exp $
+ * $Id: modem.c,v 1.11 1996/01/11 17:48:54 phk Exp $
  *
  *  TODO:
  */
@@ -30,6 +30,7 @@
 #include <time.h>
 #include "hdlc.h"
 #include "lcp.h"
+#include "ip.h"
 #include "modem.h"
 #include "vars.h"
 
@@ -53,9 +54,11 @@ extern void PacketMode();
 #define	Online	(mbits & TIOCM_CD)
 
 static struct mbuf *modemout;
-static struct mqueue OutputQueues[PRI_URGENT+1];
+static struct mqueue OutputQueues[PRI_LINK+1];
 static int dev_is_modem;
 
+#undef QDEBUG
+
 void
 Enqueue(queue, bp)
 struct mqueue *queue;
@@ -627,6 +630,10 @@ int count;
 
   bp = mballoc(count, MB_MODEM);
   bcopy(ptr, MBUF_CTOP(bp), count);
+
+  /* Should be NORMAL and LINK only.
+   * All IP frames get here marked NORMAL.
+  */
   Enqueue(&OutputQueues[pri], bp);
 }
 
@@ -652,7 +659,7 @@ ModemQlen()
   int len = 0;
   int i;
 
-  for ( i = PRI_NORMAL; i <= PRI_URGENT; i ++ ) {
+  for ( i = PRI_NORMAL; i <= PRI_LINK; i ++ ) {
         queue = &OutputQueues[i];
 	len += queue->qlen;
   }
@@ -667,13 +674,15 @@ int fd;
   struct mqueue *queue;
   int nb, nw, i;
 
+    if (modemout == NULL && ModemQlen() == 0)
+      IpStartOutput();
   if (modemout == NULL) {
-    i = 0;
-    for (queue = &OutputQueues[PRI_URGENT]; queue >= OutputQueues; queue--) {
+    i = PRI_LINK;
+    for (queue = &OutputQueues[PRI_LINK]; queue >= OutputQueues; queue--) {
       if (queue->top) {
 	modemout = Dequeue(queue);
 #ifdef QDEBUG
-	if (i < 2) {
+	if (i > PRI_NORMAL) {
 	  struct mqueue *q;
 
 	  q = &OutputQueues[0];
@@ -683,13 +692,13 @@ int fd;
 #endif
 	break;
       }
-      i++;
+      i--;
     }
   }
   if (modemout) {
     nb = modemout->cnt;
     if (nb > 1600) nb = 1600;
-    if (fd == 0) fd = 1;
+    if (fd == 0) fd = 1;	/* XXX WTFO!  This is bogus */
     nw = write(fd, MBUF_CTOP(modemout), nb);
 #ifdef QDEBUG
     logprintf("wrote: %d(%d)\n", nw, nb);
diff --git a/usr.sbin/ppp/pap.c b/usr.sbin/ppp/pap.c
index 0c06307..a76fc31 100644
--- a/usr.sbin/ppp/pap.c
+++ b/usr.sbin/ppp/pap.c
@@ -18,7 +18,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: pap.c,v 1.2 1995/02/26 12:17:51 amurai Exp $
+ * $Id: pap.c,v 1.3 1995/05/30 03:50:53 rgrimes Exp $
  *
  *	TODO:
  */
@@ -67,7 +67,7 @@ int papid;
   *cp++ = keylen;
   bcopy(VarAuthKey, cp, keylen);
 
-  HdlcOutput(PRI_NORMAL, PROTO_PAP, bp);
+  HdlcOutput(PRI_LINK, PROTO_PAP, bp);
 }
 
 static void
@@ -92,7 +92,7 @@ int code;
   *cp++ = mlen;
   bcopy(message, cp, mlen);
   LogPrintf(LOG_PHASE, "PapOutput: %s\n", papcodes[code]);
-  HdlcOutput(PRI_NORMAL, PROTO_PAP, bp);
+  HdlcOutput(PRI_LINK, PROTO_PAP, bp);
 }
 
 /*
diff --git a/usr.sbin/ppp/pred.c b/usr.sbin/ppp/pred.c
index 6cac683..1c49c55 100644
--- a/usr.sbin/ppp/pred.c
+++ b/usr.sbin/ppp/pred.c
@@ -5,7 +5,7 @@
 
 /*
  *
- * $Id: pred.c,v 1.3 1995/05/30 03:50:55 rgrimes Exp $
+ * $Id: pred.c,v 1.4 1996/01/10 21:27:59 phk Exp $
  *
  * pred.c -- Test program for Dave Rand's rendition of the
  * predictor algorithm
@@ -152,7 +152,7 @@ struct mbuf *bp;
   *wp++ = fcs & 0377;
   *wp++ = fcs >> 8;
   mwp->cnt = wp - MBUF_CTOP(mwp);
-  HdlcOutput(pri, PROTO_COMPD, mwp);
+  HdlcOutput(PRI_NORMAL, PROTO_COMPD, mwp);
 }
 
 void
@@ -179,8 +179,10 @@ struct mbuf *bp;
     CcpInfo.compin += olen;
     len &= 0x7fff;
     if (len != len1) {	/* Error is detected. Send reset request */
+      LogPrintf(LOG_LCP, "%s: Length Error\n", CcpFsm.name);
       CcpSendResetReq(&CcpFsm);
       pfree(bp);
+      pfree(wp);
       return;
     }
     cp += olen - 4;
@@ -195,7 +197,8 @@ struct mbuf *bp;
   *pp++ = *cp++;
   fcs = HdlcFcs(INITFCS, bufp, wp->cnt = pp - bufp);
 #ifdef DEBUG
-  logprintf("fcs = %04x (%s), len = %x, olen = %x\n",
+  if (fcs != GOODFCS)
+  logprintf("fcs = 0x%04x (%s), len = 0x%x, olen = 0x%x\n",
        fcs, (fcs == GOODFCS)? "good" : "bad", len, olen);
 #endif
   if (fcs == GOODFCS) {
@@ -213,5 +216,10 @@ struct mbuf *bp;
     }
     DecodePacket(proto, wp);
   }
+  else
+  {
+      LogDumpBp(LOG_HDLC, "Bad FCS", wp);
+      pfree(wp);
+  }
   pfree(bp);
 }
diff --git a/usr.sbin/ppp/vjcomp.c b/usr.sbin/ppp/vjcomp.c
index 0c9c856..c0718fe 100644
--- a/usr.sbin/ppp/vjcomp.c
+++ b/usr.sbin/ppp/vjcomp.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: vjcomp.c,v 1.2 1995/02/26 12:18:08 amurai Exp $
+ * $Id: vjcomp.c,v 1.3 1995/05/30 03:51:02 rgrimes Exp $
  *
  *  TODO:
  */
@@ -40,8 +40,7 @@ VjInit()
 }
 
 void
-SendPppFrame(pri, bp)
-int pri;
+SendPppFrame(bp)
 struct mbuf *bp;
 {
   int type;
@@ -74,7 +73,7 @@ struct mbuf *bp;
     }
   } else
     proto = PROTO_IP;
-  HdlcOutput(pri, proto, bp);
+  HdlcOutput(PRI_NORMAL, proto, bp);
 }
 
 static struct mbuf *