IFF_DRV_OACTIVE handling in *_stop() for intel nic drivers ?

[Adrian Chadd]

... 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 at 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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"