From owner-freebsd-mobile  Wed Dec  6 12: 7:12 2000
From owner-freebsd-mobile@FreeBSD.ORG  Wed Dec  6 12:07:04 2000
Return-Path: <owner-freebsd-mobile@FreeBSD.ORG>
Delivered-To: freebsd-mobile@freebsd.org
Received: by hub.freebsd.org (Postfix, from userid 618)
	id A2BFF37B401; Wed,  6 Dec 2000 12:07:03 -0800 (PST)
Subject: Raylink driver nits (plus patch)
To: freebsd-mobile@freebsd.org, dmlb@dmlb.org
Date: Wed, 6 Dec 2000 12:07:03 -0800 (PST)
X-Mailer: ELM [version 2.4ME+ PL54 (25)]
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Message-Id: <20001206200703.A2BFF37B401@hub.freebsd.org>
From: wpaul@FreeBSD.ORG (Bill Paul)
Sender: owner-freebsd-mobile@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.org

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