Subject: Re: kern/31130: [PATCH] Fix vge(4) SIOC{ADD,DEL}MULTI handling
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: FUKAUMI Naoki <fun@naobsd.org>
List: netbsd-bugs
Date: 09/05/2005 16:04:02
The following reply was made to PR kern/31130; it has been noted by GNATS.

From: FUKAUMI Naoki <fun@naobsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/31130: [PATCH] Fix vge(4) SIOC{ADD,DEL}MULTI handling
Date: Tue, 6 Sep 2005 01:03:37 +0900

 Previous patch was broken, too ;)
 New patch is attached. It's tested on NetBSD 3.0_BETA.
 
 List of changes:
 
   - Fix IFF_ALLMULTI handling
   - Use enm_addrlo directly
   - Stop endless loop in multicast hash table filter mode
   - Protect splnet() around vge_ioctl() operations
   - Call ether_{add,del}multi in SIOC{ADD,DEL}MULTI
   - Check IFF_RUNNING before vge_setmulti()
 
 Regards,
 
 --
 FUKAUMI Naoki
 
 Index: sys/dev/pci/if_vge.c
 ===================================================================
 RCS file: /home/fun/cvsroot/NetBSD/src/sys/dev/pci/if_vge.c,v
 retrieving revision 1.5
 diff -u -p -r1.5 if_vge.c
 --- sys/dev/pci/if_vge.c	2 May 2005 15:34:32 -0000	1.5
 +++ sys/dev/pci/if_vge.c	5 Sep 2005 15:41:32 -0000
 @@ -549,15 +549,17 @@ vge_setmulti(sc)
  	vge_cam_clear(sc);
  	CSR_WRITE_4(sc, VGE_MAR0, 0);
  	CSR_WRITE_4(sc, VGE_MAR1, 0);
 +	ifp->if_flags &= ~IFF_ALLMULTI;
  
  	/*
  	 * If the user wants allmulti or promisc mode, enable reception
  	 * of all multicast frames.
  	 */
 -	if (ifp->if_flags & IFF_ALLMULTI || ifp->if_flags & IFF_PROMISC) {
 +	if (ifp->if_flags & IFF_PROMISC) {
      allmulti:
  		CSR_WRITE_4(sc, VGE_MAR0, 0xFFFFFFFF);
  		CSR_WRITE_4(sc, VGE_MAR1, 0xFFFFFFFF);
 +		ifp->if_flags |= IFF_ALLMULTI;
  		return;
  	}
  
 @@ -571,8 +573,7 @@ vge_setmulti(sc)
  				ETHER_ADDR_LEN) != 0)
  			goto allmulti;
  
 -		error = vge_cam_set(sc,
 -		    LLADDR((struct sockaddr_dl *)enm->enm_addrlo));
 +		error = vge_cam_set(sc, enm->enm_addrlo);
  		if (error)
  			break;
  
 @@ -585,12 +586,18 @@ vge_setmulti(sc)
  
  		ETHER_FIRST_MULTI(step, &sc->sc_ethercom, enm);
  		while(enm != NULL) {
 -			h = ether_crc32_be(LLADDR((struct sockaddr_dl *)
 -			    enm->enm_addrlo), ETHER_ADDR_LEN) >> 26;
 -			if (h < 32)
 -				hashes[0] |= (1 << h);
 -			else
 -				hashes[1] |= (1 << (h - 32));
 +			/*
 +			 * If multicast range, fall back to ALLMULTI.
 +			 */
 +			if (memcmp(enm->enm_addrlo, enm->enm_addrhi,
 +					ETHER_ADDR_LEN) != 0)
 +				goto allmulti;
 +
 +			h = ether_crc32_be(enm->enm_addrlo,
 +			    ETHER_ADDR_LEN) >> 26;
 +			hashes[h >> 5] |= 1 << (h & 0x1f);
 +
 +			ETHER_NEXT_MULTI(step, enm);
  		}
  
  		CSR_WRITE_4(sc, VGE_MAR0, hashes[0]);
 @@ -2060,7 +2067,9 @@ vge_ioctl(ifp, command, data)
  	struct vge_softc	*sc = ifp->if_softc;
  	struct ifreq		*ifr = (struct ifreq *) data;
  	struct mii_data		*mii;
 -	int			error = 0;
 +	int			s, error = 0;
 +
 +	s = splnet();
  
  	switch (command) {
  	case SIOCSIFMTU:
 @@ -2092,7 +2101,19 @@ vge_ioctl(ifp, command, data)
  		break;
  	case SIOCADDMULTI:
  	case SIOCDELMULTI:
 -		vge_setmulti(sc);
 +		error = (command == SIOCADDMULTI) ?
 +		    ether_addmulti(ifr, &sc->sc_ethercom) :
 +		    ether_delmulti(ifr, &sc->sc_ethercom);
 +
 +		if (error == ENETRESET) {
 +			/*
 +			 * Multicast list has changed; set the hardware filter
 +			 * accordingly.
 +			 */
 +			if (ifp->if_flags & IFF_RUNNING)
 +				vge_setmulti(sc);
 +			error = 0;
 +		}
  		break;
  	case SIOCGIFMEDIA:
  	case SIOCSIFMEDIA:
 @@ -2104,6 +2125,7 @@ vge_ioctl(ifp, command, data)
  		break;
  	}
  
 +	splx(s);
  	return (error);
  }