On 23 December 2011 23:46, John Baldwin <j...@freebsd.org> wrote:
> I found these nits while working on the patches to convert if_addr_mtx to an
> rwlock. The first change is cosmetic, it just un-inlines a TAILQ_FOREACH().
> The second change is an actual bug. The code is currently reading
> TAILQ_FIRST(&V_ifnet) without holding the appropriate lock.
>
> Index: icmp6.c
> ===================================================================
> --- icmp6.c (revision 228777)
> +++ icmp6.c (working copy)
> @@ -1780,7 +1780,7 @@ ni6_addrs(struct icmp6_nodeinfo *ni6, struct mbuf
> }
>
> IFNET_RLOCK_NOSLEEP();
> - for (ifp = TAILQ_FIRST(&V_ifnet); ifp; ifp = TAILQ_NEXT(ifp,
> if_list)) {
> + TAILQ_FOREACH(ifp, &V_ifnet, if_list) {
> addrsofif = 0;
> IF_ADDR_LOCK(ifp);
> TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
FWIW, there are much more of them in netinet6.
Some time ago I started to un-expand them to queue(3).
[not unfinished yet..]
Index: /sys/netinet6/in6_ifattach.c
===================================================================
--- /sys/netinet6/in6_ifattach.c (revision 228686)
+++ /sys/netinet6/in6_ifattach.c (working copy)
@@ -405,7 +405,7 @@
/* next, try to get it from some other hardware interface */
IFNET_RLOCK_NOSLEEP();
- for (ifp = V_ifnet.tqh_first; ifp; ifp = ifp->if_list.tqe_next) {
+ TAILQ_FOREACH(ifp, &V_ifnet, if_list) {
if (ifp == ifp0)
continue;
if (in6_get_hw_ifid(ifp, in6) != 0)
@@ -820,7 +820,7 @@
/*
* leave from multicast groups we have joined for the interface
*/
- while ((imm = ia->ia6_memberships.lh_first) != NULL) {
+ while ((imm = LIST_FIRST(&ia->ia6_memberships)) != NULL) {
LIST_REMOVE(imm, i6mm_chain);
in6_leavegroup(imm);
}
@@ -923,8 +923,7 @@
V_ip6_temp_regen_advance) * hz, in6_tmpaddrtimer, curvnet);
bzero(nullbuf, sizeof(nullbuf));
- for (ifp = TAILQ_FIRST(&V_ifnet); ifp;
- ifp = TAILQ_NEXT(ifp, if_list)) {
+ TAILQ_FOREACH(ifp, &V_ifnet, if_list) {
ndi = ND_IFINFO(ifp);
if (bcmp(ndi->randomid, nullbuf, sizeof(nullbuf)) != 0) {
/*
Index: /sys/netinet6/icmp6.c
===================================================================
--- /sys/netinet6/icmp6.c (revision 228686)
+++ /sys/netinet6/icmp6.c (working copy)
@@ -779,9 +779,8 @@
/* -1 == no app on SEND socket */
if (error == 0)
return (IPPROTO_DONE);
- nd6_rs_input(m, off, icmp6len);
- } else
- nd6_rs_input(m, off, icmp6len);
+ }
+ nd6_rs_input(m, off, icmp6len);
m = NULL;
goto freeit;
}
@@ -793,9 +792,8 @@
if (error == 0)
goto freeit;
/* -1 == no app on SEND socket */
- nd6_rs_input(n, off, icmp6len);
- } else
- nd6_rs_input(n, off, icmp6len);
+ }
+ nd6_rs_input(n, off, icmp6len);
/* m stays. */
break;
@@ -813,9 +811,8 @@
SND_IN, ip6len);
if (error == 0)
return (IPPROTO_DONE);
- nd6_ra_input(m, off, icmp6len);
- } else
- nd6_ra_input(m, off, icmp6len);
+ }
+ nd6_ra_input(m, off, icmp6len);
m = NULL;
goto freeit;
}
@@ -824,9 +821,8 @@
SND_IN, ip6len);
if (error == 0)
goto freeit;
- nd6_ra_input(n, off, icmp6len);
- } else
- nd6_ra_input(n, off, icmp6len);
+ }
+ nd6_ra_input(n, off, icmp6len);
/* m stays. */
break;
@@ -842,9 +838,8 @@
SND_IN, ip6len);
if (error == 0)
return (IPPROTO_DONE);
- nd6_ns_input(m, off, icmp6len);
- } else
- nd6_ns_input(m, off, icmp6len);
+ }
+ nd6_ns_input(m, off, icmp6len);
m = NULL;
goto freeit;
}
@@ -853,9 +848,8 @@
SND_IN, ip6len);
if (error == 0)
goto freeit;
- nd6_ns_input(n, off, icmp6len);
- } else
- nd6_ns_input(n, off, icmp6len);
+ }
+ nd6_ns_input(n, off, icmp6len);
/* m stays. */
break;
@@ -873,9 +867,8 @@
SND_IN, ip6len);
if (error == 0)
return (IPPROTO_DONE);
- nd6_na_input(m, off, icmp6len);
- } else
- nd6_na_input(m, off, icmp6len);
+ }
+ nd6_na_input(m, off, icmp6len);
m = NULL;
goto freeit;
}
@@ -884,9 +877,8 @@
SND_IN, ip6len);
if (error == 0)
goto freeit;
- nd6_na_input(n, off, icmp6len);
- } else
- nd6_na_input(n, off, icmp6len);
+ }
+ nd6_na_input(n, off, icmp6len);
/* m stays. */
break;
@@ -902,9 +894,8 @@
SND_IN, ip6len);
if (error == 0)
return (IPPROTO_DONE);
- icmp6_redirect_input(m, off);
- } else
- icmp6_redirect_input(m, off);
+ }
+ icmp6_redirect_input(m, off);
m = NULL;
goto freeit;
}
@@ -913,9 +904,8 @@
SND_IN, ip6len);
if (error == 0)
goto freeit;
- icmp6_redirect_input(n, off);
- } else
- icmp6_redirect_input(n, off);
+ }
+ icmp6_redirect_input(n, off);
/* m stays. */
break;
Index: /sys/netinet6/nd6.c
===================================================================
--- /sys/netinet6/nd6.c (revision 228686)
+++ /sys/netinet6/nd6.c (working copy)
@@ -575,7 +575,6 @@
struct nd_defrouter *dr;
struct nd_prefix *pr;
struct in6_ifaddr *ia6, *nia6;
- struct in6_addrlifetime *lt6;
callout_reset(&V_nd6_timer_ch, V_nd6_prune * hz,
nd6_timer, curvnet);
@@ -604,8 +603,6 @@
*/
addrloop:
TAILQ_FOREACH_SAFE(ia6, &V_in6_ifaddrhead, ia_link, nia6) {
- /* check address lifetime */
- lt6 = &ia6->ia6_lifetime;
if (IFA6_IS_INVALID(ia6)) {
int regen = 0;
@@ -668,7 +665,7 @@
}
/* expire prefix list */
- pr = V_nd_prefix.lh_first;
+ pr = LIST_FIRST(&V_nd_prefix);
while (pr) {
/*
* check prefix lifetime.
@@ -800,7 +797,7 @@
}
/* Nuke prefix list entries toward ifp */
- for (pr = V_nd_prefix.lh_first; pr; pr = npr) {
+ for (pr = LIST_FIRST(&V_nd_prefix); pr; pr = npr) {
npr = pr->ndpr_next;
if (pr->ndpr_ifp == ifp) {
/*
@@ -912,7 +909,7 @@
* If the address matches one of our on-link prefixes, it should be a
* neighbor.
*/
- for (pr = V_nd_prefix.lh_first; pr; pr = pr->ndpr_next) {
+ for (pr = LIST_FIRST(&V_nd_prefix); pr; pr = pr->ndpr_next) {
if (pr->ndpr_ifp != ifp)
continue;
@@ -1263,7 +1260,7 @@
*/
bzero(oprl, sizeof(*oprl));
s = splnet();
- pr = V_nd_prefix.lh_first;
+ pr = LIST_FIRST(&V_nd_prefix);
while (pr && i < PRLSTSIZ) {
struct nd_pfxrouter *pfr;
int j;
@@ -1292,7 +1289,7 @@
oprl->prefix[i].expire = maxexpire;
}
- pfr = pr->ndpr_advrtrs.lh_first;
+ pfr = LIST_FIRST(&pr->ndpr_advrtrs);
j = 0;
while (pfr) {
if (j < DRLSTSIZ) {
@@ -1470,7 +1467,7 @@
struct nd_prefix *pr, *next;
s = splnet();
- for (pr = V_nd_prefix.lh_first; pr; pr = next) {
+ for (pr = LIST_FIRST(&V_nd_prefix); pr; pr = next) {
struct in6_ifaddr *ia, *ia_next;
next = pr->ndpr_next;
@@ -2335,7 +2332,7 @@
return EPERM;
error = 0;
- for (pr = V_nd_prefix.lh_first; pr; pr = pr->ndpr_next) {
+ for (pr = LIST_FIRST(&V_nd_prefix); pr; pr = pr->ndpr_next) {
u_short advrtrs;
size_t advance;
struct sockaddr_in6 *sin6, *s6;
@@ -2380,7 +2377,7 @@
p->flags = pr->ndpr_stateflags;
p->origin = PR_ORIG_RA;
advrtrs = 0;
- for (pfr = pr->ndpr_advrtrs.lh_first; pfr;
+ for (pfr = LIST_FIRST(&pr->ndpr_advrtrs); pfr;
pfr = pfr->pfr_next) {
if ((void *)&sin6[advrtrs + 1] > (void *)pe) {
advrtrs++;
Index: /sys/netinet6/in6.c
===================================================================
--- /sys/netinet6/in6.c (revision 228686)
+++ /sys/netinet6/in6.c (working copy)
@@ -1362,7 +1362,7 @@
/*
* leave from multicast groups we have joined for the interface
*/
- while ((imm = ia->ia6_memberships.lh_first) != NULL) {
+ while ((imm = LIST_FIRST(&ia->ia6_memberships)) != NULL) {
LIST_REMOVE(imm, i6mm_chain);
in6_leavegroup(imm);
}
Index: /sys/netinet6/nd6_rtr.c
===================================================================
--- /sys/netinet6/nd6_rtr.c (revision 228686)
+++ /sys/netinet6/nd6_rtr.c (working copy)
@@ -581,7 +581,7 @@
/*
* Also delete all the pointers to the router in each prefix lists.
*/
- for (pr = V_nd_prefix.lh_first; pr; pr = pr->ndpr_next) {
+ LIST_FOREACH(pr, &V_nd_prefix, ndpr_entry) {
struct nd_pfxrouter *pfxrtr;
if ((pfxrtr = pfxrtr_lookup(pr, dr)) != NULL)
pfxrtr_del(pfxrtr);
--
wbr,
pluknet
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
