| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1287629751.20865.220.camel@localhost>
Date: Thu, 21 Oct 2010 03:55:51 +0100
From: Ben Hutchings <ben@...adent.org.uk>
To: Florian Fainelli <florian@...nwrt.org>
Cc: netdev@...r.kernel.org, Shawn Lin <shawn@....com.tw>,
Marc Leclerc <marc-leclerc@...naturealpha.com>,
Albert Chen <albert.chen@....com.tw>,
David Miller <davem@...emloft.net>
Subject: Re: [PATCH 1/2] r6040: fix multicast operations
On Wed, 2010-10-20 at 22:25 +0100, Florian Fainelli wrote:
> This patch fixes the following issues with the r6040 NIC operating in
> multicast:
>
> 1) When the IFF_ALLMULTI flag is set, we should write 0xffff to the NIC hash
> table registers to make it process multicast traffic
> 2) When the number of multicast address to handle is smaller than MCAST_MAX
> we should use the NIC multicast registers MID1_{L,M,H}.
> 3) The hashing of the address was not correct, due to an invalid substraction
> (15 - (crc & 0x0f)) instead of (crc & 0x0f)
> Reported-by: Marc Leclerc <marc-leclerc@...naturealpha.com>
> Tested-by: Marc Leclerc <marc-leclerc@...naturealpha.com>
> Signed-off-by: Shawn Lin <shawn@....com.tw>
> Signed-off-by: Albert Chen <albert.chen@....com.tw>
> Signed-off-by: Florian Fainelli <florian@...nwrt.org>
> CC: stable@...nel.org
Remember you'll need to provide a different version for 2.6.27.y and
2.6.32.y.
> ---
> diff --git a/drivers/net/r6040.c b/drivers/net/r6040.c
> index 68a8419..3843363 100644
> --- a/drivers/net/r6040.c
> +++ b/drivers/net/r6040.c
> @@ -852,74 +852,90 @@ static void r6040_multicast_list(struct net_device *dev)
> struct r6040_private *lp = netdev_priv(dev);
> void __iomem *ioaddr = lp->base;
> u16 *adrp;
> - u16 reg;
> unsigned long flags;
> struct netdev_hw_addr *ha;
> int i;
>
> - /* MAC Address */
> - adrp = (u16 *)dev->dev_addr;
> - iowrite16(adrp[0], ioaddr + MID_0L);
> - iowrite16(adrp[1], ioaddr + MID_0M);
> - iowrite16(adrp[2], ioaddr + MID_0H);
> -
> - /* Promiscous Mode */
> spin_lock_irqsave(&lp->lock, flags);
>
> /* Clear AMCP & PROM bits */
> - reg = ioread16(ioaddr) & ~0x0120;
> - if (dev->flags & IFF_PROMISC) {
> - reg |= 0x0020;
> + lp->mcr0 = ioread16(ioaddr) & ~0x0120;
> +
> + /* Promiscuous Mode */
> + if (dev->flags & IFF_PROMISC)
> lp->mcr0 |= 0x0020;
> - }
> - /* Too many multicast addresses
> - * accept all traffic */
> - else if ((netdev_mc_count(dev) > MCAST_MAX) ||
> - (dev->flags & IFF_ALLMULTI))
> - reg |= 0x0020;
>
> - iowrite16(reg, ioaddr);
> - spin_unlock_irqrestore(&lp->lock, flags);
> + /* Enable multicast hash table function to
> + * receive all multicast packets.
> + */
> + else if (dev->flags & IFF_ALLMULTI) {
> + lp->mcr0 |= 0x0100;
Please give these flags names.
>
> - /* Build the hash table */
> - if (netdev_mc_count(dev) > MCAST_MAX) {
> - u16 hash_table[4];
> + for (i = 0; i < MCAST_MAX ; i++) {
> + iowrite16(0, ioaddr + MID_1L + 8 * i);
> + iowrite16(0, ioaddr + MID_1M + 8 * i);
> + iowrite16(0, ioaddr + MID_1H + 8 * i);
> + }
> +
> + iowrite16(0xffff, ioaddr + MAR0);
> + iowrite16(0xffff, ioaddr + MAR1);
> + iowrite16(0xffff, ioaddr + MAR2);
> + iowrite16(0xffff, ioaddr + MAR3);
> + }
> +
> + /* Use internal multicast address registers
> + * if the number of multicast addresses is not greater than MCAST_MAX.
> + */
> + else if (netdev_mc_empty(dev)) {
> + for (i = 0; i < MCAST_MAX ; i++) {
> + iowrite16(0, ioaddr + MID_1L + 8 * i);
> + iowrite16(0, ioaddr + MID_1M + 8 * i);
> + iowrite16(0, ioaddr + MID_1H + 8 * i);
> + }
> + } else if (netdev_mc_count(dev) <= MCAST_MAX) {
> + i = 0;
> + netdev_for_each_mc_addr(ha, dev) {
> + 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++;
> + }
What about the unused exact match entries? And why is the empty case
special?
> + }
> + /* Otherwise, Enable multicast hash table function. */
> + else {
> + u16 hash_table[4] = { 0, };
> u32 crc;
>
> - for (i = 0; i < 4; i++)
> - hash_table[i] = 0;
> + lp->mcr0 |= 0x0100;
>
> + for (i = 0; i < MCAST_MAX ; i++) {
> + iowrite16(0, ioaddr + MID_1L + 8 * i);
> + iowrite16(0, ioaddr + MID_1M + 8 * i);
> + iowrite16(0, ioaddr + MID_1H + 8 * i);
> + }
> +
> + /* Build multicast hash table */
> netdev_for_each_mc_addr(ha, dev) {
> char *addrs = ha->addr;
>
> if (!(*addrs & 1))
> continue;
>
> - crc = ether_crc_le(6, addrs);
> + crc = ether_crc(ETH_ALEN, addrs);
You're reversing the order of bits in the CRC, which is not mentioned in
the commit message; are you sure that's right?
> crc >>= 26;
> - hash_table[crc >> 4] |= 1 << (15 - (crc & 0xf));
> + hash_table[crc >> 4] |= 1 << (crc & 0xf);
> }
> +
> /* Fill the MAC hash tables with their values */
> iowrite16(hash_table[0], ioaddr + MAR0);
> iowrite16(hash_table[1], ioaddr + MAR1);
> iowrite16(hash_table[2], ioaddr + MAR2);
> iowrite16(hash_table[3], ioaddr + MAR3);
> }
> - /* 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);
> - }
This conflicts with my patch in
<http://article.gmane.org/gmane.linux.network/174926> which Dave has
already applied (but not pushed out).
Ben.
> - i++;
> - }
> + iowrite16(lp->mcr0, ioaddr);
> +
> + spin_unlock_irqrestore(&lp->lock, flags);
> }
>
> static void netdev_get_drvinfo(struct net_device *dev,
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.
Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)
Powered by blists - more mailing lists