Subject: Re: sys/dev/ic/an.c:an_ioctl, SIOCSIFFLAGS case
To: None <tech-net@netbsd.org>
From: der Mouse <mouse@Rodents.Montreal.QC.CA>
List: tech-net
Date: 05/24/2001 04:42:52
> in an driver, what is the intention of the following code?
> it seems odd to me that it takes & between new if_flags value and old
> value.  i guess it should be ifp->if_flags only.

Well, I didn't write it, but it seems fairly clear to me:

> 		if ((ifp->if_flags & sc->an_if_flags &
> 		    (IFF_UP | IFF_RUNNING)) == (IFF_UP | IFF_RUNNING)) {

If we used to be up-and-running and we still will be up-and-running:

> 			if (ifp->if_flags & IFF_PROMISC &&
> 			    !(sc->an_if_flags & IFF_PROMISC)) {

If we are going to be PROMISC and we used to be !PROMISC,

> 				an_promisc(sc, 1);

turn promiscuous mode on.

> 				break;
> 			}
> 			if (!(ifp->if_flags & IFF_PROMISC) &&
> 			    sc->an_if_flags & IFF_PROMISC) {

If we are going to be !PROMISC and we used to be PROMISC,

> 				an_promisc(sc, 0);

turn promiscuous mode off.

> 				break;
> 			}
> 		}

The outer test is, it seems to me, exactly what it should be, on the
assumption that there's no point calling an_promisc() if we are coming
up, going down, or staying down - only if we were up and are staying
up.  Conceptually, that's ((ifp->if_flags&(UP|RUNNING))==(UP|RUNNING))
&& ((sc->an_if_flags&(UP|RUNNING))==(UP|RUNNING)), but what's there is
a bit of an optimization.

Of course, if anyone who actually knows this code thinks I'm wrong,
please, correct me. :-)

/~\ The ASCII				der Mouse
\ / Ribbon Campaign
 X  Against HTML	       mouse@rodents.montreal.qc.ca
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B