Date: Thu, 19 Nov 2009 17:01:33 -0500 From: John Baldwin <jhb@freebsd.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: arm@freebsd.org Subject: Re: [PATCH] Fix a few nits in ate(4) Message-ID: <200911191701.33852.jhb@freebsd.org> In-Reply-To: <20091119.141917.-1843205663.imp@bsdimp.com> References: <200911191122.02975.jhb@freebsd.org> <20091119.141917.-1843205663.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 19 November 2009 4:19:17 pm M. Warner Losh wrote:
> In message: <200911191122.02975.jhb@freebsd.org>
> John Baldwin <jhb@FreeBSD.org> writes:
> : @@ -1109,11 +1105,9 @@
> : & (IFF_PROMISC | IFF_ALLMULTI)) != 0)
> : ate_rxfilter(sc);
> : } else {
> : - if ((sc->flags & ATE_FLAG_DETACHING) == 0)
> : - ateinit_locked(sc);
> : + ateinit_locked(sc);
>
> Here we reinitialize the device just before we detach it. I put the
> detaching stuff in to prevent that. With this change, are you saying
> this routine won't be called, so we don't need this check anymore?
Basically, yes. More to the point, the previous order of calling atestop()
before ether_ifdetach() opened up a race wherein userland (or bpf_detach()
within if_detach()) could cause this to get invoked after atestop() had been
called. Now that we call ether_ifdetach() first, we know that neither bpf
nor userland is going to mess with the device anymore, and at that time we
can safely call atestop(). That now removes the need for doing this check.
> : }
> : } else if ((drv_flags & IFF_DRV_RUNNING) != 0) {
> : - ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
> : atestop(sc);
> : }
> : sc->if_flags = flags;
>
> Why remove this line? I think it is kinda needed. I don't understand.
It is a duplicate of the first line of atestop().
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200911191701.33852.jhb>