git: 89042ff77668 - main - ng_l2tp: improve callout locking.
Gleb Smirnoff
glebius at FreeBSD.org
Fri Sep 10 18:28:02 UTC 2021
The branch main has been updated by glebius:
URL: https://cgit.FreeBSD.org/src/commit/?id=89042ff77668555e77c88549e6ba697088ee72f9
commit 89042ff77668555e77c88549e6ba697088ee72f9
Author: Gleb Smirnoff <glebius at FreeBSD.org>
AuthorDate: 2021-08-06 23:04:31 +0000
Commit: Gleb Smirnoff <glebius at 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++;
More information about the dev-commits-src-main
mailing list