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>