Date: Fri, 15 May 2009 00:24:45 GMT From: Marko Zec <zec@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 162097 for review Message-ID: <200905150024.n4F0Ojix001733@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=162097 Change 162097 by zec@zec_tpx32 on 2009/05/15 00:23:50 First cut implementation of if_vmove(), inspired by Julian's sketches from a few days ago. if_vmove() calls if_detach1() with a flag set resulting in only a subset of common if_detach() tasks being done, i.e. vnet-agnostic state being preserved. The ifnet is then detached from ifindex_table in current vnet, and linked to the ifindex_table in target vnet. Finally, if_attach1() is called with a vmove flag set, resulting in ifnet being attached to the target vnet while preserving vnet-agnostic state such ase LLADDRs, if_* interface methods etc. While here, nuke if_reassign_common() which is no longer used. Affected files ... .. //depot/projects/vimage-commit2/src/sys/kern/kern_vimage.c#41 edit .. //depot/projects/vimage-commit2/src/sys/net/if.c#60 edit .. //depot/projects/vimage-commit2/src/sys/net/if_ethersubr.c#26 edit .. //depot/projects/vimage-commit2/src/sys/net/if_var.h#23 edit Differences ... ==== //depot/projects/vimage-commit2/src/sys/kern/kern_vimage.c#41 (text+ko) ==== @@ -115,86 +115,11 @@ } /* - * if_reassign_common() should be called by all device specific - * ifnet reassignment routines after the interface is detached from - * current vnet and before the interface gets attached to the target - * vnet. This routine attempts to shrink if_index in current vnet, - * find an unused if_index in target vnet and calls if_grow() if - * necessary, and finally finds an unused if_xname for the target - * vnet. - * - * XXX this routine should hold a lock over if_index and return with - * such a lock held, and the caller should release that lock - * after ifattach completes! - */ -void -if_reassign_common(struct ifnet *ifp, struct vnet *new_vnet, const char *dname) -{ - - /* do / while construct needed to confine scope of INIT_VNET_NET(). */ - do { - INIT_VNET_NET(curvnet); - - IFNET_WLOCK(); - ifnet_setbyindex(ifp->if_index, NULL); - while (V_if_index > 0 && \ - ifnet_byindex_locked(V_if_index) == NULL) - V_if_index--; - IFNET_WUNLOCK(); - } while (0); - - CURVNET_SET_QUIET(new_vnet); - INIT_VNET_NET(new_vnet); - /* - * Try to find an empty slot below if_index. If we fail, take - * the next slot. - */ - IFNET_WLOCK(); - for (ifp->if_index = 1; ifp->if_index <= V_if_index; ifp->if_index++) { - if (ifnet_byindex_locked(ifp->if_index) == NULL) - break; - } - /* Catch if_index overflow. */ - if (ifp->if_index < 1) - panic("vi_if_move: if_index overflow"); - - if (ifp->if_index > V_if_index) - V_if_index = ifp->if_index; - if (V_if_index >= V_if_indexlim) - if_grow(); - ifnet_setbyindex(ifp->if_index, ifp); - IFNET_WUNLOCK(); - -#ifdef NOTYET - /* Rename the ifnet */ - if (new_vnet == ifp->if_home_vnet) { - /* always restore the original name on return to home vnet */ - snprintf(ifp->if_xname, IFNAMSIZ, "%s%d", ifp->if_dname, - ifp->if_dunit); - } else { - int unit = 0; - struct ifnet *iter; - - do { - snprintf(ifp->if_xname, IFNAMSIZ, "%s%d", dname, unit); - TAILQ_FOREACH(iter, &V_ifnet, if_link) - if (strcmp(ifp->if_xname, iter->if_xname) == 0) - break; - unit++; - } while (iter); - } -#endif - CURVNET_RESTORE(); -} - -/* - * Move the interface to another vnet. The interface can be specified either + * Move an ifnet to another vnet. The ifnet can be specified either * by ifp argument, or by name contained in vi_req->vi_if_xname if NULL is - * passed as ifp. The interface will be renamed to vi_req->vi_parent_name - * if vi_req->vi_parent_name is not an empty string (uff ugly ugly)... - * Similary, the target vnet can be specified either by vnet argument or - * by name. If vnet name equals to ".." or vi_req is set to NULL the - * interface is moved to the parent vnet. + * passed as ifp. The target vnet can be specified either by vnet + * argument or by name. If vnet name equals to ".." or vi_req is set to + * NULL the interface is moved to the parent vnet. */ int vi_if_move(struct vi_req *vi_req, struct ifnet *ifp, struct vimage *vip) @@ -206,6 +131,7 @@ if (vi_req->vi_api_cookie != VI_API_COOKIE) return (EDOOFUS); + /* Find the target vnet. */ if (vi_req == NULL || strcmp(vi_req->vi_name, "..") == 0) { if (IS_DEFAULT_VIMAGE(vip)) return (ENXIO); @@ -217,15 +143,23 @@ new_vnet = new_vip->v_net; } + /* Try to find the target ifnet by name. */ if (ifp == NULL) ifp = ifunit(vi_req->vi_if_xname); + if (ifp == NULL) return (ENXIO); +#if 0 /* Abort if driver did not provide a if_reassign() method */ if (ifp->if_reassign == NULL) return (ENODEV); +#endif + /* + * Check for naming clashes in target vnet. Not locked so races + * are possible. + */ if (vi_req != NULL) { struct ifnet *t_ifp; @@ -236,11 +170,8 @@ return (EEXIST); } - if (vi_req && strlen(vi_req->vi_if_xname) > 0) - ifp->if_reassign(ifp, new_vnet, vi_req->vi_if_xname); - else - ifp->if_reassign(ifp, new_vnet, NULL); - getmicrotime(&ifp->if_lastchange); + /* Detach from curvnet and attach to new_vnet. */ + if_vmove(ifp, new_vnet); /* Report the new if_xname back to the userland */ if (vi_req != NULL) ==== //depot/projects/vimage-commit2/src/sys/net/if.c#60 (text+ko) ==== @@ -141,6 +141,8 @@ static int if_getgroup(struct ifgroupreq *, struct ifnet *); static int if_getgroupmembers(struct ifgroupreq *); static void if_delgroups(struct ifnet *); +static void if_attach1(struct ifnet *, int); +static void if_detach1(struct ifnet *, int); #ifdef INET6 /* @@ -652,7 +654,10 @@ /* * Perform generic interface initalization tasks and attach the interface - * to the list of "active" interfaces. + * to the list of "active" interfaces. If vmove flag is set on entry + * to if_attach1(), perform only a limited subset of initialization + * tasks, given that we are moving an ifnet which has already been fully + * initialized from one vnet to another. * * XXX: * - The decision to return void and thus require this function to @@ -663,6 +668,13 @@ void if_attach(struct ifnet *ifp) { + + if_attach1(ifp, 0); +} + +static void +if_attach1(struct ifnet *ifp, int vmove) +{ INIT_VNET_NET(curvnet); unsigned socksize, ifasize; int namelen, masklen; @@ -679,7 +691,7 @@ * Record in which vnet has this ifnet been attached * for the first time. */ - if (ifp->if_home_vnet == NULL) + if (ifp->if_home_vnet == NULL) ifp->if_home_vnet = curvnet; #endif @@ -697,55 +709,58 @@ ifp->if_qflush = if_qflush; } + if (!vmove) { #ifdef MAC - mac_ifnet_create(ifp); + mac_ifnet_create(ifp); #endif - if (IS_DEFAULT_VNET(curvnet)) { - ifdev_setbyindex(ifp->if_index, make_dev(&net_cdevsw, - ifp->if_index, UID_ROOT, GID_WHEEL, 0600, "%s/%s", - net_cdevsw.d_name, ifp->if_xname)); - make_dev_alias(ifdev_byindex(ifp->if_index), "%s%d", - net_cdevsw.d_name, ifp->if_index); - } + if (IS_DEFAULT_VNET(curvnet)) { + ifdev_setbyindex(ifp->if_index, make_dev(&net_cdevsw, + ifp->if_index, UID_ROOT, GID_WHEEL, 0600, "%s/%s", + net_cdevsw.d_name, ifp->if_xname)); + make_dev_alias(ifdev_byindex(ifp->if_index), "%s%d", + net_cdevsw.d_name, ifp->if_index); + } - ifq_attach(&ifp->if_snd, ifp); + ifq_attach(&ifp->if_snd, ifp); - /* - * create a Link Level name for this device - */ - namelen = strlen(ifp->if_xname); - /* - * Always save enough space for any possiable name so we can do - * a rename in place later. - */ - masklen = offsetof(struct sockaddr_dl, sdl_data[0]) + IFNAMSIZ; - socksize = masklen + ifp->if_addrlen; - if (socksize < sizeof(*sdl)) - socksize = sizeof(*sdl); - socksize = roundup2(socksize, sizeof(long)); - ifasize = sizeof(*ifa) + 2 * socksize; - ifa = malloc(ifasize, M_IFADDR, M_WAITOK | M_ZERO); - IFA_LOCK_INIT(ifa); - sdl = (struct sockaddr_dl *)(ifa + 1); - sdl->sdl_len = socksize; - sdl->sdl_family = AF_LINK; - bcopy(ifp->if_xname, sdl->sdl_data, namelen); - sdl->sdl_nlen = namelen; - sdl->sdl_index = ifp->if_index; - sdl->sdl_type = ifp->if_type; - ifp->if_addr = ifa; - ifa->ifa_ifp = ifp; - ifa->ifa_rtrequest = link_rtrequest; - ifa->ifa_addr = (struct sockaddr *)sdl; - sdl = (struct sockaddr_dl *)(socksize + (caddr_t)sdl); - ifa->ifa_netmask = (struct sockaddr *)sdl; - sdl->sdl_len = masklen; - while (namelen != 0) - sdl->sdl_data[--namelen] = 0xff; - ifa->ifa_refcnt = 1; - TAILQ_INSERT_HEAD(&ifp->if_addrhead, ifa, ifa_link); - ifp->if_broadcastaddr = NULL; /* reliably crash if used uninitialized */ + /* + * Create a Link Level name for this device. + */ + namelen = strlen(ifp->if_xname); + /* + * Always save enough space for any possiable name so we + * can do a rename in place later. + */ + masklen = offsetof(struct sockaddr_dl, sdl_data[0]) + IFNAMSIZ; + socksize = masklen + ifp->if_addrlen; + if (socksize < sizeof(*sdl)) + socksize = sizeof(*sdl); + socksize = roundup2(socksize, sizeof(long)); + ifasize = sizeof(*ifa) + 2 * socksize; + ifa = malloc(ifasize, M_IFADDR, M_WAITOK | M_ZERO); + IFA_LOCK_INIT(ifa); + sdl = (struct sockaddr_dl *)(ifa + 1); + sdl->sdl_len = socksize; + sdl->sdl_family = AF_LINK; + bcopy(ifp->if_xname, sdl->sdl_data, namelen); + sdl->sdl_nlen = namelen; + sdl->sdl_index = ifp->if_index; + sdl->sdl_type = ifp->if_type; + ifp->if_addr = ifa; + ifa->ifa_ifp = ifp; + ifa->ifa_rtrequest = link_rtrequest; + ifa->ifa_addr = (struct sockaddr *)sdl; + sdl = (struct sockaddr_dl *)(socksize + (caddr_t)sdl); + ifa->ifa_netmask = (struct sockaddr *)sdl; + sdl->sdl_len = masklen; + while (namelen != 0) + sdl->sdl_data[--namelen] = 0xff; + ifa->ifa_refcnt = 1; + TAILQ_INSERT_HEAD(&ifp->if_addrhead, ifa, ifa_link); + /* Reliably crash if used uninitialized. */ + ifp->if_broadcastaddr = NULL; + } IFNET_WLOCK(); TAILQ_INSERT_TAIL(&V_ifnet, ifp, if_link); @@ -764,7 +779,7 @@ /* Announce the interface. */ rt_ifannouncemsg(ifp, IFAN_ARRIVAL); - if (ifp->if_watchdog != NULL) { + if (!vmove && ifp->if_watchdog != NULL) { if_printf(ifp, "WARNING: using obsoleted if_watchdog interface\n"); @@ -882,8 +897,10 @@ } /* - * Detach an interface, removing it from the - * list of "active" interfaces. + * Detach an interface, removing it from the list of "active" interfaces. + * If vmove flag is set on entry to if_detach1(), perform only a limited + * subset of cleanup tasks, given that we are moving an ifnet from one + * vnet to another, where it must be fully operational. * * XXXRW: There are some significant questions about event ordering, and * how to prevent things from starting to use the interface during detach. @@ -891,10 +908,17 @@ void if_detach(struct ifnet *ifp) { + + if_detach1(ifp, 0); +} + +static void +if_detach1(struct ifnet *ifp, int vmove) +{ INIT_VNET_NET(ifp->if_vnet); struct ifaddr *ifa; struct radix_node_head *rnh; - int s, i, j; + int i, j; struct domain *dp; struct ifnet *iter; int found = 0; @@ -911,8 +935,12 @@ --curvnet->ifcnt; #endif IFNET_WUNLOCK(); - if (!found) - return; + if (!found) { + if (vmove) + panic("interface not in it's own ifnet list"); + else + return; /* XXX this should panic as well? */ + } /* * Remove/wait for pending events. @@ -922,7 +950,6 @@ /* * Remove routes and flush queues. */ - s = splnet(); if_down(ifp); #ifdef ALTQ if (ALTQ_IS_ENABLED(&ifp->if_snd)) @@ -948,29 +975,27 @@ #endif if_purgemaddrs(ifp); - /* - * Prevent further calls into the device driver via ifnet. - * XXX temporarily disabled in VIMAGE builds as we may need to - * to reattach the ifnet to another vnet. - */ -#ifndef VIMAGE - if_dead(ifp); -#endif + if (!vmove) { + /* + * Prevent further calls into the device driver via ifnet. + */ + if_dead(ifp); - /* - * Remove link ifaddr pointer and maybe decrement if_index. - * Clean up all addresses. - */ - ifp->if_addr = NULL; - if (IS_DEFAULT_VNET(curvnet)) - destroy_dev(ifdev_byindex(ifp->if_index)); - ifdev_setbyindex(ifp->if_index, NULL); + /* + * Remove link ifaddr pointer and maybe decrement if_index. + * Clean up all addresses. + */ + ifp->if_addr = NULL; + if (IS_DEFAULT_VNET(curvnet)) + destroy_dev(ifdev_byindex(ifp->if_index)); + ifdev_setbyindex(ifp->if_index, NULL); - /* We can now free link ifaddr. */ - if (!TAILQ_EMPTY(&ifp->if_addrhead)) { - ifa = TAILQ_FIRST(&ifp->if_addrhead); - TAILQ_REMOVE(&ifp->if_addrhead, ifa, ifa_link); - IFAFREE(ifa); + /* We can now free link ifaddr. */ + if (!TAILQ_EMPTY(&ifp->if_addrhead)) { + ifa = TAILQ_FIRST(&ifp->if_addrhead); + TAILQ_REMOVE(&ifp->if_addrhead, ifa, ifa_link); + IFAFREE(ifa); + } } /* @@ -1003,13 +1028,83 @@ ifp->if_afdata[dp->dom_family]); } IF_AFDATA_UNLOCK(ifp); - ifq_detach(&ifp->if_snd); + + if (!vmove) + ifq_detach(&ifp->if_snd); +} + #ifdef VIMAGE - ifp->if_vnet = NULL; - ifp->if_afdata_initialized = 0; -#endif - splx(s); +/* + * if_vmove() should be called by all device specific + * ifnet reassignment routines after the interface is detached from + * current vnet and before the interface gets attached to the target + * vnet. This routine attempts to shrink if_index in current vnet, + * find an unused if_index in target vnet and calls if_grow() if + * necessary, and finally finds an unused if_xname for the target + * vnet. + * + * XXX this routine should hold a lock over if_index and return with + * such a lock held, and the caller should release that lock + * after ifattach completes! + */ +void +if_vmove(struct ifnet *ifp, struct vnet *new_vnet) +{ + + /* + * Detach from current vnet, but preserve LLADDR info, do not + * mark as dead etc. so that we can reattach later. + */ + if_detach1(ifp, 1); + + /* + * Unlink the ifnet from ifindex_table[] in current vnet, + * and shrink the if_index for that vnet if possible. + * do / while construct below is needed to confine the scope + * of INIT_VNET_NET(). + */ + do { + INIT_VNET_NET(curvnet); + + IFNET_WLOCK(); + ifnet_setbyindex(ifp->if_index, NULL); + while (V_if_index > 0 && \ + ifnet_byindex_locked(V_if_index) == NULL) + V_if_index--; + IFNET_WUNLOCK(); + } while (0); + + /* + * Switch to the context of the target vnet. + */ + CURVNET_SET_QUIET(new_vnet); + INIT_VNET_NET(new_vnet); + + /* + * Try to find an empty slot below if_index. If we fail, take + * the next slot. + */ + IFNET_WLOCK(); + for (ifp->if_index = 1; ifp->if_index <= V_if_index; ifp->if_index++) { + if (ifnet_byindex_locked(ifp->if_index) == NULL) + break; + } + /* Catch if_index overflow. */ + if (ifp->if_index < 1) + panic("vi_if_move: if_index overflow"); + + if (ifp->if_index > V_if_index) + V_if_index = ifp->if_index; + if (V_if_index >= V_if_indexlim) + if_grow(); + ifnet_setbyindex(ifp->if_index, ifp); + IFNET_WUNLOCK(); + + if_attach1(ifp, 1); + + CURVNET_RESTORE(); } +#endif /* VIMAGE */ /* * Add a group to an interface ==== //depot/projects/vimage-commit2/src/sys/net/if_ethersubr.c#26 (text+ko) ==== @@ -924,25 +924,6 @@ return (etherbuf); } -#ifdef VIMAGE -static void -ether_reassign(struct ifnet *ifp, struct vnet *vnet, char *dname) -{ - u_char eaddr[6]; - - bcopy(IF_LLADDR(ifp), eaddr, 6); - ether_ifdetach(ifp); - ifp->if_bpf = NULL; - if_reassign_common(ifp, vnet, "eth"); - if (dname) - snprintf(ifp->if_xname, IFNAMSIZ, "%s", dname); - - CURVNET_SET_QUIET(vnet); - ether_ifattach(ifp, eaddr); - CURVNET_RESTORE(); -} -#endif - /* * Perform common duties while attaching to interface list */ @@ -952,9 +933,6 @@ int i; struct ifaddr *ifa; struct sockaddr_dl *sdl; -#ifdef VIMAGE - struct vnet *home_vnet_0 = ifp->if_home_vnet; -#endif ifp->if_addrlen = ETHER_ADDR_LEN; ifp->if_hdrlen = ETHER_HDR_LEN; @@ -963,9 +941,6 @@ ifp->if_output = ether_output; ifp->if_input = ether_input; ifp->if_resolvemulti = ether_resolvemulti; -#ifdef VIMAGE - ifp->if_reassign = ether_reassign; -#endif if (ifp->if_baudrate == 0) ifp->if_baudrate = IF_Mbps(10); /* just a default */ ifp->if_broadcastaddr = etherbroadcastaddr; @@ -985,11 +960,7 @@ for (i = 0; i < ifp->if_addrlen; i++) if (lla[i] != 0) break; -#ifdef VIMAGE - if (i != ifp->if_addrlen && home_vnet_0 != ifp->if_home_vnet) -#else if (i != ifp->if_addrlen) -#endif if_printf(ifp, "Ethernet address: %6D\n", lla, ":"); } ==== //depot/projects/vimage-commit2/src/sys/net/if_var.h#23 (text+ko) ==== @@ -765,6 +765,7 @@ int if_delmulti(struct ifnet *, struct sockaddr *); void if_delmulti_ifma(struct ifmultiaddr *); void if_detach(struct ifnet *); +void if_vmove(struct ifnet *, struct vnet *); void if_purgeaddrs(struct ifnet *); void if_purgemaddrs(struct ifnet *); void if_down(struct ifnet *);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200905150024.n4F0Ojix001733>