Date: Mon, 23 Jun 2014 19:12:54 +0200 From: =?ISO-8859-15?Q?Roger_Pau_Monn=E9?= <roger.pau@citrix.com> To: "Alexander V. Chernikov" <melifaro@FreeBSD.org>, John Baldwin <jhb@freebsd.org>, <freebsd-net@freebsd.org> Subject: Re: Ordering problem in if_detach_internal regarding if_bridge Message-ID: <53A86016.5000102@citrix.com> In-Reply-To: <53A85A8D.4090208@FreeBSD.org> References: <53A4527F.90008@citrix.com> <201406231132.47487.jhb@freebsd.org> <53A8585C.3080000@FreeBSD.org> <53A85A8D.4090208@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 23/06/14 18:49, Alexander V. Chernikov wrote: > On 23.06.2014 20:39, Alexander V. Chernikov wrote: >> 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. > Whoops. I misread the patch. > It should be OK. > >> >> 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? > However, these two still stands. > (And I'm wondering how you're getting any traffic on down/dying interface). I'm not getting the traffic from the dying interface, I'm getting the traffic from another interface on the bridge (a physical bce interface), which injects traffic into the bridge, that calls bridge_input, which tries to read ifp->if_addr->ifa_addr from the dying interface, and that leads to the panic. Line numbers: /usr/src/sys/modules/if_bridge/../../net/if_bridge.c:2410 (bridge_input) /usr/src/sys/net/if_ethersubr.c:543 (ether_input_internal) /usr/src/sys/net/netisr.c:972 (netisr_dispatch_src) /usr/src/sys/net/if_ethersubr.c:674 (ether_input) /usr/src/sys/dev/bce/if_bce.c:6861 (bce_rx_intr) Roger.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?53A86016.5000102>