From owner-p4-projects@FreeBSD.ORG Tue Jul 19 21:18:28 2005 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 4BE4816A41F; Tue, 19 Jul 2005 21:18:28 +0000 (GMT) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id E669216A41F for ; Tue, 19 Jul 2005 21:18:27 +0000 (GMT) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 9BC0D43D46 for ; Tue, 19 Jul 2005 21:18:27 +0000 (GMT) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.1/8.13.1) with ESMTP id j6JLIR83090488 for ; Tue, 19 Jul 2005 21:18:27 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.1/8.13.1/Submit) id j6JLIRtB090485 for perforce@freebsd.org; Tue, 19 Jul 2005 21:18:27 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Date: Tue, 19 Jul 2005 21:18:27 GMT Message-Id: <200507192118.j6JLIRtB090485@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to bb+lists.freebsd.perforce@cyrus.watson.org using -f From: Robert Watson To: Perforce Change Reviews Cc: Subject: PERFORCE change 80552 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jul 2005 21:18:29 -0000 http://perforce.freebsd.org/chv.cgi?CH=80552 Change 80552 by rwatson@rwatson_zoo on 2005/07/19 21:17:57 Merge 80535 from rwatson_netperf to netsmp: Initial cut at locking multicast address list frobbing at the link layer. Affected files ... .. //depot/projects/netsmp/src/sys/net/if.c#2 edit .. //depot/projects/netsmp/src/sys/net/if_var.h#2 edit Differences ... ==== //depot/projects/netsmp/src/sys/net/if.c#2 (text+ko) ==== @@ -417,6 +417,8 @@ { if_free_type(ifp, ifp->if_type); + + IF_ADDR_LOCK_DESTROY(ifp); } void @@ -460,6 +462,7 @@ TASK_INIT(&ifp->if_starttask, 0, if_start_deferred, ifp); TASK_INIT(&ifp->if_linktask, 0, do_link_state_change, ifp); IF_AFDATA_LOCK_INIT(ifp); + IF_ADDR_LOCK_INIT(ifp); ifp->if_afdata_initialized = 0; IFNET_WLOCK(); TAILQ_INSERT_TAIL(&ifnet, ifp, if_link); @@ -1824,99 +1827,219 @@ return (if_setflag(ifp, IFF_ALLMULTI, 0, &ifp->if_amcount, onswitch)); } +struct ifmultiaddr * +ifmaof_ifpforaddr(struct sockaddr *sa, struct ifnet *ifp) +{ + struct ifmultiaddr *ifma; + + IF_ADDR_LOCK_ASSERT(ifp); + + TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) + if (sa_equal(ifma->ifma_addr, sa)) + break; + + return ifma; +} + +/* + * XXXRW: ifmaof_ifpforaddr() is an unsensible name, and appears not to be + * used, so use my more sensibly named version here. + */ +static struct ifmultiaddr * +if_findmulti(struct ifnet *ifp, struct sockaddr *sa) +{ + + IF_ADDR_LOCK_ASSERT(ifp); + + return (ifmaof_ifpforaddr(sa, ifp)); +} + +/* + * Allocate a new ifmultiaddr and initialize based on passed arguments. We + * make copies of passed sockaddrs. The ifmultiaddr will not be added to + * the ifnet multicast address list here, so the caller must do that and + * other setup work (such as notifying the device driver). The reference + * count is initialized to 1. + */ +static struct ifmultiaddr * +if_allocmulti(struct ifnet *ifp, struct sockaddr *sa, struct sockaddr *llsa, + int mflags) +{ + struct ifmultiaddr *ifma; + struct sockaddr *dupsa; + + KASSERT(ifp != NULL, ("if_allocmulti: NULL ifp")); + KASSERT(sa != NULL, ("if_allocmulti: NULL sa")); + + MALLOC(ifma, struct ifmultiaddr *, sizeof *ifma, M_IFMADDR, mflags); + if (ifma == NULL) + return (NULL); + + MALLOC(dupsa, struct sockaddr *, sa->sa_len, M_IFMADDR, mflags); + if (dupsa == NULL) { + FREE(ifma, M_IFMADDR); + return (NULL); + } + bcopy(sa, dupsa, sa->sa_len); + ifma->ifma_addr = dupsa; + + if (llsa != NULL) { + MALLOC(dupsa, struct sockaddr *, llsa->sa_len, M_IFMADDR, + mflags); + if (dupsa == NULL) { + FREE(ifma->ifma_addr, M_IFMADDR); + FREE(ifma, M_IFMADDR); + return (NULL); + } + bcopy(llsa, dupsa, llsa->sa_len); + ifma->ifma_lladdr = llsa; + } else + ifma->ifma_lladdr = NULL; + + ifma->ifma_ifp = ifp; + ifma->ifma_refcount = 1; + ifma->ifma_protospec = NULL; + + return (ifma); +} + +/* + * if_freemulti: free ifmultiaddr structure and possibly attached related + * addresses. The caller is responsible for implementing reference + * counting, notifying the driver, handling routing messages, and releasing + * any dependent link layer state. + */ +static void +if_freemulti(struct ifmultiaddr *ifma) +{ + + KASSERT(ifma->ifma_refcount == 1, ("if_freemulti: refcount %d", + ifma->ifma_refcount)); + KASSERT(ifma->ifma_protospec == NULL, + ("if_freemulti: protospec not NULL")); + + if (ifma->ifma_lladdr != NULL) + FREE(ifma->ifma_lladdr, M_IFMADDR); + FREE(ifma->ifma_addr, M_IFMADDR); + FREE(ifma, M_IFMADDR); +} + /* - * Add a multicast listenership to the interface in question. - * The link layer provides a routine which converts + * Register an additional multicast address with a network interface. + * + * - If the address is already present, bump the reference count on the + * address and return. + * - If the address is not link-layer, look up a link layer address. + * - Allocate address structures for one or both addresses, and attach to the + * multicast address list on the interface. If automatically adding a link + * layer address, the protocol address will own a reference to the link + * layer address, to be freed when it is freed. + * - Notify the network device driver of an addition to the multicast address + * list. + * + * 'sa' points to caller-owned memory with the desired multicast address. + * + * 'retifma' will be used to return a pointer to the resulting multicast + * address reference, if desired. */ int -if_addmulti(struct ifnet *ifp, struct sockaddr *sa, struct ifmultiaddr **retifma) +if_addmulti(struct ifnet *ifp, struct sockaddr *sa, + struct ifmultiaddr **retifma) { - struct sockaddr *llsa, *dupsa; - int error, s; - struct ifmultiaddr *ifma; + struct ifmultiaddr *ifma, *ll_ifma, *new_ifma, *new_ll_ifma; + struct sockaddr *llsa; + int error; /* - * If the matching multicast address already exists - * then don't add a new one, just add a reference + * If the address is already present, return a new reference to it; + * otherwise, allocate storage and set up a new address. Since we + * release the interface lock, we have to check for races in which + * another thread may also have set up the same address. */ - TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { - if (sa_equal(sa, ifma->ifma_addr)) { - ifma->ifma_refcount++; - if (retifma) - *retifma = ifma; - return 0; - } + IF_ADDR_LOCK(ifp); + ifma = if_findmulti(ifp, sa); + if (ifma != NULL) { + ifma->ifma_refcount++; + if (retifma != NULL) + *retifma = ifma; + IF_ADDR_UNLOCK(ifp); + return (0); } + IF_ADDR_UNLOCK(ifp); /* - * Give the link layer a chance to accept/reject it, and also - * find out which AF_LINK address this maps to, if it isn't one - * already. + * The address isn't already present; perform a link layer + * resolution if it's not already a link layer address, and allocate + * and set up the address. As of this point, if llsa is non-NULL, + * we must free the sockaddr if we don't need it. */ if (ifp->if_resolvemulti != NULL) { error = ifp->if_resolvemulti(ifp, &llsa, sa); - if (error) return error; - } else { + if (error) + return error; + } else llsa = NULL; - } - MALLOC(ifma, struct ifmultiaddr *, sizeof *ifma, M_IFMADDR, M_WAITOK); - MALLOC(dupsa, struct sockaddr *, sa->sa_len, M_IFMADDR, M_WAITOK); - bcopy(sa, dupsa, sa->sa_len); - - ifma->ifma_addr = dupsa; - ifma->ifma_lladdr = llsa; - ifma->ifma_ifp = ifp; - ifma->ifma_refcount = 1; - ifma->ifma_protospec = NULL; - rt_newmaddrmsg(RTM_NEWMADDR, ifma); + new_ifma = if_allocmulti(ifp, sa, llsa, M_WAITOK); + if (llsa != NULL) + new_ll_ifma = if_allocmulti(ifp, llsa, NULL, M_WAITOK); + else + new_ll_ifma = NULL; /* gcc */ /* - * Some network interfaces can scan the address list at - * interrupt time; lock them out. + * Now check to see if we lost the race, and continue inserting if + * not. Note that we need to separately consider the link layer + * and protocol layer multicast addresses. */ - s = splimp(); - TAILQ_INSERT_HEAD(&ifp->if_multiaddrs, ifma, ifma_link); - splx(s); - if (retifma != NULL) - *retifma = ifma; + IF_ADDR_LOCK(ifp); + ifma = if_findmulti(ifp, sa); + if (llsa != NULL) + ll_ifma = if_findmulti(ifp, llsa); + else + ll_ifma = NULL; /* gcc */ + + if (ifma != NULL) { + if_freemulti(new_ifma); + ifma->ifma_refcount++; + } else { + ifma = new_ifma; + TAILQ_INSERT_HEAD(&ifp->if_multiaddrs, ifma, ifma_link); + } if (llsa != NULL) { - TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { - if (sa_equal(ifma->ifma_addr, llsa)) - break; - } - if (ifma) { - ifma->ifma_refcount++; + if (ll_ifma != NULL) { + if_freemulti(new_ll_ifma); + ll_ifma->ifma_refcount++; } else { - MALLOC(ifma, struct ifmultiaddr *, sizeof *ifma, - M_IFMADDR, M_WAITOK); - MALLOC(dupsa, struct sockaddr *, llsa->sa_len, - M_IFMADDR, M_WAITOK); - bcopy(llsa, dupsa, llsa->sa_len); - ifma->ifma_addr = dupsa; - ifma->ifma_lladdr = NULL; - ifma->ifma_ifp = ifp; - ifma->ifma_refcount = 1; - ifma->ifma_protospec = NULL; - s = splimp(); - TAILQ_INSERT_HEAD(&ifp->if_multiaddrs, ifma, ifma_link); - splx(s); + ll_ifma = new_ll_ifma; + TAILQ_INSERT_HEAD(&ifp->if_multiaddrs, ll_ifma, + ifma_link); } } + + if (retifma != NULL) + *retifma = ifma; + + /* + * Must generate the message while holding the lock so that 'ifma' + * pointer is still valid. + */ + rt_newmaddrmsg(RTM_NEWMADDR, ifma); + IF_ADDR_UNLOCK(ifp); + /* * We are certain we have added something, so call down to the * interface to let them know about it. */ if (ifp->if_ioctl != NULL) { - s = splimp(); IFF_LOCKGIANT(ifp); (void) (*ifp->if_ioctl)(ifp, SIOCADDMULTI, 0); IFF_UNLOCKGIANT(ifp); - splx(s); } - return 0; + if (llsa != NULL) + FREE(llsa, M_IFMADDR); + return (0); } /* @@ -2070,18 +2193,6 @@ return (0); } -struct ifmultiaddr * -ifmaof_ifpforaddr(struct sockaddr *sa, struct ifnet *ifp) -{ - struct ifmultiaddr *ifma; - - TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) - if (sa_equal(ifma->ifma_addr, sa)) - break; - - return ifma; -} - /* * The name argument must be a pointer to storage which will last as * long as the interface does. For physical devices, the result of ==== //depot/projects/netsmp/src/sys/net/if_var.h#2 (text+ko) ==== @@ -177,6 +177,7 @@ void *if_afdata[AF_MAX]; int if_afdata_initialized; struct mtx if_afdata_mtx; + struct mtx if_addr_mtx; /* mutex to protect address lists */ struct task if_starttask; /* task for IFF_NEEDSGIANT */ struct task if_linktask; /* task for link change events */ }; @@ -217,6 +218,16 @@ #define if_list if_link /* + * Locks for address lists on the network interface. + */ +#define IF_ADDR_LOCK_INIT(if) mtx_init(&(if)->if_addr_mtx, \ + "if_addr_mtx", NULL, MTX_DEF) +#define IF_ADDR_LOCK_DESTROY(if) mtx_destroy(&(if)->if_addr_mtx) +#define IF_ADDR_LOCK(if) mtx_lock(&(if)->if_addr_mtx) +#define IF_ADDR_UNLOCK(if) mtx_unlock(&(if)->if_addr_mtx) +#define IF_ADDR_LOCK_ASSERT(if) mtx_assert(&(if)->if_addr_mtx, MA_OWNED) + +/* * Output queues (ifp->if_snd) and slow device input queues (*ifp->if_slowq) * are queues of messages stored on ifqueue structures * (defined above). Entries are added to and deleted from these structures