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