vlan: Propagate MAC address to VLANs

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.

The VLANs should continue inheriting the MAC address of the physical interface until the VLAN MAC address is explicitly set to any value. This allows IPv6 EUI64 addresses for the VLAN to reflect any changes to the MAC of the physical interface and thus for DAD to behave as expected.

Signed-off-by Mike Manning <[email protected]>

---
 net/8021q/vlan.c     |    7 +++++++
 net/8021q/vlan_dev.c |   14 ++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

net/8021q/vlan.c +1 -0

 	/* 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) &&

@@ -303,6 +309,7 @@ static void vlan_sync_address(struct net
 	    !ether_addr_equal(vlandev->dev_addr, dev->dev_addr))
 		dev_uc_add(dev, vlandev->dev_addr);
 
+out:
 	ether_addr_copy(vlan->real_dev_addr, dev->dev_addr);
 }
 

net/8021q/vlan_dev.c +5 -0

+		} else {
+			err = dev_uc_add(real_dev, dev->dev_addr);
+			if (err < 0)
+				goto out;
+		}
 	}
 
 	if (dev->flags & IFF_ALLMULTI) {

net/8021q/vlan_dev.c +1 -0

+	}
 	if (is_zero_ether_addr(dev->broadcast))
 		memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len);
 

Signature · 1 line

1.7.10.4

Quoted 19 lines · Mike Manning

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.

The VLANs should continue inheriting the MAC address of the physical interface until the VLAN MAC address is explicitly set to any value. This allows IPv6 EUI64 addresses for the VLAN to reflect any changes to the MAC of the physical interface and thus for DAD to behave as expected.

Signed-off-by Mike Manning <[email protected]>

---
 net/8021q/vlan.c     |    7 +++++++
 net/8021q/vlan_dev.c |   14 ++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

net/8021q/vlan.c static void vlan_sync_address(struct net · line 299

Quoted 9 lines

net/8021q/vlan.c +6 -0

@@ -291,6 +291,12 @@ static void vlan_sync_address(struct net
 	if (ether_addr_equal(vlan->real_dev_addr, dev->dev_addr))
 		return;
 
+	/* vlan continues to inherit address of parent interface */
+	if (vlandev->addr_assign_type == NET_ADDR_STOLEN) {
+		ether_addr_copy(vlandev->dev_addr, dev->dev_addr);
+		goto out;
+	}
+

I might have missed something in the previous discussion but as ether_addr_copy() is just an optimized memcpy(), how is this going to handle the setups where the vlan device itself has an upper device? For example,

- if it is a bridge port, how is the bridge going to learn about its address change so that it can update its FDB?
- if it is a bond slave or team port, current code preserves the vlan device address on real device change so everything is fine; your proposal would change vlan device's address without bond being even notified, I believe
- there might be a macvlan on top of the vlan and you might accidentally match its address with the new one

Quoted 14 lines

net/8021q/vlan.c +1 -0

 	/* 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) &&

@@ -303,6 +309,7 @@ static void vlan_sync_address(struct net
 	    !ether_addr_equal(vlandev->dev_addr, dev->dev_addr))
 		dev_uc_add(dev, vlandev->dev_addr);
 
+out:
 	ether_addr_copy(vlan->real_dev_addr, dev->dev_addr);
 }
 

net/8021q/vlan_dev.c static int vlan_dev_open(struct net_devi · line 259

Quoted 8 lines

net/8021q/vlan_dev.c +2 -3

@@ -255,9 +255,13 @@ static int vlan_dev_open(struct net_devi
 		return -ENETDOWN;
 
 	if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr)) {
-		err = dev_uc_add(real_dev, dev->dev_addr);
-		if (err < 0)
-			goto out;
+		if (dev->addr_assign_type == NET_ADDR_STOLEN) {
+			ether_addr_copy(dev->dev_addr, real_dev->dev_addr);

The same question here.

Quoted 9 lines

net/8021q/vlan_dev.c +5 -0

+		} else {
+			err = dev_uc_add(real_dev, dev->dev_addr);
+			if (err < 0)
+				goto out;
+		}
 	}
 
 	if (dev->flags & IFF_ALLMULTI) {

net/8021q/vlan_dev.c static int vlan_dev_init(struct net_devi · line 567

Quoted 7 lines

net/8021q/vlan_dev.c +2 -1

@@ -558,8 +562,10 @@ static int vlan_dev_init(struct net_devi
 	/* ipv6 shared card related stuff */
 	dev->dev_id = real_dev->dev_id;
 
-	if (is_zero_ether_addr(dev->dev_addr))
+	if (is_zero_ether_addr(dev->dev_addr)) {
 		eth_hw_addr_inherit(dev, real_dev);
+		dev->addr_assign_type = NET_ADDR_STOLEN;

You might want to replace eth_hw_addr_inherit() with ether_addr_copy() here as they only differ in the former copying addr_assign_type which you are going to rewrite anyway. (But as both are most likely inlined, I would expect the resulting code to be the same in the end.)

Michal Kubecek

Quoted 29 lines

net/8021q/vlan_dev.c +1 -0

+	}
 	if (is_zero_ether_addr(dev->broadcast))
 		memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len);
 

Signature · 1 line

1.7.10.4