git: 5a894ac4311e - stable/13 - if_vmove: improve restoration in cloner's ifgroup membership

From: Zhenlei Huang <zlei_at_FreeBSD.org>
Date: Thu, 10 Oct 2024 10:03:23 UTC
The branch stable/13 has been updated by zlei:

URL: https://cgit.FreeBSD.org/src/commit/?id=5a894ac4311e8c00489be779756b5e03c6e82570

commit 5a894ac4311e8c00489be779756b5e03c6e82570
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2022-01-25 05:06:59 +0000
Commit:     Zhenlei Huang <zlei@FreeBSD.org>
CommitDate: 2024-10-10 10:00:47 +0000

    if_vmove: improve restoration in cloner's ifgroup membership
    
    * Do a single call into if_clone.c instead of two.  The cloner
      can't disappear since the interface sits on its list.
    * Make restoration smarter - check that cloner with same name
      exists in the new vnet.
    
    Differential revision:  https://reviews.freebsd.org/D33941
    
    (cherry picked from commit 54712fc42350acb4991d8bd8604b08562660e962)
---
 sys/net/if.c       | 34 +++++++++++++---------------------
 sys/net/if_clone.c | 47 +++++++++++++++++++++++------------------------
 sys/net/if_clone.h |  3 +--
 3 files changed, 37 insertions(+), 47 deletions(-)

diff --git a/sys/net/if.c b/sys/net/if.c
index 4182f22c848e..d2b47a749873 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -277,8 +277,8 @@ static void	do_link_state_change(void *, int);
 static int	if_getgroup(struct ifgroupreq *, struct ifnet *);
 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_attach_internal(struct ifnet *, bool);
+static int	if_detach_internal(struct ifnet *, bool);
 static void	if_siocaddmulti(void *, int);
 static void	if_link_ifnet(struct ifnet *);
 static bool	if_unlink_ifnet(struct ifnet *, bool);
@@ -797,7 +797,7 @@ void
 if_attach(struct ifnet *ifp)
 {
 
-	if_attach_internal(ifp, 0, NULL);
+	if_attach_internal(ifp, false);
 }
 
 /*
@@ -852,7 +852,7 @@ if_hw_tsomax_update(if_t ifp, struct ifnet_hw_tsomax *pmax)
 }
 
 static void
-if_attach_internal(struct ifnet *ifp, int vmove, struct if_clone *ifc)
+if_attach_internal(struct ifnet *ifp, bool vmove)
 {
 	unsigned socksize, ifasize;
 	int namelen, masklen;
@@ -871,9 +871,11 @@ if_attach_internal(struct ifnet *ifp, int vmove, struct if_clone *ifc)
 
 	if_addgroup(ifp, IFG_ALL);
 
-	/* Restore group membership for cloned interfaces. */
-	if (vmove && ifc != NULL)
-		if_clone_addgroup(ifp, ifc);
+#ifdef VIMAGE
+	/* Restore group membership for cloned interface. */
+	if (vmove)
+		if_clone_restoregroup(ifp);
+#endif
 
 	getmicrotime(&ifp->if_lastchange);
 	ifp->if_epoch = time_uptime;
@@ -1118,7 +1120,7 @@ if_detach(struct ifnet *ifp)
 	found = if_unlink_ifnet(ifp, false);
 	if (found) {
 		sx_xlock(&ifnet_detach_sxlock);
-		if_detach_internal(ifp, 0, NULL);
+		if_detach_internal(ifp, false);
 		sx_xunlock(&ifnet_detach_sxlock);
 	}
 	CURVNET_RESTORE();
@@ -1135,7 +1137,7 @@ if_detach(struct ifnet *ifp)
  * the cloned interfaces are destoyed as first thing of teardown.
  */
 static int
-if_detach_internal(struct ifnet *ifp, int vmove, struct if_clone **ifcp)
+if_detach_internal(struct ifnet *ifp, bool vmove)
 {
 	struct ifaddr *ifa;
 	int i;
@@ -1170,15 +1172,6 @@ if_detach_internal(struct ifnet *ifp, int vmove, struct if_clone **ifcp)
 	taskqueue_drain(taskqueue_swi, &ifp->if_linktask);
 	taskqueue_drain(taskqueue_swi, &ifp->if_addmultitask);
 
-	/*
-	 * Check if this is a cloned interface or not. Must do even if
-	 * shutting down as a if_vmove_reclaim() would move the ifp and
-	 * the if_clone_addgroup() will have a corrupted string overwise
-	 * from a gibberish pointer.
-	 */
-	if (vmove && ifcp != NULL)
-		*ifcp = if_clone_findifc(ifp);
-
 	if_down(ifp);
 
 #ifdef VIMAGE
@@ -1293,7 +1286,6 @@ finish_vnet_shutdown:
 static int
 if_vmove(struct ifnet *ifp, struct vnet *new_vnet)
 {
-	struct if_clone *ifc;
 	void *old;
 	int rc;
 
@@ -1302,7 +1294,7 @@ if_vmove(struct ifnet *ifp, struct vnet *new_vnet)
 	 * mark as dead etc. so that the ifnet can be reattached later.
 	 * If we cannot find it, we lost the race to someone else.
 	 */
-	rc = if_detach_internal(ifp, 1, &ifc);
+	rc = if_detach_internal(ifp, true);
 	if (rc != 0)
 		return (rc);
 
@@ -1340,7 +1332,7 @@ if_vmove(struct ifnet *ifp, struct vnet *new_vnet)
 	ifnet_setbyindex(ifp->if_index, ifp);
 	IFNET_WUNLOCK();
 
-	if_attach_internal(ifp, 1, ifc);
+	if_attach_internal(ifp, true);
 
 	CURVNET_RESTORE();
 	return (0);
diff --git a/sys/net/if_clone.c b/sys/net/if_clone.c
index 9b067976b182..803d9b1f307a 100644
--- a/sys/net/if_clone.c
+++ b/sys/net/if_clone.c
@@ -621,49 +621,48 @@ done:
 	return (err);
 }
 
+#ifdef VIMAGE
 /*
- * if_clone_findifc() looks up ifnet from the current
- * cloner list, and returns ifc if found.  Note that ifc_refcnt
- * is incremented.
+ * if_clone_restoregroup() is used in context of if_vmove().
+ *
+ * Since if_detach_internal() has removed the interface from ALL groups, we
+ * need to "restore" interface membership in the cloner's group.  Note that
+ * interface belongs to cloner in its home vnet, so we first find the original
+ * cloner, and then we confirm that cloner with the same name exists in the
+ * current vnet.
  */
-struct if_clone *
-if_clone_findifc(struct ifnet *ifp)
+void
+if_clone_restoregroup(struct ifnet *ifp)
 {
-	struct if_clone *ifc, *ifc0;
+	struct if_clone *ifc;
 	struct ifnet *ifcifp;
+	char ifc_name[IFCLOSIZ] = { [0] = '\0' };
 
-	ifc0 = NULL;
+	CURVNET_SET_QUIET(ifp->if_home_vnet);
 	IF_CLONERS_LOCK();
 	LIST_FOREACH(ifc, &V_if_cloners, ifc_list) {
 		IF_CLONE_LOCK(ifc);
 		LIST_FOREACH(ifcifp, &ifc->ifc_iflist, if_clones) {
 			if (ifp == ifcifp) {
-				ifc0 = ifc;
-				IF_CLONE_ADDREF_LOCKED(ifc);
+				strncpy(ifc_name, ifc->ifc_name, IFCLOSIZ-1);
 				break;
 			}
 		}
 		IF_CLONE_UNLOCK(ifc);
-		if (ifc0 != NULL)
+		if (ifc_name[0] != '\0')
 			break;
 	}
+	CURVNET_RESTORE();
+	LIST_FOREACH(ifc, &V_if_cloners, ifc_list)
+		if (strcmp(ifc->ifc_name, ifc_name) == 0 &&
+		    ((ifc->ifc_flags & IFC_NOGROUP) == 0))
+			break;
 	IF_CLONERS_UNLOCK();
 
-	return (ifc0);
-}
-
-/*
- * if_clone_addgroup() decrements ifc_refcnt because it is called after
- * if_clone_findifc().
- */
-void
-if_clone_addgroup(struct ifnet *ifp, struct if_clone *ifc)
-{
-	if ((ifc->ifc_flags & IFC_NOGROUP) == 0) {
-		if_addgroup(ifp, ifc->ifc_name);
-		IF_CLONE_REMREF(ifc);
-	}
+	if (ifc != NULL)
+		if_addgroup(ifp, ifc_name);
 }
+#endif
 
 /*
  * A utility function to extract unit numbers from interface names of
diff --git a/sys/net/if_clone.h b/sys/net/if_clone.h
index ad1689c61d3b..187f2518f057 100644
--- a/sys/net/if_clone.h
+++ b/sys/net/if_clone.h
@@ -114,8 +114,7 @@ void	vnet_if_clone_init(void);
 int	if_clone_create(char *, size_t, caddr_t);
 int	if_clone_destroy(const char *);
 int	if_clone_list(struct if_clonereq *);
-struct if_clone *if_clone_findifc(struct ifnet *);
-void	if_clone_addgroup(struct ifnet *, struct if_clone *);
+void	if_clone_restoregroup(struct ifnet *);
 
 /* The below interfaces are used only by epair(4). */
 void	if_clone_addif(struct if_clone *, struct ifnet *);