Date: Wed, 25 Jun 2014 11:43:11 +0200 From: =?ISO-8859-15?Q?Roger_Pau_Monn=E9?= <roger.pau@citrix.com> To: John Baldwin <jhb@freebsd.org>, <freebsd-net@freebsd.org> Cc: "Alexander V. Chernikov" <melifaro@freebsd.org> Subject: Re: Ordering problem in if_detach_internal regarding if_bridge Message-ID: <53AA99AF.4020205@citrix.com> In-Reply-To: <201406241306.40064.jhb@freebsd.org> References: <53A4527F.90008@citrix.com> <53A85A8D.4090208@FreeBSD.org> <53A86016.5000102@citrix.com> <201406241306.40064.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On 24/06/14 19:06, John Baldwin wrote:
> On Monday, June 23, 2014 1:12:54 pm Roger Pau Monné wrote:
>> 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)
>
> I think this certainly suggests moving at least the eventhandler up so that
> things like vlans and bridges can detach from an interface while it is still
> constructed. I do think it would be ideal to move all three notifications
> to the same place though. (So your original patch plus moving the
> routing socket message.)
OK, I have updated the patch, now the devctl_notify is also done in the
same block. Let me know what you think about it.
Roger.
[-- Attachment #2 --]
>From a3fdee18fa3f0d8707bfbf2d9324e3103e1cede8 Mon Sep 17 00:00:00 2001
From: Roger Pau Monne <roger.pau@citrix.com>
Date: Fri, 20 Jun 2014 15:16:58 +0200
Subject: [PATCH]
---
sys/net/if.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/sys/net/if.c b/sys/net/if.c
index 47b5b9d..1c4bcab 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -875,6 +875,12 @@ if_detach_internal(struct ifnet *ifp, int vmove)
#endif
if_purgemaddrs(ifp);
+ /* Announce that the interface is gone. */
+ rt_ifannouncemsg(ifp, IFAN_DEPARTURE);
+ EVENTHANDLER_INVOKE(ifnet_departure_event, ifp);
+ if (IS_DEFAULT_VNET(curvnet))
+ devctl_notify("IFNET", ifp->if_xname, "DETACH", NULL);
+
if (!vmove) {
/*
* Prevent further calls into the device driver via ifnet.
@@ -912,11 +918,6 @@ if_detach_internal(struct ifnet *ifp, int vmove)
}
}
- /* Announce that the interface is gone. */
- rt_ifannouncemsg(ifp, IFAN_DEPARTURE);
- EVENTHANDLER_INVOKE(ifnet_departure_event, ifp);
- if (IS_DEFAULT_VNET(curvnet))
- devctl_notify("IFNET", ifp->if_xname, "DETACH", NULL);
if_delgroups(ifp);
/*
--
1.7.7.5 (Apple Git-26)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?53AA99AF.4020205>