Subject: please review a diff to handle allmulti case correctly with some driver.
To: None <tech-net@netbsd.org>
From: enami tsugutomo <enami@sm.sony.co.jp>
List: tech-net
Date: 01/28/2001 23:03:21
Hi all (and people originated PR12054 and who replied to the PR).

As far as reading the code, some driver (rtk/vr/ti/aue/cue/kue)
doesn't handle allmulti case correctly as NetBSD network driver.  And
it hurts when range of multicast address is added (as mentioned in PR
12054)

I can't test these code (except compilation test) since I don't have
hardware.  But the current code is obviously wrong as NetBSD driver,
and actually there is a problem with it, i'm planing to commit this
anyway in a few days.

enami.
Index: ic/rtl81x9.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/ic/rtl81x9.c,v
retrieving revision 1.27
diff -u -r1.27 rtl81x9.c
--- ic/rtl81x9.c	2001/01/11 14:38:58	1.27
+++ ic/rtl81x9.c	2001/01/28 13:35:46
@@ -564,7 +564,9 @@
 
 	rxfilt = CSR_READ_4(sc, RTK_RXCFG);
 
-	if (ifp->if_flags & IFF_ALLMULTI || ifp->if_flags & IFF_PROMISC) {
+	if (ifp->if_flags & IFF_PROMISC) {
+allmulti:
+		ifp->if_flags |= IFF_ALLMULTI;
 		rxfilt |= RTK_RXCFG_RX_MULTI;
 		CSR_WRITE_4(sc, RTK_RXCFG, rxfilt);
 		CSR_WRITE_4(sc, RTK_MAR0, 0xFFFFFFFF);
@@ -581,7 +583,7 @@
 	while (enm != NULL) {
 		if (memcmp(enm->enm_addrlo, enm->enm_addrhi,
 		    ETHER_ADDR_LEN) != 0)
-			continue;
+			goto allmulti;
 
 		h = rtk_calchash(enm->enm_addrlo);
 		if (h < 32)
@@ -591,6 +593,8 @@
 		mcnt++;
 		ETHER_NEXT_MULTI(step, enm);
 	}
+
+	ifp->if_flags &= ~IFF_ALLMULTI;
 
 	if (mcnt)
 		rxfilt |= RTK_RXCFG_RX_MULTI;
Index: pci/if_vr.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/pci/if_vr.c,v
retrieving revision 1.44
diff -u -r1.44 if_vr.c
--- pci/if_vr.c	2000/12/28 22:59:14	1.44
+++ pci/if_vr.c	2001/01/28 13:35:47
@@ -479,7 +479,9 @@
 
 	rxfilt = CSR_READ_1(sc, VR_RXCFG);
 
-	if (ifp->if_flags & IFF_ALLMULTI || ifp->if_flags & IFF_PROMISC) {
+	if (ifp->if_flags & IFF_PROMISC) {
+allmulti:
+		ifp->if_flags |= IFF_ALLMULTI;
 		rxfilt |= VR_RXCFG_RX_MULTI;
 		CSR_WRITE_1(sc, VR_RXCFG, rxfilt);
 		CSR_WRITE_4(sc, VR_MAR0, 0xFFFFFFFF);
@@ -494,8 +496,9 @@
 	/* now program new ones */
 	ETHER_FIRST_MULTI(step, &sc->vr_ec, enm);
 	while (enm != NULL) {
-		if (memcmp(enm->enm_addrlo, enm->enm_addrhi, 6) != 0)
-			continue;
+		if (memcmp(enm->enm_addrlo, enm->enm_addrhi,
+		    ETHER_ADDR_LEN) != 0)
+			goto allmulti;
 
 		h = vr_calchash(enm->enm_addrlo);
 
@@ -506,6 +509,8 @@
 		ETHER_NEXT_MULTI(step, enm);
 		mcnt++;
 	}
+
+	ifp->if_flags &= ~IFF_ALLMULTI;
 
 	if (mcnt)
 		rxfilt |= VR_RXCFG_RX_MULTI;
Index: pci/if_ti.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/pci/if_ti.c,v
retrieving revision 1.19
diff -u -r1.19 if_ti.c
--- pci/if_ti.c	2001/01/18 20:28:15	1.19
+++ pci/if_ti.c	2001/01/28 13:35:49
@@ -1189,40 +1189,62 @@
 
 	ifp = &sc->ethercom.ec_if;
 
-	if (ifp->if_flags & IFF_ALLMULTI) {
-		TI_DO_CMD(TI_CMD_SET_ALLMULTI, TI_CMD_CODE_ALLMULTI_ENB, 0);
-		return;
-	} else {
-		TI_DO_CMD(TI_CMD_SET_ALLMULTI, TI_CMD_CODE_ALLMULTI_DIS, 0);
-	}
-
 	/* Disable interrupts. */
 	intrs = CSR_READ_4(sc, TI_MB_HOSTINTR);
 	CSR_WRITE_4(sc, TI_MB_HOSTINTR, 1);
 
 	/* First, zot all the existing filters. */
-	while (SIMPLEQ_FIRST(&sc->ti_mc_listhead) != NULL) {
-		mc = SIMPLEQ_FIRST(&sc->ti_mc_listhead);
+	while ((mc = SIMPLEQ_FIRST(&sc->ti_mc_listhead)) != NULL) {
 		ti_del_mcast(sc, &mc->mc_addr);
 		SIMPLEQ_REMOVE_HEAD(&sc->ti_mc_listhead, mc, mc_entries);
 		free(mc, M_DEVBUF);
 	}
 
-	/* Now program new ones. */
+	/*
+	 * Remember all multicast addresses so that we can delete them
+	 * later.  Punt if there is a range of addresses or memory shortage.
+	 */
 	ETHER_FIRST_MULTI(step, &sc->ethercom, enm);
 	while (enm != NULL) {
-		mc = malloc(sizeof(struct ti_mc_entry), M_DEVBUF, M_NOWAIT);
-		bcopy(enm->enm_addrlo,
-		    (char *)&mc->mc_addr, ETHER_ADDR_LEN);
+		if (memcmp(enm->enm_addrlo, enm->enm_addrhi,
+		    ETHER_ADDR_LEN) != 0)
+			goto allmulti;
+		if ((mc = malloc(sizeof(struct ti_mc_entry), M_DEVBUF,
+		    M_NOWAIT)) == NULL)
+			goto allmulti;
+		memcpy(&mc->mc_addr, enm->enm_addrlo, ETHER_ADDR_LEN);
 		SIMPLEQ_INSERT_HEAD(&sc->ti_mc_listhead, mc, mc_entries);
-		ti_add_mcast(sc, &mc->mc_addr);
 		ETHER_NEXT_MULTI(step, enm);
 	}
 
+	/* Accept only programmed multicast addresses */
+	ifp->if_flags &= ~IFF_ALLMULTI;
+	TI_DO_CMD(TI_CMD_SET_ALLMULTI, TI_CMD_CODE_ALLMULTI_DIS, 0);
+
+	/* Now program new ones. */
+	for (mc = SIMPLEQ_FIRST(&sc->ti_mc_listhead); mc != NULL;
+	    mc = SIMPLEQ_NEXT(mc, mc_entries))
+		ti_add_mcast(sc, &mc->mc_addr);
+
 	/* Re-enable interrupts. */
 	CSR_WRITE_4(sc, TI_MB_HOSTINTR, intrs);
 
 	return;
+
+allmulti:
+	/* No need to keep individual multicast addresses */
+	while ((mc = SIMPLEQ_FIRST(&sc->ti_mc_listhead)) != NULL) {
+		SIMPLEQ_REMOVE_HEAD(&sc->ti_mc_listhead, mc,
+		    mc_entries);
+		free(mc, M_DEVBUF);
+	}
+
+	/* Accept all multicast addresses */
+	ifp->if_flags |= IFF_ALLMULTI;
+	TI_DO_CMD(TI_CMD_SET_ALLMULTI, TI_CMD_CODE_ALLMULTI_ENB, 0);
+
+	/* Re-enable interrupts. */
+	CSR_WRITE_4(sc, TI_MB_HOSTINTR, intrs);
 }
 
 /*
@@ -1761,6 +1783,8 @@
 		goto fail2;
 	}
 
+	SIMPLEQ_INIT(&sc->ti_mc_listhead);
+
 	/*
 	 * We really need a better way to tell a 1000baseTX card
 	 * from a 1000baseSX one, since in theory there could be
@@ -2600,12 +2624,12 @@
 		break;
 	case SIOCADDMULTI:
 	case SIOCDELMULTI:
-		if (command == SIOCADDMULTI)
-			ether_addmulti(ifr, &sc->ethercom);
-		else
-			ether_delmulti(ifr, &sc->ethercom);
-		if (ifp->if_flags & IFF_RUNNING) {
-			ti_setmulti(sc);
+		error = (command == SIOCADDMULTI) ?
+		    ether_addmulti(ifr, &sc->ethercom) :
+		    ether_delmulti(ifr, &sc->ethercom);
+		if (error == ENETRESET) {
+			if (ifp->if_flags & IFF_RUNNING)
+				ti_setmulti(sc);
 			error = 0;
 		}
 		break;
Index: usb/if_aue.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/usb/if_aue.c,v
retrieving revision 1.53
diff -u -r1.53 if_aue.c
--- usb/if_aue.c	2001/01/21 19:42:29	1.53
+++ usb/if_aue.c	2001/01/28 13:35:50
@@ -530,7 +530,9 @@
 
 	ifp = GET_IFP(sc);
 
-	if (ifp->if_flags & IFF_ALLMULTI || ifp->if_flags & IFF_PROMISC) {
+	if (ifp->if_flags & IFF_PROMISC) {
+allmulti:
+		ifp->if_flags |= IFF_ALLMULTI;
 		AUE_SETBIT(sc, AUE_CTL0, AUE_CTL0_ALLMULTI);
 		return;
 	}
@@ -548,18 +550,16 @@
 	ETHER_FIRST_MULTI(step, &sc->arpcom, enm);
 #endif
 	while (enm != NULL) {
-#if 1
 		if (memcmp(enm->enm_addrlo,
-			   enm->enm_addrhi, ETHER_ADDR_LEN) != 0) {
-			ifp->if_flags |= IFF_ALLMULTI;
-			AUE_SETBIT(sc, AUE_CTL0, AUE_CTL0_ALLMULTI);
-			return;
-		}
-#endif
+		    enm->enm_addrhi, ETHER_ADDR_LEN) != 0)
+			goto allmulti;
+
 		h = aue_crc(enm->enm_addrlo);
 		AUE_SETBIT(sc, AUE_MAR + (h >> 3), 1 << (h & 0x7));
 		ETHER_NEXT_MULTI(step, enm);
 	}
+
+	ifp->if_flags &= ~IFF_ALLMULTI;
 }
 
 Static void
Index: usb/if_cue.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/usb/if_cue.c,v
retrieving revision 1.32
diff -u -r1.32 if_cue.c
--- usb/if_cue.c	2001/01/23 17:04:30	1.32
+++ usb/if_cue.c	2001/01/28 13:35:50
@@ -389,7 +389,9 @@
 	DPRINTFN(2,("%s: cue_setmulti if_flags=0x%x\n", 
 		    USBDEVNAME(sc->cue_dev), ifp->if_flags));
 
-	if (ifp->if_flags & IFF_ALLMULTI || ifp->if_flags & IFF_PROMISC) {
+	if (ifp->if_flags & IFF_PROMISC) {
+allmulti:
+		ifp->if_flags |= IFF_ALLMULTI;
 		for (i = 0; i < CUE_MCAST_TABLE_LEN; i++)
 			sc->cue_mctab[i] = 0xFF;
 		cue_mem(sc, CUE_CMD_WRITESRAM, CUE_MCAST_TABLE_ADDR,
@@ -408,18 +410,16 @@
 	ETHER_FIRST_MULTI(step, &sc->arpcom, enm);
 #endif
 	while (enm != NULL) {
-#if 0
 		if (memcmp(enm->enm_addrlo,
-			   enm->enm_addrhi, ETHER_ADDR_LEN) != 0) {
-			ifp->if_flags |= IFF_ALLMULTI;
-			/* XXX what now? */
-			return;
-		}
-#endif
+		    enm->enm_addrhi, ETHER_ADDR_LEN) != 0)
+			goto allmulti;
+
 		h = cue_crc(enm->enm_addrlo);
 		sc->cue_mctab[h >> 3] |= 1 << (h & 0x7);		
 		ETHER_NEXT_MULTI(step, enm);
 	}
+
+	ifp->if_flags &= ~IFF_ALLMULTI;
 
 	/*
 	 * Also include the broadcast address in the filter
Index: usb/if_kue.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/usb/if_kue.c,v
retrieving revision 1.37
diff -u -r1.37 if_kue.c
--- usb/if_kue.c	2001/01/21 15:55:05	1.37
+++ usb/if_kue.c	2001/01/28 13:35:51
@@ -321,7 +321,9 @@
 
 	DPRINTFN(5,("%s: %s: enter\n", USBDEVNAME(sc->kue_dev), __FUNCTION__));
 
-	if (ifp->if_flags & IFF_ALLMULTI || ifp->if_flags & IFF_PROMISC) {
+	if (ifp->if_flags & IFF_PROMISC) {
+allmulti:
+		ifp->if_flags |= IFF_ALLMULTI;
 		sc->kue_rxfilt |= KUE_RXFILT_ALLMULTI;
 		sc->kue_rxfilt &= ~KUE_RXFILT_MULTICAST;
 		kue_setword(sc, KUE_CMD_SET_PKT_FILTER, sc->kue_rxfilt);
@@ -337,28 +339,21 @@
 	ETHER_FIRST_MULTI(step, &sc->arpcom, enm);
 #endif
 	while (enm != NULL) {
-		if (i == KUE_MCFILTCNT(sc))
-			break;
-#if 0
-		if (memcmp(enm->enm_addrlo,
-			   enm->enm_addrhi, ETHER_ADDR_LEN) != 0) {
-			ifp->if_flags |= IFF_ALLMULTI;
-			/* XXX what now? */
-			return;
-		}
-#endif
+		if (i == KUE_MCFILTCNT(sc) ||
+		    memcmp(enm->enm_addrlo, enm->enm_addrhi,
+			ETHER_ADDR_LEN) != 0)
+			goto allmulti;
+
 		memcpy(KUE_MCFILT(sc, i), enm->enm_addrlo, ETHER_ADDR_LEN);
 		ETHER_NEXT_MULTI(step, enm);
 		i++;
 	}
 
-	if (i == KUE_MCFILTCNT(sc))
-		sc->kue_rxfilt |= KUE_RXFILT_ALLMULTI;
-	else {
-		sc->kue_rxfilt |= KUE_RXFILT_MULTICAST;
-		kue_ctl(sc, KUE_CTL_WRITE, KUE_CMD_SET_MCAST_FILTERS,
-		    i, sc->kue_mcfilters, i * ETHER_ADDR_LEN);
-	}
+	ifp->if_flags &= ~IFF_ALLMULTI;
+
+	sc->kue_rxfilt |= KUE_RXFILT_MULTICAST;
+	kue_ctl(sc, KUE_CTL_WRITE, KUE_CMD_SET_MCAST_FILTERS,
+	    i, sc->kue_mcfilters, i * ETHER_ADDR_LEN);
 
 	kue_setword(sc, KUE_CMD_SET_PKT_FILTER, sc->kue_rxfilt);
 }