Date: Fri, 16 Sep 2005 23:22:07 +0300 From: Ruslan Ermilov <ru@FreeBSD.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, jhb@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/re if_re.c Message-ID: <20050916202207.GA22151@ip.net.ua> In-Reply-To: <20050916.135841.130619528.imp@bsdimp.com> References: <20050916091928.GG88456@ip.net.ua> <20050916.090140.58827157.imp@bsdimp.com> <20050916194405.GB24879@ip.net.ua> <20050916.135841.130619528.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--h31gzZEtNLTqOjlF
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Sep 16, 2005 at 01:58:41PM -0600, M. Warner Losh wrote:
> In message: <20050916194405.GB24879@ip.net.ua>
> Ruslan Ermilov <ru@FreeBSD.org> writes:
> : On Fri, Sep 16, 2005 at 09:01:40AM -0600, M. Warner Losh wrote:
> : > In message: <20050916091928.GG88456@ip.net.ua>
> : > Ruslan Ermilov <ru@FreeBSD.org> writes:
> : > : On Thu, Sep 15, 2005 at 11:56:39PM +0300, Ruslan Ermilov wrote:
> : > : > The first is the BPF detach bad interaction with foo_detach(),
> : > : > as described in re_detach(). This panic is real with (I think)
> : > : > all drivers. And testing IFF_DRV_RUNNING here doesn't seem to
> : > : > be able to prevent the panic. Perhaps the fix would be to
> : > : > move ether_ifdetach() before foo_stop() in foo_detach(), I'm
> : > : > not yet sure.
> : > : >=20
> : > : I tried with rl(4) PCCARD, by moving ether_ifdetach() before
> : > : rl_stop() in rl_detach(). It fixes the panic when you eject
> : > : the card, but doesn't fix it when kldunloading the module.
> : > : The difference is that rl_detach() is called already after
> : > : miibus0 and rlphy0 has been detached when kldunloading the
> : > : module. When ejecting the card, rl_detach() is called first.
> : > : What happens when you kldunload the module with BPF listener
> : > : attached, is that bpfdetach() calls rl_ioctl() to reset
> : > : promisc, that calls rl_init_locked(), and that results in
> : > :=20
> : > : mii =3D device_get_softc(sc->rl_miibus);
> : > :=20
> : > : being NULL (remember the miibus has already been detached),
> : > : and that panics later here:
> : > :=20
> : > : mii_mediachg(mii);
> : > :=20
> : > : When we reset IFF_UP, rl_ioctl(SIOCSIFFLAGS) silently exits
> : > : and no harm is done. So the question is: how do we prevent
> : > : this from happening without resetting IFF_UP. One possible
> : > : solution would be to add sc->detaching, similar to
> : > : sc->suspended, abd check it in rl_ioctl().
> : >=20
> : > Ugg. In ed, we check to make sure that we still have a child before
> : > doing things with mii bus. A similar fix could be made.
> : >=20
> : No, ed(4) has the same problem:
> :=20
> : if (sc->miibus !=3D NULL) {
> : struct mii_data *mii;
> : mii =3D device_get_softc(sc->miibus);
> : mii_mediachg(mii);
> : }
> :=20
>=20
> No it doesn't:
>=20
> void
> ed_child_detached(device_t dev, device_t child)
> {
> struct ed_softc *sc;
>=20
> sc =3D device_get_softc(dev);
> if (child =3D=3D sc->miibus)
> sc->miibus =3D NULL;
> }
>=20
> : The device (sc->miibus) will still be there but already detached,
> : and its softc will already be freed, so "mii" will be NULL, and
> : mii_mediachg(NULL) will panic the system.
>=20
> sc->miibus will be NULL after the device is detached, so you don't get
> an error.
>=20
Hmm, I'm not very fluent in device(9) API, but I wonder what's then
the analog of device_delete_child(sc->miibus) that the majority of
foo_detach() methods do. I.e., will the miibus device really be
removed?
> How again can this happen?
>=20
tcpdump -n -i ed0 &
kldunload if_ed
Still, ed_init_locked() will instantiate many things inappropriate
for ed_detach() context.
When experimenting with removing device_delete_child(sc->miibus)
in rl(4), every new kldunload/kldload will add another miibusX
device, showing that the child device removal doesn't happen
implicitly. I wonder if you can see the same with ed(4), or
if there's some code that does this. Also, the code that you
refer seems to only work for pccard, while PCI version should
be affected by the same "mii =3D=3D NULL" bug.
Cheers,
--=20
Ruslan Ermilov
ru@FreeBSD.org
FreeBSD committer
--h31gzZEtNLTqOjlF
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (FreeBSD)
iD8DBQFDKylvqRfpzJluFF4RAnHNAJ9CoWZTm4FQbhDYY6MlYU3BrI8PHgCdFi/F
yuCcDlFjA8kl64/rtOORkSE=
=m7i9
-----END PGP SIGNATURE-----
--h31gzZEtNLTqOjlF--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050916202207.GA22151>