The branch main has been updated by glebius:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=89042ff77668555e77c88549e6ba697088ee72f9

commit 89042ff77668555e77c88549e6ba697088ee72f9
Author:     Gleb Smirnoff <gleb...@freebsd.org>
AuthorDate: 2021-08-06 23:04:31 +0000
Commit:     Gleb Smirnoff <gleb...@freebsd.org>
CommitDate: 2021-09-10 18:27:19 +0000

    ng_l2tp: improve callout locking.
    
    Apparently e62e4b85942 wasn't enough to close the race between
    a queue being flushed by a packet and callout executing, because
    the callouts used without a lock aren't 100% bulletproof. To close
    the race use callout_init_mtx() for L2TP timers, and make sure that
    all calls to ng_callout()/ng_uncallout() are done under the seq lock.
    
    If used properly, a locked callout can be used transparently with
    old netgraph KPI of ng_callout/ng_uncallout which predates locked
    callouts.
    
    While here, utilize ng_uncallout_drain() instead of ng_uncallout()
    on the node shutdown.
    
    PR:                     241133
    Reviewed by:            mjg, markj
    Differential Revision:  https://reviews.freebsd.org/D31476
---
 sys/netgraph/ng_l2tp.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/sys/netgraph/ng_l2tp.c b/sys/netgraph/ng_l2tp.c
index c62bde0c54f7..916f5db6c2ec 100644
--- a/sys/netgraph/ng_l2tp.c
+++ b/sys/netgraph/ng_l2tp.c
@@ -662,8 +662,8 @@ ng_l2tp_shutdown(node_p node)
        SEQ_UNLOCK(seq);
 
        /* Free private data if neither timer is running */
-       ng_uncallout(&seq->rack_timer, node);
-       ng_uncallout(&seq->xack_timer, node);
+       ng_uncallout_drain(&seq->rack_timer, node);
+       ng_uncallout_drain(&seq->xack_timer, node);
 
        mtx_destroy(&seq->mtx);
 
@@ -1192,9 +1192,9 @@ ng_l2tp_seq_init(priv_p priv)
        if (seq->wmax > L2TP_MAX_XWIN)
                seq->wmax = L2TP_MAX_XWIN;
        seq->ssth = seq->wmax;
-       ng_callout_init(&seq->rack_timer);
-       ng_callout_init(&seq->xack_timer);
        mtx_init(&seq->mtx, "ng_l2tp", NULL, MTX_DEF);
+       callout_init_mtx(&seq->rack_timer, &seq->mtx, CALLOUT_RETURNUNLOCKED);
+       callout_init_mtx(&seq->xack_timer, &seq->mtx, CALLOUT_RETURNUNLOCKED);
 }
 
 /*
@@ -1408,12 +1408,9 @@ ng_l2tp_seq_xack_timeout(node_p node, hook_p hook, void 
*arg1, int arg2)
        const priv_p priv = NG_NODE_PRIVATE(node);
        struct l2tp_seq *const seq = &priv->seq;
 
-       /* Make sure callout is still active before doing anything */
-       if (callout_pending(&seq->xack_timer) ||
-           (!callout_active(&seq->xack_timer)))
-               return;
-
-       SEQ_LOCK(seq);
+       SEQ_LOCK_ASSERT(seq);
+       MPASS(!callout_pending(&seq->xack_timer));
+       MPASS(callout_active(&seq->xack_timer));
 
        /* Send a ZLB */
        ng_l2tp_xmit_ctrl(priv, NULL, seq->ns);
@@ -1434,14 +1431,10 @@ ng_l2tp_seq_rack_timeout(node_p node, hook_p hook, void 
*arg1, int arg2)
        struct mbuf *m;
        u_int delay;
 
-       SEQ_LOCK(seq);
-
-       /* Make sure callout is still active before doing anything */
-       if (callout_pending(&seq->rack_timer) ||
-           !callout_active(&seq->rack_timer)) {
-               SEQ_UNLOCK(seq);
-               return;
-       }
+       SEQ_LOCK_ASSERT(seq);
+       MPASS(seq->xwin[0]);
+       MPASS(!callout_pending(&seq->rack_timer));
+       MPASS(callout_active(&seq->rack_timer));
 
        priv->stats.xmitRetransmits++;
 
_______________________________________________
dev-commits-src-main@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main
To unsubscribe, send any mail to "dev-commits-src-main-unsubscr...@freebsd.org"

Reply via email to