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

1 files 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++;