Author: rwatson
Date: Fri May 3 20:38:43 2019
New Revision: 347058
URL: https://svnweb.freebsd.org/changeset/base/347058
Log:
When MAC is enabled and a policy module is loaded, don't unconditionally
lock mac_ifnet_mtx, which protects labels on struct ifnet, unless at least
one policy is actively using labels on ifnets. This avoids a global mutex
acquire in certain fast paths -- most noticeably ifnet transmit. This was
previously invisible by default, as no MAC policies were loaded by default,
but recently became visible due to mac_ntpd being enabled by default.
gallatin@ reports a reduction in PPS overhead from 300% to 2.2% with this
change. We will want to explore further MAC Framework optimisation to
reduce overhead further, but this brings things more back into the world
of the sane.
MFC after: 3 days
Modified:
head/sys/security/mac/mac_inet.c
head/sys/security/mac/mac_internal.h
head/sys/security/mac/mac_net.c
Modified: head/sys/security/mac/mac_inet.c
==============================================================================
--- head/sys/security/mac/mac_inet.c Fri May 3 20:05:31 2019
(r347057)
+++ head/sys/security/mac/mac_inet.c Fri May 3 20:38:43 2019
(r347058)
@@ -1,5 +1,5 @@
/*-
- * Copyright (c) 1999-2002, 2007, 2009 Robert N. M. Watson
+ * Copyright (c) 1999-2002, 2007, 2009, 2019 Robert N. M. Watson
* Copyright (c) 2001 Ilmar S. Habibulin
* Copyright (c) 2001-2004 Networks Associates Technology, Inc.
* Copyright (c) 2006 SPARTA, Inc.
@@ -266,16 +266,17 @@ void
mac_netinet_arp_send(struct ifnet *ifp, struct mbuf *m)
{
struct label *mlabel;
+ int locked;
if (mac_policy_count == 0)
return;
mlabel = mac_mbuf_to_label(m);
- MAC_IFNET_LOCK(ifp);
+ MAC_IFNET_LOCK(ifp, locked);
MAC_POLICY_PERFORM_NOSLEEP(netinet_arp_send, ifp, ifp->if_label, m,
mlabel);
- MAC_IFNET_UNLOCK(ifp);
+ MAC_IFNET_UNLOCK(ifp, locked);
}
void
@@ -310,16 +311,17 @@ void
mac_netinet_igmp_send(struct ifnet *ifp, struct mbuf *m)
{
struct label *mlabel;
+ int locked;
if (mac_policy_count == 0)
return;
mlabel = mac_mbuf_to_label(m);
- MAC_IFNET_LOCK(ifp);
+ MAC_IFNET_LOCK(ifp, locked);
MAC_POLICY_PERFORM_NOSLEEP(netinet_igmp_send, ifp, ifp->if_label, m,
mlabel);
- MAC_IFNET_UNLOCK(ifp);
+ MAC_IFNET_UNLOCK(ifp, locked);
}
void
Modified: head/sys/security/mac/mac_internal.h
==============================================================================
--- head/sys/security/mac/mac_internal.h Fri May 3 20:05:31 2019
(r347057)
+++ head/sys/security/mac/mac_internal.h Fri May 3 20:38:43 2019
(r347058)
@@ -1,5 +1,5 @@
/*-
- * Copyright (c) 1999-2002, 2006, 2009 Robert N. M. Watson
+ * Copyright (c) 1999-2002, 2006, 2009, 2019 Robert N. M. Watson
* Copyright (c) 2001 Ilmar S. Habibulin
* Copyright (c) 2001-2004 Networks Associates Technology, Inc.
* Copyright (c) 2006 nCircle Network Security, Inc.
@@ -216,8 +216,24 @@ void mac_destroy_label(struct label *label);
int mac_check_structmac_consistent(struct mac *mac);
int mac_allocate_slot(void);
-#define MAC_IFNET_LOCK(ifp) mtx_lock(&mac_ifnet_mtx)
-#define MAC_IFNET_UNLOCK(ifp) mtx_unlock(&mac_ifnet_mtx)
+/*
+ * Lock ifnets to protect labels only if ifnet labels are in use.
+ */
+#define MAC_IFNET_LOCK(ifp, locked) do { \
+ if (mac_labeled & MPC_OBJECT_IFNET) { \
+ mtx_lock(&mac_ifnet_mtx); \
+ locked = 1; \
+ } else { \
+ locked = 0; \
+ } \
+} while (0)
+
+#define MAC_IFNET_UNLOCK(ifp, locked) do { \
+ if (locked) { \
+ mtx_unlock(&mac_ifnet_mtx); \
+ locked = 0; \
+ } \
+} while (0)
/*
* MAC Framework per-object type functions. It's not yet clear how the
Modified: head/sys/security/mac/mac_net.c
==============================================================================
--- head/sys/security/mac/mac_net.c Fri May 3 20:05:31 2019
(r347057)
+++ head/sys/security/mac/mac_net.c Fri May 3 20:38:43 2019
(r347058)
@@ -1,5 +1,5 @@
/*-
- * Copyright (c) 1999-2002, 2009 Robert N. M. Watson
+ * Copyright (c) 1999-2002, 2009, 2019 Robert N. M. Watson
* Copyright (c) 2001 Ilmar S. Habibulin
* Copyright (c) 2001-2004 Networks Associates Technology, Inc.
* Copyright (c) 2006 SPARTA, Inc.
@@ -77,6 +77,11 @@ __FBSDID("$FreeBSD$");
* XXXRW: struct ifnet locking is incomplete in the network code, so we use
* our own global mutex for struct ifnet. Non-ideal, but should help in the
* SMP environment.
+ *
+ * This lock is acquired only if a loaded policy is using ifnet labeling.
+ * This should not ever change during a MAC policy check, itself, but could
+ * change during setup/return from a check, so we have to condition unlock on
+ * previous lock.
*/
struct mtx mac_ifnet_mtx;
MTX_SYSINIT(mac_ifnet_mtx, &mac_ifnet_mtx, "mac_ifnet", MTX_DEF);
@@ -297,13 +302,14 @@ mac_ifnet_internalize_label(struct label *label, char
void
mac_ifnet_create(struct ifnet *ifp)
{
+ int locked;
if (mac_policy_count == 0)
return;
- MAC_IFNET_LOCK(ifp);
+ MAC_IFNET_LOCK(ifp, locked);
MAC_POLICY_PERFORM_NOSLEEP(ifnet_create, ifp, ifp->if_label);
- MAC_IFNET_UNLOCK(ifp);
+ MAC_IFNET_UNLOCK(ifp, locked);
}
void
@@ -334,16 +340,17 @@ void
mac_ifnet_create_mbuf(struct ifnet *ifp, struct mbuf *m)
{
struct label *label;
+ int locked;
if (mac_policy_count == 0)
return;
label = mac_mbuf_to_label(m);
- MAC_IFNET_LOCK(ifp);
+ MAC_IFNET_LOCK(ifp, locked);
MAC_POLICY_PERFORM_NOSLEEP(ifnet_create_mbuf, ifp, ifp->if_label, m,
label);
- MAC_IFNET_UNLOCK(ifp);
+ MAC_IFNET_UNLOCK(ifp, locked);
}
MAC_CHECK_PROBE_DEFINE2(bpfdesc_check_receive, "struct bpf_d *",
@@ -352,7 +359,7 @@ MAC_CHECK_PROBE_DEFINE2(bpfdesc_check_receive, "struct
int
mac_bpfdesc_check_receive(struct bpf_d *d, struct ifnet *ifp)
{
- int error;
+ int error, locked;
/* Assume reader lock is enough. */
BPFD_LOCK_ASSERT(d);
@@ -360,11 +367,11 @@ mac_bpfdesc_check_receive(struct bpf_d *d, struct ifne
if (mac_policy_count == 0)
return (0);
- MAC_IFNET_LOCK(ifp);
+ MAC_IFNET_LOCK(ifp, locked);
MAC_POLICY_CHECK_NOSLEEP(bpfdesc_check_receive, d, d->bd_label, ifp,
ifp->if_label);
MAC_CHECK_PROBE2(bpfdesc_check_receive, error, d, ifp);
- MAC_IFNET_UNLOCK(ifp);
+ MAC_IFNET_UNLOCK(ifp, locked);
return (error);
}
@@ -376,7 +383,7 @@ int
mac_ifnet_check_transmit(struct ifnet *ifp, struct mbuf *m)
{
struct label *label;
- int error;
+ int error, locked;
M_ASSERTPKTHDR(m);
@@ -385,11 +392,11 @@ mac_ifnet_check_transmit(struct ifnet *ifp, struct mbu
label = mac_mbuf_to_label(m);
- MAC_IFNET_LOCK(ifp);
+ MAC_IFNET_LOCK(ifp, locked);
MAC_POLICY_CHECK_NOSLEEP(ifnet_check_transmit, ifp, ifp->if_label, m,
label);
MAC_CHECK_PROBE2(ifnet_check_transmit, error, ifp, m);
- MAC_IFNET_UNLOCK(ifp);
+ MAC_IFNET_UNLOCK(ifp, locked);
return (error);
}
@@ -401,7 +408,7 @@ mac_ifnet_ioctl_get(struct ucred *cred, struct ifreq *
char *elements, *buffer;
struct label *intlabel;
struct mac mac;
- int error;
+ int error, locked;
if (!(mac_labeled & MPC_OBJECT_IFNET))
return (EINVAL);
@@ -423,9 +430,9 @@ mac_ifnet_ioctl_get(struct ucred *cred, struct ifreq *
buffer = malloc(mac.m_buflen, M_MACTEMP, M_WAITOK | M_ZERO);
intlabel = mac_ifnet_label_alloc();
- MAC_IFNET_LOCK(ifp);
+ MAC_IFNET_LOCK(ifp, locked);
mac_ifnet_copy_label(ifp->if_label, intlabel);
- MAC_IFNET_UNLOCK(ifp);
+ MAC_IFNET_UNLOCK(ifp, locked);
error = mac_ifnet_externalize_label(intlabel, elements, buffer,
mac.m_buflen);
mac_ifnet_label_free(intlabel);
@@ -444,7 +451,7 @@ mac_ifnet_ioctl_set(struct ucred *cred, struct ifreq *
struct label *intlabel;
struct mac mac;
char *buffer;
- int error;
+ int error, locked;
if (!(mac_labeled & MPC_OBJECT_IFNET))
return (EINVAL);
@@ -483,18 +490,18 @@ mac_ifnet_ioctl_set(struct ucred *cred, struct ifreq *
return (error);
}
- MAC_IFNET_LOCK(ifp);
+ MAC_IFNET_LOCK(ifp, locked);
MAC_POLICY_CHECK_NOSLEEP(ifnet_check_relabel, cred, ifp,
ifp->if_label, intlabel);
if (error) {
- MAC_IFNET_UNLOCK(ifp);
+ MAC_IFNET_UNLOCK(ifp, locked);
mac_ifnet_label_free(intlabel);
return (error);
}
MAC_POLICY_PERFORM_NOSLEEP(ifnet_relabel, cred, ifp, ifp->if_label,
intlabel);
- MAC_IFNET_UNLOCK(ifp);
+ MAC_IFNET_UNLOCK(ifp, locked);
mac_ifnet_label_free(intlabel);
return (0);
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
