Date: Fri, 12 Jan 1996 00:21:36 -0500 (EST) From: John Capo <jc@irbs.com> To: freebsd-hackers@FreeBSD.org Subject: Re: pppd vs ijppp (Bug Fixes) Message-ID: <199601120521.AAA27898@irbs.irbs.com> In-Reply-To: <Pine.SUN.3.91.960111224658.14135B-100000@lupine.nsi.nasa.gov> from "Michael C. Newell" at Jan 11, 96 11:06:53 pm
next in thread | previous in thread | raw e-mail | index | archive | help
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.
#0 0xa2da in SendLqrReport () at lqr.c:121
#1 0xa5e5 in StopLqr (method=1) at lqr.c:227
#2 0x97d1 in LcpLayerDown (fp=0x17088) at lcp.c:375
#3 0x6046 in FsmClose (fp=0x17088) at fsm.c:185
#4 0x985d in LcpClose () at lcp.c:405
#5 0xa2da in SendLqrReport () at lqr.c:121
#6 0xa5e5 in StopLqr (method=1) at lqr.c:227
#7 0x97d1 in LcpLayerDown (fp=0x17088) at lcp.c:375
#8 0x6046 in FsmClose (fp=0x17088) at fsm.c:185
#9 0x985d in LcpClose () at lcp.c:405
#10 0xa2da in SendLqrReport () at lqr.c:121
#11 0xa5e5 in StopLqr (method=1) at lqr.c:227
#12 0x97d1 in LcpLayerDown (fp=0x17088) at lcp.c:375
#13 0x5fb6 in FsmDown (fp=0x17088) at fsm.c:164
#14 0x9829 in LcpDown () at lcp.c:391
#15 0xcfe3 in DownConnection () at modem.c:212
There is a send-pr black hole report on the recursion bug.
I suspect another recursion problem somewhere that does not
spit out a message. Symptoms are illegal instructions and
a completely trashed core dump, 90% 0's.
These patches were generated against supped -stable with some
massaging. I may have ommitted something.
I have Nate's "ddial" patch working too but it is not included
here. Email if you are interested.
John Capo jc@irbs.com
IRBS Engineering High performance FreeBSD systems
(305) 792-9551 Unix/Internet Consulting - ISP Solutions
diff -c ../ppp/chap.c ./chap.c
*** ../ppp/chap.c Mon May 29 23:50:28 1995
--- ./chap.c Thu Jan 11 21:55:38 1996
***************
*** 62,68 ****
DumpBp(bp);
#endif
LogPrintf(LOG_LCP, "ChapOutput: %s\n", chapcodes[code]);
! HdlcOutput(PRI_NORMAL, PROTO_CHAP, bp);
}
--- 62,68 ----
DumpBp(bp);
#endif
LogPrintf(LOG_LCP, "ChapOutput: %s\n", chapcodes[code]);
! HdlcOutput(PRI_LINK, PROTO_CHAP, bp);
}
diff -c ../ppp/fsm.c ./fsm.c
*** ../ppp/fsm.c Fri Oct 6 11:10:02 1995
--- ./fsm.c Thu Jan 11 21:55:39 1996
***************
*** 86,92 ****
#ifdef DEBUG
DumpBp(bp);
#endif
! HdlcOutput(PRI_NORMAL, fp->proto, bp);
}
void
--- 86,92 ----
#ifdef DEBUG
DumpBp(bp);
#endif
! HdlcOutput(PRI_LINK, fp->proto, bp);
}
void
diff -c ../ppp/hdlc.h ./hdlc.h
*** ../ppp/hdlc.h Sun Feb 26 07:17:31 1995
--- ./hdlc.h Thu Jan 11 21:55:39 1996
***************
*** 45,53 ****
/*
* Output priority
*/
#define PRI_NORMAL 0 /* Normal priority */
! #define PRI_FAST 1 /* Fast (interructive) */
! #define PRI_URGENT 2 /* Urgent (LQR packets) */
unsigned char EscMap[33];
--- 45,58 ----
/*
* 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 (interractive) */
! #define PRI_LINK 1 /* Urgent (LQR packets) */
unsigned char EscMap[33];
diff -c ../ppp/ip.c ./ip.c
*** ../ppp/ip.c Fri Oct 6 11:10:03 1995
--- ./ip.c Thu Jan 11 22:14:50 1996
***************
*** 35,41 ****
#include "filter.h"
extern void SendPppFrame();
- extern int PacketCheck();
extern void LcpClose();
static struct pppTimer IdleTimer;
--- 35,40 ----
***************
*** 80,90 ****
}
}
! static u_short interactive_ports[8] = {
! 0, 513, 0, 0, 0, 21, 0, 23,
};
! #define INTERACTIVE(p) (interactive_ports[(p) & 7] == (p))
static char *TcpFlags[] = {
"FIN", "SYN", "RST", "PSH", "ACK", "URG",
--- 79,90 ----
}
}
! 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) & 0x1F] == (p))
static char *TcpFlags[] = {
"FIN", "SYN", "RST", "PSH", "ACK", "URG",
***************
*** 133,139 ****
if (fp->action) {
/* permit fragments on in and out filter */
if ((direction == FL_IN || direction == FL_OUT) &&
! (pip->ip_off & IP_OFFMASK) != 0) {
return(A_PERMIT);
}
#ifdef DEBUG
--- 133,139 ----
if (fp->action) {
/* permit fragments on in and out filter */
if ((direction == FL_IN || direction == FL_OUT) &&
! (ntohs(pip->ip_off) & IP_OFFMASK) != 0) {
return(A_PERMIT);
}
#ifdef DEBUG
***************
*** 217,223 ****
if (pip->ip_p != IPPROTO_ICMP) {
bp = mballoc(cnt, MB_IPIN);
bcopy(ptr, MBUF_CTOP(bp), cnt);
! SendPppFrame(PRI_URGENT, bp);
RestartIdleTimer();
ipOutOctets += cnt;
}
--- 217,223 ----
if (pip->ip_p != IPPROTO_ICMP) {
bp = mballoc(cnt, MB_IPIN);
bcopy(ptr, MBUF_CTOP(bp), cnt);
! SendPppFrame(bp);
RestartIdleTimer();
ipOutOctets += cnt;
}
***************
*** 269,275 ****
th = (struct tcphdr *)ptop;
if (pip->ip_tos == IPTOS_LOWDELAY)
pri = PRI_FAST;
! else if (pip->ip_off == 0) {
if (INTERACTIVE(ntohs(th->th_sport)) || INTERACTIVE(ntohs(th->th_dport)))
pri = PRI_FAST;
}
--- 269,275 ----
th = (struct tcphdr *)ptop;
if (pip->ip_tos == IPTOS_LOWDELAY)
pri = PRI_FAST;
! else if ((ntohs(pip->ip_off) & IP_OFFMASK) == 0) {
if (INTERACTIVE(ntohs(th->th_sport)) || INTERACTIVE(ntohs(th->th_dport)))
pri = PRI_FAST;
}
***************
*** 297,304 ****
}
break;
}
! pri = FilterCheck(pip, direction);
! if (pri & A_DENY) {
#ifdef DEBUG
logprintf("blocked.\n");
#endif
--- 297,304 ----
}
break;
}
!
! if ((FilterCheck(pip, direction) & A_DENY)) {
#ifdef DEBUG
logprintf("blocked.\n");
#endif
***************
*** 348,375 ****
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];
void
IpEnqueue(pri, ptr, count)
--- 348,354 ----
RestartIdleTimer();
}
! static struct mqueue IpOutputQueues[PRI_FAST+1];
void
IpEnqueue(pri, ptr, count)
***************
*** 389,395 ****
{
struct mqueue *queue;
int exist = FALSE;
! for (queue = &IpOutputQueues[PRI_URGENT]; queue >= IpOutputQueues; queue--) {
if ( queue->qlen > 0 ) {
exist = TRUE;
break;
--- 368,374 ----
{
struct mqueue *queue;
int exist = FALSE;
! for (queue = &IpOutputQueues[PRI_FAST]; queue >= IpOutputQueues; queue--) {
if ( queue->qlen > 0 ) {
exist = TRUE;
break;
***************
*** 407,421 ****
if (IpcpFsm.state != ST_OPENED)
return;
! pri = PRI_URGENT;
! for (queue = &IpOutputQueues[PRI_URGENT]; queue >= IpOutputQueues; queue--) {
if (queue->top) {
bp = Dequeue(queue);
if (bp) {
cnt = plength(bp);
! SendPppFrame(pri, bp);
RestartIdleTimer();
ipOutOctets += cnt;
}
}
pri--;
--- 386,401 ----
if (IpcpFsm.state != ST_OPENED)
return;
! pri = PRI_FAST;
! for (queue = &IpOutputQueues[PRI_FAST]; queue >= IpOutputQueues; queue--) {
if (queue->top) {
bp = Dequeue(queue);
if (bp) {
cnt = plength(bp);
! SendPppFrame(bp);
RestartIdleTimer();
ipOutOctets += cnt;
+ break;
}
}
pri--;
diff -c ../ppp/lcp.c ./lcp.c
*** ../ppp/lcp.c Fri Oct 6 11:10:05 1995
--- ./lcp.c Thu Jan 11 21:55:40 1996
***************
*** 327,332 ****
--- 327,333 ----
StopIdleTimer();
StopTimer(&AuthPapInfo.authtimer);
StopTimer(&AuthChapInfo.authtimer);
+ StopLqrTimer();
}
static void
***************
*** 372,378 ****
{
LogPrintf(LOG_LCP, "%s: LayerDown\n", fp->name);
StopAllTimers();
- StopLqr( LQM_LQR );
OsLinkdown();
NewPhase(PHASE_TERMINATE);
}
--- 373,378 ----
diff -c ../ppp/lqr.c ./lqr.c
*** ../ppp/lqr.c Mon May 29 23:50:44 1995
--- ./lqr.c Thu Jan 11 21:55:40 1996
***************
*** 107,125 ****
/*
* XXX: Should implement LQM strategy
*/
! LogPrintf(LOG_PHASE, "** Too many ECHO packets are lost. **\n");
LcpClose();
- Cleanup(EX_ERRDEAD);
} else {
bp = mballoc(sizeof(struct lqrdata), MB_LQR);
! HdlcOutput(PRI_URGENT, PROTO_LQR, bp);
lqrsendcnt++;
}
} else if (lqmmethod & LQM_ECHO) {
if (echoseq - gotseq > 5) {
! LogPrintf(LOG_PHASE, "** Too many ECHO packets are lost. **\n");
LcpClose();
- Cleanup(EX_ERRDEAD);
} else
SendEchoReq();
}
--- 107,125 ----
/*
* XXX: Should implement LQM strategy
*/
! LogPrintf(LOG_PHASE, "** 1 Too many ECHO packets are lost. **\n");
! lqmmethod = 0; /* Prevent rcursion via LcpClose() */
LcpClose();
} else {
bp = mballoc(sizeof(struct lqrdata), MB_LQR);
! HdlcOutput(PRI_LINK, PROTO_LQR, bp);
lqrsendcnt++;
}
} else if (lqmmethod & LQM_ECHO) {
if (echoseq - gotseq > 5) {
! LogPrintf(LOG_PHASE, "** 2 Too many ECHO packets are lost. **\n");
! lqmmethod = 0; /* Prevent rcursion via LcpClose() */
LcpClose();
} else
SendEchoReq();
}
***************
*** 210,215 ****
--- 210,221 ----
} else {
LogPrintf(LOG_LQM, "LQR is not activated.\n");
}
+ }
+
+ void
+ StopLqrTimer(void)
+ {
+ StopTimer(&LqrTimer);
}
void
diff -c ../ppp/main.c ./main.c
*** ../ppp/main.c Fri Oct 6 11:10:07 1995
--- ./main.c Thu Jan 11 22:01:03 1996
***************
*** 611,618 ****
dial_up = FALSE; /* XXXX */
for (;;) {
- if ( modem )
- IpStartOutput();
FD_ZERO(&rfds); FD_ZERO(&wfds); FD_ZERO(&efds);
/*
--- 611,616 ----
***************
*** 641,646 ****
--- 639,650 ----
}
}
qlen = ModemQlen();
+
+ if (qlen == 0) {
+ IpStartOutput();
+ qlen = ModemQlen();
+ }
+
if (modem) {
FD_SET(modem, &rfds);
FD_SET(modem, &efds);
diff -c ../ppp/modem.c ./modem.c
*** ../ppp/modem.c Fri Oct 6 11:10:09 1995
--- ./modem.c Thu Jan 11 22:14:59 1996
***************
*** 52,60 ****
#define Online (mbits & TIOCM_CD)
static struct mbuf *modemout;
! static struct mqueue OutputQueues[PRI_URGENT+1];
static int dev_is_modem;
void
Enqueue(queue, bp)
struct mqueue *queue;
--- 52,62 ----
#define Online (mbits & TIOCM_CD)
static struct mbuf *modemout;
! static struct mqueue OutputQueues[PRI_LINK+1];
static int dev_is_modem;
+ #undef QDEBUG
+
void
Enqueue(queue, bp)
struct mqueue *queue;
***************
*** 624,629 ****
--- 626,635 ----
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);
}
***************
*** 649,655 ****
int len = 0;
int i;
! for ( i = PRI_NORMAL; i <= PRI_URGENT; i ++ ) {
queue = &OutputQueues[i];
len += queue->qlen;
}
--- 655,661 ----
int len = 0;
int i;
! for ( i = PRI_NORMAL; i <= PRI_LINK; i ++ ) {
queue = &OutputQueues[i];
len += queue->qlen;
}
***************
*** 664,676 ****
struct mqueue *queue;
int nb, nw, i;
if (modemout == NULL) {
! i = 0;
! for (queue = &OutputQueues[PRI_URGENT]; queue >= OutputQueues; queue--) {
if (queue->top) {
modemout = Dequeue(queue);
#ifdef QDEBUG
! if (i < 2) {
struct mqueue *q;
q = &OutputQueues[0];
--- 670,684 ----
struct mqueue *queue;
int nb, nw, i;
+ if (modemout == NULL && ModemQlen() == 0)
+ IpStartOutput();
if (modemout == NULL) {
! i = PRI_LINK;
! for (queue = &OutputQueues[PRI_LINK]; queue >= OutputQueues; queue--) {
if (queue->top) {
modemout = Dequeue(queue);
#ifdef QDEBUG
! if (i > PRI_NORMAL) {
struct mqueue *q;
q = &OutputQueues[0];
***************
*** 680,692 ****
#endif
break;
}
! i++;
}
}
if (modemout) {
nb = modemout->cnt;
if (nb > 1600) nb = 1600;
! if (fd == 0) fd = 1;
nw = write(fd, MBUF_CTOP(modemout), nb);
#ifdef QDEBUG
logprintf("wrote: %d(%d)\n", nw, nb);
--- 688,700 ----
#endif
break;
}
! i--;
}
}
if (modemout) {
nb = modemout->cnt;
if (nb > 1600) nb = 1600;
! 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 -c ../ppp/pap.c ./pap.c
*** ../ppp/pap.c Mon May 29 23:50:53 1995
--- ./pap.c Thu Jan 11 21:55:40 1996
***************
*** 67,73 ****
*cp++ = keylen;
bcopy(VarAuthKey, cp, keylen);
! HdlcOutput(PRI_NORMAL, PROTO_PAP, bp);
}
static void
--- 67,73 ----
*cp++ = keylen;
bcopy(VarAuthKey, cp, keylen);
! HdlcOutput(PRI_LINK, PROTO_PAP, bp);
}
static void
***************
*** 92,98 ****
*cp++ = mlen;
bcopy(message, cp, mlen);
LogPrintf(LOG_PHASE, "PapOutput: %s\n", papcodes[code]);
! HdlcOutput(PRI_NORMAL, PROTO_PAP, bp);
}
/*
--- 92,98 ----
*cp++ = mlen;
bcopy(message, cp, mlen);
LogPrintf(LOG_PHASE, "PapOutput: %s\n", papcodes[code]);
! HdlcOutput(PRI_LINK, PROTO_PAP, bp);
}
/*
diff -c ../ppp/pred.c ./pred.c
*** ../ppp/pred.c Mon May 29 23:50:55 1995
--- ./pred.c Thu Jan 11 22:15:19 1996
***************
*** 153,159 ****
*wp++ = fcs & 0377;
*wp++ = fcs >> 8;
mwp->cnt = wp - MBUF_CTOP(mwp);
! HdlcOutput(pri, PROTO_COMPD, mwp);
}
void
--- 153,159 ----
*wp++ = fcs & 0377;
*wp++ = fcs >> 8;
mwp->cnt = wp - MBUF_CTOP(mwp);
! HdlcOutput(PRI_NORMAL, PROTO_COMPD, mwp);
}
void
***************
*** 180,187 ****
--- 180,189 ----
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;
***************
*** 196,202 ****
*pp++ = *cp++;
fcs = HdlcFcs(INITFCS, bufp, wp->cnt = pp - bufp);
#ifdef DEBUG
! logprintf("fcs = %04x (%s), len = %x, olen = %x\n",
fcs, (fcs == GOODFCS)? "good" : "bad", len, olen);
#endif
if (fcs == GOODFCS) {
--- 198,205 ----
*pp++ = *cp++;
fcs = HdlcFcs(INITFCS, bufp, wp->cnt = pp - bufp);
#ifdef DEBUG
! 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,218 ****
--- 216,226 ----
proto = (proto << 8) | *pp++;
}
DecodePacket(proto, wp);
+ }
+ else
+ {
+ LogDumpBp(LOG_HDLC, "Bad FCS", wp);
+ pfree(wp);
}
pfree(bp);
}
diff -c ../ppp/vars.c ./vars.c
*** ../ppp/vars.c Fri Oct 6 11:10:11 1995
--- ./vars.c Thu Jan 11 21:55:41 1996
***************
*** 29,35 ****
#include "defs.h"
char VarVersion[] = "Version 0.94";
! char VarLocalVersion[] = "$Date: 1995/10/06 11:24:47 $";
/*
* Order of conf option is important. See vars.h.
--- 29,35 ----
#include "defs.h"
char VarVersion[] = "Version 0.94";
! char VarLocalVersion[] = "$Date: 1995/10/08 14:57:31 $";
/*
* Order of conf option is important. See vars.h.
***************
*** 48,54 ****
struct pppvars pppVars = {
DEF_MRU, 0, MODEM_SPEED, CS8, 180, 30, 3,
! MODEM_DEV, OPEN_PASSIVE, LOCAL_NO_AUTH,
};
int
--- 48,54 ----
struct pppvars pppVars = {
DEF_MRU, 0, MODEM_SPEED, CS8, 180, 30, 3,
! REDIAL_PERIOD, 1, MODEM_DEV, OPEN_PASSIVE, LOCAL_NO_AUTH,
};
int
diff -c ../ppp/vjcomp.c ./vjcomp.c
*** ../ppp/vjcomp.c Mon May 29 23:51:02 1995
--- ./vjcomp.c Thu Jan 11 21:55:41 1996
***************
*** 40,47 ****
}
void
! SendPppFrame(pri, bp)
! int pri;
struct mbuf *bp;
{
int type;
--- 40,46 ----
}
void
! SendPppFrame(bp)
struct mbuf *bp;
{
int type;
***************
*** 74,80 ****
}
} else
proto = PROTO_IP;
! HdlcOutput(pri, proto, bp);
}
static struct mbuf *
--- 73,79 ----
}
} else
proto = PROTO_IP;
! HdlcOutput(PRI_NORMAL, proto, bp);
}
static struct mbuf *
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199601120521.AAA27898>