Module Name: src
Committed By: yamaguchi
Date: Wed Apr 28 09:36:24 UTC 2021
Modified Files:
src/sys/net: if_spppsubr.c if_spppvar.h
Log Message:
Move paese of conf-req, conf-nak and conf-rej into workqueue
from softint context
When the pases were processed in softint, the state machine
in if_spppsubr.c had been broken by simultaneous events
on rare occasions.
Example:
1. Do ifconfig pppoe* up
- lcp open event is enqueued to workqueue
2. Receive conf-ack, and parse the packet
- save mru to sp->lcp.their_mru
- lcp RCR+ event is enqueued to workqueue
3. Process lcp open event
- initialize data including sp->lcp.their_mru
4. Process lcp RCR+ event
- Use sp->lcp.their_mru
- but it was initialized
To generate a diff of this commit:
cvs rdiff -u -r1.226 -r1.227 src/sys/net/if_spppsubr.c
cvs rdiff -u -r1.33 -r1.34 src/sys/net/if_spppvar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/net/if_spppsubr.c
diff -u src/sys/net/if_spppsubr.c:1.226 src/sys/net/if_spppsubr.c:1.227
--- src/sys/net/if_spppsubr.c:1.226 Mon Apr 26 08:45:57 2021
+++ src/sys/net/if_spppsubr.c Wed Apr 28 09:36:24 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: if_spppsubr.c,v 1.226 2021/04/26 08:45:57 yamaguchi Exp $ */
+/* $NetBSD: if_spppsubr.c,v 1.227 2021/04/28 09:36:24 yamaguchi Exp $ */
/*
* Synchronous PPP/Cisco link level subroutines.
@@ -41,7 +41,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_spppsubr.c,v 1.226 2021/04/26 08:45:57 yamaguchi Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_spppsubr.c,v 1.227 2021/04/28 09:36:24 yamaguchi Exp $");
#if defined(_KERNEL_OPT)
#include "opt_inet.h"
@@ -258,7 +258,8 @@ struct cp {
void (*tls)(const struct cp *, struct sppp *);
void (*tlf)(const struct cp *, struct sppp *);
void (*scr)(struct sppp *);
- void (*scan)(const struct cp *, struct sppp *);
+ void (*screply)(const struct cp *, struct sppp *, u_char,
+ uint8_t, size_t, void *);
/* message parser */
enum cp_rcr_type
@@ -347,8 +348,8 @@ static void sppp_auth_send(const struct
unsigned int, unsigned int, ...);
static int sppp_auth_role(const struct cp *, struct sppp *);
static void sppp_auth_to_event(struct sppp *, void *);
-static void sppp_auth_sca_scn(const struct cp *, struct sppp *);
-
+static void sppp_auth_screply(const struct cp *, struct sppp *,
+ u_char, uint8_t, size_t, void *);
static void sppp_up_event(struct sppp *, void *);
static void sppp_down_event(struct sppp *, void *);
static void sppp_open_event(struct sppp *, void *);
@@ -364,7 +365,8 @@ static void sppp_rxj_event(struct sppp *
static void sppp_null(struct sppp *);
static void sppp_tls(const struct cp *, struct sppp *);
static void sppp_tlf(const struct cp *, struct sppp *);
-static void sppp_sca_scn(const struct cp *, struct sppp *);
+static void sppp_screply(const struct cp *, struct sppp *,
+ u_char, uint8_t, size_t, void *);
static void sppp_ifdown(struct sppp *, void *);
static void sppp_lcp_init(struct sppp *);
@@ -466,7 +468,7 @@ static const struct cp lcp = {
sppp_lcp_up, sppp_lcp_down, sppp_lcp_open,
sppp_close_event, sppp_to_event,
sppp_lcp_tlu, sppp_lcp_tld, sppp_lcp_tls,
- sppp_lcp_tlf, sppp_lcp_scr, sppp_sca_scn,
+ sppp_lcp_tlf, sppp_lcp_scr, sppp_screply,
sppp_lcp_confreq, sppp_lcp_confrej, sppp_lcp_confnak
};
@@ -481,7 +483,7 @@ static const struct cp ipcp = {
sppp_up_event, sppp_down_event, sppp_ipcp_open,
sppp_ipcp_close, sppp_to_event,
sppp_ipcp_tlu, sppp_null, sppp_tls,
- sppp_tlf, sppp_ipcp_scr, sppp_sca_scn,
+ sppp_tlf, sppp_ipcp_scr, sppp_screply,
sppp_ipcp_confreq, sppp_ipcp_confrej, sppp_ipcp_confnak,
};
@@ -496,7 +498,7 @@ static const struct cp ipv6cp = {
sppp_up_event, sppp_down_event, sppp_ipv6cp_open,
sppp_close_event, sppp_to_event,
sppp_ipv6cp_tlu, sppp_null, sppp_tls,
- sppp_tlf, sppp_ipv6cp_scr, sppp_sca_scn,
+ sppp_tlf, sppp_ipv6cp_scr, sppp_screply,
sppp_ipv6cp_confreq, sppp_ipv6cp_confrej, sppp_ipv6cp_confnak,
};
@@ -505,7 +507,7 @@ static const struct cp pap = {
sppp_up_event, sppp_down_event, sppp_open_event,
sppp_close_event, sppp_to_event,
sppp_pap_tlu, sppp_null, sppp_tls, sppp_tlf,
- sppp_pap_scr, sppp_auth_sca_scn,
+ sppp_pap_scr, sppp_auth_screply,
NULL, NULL, NULL
};
@@ -514,7 +516,7 @@ static const struct cp chap = {
sppp_up_event, sppp_down_event, sppp_chap_open,
sppp_close_event, sppp_auth_to_event,
sppp_chap_tlu, sppp_null, sppp_tls, sppp_tlf,
- sppp_chap_scr, sppp_auth_sca_scn,
+ sppp_chap_scr, sppp_auth_screply,
NULL, NULL, NULL
};
@@ -680,7 +682,7 @@ sppp_input(struct ifnet *ifp, struct mbu
case PPP_LCP:
SPPP_UNLOCK(sp);
sppp_cp_input(&lcp, sp, m);
- m_freem(m);
+ /* already m_freem(m) */
return;
case PPP_PAP:
SPPP_UNLOCK(sp);
@@ -701,8 +703,10 @@ sppp_input(struct ifnet *ifp, struct mbu
SPPP_UNLOCK(sp);
if (sp->pp_phase == SPPP_PHASE_NETWORK) {
sppp_cp_input(&ipcp, sp, m);
+ /* already m_freem(m) */
+ } else {
+ m_freem(m);
}
- m_freem(m);
return;
case PPP_IP:
if (sp->scp[IDX_IPCP].state == STATE_OPENED) {
@@ -716,8 +720,10 @@ sppp_input(struct ifnet *ifp, struct mbu
SPPP_UNLOCK(sp);
if (sp->pp_phase == SPPP_PHASE_NETWORK) {
sppp_cp_input(&ipv6cp, sp, m);
+ /* already m_freem(m) */
+ } else {
+ m_freem(m);
}
- m_freem(m);
return;
case PPP_IPV6:
@@ -1637,11 +1643,8 @@ sppp_cp_input(const struct cp *cp, struc
STDDCL;
struct lcp_header *h;
int printlen, len = m->m_pkthdr.len;
- enum cp_rcr_type type;
- size_t blen, rlen;
u_char *p;
uint32_t u32;
- uint8_t *buf;
SPPP_LOCK(sp, RW_WRITER);
@@ -1650,8 +1653,7 @@ sppp_cp_input(const struct cp *cp, struc
log(LOG_DEBUG,
"%s: %s invalid packet length: %d bytes\n",
ifp->if_xname, cp->name, len);
- SPPP_UNLOCK(sp);
- return;
+ goto out;
}
h = mtod(m, struct lcp_header *);
if (debug) {
@@ -1681,32 +1683,14 @@ sppp_cp_input(const struct cp *cp, struc
break;
}
- buf = NULL;
- blen = 0;
- rlen = 0;
-
- type = (cp->parse_confreq)(sp, h, len,
- &buf, &blen, &rlen);
-
- if (type == CP_RCR_ERR) {
- /* fatal error, shut down */
- sppp_wq_add(sp->wq_cp, &sp->scp[IDX_LCP].work_close);
- sppp_wq_add(sp->wq_cp, &sp->scp[IDX_LCP].work_open);
- } else if (buf != NULL) {
- if (sp->scp[cp->protoidx].rcr_buf != NULL) {
- kmem_intr_free(sp->scp[cp->protoidx].rcr_buf,
- sp->scp[cp->protoidx].rcr_blen);
- }
-
- sp->scp[cp->protoidx].rcr_buf = (void *)buf;
- buf = NULL;
-
- sp->scp[cp->protoidx].rcr_blen = blen;
- sp->scp[cp->protoidx].rcr_rlen = rlen;
- sp->scp[cp->protoidx].rcr_type = type;
- sp->scp[cp->protoidx].rconfid = h->ident;
- sppp_wq_add(sp->wq_cp, &sp->scp[cp->protoidx].work_rcr);
- }
+ sp->scp[cp->protoidx].rcr_type = CP_RCR_NONE;
+ sp->scp[cp->protoidx].rconfid = h->ident;
+ if (sp->scp[cp->protoidx].mbuf_confreq != NULL) {
+ m_freem(sp->scp[cp->protoidx].mbuf_confreq);
+ }
+ sp->scp[cp->protoidx].mbuf_confreq = m;
+ m = NULL;
+ sppp_wq_add(sp->wq_cp, &sp->scp[cp->protoidx].work_rcr);
break;
case CONF_ACK:
if (h->ident != sp->scp[cp->protoidx].confid) {
@@ -1729,14 +1713,14 @@ sppp_cp_input(const struct cp *cp, struc
if_statinc(ifp, if_ierrors);
break;
}
- if (h->type == CONF_NAK)
- (cp->parse_confnak)(sp, h, len);
- else /* CONF_REJ */
- (cp->parse_confrej)(sp, h, len);
+ if (sp->scp[cp->protoidx].mbuf_confnak) {
+ m_freem(sp->scp[cp->protoidx].mbuf_confnak);
+ }
+ sp->scp[cp->protoidx].mbuf_confnak = m;
+ m = NULL;
sppp_wq_add(sp->wq_cp, &sp->scp[cp->protoidx].work_rcn);
break;
-
case TERM_REQ:
sp->scp[cp->protoidx].rseq = h->ident;
sppp_wq_add(sp->wq_cp, &sp->scp[cp->protoidx].work_rtr);
@@ -1832,6 +1816,10 @@ sppp_cp_input(const struct cp *cp, struc
if (ntohl(u32) == sp->lcp.magic) {
/* Line loopback mode detected. */
printf("%s: loopback\n", ifp->if_xname);
+ /*
+ * There is no change for items of sp->scp[cp->protoidx]
+ * while if_down() even without SPPP_LOCK
+ */
SPPP_UNLOCK(sp);
if_down(ifp);
SPPP_LOCK(sp, RW_WRITER);
@@ -1884,7 +1872,10 @@ sppp_cp_input(const struct cp *cp, struc
if_statinc(ifp, if_ierrors);
}
+out:
SPPP_UNLOCK(sp);
+ if (m != NULL)
+ m_freem(m);
}
/*
@@ -2128,41 +2119,37 @@ sppp_to_event(struct sppp *sp, void *xcp
splx(s);
}
-
static void
-sppp_rcr_event(struct sppp *sp, void *xcp)
+sppp_rcr_update_state(const struct cp *cp, struct sppp *sp,
+ enum cp_rcr_type type, uint8_t ident, size_t msglen, void *msg)
{
- const struct cp *cp = xcp;
- enum cp_rcr_type type;
- void *buf;
- size_t blen;
STDDCL;
+ u_char ctype;
- KASSERT(!cpu_softintr_p());
-
- type = sp->scp[cp->protoidx].rcr_type;
- buf = sp->scp[cp->protoidx].rcr_buf;
- blen = sp->scp[cp->protoidx].rcr_blen;
-
- if (type == CP_RCR_ACK) {
+ if (type == CP_RCR_ERR) {
+ /* parse error, shut down */
+ sppp_wq_add(sp->wq_cp, &sp->scp[IDX_LCP].work_close);
+ sppp_wq_add(sp->wq_cp, &sp->scp[IDX_LCP].work_open);
+ } else if (type == CP_RCR_ACK) {
/* RCR+ event */
+ ctype = CONF_ACK;
switch (sp->scp[cp->protoidx].state) {
case STATE_OPENED:
sppp_cp_change_state(cp, sp, STATE_ACK_SENT);
cp->tld(sp);
cp->scr(sp);
- cp->scan(cp, sp);
+ cp->screply(cp, sp, ctype, ident, msglen, msg);
break;
case STATE_REQ_SENT:
sppp_cp_change_state(cp, sp, STATE_ACK_SENT);
/* fall through */
case STATE_ACK_SENT:
- cp->scan(cp, sp);
+ cp->screply(cp, sp, ctype, ident, msglen, msg);
break;
case STATE_STOPPED:
sppp_cp_change_state(cp, sp, STATE_ACK_SENT);
cp->scr(sp);
- cp->scan(cp, sp);
+ cp->screply(cp, sp, ctype, ident, msglen, msg);
break;
case STATE_ACK_RCVD:
sppp_cp_change_state(cp, sp, STATE_OPENED);
@@ -2171,20 +2158,15 @@ sppp_rcr_event(struct sppp *sp, void *xc
ifp->if_xname,
cp->name);
cp->tlu(sp);
- cp->scan(cp, sp);
+ cp->screply(cp, sp, ctype, ident, msglen, msg);
break;
case STATE_CLOSING:
case STATE_STOPPING:
- if (buf != NULL) {
- sp->scp[cp->protoidx].rcr_buf = NULL;
- sp->scp[cp->protoidx].rcr_blen = 0;
- kmem_free(buf, blen);
- }
break;
case STATE_CLOSED:
if ((cp->flags & CP_AUTH) == 0) {
sppp_cp_send(sp, cp->proto, TERM_ACK,
- sp->scp[cp->protoidx].rconfid, 0, 0);
+ ident, 0, 0);
}
break;
default:
@@ -2193,43 +2175,39 @@ sppp_rcr_event(struct sppp *sp, void *xc
sppp_state_name(sp->scp[cp->protoidx].state));
if_statinc(ifp, if_ierrors);
}
- } else {
+ } else if (type == CP_RCR_NAK || type == CP_RCR_REJ) {
+ ctype = type == CP_RCR_NAK ? CONF_NAK : CONF_REJ;
/* RCR- event */
switch (sp->scp[cp->protoidx].state) {
case STATE_OPENED:
sppp_cp_change_state(cp, sp, STATE_REQ_SENT);
cp->tld(sp);
cp->scr(sp);
- cp->scan(cp, sp);
+ cp->screply(cp, sp, ctype, ident, msglen, msg);
break;
case STATE_ACK_SENT:
sppp_cp_change_state(cp, sp, STATE_REQ_SENT);
/* fall through */
case STATE_REQ_SENT:
- cp->scan(cp, sp);
+ cp->screply(cp, sp, ctype, ident, msglen, msg);
break;
case STATE_STOPPED:
sppp_cp_change_state(cp, sp, STATE_REQ_SENT);
cp->scr(sp);
- cp->scan(cp, sp);
+ cp->screply(cp, sp, ctype, ident, msglen, msg);
break;
case STATE_ACK_RCVD:
sppp_cp_change_state(cp, sp, STATE_ACK_RCVD);
- cp->scan(cp, sp);
+ cp->screply(cp, sp, ctype, ident, msglen, msg);
break;
case STATE_CLOSING:
case STATE_STOPPING:
- if (buf != NULL) {
- sp->scp[cp->protoidx].rcr_buf = NULL;
- sp->scp[cp->protoidx].rcr_blen = 0;
- kmem_free(buf, blen);
- }
break;
case STATE_CLOSED:
sppp_cp_change_state(cp, sp, STATE_CLOSED);
if ((cp->flags & CP_AUTH) == 0) {
sppp_cp_send(sp, cp->proto, TERM_ACK,
- sp->scp[cp->protoidx].rconfid, 0, 0);
+ ident, 0, 0);
}
break;
default:
@@ -2242,6 +2220,52 @@ sppp_rcr_event(struct sppp *sp, void *xc
}
static void
+sppp_rcr_event(struct sppp *sp, void *xcp)
+{
+ const struct cp *cp = xcp;
+ struct lcp_header *h;
+ struct mbuf *m;
+ enum cp_rcr_type type;
+ size_t len;
+ uint8_t *buf;
+ size_t blen, rlen;
+ uint8_t ident;
+
+ KASSERT(!cpu_softintr_p());
+
+ if (cp->parse_confreq != NULL) {
+ m = sp->scp[cp->protoidx].mbuf_confreq;
+ if (m == NULL)
+ return;
+ sp->scp[cp->protoidx].mbuf_confreq = NULL;
+
+ h = mtod(m, struct lcp_header *);
+ if (h->type != CONF_REQ) {
+ m_freem(m);
+ return;
+ }
+
+ ident = h->ident;
+ len = MIN(m->m_pkthdr.len, ntohs(h->len));
+
+ type = (cp->parse_confreq)(sp, h, len,
+ &buf, &blen, &rlen);
+ m_freem(m);
+ } else {
+ /* mbuf_cofreq is already parsed and freed */
+ type = sp->scp[cp->protoidx].rcr_type;
+ ident = sp->scp[cp->protoidx].rconfid;
+ buf = NULL;
+ blen = rlen = 0;
+ }
+
+ sppp_rcr_update_state(cp, sp, type, ident, rlen, (void *)buf);
+
+ if (buf != NULL)
+ kmem_free(buf, blen);
+}
+
+static void
sppp_rca_event(struct sppp *sp, void *xcp)
{
const struct cp *cp = xcp;
@@ -2291,10 +2315,35 @@ static void
sppp_rcn_event(struct sppp *sp, void *xcp)
{
const struct cp *cp = xcp;
+ struct lcp_header *h;
+ struct mbuf *m;
struct ifnet *ifp = &sp->pp_if;
+ size_t len;
KASSERT(!cpu_softintr_p());
+ m = sp->scp[cp->protoidx].mbuf_confnak;
+ if (m == NULL)
+ return;
+ sp->scp[cp->protoidx].mbuf_confnak = NULL;
+
+ h = mtod(m, struct lcp_header *);
+ len = MIN(m->m_pkthdr.len, ntohs(h->len));
+
+ switch (h->type) {
+ case CONF_NAK:
+ (cp->parse_confnak)(sp, h, len);
+ break;
+ case CONF_REJ:
+ (cp->parse_confrej)(sp, h, len);
+ break;
+ default:
+ m_freem(m);
+ return;
+ }
+
+ m_freem(m);
+
switch (sp->scp[cp->protoidx].state) {
case STATE_CLOSED:
case STATE_STOPPED:
@@ -5386,21 +5435,20 @@ sppp_auth_to_event(struct sppp *sp, void
}
static void
-sppp_auth_sca_scn(const struct cp *cp, struct sppp *sp)
+sppp_auth_screply(const struct cp *cp, struct sppp *sp, u_char ctype,
+ uint8_t ident, size_t _mlen __unused, void *_msg __unused)
{
static const char *succmsg = "Welcome!";
static const char *failmsg = "Failed...";
const char *msg;
- u_char type, rconfid, mlen;
+ u_char type, mlen;
KASSERT(SPPP_WLOCKED(sp));
if (!ISSET(sppp_auth_role(cp, sp), SPPP_AUTH_SERV))
return;
- rconfid = sp->scp[cp->protoidx].rconfid;
-
- if (sp->scp[cp->protoidx].rcr_type == CP_RCR_ACK) {
+ if (ctype == CONF_ACK) {
type = cp->proto == PPP_CHAP ? CHAP_SUCCESS : PAP_ACK;
msg = succmsg;
mlen = sizeof(succmsg) - 1;
@@ -5416,8 +5464,7 @@ sppp_auth_sca_scn(const struct cp *cp, s
sp->pp_auth_failures++;
}
- sppp_auth_send(cp, sp, type, rconfid,
- mlen, (const u_char *)msg, 0);
+ sppp_auth_send(cp, sp, type, ident, mlen, (const u_char *)msg, 0);
}
/*
@@ -6451,62 +6498,42 @@ sppp_tlf(const struct cp *cp, struct spp
sp->lcp.protos &= ~(1 << cp->protoidx);
/* cleanup */
- if (sp->scp[cp->protoidx].rcr_buf != NULL) {
- kmem_free(sp->scp[cp->protoidx].rcr_buf,
- sp->scp[cp->protoidx].rcr_blen);
+ if (sp->scp[cp->protoidx].mbuf_confreq != NULL) {
+ m_freem(sp->scp[cp->protoidx].mbuf_confreq);
+ sp->scp[cp->protoidx].mbuf_confreq = NULL;
+ }
+ if (sp->scp[cp->protoidx].mbuf_confnak != NULL) {
+ m_freem(sp->scp[cp->protoidx].mbuf_confnak);
+ sp->scp[cp->protoidx].mbuf_confnak = NULL;
}
- sp->scp[cp->protoidx].rcr_buf = NULL;
- sp->scp[cp->protoidx].rcr_blen = 0;
- sp->scp[cp->protoidx].rcr_rlen = 0;
-
sppp_lcp_check_and_close(sp);
}
static void
-sppp_sca_scn(const struct cp *cp, struct sppp *sp)
+sppp_screply(const struct cp *cp, struct sppp *sp, u_char type,
+ uint8_t ident, size_t msglen, void *msg)
{
STDDCL;
- u_char rconfid, rlen;
- int type;
- void *buf;
- size_t blen;
-
- rconfid = sp->scp[cp->protoidx].rconfid;
- buf = sp->scp[cp->protoidx].rcr_buf;
- rlen = sp->scp[cp->protoidx].rcr_rlen;
- blen = sp->scp[cp->protoidx].rcr_blen;
-
- sp->scp[cp->protoidx].rcr_buf = NULL;
- sp->scp[cp->protoidx].rcr_blen = 0;
-
- switch (sp->scp[cp->protoidx].rcr_type) {
- case CP_RCR_ACK:
- type = CONF_ACK;
- break;
- case CP_RCR_REJ:
- type = CONF_REJ;
- break;
- case CP_RCR_NAK:
- type = CONF_NAK;
+
+ if (msglen == 0)
+ return;
+
+ switch (type) {
+ case CONF_ACK:
+ case CONF_NAK:
+ case CONF_REJ:
break;
default:
- type = -1;
- break;
+ return;
}
- sp->scp[cp->protoidx].rcr_type = CP_RCR_NONE;
-
- if (buf != NULL) {
- if (rlen > 0 && type != -1) {
- if (debug) {
- log(LOG_DEBUG, "%s: send %s\n",
- ifp->if_xname, sppp_cp_type_name(type));
- }
- sppp_cp_send(sp, cp->proto, type, rconfid, rlen, buf);
- }
- kmem_free(buf, blen);
+ if (debug) {
+ log(LOG_DEBUG, "%s: send %s\n",
+ ifp->if_xname, sppp_cp_type_name(type));
}
+
+ sppp_cp_send(sp, cp->proto, type, ident, msglen, msg);
}
static void
Index: src/sys/net/if_spppvar.h
diff -u src/sys/net/if_spppvar.h:1.33 src/sys/net/if_spppvar.h:1.34
--- src/sys/net/if_spppvar.h:1.33 Fri Apr 16 02:26:25 2021
+++ src/sys/net/if_spppvar.h Wed Apr 28 09:36:24 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: if_spppvar.h,v 1.33 2021/04/16 02:26:25 yamaguchi Exp $ */
+/* $NetBSD: if_spppvar.h,v 1.34 2021/04/28 09:36:24 yamaguchi Exp $ */
#ifndef _NET_IF_SPPPVAR_H_
#define _NET_IF_SPPPVAR_H_
@@ -117,10 +117,9 @@ struct sppp_cp {
int rst_counter; /* restart counter */
int fail_counter; /* negotiation failure counter */
struct callout ch; /* per-proto and if callouts */
- u_char rcr_type;
- void *rcr_buf;
- size_t rcr_blen;
- int rcr_rlen;
+ u_char rcr_type; /* parsing result of conf-req */
+ struct mbuf *mbuf_confreq; /* received conf-req */
+ struct mbuf *mbuf_confnak; /* received conf-nak or conf-rej */
struct sppp_work work_up;
struct sppp_work work_down;
