This patch improves fine-grained locking for the ng_ppp node.

Till now node's transmit path was completely unprotected
and so wasn't thread safe in multilink mode. It's receive path was
declared as WRITER as the simpliest protection method but it
reduces performance when compression or encryption enabled.

Approved by:	re (rwatson), glebius (mentor)
This commit is contained in:
Alexander Motin 2007-08-01 20:38:37 +00:00
parent 1b2d5599a5
commit e89c150775

View File

@ -221,6 +221,8 @@ struct ng_ppp_private {
frags;
int qlen; /* fraq queue length */
struct callout fragTimer; /* fraq queue check */
struct mtx rmtx; /* recv mutex */
struct mtx xmtx; /* xmit mutex */
};
typedef struct ng_ppp_private *priv_p;
@ -299,7 +301,7 @@ static int ng_ppp_mp_xmit(node_p node, item_p item, uint16_t proto);
static int ng_ppp_mp_recv(node_p node, item_p item, uint16_t proto,
uint16_t linkNum);
static int ng_ppp_link_xmit(node_p node, item_p item, uint16_t proto,
uint16_t linkNum);
uint16_t linkNum, int plen);
static int ng_ppp_bypass(node_p node, item_p item, uint16_t proto,
uint16_t linkNum);
@ -476,6 +478,9 @@ ng_ppp_constructor(node_p node)
priv->links[i].seq = MP_NOSEQ;
ng_callout_init(&priv->fragTimer);
mtx_init(&priv->rmtx, "ng_ppp_recv", NULL, MTX_DEF);
mtx_init(&priv->xmtx, "ng_ppp_xmit", NULL, MTX_DEF);
/* Done */
return (0);
}
@ -515,9 +520,6 @@ ng_ppp_newhook(node_p node, hook_p hook, const char *name)
!priv->conf.enableMultilink && priv->numActiveLinks >= 1)
return (ENODEV);
/* MP recv code is not thread-safe. */
NG_HOOK_FORCE_WRITER(hook);
} else { /* must be a non-link hook */
int i;
@ -684,6 +686,8 @@ ng_ppp_shutdown(node_p node)
/* Take down netgraph node */
ng_ppp_frag_reset(node);
mtx_destroy(&priv->rmtx);
mtx_destroy(&priv->xmtx);
bzero(priv, sizeof(*priv));
FREE(priv, M_NETGRAPH_PPP);
NG_NODE_SET_PRIVATE(node, NULL);
@ -812,7 +816,7 @@ ng_ppp_rcvdata_bypass(hook_p hook, item_p item)
return (ng_ppp_hcomp_xmit(NG_HOOK_NODE(hook), item, proto));
else
return (ng_ppp_link_xmit(NG_HOOK_NODE(hook), item, proto,
linkNum));
linkNum, 0));
}
static int
@ -1177,10 +1181,6 @@ ng_ppp_crypt_recv(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
{
const priv_p priv = NG_NODE_PRIVATE(node);
/* Stats */
priv->bundleStats.recvFrames++;
priv->bundleStats.recvOctets += NGI_M(item)->m_pkthdr.len;
if (proto == PROT_CRYPTD) {
if (priv->conf.enableDecryption &&
priv->hooks[HOOK_INDEX_DECRYPT] != NULL) {
@ -1234,7 +1234,7 @@ ng_ppp_rcvdata_decrypt(hook_p hook, item_p item)
*/
static int
ng_ppp_link_xmit(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
ng_ppp_link_xmit(node_p node, item_p item, uint16_t proto, uint16_t linkNum, int plen)
{
const priv_p priv = NG_NODE_PRIVATE(node);
struct ng_ppp_link *link;
@ -1244,8 +1244,7 @@ ng_ppp_link_xmit(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
/* Check if link correct. */
if (linkNum >= NG_PPP_MAX_LINKS) {
NG_FREE_ITEM(item);
return (ENETDOWN);
ERROUT(ENETDOWN);
}
/* Get link pointer (optimization). */
@ -1253,8 +1252,7 @@ ng_ppp_link_xmit(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
/* Check link status (if real). */
if (link->hook == NULL) {
NG_FREE_ITEM(item);
return (ENETDOWN);
ERROUT(ENETDOWN);
}
/* Extract mbuf. */
@ -1264,34 +1262,39 @@ ng_ppp_link_xmit(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
mru = link->conf.mru;
if (mru != 0 && m->m_pkthdr.len > mru) {
NG_FREE_M(m);
NG_FREE_ITEM(item);
return (EMSGSIZE);
ERROUT(EMSGSIZE);
}
/* Prepend protocol number, possibly compressed. */
if ((m = ng_ppp_addproto(m, proto, link->conf.enableProtoComp)) ==
NULL) {
NG_FREE_ITEM(item);
return (ENOBUFS);
ERROUT(ENOBUFS);
}
/* Prepend address and control field (unless compressed). */
if (proto == PROT_LCP || !link->conf.enableACFComp) {
if ((m = ng_ppp_prepend(m, &ng_ppp_acf, 2)) == NULL) {
NG_FREE_ITEM(item);
return (ENOBUFS);
}
if ((m = ng_ppp_prepend(m, &ng_ppp_acf, 2)) == NULL)
ERROUT(ENOBUFS);
}
/* Deliver frame. */
len = m->m_pkthdr.len;
NG_FWD_NEW_DATA(error, item, link->hook, m);
/* Update stats and 'bytes in queue' counter. */
if (error == 0) {
link->stats.xmitFrames++;
link->stats.xmitOctets += len;
mtx_lock(&priv->xmtx);
/* Update link stats. */
link->stats.xmitFrames++;
link->stats.xmitOctets += len;
/* Update bundle stats. */
if (plen > 0) {
priv->bundleStats.xmitFrames++;
priv->bundleStats.xmitOctets += plen;
}
/* Update 'bytes in queue' counter. */
if (error == 0) {
/* bytesInQueue and lastWrite required only for mp_strategy. */
if (priv->conf.enableMultilink && !priv->allLinksEqual &&
!priv->conf.enableRoundRobin) {
@ -1305,6 +1308,11 @@ ng_ppp_link_xmit(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
link->bytesInQueue = 50 * 1600;
}
}
mtx_unlock(&priv->xmtx);
return (error);
done:
NG_FREE_ITEM(item);
return (error);
}
@ -1321,47 +1329,54 @@ ng_ppp_rcvdata(hook_p hook, item_p item)
struct ng_ppp_link * const link = &priv->links[linkNum];
uint16_t proto;
struct mbuf *m;
int error = 0;
KASSERT(linkNum < NG_PPP_MAX_LINKS,
("%s: bogus index 0x%x", __func__, index));
NGI_GET_M(item, m);
mtx_lock(&priv->rmtx);
/* Stats */
link->stats.recvFrames++;
link->stats.recvOctets += m->m_pkthdr.len;
/* Strip address and control fields, if present. */
if (m->m_len < 2 && (m = m_pullup(m, 2)) == NULL) {
NG_FREE_ITEM(item);
return (ENOBUFS);
}
if (m->m_len < 2 && (m = m_pullup(m, 2)) == NULL)
ERROUT(ENOBUFS);
if (bcmp(mtod(m, uint8_t *), &ng_ppp_acf, 2) == 0)
m_adj(m, 2);
if ((m = ng_ppp_cutproto(m, &proto)) == NULL) {
NG_FREE_ITEM(item);
return (ENOBUFS);
}
/* Get protocol number */
if ((m = ng_ppp_cutproto(m, &proto)) == NULL)
ERROUT(ENOBUFS);
NGI_M(item) = m; /* Put changed m back into item. */
if (!PROT_VALID(proto)) {
link->stats.badProtos++;
NG_FREE_ITEM(item);
return (EIO);
ERROUT(EIO);
}
/* LCP packets must go directly to bypass. */
if (proto >= 0xB000)
if (proto >= 0xB000) {
mtx_unlock(&priv->rmtx);
return (ng_ppp_bypass(node, item, proto, linkNum));
if (!link->conf.enableLink) {
/* Non-LCP packets are denied on a disabled link. */
NG_FREE_ITEM(item);
return (ENXIO);
}
/* Other packets are denied on a disabled link. */
if (!link->conf.enableLink)
ERROUT(ENXIO);
return (ng_ppp_mp_recv(node, item, proto, linkNum));
/* Proceed to multilink layer. Mutex will be unlocked inside. */
error = ng_ppp_mp_recv(node, item, proto, linkNum);
mtx_assert(&priv->rmtx, MA_NOTOWNED);
return (error);
done:
mtx_unlock(&priv->rmtx);
NG_FREE_ITEM(item);
return (error);
}
/*
@ -1429,9 +1444,16 @@ ng_ppp_mp_recv(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
struct ng_ppp_frag *qent;
int i, diff, inserted;
struct mbuf *m;
int error = 0;
if ((!priv->conf.enableMultilink) || proto != PROT_MP)
if ((!priv->conf.enableMultilink) || proto != PROT_MP) {
/* Stats */
priv->bundleStats.recvFrames++;
priv->bundleStats.recvOctets += NGI_M(item)->m_pkthdr.len;
mtx_unlock(&priv->rmtx);
return (ng_ppp_crypt_recv(node, item, proto, linkNum));
}
NGI_GET_M(item, m);
NG_FREE_ITEM(item);
@ -1443,10 +1465,10 @@ ng_ppp_mp_recv(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
if (m->m_pkthdr.len < 2) {
link->stats.runts++;
NG_FREE_M(m);
return (EINVAL);
ERROUT(EINVAL);
}
if (m->m_len < 2 && (m = m_pullup(m, 2)) == NULL)
return (ENOBUFS);
ERROUT(ENOBUFS);
shdr = ntohs(*mtod(m, uint16_t *));
frag->seq = MP_SHORT_EXTEND(shdr);
@ -1460,10 +1482,10 @@ ng_ppp_mp_recv(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
if (m->m_pkthdr.len < 4) {
link->stats.runts++;
NG_FREE_M(m);
return (EINVAL);
ERROUT(EINVAL);
}
if (m->m_len < 4 && (m = m_pullup(m, 4)) == NULL)
return (ENOBUFS);
ERROUT(ENOBUFS);
lhdr = ntohl(*mtod(m, uint32_t *));
frag->seq = MP_LONG_EXTEND(lhdr);
@ -1480,7 +1502,7 @@ ng_ppp_mp_recv(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
if (diff < 0) {
link->stats.dropFragments++;
NG_FREE_M(m);
return (0);
ERROUT(0);
}
/* Update highest received sequence number on this link and MSEQ */
@ -1497,8 +1519,7 @@ ng_ppp_mp_recv(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
MALLOC(frag, struct ng_ppp_frag *, sizeof(*frag), M_NETGRAPH_PPP, M_NOWAIT);
if (frag == NULL) {
NG_FREE_M(m);
ng_ppp_frag_process(node);
return (ENOMEM);
goto process;
}
*frag = frag0;
@ -1514,15 +1535,21 @@ ng_ppp_mp_recv(node_p node, item_p item, uint16_t proto, uint16_t linkNum)
link->stats.dupFragments++;
NG_FREE_M(frag->data);
FREE(frag, M_NETGRAPH_PPP);
return (EINVAL);
ERROUT(EINVAL);
}
}
if (!inserted)
TAILQ_INSERT_HEAD(&priv->frags, frag, f_qent);
priv->qlen++;
process:
/* Process the queue */
return ng_ppp_frag_process(node);
/* NOTE: rmtx will be unlocked for sending time! */
error = ng_ppp_frag_process(node);
done:
mtx_unlock(&priv->rmtx);
return (error);
}
/************************************************************************
@ -1729,9 +1756,20 @@ ng_ppp_frag_process(node_p node)
NG_FREE_M(m);
continue;
}
if ((item = ng_package_data(m, NG_NOFLAGS)) != NULL)
if ((item = ng_package_data(m, NG_NOFLAGS)) != NULL) {
/* Stats */
priv->bundleStats.recvFrames++;
priv->bundleStats.recvOctets +=
NGI_M(item)->m_pkthdr.len;
/* Drop mutex for the sending time.
* Priv may change, but we are ready!
*/
mtx_unlock(&priv->rmtx);
ng_ppp_crypt_recv(node, item, proto,
NG_PPP_BUNDLE_LINKNUM);
mtx_lock(&priv->rmtx);
}
}
/* Delete dead fragments and try again */
} while (ng_ppp_frag_trim(node) || ng_ppp_frag_drop(node));
@ -1824,9 +1862,14 @@ ng_ppp_frag_checkstale(node_p node)
}
/* Deliver packet */
if ((item = ng_package_data(m, NG_NOFLAGS)) != NULL)
if ((item = ng_package_data(m, NG_NOFLAGS)) != NULL) {
/* Stats */
priv->bundleStats.recvFrames++;
priv->bundleStats.recvOctets += NGI_M(item)->m_pkthdr.len;
ng_ppp_crypt_recv(node, item, proto,
NG_PPP_BUNDLE_LINKNUM);
}
}
}
@ -1860,20 +1903,23 @@ ng_ppp_mp_xmit(node_p node, item_p item, uint16_t proto)
int firstFragment;
int activeLinkNum;
struct mbuf *m;
int len;
int frags;
int32_t seq;
/* At least one link must be active */
if (priv->numActiveLinks == 0) {
NG_FREE_ITEM(item);
return (ENETDOWN);
}
/* Save length for later stats. */
len = NGI_M(item)->m_pkthdr.len;
/* Update stats. */
priv->bundleStats.xmitFrames++;
priv->bundleStats.xmitOctets += NGI_M(item)->m_pkthdr.len;
if (!priv->conf.enableMultilink)
if (!priv->conf.enableMultilink) {
return (ng_ppp_link_xmit(node, item, proto,
priv->activeLinks[0]));
priv->activeLinks[0], len));
}
/* Extract mbuf. */
NGI_GET_M(item, m);
@ -1886,6 +1932,8 @@ ng_ppp_mp_xmit(node_p node, item_p item, uint16_t proto)
/* Clear distribution plan */
bzero(&distrib, priv->numActiveLinks * sizeof(distrib[0]));
mtx_lock(&priv->xmtx);
/* Round-robin strategy */
if (priv->conf.enableRoundRobin) {
activeLinkNum = priv->lastLink++ % priv->numActiveLinks;
@ -1920,6 +1968,29 @@ ng_ppp_mp_xmit(node_p node, item_p item, uint16_t proto)
ng_ppp_mp_strategy(node, m->m_pkthdr.len, distrib);
deliver:
/* Estimate fragments count */
frags = 0;
for (activeLinkNum = priv->numActiveLinks - 1;
activeLinkNum >= 0; activeLinkNum--) {
const uint16_t linkNum = priv->activeLinks[activeLinkNum];
struct ng_ppp_link *const link = &priv->links[linkNum];
frags += (distrib[activeLinkNum] + link->conf.mru - hdr_len - 1) /
(link->conf.mru - hdr_len);
}
/* Get out initial sequence number */
seq = priv->xseq;
/* Update next sequence number */
if (priv->conf.xmitShortSeq) {
priv->xseq = (seq + frags) & MP_SHORT_SEQ_MASK;
} else {
priv->xseq = (seq + frags) & MP_LONG_SEQ_MASK;
}
mtx_unlock(&priv->xmtx);
/* Send alloted portions of frame out on the link(s) */
for (firstFragment = 1, activeLinkNum = priv->numActiveLinks - 1;
activeLinkNum >= 0; activeLinkNum--) {
@ -1955,9 +2026,8 @@ ng_ppp_mp_xmit(node_p node, item_p item, uint16_t proto)
if (priv->conf.xmitShortSeq) {
uint16_t shdr;
shdr = priv->xseq;
priv->xseq =
(priv->xseq + 1) & MP_SHORT_SEQ_MASK;
shdr = seq;
seq = (seq + 1) & MP_SHORT_SEQ_MASK;
if (firstFragment)
shdr |= MP_SHORT_FIRST_FLAG;
if (lastFragment)
@ -1967,9 +2037,8 @@ ng_ppp_mp_xmit(node_p node, item_p item, uint16_t proto)
} else {
uint32_t lhdr;
lhdr = priv->xseq;
priv->xseq =
(priv->xseq + 1) & MP_LONG_SEQ_MASK;
lhdr = seq;
seq = (seq + 1) & MP_LONG_SEQ_MASK;
if (firstFragment)
lhdr |= MP_LONG_FIRST_FLAG;
if (lastFragment)
@ -1986,7 +2055,7 @@ ng_ppp_mp_xmit(node_p node, item_p item, uint16_t proto)
/* Send fragment */
if ((item = ng_package_data(m2, NG_NOFLAGS)) != NULL) {
error = ng_ppp_link_xmit(node, item, PROT_MP,
linkNum);
linkNum, (firstFragment?len:0));
if (error != 0) {
if (!lastFragment)
NG_FREE_M(m);