From owner-freebsd-net@FreeBSD.ORG  Fri Dec 13 21:31:07 2013
Return-Path: <owner-freebsd-net@FreeBSD.ORG>
Delivered-To: net@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org
 [IPv6:2001:1900:2254:206a::19:1])
 (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by hub.freebsd.org (Postfix) with ESMTPS id 278786A;
 Fri, 13 Dec 2013 21:31:07 +0000 (UTC)
Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238])
 by mx1.freebsd.org (Postfix) with ESMTP id E147A1FB4;
 Fri, 13 Dec 2013 21:31:06 +0000 (UTC)
Received: by onelab2.iet.unipi.it (Postfix, from userid 275)
 id 87A437300A; Fri, 13 Dec 2013 22:24:30 +0100 (CET)
Date: Fri, 13 Dec 2013 22:24:30 +0100
From: Luigi Rizzo <rizzo@iet.unipi.it>
To: net@freebsd.org
Subject: IFF_DRV_OACTIVE handling in *_stop() for intel nic drivers ?
Message-ID: <20131213212430.GA80282@onelab2.iet.unipi.it>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.5.20 (2009-06-14)
Cc: jfv@freebsd.org
X-BeenThere: freebsd-net@freebsd.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: Networking and TCP/IP with FreeBSD <freebsd-net.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-net>,
 <mailto:freebsd-net-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-net/>
List-Post: <mailto:freebsd-net@freebsd.org>
List-Help: <mailto:freebsd-net-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>,
 <mailto:freebsd-net-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 13 Dec 2013 21:31:07 -0000

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;