Date: Sat, 2 Dec 2006 10:41:50 +0100 From: Ed Schouten <ed@fxq.nl> To: Bill Paul <wpaul@FreeBSD.ORG> Cc: pyunyh@gmail.com, stable@freebsd.org, net@freebsd.org Subject: Re: re(4) needs promisc to work properly Message-ID: <20061202094150.GE16100@hoeg.nl> In-Reply-To: <20061201165231.4089216A416@hub.freebsd.org> References: <20061201161131.GD16100@hoeg.nl> <20061201165231.4089216A416@hub.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--+sXEj1HC0AeGgRD2 Content-Type: multipart/mixed; boundary="t1V/6vyDgJ44qiBN" Content-Disposition: inline --t1V/6vyDgJ44qiBN Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello Bill, * Bill Paul <wpaul@FreeBSD.ORG> wrote: > I'm a little concerned about the fact that now SIOCSIFFLAGS can never > cause re_init_locked() to be called. There are some cases where it > does need to be called (like when the IFF_UP flag is first set to > turn the interface on). >=20 > I usually do it like in the vge(4) driver: if it's just the IFF_PROMISC > bit that's being toggled, then I only toggle the promisc mode bit in > the RX config register. To avoid code duplication and to speed up IFF_BROADCAST as well, I decided to keep the re_init_rxcfg() function. I took a look at vge(4) and xl(4) and added `re_if_flags` to the softc to backup the original ifp->if_flags. Yours, --=20 Ed Schouten <ed@fxq.nl> WWW: http://g-rave.nl/ --t1V/6vyDgJ44qiBN Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="if_re.c.diff" Content-Transfer-Encoding: quoted-printable --- sys/dev/re/if_re.c Sat Dec 2 00:05:44 2006 +++ sys/dev/re/if_re.c Sat Dec 2 00:15:56 2006 @@ -249,6 +249,7 @@ static int re_ioctl (struct ifnet *, u_long, caddr_t); static void re_init (void *); static void re_init_locked (struct rl_softc *); +static void re_init_rxcfg (struct rl_softc *); static void re_stop (struct rl_softc *); static void re_watchdog (struct ifnet *); static int re_suspend (device_t); @@ -2254,7 +2255,6 @@ { struct ifnet *ifp =3D sc->rl_ifp; struct mii_data *mii; - u_int32_t rxcfg =3D 0; union { uint32_t align_dummy; u_char eaddr[ETHER_ADDR_LEN]; @@ -2316,31 +2316,8 @@ } else CSR_WRITE_4(sc, RL_TXCFG, RL_TXCFG_CONFIG); CSR_WRITE_4(sc, RL_RXCFG, RL_RXCFG_CONFIG); - - /* Set the individual bit to receive frames for this host only. */ - rxcfg =3D CSR_READ_4(sc, RL_RXCFG); - rxcfg |=3D RL_RXCFG_RX_INDIV; - - /* If we want promiscuous mode, set the allframes bit. */ - if (ifp->if_flags & IFF_PROMISC) - rxcfg |=3D RL_RXCFG_RX_ALLPHYS; - else - rxcfg &=3D ~RL_RXCFG_RX_ALLPHYS; - CSR_WRITE_4(sc, RL_RXCFG, rxcfg); - - /* - * Set capture broadcast bit to capture broadcast frames. - */ - if (ifp->if_flags & IFF_BROADCAST) - rxcfg |=3D RL_RXCFG_RX_BROAD; - else - rxcfg &=3D ~RL_RXCFG_RX_BROAD; - CSR_WRITE_4(sc, RL_RXCFG, rxcfg); - - /* - * Program the multicast filter, if necessary. - */ - re_setmulti(sc); +=09 + re_init_rxcfg(sc); =20 #ifdef DEVICE_POLLING /* @@ -2422,6 +2399,39 @@ callout_reset(&sc->rl_stat_callout, hz, re_tick, sc); } =20 +static void +re_init_rxcfg(sc) + struct rl_softc *sc; +{ + u_int32_t rxcfg; + struct ifnet *ifp =3D sc->rl_ifp; + + /* Set the individual bit to receive frames for this host only. */ + rxcfg =3D CSR_READ_4(sc, RL_RXCFG); + rxcfg |=3D RL_RXCFG_RX_INDIV; + + /* If we want promiscuous mode, set the allframes bit. */ + if (ifp->if_flags & IFF_PROMISC) + rxcfg |=3D RL_RXCFG_RX_ALLPHYS; + else + rxcfg &=3D ~RL_RXCFG_RX_ALLPHYS; + CSR_WRITE_4(sc, RL_RXCFG, rxcfg); + + /* + * Set capture broadcast bit to capture broadcast frames. + */ + if (ifp->if_flags & IFF_BROADCAST) + rxcfg |=3D RL_RXCFG_RX_BROAD; + else + rxcfg &=3D ~RL_RXCFG_RX_BROAD; + CSR_WRITE_4(sc, RL_RXCFG, rxcfg); + + /* + * Program the multicast filter, if necessary. + */ + re_setmulti(sc); +} + /* * Set media options. */ @@ -2483,10 +2493,16 @@ break; case SIOCSIFFLAGS: RL_LOCK(sc); - if (ifp->if_flags & IFF_UP) - re_init_locked(sc); - else if (ifp->if_drv_flags & IFF_DRV_RUNNING) + if (ifp->if_flags & IFF_UP) { + if ((ifp->if_flags ^ sc->rl_if_flags) & + (IFF_PROMISC | IFF_BROADCAST)) + re_init_rxcfg(sc); + else + re_init_locked(sc); + } else if (ifp->if_drv_flags & IFF_DRV_RUNNING) { re_stop(sc); + } + sc->rl_if_flags =3D ifp->if_flags; RL_UNLOCK(sc); break; case SIOCADDMULTI: --- sys/pci/if_rlreg.h Sat Dec 2 00:07:27 2006 +++ sys/pci/if_rlreg.h Sat Dec 2 00:18:53 2006 @@ -737,6 +737,7 @@ struct mtx rl_intlock; int rl_txstart; int rl_link; + int rl_if_flags; }; =20 #define RL_LOCK(_sc) mtx_lock(&(_sc)->rl_mtx) --t1V/6vyDgJ44qiBN-- --+sXEj1HC0AeGgRD2 Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (FreeBSD) iD8DBQFFcUpe52SDGA2eCwURAs+PAJ95X+L3/oyBpa/n5p7lzIRSKRkIeQCdF5ZF dYySYWoSmLiN404oONxwOfw= =itll -----END PGP SIGNATURE----- --+sXEj1HC0AeGgRD2--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061202094150.GE16100>