vlan: Propagate MAC address changes properly

The MAC address of the physical interface is only copied to the VLAN when it is first created, resulting in an inconsistency after MAC address changes of only newly created VLANs having an up-to-date MAC.

Continuing to inherit the MAC address unless explicitly changed for the VLAN allows IPv6 EUI64 addresses for the VLAN to reflect the change and thus for DAD to behave as expected for the given MAC.

---
 net/8021q/vlan.c |   17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

index d2cd9de..2f57cf2 100644
@@ -293,15 +293,15 @@ static void vlan_sync_address(struct net_device *dev,
 
 	/* vlan address was different from the old address and is equal to
 	 * the new address */
-	if (!ether_addr_equal(vlandev->dev_addr, vlan->real_dev_addr) &&
+	if ((vlandev->flags & IFF_UP) &&
+	    !ether_addr_equal(vlandev->dev_addr, vlan->real_dev_addr) &&
 	    ether_addr_equal(vlandev->dev_addr, dev->dev_addr))
 		dev_uc_del(dev, vlandev->dev_addr);
 
-	/* vlan address was equal to the old address and is different from
+	/* vlan address was equal to the old address so now also inherit
 	 * the new address */
-	if (ether_addr_equal(vlandev->dev_addr, vlan->real_dev_addr) &&
-	    !ether_addr_equal(vlandev->dev_addr, dev->dev_addr))
-		dev_uc_add(dev, vlandev->dev_addr);
+	if (ether_addr_equal(vlandev->dev_addr, vlan->real_dev_addr))
+		ether_addr_copy(vlandev->dev_addr, dev->dev_addr);
 
 	ether_addr_copy(vlan->real_dev_addr, dev->dev_addr);
 }

@@ -389,13 +389,8 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 
 	case NETDEV_CHANGEADDR:
 		/* Adjust unicast filters on underlying device */
-		vlan_group_for_each_dev(grp, i, vlandev) {
-			flgs = vlandev->flags;
-			if (!(flgs & IFF_UP))
-				continue;
-
+		vlan_group_for_each_dev(grp, i, vlandev)
 			vlan_sync_address(dev, vlandev);
-		}
 		break;
 
 	case NETDEV_CHANGEMTU:

1.7.10.4

From: Mike Manning <[email protected]>
Date: Sat, 30 Apr 2016 11:32:37 +0100

What is this code really trying to achieve?

Is it "Propagate real device MAC changes to undelying vlan device, but not if the user set the vlan MAC explicitly."?

If so, implement that instead of all of these confusing tests.

If the vlan device's set_mac_address operation is ever called, set a boolean value in the vlan device private to true and test it here.

Right, I will update the subject header to make this clearer

Given that this information is implicit in real_dev_addr, I am reluctant to add another member to the vlan_dev_priv data structure, especially given that there may be a large number of VLANs. Instead I have added a variable real_addr_in_use in vlan_sync_address() to make this clearer.

From: Mike Manning <[email protected]>
Date: Tue, 3 May 2016 16:18:42 +0100

That's not the reason to make or not make the simplification I asked you do.

You can add a multi-page comment to this code, and the confusing tests are still rediculous.

Please add the boolean state as I asked you to.