Date: Fri, 13 Dec 2013 13:52:44 -0800 From: Adrian Chadd <adrian@freebsd.org> To: Luigi Rizzo <rizzo@iet.unipi.it> Cc: Jack F Vogel <jfv@freebsd.org>, "net@freebsd.org" <net@freebsd.org> Subject: Re: IFF_DRV_OACTIVE handling in *_stop() for intel nic drivers ? Message-ID: <CAJ-Vmom8u9mw9h-Eq5EKvcwcpMKjNzdxcwVJLtg_jRfA0DWYxg@mail.gmail.com> In-Reply-To: <20131213212430.GA80282@onelab2.iet.unipi.it> References: <20131213212430.GA80282@onelab2.iet.unipi.it>
next in thread | previous in thread | raw e-mail | index | archive | help
... OACTIVE by design is racy. We should just not be using it in newer drivers. I think the correct thing to do is to just plain ignore setting it ever. -a On 13 December 2013 13:24, Luigi Rizzo <rizzo@iet.unipi.it> wrote: > I am trying to make sense of what is the intended handling of > if_drv_flags in the *_stop() and *_init() routines for the > intel nic drivers and i see three different patterns (below). > I think the correct one is ixgbe.c which leaves IFF_DRV_OACTIVE alone, > although one could argue that IFF_DRV_OACTIVE should be cleaned > up just in case (as done in if_lem.c). > What really seems wrong is setting the flag in the _stop() > function as it is done in if_em.c and if_igb.c . > > So which one should we pick ? > > cheers > luigi > > if_lem.c > lem_stop() > ... > /* Tell the stack that the interface is no longer active */ > ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE); > > lem_init_locked() > ... (near the end) > ifp->if_drv_flags |= IFF_DRV_RUNNING; > ifp->if_drv_flags &= ~IFF_DRV_OACTIVE; > > > if_em.c > em_stop() > ... > /* Tell the stack that the interface is no longer active */ > ifp->if_drv_flags &= ~IFF_DRV_RUNNING; > ifp->if_drv_flags |= IFF_DRV_OACTIVE; > > em_init_locked() > ... (near the end) > /* Set the interface as ACTIVE */ > ifp->if_drv_flags |= IFF_DRV_RUNNING; > ifp->if_drv_flags &= ~IFF_DRV_OACTIVE; > > if_igb.c > igb_stop() > ... > /* Tell the stack that the interface is no longer active */ > ifp->if_drv_flags &= ~IFF_DRV_RUNNING; > ifp->if_drv_flags |= IFF_DRV_OACTIVE; > > igb_init_locked() > ... (near the end) > ifp->if_drv_flags |= IFF_DRV_RUNNING; > ifp->if_drv_flags &= ~IFF_DRV_OACTIVE; > > ixgbe.c > ixgbe_stop() > ... > /* Let the stack know...*/ > ifp->if_drv_flags &= ~IFF_DRV_RUNNING; > > ixgbe_init_locked() > ... (at the very end) > /* Now inform the stack we're ready */ > ifp->if_drv_flags |= IFF_DRV_RUNNING; > _______________________________________________ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-Vmom8u9mw9h-Eq5EKvcwcpMKjNzdxcwVJLtg_jRfA0DWYxg>