Date: Tue, 22 Aug 2006 19:32:04 +0200 From: Max Laier <max@love2party.net> To: freebsd-pf@freebsd.org Subject: Re: CARP panics on RELENG_6 when destroying a CARP interface Message-ID: <200608221932.10056.max@love2party.net> In-Reply-To: <200608141947.06724.max@love2party.net> References: <d5992baf0608140939u5cd4ca8docd7730132095f7fb@mail.gmail.com> <200608141947.06724.max@love2party.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart3648429.JpapihDmP6
Content-Type: multipart/mixed;
boundary="Boundary-01=_V+z6EKu4gVOwhPv"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
--Boundary-01=_V+z6EKu4gVOwhPv
Content-Type: text/plain;
charset="iso-8859-6"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
On Monday 14 August 2006 19:46, I wrote:
> On Monday 14 August 2006 18:39, Scott Ullrich wrote:
> > I am testing out CARP on RELENG_6 as of yesterday and I am seeing a
> > panic when attempting to destory a CARP interface:
> >
> > # ifconfig carp0 delete
> > # ifconfig carp0 destroy
> > # panic: thread 100049(ifconfig):0 holds carp_if but isn't blocked on
> > a lock
> >
> > KDB: enter: panic
> > [thread pid 12 tid 100004 ]
> > Stopped at kdb_enter+0x2b: nop
> > db> bt
> > Tracing pid 12 tid 100004 td 0xc14d6900
> > kdb_enter(c08690a0) at kdb_enter+0x2b
> > panic(c086c2f3,186d6,c1630bc4,0,c0876fc4) at panic+0xbb
> > propagate_priority(c14d6900,c0948fd0,c15a7e90,c14d6900,c1575000) at
> > propagate_pr iority+0x137
> > turnstile_wait(c15a7e90,c1632000,c15a7e90,2,c0868048,225) at
> > turnstile_wait+0x2f 0
> > _mtx_lock_sleep(c15a7e90,c14d6900,0,c0876cbe,283) at
> > _mtx_lock_sleep+0x102 _mtx_lock_flags(c15a7e90,0,c0876cbe,283,0) at
> > _mtx_lock_flags+0x72
> > carp_input_c(c15e8500,c15e8544,2,c15e8544,c172100e) at
> > carp_input_c+0x30 carp_input(c16eb700,14,c15fa940,0,0) at
> > carp_input+0x216
> > ip_input(c16eb700) at ip_input+0x7ad
>
> Looks like a race between the check in ip_carp.c:502
> m->m_pkthdr.rcvif->if_carp =3D=3D NULL
> and the actual use of that interface pointer. I'm afraid we need some
> form of synchronization for access to ifnet.if_carp From a quick
> glance we could either use IFADDR_LOCK() or the global IFNET_{W,R}LOCK=20
> I will look at producing a patch later tonight.
Took a little longer - I actually forgot about it :-\ Anyway, find=20
attached a WIP of the above idea. It's completely untested (it should=20
compile, though) - so test with care. Also, testing this should happen=20
with WITNESS enabled since I didn't really look closely if I produced=20
LORs or the like - I doubt it, however.
This is more or less to verify the hypothesis - let me know if it works.
> > netisr_processqueue(c094a958) at netisr_processqueue+0x6e
> > swi_net(0) at swi_net+0xc6
> > ithread_execute_handlers(c14d5830,c14d3580) at
> > ithread_execute_handlers+0xe6
> > ithread_loop(c14bd760,c796cd38,c14bd760,c05f76a8,0) at
> > ithread_loop+0x66 fork_exit(c05f76a8,c14bd760,c796cd38) at
> > fork_exit+0xa0
> > fork_trampoline() at fork_trampoline+0x8
> > --- trap 0x1, eip =3D 0, esp =3D 0xc796cd6c, ebp =3D 0 ---
> > db>
> >
> > Please let me know if I can supply more information.
=2D-=20
/"\ Best regards, | mlaier@freebsd.org
\ / Max Laier | ICQ #67774661
X http://pf4freebsd.love2party.net/ | mlaier@EFnet
/ \ ASCII Ribbon Campaign | Against HTML Mail and News
--Boundary-01=_V+z6EKu4gVOwhPv
Content-Type: text/x-diff;
charset="iso-8859-6";
name="if_carp.lock.diff"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
filename="if_carp.lock.diff"
Index: net/if.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /usr/store/mlaier/fcvs/src/sys/net/if.c,v
retrieving revision 1.261
diff -u -p -r1.261 if.c
=2D-- net/if.c 9 Jul 2006 06:04:00 -0000 1.261
+++ net/if.c 22 Aug 2006 16:45:55 -0000
@@ -1223,8 +1223,10 @@ if_unroute(struct ifnet *ifp, int flag,=20
pfctlinput(PRC_IFDOWN, ifa->ifa_addr);
if_qflush(&ifp->if_snd);
#ifdef DEV_CARP
+ IF_ADDR_LOCK(ifp);
if (ifp->if_carp)
carp_carpdev_state(ifp->if_carp);
+ IF_ADDR_UNLOCK(ifp);
#endif
rt_ifmsg(ifp);
}
@@ -1247,8 +1249,10 @@ if_route(struct ifnet *ifp, int flag, in
if (fam =3D=3D PF_UNSPEC || (fam =3D=3D ifa->ifa_addr->sa_family))
pfctlinput(PRC_IFUP, ifa->ifa_addr);
#ifdef DEV_CARP
+ IF_ADDR_LOCK(ifp);
if (ifp->if_carp)
carp_carpdev_state(ifp->if_carp);
+ IF_ADDR_UNLOCK(ifp);
#endif
rt_ifmsg(ifp);
#ifdef INET6
@@ -1300,8 +1304,10 @@ do_link_state_change(void *arg, int pend
IFP2AC(ifp)->ac_netgraph !=3D NULL)
(*ng_ether_link_state_p)(ifp, link_state);
#ifdef DEV_CARP
+ IF_ADDR_LOCK(ifp);
if (ifp->if_carp)
carp_carpdev_state(ifp->if_carp);
+ IF_ADDR_UNLOCK(ifp);
#endif
if (ifp->if_bridge) {
KASSERT(bstp_linkstate_p !=3D NULL,("if_bridge bstp not loaded!"));
Index: net/if_var.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /usr/store/mlaier/fcvs/src/sys/net/if_var.h,v
retrieving revision 1.108
diff -u -p -r1.108 if_var.h
=2D-- net/if_var.h 4 Aug 2006 21:27:37 -0000 1.108
+++ net/if_var.h 22 Aug 2006 16:43:02 -0000
@@ -227,12 +227,12 @@ typedef void if_init_f_t(void *);
/*
* Locks for address lists on the network interface.
*/
=2D#define IF_ADDR_LOCK_INIT(if) mtx_init(&(if)->if_addr_mtx, \
+#define IF_ADDR_LOCK_INIT(ifp) mtx_init(&(ifp)->if_addr_mtx, \
"if_addr_mtx", NULL, MTX_DEF)
=2D#define IF_ADDR_LOCK_DESTROY(if) mtx_destroy(&(if)->if_addr_mtx)
=2D#define IF_ADDR_LOCK(if) mtx_lock(&(if)->if_addr_mtx)
=2D#define IF_ADDR_UNLOCK(if) mtx_unlock(&(if)->if_addr_mtx)
=2D#define IF_ADDR_LOCK_ASSERT(if) mtx_assert(&(if)->if_addr_mtx, MA_OWNED)
+#define IF_ADDR_LOCK_DESTROY(ifp) mtx_destroy(&(ifp)->if_addr_mtx)
+#define IF_ADDR_LOCK(ifp) mtx_lock(&(ifp)->if_addr_mtx)
+#define IF_ADDR_UNLOCK(ifp) mtx_unlock(&(ifp)->if_addr_mtx)
+#define IF_ADDR_LOCK_ASSERT(ifp) mtx_assert(&(ifp)->if_addr_mtx, MA_OWNED)
=20
/*
* Output queues (ifp->if_snd) and slow device input queues (*ifp->if_slow=
q)
Index: netinet/if_ether.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /usr/store/mlaier/fcvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.153
diff -u -p -r1.153 if_ether.c
=2D-- netinet/if_ether.c 29 Jun 2006 19:22:04 -0000 1.153
+++ netinet/if_ether.c 22 Aug 2006 16:50:31 -0000
@@ -636,12 +636,15 @@ in_arpinput(m)
itaddr.s_addr =3D=3D ia->ia_addr.sin_addr.s_addr)
goto match;
#ifdef DEV_CARP
+ IF_ADDR_LOCK(ifp);
if (ifp->if_carp !=3D NULL &&
carp_iamatch(ifp->if_carp, ia, &isaddr, &enaddr) &&
itaddr.s_addr =3D=3D ia->ia_addr.sin_addr.s_addr) {
carp_match =3D 1;
+ IF_ADDR_UNLOCK(ifp);
goto match;
}
+ IF_ADDR_UNLOCK(ifp);
#endif
}
LIST_FOREACH(ia, INADDR_HASH(isaddr.s_addr), ia_hash)
Index: netinet/ip_carp.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /usr/store/mlaier/fcvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.42
diff -u -p -r1.42 ip_carp.c
=2D-- netinet/ip_carp.c 9 Jul 2006 06:04:01 -0000 1.42
+++ netinet/ip_carp.c 22 Aug 2006 17:04:52 -0000
@@ -451,7 +451,9 @@ carpdetach(struct carp_softc *sc)
TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);
if (!--cif->vhif_nvrs) {
ifpromisc(sc->sc_carpdev, 0);
+ IF_ADDR_LOCK(ifp);
sc->sc_carpdev->if_carp =3D NULL;
+ IF_ADDR_UNLOCK(ifp);
CARP_LOCK_DESTROY(cif);
FREE(cif, M_IFADDR);
}
@@ -489,6 +491,7 @@ carp_input(struct mbuf *m, int hlen)
{
struct ip *ip =3D mtod(m, struct ip *);
struct carp_header *ch;
+ struct ifnet *ifp =3D m->m_pkthdr.rcvif;
int iplen, len;
=20
carpstats.carps_ipackets++;
@@ -499,13 +502,13 @@ carp_input(struct mbuf *m, int hlen)
}
=20
/* check if received on a valid carp interface */
=2D if (m->m_pkthdr.rcvif->if_carp =3D=3D NULL) {
+ IF_ADDR_LOCK(ifp);
+ if (ifp->if_carp =3D=3D NULL) {
carpstats.carps_badif++;
CARP_LOG("carp_input: packet received on non-carp "
"interface: %s\n",
m->m_pkthdr.rcvif->if_xname);
=2D m_freem(m);
=2D return;
+ goto drop_pkt;
}
=20
/* verify that the IP TTL is 255. */
@@ -514,8 +517,7 @@ carp_input(struct mbuf *m, int hlen)
CARP_LOG("carp_input: received ttl %d !=3D 255i on %s\n",
ip->ip_ttl,
m->m_pkthdr.rcvif->if_xname);
=2D m_freem(m);
=2D return;
+ goto drop_pkt;
}
=20
iplen =3D ip->ip_hl << 2;
@@ -525,15 +527,14 @@ carp_input(struct mbuf *m, int hlen)
CARP_LOG("carp_input: received len %zd < "
"sizeof(struct carp_header)\n",
m->m_len - sizeof(struct ip));
=2D m_freem(m);
=2D return;
+ goto drop_pkt;
}
=20
if (iplen + sizeof(*ch) < m->m_len) {
if ((m =3D m_pullup(m, iplen + sizeof(*ch))) =3D=3D NULL) {
carpstats.carps_hdrops++;
CARP_LOG("carp_input: pullup failed\n");
=2D return;
+ goto drop_pkt;
}
ip =3D mtod(m, struct ip *);
}
@@ -549,13 +550,12 @@ carp_input(struct mbuf *m, int hlen)
CARP_LOG("carp_input: packet too short %d on %s\n",
m->m_pkthdr.len,
m->m_pkthdr.rcvif->if_xname);
=2D m_freem(m);
=2D return;
+ goto drop_pkt;
}
=20
if ((m =3D m_pullup(m, len)) =3D=3D NULL) {
carpstats.carps_hdrops++;
=2D return;
+ goto drop_pkt;
}
ip =3D mtod(m, struct ip *);
ch =3D (struct carp_header *)((char *)ip + iplen);
@@ -566,12 +566,17 @@ carp_input(struct mbuf *m, int hlen)
carpstats.carps_badsum++;
CARP_LOG("carp_input: checksum failed on %s\n",
m->m_pkthdr.rcvif->if_xname);
=2D m_freem(m);
=2D return;
+ goto drop_pkt;
}
m->m_data -=3D iplen;
=20
carp_input_c(m, ch, AF_INET);
+ m =3D NULL;
+drop_pkt:
+ IF_ADDR_UNLOCK(ifp);
+ if (m !=3D NULL)
+ m_freem(m);
+ return;
}
=20
#ifdef INET6
@@ -581,6 +586,7 @@ carp6_input(struct mbuf **mp, int *offp,
struct mbuf *m =3D *mp;
struct ip6_hdr *ip6 =3D mtod(m, struct ip6_hdr *);
struct carp_header *ch;
+ struct ifnet *ifp =3D m->m_pkthdr.rcvif;
u_int len;
=20
carpstats.carps_ipackets6++;
@@ -591,13 +597,13 @@ carp6_input(struct mbuf **mp, int *offp,
}
=20
/* check if received on a valid carp interface */
+ IF_ADDR_LOCK(ifp);
if (m->m_pkthdr.rcvif->if_carp =3D=3D NULL) {
carpstats.carps_badif++;
CARP_LOG("carp6_input: packet received on non-carp "
"interface: %s\n",
m->m_pkthdr.rcvif->if_xname);
=2D m_freem(m);
=2D return (IPPROTO_DONE);
+ goto drop_pkt;
}
=20
/* verify that the IP TTL is 255 */
@@ -606,8 +612,7 @@ carp6_input(struct mbuf **mp, int *offp,
CARP_LOG("carp6_input: received ttl %d !=3D 255 on %s\n",
ip6->ip6_hlim,
m->m_pkthdr.rcvif->if_xname);
=2D m_freem(m);
=2D return (IPPROTO_DONE);
+ goto drop_pkt;
}
=20
/* verify that we have a complete carp packet */
@@ -616,7 +621,7 @@ carp6_input(struct mbuf **mp, int *offp,
if (ch =3D=3D NULL) {
carpstats.carps_badlen++;
CARP_LOG("carp6_input: packet size %u too small\n", len);
=2D return (IPPROTO_DONE);
+ goto drop_pkt;
}
=20
=20
@@ -626,12 +631,16 @@ carp6_input(struct mbuf **mp, int *offp,
carpstats.carps_badsum++;
CARP_LOG("carp6_input: checksum failed, on %s\n",
m->m_pkthdr.rcvif->if_xname);
=2D m_freem(m);
=2D return (IPPROTO_DONE);
+ goto drop_pkt;
}
m->m_data -=3D *offp;
=20
carp_input_c(m, ch, AF_INET6);
+ m =3D NULL;
+drop_pkt:
+ IF_ADDR_UNLOCK(ifp);
+ if (m !=3D NULL)
+ m_freem(m);
return (IPPROTO_DONE);
}
#endif /* INET6 */
@@ -1466,7 +1475,9 @@ carp_set_addr(struct carp_softc *sc, str
CARP_LOCK(cif);
cif->vhif_ifp =3D ifp;
TAILQ_INIT(&cif->vhif_vrs);
+ IF_ADDR_LOCK(ifp);
ifp->if_carp =3D cif;
+ IF_ADDR_UNLOCK(ifp);
=20
} else {
struct carp_softc *vr;
@@ -1543,7 +1554,9 @@ carp_del_addr(struct carp_softc *sc, str
imo->imo_multicast_ifp =3D NULL;
TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);
if (!--cif->vhif_nvrs) {
+ IF_ADDR_LOCK(sc->sc_carpdev);
sc->sc_carpdev->if_carp =3D NULL;
+ IF_ADDR_UNLOCK(sc->sc_carpdev);
CARP_LOCK_DESTROY(cif);
FREE(cif, M_IFADDR);
} else {
@@ -1651,7 +1664,9 @@ carp_set_addr6(struct carp_softc *sc, st
CARP_LOCK(cif);
cif->vhif_ifp =3D ifp;
TAILQ_INIT(&cif->vhif_vrs);
+ IF_ADDR_LOCK(ifp);
ifp->if_carp =3D cif;
+ IF_ADDR_UNLOCK(ifp);
=20
} else {
struct carp_softc *vr;
@@ -1739,8 +1754,10 @@ carp_del_addr6(struct carp_softc *sc, st
im6o->im6o_multicast_ifp =3D NULL;
TAILQ_REMOVE(&cif->vhif_vrs, sc, sc_list);
if (!--cif->vhif_nvrs) {
=2D CARP_LOCK_DESTROY(cif);
+ IF_ADDR_LOCK(sc->sc_carpdev);
sc->sc_carpdev->if_carp =3D NULL;
+ IF_ADDR_UNLOCK(sc->sc_carpdev);
+ CARP_LOCK_DESTROY(cif);
FREE(cif, M_IFADDR);
} else
CARP_UNLOCK(cif);
Index: netinet6/nd6_nbr.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /usr/store/mlaier/fcvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.41
diff -u -p -r1.41 nd6_nbr.c
=2D-- netinet6/nd6_nbr.c 4 Aug 2006 21:27:39 -0000 1.41
+++ netinet6/nd6_nbr.c 22 Aug 2006 16:51:49 -0000
@@ -194,8 +194,10 @@ nd6_ns_input(m, off, icmp6len)
*/
/* (1) and (3) check. */
#ifdef DEV_CARP
+ IF_ADDR_LOCK(ifp);
if (ifp->if_carp)
ifa =3D carp_iamatch6(ifp->if_carp, &taddr6);
+ IF_ADDR_UNLOCK(ifp);
if (ifa =3D=3D NULL)
ifa =3D (struct ifaddr *)in6ifa_ifpwithaddr(ifp, &taddr6);
#else
@@ -962,8 +964,10 @@ nd6_na_output(ifp, daddr6_0, taddr6, fla
*/
if (sdl0 =3D=3D NULL) {
#ifdef DEV_CARP
+ IF_ADDR_LOCK(ifp);
if (ifp->if_carp)
mac =3D carp_macmatch6(ifp->if_carp, m, taddr6);
+ IF_ADDR_UNLOCK(ifp);
if (mac =3D=3D NULL)
mac =3D nd6_ifptomac(ifp);
#else
--Boundary-01=_V+z6EKu4gVOwhPv--
--nextPart3648429.JpapihDmP6
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (FreeBSD)
iD8DBQBE6z+aXyyEoT62BG0RAjzdAJ92bk2DjDwev6+Ie2W90Ubf9CYI/ACfRVvu
f8c2DQc7HYxzYO0vYaARpWU=
=mJNS
-----END PGP SIGNATURE-----
--nextPart3648429.JpapihDmP6--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200608221932.10056.max>