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);
}