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
_______________________________________________
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to