Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/net Move paese of conf-req, conf-nak and conf-rej into w...



details:   https://anonhg.NetBSD.org/src/rev/ab24529eee46
branches:  trunk
changeset: 378908:ab24529eee46
user:      yamaguchi <yamaguchi%NetBSD.org@localhost>
date:      Wed Apr 28 09:36:24 2021 +0000

description:
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

diffstat:

 sys/net/if_spppsubr.c |  297 +++++++++++++++++++++++++++----------------------
 sys/net/if_spppvar.h  |    9 +-
 2 files changed, 166 insertions(+), 140 deletions(-)

diffs (truncated from 595 to 300 lines):

diff -r c0d1a034890c -r ab24529eee46 sys/net/if_spppsubr.c
--- a/sys/net/if_spppsubr.c     Wed Apr 28 04:51:41 2021 +0000
+++ b/sys/net/if_spppsubr.c     Wed Apr 28 09:36:24 2021 +0000
@@ -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;
-
-       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) {
+       u_char ctype;
+
+       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);



Home | Main Index | Thread Index | Old Index