Author: kp
Date: Fri Dec 11 15:39:22 2020
New Revision: 368554
URL: https://svnweb.freebsd.org/changeset/base/368554
Log:
MFC r368020, r368025:
if: Protect V_ifnet in vnet_if_return()
When we terminate a vnet (i.e. jail) we move interfaces back to their home
vnet. We need to protect our access to the V_ifnet CK_LIST.
We could enter NET_EPOCH, but if_detach_internal() (called from if_vmove())
waits for net epoch callback completion. That's not possible from NET_EPOCH.
Instead, we take the IFNET_WLOCK, build a list of the interfaces that need to
move and, once we've released the lock, move them back to their home vnet.
We cannot hold the IFNET_WLOCK() during if_vmove(), because that results in a
LOR between ifnet_sx, in_multi_sx and iflib ctx lock.
Separate out moving the ifp into or out of V_ifnet, so we can hold the lock as
we do the list manipulation, but do not hold it as we if_vmove().
if: Fix non-VIMAGE build
if_link_ifnet() and if_unlink_ifnet() are needed even when VIMAGE is not
enabled.
Sponsored by: Modirum MDPay
Modified:
stable/12/sys/net/if.c
Directory Properties:
stable/12/ (props changed)
Modified: stable/12/sys/net/if.c
==============================================================================
--- stable/12/sys/net/if.c Fri Dec 11 14:32:42 2020 (r368553)
+++ stable/12/sys/net/if.c Fri Dec 11 15:39:22 2020 (r368554)
@@ -274,6 +274,8 @@ static int if_getgroupmembers(struct ifgroupreq *);
static void if_delgroups(struct ifnet *);
static void if_attach_internal(struct ifnet *, int, struct if_clone *);
static int if_detach_internal(struct ifnet *, int, struct if_clone **);
+static void if_link_ifnet(struct ifnet *);
+static bool if_unlink_ifnet(struct ifnet *, bool);
#ifdef VIMAGE
static void if_vmove(struct ifnet *, struct vnet *);
#endif
@@ -472,17 +474,85 @@ vnet_if_uninit(const void *unused __unused)
}
VNET_SYSUNINIT(vnet_if_uninit, SI_SUB_INIT_IF, SI_ORDER_FIRST,
vnet_if_uninit, NULL);
+#endif
static void
+if_link_ifnet(struct ifnet *ifp)
+{
+
+ IFNET_WLOCK();
+ CK_STAILQ_INSERT_TAIL(&V_ifnet, ifp, if_link);
+#ifdef VIMAGE
+ curvnet->vnet_ifcnt++;
+#endif
+ IFNET_WUNLOCK();
+}
+
+static bool
+if_unlink_ifnet(struct ifnet *ifp, bool vmove)
+{
+ struct ifnet *iter;
+ int found = 0;
+
+ IFNET_WLOCK();
+ CK_STAILQ_FOREACH(iter, &V_ifnet, if_link)
+ if (iter == ifp) {
+ CK_STAILQ_REMOVE(&V_ifnet, ifp, ifnet, if_link);
+ if (!vmove)
+ ifp->if_flags |= IFF_DYING;
+ found = 1;
+ break;
+ }
+#ifdef VIMAGE
+ curvnet->vnet_ifcnt--;
+#endif
+ IFNET_WUNLOCK();
+
+ return (found);
+}
+
+#ifdef VIMAGE
+static void
vnet_if_return(const void *unused __unused)
{
struct ifnet *ifp, *nifp;
+ struct ifnet **pending;
+ int found, i;
+ i = 0;
+
+ /*
+ * We need to protect our access to the V_ifnet tailq. Ordinarily we'd
+ * enter NET_EPOCH, but that's not possible, because if_vmove() calls
+ * if_detach_internal(), which waits for NET_EPOCH callbacks to
+ * complete. We can't do that from within NET_EPOCH.
+ *
+ * However, we can also use the IFNET_xLOCK, which is the V_ifnet
+ * read/write lock. We cannot hold the lock as we call if_vmove()
+ * though, as that presents LOR w.r.t ifnet_sx, in_multi_sx and iflib
+ * ctx lock.
+ */
+ IFNET_WLOCK();
+
+ pending = malloc(sizeof(struct ifnet *) * curvnet->vnet_ifcnt,
+ M_IFNET, M_WAITOK | M_ZERO);
+
/* Return all inherited interfaces to their parent vnets. */
CK_STAILQ_FOREACH_SAFE(ifp, &V_ifnet, if_link, nifp) {
- if (ifp->if_home_vnet != ifp->if_vnet)
- if_vmove(ifp, ifp->if_home_vnet);
+ if (ifp->if_home_vnet != ifp->if_vnet) {
+ found = if_unlink_ifnet(ifp, true);
+ MPASS(found);
+
+ pending[i++] = ifp;
+ }
}
+ IFNET_WUNLOCK();
+
+ for (int j = 0; j < i; j++) {
+ if_vmove(pending[j], pending[j]->if_home_vnet);
+ }
+
+ free(pending, M_IFNET);
}
VNET_SYSUNINIT(vnet_if_return, SI_SUB_VNET_DONE, SI_ORDER_ANY,
vnet_if_return, NULL);
@@ -890,12 +960,7 @@ if_attach_internal(struct ifnet *ifp, int vmove, struc
}
#endif
- IFNET_WLOCK();
- CK_STAILQ_INSERT_TAIL(&V_ifnet, ifp, if_link);
-#ifdef VIMAGE
- curvnet->vnet_ifcnt++;
-#endif
- IFNET_WUNLOCK();
+ if_link_ifnet(ifp);
if (domain_init_status >= 2)
if_attachdomain1(ifp);
@@ -1033,9 +1098,12 @@ if_purgemaddrs(struct ifnet *ifp)
void
if_detach(struct ifnet *ifp)
{
+ bool found;
CURVNET_SET_QUIET(ifp->if_vnet);
- if_detach_internal(ifp, 0, NULL);
+ found = if_unlink_ifnet(ifp, false);
+ if (found)
+ if_detach_internal(ifp, 0, NULL);
CURVNET_RESTORE();
}
@@ -1055,47 +1123,17 @@ if_detach_internal(struct ifnet *ifp, int vmove, struc
struct ifaddr *ifa;
int i;
struct domain *dp;
- struct ifnet *iter;
- int found = 0;
#ifdef VIMAGE
int shutdown;
shutdown = (ifp->if_vnet->vnet_state > SI_SUB_VNET &&
ifp->if_vnet->vnet_state < SI_SUB_VNET_DONE) ? 1 : 0;
#endif
- IFNET_WLOCK();
- CK_STAILQ_FOREACH(iter, &V_ifnet, if_link)
- if (iter == ifp) {
- CK_STAILQ_REMOVE(&V_ifnet, ifp, ifnet, if_link);
- if (!vmove)
- ifp->if_flags |= IFF_DYING;
- found = 1;
- break;
- }
- IFNET_WUNLOCK();
- if (!found) {
- /*
- * While we would want to panic here, we cannot
- * guarantee that the interface is indeed still on
- * the list given we don't hold locks all the way.
- */
- return (ENOENT);
-#if 0
- if (vmove)
- panic("%s: ifp=%p not on the ifnet tailq %p",
- __func__, ifp, &V_ifnet);
- else
- return; /* XXX this should panic as well? */
-#endif
- }
/*
* At this point we know the interface still was on the ifnet list
* and we removed it so we are in a stable state.
*/
-#ifdef VIMAGE
- curvnet->vnet_ifcnt--;
-#endif
epoch_wait_preempt(net_epoch_preempt);
/*
@@ -1322,6 +1360,7 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, ch
struct prison *pr;
struct ifnet *difp;
int shutdown;
+ bool found;
/* Try to find the prison within our visibility. */
sx_slock(&allprison_lock);
@@ -1358,6 +1397,9 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, ch
}
CURVNET_RESTORE();
+ found = if_unlink_ifnet(ifp, true);
+ MPASS(found);
+
/* Move the interface into the child jail/vnet. */
if_vmove(ifp, pr->pr_vnet);
@@ -1374,7 +1416,8 @@ if_vmove_reclaim(struct thread *td, char *ifname, int
struct prison *pr;
struct vnet *vnet_dst;
struct ifnet *ifp;
- int shutdown;
+ int shutdown;
+ bool found;
/* Try to find the prison within our visibility. */
sx_slock(&allprison_lock);
@@ -1412,6 +1455,8 @@ if_vmove_reclaim(struct thread *td, char *ifname, int
}
/* Get interface back from child jail/vnet. */
+ found = if_unlink_ifnet(ifp, true);
+ MPASS(found);
if_vmove(ifp, vnet_dst);
CURVNET_RESTORE();
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
