Date: Tue, 20 Apr 1999 11:42:39 -0700 (PDT) From: vak@cronyx.ru To: FreeBSD-gnats-submit@freebsd.org Subject: kern/11238: [patch] if_spppsubr.c - critical bug fixes Message-ID: <19990420184239.19B8015586@hub.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 11238
>Category: kern
>Synopsis: Synchronous PPP not functional in leased line mode
>Confidential: no
>Severity: critical
>Priority: high
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Tue Apr 20 11:50:01 PDT 1999
>Closed-Date:
>Last-Modified:
>Originator: Serge Vakulenko
>Release: FreeBSD-current i386
>Organization:
Cronyx Engineering Ltd.
>Environment:
Cronyx-Sigma and Cronyx-Tau synchronous adapters.
HDSL modem link to Cisco router.
>Description:
1) Gated logs the 'no buffer space available' error.
The reason was in too small output queue length
(was 32, increased to max 50 packets).
2) LCP options were not reset to default values on
every new connection. Instead the values from the previous
connection vere incorrectly used.
3) Line loopback was not detected properly.
4) Cisco/HDLC protocol was not functional.
5) IPCP could lock up in stopped state, up to the next reboot.
>How-To-Repeat:
cxconfig cx0 hdlc ppp +extclock
ifconfig cx0 <addr> <addr> up debug
>Fix:
Changes:
1) Output queue length increased to 50 packets.
Old length (32) was not enough in some cases.
2) Reset LCP options to default values, when starting new session.
3) Fixed line loopback detection.
4) Fixed Cisco/HDLC protocol.
5) Fixed IPCP lock up in stopped state.
___
Serge Vakulenko, <vak@cronyx.ru>
Cronyx Engineering Ltd.
--- if_sppp-cur.c Tue Apr 20 21:14:30 1999
+++ if_spppsubr.c Tue Apr 20 21:18:10 1999
@@ -664,7 +664,7 @@
* become invalid. So we
* - don't let packets with src ip addr 0 thru
* - we flag TCP packets with src ip 0 as an error
- */
+ */
if(ip->ip_src.s_addr == INADDR_ANY) /* -hm */
{
@@ -675,7 +675,7 @@
else
return(0);
}
-
+
/*
* Put low delay, telnet, rlogin and ftp control packets
* in front of the queue.
@@ -813,8 +813,9 @@
#if 0
sp->pp_flags = PP_KEEPALIVE;
#endif
- sp->pp_fastq.ifq_maxlen = 32;
- sp->pp_cpq.ifq_maxlen = 20;
+ sp->pp_if.if_snd.ifq_maxlen = 50;
+ sp->pp_fastq.ifq_maxlen = 50;
+ sp->pp_cpq.ifq_maxlen = 50;
sp->pp_loopcnt = 0;
sp->pp_alivecnt = 0;
sp->pp_seq = 0;
@@ -971,7 +972,7 @@
}
if (going_down) {
- if (sp->pp_mode != IFF_CISCO)
+ if (sp->pp_mode != IFF_CISCO)
lcp.Close(sp);
else if (sp->pp_tlf)
(sp->pp_tlf)(sp);
@@ -981,7 +982,7 @@
}
if (going_up) {
- if (sp->pp_mode != IFF_CISCO)
+ if (sp->pp_mode != IFF_CISCO)
lcp.Close(sp);
sp->pp_mode = newmode;
if (sp->pp_mode == 0) {
@@ -1138,7 +1139,7 @@
#if defined(__FreeBSD__) && __FreeBSD__ >= 3
getmicrouptime(&tv);
#endif
-
+
MGETHDR (m, M_DONTWAIT, MT_DATA);
if (! m)
return;
@@ -1531,13 +1532,8 @@
ntohl (*(long*)(h+1)) == sp->lcp.magic) {
/* Line loopback mode detected. */
printf(SPP_FMT "loopback\n", SPP_ARGS(ifp));
- if_down (ifp);
- sppp_qflush (&sp->pp_cpq);
-
/* Shut down the PPP link. */
- /* XXX */
- lcp.Down(sp);
- lcp.Up(sp);
+ lcp.Close(sp);
break;
}
*(long*)(h+1) = htonl (sp->lcp.magic);
@@ -1759,7 +1755,7 @@
case STATE_STOPPING:
sppp_cp_send(sp, cp->proto, TERM_REQ, ++sp->pp_seq,
0, 0);
- TIMEOUT(cp->TO, (void *)sp, sp->lcp.timeout,
+ TIMEOUT(cp->TO, (void *)sp, sp->lcp.timeout,
sp->ch[cp->protoidx]);
break;
case STATE_REQ_SENT:
@@ -1800,7 +1796,7 @@
case STATE_REQ_SENT:
case STATE_ACK_RCVD:
case STATE_ACK_SENT:
- TIMEOUT(cp->TO, (void *)sp, sp->lcp.timeout,
+ TIMEOUT(cp->TO, (void *)sp, sp->lcp.timeout,
sp->ch[cp->protoidx]);
break;
}
@@ -1837,6 +1833,11 @@
{
STDDCL;
+ sp->pp_alivecnt = 0;
+ sp->lcp.opts = (1 << LCP_OPT_MAGIC);
+ sp->lcp.magic = 0;
+ sp->lcp.protos = 0;
+ sp->lcp.mru = sp->lcp.their_mru = PP_MTU;
/*
* If this interface is passive or dial-on-demand, and we are
* still in Initial state, it means we've got an incoming
@@ -1875,19 +1876,18 @@
*/
if ((ifp->if_flags & (IFF_AUTO | IFF_PASSIVE)) == 0) {
log(LOG_INFO,
- SPP_FMT "Down event, taking interface down.\n",
+ SPP_FMT "Down event, continuing.\n",
SPP_ARGS(ifp));
- if_down(ifp);
} else {
if (debug)
log(LOG_DEBUG,
SPP_FMT "Down event (carrier loss)\n",
SPP_ARGS(ifp));
+ sp->pp_flags &= ~PP_CALLIN;
+ if (sp->state[IDX_LCP] != STATE_INITIAL)
+ lcp.Close(sp);
+ ifp->if_flags &= ~IFF_RUNNING;
}
- sp->pp_flags &= ~PP_CALLIN;
- if (sp->state[IDX_LCP] != STATE_INITIAL)
- lcp.Close(sp);
- ifp->if_flags &= ~IFF_RUNNING;
}
static void
@@ -1949,10 +1949,14 @@
switch (*p) {
case LCP_OPT_MAGIC:
/* Magic number. */
- /* fall through, both are same length */
+ if (len >= 6 && p[1] == 6)
+ continue;
+ if (debug)
+ addlog("[invalid] ");
+ break;
case LCP_OPT_ASYNC_MAP:
/* Async control character map. */
- if (len >= 6 || p[1] == 6)
+ if (len >= 6 && p[1] == 6)
continue;
if (debug)
addlog("[invalid] ");
@@ -2031,21 +2035,7 @@
addlog("0x%lx ", nmagic);
continue;
}
- /*
- * Local and remote magics equal -- loopback?
- */
- if (sp->pp_loopcnt >= MAXALIVECNT*5) {
- printf (SPP_FMT "loopback\n",
- SPP_ARGS(ifp));
- sp->pp_loopcnt = 0;
- if (ifp->if_flags & IFF_UP) {
- if_down(ifp);
- sppp_qflush(&sp->pp_cpq);
- /* XXX ? */
- lcp.Down(sp);
- lcp.Up(sp);
- }
- } else if (debug)
+ if (debug)
addlog("[glitch] ");
++sp->pp_loopcnt;
/*
@@ -2110,17 +2100,21 @@
rlen += p[1];
}
if (rlen) {
- if (++sp->fail_counter[IDX_LCP] >= sp->lcp.max_failure) {
- if (debug)
- addlog(" max_failure (%d) exceeded, "
- "send conf-rej\n",
- sp->lcp.max_failure);
- sppp_cp_send(sp, PPP_LCP, CONF_REJ, h->ident, rlen, buf);
- } else {
+ if (++sp->fail_counter[IDX_LCP] < sp->lcp.max_failure) {
if (debug)
- addlog(" send conf-nak\n");
+ addlog("send conf-nak\n");
sppp_cp_send (sp, PPP_LCP, CONF_NAK, h->ident, rlen, buf);
+ return 0;
}
+ if (debug)
+ addlog("max_failure (%d) exceeded, closing\n",
+ sp->lcp.max_failure);
+ if (sp->pp_loopcnt >= MAXALIVECNT)
+ printf ("%s%d: loopback\n",
+ ifp->if_name, ifp->if_unit);
+ lcp.Close(sp);
+ sp->fail_counter[IDX_LCP] = 0;
+ sp->pp_loopcnt = 0;
return 0;
} else {
if (debug)
@@ -2337,7 +2331,7 @@
/* notify low-level driver of state change */
if (sp->pp_chg)
sp->pp_chg(sp, (int)sp->pp_phase);
-
+
if (sp->pp_phase == PHASE_NETWORK)
/* if no NCP is starting, close down */
sppp_lcp_check_and_close(sp);
@@ -2518,6 +2512,10 @@
sp->ipcp.flags &= ~(IPCP_HISADDR_SEEN|IPCP_MYADDR_SEEN|IPCP_MYADDR_DYN);
+ /* To prevent IPCP lock up in stopped state. */
+ if (sp->state[IDX_IPCP] == STATE_STOPPED)
+ sp->state[IDX_IPCP] = STATE_CLOSED;
+
sppp_get_ip_addrs(sp, &myaddr, &hisaddr, 0);
/*
* If we don't have his address, this probably means our
@@ -2642,7 +2640,7 @@
(hisaddr == 1 && desiredaddr != 0)) {
/*
* Peer's address is same as our value,
- * or we have set it to 0.0.0.1 to
+ * or we have set it to 0.0.0.1 to
* indicate that we do not really care,
* this is agreeable. Gonna conf-ack
* it.
@@ -3003,7 +3001,7 @@
}
break;
}
-
+
if (debug) {
log(LOG_DEBUG,
SPP_FMT "chap input <%s id=0x%x len=%d name=",
@@ -3112,7 +3110,7 @@
sppp_print_string(sp->hisauth.name,
sppp_strnlen(sp->hisauth.name, AUTHNAMELEN));
addlog("\n");
- }
+ }
if (debug) {
log(LOG_DEBUG, SPP_FMT "chap input(%s) "
"<%s id=0x%x len=%d name=",
@@ -3792,14 +3790,13 @@
if (sp->pp_alivecnt == MAXALIVECNT) {
/* No keepalive packets got. Stop the interface. */
printf (SPP_FMT "down\n", SPP_ARGS(ifp));
- if_down (ifp);
- sppp_qflush (&sp->pp_cpq);
- if (sp->pp_mode != IFF_CISCO) {
+ if (sp->pp_mode == IFF_CISCO) {
+ if_down (ifp);
+ sppp_qflush (&sp->pp_cpq);
+ } else {
/* XXX */
/* Shut down the PPP link. */
- lcp.Down(sp);
- /* Initiate negotiation. XXX */
- lcp.Up(sp);
+ lcp.Close(sp);
}
}
if (sp->pp_alivecnt <= MAXALIVECNT)
@@ -3930,7 +3927,7 @@
si->sin_addr.s_addr = htonl(src);
/* add new route */
- error = rtinit(ifa, (int)RTM_ADD, RTF_HOST);
+ error = rtinit(ifa, (int)RTM_ADD, RTF_HOST);
if (debug && error)
{
log(LOG_DEBUG, SPP_FMT "sppp_set_ip_addr: rtinit ADD failed, error=%d",
@@ -3938,7 +3935,7 @@
}
#endif
}
-}
+}
static int
sppp_params(struct sppp *sp, u_long cmd, void *data)
@@ -4070,7 +4067,7 @@
/* if no NCP is starting, all this was in vain, close down */
sppp_lcp_check_and_close(sp);
}
-
+
static const char *
sppp_cp_type_name(u_char type)
>Release-Note:
>Audit-Trail:
>Unformatted:
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19990420184239.19B8015586>