* [PATCH net] Propagate MAC address changes to VLANs
@ 2016-02-29 11:32 Mike Manning
2016-03-03 21:12 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Mike Manning @ 2016-02-29 11:32 UTC (permalink / raw)
To: netdev
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.
Signed-off-by: Mike Manning <mmanning@brocade.com>
---
net/8021q/vlan.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index d2cd9de..2f57cf2 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] Propagate MAC address changes to VLANs
2016-02-29 11:32 [PATCH net] Propagate MAC address changes to VLANs Mike Manning
@ 2016-03-03 21:12 ` David Miller
2016-04-30 10:28 ` Mike Manning
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2016-03-03 21:12 UTC (permalink / raw)
To: mmanning; +Cc: netdev
From: Mike Manning <mmanning@brocade.com>
Date: Mon, 29 Feb 2016 11:32:51 +0000
>
> - /* 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);
>
This dev_uc_add() call removal cannot be correct, if the device is up
we must programe it into the hardware unicast filters and if also
potentially put it into promiscuous mode via __dev_set_rx_mode().
Also your subject line isn't formatted properly, it should be:
[PATCH net] vlan: Propagate MAC address changes properly.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] Propagate MAC address changes to VLANs
2016-03-03 21:12 ` David Miller
@ 2016-04-30 10:28 ` Mike Manning
0 siblings, 0 replies; 5+ messages in thread
From: Mike Manning @ 2016-04-30 10:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On 03/03/2016 09:12 PM, David Miller wrote:
> From: Mike Manning <mmanning@brocade.com>
> Date: Mon, 29 Feb 2016 11:32:51 +0000
>
>>
>> - /* 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);
>>
>
> This dev_uc_add() call removal cannot be correct, if the device is up
> we must programe it into the hardware unicast filters and if also
> potentially put it into promiscuous mode via __dev_set_rx_mode().
>
The call to dev_uc_add() to add a secondary address is only needed if the VLAN MAC is different from that for the physical interface. For the proposed changes, the VLAN MAC is tracking that of the physical interface and so is the same (as typically it does not make sense for these to be different), so dev_uc_add() should not be called. The easiest way to demonstrate equivalence with the original code, where the MAC address has to be set manually, is with some test debugs. Here, first the MAC of the interface itself is changed (so dev_uc_add() is called), then the MAC of the VLAN is changed (so dev_uc_del() is called):
1) ORIGINAL CODE:
ip addr show dev dp0s8 | grep ether
link/ether 52:54:00:1f:06:2a brd ff:ff:ff:ff:ff:ff
ip addr show dev dp0s8.40 | grep ether
link/ether 52:54:00:1f:06:2a brd ff:ff:ff:ff:ff:ff
sudo ip link set dp0s8.40 addr 10:20:30:40:50:61
sudo ip link set dp0s8 addr 10:20:30:40:50:61
ip addr show dev dp0s8 | grep ether
link/ether 10:20:30:40:50:61 brd ff:ff:ff:ff:ff:ff
ip addr show dev dp0s8.40 | grep ether
link/ether 10:20:30:40:50:61 brd ff:ff:ff:ff:ff:ff
[ 3990.332577] --- vlan_dev_set_mac_address: id 40, call dev_uc_add for 10:20:30:40:50:61 on dp
0s8
[ 3990.332579] device dp0s8 entered promiscuous mode
[ 4002.425234] 8021q: --- vlan_sync_address: id 40, for 10:20:30:40:50:61 on dp0s8.40 (from dp0
s8)
[ 4002.425472] --- vlan_sync_address: id 40, call dev_uc_del for 10:20:30:40:50:61 on dp0s8
[ 4002.425475] --- __hw_addr_del_entry: refcount 0 for 10:20:30:40:50:61
[ 4002.425477] device dp0s8 left promiscuous mode
sudo ip link set dp0s8 addr 52:54:00:1f:06:2a
ip addr show dev dp0s8.40 | grep ether
link/ether 10:20:30:40:50:61 brd ff:ff:ff:ff:ff:ff
sudo ip link set dp0s8.40 addr 52:54:00:1f:06:2a
ip addr show dev dp0s8 | grep ether
link/ether 52:54:00:1f:06:2a brd ff:ff:ff:ff:ff:ff
ip addr show dev dp0s8.40 | grep ether
link/ether 52:54:00:1f:06:2a brd ff:ff:ff:ff:ff:ff
[ 4121.606671] --- vlan_sync_address: id, 40, call dev_uc_add for 10:20:30:40:50:61 on dp0s8
[ 4121.606673] device dp0s8 entered promiscuous mode
[ 4147.487780] --- vlan_dev_set_mac_address: id 40, for 52:54:00:1f:06:2a on dp0s8
[ 4147.487782] --- vlan_dev_set_mac_address: id 40, call dev_uc_del for 10:20:30:40:50:61 dp0s8
[ 4147.487784] --- __hw_addr_del_entry: refcount 0 for 10:20:30:40:50:61
[ 4147.487786] device dp0s8 left promiscuous mode
2) WITH IMPROVEMENT FOR VLAN MAC TO FOLLOW THAT OF PHYSICAL INTF, UNLESS EXPLICITLY SET:
ip addr show dev dp0s8 | grep ether
link/ether 52:54:00:1f:06:2a brd ff:ff:ff:ff:ff:ff
ip addr show dev dp0s8.40 | grep ether
link/ether 52:54:00:1f:06:2a brd ff:ff:ff:ff:ff:ff
sudo ip link set dp0s8 addr 10:20:30:40:50:61
ip addr show dev dp0s8 | grep ether
link/ether 10:20:30:40:50:61 brd ff:ff:ff:ff:ff:ff
ip addr show dev dp0s8.40 | grep ether
link/ether 10:20:30:40:50:61 brd ff:ff:ff:ff:ff:ff
[ 196.574789] 8021q: --- vlan_sync_address: id 40, for 10:20:30:40:50:61 on dp0s8.40 (from dp0
s8)
[ 196.575004] --- vlan_sync_address: id 40, update to 10:20:30:40:50:61 on dp0s8.40 (from dp0s
8)
sudo ip link set dp0s8 addr 52:54:00:1f:06:2a
ip addr show dev dp0s8 | grep ether
link/ether 52:54:00:1f:06:2a brd ff:ff:ff:ff:ff:ff
ip addr show dev dp0s8.40 | grep ether
link/ether 52:54:00:1f:06:2a brd ff:ff:ff:ff:ff:ff
[ 265.683313] 8021q: --- vlan_sync_address: id 40, for 52:54:00:1f:06:2a on dp0s8.40 (from dp0
s8)
[ 265.683534] --- vlan_sync_address: id 40, update to 52:54:00:1f:06:2a on dp0s8.40 (from dp0s
8)
sudo ip link set dp0s8.40 addr 10:20:30:40:50:61
sudo ip link set dp0s8 addr 10:20:30:40:50:99
ip addr show dev dp0s8 | grep ether
link/ether 10:20:30:40:50:99 brd ff:ff:ff:ff:ff:ff
ip addr show dev dp0s8.40 | grep ether
link/ether 10:20:30:40:50:61 brd ff:ff:ff:ff:ff:ff
sudo ip link set dp0s8 addr 10:20:30:40:50:61
[ 5561.791222] --- vlan_dev_set_mac_address: id 40, for 10:20:30:40:50:61 on dp0s8
[ 5561.791225] --- vlan_dev_set_mac_address: id 40, call dev_uc_add for 10:20:30:40:50:61 on dp
0s8
[ 5561.791227] device dp0s8 entered promiscuous mode
[ 5601.050450] 8021q: --- vlan_sync_address: id 40, for 10:20:30:40:50:99 on dp0s8.40 (from dp0
s8)
[ 5630.258345] 8021q: --- vlan_sync_address: id 40, for 10:20:30:40:50:61 on dp0s8.40 (from dp0
s8)
[ 5630.258497] --- vlan_sync_address: id 40, call dev_uc_del for 10:20:30:40:50:61 on dp0s8
[ 5630.258499] --- __hw_addr_del_entry: refcount 0 for 10:20:30:40:50:61
[ 5630.258501] device dp0s8 left promiscuous mode
sudo ip link set dp0s8.40 addr 52:54:00:1f:06:2a
sudo ip link set dp0s8 addr 52:54:00:1f:06:2a
[ 5730.706196] --- vlan_dev_set_mac_address: id 40, for 52:54:00:1f:06:2a on dp0s8
[ 5730.706199] --- vlan_dev_set_mac_address: id 40, call dev_uc_add for 52:54:00:1f:06:2a on dp
0s8
[ 5730.706211] device dp0s8 entered promiscuous mode
[ 5737.378102] 8021q: --- vlan_sync_address: id 40, for 52:54:00:1f:06:2a on dp0s8.40 (from dp0
s8)
[ 5737.378236] --- vlan_sync_address: id 40, call dev_uc_del for 52:54:00:1f:06:2a on dp0s8
[ 5737.378238] --- __hw_addr_del_entry: refcount 0 for 52:54:00:1f:06:2a
[ 5737.378239] device dp0s8 left promiscuous mode
> Also your subject line isn't formatted properly, it should be:
>
> [PATCH net] vlan: Propagate MAC address changes properly.
>
Will resend with correct subject line, apologies for delay in replying.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] Propagate MAC address changes to VLANs
2016-02-23 20:56 Mike Manning
@ 2016-02-25 21:15 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2016-02-25 21:15 UTC (permalink / raw)
To: mmanning; +Cc: netdev
From: Mike Manning <mmanning@brocade.com>
Date: Tue, 23 Feb 2016 20:56:31 +0000
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -293,15 +293,15 @@ static void vlan_sync_address(struct net
>
> /* 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) &&
This patch is corrupted by your email client.
Please read Documentation/email-clients.txt, fix your setup, and email a
test patch to yourself.
Only resubmit this patch to the list once you are able to successfully
apply the patch in that test email.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net] Propagate MAC address changes to VLANs
@ 2016-02-23 20:56 Mike Manning
2016-02-25 21:15 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Mike Manning @ 2016-02-23 20:56 UTC (permalink / raw)
To: netdev
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.
Signed-off-by: Mike Manning <mmanning@brocade.com>
---
net/8021q/vlan.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -293,15 +293,15 @@ static void vlan_sync_address(struct net
/* 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 noti
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:
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-04-30 10:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-29 11:32 [PATCH net] Propagate MAC address changes to VLANs Mike Manning
2016-03-03 21:12 ` David Miller
2016-04-30 10:28 ` Mike Manning
-- strict thread matches above, loose matches on Subject: below --
2016-02-23 20:56 Mike Manning
2016-02-25 21:15 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.