PERFORCE change 80817 for review
Robert Watson
rwatson at FreeBSD.org
Sat Jul 23 00:00:11 GMT 2005
http://perforce.freebsd.org/chv.cgi?CH=80817
Change 80817 by rwatson at rwatson_zoo on 2005/07/22 23:59:30
Pass M_ZERO to MALLOC() when allocating multicast addresses. This
isn't strictly necessary, but will avoid confusing when looking at
the structure later in a debugger.
Since we now allow M_NOWAIT to be used throughout the multicast
code, rearrange the if_addmulti() code to no longer use the "detect
race and recover" approach, but instead detect allocation failure
and abort. This simplifies the logic quite a bit, and avoids having
lots of memory floating around that may or may not need to be freed.
Affected files ...
.. //depot/projects/netsmp/src/sys/net/if.c#5 edit
Differences ...
==== //depot/projects/netsmp/src/sys/net/if.c#5 (text+ko) ====
@@ -1834,9 +1834,10 @@
IF_ADDR_LOCK_ASSERT(ifp);
- TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link)
+ TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) {
if (sa_equal(ifma->ifma_addr, sa))
break;
+ }
return ifma;
}
@@ -1868,10 +1869,8 @@
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);
+ MALLOC(ifma, struct ifmultiaddr *, sizeof *ifma, M_IFMADDR, mflags |
+ M_ZERO);
if (ifma == NULL)
return (NULL);
@@ -1883,23 +1882,24 @@
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;
+ if (llsa == NULL) {
+ ifma->ifma_lladdr = NULL;
+ return (ifma);
+ }
+
+ 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 = dupsa;
+
return (ifma);
}
@@ -1946,15 +1946,13 @@
if_addmulti(struct ifnet *ifp, struct sockaddr *sa,
struct ifmultiaddr **retifma)
{
- struct ifmultiaddr *ifma, *ll_ifma, *new_ifma, *new_ll_ifma;
+ struct ifmultiaddr *ifma, *ll_ifma;
struct sockaddr *llsa;
int error;
/*
* 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.
+ * otherwise, allocate storage and set up a new address.
*/
IF_ADDR_LOCK(ifp);
ifma = if_findmulti(ifp, sa);
@@ -1965,69 +1963,59 @@
IF_ADDR_UNLOCK(ifp);
return (0);
}
- IF_ADDR_UNLOCK(ifp);
/*
- * 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.
+ * The address isn't already present; resolve the protocol address
+ * into a link layer address, and then look that up, bump its
+ * refcount or allocate an ifma for that also. If 'llsa' was
+ * returned, we will need to free it later.
*/
+ llsa = NULL;
+ ll_ifma = NULL;
if (ifp->if_resolvemulti != NULL) {
error = ifp->if_resolvemulti(ifp, &llsa, sa);
if (error)
- return error;
- } else
- llsa = NULL;
-
- new_ifma = if_allocmulti(ifp, sa, llsa, M_NOWAIT);
- if (new_ifma == NULL) {
- if (llsa != NULL)
- free(llsa, M_IFMADDR);
- return (ENOMEM);
+ goto unlock_out;
}
- if (llsa != NULL) {
- new_ll_ifma = if_allocmulti(ifp, llsa, NULL, M_NOWAIT);
- if (new_ll_ifma == NULL) {
- if_freemulti(new_ifma);
- if (llsa != NULL)
- free(llsa, M_IFMADDR);
- return (ENOMEM);
- }
- } else
- new_ll_ifma = NULL; /* gcc */
/*
- * 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.
+ * Allocate the new address. Don't hook it up yet, as we may also
+ * need to allocate a link layer multicast address.
*/
- 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);
+ ifma = if_allocmulti(ifp, sa, llsa, M_NOWAIT);
+ if (ifma == NULL) {
+ error = ENOMEM;
+ goto free_llsa_out;
}
+ /*
+ * If a link layer address is found, we'll need to see if it's
+ * already present in the address list, or allocate is as well.
+ * When this block finishes, the link layer address will be on the
+ * list.
+ */
if (llsa != NULL) {
- if (ll_ifma != NULL) {
- if_freemulti(new_ll_ifma);
- ll_ifma->ifma_refcount++;
- } else {
- ll_ifma = new_ll_ifma;
+ ll_ifma = if_findmulti(ifp, llsa);
+ if (ll_ifma == NULL) {
+ ll_ifma = if_allocmulti(ifp, llsa, NULL, M_NOWAIT);
+ if (ll_ifma == NULL) {
+ if_freemulti(ifma);
+ error = ENOMEM;
+ goto free_llsa_out;
+ }
TAILQ_INSERT_HEAD(&ifp->if_multiaddrs, ll_ifma,
ifma_link);
- }
+ } else
+ ll_ifma->ifma_refcount++;
}
+ /*
+ * We now have a new multicast address, ifma, and possibly a new or
+ * referenced link layer address. Add the primary address to the
+ * ifnet address list.
+ */
+ TAILQ_INSERT_HEAD(&ifp->if_multiaddrs, ifma, ifma_link);
+
if (retifma != NULL)
*retifma = ifma;
@@ -2052,7 +2040,16 @@
if (llsa != NULL)
FREE(llsa, M_IFMADDR);
+
return (0);
+
+free_llsa_out:
+ if (llsa != NULL)
+ FREE(llsa, M_IFMADDR);
+
+unlock_out:
+ IF_ADDR_UNLOCK(ifp);
+ return (error);
}
/*
@@ -2078,7 +2075,10 @@
}
sa = ifma->ifma_lladdr;
- ll_ifma = if_findmulti(ifp, sa);
+ if (sa != NULL)
+ ll_ifma = if_findmulti(ifp, sa);
+ else
+ ll_ifma = NULL;
/*
* XXXRW: How come we don't announce ll_ifma?
More information about the p4-projects
mailing list