r6040: Fix multicast filter some more
Subsystems (2)
PATCH 1/1 r6040: Fix multicast filter some more
This code has been broken forever, but in several different and creative ways.
So far as I can work out, the R6040 MAC filter has 4 exact-match entries, the first of which the driver uses for its assigned unicast address, plus a 64-entry hash-based filter for multicast addresses (maybe unicast as well?).
The original version of this code would write the first 4 multicast addresses as exact-match entries from offset 1 (bug #1: there is no entry 4 so this could write to some PHY registers). It would fill the remainder of the exact-match entries with the broadcast address (bug addresses were configured, it would set up the hash table, write some random crap to the MAC control register (bug #3) and finally walk off the end of the list when filling the exact-match entries (bug #4).
All of this seems to be pointless, since it sets the promiscuous bit when the interface is made promiscuous or if >4 multicast addresses are enabled, and never clears it (bug #5, masking bug #2).
The recent(ish) changes to the multicast list fixed bug #4, but completely removed the limit on iteration over the exact-match entries (bug #6).
Bug #4 was reported as
<https://bugzilla.kernel.org/show_bug.cgi?id=15355> and more recently as <http://bugs.debian.org/600155>. Florian Fainelli attempted to fix these in commit 3bcf8229a8c49769e48d3e0bd1e20d8e003f8106, but that actually dealt with bugs #1-3, bug #4 having been fixed in mainline at that point.
That commit fixes the most important current bug #6.
---
Compile-tested only.
Ben.
drivers/net/r6040.c | 22 ++++++++++++---------- 1 files changed, 12 insertions(+), 10 deletions(-)
drivers/net/r6040.c +12 -10
@@ -893,16 +893,18 @@ static void r6040_multicast_list(struct net_device *dev)/* Multicast Address 1~4 case */ i =0 ;netdev_for_each_mc_addr (ha, dev) { -if (i <MCAST_MAX ) { - adrp = (u16 *) ha->addr; -iowrite16 (adrp[0 ], ioaddr +MID_1L +8 * i); -iowrite16 (adrp[1 ], ioaddr +MID_1M +8 * i); -iowrite16 (adrp[2 ], ioaddr +MID_1H +8 * i); - }else { -iowrite16 (0xffff , ioaddr +MID_1L +8 * i); -iowrite16 (0xffff , ioaddr +MID_1M +8 * i); -iowrite16 (0xffff , ioaddr +MID_1H +8 * i); - } +if (i >=MCAST_MAX ) +break ; + adrp = (u16 *) ha->addr; +iowrite16 (adrp[0 ], ioaddr +MID_1L +8 * i); +iowrite16 (adrp[1 ], ioaddr +MID_1M +8 * i); +iowrite16 (adrp[2 ], ioaddr +MID_1H +8 * i); + i++; + } +while (i <MCAST_MAX ) { +iowrite16 (0xffff , ioaddr +MID_1L +8 * i); +iowrite16 (0xffff , ioaddr +MID_1M +8 * i); +iowrite16 (0xffff , ioaddr +MID_1H +8 * i); i++; } }
Signature · 1 line
1.7.1
PATCH v2 1/1 r6040: Fix multicast filter some more
This code has been broken forever, but in several different and creative ways.
So far as I can work out, the R6040 MAC filter has 4 exact-match entries, the first of which the driver uses for its assigned unicast address, plus a 64-entry hash-based filter for multicast addresses (maybe unicast as well?).
The original version of this code would write the first 4 multicast addresses as exact-match entries from offset 1 (bug #1: there is no entry 4 so this could write to some PHY registers). It would fill the remainder of the exact-match entries with the broadcast address (bug #2:
this would overwrite the last used entry). If more than 4 multicast addresses were configured, it would set up the hash table, write some random crap to the MAC control register (bug #3) and finally walk off the end of the list when filling the exact-match entries (bug #4).
All of this seems to be pointless, since it sets the promiscuous bit when the interface is made promiscuous or if >4 multicast addresses are enabled, and never clears it (bug #5, masking bug #2).
The recent(ish) changes to the multicast list fixed bug #4, but completely removed the limit on iteration over the exact-match entries (bug #6).
Bug #4 was reported as
<https://bugzilla.kernel.org/show_bug.cgi?id=15355> and more recently as <http://bugs.debian.org/600155>. Florian Fainelli attempted to fix these in commit 3bcf8229a8c49769e48d3e0bd1e20d8e003f8106, but that actually dealt with bugs #1-3, bug #4 having been fixed in mainline at that point.
That commit fixes the most important current bug #6.
---
Commit message was slightly mangled in the previous message.
Ben.
drivers/net/r6040.c | 22 ++++++++++++---------- 1 files changed, 12 insertions(+), 10 deletions(-)
drivers/net/r6040.c +12 -10
@@ -893,16 +893,18 @@ static void r6040_multicast_list(struct net_device *dev)/* Multicast Address 1~4 case */ i =0 ;netdev_for_each_mc_addr (ha, dev) { -if (i <MCAST_MAX ) { - adrp = (u16 *) ha->addr; -iowrite16 (adrp[0 ], ioaddr +MID_1L +8 * i); -iowrite16 (adrp[1 ], ioaddr +MID_1M +8 * i); -iowrite16 (adrp[2 ], ioaddr +MID_1H +8 * i); - }else { -iowrite16 (0xffff , ioaddr +MID_1L +8 * i); -iowrite16 (0xffff , ioaddr +MID_1M +8 * i); -iowrite16 (0xffff , ioaddr +MID_1H +8 * i); - } +if (i >=MCAST_MAX ) +break ; + adrp = (u16 *) ha->addr; +iowrite16 (adrp[0 ], ioaddr +MID_1L +8 * i); +iowrite16 (adrp[1 ], ioaddr +MID_1M +8 * i); +iowrite16 (adrp[2 ], ioaddr +MID_1H +8 * i); + i++; + } +while (i <MCAST_MAX ) { +iowrite16 (0xffff , ioaddr +MID_1L +8 * i); +iowrite16 (0xffff , ioaddr +MID_1M +8 * i); +iowrite16 (0xffff , ioaddr +MID_1H +8 * i); i++; } }
Signature · 1 line
1.7.1
From: Ben Hutchings <[email protected]>
Date: Fri, 15 Oct 2010 04:17:16 +0100
Quoted 2 lines
This code has been broken forever, but in several different and creative ways.
...
Quoted 3 lines
That commit fixes the most important current bug #6.
Signed-off-by Ben Hutchings <[email protected]>
Thanks for doing all of this tedious analysis and work.
Applied.