With bluhm@'s diff for parallel forwarding pipex(4) could be accessed in
parallel through (*ifp->if_input)() -> ether_input() ->
pipex_pppoe_input(). PPPOE pipex(4) sessions are mostly immutable except
MPPE crypt. 

The diff below makes pipex(4) a little bit more parallelization
reliable.

The new per-session `pxs_mtx' mutex(9) used to protect session's
`ccp-id' which is incremented each time we send CCP reset-request.

The new per-MPPE context `pxm_mtx' mutex(9) used to protect MPPE
context. We have two of them: one for the input and one for output path.
With bluhm@'s diff those paths are mostly serialized, except the case
when we send CCP reset-request. I don't see the reason to other
pipex_pppoe_input() threads spin while we perform pipex_ccp_output(). 

Where is no lock order limitations because those new mutex(9)'es newer
held together.

I'm not PPPOE user and I'll be happy if such user tests this diff in
real workflow. 

Index: sys/net/pipex.c
===================================================================
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.134
diff -u -p -r1.134 pipex.c
--- sys/net/pipex.c     20 Jul 2021 16:44:55 -0000      1.134
+++ sys/net/pipex.c     21 Jul 2021 22:27:47 -0000
@@ -263,6 +263,7 @@ pipex_init_session(struct pipex_session 
 
        /* prepare a new session */
        session = pool_get(&pipex_session_pool, PR_WAITOK | PR_ZERO);
+       mtx_init(&session->pxs_mtx, IPL_SOFTNET);
        session->state = PIPEX_STATE_INITIAL;
        session->protocol = req->pr_protocol;
        session->session_id = req->pr_session_id;
@@ -2099,6 +2100,7 @@ pipex_mppe_init(struct pipex_mppe *mppe,
     u_char *master_key, int has_oldkey)
 {
        memset(mppe, 0, sizeof(struct pipex_mppe));
+       mtx_init(&mppe->pxm_mtx, IPL_SOFTNET);
        if (stateless)
                mppe->stateless = 1;
        if (has_oldkey)
@@ -2238,10 +2240,6 @@ pipex_mppe_input(struct mbuf *m0, struct
        coher_cnt &= PIPEX_COHERENCY_CNT_MASK;
        pktloss = 0;
 
-       PIPEX_MPPE_DBG((session, LOG_DEBUG, "in coher_cnt=%03x %s%s",
-           mppe->coher_cnt, (flushed) ? "[flushed]" : "",
-           (encrypt) ? "[encrypt]" : ""));
-
        if (encrypt == 0) {
                pipex_session_log(session, LOG_DEBUG,
                    "Received unexpected MPPE packet.(no ecrypt)");
@@ -2251,6 +2249,12 @@ pipex_mppe_input(struct mbuf *m0, struct
        /* adjust mbuf */
        m_adj(m0, sizeof(coher_cnt));
 
+       mtx_enter(&mppe->pxm_mtx);
+
+       PIPEX_MPPE_DBG((session, LOG_DEBUG, "in coher_cnt=%03x %s%s",
+           mppe->coher_cnt, (flushed) ? "[flushed]" : "",
+           (encrypt) ? "[encrypt]" : ""));
+
        /*
         * L2TP data session may be used without sequencing, PPP frames may
         * arrive in disorder.  The 'coherency counter' of MPPE detects such
@@ -2274,6 +2278,7 @@ pipex_mppe_input(struct mbuf *m0, struct
                        pipex_session_log(session, LOG_DEBUG,
                            "Workaround the out-of-sequence PPP framing 
problem: "
                            "%d => %d", mppe->coher_cnt, coher_cnt);
+                       mtx_leave(&mppe->pxm_mtx);
                        goto drop;
                }
                rewind = 1;
@@ -2305,10 +2310,19 @@ pipex_mppe_input(struct mbuf *m0, struct
                        coher_cnt &= PIPEX_COHERENCY_CNT_MASK;
                        mppe->coher_cnt = coher_cnt;
                } else if (mppe->coher_cnt != coher_cnt) {
+                       int ccp_id;
+
+                       mtx_leave(&mppe->pxm_mtx);
+
                        /* Send CCP ResetReq */
                        PIPEX_DBG((session, LOG_DEBUG, "CCP SendResetReq"));
-                       pipex_ccp_output(session, CCP_RESETREQ,
-                           session->ccp_id++);
+
+                       mtx_enter(&session->pxs_mtx);
+                       ccp_id=session->ccp_id;
+                       session->ccp_id++;
+                       mtx_leave(&session->pxs_mtx);
+
+                       pipex_ccp_output(session, CCP_RESETREQ, ccp_id);
                        goto drop;
                }
                if ((coher_cnt & 0xff) == 0xff) {
@@ -2336,6 +2350,9 @@ pipex_mppe_input(struct mbuf *m0, struct
                mppe->coher_cnt++;
                mppe->coher_cnt &= PIPEX_COHERENCY_CNT_MASK;
        }
+
+       mtx_leave(&mppe->pxm_mtx);
+
        if (m0->m_pkthdr.len < PIPEX_PPPMINLEN)
                goto drop;
 
@@ -2387,6 +2404,8 @@ pipex_mppe_output(struct mbuf *m0, struc
        flushed = 0;
        encrypt = 1;
 
+       mtx_enter(&mppe->pxm_mtx);
+
        if (mppe->stateless != 0) {
                flushed = 1;
                mppe_key_change(mppe);
@@ -2429,6 +2448,8 @@ pipex_mppe_output(struct mbuf *m0, struc
                pipex_mppe_crypt(mppe, len, cp, cp);
        }
 
+       mtx_leave(&mppe->pxm_mtx);
+
        pipex_ppp_output(m0, session, PPP_COMP);
 
        return;
@@ -2455,7 +2476,9 @@ pipex_ccp_input(struct mbuf *m0, struct 
        switch (code) {
        case CCP_RESETREQ:
                PIPEX_DBG((session, LOG_DEBUG, "CCP RecvResetReq"));
+               mtx_enter(&session->mppe_send.pxm_mtx);
                session->mppe_send.resetreq = 1;
+               mtx_leave(&session->mppe_send.pxm_mtx);
 #ifndef PIPEX_NO_CCP_RESETACK
                PIPEX_DBG((session, LOG_DEBUG, "CCP SendResetAck"));
                pipex_ccp_output(session, CCP_RESETACK, id);
Index: sys/net/pipex_local.h
===================================================================
RCS file: /cvs/src/sys/net/pipex_local.h,v
retrieving revision 1.42
diff -u -p -r1.42 pipex_local.h
--- sys/net/pipex_local.h       20 Jul 2021 16:44:55 -0000      1.42
+++ sys/net/pipex_local.h       21 Jul 2021 22:27:47 -0000
@@ -57,22 +57,25 @@
  * Locks used to protect struct members:
  *      I       immutable after creation
  *      N       net lock
+ *      s       this pipex_session' `pxs_mtx'
+ *      m       this pipex_mppe' `pxm_mtx'
  */
 
 #ifdef PIPEX_MPPE
 /* mppe rc4 key */
 struct pipex_mppe {
+       struct mutex pxm_mtx;
        int16_t stateless:1,                    /* [I] key change mode */
-               resetreq:1,                     /* [N] */
+               resetreq:1,                     /* [m] */
                reserved:14;
        int16_t keylenbits;                     /* [I] key length */
        int16_t keylen;                         /* [I] */
-       uint16_t coher_cnt;                     /* [N] cohency counter */
-       struct  rc4_ctx rc4ctx;                 /* [N] */
-       u_char master_key[PIPEX_MPPE_KEYLEN];   /* [N] master key of MPPE */
-       u_char session_key[PIPEX_MPPE_KEYLEN];  /* [N] session key of MPPE */
+       uint16_t coher_cnt;                     /* [m] cohency counter */
+       struct  rc4_ctx rc4ctx;                 /* [m] */
+       u_char master_key[PIPEX_MPPE_KEYLEN];   /* [m] master key of MPPE */
+       u_char session_key[PIPEX_MPPE_KEYLEN];  /* [m] session key of MPPE */
        u_char (*old_session_keys)[PIPEX_MPPE_KEYLEN];
-                                               /* [N] old session keys */
+                                               /* [m] old session keys */
 };
 #endif /* PIPEX_MPPE */
 
@@ -156,6 +159,8 @@ struct pipex_session {
                                        /* [N] tree glue, and other values */
        struct radix_node       ps6_rn[2];
                                        /* [N] tree glue, and other values */
+       struct mutex pxs_mtx;
+
        LIST_ENTRY(pipex_session) session_list; /* [N] all session chain */
        LIST_ENTRY(pipex_session) state_list;   /* [N] state list chain */
        LIST_ENTRY(pipex_session) id_chain;     /* [N] id hash chain */
@@ -191,7 +196,7 @@ struct pipex_session {
 
        uint32_t        ppp_flags;              /* [I] configure flags */
 #ifdef PIPEX_MPPE
-       int ccp_id;                             /* [N] CCP packet id */
+       int ccp_id;                             /* [s] CCP packet id */
        struct pipex_mppe
            mppe_recv,                          /* MPPE context for incoming */
            mppe_send;                          /* MPPE context for outgoing */ 

Reply via email to