d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

ng_l2tp: improve callout locking.

Browse Source 
Apparently e62e4b8594 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
This commit is contained in:
Gleb Smirnoff 2021-08-06 16:04:31 -07:00
parent 0a76c63dd4
commit 89042ff776
1 changed files with 11 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
sys/netgraph/ng_l2tp.c
View File

@ -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++;