multicast reception fails for certain groups on interfaces with TI ThunderLAN chip

[Nick Briggs]

Synopsis: multicast reception fails for certain groups on interfaces with TI ThunderLAN chip
Category: system

System : OpenBSD 6.1
Details : OpenBSD 6.1-current (GENERIC) #4: Tue Jul 11 21:35:56 PDT 2017
***@pigeon:/usr/obj/sys/arch/i386/compile/GENERIC

Architecture: OpenBSD.i386
Machine : i386
Applications running on a system with a TI ThunderLAN ethernet controller chip,
for example the "Compaq Netelligent 10/100TX", that receive multicast packets may find
that no packets are received for certain multicast hardware addresses.
On problem system: with openmdns-0.7 package installed and running on a "tl" interface
# tcpdump -p -w -i tl0 multicast
check whether any multicast DNS packets are seen to destination 01:00:5e:00:00:fb
(generate traffic from, for example "ping problemhost.local" from an mdns aware system on the local network)
if NO IPv4 multicast packets to 224.0.0.251 are observed the problem has been replicated.
if packets such as

22:28:45.774775 00:25:00:3e:ae:2d 01:00:5e:00:00:fb ip 77: 192.168.42.64.mdns > 224.0.0.251.mdns: 0 A (QU)? problemhost.local. (35)

are reported the problem is fixed.
The code in if_tl.c, tl_calchash() is wrong in that it doesn't compensate for signed char type so that the XOR operations to generate the multicast hash index are polluted by sign extension if the MSbit of the 1st and 4th or 2nd and 5th bytes of the multicast destination ethernet address are not equal. This is the patch for that problem:

Index: if_tl.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_tl.c,v
retrieving revision 1.70
diff -u -p -r1.70 if_tl.c
--- if_tl.c 22 Jan 2017 10:17:38 -0000 1.70
+++ if_tl.c 12 Jul 2017 05:08:46 -0000
@@ -751,8 +751,8 @@ tl_calchash(caddr_t addr)
{
int t;

- t = (addr[0] ^ addr[3]) << 16 | (addr[1] ^ addr[4]) << 8 |
- (addr[2] ^ addr[5]);
+ t = (addr[0] ^ addr[3]) << 16 | (0xff & (addr[1] ^ addr[4])) << 8 |
+ (0xff & (addr[2] ^ addr[5]));
return ((t >> 18) ^ (t >> 12) ^ (t >> 6) ^ t) & 0x3f;
}

dmesg:
(this kernel has had the above patch applied and now functions correctly)
OpenBSD 6.1-current (GENERIC) #4: Tue Jul 11 21:35:56 PDT 2017
***@pigeon:/usr/obj/sys/arch/i386/compile/GENERIC
cpu0: Intel Pentium II ("GenuineIntel" 686-class, 512KB L2 cache) 300 MHz
cpu0: FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,SEP,MTRR,PGE,MCA,CMOV,MMX,PERF
real mem = 402145280 (383MB)
avail mem = 381214720 (363MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: date 05/12/99, BIOS32 rev. 0 @ 0xed000
apm0 at bios0: Power Management spec V1.2
pcibios0 at bios0: rev 2.1 @ 0xed000/0x3000
pcibios0: PCI IRQ Routing Table rev 1.0 @ 0xf6fa0/160 (8 entries)
pcibios0: PCI Interrupt Router at 000:20:0 ("Intel 82371AB PIIX4 ISA" rev 0x00)
pcibios0: PCI bus #1 is the last bus
bios0: ROM list: 0xc0000/0x8000 0xc8000/0x3800
cpu0 at mainbus0: (uniprocessor)
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
pci0 at mainbus0 bus 0: configuration mode 1 (bios)
pchb0 at pci0 dev 0 function 0 "Intel 82443LX AGP" rev 0x03
intelagp0 at pchb0
agp0 at intelagp0: aperture at 0x44000000, size 0x4000000
ppb0 at pci0 dev 1 function 0 "Intel 82443LX AGP" rev 0x03
pci1 at ppb0 bus 1
vga1 at pci1 dev 0 function 0 "Matrox MGA Millennium II 2164WA-B AGP" rev 0x00
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
ahc0 at pci0 dev 15 function 0 "Adaptec AIC-7860" rev 0x03: irq 10
ahc0: Host Adapter Bios disabled. Using default SCSI device parameters
scsibus1 at ahc0: 8 targets, initiator 7
tl0 at pci0 dev 16 function 0 "Compaq Embedded Netelligent 10/100TX" rev 0x10: irq 11 address 00:80:5f:bd:53:49
lxtphy0 at tl0 phy 1: LXT970 10/100 PHY, rev. 0
tlphy0 at tl0 phy 31: ThunderLAN 10baseT PHY, rev. 6
piixpcib0 at pci0 dev 20 function 0 "Intel 82371AB PIIX4 ISA" rev 0x01
pciide0 at pci0 dev 20 function 1 "Intel 82371AB IDE" rev 0x01: DMA, channel 0 wired to compatibility, channel 1 wired to compatibility
wd0 at pciide0 channel 0 drive 0: <ST340014A>
wd0: 16-sector PIO, LBA48, 38146MB, 78125000 sectors
wd1 at pciide0 channel 0 drive 1: <WDC WD1600AAJB-00J3A0>
wd1: 16-sector PIO, LBA48, 152627MB, 312581808 sectors
wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 2
wd1(pciide0:0:1): using PIO mode 4, Ultra-DMA mode 2
atapiscsi0 at pciide0 channel 1 drive 0
scsibus2 at atapiscsi0: 2 targets
cd0 at scsibus2 targ 0 lun 0: <YAMAHA, CRW2100E, 1.0N> ATAPI 5/cdrom removable
cd0(pciide0:1:0): using PIO mode 4, Ultra-DMA mode 1
uhci0 at pci0 dev 20 function 2 "Intel 82371AB USB" rev 0x01: irq 11
piixpm0 at pci0 dev 20 function 3 "Intel 82371AB Power" rev 0x01: SMI
iic0 at piixpm0
spdmem0 at iic0 addr 0x50: 128MB SDRAM ECC PC100CL3
spdmem1 at iic0 addr 0x51: 128MB SDRAM ECC PC100CL3
spdmem2 at iic0 addr 0x52: 128MB SDRAM ECC PC66CL2
isa0 at piixpcib0
isadma0 at isa0
fdc0 at isa0 port 0x3f0/6 irq 6 drq 2
fd0 at fdc0 drive 0: 1.44MB 80 cyl, 2 head, 18 sec
com0 at isa0 port 0x3f8/8 irq 4: ns16550a, 16 byte fifo
com1 at isa0 port 0x2f8/8 irq 3: ns16550a, 16 byte fifo
pckbc0 at isa0 port 0x60/5 irq 1 irq 12
pckbd0 at pckbc0 (kbd slot)
wskbd0 at pckbd0: console keyboard, using wsdisplay0
pms0 at pckbc0 (aux slot)
wsmouse0 at pms0 mux 0
pcppi0 at isa0 port 0x61
spkr0 at pcppi0
lpt0 at isa0 port 0x378/4 irq 7
npx0 at isa0 port 0xf0/16: reported by CPUID; using exception 16
isapnp0 at isa0 port 0x279: read port 0x203
"ESS ES1869 Plug and Play AudioD, ESS0006, , " at isapnp0 port 0x800/8 not configured
ess0 at isapnp0 "ESS ES1869 Plug and Play AudioD, ESS1869, , " port 0x220/16,0x388/4,0x330/2 irq 5 drq 1,0: ESS Technology ES1869 [version 0x688b]
ess0: audio1 interrupting at irq 5
audio0 at ess0
opl at ess0 not configured
usb0 at uhci0: USB revision 1.0
uhub0 at usb0 configuration 1 interface 0 "Intel UHCI root hub" rev 1.00/1.00 addr 1
uhidev0 at uhub0 port 2 configuration 1 interface 0 "American Power Conversion Back-UPS RS 1500G FW:865.L6 .D USB FW:L6" rev 2.00/0.90 addr 2
uhidev0: iclass 3/0, 146 report ids
upd0 at uhidev0
vscsi0 at root
scsibus3 at vscsi0: 256 targets
softraid0 at root
scsibus4 at softraid0: 256 targets
root on wd1a (c082219326d04173.a) swap on wd1b dump on wd1b

usbdevs:
Controller /dev/usb0:
addr 1: full speed, self powered, config 1, UHCI root hub(0x0000), Intel(0x8086), rev 1.00
port 1 powered
port 2 addr 2: full speed, power 2 mA, config 1, Back-UPS RS 1500G FW:865.L6 .D USB FW:L6(0x0002), American Power Conversion(0x051d), rev 0.90, iSerialNumber 4B1634P17214

[Christian Weisgerber]

We should just use unsigned chars.
Kill a prototype for a nonexisting function in a neighboring file,
while here.

Index: if_tl.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_tl.c,v
retrieving revision 1.70
diff -u -p -r1.70 if_tl.c
--- if_tl.c 22 Jan 2017 10:17:38 -0000 1.70
+++ if_tl.c 13 Jul 2017 14:39:09 -0000
@@ -272,7 +272,7 @@ void tl_miibus_writereg(struct device *,
void tl_miibus_statchg(struct device *);

void tl_setmode(struct tl_softc *, uint64_t);
-int tl_calchash(caddr_t);
+int tl_calchash(u_int8_t *);
void tl_iff(struct tl_softc *);
void tl_setfilt(struct tl_softc *, caddr_t, int);
void tl_softreset(struct tl_softc *, int);
@@ -747,7 +747,7 @@ tl_setmode(struct tl_softc *sc, uint64_t
* the folded 24-bit value is split into 6-bit portions and XOR'd.
*/
int
-tl_calchash(caddr_t addr)
+tl_calchash(u_int8_t *addr)
{
int t;

Index: if_wb.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_wb.c,v
retrieving revision 1.68
diff -u -p -r1.68 if_wb.c
--- if_wb.c 22 Jan 2017 10:17:38 -0000 1.68
+++ if_wb.c 13 Jul 2017 14:36:07 -0000
@@ -154,7 +154,6 @@ int wb_mii_readreg(struct wb_softc *, st
int wb_mii_writereg(struct wb_softc *, struct wb_mii_frame *);

void wb_setcfg(struct wb_softc *, uint64_t);
-u_int8_t wb_calchash(caddr_t);
void wb_setmulti(struct wb_softc *);
void wb_reset(struct wb_softc *);
void wb_fixmedia(struct wb_softc *);

[Nick Briggs]

Yeah, I'm good with that, and in fact enm_addrlo is defined as u_int8_t which I hadn't gone to check.

../sys/netinet/if_ether.h: u_int8_t enm_addrlo[ETHER_ADDR_LEN]; /* low or only address of range */

I've verified the code works:

# tcpdump -e -s 1500 -p multicast and not broadcast
tcpdump: listening on tl0, link-type EN10MB
08:49:37.903615 00:25:00:3e:ae:2d 01:00:5e:00:00:fb ip 72: 192.168.42.64.mdns > 224.0.0.251.mdns: 0 A (QU)? nobody.local. (30)
08:49:38.936298 00:25:00:3e:ae:2d 01:00:5e:00:00:fb ip 72: 192.168.42.64.mdns > 224.0.0.251.mdns: 0 A? nobody.local. (30)
08:49:41.963983 00:25:00:3e:ae:2d 01:00:5e:00:00:fb ip 72: 192.168.42.64.mdns > 224.0.0.251.mdns: 0 A? nobody.local. (30)

Thanks.

-- Nick Briggs

Post by Christian Weisgerber
We should just use unsigned chars.
Kill a prototype for a nonexisting function in a neighboring file,
while here.
Index: if_tl.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_tl.c,v
retrieving revision 1.70
diff -u -p -r1.70 if_tl.c
--- if_tl.c 22 Jan 2017 10:17:38 -0000 1.70
+++ if_tl.c 13 Jul 2017 14:39:09 -0000
@@ -272,7 +272,7 @@ void tl_miibus_writereg(struct device *,
void tl_miibus_statchg(struct device *);
void tl_setmode(struct tl_softc *, uint64_t);
-int tl_calchash(caddr_t);
+int tl_calchash(u_int8_t *);
void tl_iff(struct tl_softc *);
void tl_setfilt(struct tl_softc *, caddr_t, int);
void tl_softreset(struct tl_softc *, int);
@@ -747,7 +747,7 @@ tl_setmode(struct tl_softc *sc, uint64_t
* the folded 24-bit value is split into 6-bit portions and XOR'd.
*/
int
-tl_calchash(caddr_t addr)
+tl_calchash(u_int8_t *addr)
{
int t;
Index: if_wb.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_wb.c,v
retrieving revision 1.68
diff -u -p -r1.68 if_wb.c
--- if_wb.c 22 Jan 2017 10:17:38 -0000 1.68
+++ if_wb.c 13 Jul 2017 14:36:07 -0000
@@ -154,7 +154,6 @@ int wb_mii_readreg(struct wb_softc *, st
int wb_mii_writereg(struct wb_softc *, struct wb_mii_frame *);
void wb_setcfg(struct wb_softc *, uint64_t);
-u_int8_t wb_calchash(caddr_t);
void wb_setmulti(struct wb_softc *);
void wb_reset(struct wb_softc *);
void wb_fixmedia(struct wb_softc *);
--