Subject: Allowing ether_ifattach() to fail
To: None <tech-net@netbsd.org>
From: John Hawkinson <jhawk@mit.edu>
List: tech-net
Date: 05/23/2000 23:28:43
As part of addressing kern/9548 ("fxp0 can get all 1's mac address"),,
I'd like to allow ether_ifattach() to fail (and return failure). It
would do this in the case of invalid MAC addresses (like
multicast/broadcast addresses, that seem to have a high potential to
pop up in the case of buggy drivers/flakey hardware).
Since this requires touching a lot of the tree (ether_ifattach() is
called from every network driver), I thought I should send a probe
here first, since modifications might be signficant (see below).
Careful examination of a bunch of drivers shows that they don't deal
very well with the case of *_attach() failing. For instance there are
a wide variety of failure conditions (7?) where fxp_attach() might
not call if_attach(); but fxp_detach() unconditionally calls
if_detach().
(At the present time, this is only a problem for drivers where
the *_detach() routing might be called, which is probably not many
at this point...)
It was suggested to me that the correct way around this is for
the softc to carry a bit indicating whether the attach was complete,
and for the *_attach() functions to reverse any operations in
the event of a failure, and ensure the bit is clear. The tulip
driver seems to do this
sys/dev/ic/tulipvar.h:
309 struct tulip_softc {
...
340 tulip_chip_t sc_chip; /* chip type */
341 int sc_rev; /* chip revision */
342 int sc_flags; /* misc flags. */
...
424 /* sc_flags */
...
436 #define TULIPF_ATTACHED 0x00000800 /* attach has succeeded
sys/dev/ic/tulip.c:
197 void
198 tlp_attach(sc, enaddr)
...
418 * From this point forward, the attachment cannot fail. A failu
418 re
419 * before this point releases all resources that may have been
420 * allocated.
421 */
422 sc->sc_flags |= TULIPF_ATTACHED;
...
535 /*
536 * tlp_detach:
537 *
538 * Detach a Tulip interface.
539 */
540 int
541 tlp_detach(sc)
542 struct tulip_softc *sc;
543 {
544 struct ifnet *ifp = &sc->sc_ethercom.ec_if;
545 struct tulip_rxsoft *rxs;
546 struct tulip_txsoft *txs;
547 int i;
548
549 /*
550 * Suceed now if there isn't any work to do.
551 */
552 if ((sc->sc_flags & TULIPF_ATTACHED) == 0)
553 return (0);
Are there are other suggestions or techniques that should be
applied here? Are we happy standardizing on a *_ATTACHED bit in
sc_flags within the softc?
--jhawk