Date: Mon, 23 Jun 2014 20:39:56 +0400 From: "Alexander V. Chernikov" <melifaro@FreeBSD.org> To: John Baldwin <jhb@freebsd.org>, freebsd-net@freebsd.org Cc: =?ISO-8859-15?Q?Roger_Pau_Monn=E9?= <roger.pau@citrix.com> Subject: Re: Ordering problem in if_detach_internal regarding if_bridge Message-ID: <53A8585C.3080000@FreeBSD.org> In-Reply-To: <201406231132.47487.jhb@freebsd.org> References: <53A4527F.90008@citrix.com> <201406231132.47487.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 23.06.2014 19:32, John Baldwin wrote: > On Friday, June 20, 2014 11:25:51 am Roger Pau Monné wrote: >> Hello, >> >> I've stumbled across the following panic when testing Xen netback with >> if_bridge: >> >> Kernel page fault with the following non-sleepable locks held: >> exclusive sleep mutex if_bridge (if_bridge) r = 0 (0xfffff80006306c18) > locked @ /usr/src/sys/m >> KDB: stack backtrace: >> X_db_symbol_values() at X_db_symbol_values+0x10b/frame 0xfffffe0000213490 >> kdb_backtrace() at kdb_backtrace+0x39/frame 0xfffffe0000213540 >> witness_warn() at witness_warn+0x4a8/frame 0xfffffe0000213600 >> trap() at trap+0xc9d/frame 0xfffffe00002136a0 >> trap() at trap+0x669/frame 0xfffffe00002138b0 >> calltrap() at calltrap+0x8/frame 0xfffffe00002138b0 >> --- trap 0xc, rip = 0xffffffff8221a0ef, rsp = 0xfffffe0000213970, rbp = > 0xfffffe00002139e0 --- >> bridge_input() at bridge_input+0x5ff/frame 0xfffffe00002139e0 >> ether_vlanencap() at ether_vlanencap+0x4a3/frame 0xfffffe0000213a10 >> netisr_dispatch_src() at netisr_dispatch_src+0x90/frame 0xfffffe0000213a80 >> ether_ifattach() at ether_ifattach+0x19f/frame 0xfffffe0000213ab0 >> ath_dfs_get_thresholds() at ath_dfs_get_thresholds+0x81ce/frame > 0xfffffe0000213b30 >> intr_event_execute_handlers() at intr_event_execute_handlers+0x93/frame > 0xfffffe0000213b70 >> db_dump_intr_event() at db_dump_intr_event+0x796/frame 0xfffffe0000213bb0 >> fork_exit() at fork_exit+0x84/frame 0xfffffe0000213bf0 >> fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe0000213bf0 >> --- trap 0, rip = 0, rsp = 0xfffffe0000213cb0, rbp = 0 --- >> >> I've tracked this down to if_detach_internal setting ifp->if_addr to >> NULL before calling EVENTHANDLER_INVOKE(ifnet_departure_event..., which >> causes a panic in GRAB_OUR_PACKETS in the if_bridge code when it tries >> to perform IF_LLADDR on an interface that's in the process of being >> destroyed (ifp->if_addr set to NULL, but the ifnet_departure_event event >> has not fired yet). >> >> I have the following naive patch that moves the firing of the event >> before if_addr is set to NULL, but I'm not familiar with the ordering >> in if_detach_internal, so I'm not sure if this might cause problems in >> other parts of the code, could someone familiar with the net stuff >> comment on the best way to deal with it? We should notify kernel customers only when we are really taking this interface down and every other subsystem cannot add any new state to the interface. In this patch you're sending notification before taking ifnet down, removing its L3 addresses, routes, and so on. This can easily lead to panic in, for example, BPF subsystem (since BPF state is freed in bpf_ifdetach() handler). Addintionally, this will introduce ifaddr / iface messages reversal for rtsock. It looks like we'd better fix if_bridge (and it is still using mutexes, what a shame!). Can you send me trace with line numbers? > > Hmmm, I have no idea if this is ok or not. I do think the route message > should go out at the same time as the devctl_notify() call however. My guess > is it is actually better to do this earlier so that we allow outside consumers > to detach from an interface before it is destroyed. I'm not sure if it would > break things, but I would be tempted to move this even earlier right after it > is removed from the global ifnet list but before the taskqueue_drain, etc. >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?53A8585C.3080000>