Fix IXP4xx Ethernet RX with both IFF_ALLMULTI and IFF_PROMISC.

PATCH In review Krzysztof Halasa 2 participants 2 messages Mar 9, 2015 – Mar 10, 2015

Subsystems (3)

Networking Network drivers Arm/Intel IXP4XX Arm architecture

lore

Share & tools

lore

Krzysztof Halasa

author

2015-03-09 22:14 UTC #0 lore raw

PATCH 1/1 Fix IXP4xx Ethernet RX with both IFF_ALLMULTI and IFF_PROMISC.

This long standing bug caused IFF_PROMISC to be ignored when IFF_ALLMULTI was requested.

Also, without any multicast address configured, the driver will use device MAC address for the multicast filter, instead of accepting all inbound traffic.

Signed-off-by Krzysztof Hałasa <[email protected]>

drivers/net/ethernet/xscale/ixp4xx_eth.c +26 ~~-24~~

@@ -933,40 +933,42 @@ static int eth_xmit(struct sk_buff *skb, struct net_device *dev)
 static void eth_set_mcast_list(struct net_device *dev)
 {
 	struct port *port = netdev_priv(dev);
-	struct netdev_hw_addr *ha;
-	u8 diffs[ETH_ALEN], *addr;
 	int i;
 	static const u8 allmulti[] = { 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 };
 
-	if (dev->flags & IFF_ALLMULTI) {
-		for (i = 0; i < ETH_ALEN; i++) {
-			__raw_writel(allmulti[i], &port->regs->mcast_addr[i]);
-			__raw_writel(allmulti[i], &port->regs->mcast_mask[i]);
-		}
-		__raw_writel(DEFAULT_RX_CNTRL0 | RX_CNTRL0_ADDR_FLTR_EN,
-			&port->regs->rx_control[0]);
-		return;
-	}
-
-	if ((dev->flags & IFF_PROMISC) || netdev_mc_empty(dev)) {
+	if (dev->flags & IFF_PROMISC) {
 		__raw_writel(DEFAULT_RX_CNTRL0 & ~RX_CNTRL0_ADDR_FLTR_EN,
 			     &port->regs->rx_control[0]);
 		return;
 	}
 
-	memset(diffs, 0, ETH_ALEN);
+	if (dev->flags & IFF_ALLMULTI) {
+		for (i = 0; i < ETH_ALEN; i++) {
+			__raw_writel(allmulti[i], &port->regs->mcast_addr[i]);
+			__raw_writel(allmulti[i], &port->regs->mcast_mask[i]);
+		}
+	} else {
+		struct netdev_hw_addr *ha;
+		u8 diffs[ETH_ALEN], *addr = NULL;
+
+		memset(diffs, 0, ETH_ALEN);
+
+		/* calculate bits which differ between multicast addresses */
+		netdev_for_each_mc_addr(ha, dev) {
+			if (!addr)
+				addr = ha->addr; /* first multicast address */
+			else
+				for (i = 0; i < ETH_ALEN; i++)
+					diffs[i] |= addr[i] ^ ha->addr[i];
+		}
 
-	addr = NULL;
-	netdev_for_each_mc_addr(ha, dev) {
-		if (!addr)
-			addr = ha->addr; /* first MAC address */
-		for (i = 0; i < ETH_ALEN; i++)
-			diffs[i] |= addr[i] ^ ha->addr[i];
-	}
+		if (!addr) /* use device MAC address instead */
+			addr = dev->dev_addr;
 
-	for (i = 0; i < ETH_ALEN; i++) {
-		__raw_writel(addr[i], &port->regs->mcast_addr[i]);
-		__raw_writel(~diffs[i], &port->regs->mcast_mask[i]);
+		for (i = 0; i < ETH_ALEN; i++) {
+			__raw_writel(addr[i], &port->regs->mcast_addr[i]);
+			__raw_writel(~diffs[i], &port->regs->mcast_mask[i]);
+		}
 	}
 
 	__raw_writel(DEFAULT_RX_CNTRL0 | RX_CNTRL0_ADDR_FLTR_EN,

Signature · 1 line

Krzysztof Halasa

David Miller

maintainer

2015-03-10 17:32 UTC #1 lore raw

From: Krzysztof Halasa <[email protected]>
Date: Mon, 09 Mar 2015 23:07:28 +0100

Quoted 8 lines

This long standing bug caused IFF_PROMISC to be ignored when IFF_ALLMULTI was requested.
Also, without any multicast address configured, the driver will use device MAC address for the multicast filter, instead of accepting all inbound traffic.
Signed-off-by Krzysztof Hałasa <[email protected]>

One big consequence of not basing your patches on the correct tree, is that you don't even notice when the bug you are fixing is already fixed (whether partially or fully).

Please see:

commit f81edc6ac1e1e2e2cbe98bcd6ef5ebb7afb00807

Author: Derrick Pallas <[email protected]>
Date:   Wed Feb 18 00:50:25 2015 -0800

ethernet/ixp4xx: prevent allmulti from clobbering promisc

If both promisc and allmulti are set, promisc should trump allmulti and disable the MAC filter; otherwise, the interface is not really promisc.

Previously, this code checked IFF_ALLMULTI prior to and without regard for IFF_PROMISC; if both were set, only multicast and direct unicast traffic would make it through the filter.

Signed-off-by Derrick Pallas <[email protected]>

Signed-off-by David S. Miller <[email protected]>

drivers/net/ethernet/xscale/ixp4xx_eth.c +1 -1

index f7e0f0f..9e16a28 100644
@@ -938,7 +938,7 @@ static void eth_set_mcast_list(struct net_device *dev)
 	int i;
 	static const u8 allmulti[] = { 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 };
 
-	if (dev->flags & IFF_ALLMULTI) {
+	if ((dev->flags & IFF_ALLMULTI) && !(dev->flags & IFF_PROMISC)) {
 		for (i = 0; i < ETH_ALEN; i++) {
 			__raw_writel(allmulti[i], &port->regs->mcast_addr[i]);
 			__raw_writel(allmulti[i], &port->regs->mcast_mask[i]);