Re: ld.so: -fno-builtin?
On Thu, Dec 22, 2016 at 12:33 PM, Joerg Sonnenberger wrote: > On Thu, Dec 22, 2016 at 05:47:05PM +0100, Christian Weisgerber wrote: >> Building ld.so with clang on amd64 fails with undefined references >> to memset and memcpy. That is odd, since neither function appears >> in the source. Apparently clang optimizes the _dl_memset and >> _dl_bcopy functions into calls to memset and memcpy, respectively. >> >> I tentatively propose to add -fno-builtin to fix this. >> (Builds, runs, and passes regress with gcc on amd64 and i386.) > > You are lucky if it fixes the problem, infact, -fno-builtin has quite a > chance to make the problem worse. Clang assumes the same set of basic > functions to be present even for -ffreestanding environments and can > lower e.g. initialisation of local variables to memset/memcpy. Yeah. I suspect the better answer is to a) convert _dl_bcopy() to _dl_memcpy() (it's misnamed anyway, as it doesn't behave like bcopy), and b) in a header that's #included everywhere in ld.so, use the gcc asm names extension to redirect memcpy and memset to _dl_memcpy and _dl_memset (and as long as you're doing that, mark them hidden...) I'm assuming clang handles asm names like gcc, such that declaring void *memcpy(void *__restrict, const void *__restrict, __size_t) __dso_hidden __asm("_dl_memcpy"); will make even internally generated calls go to _dl_memcpy instead. Philip Guenther
Re: ospf6d: handle interface MTU changes
On Wed, Dec 21, 2016 at 12:08:23PM +0100, Jeremie Courreges-Anglas wrote: > > Hi, > > After ospfd here's a diff to make ospf6d refresh his view of an > interface's MTU at runtime. This needs a fresh kernel. > > The parent should pass the IFINFO message to its children first, and > then decide to react to a possible interface change. Like for ospfd, > the engine runs the FSM only if the interface state has actually > changed. > > ok? > This works fine for me. Thank you! Remi
Interrupt race in NET_LOCK/NET_UNLOCK
NET_LOCK() should raise IPL before acquiring the lock, and NET_UNLOCK() should restore the level after releasing the lock. Otherwise, lock recursion can occur, most likely right after the splx(). An example: nd6_slowtimo <- NET_LOCK() timeout_run softclock softintr_dispatch dosoftint interrupt k_intr if_netisr <- NET_LOCK() taskq_thread OK? Index: sys/systm.h === RCS file: src/sys/sys/systm.h,v retrieving revision 1.120 diff -u -p -r1.120 systm.h --- sys/systm.h 19 Dec 2016 08:36:50 - 1.120 +++ sys/systm.h 23 Dec 2016 05:59:26 - @@ -297,14 +297,14 @@ extern struct rwlock netlock; #defineNET_LOCK(s) \ do { \ - rw_enter_write(&netlock); \ s = splsoftnet(); \ + rw_enter_write(&netlock); \ } while (0) -#defineNET_UNLOCK(s) \ +#defineNET_UNLOCK(s) \ do { \ - splx(s);\ rw_exit_write(&netlock);\ + splx(s);\ } while (0) #defineNET_ASSERT_LOCKED() \
Re: if attach/detach netlocks
On 22/12/16(Thu) 20:45, Mike Belopuhov wrote: > On Wed, Dec 21, 2016 at 13:06 +0100, Alexander Bluhm wrote: > > On Wed, Dec 21, 2016 at 12:45:37PM +0100, Mike Belopuhov wrote: > > > Anyways, OK for the diff below? > > > > OK bluhm@ > > > > > diff --git sys/net/if_pflow.c sys/net/if_pflow.c > > > index 0df0b69fd8a..8592d98a743 100644 > > > --- sys/net/if_pflow.c > > > +++ sys/net/if_pflow.c > > > @@ -265,14 +265,11 @@ pflow_clone_destroy(struct ifnet *ifp) > > > if (timeout_initialized(&sc->sc_tmo_tmpl)) > > > timeout_del(&sc->sc_tmo_tmpl); > > > pflow_flush(sc); > > > m_freem(sc->send_nam); > > > if (sc->so != NULL) { > > > - /* XXXSMP breaks atomicity */ > > > - rw_exit_write(&netlock); > > > error = soclose(sc->so); > > > - rw_enter_write(&netlock); > > > sc->so = NULL; > > > } > > > if (sc->sc_flowdst != NULL) > > > free(sc->sc_flowdst, M_DEVBUF, sc->sc_flowdst->sa_len); > > > if (sc->sc_flowsrc != NULL) > > I think this is what is required here. Works here, but YMMV. splnet() in a pseudo-driver seems completely wrong, you could get rid of it. > > diff --git sys/net/if_vxlan.c sys/net/if_vxlan.c > index e9bc1cb8305..dfb71cf9467 100644 > --- sys/net/if_vxlan.c > +++ sys/net/if_vxlan.c > @@ -178,13 +178,15 @@ int > vxlan_clone_destroy(struct ifnet *ifp) > { > struct vxlan_softc *sc = ifp->if_softc; > int s; > > + NET_LOCK(s); > s = splnet(); > vxlan_multicast_cleanup(ifp); > splx(s); > + NET_UNLOCK(s); > > vxlan_enable--; > LIST_REMOVE(sc, sc_entry); > > ifmedia_delete_instance(&sc->sc_media, IFM_INST_ANY);
Re: splassert: ip_output: want 1 have 0
> Date: Thu, 22 Dec 2016 14:56:43 +0100 > From: Martin Pieuchot > > On 22/12/16(Thu) 10:45, Martin Pieuchot wrote: > > On 22/12/16(Thu) 00:32, Mark Kettenis wrote: > > > splassert: ip_output: want 1 have 0 > > > Starting stack trace... > > > ip_output() at ip_output+0x7d > > > ipsp_process_done() at ipsp_process_done+0x2ad > > > esp_output_cb() at esp_output_cb+0x135 > > > taskq_thread() at taskq_thread+0x6c > > > end trace frame: 0x0, count: 253 > > > End of stack trace. > > > > > > This makes no sense to me since esp_output_cb() calls > > > ipsp_process_done() while at splsoftnet. What am I missing? > > > > It's a missing NET_LOCK(), right now we're abusing splassert() to find > > code paths where the lock is not held and should be. > > This should fix it, do you confirm? Yes, this fixes the problem. Will run with it for a bit and take a closer look tomorrow. > Index: netinet/ip_ah.c > === > RCS file: /cvs/src/sys/netinet/ip_ah.c,v > retrieving revision 1.123 > diff -u -p -r1.123 ip_ah.c > --- netinet/ip_ah.c 19 Sep 2016 18:09:22 - 1.123 > +++ netinet/ip_ah.c 22 Dec 2016 13:55:02 - > @@ -1219,7 +1219,7 @@ ah_output_cb(struct cryptop *crp) > return (EINVAL); > } > > - s = splsoftnet(); > + NET_LOCK(s); > > tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto); > if (tdb == NULL) { > @@ -1236,7 +1236,7 @@ ah_output_cb(struct cryptop *crp) > /* Reset the session ID */ > if (tdb->tdb_cryptoid != 0) > tdb->tdb_cryptoid = crp->crp_sid; > - splx(s); > + NET_UNLOCK(s); > return crypto_dispatch(crp); > } > free(tc, M_XDATA, 0); > @@ -1258,11 +1258,11 @@ ah_output_cb(struct cryptop *crp) > crypto_freereq(crp); > > err = ipsp_process_done(m, tdb); > - splx(s); > + NET_UNLOCK(s); > return err; > > baddone: > - splx(s); > + NET_UNLOCK(s); > > m_freem(m); > > Index: netinet/ip_esp.c > === > RCS file: /cvs/src/sys/netinet/ip_esp.c,v > retrieving revision 1.141 > diff -u -p -r1.141 ip_esp.c > --- netinet/ip_esp.c 19 Sep 2016 18:09:22 - 1.141 > +++ netinet/ip_esp.c 22 Dec 2016 13:55:30 - > @@ -1064,7 +1064,7 @@ esp_output_cb(struct cryptop *crp) > } > > > - s = splsoftnet(); > + NET_LOCK(s); > > tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto); > if (tdb == NULL) { > @@ -1081,7 +1081,7 @@ esp_output_cb(struct cryptop *crp) > /* Reset the session ID */ > if (tdb->tdb_cryptoid != 0) > tdb->tdb_cryptoid = crp->crp_sid; > - splx(s); > + NET_UNLOCK(s); > return crypto_dispatch(crp); > } > free(tc, M_XDATA, 0); > @@ -1098,11 +1098,11 @@ esp_output_cb(struct cryptop *crp) > > /* Call the IPsec input callback. */ > error = ipsp_process_done(m, tdb); > - splx(s); > + NET_UNLOCK(s); > return error; > > baddone: > - splx(s); > + NET_UNLOCK(s); > > m_freem(m); > > Index: netinet/ip_ipcomp.c > === > RCS file: /cvs/src/sys/netinet/ip_ipcomp.c,v > retrieving revision 1.48 > diff -u -p -r1.48 ip_ipcomp.c > --- netinet/ip_ipcomp.c 24 Sep 2016 14:51:37 - 1.48 > +++ netinet/ip_ipcomp.c 22 Dec 2016 13:54:18 - > @@ -554,7 +554,7 @@ ipcomp_output_cb(struct cryptop *crp) > return (EINVAL); > } > > - s = splsoftnet(); > + NET_LOCK(s); > > tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto); > if (tdb == NULL) { > @@ -571,7 +571,7 @@ ipcomp_output_cb(struct cryptop *crp) > /* Reset the session ID */ > if (tdb->tdb_cryptoid != 0) > tdb->tdb_cryptoid = crp->crp_sid; > - splx(s); > + NET_UNLOCK(s); > return crypto_dispatch(crp); > } > free(tc, M_XDATA, 0); > @@ -588,7 +588,7 @@ ipcomp_output_cb(struct cryptop *crp) > /* Compression was useless, we have lost time. */ > crypto_freereq(crp); > error = ipsp_process_done(m, tdb); > - splx(s); > + NET_UNLOCK(s); > return error; > } > > @@ -638,11 +638,11 @@ ipcomp_output_cb(struct cryptop *crp) > crypto_freereq(crp); > > error = ipsp_process_done(m, tdb); > - splx(s); > + NET_UNLOCK(s); > return error; > > baddone: > - splx(s); > + NET_UNLOCK(s); > > m_freem(m); > >
Re: ld.so: -fno-builtin?
On Thu, Dec 22, 2016 at 05:47:05PM +0100, Christian Weisgerber wrote: > Building ld.so with clang on amd64 fails with undefined references > to memset and memcpy. That is odd, since neither function appears > in the source. Apparently clang optimizes the _dl_memset and > _dl_bcopy functions into calls to memset and memcpy, respectively. > > I tentatively propose to add -fno-builtin to fix this. > (Builds, runs, and passes regress with gcc on amd64 and i386.) You are lucky if it fixes the problem, infact, -fno-builtin has quite a chance to make the problem worse. Clang assumes the same set of basic functions to be present even for -ffreestanding environments and can lower e.g. initialisation of local variables to memset/memcpy. Joerg
Re: if attach/detach netlocks
On Wed, Dec 21, 2016 at 13:06 +0100, Alexander Bluhm wrote: > On Wed, Dec 21, 2016 at 12:45:37PM +0100, Mike Belopuhov wrote: > > Anyways, OK for the diff below? > > OK bluhm@ > > > diff --git sys/net/if_pflow.c sys/net/if_pflow.c > > index 0df0b69fd8a..8592d98a743 100644 > > --- sys/net/if_pflow.c > > +++ sys/net/if_pflow.c > > @@ -265,14 +265,11 @@ pflow_clone_destroy(struct ifnet *ifp) > > if (timeout_initialized(&sc->sc_tmo_tmpl)) > > timeout_del(&sc->sc_tmo_tmpl); > > pflow_flush(sc); > > m_freem(sc->send_nam); > > if (sc->so != NULL) { > > - /* XXXSMP breaks atomicity */ > > - rw_exit_write(&netlock); > > error = soclose(sc->so); > > - rw_enter_write(&netlock); > > sc->so = NULL; > > } > > if (sc->sc_flowdst != NULL) > > free(sc->sc_flowdst, M_DEVBUF, sc->sc_flowdst->sa_len); > > if (sc->sc_flowsrc != NULL) I think this is what is required here. Works here, but YMMV. diff --git sys/net/if_vxlan.c sys/net/if_vxlan.c index e9bc1cb8305..dfb71cf9467 100644 --- sys/net/if_vxlan.c +++ sys/net/if_vxlan.c @@ -178,13 +178,15 @@ int vxlan_clone_destroy(struct ifnet *ifp) { struct vxlan_softc *sc = ifp->if_softc; int s; + NET_LOCK(s); s = splnet(); vxlan_multicast_cleanup(ifp); splx(s); + NET_UNLOCK(s); vxlan_enable--; LIST_REMOVE(sc, sc_entry); ifmedia_delete_instance(&sc->sc_media, IFM_INST_ANY);
Re: snmpd improvements
On Wed, Dec 21, 2016 at 10:40:48AM +0100, Franco Fichtner wrote: > Hi, > > Switching from net-snmp to OpenBSD's snmpd raised two > issues and I'd like to know if they make sense to address: > > A pid file is missing. Would a patch for this be accepted? Most probably not. We don't see the benefits in having pid files and prefer to use tools like pgrep to verify if a process is running. > The snmpd.conf can contain static values. If these values > are rewritten/changed over time by rewriting the config, > snmpd needs to be restarted. Is there a technical reason > for not supporting e.g. reload using HUP? This would be accepted for sure but will require a fair amount of work because of the privsep nature of the daemon. You need to pass the conifg to the childs and apply it there. Doing this will probably be appreciated. -- :wq Claudio
Re: ospf6d: handle interface MTU changes
On Wed, Dec 21, 2016 at 12:08:23PM +0100, Jeremie Courreges-Anglas wrote: > > Hi, > > After ospfd here's a diff to make ospf6d refresh his view of an > interface's MTU at runtime. This needs a fresh kernel. > > The parent should pass the IFINFO message to its children first, and > then decide to react to a possible interface change. Like for ospfd, > the engine runs the FSM only if the interface state has actually > changed. > > ok? Looks like what you did in ospfd. So OK claudio@ > > > Index: ospf6ctl/ospf6ctl.c > === > RCS file: /d/cvs/src/usr.sbin/ospf6ctl/ospf6ctl.c,v > retrieving revision 1.43 > diff -u -p -r1.43 ospf6ctl.c > --- ospf6ctl/ospf6ctl.c 5 Dec 2015 13:12:40 - 1.43 > +++ ospf6ctl/ospf6ctl.c 5 Dec 2016 22:40:53 - > @@ -409,9 +409,10 @@ show_interface_detail_msg(struct imsg *i > iface->name, print_link(iface->flags)); > printf(" Internet address %s Area %s\n", > log_in6addr(&iface->addr), inet_ntoa(iface->area)); > - printf(" Link type %s, state %s", > + printf(" Link type %s, state %s, mtu %d", > get_media_descr(get_ifms_type(iface->if_type)), > - get_linkstate(iface->if_type, iface->linkstate)); > + get_linkstate(iface->if_type, iface->linkstate), > + iface->mtu); > if (iface->linkstate != LINK_STATE_DOWN && > iface->baudrate > 0) { > printf(", "); > Index: ospf6d/kroute.c > === > RCS file: /d/cvs/src/usr.sbin/ospf6d/kroute.c,v > retrieving revision 1.48 > diff -u -p -r1.48 kroute.c > --- ospf6d/kroute.c 17 Jul 2015 20:12:38 - 1.48 > +++ ospf6d/kroute.c 20 Dec 2016 16:09:14 - > @@ -729,12 +729,6 @@ if_change(u_short ifindex, int flags, st > return; > } > > - isvalid = (iface->flags & IFF_UP) && > - LINK_STATE_IS_UP(iface->linkstate); > - > - if (wasvalid == isvalid) > - return; /* nothing changed wrt validity */ > - > /* inform engine and rde about state change if interface is used */ > if (iface->cflags & F_IFACE_CONFIGURED) { > main_imsg_compose_ospfe(IMSG_IFINFO, 0, iface, > @@ -742,6 +736,12 @@ if_change(u_short ifindex, int flags, st > main_imsg_compose_rde(IMSG_IFINFO, 0, iface, > sizeof(struct iface)); > } > + > + isvalid = (iface->flags & IFF_UP) && > + LINK_STATE_IS_UP(iface->linkstate); > + > + if (wasvalid == isvalid) > + return; /* nothing changed wrt validity */ > > /* update redistribute list */ > RB_FOREACH(kr, kroute_tree, &krt) { > Index: ospf6d/ospfe.c > === > RCS file: /d/cvs/src/usr.sbin/ospf6d/ospfe.c,v > retrieving revision 1.49 > diff -u -p -r1.49 ospfe.c > --- ospf6d/ospfe.c3 Sep 2016 10:25:36 - 1.49 > +++ ospf6d/ospfe.c21 Dec 2016 10:55:11 - > @@ -260,7 +260,7 @@ ospfe_dispatch_main(int fd, short event, > struct imsg imsg; > struct imsgev *iev = bula; > struct imsgbuf *ibuf = &iev->ibuf; > - int n, stub_changed, shut = 0; > + int n, stub_changed, shut = 0, isvalid, wasvalid; > unsigned int ifindex; > > if (event & EV_READ) { > @@ -293,11 +293,19 @@ ospfe_dispatch_main(int fd, short event, > if (iface == NULL) > fatalx("interface lost in ospfe"); > > + wasvalid = (iface->flags & IFF_UP) && > + LINK_STATE_IS_UP(iface->linkstate); > + > if_update(iface, ifp->mtu, ifp->flags, ifp->if_type, > ifp->linkstate, ifp->baudrate); > > - if ((iface->flags & IFF_UP) && > - LINK_STATE_IS_UP(iface->linkstate)) { > + isvalid = (iface->flags & IFF_UP) && > + LINK_STATE_IS_UP(iface->linkstate); > + > + if (wasvalid == isvalid) > + break; > + > + if (isvalid) { > if_fsm(iface, IF_EVT_UP); > log_warnx("interface %s up", iface->name); > } else { > > > -- > jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE -- :wq Claudio
ld.so: -fno-builtin?
Building ld.so with clang on amd64 fails with undefined references to memset and memcpy. That is odd, since neither function appears in the source. Apparently clang optimizes the _dl_memset and _dl_bcopy functions into calls to memset and memcpy, respectively. I tentatively propose to add -fno-builtin to fix this. (Builds, runs, and passes regress with gcc on amd64 and i386.) Index: Makefile === RCS file: /cvs/src/libexec/ld.so/Makefile,v retrieving revision 1.62 diff -u -p -r1.62 Makefile --- Makefile4 Jul 2016 21:15:06 - 1.62 +++ Makefile22 Dec 2016 16:27:33 - @@ -31,6 +31,7 @@ SRCS+=library.c .PATH: ${.CURDIR}/${MACHINE_CPU} DEBUG?=-g +CFLAGS += -fno-builtin CFLAGS += -Wall -Werror CFLAGS += -I${.CURDIR} -I${.CURDIR}/${MACHINE_CPU} \ -D'DEF_WEAK(x)=asm("")' -D'DEF_STRONG(x)=asm("")' \ -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: splassert: ip_output: want 1 have 0
On 22/12/16(Thu) 10:45, Martin Pieuchot wrote: > On 22/12/16(Thu) 00:32, Mark Kettenis wrote: > > splassert: ip_output: want 1 have 0 > > Starting stack trace... > > ip_output() at ip_output+0x7d > > ipsp_process_done() at ipsp_process_done+0x2ad > > esp_output_cb() at esp_output_cb+0x135 > > taskq_thread() at taskq_thread+0x6c > > end trace frame: 0x0, count: 253 > > End of stack trace. > > > > This makes no sense to me since esp_output_cb() calls > > ipsp_process_done() while at splsoftnet. What am I missing? > > It's a missing NET_LOCK(), right now we're abusing splassert() to find > code paths where the lock is not held and should be. This should fix it, do you confirm? Index: netinet/ip_ah.c === RCS file: /cvs/src/sys/netinet/ip_ah.c,v retrieving revision 1.123 diff -u -p -r1.123 ip_ah.c --- netinet/ip_ah.c 19 Sep 2016 18:09:22 - 1.123 +++ netinet/ip_ah.c 22 Dec 2016 13:55:02 - @@ -1219,7 +1219,7 @@ ah_output_cb(struct cryptop *crp) return (EINVAL); } - s = splsoftnet(); + NET_LOCK(s); tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto); if (tdb == NULL) { @@ -1236,7 +1236,7 @@ ah_output_cb(struct cryptop *crp) /* Reset the session ID */ if (tdb->tdb_cryptoid != 0) tdb->tdb_cryptoid = crp->crp_sid; - splx(s); + NET_UNLOCK(s); return crypto_dispatch(crp); } free(tc, M_XDATA, 0); @@ -1258,11 +1258,11 @@ ah_output_cb(struct cryptop *crp) crypto_freereq(crp); err = ipsp_process_done(m, tdb); - splx(s); + NET_UNLOCK(s); return err; baddone: - splx(s); + NET_UNLOCK(s); m_freem(m); Index: netinet/ip_esp.c === RCS file: /cvs/src/sys/netinet/ip_esp.c,v retrieving revision 1.141 diff -u -p -r1.141 ip_esp.c --- netinet/ip_esp.c19 Sep 2016 18:09:22 - 1.141 +++ netinet/ip_esp.c22 Dec 2016 13:55:30 - @@ -1064,7 +1064,7 @@ esp_output_cb(struct cryptop *crp) } - s = splsoftnet(); + NET_LOCK(s); tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto); if (tdb == NULL) { @@ -1081,7 +1081,7 @@ esp_output_cb(struct cryptop *crp) /* Reset the session ID */ if (tdb->tdb_cryptoid != 0) tdb->tdb_cryptoid = crp->crp_sid; - splx(s); + NET_UNLOCK(s); return crypto_dispatch(crp); } free(tc, M_XDATA, 0); @@ -1098,11 +1098,11 @@ esp_output_cb(struct cryptop *crp) /* Call the IPsec input callback. */ error = ipsp_process_done(m, tdb); - splx(s); + NET_UNLOCK(s); return error; baddone: - splx(s); + NET_UNLOCK(s); m_freem(m); Index: netinet/ip_ipcomp.c === RCS file: /cvs/src/sys/netinet/ip_ipcomp.c,v retrieving revision 1.48 diff -u -p -r1.48 ip_ipcomp.c --- netinet/ip_ipcomp.c 24 Sep 2016 14:51:37 - 1.48 +++ netinet/ip_ipcomp.c 22 Dec 2016 13:54:18 - @@ -554,7 +554,7 @@ ipcomp_output_cb(struct cryptop *crp) return (EINVAL); } - s = splsoftnet(); + NET_LOCK(s); tdb = gettdb(tc->tc_rdomain, tc->tc_spi, &tc->tc_dst, tc->tc_proto); if (tdb == NULL) { @@ -571,7 +571,7 @@ ipcomp_output_cb(struct cryptop *crp) /* Reset the session ID */ if (tdb->tdb_cryptoid != 0) tdb->tdb_cryptoid = crp->crp_sid; - splx(s); + NET_UNLOCK(s); return crypto_dispatch(crp); } free(tc, M_XDATA, 0); @@ -588,7 +588,7 @@ ipcomp_output_cb(struct cryptop *crp) /* Compression was useless, we have lost time. */ crypto_freereq(crp); error = ipsp_process_done(m, tdb); - splx(s); + NET_UNLOCK(s); return error; } @@ -638,11 +638,11 @@ ipcomp_output_cb(struct cryptop *crp) crypto_freereq(crp); error = ipsp_process_done(m, tdb); - splx(s); + NET_UNLOCK(s); return error; baddone: - splx(s); + NET_UNLOCK(s); m_freem(m);
Re: splassert: sowakeup/sorwakeup: want 1 have 0
On 22/12/16(Thu) 11:59, Stefan Sperling wrote: > Some additional stack traces which run through nd6: > > splassert: sowakeup: want 1 have 0 > Starting stack trace... > sowakeup(1,0,d09d8d63,d09c8abf,d03cfbb0) at sowakeup+0x43 > sowakeup(d95478ec,d954793c,d09d8be1,d378a4a0,d41150e0) at sowakeup+0x43 > sorwakeup(d95478ec,d0b57b50,d94bd000,0,2) at sorwakeup+0xbf > route_input(d94bd000,d0b57b60,d0b57b50,d0b57b40,d3939030) at route_input+0x26a > rt_sendaddrmsg(d96b463c,c,d411c100,f5d7ec58,0) at rt_sendaddrmsg+0xd4 > rt_ifa_add(d411c100,200404,d411c124,d05305b7,d9787d00) at rt_ifa_add+0x18d > rt_ifa_addlocal(d411c100,d411c100,1,d3939030,d9787d00) at rt_ifa_addlocal+0x7f > in6_update_ifa(d3939030,f5d7ee2c,0,d37ce034,d0bbdb80) at in6_update_ifa+0x2a1 > in6_ifadd(d4112600,1,20,d09d4a10,d4112614) at in6_ifadd+0x22f > nd6_addr_add(d4112600,f5d7ef68,d03bb060,f5d7ef90,d03a90e2) at > nd6_addr_add+0x124 Thanks for the report, this is fixed by the ND6 diff I just committed.
Re: splassert: sowakeup/sorwakeup/sonewconn: want 1 have 0
On 22/12/16(Thu) 12:00, Theo Buehler wrote: > Not sure if these have been reported. With a kernel freshly built from > sources this morning, my dmesg is full of these: > > splassert: sorwakeup: want 1 have 0 > Starting stack trace... > sorwakeup() at sorwakeup+0x3c > soisconnected() at soisconnected+0x6f > unp_connect2() at unp_connect2+0x62 > unp_connect() at unp_connect+0x22e > uipc_usrreq() at uipc_usrreq+0x27a > soconnect() at soconnect+0xbb > sys_connect() at sys_connect+0x1c2 > syscall() at syscall+0x27b Same regression reported by stsp@ and RD Trush, I reverted the workaround.
Re: splassert: sowakeup/sorwakeup/sowwakeup: want 1 have 0
On 22/12/16(Thu) 11:56, Stefan Sperling wrote: > Getting a lot of these with 'dhclient iwm0'. > > splassert: sowakeup: want 1 have 0 > Starting stack trace... > sowakeup(1,0,d09d8d63,d09c8abf,d03cfbb0) at sowakeup+0x43 > sowakeup(d90431fc,d9043288,d09d8bd7,d3be1c00,d978ad00,f61cbd78,d978ad00,d03e5d6f,d90431fc) > at sowakeup+0x43 > sowwakeup(d90431fc,d978ad00,0,4000,585baed1) at sowwakeup+0x94 > unp_connect2(d90431fc,d90432e0,3b9aca00,2,ff9c) at unp_connect2+0x62 > unp_connect(d90431fc,d9785a00,d969c8a8,0,0) at unp_connect+0x281 > uipc_usrreq(d90431fc,4,0,d9785a00,0) at uipc_usrreq+0x285 > soconnect(d90431fc,d9785a00,0,3,d0bbe320) at soconnect+0xba > sys_connect(d969c8a8,f61cbf5c,f61cbf7c,13,286) at sys_connect+0x1c1 > syscall() at syscall+0x250 That's a regression of the NFS boot workaround, I reverted it for the moment.
Re: ND6 and splsoftnet()
On Wed, Dec 21, 2016 at 07:51:03PM +0100, Martin Pieuchot wrote: > > This diff replaces the nd6_timer timeout and task with a timeout > > proc. > > Can we wait until the experiment is complete? Until then I'd like > to keep timeout_set_proc() only for timeout converted because they > need the NET_LOCK(). Fine. But let's do the other changes. Move timer initialisation to nd6_init() and call timeout_set() only once during init. Then I don't have to think about wether it is MP safe. ok? bluhm Index: netinet6/ip6_input.c === RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v retrieving revision 1.172 diff -u -p -r1.172 ip6_input.c --- netinet6/ip6_input.c20 Dec 2016 18:33:43 - 1.172 +++ netinet6/ip6_input.c22 Dec 2016 12:51:04 - @@ -119,7 +119,6 @@ struct niqueue ip6intrq = NIQUEUE_INITIA struct ip6stat ip6stat; -void ip6_init2(void *); int ip6_check_rh0hdr(struct mbuf *, int *); int ip6_hbhchcheck(struct mbuf *, int *, int *, int *); @@ -157,19 +156,8 @@ ip6_init(void) ip6_randomid_init(); nd6_init(); frag6_init(); - ip6_init2(NULL); mq_init(&ip6send_mq, 64, IPL_SOFTNET); -} - -void -ip6_init2(void *dummy) -{ - - /* nd6_timer_init */ - bzero(&nd6_timer_ch, sizeof(nd6_timer_ch)); - timeout_set(&nd6_timer_ch, nd6_timer, NULL); - timeout_add_sec(&nd6_timer_ch, 1); } /* Index: netinet6/nd6.c === RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v retrieving revision 1.199 diff -u -p -r1.199 nd6.c --- netinet6/nd6.c 20 Dec 2016 18:33:43 - 1.199 +++ netinet6/nd6.c 22 Dec 2016 12:59:31 - @@ -93,6 +93,8 @@ struct nd_prhead nd_prefix = { 0 }; int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL; void nd6_slowtimo(void *); +void nd6_timer_work(void *); +void nd6_timer(void *); void nd6_invalidate(struct rtentry *); struct llinfo_nd6 *nd6_free(struct rtentry *, int); void nd6_llinfo_timer(void *); @@ -100,7 +102,6 @@ void nd6_llinfo_timer(void *); struct timeout nd6_slowtimo_ch; struct timeout nd6_timer_ch; struct task nd6_timer_task; -void nd6_timer_work(void *); int fill_drlist(void *, size_t *, size_t); int fill_prlist(void *, size_t *, size_t); @@ -129,6 +130,8 @@ nd6_init(void) /* start timer */ timeout_set(&nd6_slowtimo_ch, nd6_slowtimo, NULL); timeout_add_sec(&nd6_slowtimo_ch, ND6_SLOWTIMER_INTERVAL); + timeout_set(&nd6_timer_ch, nd6_timer, NULL); + timeout_add_sec(&nd6_timer_ch, nd6_prune); nd6_rs_init(); } @@ -436,7 +439,6 @@ nd6_timer_work(void *null) struct in6_ifaddr *ia6, *nia6; s = splsoftnet(); - timeout_set(&nd6_timer_ch, nd6_timer, NULL); timeout_add_sec(&nd6_timer_ch, nd6_prune); /* expire default router list */ @@ -1482,7 +1484,6 @@ nd6_slowtimo(void *ignored_arg) struct nd_ifinfo *nd6if; struct ifnet *ifp; - timeout_set(&nd6_slowtimo_ch, nd6_slowtimo, NULL); timeout_add_sec(&nd6_slowtimo_ch, ND6_SLOWTIMER_INTERVAL); TAILQ_FOREACH(ifp, &ifnet, if_list) { nd6if = ND_IFINFO(ifp); Index: netinet6/nd6.h === RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.h,v retrieving revision 1.65 diff -u -p -r1.65 nd6.h --- netinet6/nd6.h 28 Nov 2016 13:59:51 - 1.65 +++ netinet6/nd6.h 22 Dec 2016 12:52:54 - @@ -223,8 +223,6 @@ extern int nd6_debug; #define nd6log(x) do { if (nd6_debug) log x; } while (0) -extern struct timeout nd6_timer_ch; - union nd_opts { struct nd_opt_hdr *nd_opt_array[9]; struct { @@ -260,7 +258,6 @@ int nd6_options(union nd_opts *); struct rtentry *nd6_lookup(struct in6_addr *, int, struct ifnet *, u_int); void nd6_setmtu(struct ifnet *); void nd6_llinfo_settimer(struct llinfo_nd6 *, int); -void nd6_timer(void *); void nd6_purge(struct ifnet *); void nd6_nud_hint(struct rtentry *); void nd6_rtrequest(struct ifnet *, int, struct rtentry *);
splassert: sowakeup/sorwakeup/sonewconn: want 1 have 0
Not sure if these have been reported. With a kernel freshly built from sources this morning, my dmesg is full of these: splassert: sorwakeup: want 1 have 0 Starting stack trace... sorwakeup() at sorwakeup+0x3c soisconnected() at soisconnected+0x6f unp_connect2() at unp_connect2+0x62 unp_connect() at unp_connect+0x22e uipc_usrreq() at uipc_usrreq+0x27a soconnect() at soconnect+0xbb sys_connect() at sys_connect+0x1c2 syscall() at syscall+0x27b --- syscall (number 98) --- end of kernel end trace frame: 0x1b469c13bec0, count: 249 0x1b46c11327da: End of stack trace. splassert: sowakeup: want 1 have 0 Starting stack trace... sowakeup() at sowakeup+0x41 sorwakeup() at sorwakeup+0xb4 soisconnected() at soisconnected+0x6f unp_connect2() at unp_connect2+0x62 unp_connect() at unp_connect+0x22e uipc_usrreq() at uipc_usrreq+0x27a soconnect() at soconnect+0xbb sys_connect() at sys_connect+0x1c2 syscall() at syscall+0x27b --- syscall (number 98) --- end of kernel end trace frame: 0x1b469c13bec0, count: 248 0x1b46c11327da: End of stack trace. splassert: sonewconn: want 1 have 0 Starting stack trace... sonewconn() at sonewconn+0x4e unp_connect() at unp_connect+0x17d uipc_usrreq() at uipc_usrreq+0x27a soconnect() at soconnect+0xbb sys_connect() at sys_connect+0x1c2 syscall() at syscall+0x27b --- syscall (number 98) --- end of kernel end trace frame: 0xf7cb6dd87e0, count: 251 0xf7c1ce8e7da: End of stack trace.
splassert: sowakeup/sorwakeup: want 1 have 0
Some additional stack traces which run through nd6: splassert: sowakeup: want 1 have 0 Starting stack trace... sowakeup(1,0,d09d8d63,d09c8abf,d03cfbb0) at sowakeup+0x43 sowakeup(d95478ec,d954793c,d09d8be1,d378a4a0,d41150e0) at sowakeup+0x43 sorwakeup(d95478ec,d0b57b50,d94bd000,0,2) at sorwakeup+0xbf route_input(d94bd000,d0b57b60,d0b57b50,d0b57b40,d3939030) at route_input+0x26a rt_sendaddrmsg(d96b463c,c,d411c100,f5d7ec58,0) at rt_sendaddrmsg+0xd4 rt_ifa_add(d411c100,200404,d411c124,d05305b7,d9787d00) at rt_ifa_add+0x18d rt_ifa_addlocal(d411c100,d411c100,1,d3939030,d9787d00) at rt_ifa_addlocal+0x7f in6_update_ifa(d3939030,f5d7ee2c,0,d37ce034,d0bbdb80) at in6_update_ifa+0x2a1 in6_ifadd(d4112600,1,20,d09d4a10,d4112614) at in6_ifadd+0x22f nd6_addr_add(d4112600,f5d7ef68,d03bb060,f5d7ef90,d03a90e2) at nd6_addr_add+0x124 taskq_thread(d0b56020) at taskq_thread+0x60 End of stack trace. splassert: sorwakeup: want 1 have 0 Starting stack trace... sorwakeup(1,0,d09d8be1,d378a4a0,d3cbcbe0) at sorwakeup+0x3f sorwakeup(d95478ec,d0b57b50,d94bd700,0,2) at sorwakeup+0x3f route_input(d94bd700,d0b57b60,d0b57b50,d0b57b40,d3939030) at route_input+0x26a rt_sendaddrmsg(d96b45a4,c,d4117f00,f5d7ec58,0) at rt_sendaddrmsg+0xd4 rt_ifa_add(d4117f00,200404,d4117f24,0,11) at rt_ifa_add+0x18d rt_ifa_addlocal(d4117f00,d4117f00,1,d041304e,f5d7ecf8) at rt_ifa_addlocal+0x7f in6_update_ifa(d3939030,f5d7ee2c,0,d37ce034,d0bbdb80) at in6_update_ifa+0x2a1 in6_ifadd(d4112600,0,20,d09d4a10,d4112614) at in6_ifadd+0x22f nd6_addr_add(d4112600,f5d7ef68,d03bb060,f5d7ef90,d03a90e2) at nd6_addr_add+0x102 taskq_thread(d0b56020) at taskq_thread+0x60 End of stack trace.
splassert: sowakeup/sorwakeup/sowwakeup: want 1 have 0
Getting a lot of these with 'dhclient iwm0'. splassert: sowakeup: want 1 have 0 Starting stack trace... sowakeup(1,0,d09d8d63,d09c8abf,d03cfbb0) at sowakeup+0x43 sowakeup(d90431fc,d9043288,d09d8bd7,d3be1c00,d978ad00,f61cbd78,d978ad00,d03e5d6f,d90431fc) at sowakeup+0x43 sowwakeup(d90431fc,d978ad00,0,4000,585baed1) at sowwakeup+0x94 unp_connect2(d90431fc,d90432e0,3b9aca00,2,ff9c) at unp_connect2+0x62 unp_connect(d90431fc,d9785a00,d969c8a8,0,0) at unp_connect+0x281 uipc_usrreq(d90431fc,4,0,d9785a00,0) at uipc_usrreq+0x285 soconnect(d90431fc,d9785a00,0,3,d0bbe320) at soconnect+0xba sys_connect(d969c8a8,f61cbf5c,f61cbf7c,13,286) at sys_connect+0x1c1 syscall() at syscall+0x250 --- syscall (number 2130164170) --- 0x7ef7bb40: End of stack trace. splassert: sorwakeup: want 1 have 0 Starting stack trace... sorwakeup(1,0,d09d8be1,d09c8abf,d03cfbb0) at sorwakeup+0x3f sorwakeup(d93a8e54,d90432e0,1,d3be1c00,d978ad00) at sorwakeup+0x3f soisconnected(d90432e0,d978ad00,0,4000,585baed1) at soisconnected+0x7c unp_connect2(d90431fc,d90432e0,3b9aca00,2,ff9c) at unp_connect2+0x6a unp_connect(d90431fc,d9785a00,d969c8a8,0,0) at unp_connect+0x281 uipc_usrreq(d90431fc,4,0,d9785a00,0) at uipc_usrreq+0x285 soconnect(d90431fc,d9785a00,0,3,d0bbe320) at soconnect+0xba sys_connect(d969c8a8,f61cbf5c,f61cbf7c,13,286) at sys_connect+0x1c1 syscall() at syscall+0x250 --- syscall (number 2130164170) --- 0x7ef7bb40: End of stack trace. splassert: sowwakeup: want 1 have 0 Starting stack trace... sowwakeup(1,0,d09d8bd7,d3be1c00,d978ad00) at sowwakeup+0x3f sowwakeup(d90431fc,d978ad00,0,4000,585baed1) at sowwakeup+0x3f unp_connect2(d90431fc,d90432e0,3b9aca00,2,ff9c) at unp_connect2+0x62 unp_connect(d90431fc,d9785a00,d969c8a8,0,0) at unp_connect+0x281 uipc_usrreq(d90431fc,4,0,d9785a00,0) at uipc_usrreq+0x285 soconnect(d90431fc,d9785a00,0,3,d0bbe320) at soconnect+0xba sys_connect(d969c8a8,f61cbf5c,f61cbf7c,13,286) at sys_connect+0x1c1 syscall() at syscall+0x250 --- syscall (number 2130164170) --- 0x7ef7bb40: End of stack trace.
Re: [patch]: Make ip4 mcast membership data structure similar to ip6
On Tue, Dec 13, 2016 at 12:35:07PM +0100, Martin Pieuchot wrote: > Hello, > > On 10/12/16(Sat) 11:08, Dimitris Papastamos wrote: > > While I was playing around with the ip4 multicast code, I thought > > I would attempt to make the membership data structure similar to that > > of ip6. This means changing from a dynamic array to a linked list. > > I like this a lot. Coherency is good. > > > The max membership limit has been lifted (I did not see a similar limit > > in the ip6 code). I am not sure if this is appropriate for ip4 though. > > I can add the limit back if necessary. > > I can't think of a reason why it was there in the first place. > > > My system seems to work with this patch but I have not explicitly > > tested all the code paths I have modified. I am working on some small > > tests to prove that it works. > > carp(4), pfsync(4) and vxlan(4) as well as ports using multicast like > avahi-daemon should be tested. Updated diff based on your comments below. I am still testing this though. If anyone can help with the testing, it would be great! diff --git a/share/man/man4/ip.4 b/share/man/man4/ip.4 index 82ad06c..607c500 100644 --- a/share/man/man4/ip.4 +++ b/share/man/man4/ip.4 @@ -431,10 +431,6 @@ the host is multihomed. Membership is associated with a single interface; programs running on multihomed hosts may need to join the same group on more than one interface. -Up to -.Dv IP_MAX_MEMBERSHIPS -(currently 4095) memberships may be added on a -single socket. .Pp To drop a membership, use: .Bd -literal -offset indent diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c index 117d03f..079a593 100644 --- a/sys/net/if_pfsync.c +++ b/sys/net/if_pfsync.c @@ -313,10 +313,7 @@ pfsync_clone_create(struct if_clone *ifc, int unit) sc->sc_len = PFSYNC_MINPKT; sc->sc_maxupdates = 128; - sc->sc_imo.imo_membership = (struct in_multi **)malloc( - (sizeof(struct in_multi *) * IP_MIN_MEMBERSHIPS), M_IPMOPTS, - M_WAITOK | M_ZERO); - sc->sc_imo.imo_max_memberships = IP_MIN_MEMBERSHIPS; + LIST_INIT(&sc->sc_imo.imo_memberships); ifp = &sc->sc_if; snprintf(ifp->if_xname, sizeof ifp->if_xname, "pfsync%d", unit); @@ -378,7 +375,6 @@ pfsync_clone_destroy(struct ifnet *ifp) } pool_destroy(&sc->sc_pool); - free(sc->sc_imo.imo_membership, M_IPMOPTS, 0); free(sc, M_DEVBUF, sizeof(*sc)); pfsyncif = NULL; @@ -1319,11 +1315,14 @@ pfsyncioctl(struct ifnet *ifp, u_long cmd, caddr_t data) sc->sc_sync_if->if_linkstatehooks, sc->sc_lhcookie); sc->sc_sync_if = NULL; - if (imo->imo_num_memberships > 0) { - in_delmulti(imo->imo_membership[ - --imo->imo_num_memberships]); - imo->imo_ifidx = 0; + if (!LIST_EMPTY(&imo->imo_memberships)) { + struct in_multi_mship *imm = + LIST_FIRST(&imo->imo_memberships); + + LIST_REMOVE(imm, imm_chain); + in_leavegroup(imm); } + imo->imo_ifidx = 0; splx(s); break; } @@ -1345,13 +1344,18 @@ pfsyncioctl(struct ifnet *ifp, u_long cmd, caddr_t data) sc->sc_lhcookie); sc->sc_sync_if = sifp; - if (imo->imo_num_memberships > 0) { - in_delmulti(imo->imo_membership[--imo->imo_num_memberships]); - imo->imo_ifidx = 0; + if (!LIST_EMPTY(&imo->imo_memberships)) { + struct in_multi_mship *imm = + LIST_FIRST(&imo->imo_memberships); + + LIST_REMOVE(imm, imm_chain); + in_leavegroup(imm); } + imo->imo_ifidx = 0; if (sc->sc_sync_if && sc->sc_sync_peer.s_addr == INADDR_PFSYNC_GROUP) { + struct in_multi_mship *imm; struct in_addr addr; if (!(sc->sc_sync_if->if_flags & IFF_MULTICAST)) { @@ -1362,16 +1366,16 @@ pfsyncioctl(struct ifnet *ifp, u_long cmd, caddr_t data) addr.s_addr = INADDR_PFSYNC_GROUP; - if ((imo->imo_membership[0] = - in_addmulti(&addr, sc->sc_sync_if)) == NULL) { + if ((imm = in_joingroup(&addr, + sc->sc_sync_if)) == NULL) { sc->sc_sync_if = NULL; splx(s); return (ENOBUFS); } - imo->imo_num_memberships++;
Re: [patch] turn igmpstat into a set of percpu counters
On Sun, Dec 11, 2016 at 07:24:40PM +, Dimitris Papastamos wrote: > Hi, > > I converted the igmp stats to use percpu counters. This work is > basically the same as what dlg@ did for other parts of the stack. > I looked at the diff and adjusted it for igmp. ping
Re: splassert: ip_output: want 1 have 0
On 22/12/16(Thu) 00:32, Mark Kettenis wrote: > splassert: ip_output: want 1 have 0 > Starting stack trace... > ip_output() at ip_output+0x7d > ipsp_process_done() at ipsp_process_done+0x2ad > esp_output_cb() at esp_output_cb+0x135 > taskq_thread() at taskq_thread+0x6c > end trace frame: 0x0, count: 253 > End of stack trace. > > This makes no sense to me since esp_output_cb() calls > ipsp_process_done() while at splsoftnet. What am I missing? It's a missing NET_LOCK(), right now we're abusing splassert() to find code paths where the lock is not held and should be.