Date: Wed, 6 Dec 2000 12:07:03 -0800 (PST) From: wpaul@FreeBSD.ORG (Bill Paul) To: freebsd-mobile@freebsd.org, dmlb@dmlb.org Subject: Raylink driver nits (plus patch) Message-ID: <20001206200703.A2BFF37B401@hub.freebsd.org>
next in thread | raw e-mail | index | archive | help
I took a quick look at /sys/dev/ray/if_ray.c and noticed a couple things
that aren't quite right. I'm including a patch to fix them. The problems
are:
- In ray_attach(), the code calls ether_ifattach(), then later it calls
bpfattach(). This is unnecessary: the whole point of calling
ether_ifattach() with the BPF_SUPPORTED flag is that it will do the
bpfattach() for you. Calling bpfattach() again is unnecessary, and
probably causes bugs, or at the very least a resource leak.
- In ray_attach(), the code checks for ifp->if_name being NULL before
initializing the ifnet structure info. This isn't necessary anymore.
It used to be in FreeBSD 3.x and earlier that there was no if_detach()
function, so once you created an ifnet structure you couldn't get rid
of it, and you had to be careful not to initialize/attach it twice.
Now the test is redundant.
- In ray_detach(), the code calls if_detach() when it should be calling
ether_ifdetach(). Among other things, this does a bpfdetach(), which
isn't being done now and should be.
Again, patch is included. The raylink driver maintainer can check this
in, or if nobody objects, I can do it.
-Bill
*** if_ray.c.orig Wed Dec 6 11:54:55 2000
--- if_ray.c Wed Dec 6 11:56:46 2000
***************
*** 491,516 ****
/*
* Initialise the network interface structure
*/
! if (!ifp->if_name) {
! bcopy((char *)&ep->e_station_addr,
! (char *)&sc->arpcom.ac_enaddr, ETHER_ADDR_LEN);
! ifp->if_softc = sc;
! ifp->if_name = "ray";
! ifp->if_unit = device_get_unit(dev);
! ifp->if_timer = 0;
! ifp->if_flags = (IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST);
! ifp->if_hdrlen = sizeof(struct ieee80211_frame) +
! sizeof(struct ether_header);
! ifp->if_baudrate = 1000000; /* Is this baud or bps ;-) */
! ifp->if_output = ether_output;
! ifp->if_start = ray_tx;
! ifp->if_ioctl = ray_ioctl;
! ifp->if_watchdog = ray_watchdog;
! ifp->if_init = ray_init_user;
! ifp->if_snd.ifq_maxlen = IFQ_MAXLEN;
! ether_ifattach(ifp, ETHER_BPF_SUPPORTED);
! }
/*
* Initialise the timers and driver
--- 491,514 ----
/*
* Initialise the network interface structure
*/
! bcopy((char *)&ep->e_station_addr,
! (char *)&sc->arpcom.ac_enaddr, ETHER_ADDR_LEN);
! ifp->if_softc = sc;
! ifp->if_name = "ray";
! ifp->if_unit = device_get_unit(dev);
! ifp->if_timer = 0;
! ifp->if_flags = (IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST);
! ifp->if_hdrlen = sizeof(struct ieee80211_frame) +
! sizeof(struct ether_header);
! ifp->if_baudrate = 1000000; /* Is this baud or bps ;-) */
! ifp->if_output = ether_output;
! ifp->if_start = ray_tx;
! ifp->if_ioctl = ray_ioctl;
! ifp->if_watchdog = ray_watchdog;
! ifp->if_init = ray_init_user;
! ifp->if_snd.ifq_maxlen = IFQ_MAXLEN;
! ether_ifattach(ifp, ETHER_BPF_SUPPORTED);
/*
* Initialise the timers and driver
***************
*** 518,524 ****
callout_handle_init(&sc->com_timerh);
callout_handle_init(&sc->tx_timerh);
TAILQ_INIT(&sc->sc_comq);
- bpfattach(ifp, DLT_EN10MB, sizeof(struct ether_header));
/*
* Print out some useful information
--- 516,521 ----
***************
*** 587,593 ****
sc->gone = 1;
sc->sc_havenet = 0;
ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
! if_detach(ifp);
/*
* Stop the runq and wake up anyone sleeping for us.
--- 584,590 ----
sc->gone = 1;
sc->sc_havenet = 0;
ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
! ether_ifdetach(ifp, ETHER_BPF_SUPPORTED);
/*
* Stop the runq and wake up anyone sleeping for us.
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-mobile" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20001206200703.A2BFF37B401>