Skip site navigation (1)Skip section navigation (2)
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
--------------010303040404000907010805
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: 8bit

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.

--------------010303040404000907010805
Content-Type: text/plain; charset="UTF-8"; x-mac-type=0; x-mac-creator=0;
	name="if_detach.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="if_detach.patch"

>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)


--------------010303040404000907010805--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?53AA99AF.4020205>