Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 23 Jul 2020 09:00:09 +0000
From:      "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net>
To:        "Kristof Provost" <kp@FreeBSD.org>
Cc:        "John-Mark Gurney" <jmg@funkthat.com>, freebsd-net@freebsd.org, freebsd-current@freebsd.org
Subject:   Re: somewhat reproducable vimage panic
Message-ID:  <8B72C0B9-9CF0-4557-81D7-77190775805C@lists.zabbadoz.net>
In-Reply-To: <6847FB6B-0B1A-43C7-B567-15BF21AB5D56@FreeBSD.org>
References:  <20200721091654.GC4213@funkthat.com> <20200721113153.42d83119@x23> <20200721202323.GE4213@funkthat.com> <38F5A3A6-B578-4BA4-8F69-C248163CB6E0@libassi.se> <20200722060514.GF4213@funkthat.com> <20200722193443.GG4213@funkthat.com> <6C149617-55BB-4A87-B993-195E5E133790@lists.zabbadoz.net> <20200722221509.GI4213@funkthat.com> <2FFC49F9-83DE-4FA1-A47F-1D8A7AF4B241@FreeBSD.org> <6847FB6B-0B1A-43C7-B567-15BF21AB5D56@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 23 Jul 2020, at 8:09, Kristof Provost wrote:

> On 23 Jul 2020, at 9:19, Kristof Provost wrote:
>> On 23 Jul 2020, at 0:15, John-Mark Gurney wrote:
>>> So, it's pretty easy to trigger, just attach a couple USB ethernet
>>> adapters, in my case, they were ure, but likely any two spare 
>>> ethernet
>>> interfaces will work, and wire them back to back..
>>>
>> I’ve been able to trigger it using epair as well:
>>
>> `sudo sh testinterfaces.txt epair0a epair0b`
>>
>> I did have to comment out the waitcarrier() check.
>>
> I’ve done a little bit of digging, and I think I’m starting to see 
> how this breaks.
>
> This always affects the jailed vlan interfaces. They’re getting 
> deleted, but the ifp doesn’t go away just yet because it’s still 
> in use by the multicast code.
> The multicast code does its cleanup in task queues,

Wow, did I miss that back then? Did I review a change and not notice? 
Sorry if that was the case.

Vnet teardown is blocking and forceful.
Doing deferred cleanup work isn’t a good idea at all.
I think that is the real problem here.

I’d rather have us fix this than putting more bandaids into the code.

/bz

PS:  I love that you can repro this with epairs, means we can write a 
generic test code piece for this and commit it.


> so by the time it gets around to doing that the ifp is already marked 
> as dying and the vnet is gone.
> There are still references to the ifp though, and when the multicast 
> code tries to do its cleanup we get the panic.
>
> This hack stops the panic for me, but I don’t know if this is the 
> best solution:
>
> 	diff --git a/sys/net/if.c b/sys/net/if.c
> 	index 59dd38267cf..bd0c87eddf1 100644
> 	--- a/sys/net/if.c
> 	+++ b/sys/net/if.c
> 	@@ -3681,6 +3685,10 @@ if_delmulti_ifma_flags(struct ifmultiaddr 
> *ifma, int flags)
> 	 			ifp = NULL;
> 	 	}
> 	 #endif
> 	+
> 	+	if (ifp && ifp->if_flags & IFF_DYING)
> 	+		return;
> 	+
> 	 	/*
> 	 	 * If and only if the ifnet instance exists: Acquire the address 
> lock.
> 	 	 */
> 	diff --git a/sys/netinet/in_mcast.c b/sys/netinet/in_mcast.c
> 	index 39fc82c5372..6493e2a5bfb 100644
> 	--- a/sys/netinet/in_mcast.c
> 	+++ b/sys/netinet/in_mcast.c
> 	@@ -623,7 +623,7 @@ inm_release(struct in_multi *inm)
>
> 	 	/* XXX this access is not covered by IF_ADDR_LOCK */
> 	 	CTR2(KTR_IGMPV3, "%s: purging ifma %p", __func__, ifma);
> 	-	if (ifp != NULL) {
> 	+	if (ifp != NULL && (ifp->if_flags & IFF_DYING) == 0) {
> 	 		CURVNET_SET(ifp->if_vnet);
> 	 		inm_purge(inm);
> 	 		free(inm, M_IPMADDR);
>
> Best regards,
> Kristof



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8B72C0B9-9CF0-4557-81D7-77190775805C>