tech-net archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
missing KERNEL_LOCK ?
Hello,
as reported on current-users@ some time ago, I experience mbuf leaks
on a router running netbsd-6-1 of december. It's not a slow, continous leak
but a very sudden one:
in regular use netstat -m reports between 800 and 3000 mbufs in use,
and it can run for days. But very suddenly, in a few minutes the number of
mbufs can rise up to the limit (which I raised to 512k mbclusters).
I suspected it was associated with a specific traffic, eventually associated
with ipf. I suspected this because it seems it started to show up after
a ipf rules changes, which caused ipf to return icmp errors to ntp scans,
and MBUFTRACE showed that the lost mbufs were in output queues.
The attached patch does 2 things.
The first one is to add KASSERT(KERNEL_LOCKED_P()) at strategic places,
where we're about to touch the output queues. This is because the
output queues are protected by spl() calls only, so the kernel lock
must be held to protect them.
As the KASSERT() did fire, I then added KERNEL_LOCK()/UNLOCK() where needed.
I found that ifp_ouput() was being called without KERNEL_LOCK() from
ipf(4), and carp(4) so far.
I'm now running with this patch for 2 days without apparent problem, and
the leak didn't show up (but it's too soon to say if the issue is
fixed or not).
So, 2 questions:
- is my analysis right ?
- if so, should I commit the patch as is ?
--
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
NetBSD: 26 ans d'experience feront toujours la difference
--
Index: dist/ipf/netinet/ip_fil_netbsd.c
===================================================================
RCS file: /cvsroot/src/sys/dist/ipf/netinet/Attic/ip_fil_netbsd.c,v
retrieving revision 1.61.2.1
diff -u -r1.61.2.1 ip_fil_netbsd.c
--- dist/ipf/netinet/ip_fil_netbsd.c 8 Feb 2013 19:54:45 -0000 1.61.2.1
+++ dist/ipf/netinet/ip_fil_netbsd.c 7 Apr 2014 15:17:25 -0000
@@ -1377,11 +1377,13 @@
if (!ip->ip_sum)
ip->ip_sum = in_cksum(m, hlen);
# endif /* M_CSUM_IPv4 */
+ KERNEL_LOCK(1, NULL);
# if __NetBSD_Version__ >= 499001100
error = (*ifp->if_output)(ifp, m, dst, rt);
# else
error = (*ifp->if_output)(ifp, m, (struct sockaddr *)dst, rt);
# endif
+ KERNEL_UNLOCK_ONE(NULL);
if (i) {
ip->ip_len = ntohs(ip->ip_len);
ip->ip_off = ntohs(ip->ip_off);
@@ -1471,6 +1473,7 @@
for (m = m0; m; m = m0) {
m0 = m->m_act;
m->m_act = 0;
+ KERNEL_LOCK(1, NULL);
# if __NetBSD_Version__ >= 499001100
if (error == 0)
error = (*ifp->if_output)(ifp, m, dst, rt);
@@ -1483,6 +1486,7 @@
else
FREE_MB_T(m);
# endif
+ KERNEL_UNLOCK_ONE(NULL);
}
}
done:
Index: net/if_ethersubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ethersubr.c,v
retrieving revision 1.188.8.3
diff -u -r1.188.8.3 if_ethersubr.c
--- net/if_ethersubr.c 31 Oct 2012 16:07:46 -0000 1.188.8.3
+++ net/if_ethersubr.c 7 Apr 2014 15:17:27 -0000
@@ -222,6 +222,8 @@
struct at_ifaddr *aa;
#endif /* NETATALK */
+ KASSERT(KERNEL_LOCKED_P());
+
#ifdef MBUFTRACE
m_claimm(m, ifp->if_mowner);
#endif
Index: net/if_loop.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_loop.c,v
retrieving revision 1.75
diff -u -r1.75 if_loop.c
--- net/if_loop.c 20 Jun 2011 09:43:27 -0000 1.75
+++ net/if_loop.c 7 Apr 2014 15:17:27 -0000
@@ -221,7 +222,9 @@
struct ifqueue *ifq = NULL;
int csum_flags;
MCLAIM(m, ifp->if_mowner);
+ KASSERT(KERNEL_LOCKED_P());
+
if ((m->m_flags & M_PKTHDR) == 0)
panic("looutput: no header mbuf");
if (ifp->if_flags & IFF_LOOPBACK)
Index: net/if_vlan.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_vlan.c,v
retrieving revision 1.69
diff -u -r1.69 if_vlan.c
--- net/if_vlan.c 19 Oct 2011 22:07:09 -0000 1.69
+++ net/if_vlan.c 7 Apr 2014 15:17:27 -0000
@@ -681,6 +681,8 @@
int error;
ALTQ_DECL(struct altq_pktattr pktattr;)
+ KASSERT(KERNEL_LOCKED_P());
+
ifp->if_flags |= IFF_OACTIVE;
for (;;) {
Index: net/pfil.c
===================================================================
RCS file: /cvsroot/src/sys/net/pfil.c,v
retrieving revision 1.27
diff -u -r1.27 pfil.c
--- net/pfil.c 23 Jun 2008 03:13:12 -0000 1.27
+++ net/pfil.c 7 Apr 2014 15:17:27 -0000
@@ -82,6 +82,7 @@
if ((dir & PFIL_ALL) && mp)
*mp = m;
+ KASSERT(rv == 0 || mp == NULL || *mp == NULL);
return (rv);
}
Index: netinet/ip_carp.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/ip_carp.c,v
retrieving revision 1.47.4.1
diff -u -r1.47.4.1 ip_carp.c
--- netinet/ip_carp.c 2 Apr 2012 18:25:35 -0000 1.47.4.1
+++ netinet/ip_carp.c 7 Apr 2014 15:17:28 -0000
@@ -347,6 +356,7 @@
struct ifaddr *ifa;
int s;
+ KERNEL_LOCK(1, NULL);
s = splsoftnet();
IFADDR_FOREACH(ifa, &sc->sc_if) {
switch (ifa->ifa_addr->sa_family) {
@@ -444,6 +454,7 @@
}
}
splx(s);
+ KERNEL_UNLOCK_ONE(NULL);
}
/*
@@ -844,6 +884,7 @@
carp_setrun(sc, 0);
carp_multicast_cleanup(sc);
+ KERNEL_LOCK(1, NULL);
s = splnet();
if (sc->sc_carpdev != NULL) {
/* XXX linkstatehook removal */
@@ -857,6 +898,7 @@
}
sc->sc_carpdev = NULL;
splx(s);
+ KERNEL_UNLOCK_ONE(NULL);
}
/* Detach an interface from the carp. */
@@ -923,6 +965,7 @@
struct ifaddr *ifa;
struct sockaddr sa;
+ KERNEL_LOCK(1, NULL);
s = splsoftnet();
advbase = advskew = 0; /* Sssssh compiler */
@@ -1047,6 +1091,7 @@
/* XXX maybe less ? */
goto retry_later;
}
+ MCLAIM(m, &carp_proto6_mowner_tx);
len = sizeof(*ip6) + sizeof(ch);
m->m_pkthdr.len = len;
m->m_pkthdr.rcvif = NULL;
@@ -1123,6 +1168,7 @@
retry_later:
splx(s);
+ KERNEL_UNLOCK_ONE(NULL);
if (advbase != 255 || advskew != 255)
callout_schedule(&sc->sc_ad_tmo, tvtohz(&tv));
}
@@ -1137,8 +1183,10 @@
{
struct ifaddr *ifa;
struct in_addr *in;
- int s = splsoftnet();
+ int s;
+ KERNEL_LOCK(1, NULL);
+ s = splsoftnet();
IFADDR_FOREACH(ifa, &sc->sc_if) {
if (ifa->ifa_addr->sa_family != AF_INET)
@@ -1148,6 +1196,7 @@
arprequest(sc->sc_carpdev, in, in, CLLADDR(sc->sc_if.if_sadl));
}
splx(s);
+ KERNEL_UNLOCK_ONE(NULL);
}
#ifdef INET6
@@ -1157,7 +1206,10 @@
struct ifaddr *ifa;
struct in6_addr *in6;
static struct in6_addr mcast = IN6ADDR_LINKLOCAL_ALLNODES_INIT;
- int s = splsoftnet();
+ int s;
+
+ KERNEL_LOCK(1, NULL);
+ s = splsoftnet();
IFADDR_FOREACH(ifa, &sc->sc_if) {
@@ -1169,6 +1221,7 @@
ND_NA_FLAG_OVERRIDE, 1, NULL);
}
splx(s);
+ KERNEL_UNLOCK_ONE(NULL);
}
#endif /* INET6 */
@@ -1573,10 +1636,12 @@
if (sc->sc_naddrs || sc->sc_naddrs6)
sc->sc_if.if_flags |= IFF_UP;
carp_set_enaddr(sc);
+ KERNEL_LOCK(1, NULL);
s = splnet();
/* XXX linkstatehooks establish */
carp_carpdev_state(ifp);
splx(s);
+ KERNEL_UNLOCK_ONE(NULL);
} else {
carpdetach(sc);
sc->sc_if.if_flags &= ~(IFF_UP|IFF_RUNNING);
@@ -2035,6 +2105,7 @@
struct rtentry *rt)
{
struct carp_softc *sc = ((struct carp_softc *)ifp->if_softc);
+ KASSERT(KERNEL_LOCKED_P());
if (sc->sc_carpdev != NULL && sc->sc_state == MASTER) {
return (sc->sc_carpdev->if_output(ifp, m, sa, rt));
Home |
Main Index |
Thread Index |
Old Index