[PATCH net-next 0/2] macvlan: fix the problem when mac address changes for passthru mode

[PATCH net-next 0/2] macvlan: fix the problem when mac address changes for passthru mode

[Ding Tianhong]

Ding Tianhong (2):
  macvlan: don't change macvlan address alone for passthru mode
  macvlan: propagate lowerdev mac address changes for passthru mode

 drivers/net/macvlan.c | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

-- 
1.8.0

[PATCH net-next 1/2] macvlan: don't change macvlan address alone for passthru mode

[Ding Tianhong]

The macvlan dev should have the same address like lowerdev when in the passthru
mode, change the upperdev address alone will break the work mechanism, so don't
change the upper dev address for passthru mode.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/macvlan.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index a665e90..7bd9082 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -494,35 +494,45 @@ hash_del:
 	return 0;
 }
 
-static int macvlan_set_mac_address(struct net_device *dev, void *p)
+static int macvlan_sync_address(struct net_device *dev, unsigned char *addr)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
 	struct net_device *lowerdev = vlan->lowerdev;
-	struct sockaddr *addr = p;
 	int err;
 
-	if (!is_valid_ether_addr(addr->sa_data))
-		return -EADDRNOTAVAIL;
-
 	if (!(dev->flags & IFF_UP)) {
 		/* Just copy in the new address */
-		memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
+		ether_addr_copy(dev->dev_addr, addr);
 	} else {
 		/* Rehash and update the device filters */
-		if (macvlan_addr_busy(vlan->port, addr->sa_data))
+		if (macvlan_addr_busy(vlan->port, addr))
 			return -EBUSY;
 
-		err = dev_uc_add(lowerdev, addr->sa_data);
+		err = dev_uc_add(lowerdev, addr);
 		if (err)
 			return err;
 
 		dev_uc_del(lowerdev, dev->dev_addr);
 
-		macvlan_hash_change_addr(vlan, addr->sa_data);
+		macvlan_hash_change_addr(vlan, addr);
 	}
 	return 0;
 }
 
+static int macvlan_set_mac_address(struct net_device *dev, void *p)
+{
+	struct macvlan_dev *vlan = netdev_priv(dev);
+	struct sockaddr *addr = p;
+
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EADDRNOTAVAIL;
+
+	if (vlan->mode == MACVLAN_MODE_PASSTHRU)
+		return -EPERM;
+
+	return macvlan_sync_address(dev, addr->sa_data);
+}
+
 static void macvlan_change_rx_flags(struct net_device *dev, int change)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
-- 
1.8.0

[PATCH net-next 2/2] macvlan: propagate lowerdev mac address changes for passthru mode

[Ding Tianhong]

The macvlan dev should have the same mac address like lowerdev for passthru
mode, so when the lowerdev address changes we should change the macvlan dev
to the new mac address. This patch adds that propagation.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/macvlan.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 7bd9082..55faff3 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1116,6 +1116,18 @@ static int macvlan_device_event(struct notifier_block *unused,
 			dev_set_mtu(vlan->dev, dev->mtu);
 		}
 		break;
+	case NETDEV_CHANGEADDR:
+		if (!port->passthru)
+			return NOTIFY_DONE;
+
+		vlan = list_first_entry_or_null(&port->vlans,
+						struct macvlan_dev,
+						list);
+
+		if (macvlan_sync_address(vlan->dev, dev->dev_addr))
+			return NOTIFY_BAD;
+
+		break;
 	case NETDEV_UNREGISTER:
 		/* twiddle thumbs on netns device moves */
 		if (dev->reg_state != NETREG_UNREGISTERING)
-- 
1.8.0

Re: [PATCH net-next 0/2] macvlan: fix the problem when mac address changes for passthru mode

[Vlad Yasevich]

On 05/26/2014 08:10 AM, Ding Tianhong wrote:
> Ding Tianhong (2):
>   macvlan: don't change macvlan address alone for passthru mode
>   macvlan: propagate lowerdev mac address changes for passthru mode
> 
>  drivers/net/macvlan.c | 40 +++++++++++++++++++++++++++++++---------
>  1 file changed, 31 insertions(+), 9 deletions(-)
> 

This ends up working kind-of backwards...  A user now has to know
that passthru macvlans he has to change the lower-level mac, but
for all others, changing the macvlan mac is ok and will work...

This also will not work if the passthru macvlan is stacked on
something other the hw (for example: a vlan).

Why not simply propagate the mac address change of the passhtru macvlan
down to the real device?

Thanks
-vlad

Re: [PATCH net-next 0/2] macvlan: fix the problem when mac address changes for passthru mode

[Ding Tianhong]

On 2014/5/29 0:24, Vlad Yasevich wrote:
> On 05/26/2014 08:10 AM, Ding Tianhong wrote:
>> Ding Tianhong (2):
>>   macvlan: don't change macvlan address alone for passthru mode
>>   macvlan: propagate lowerdev mac address changes for passthru mode
>>
>>  drivers/net/macvlan.c | 40 +++++++++++++++++++++++++++++++---------
>>  1 file changed, 31 insertions(+), 9 deletions(-)
>>
> 
> This ends up working kind-of backwards...  A user now has to know
> that passthru macvlans he has to change the lower-level mac, but
> for all others, changing the macvlan mac is ok and will work...

Yes, more rules for it, but I think a user should know it.

> 
> This also will not work if the passthru macvlan is stacked on
> something other the hw (for example: a vlan).

passthru is a special mode, will not be used for every situation.

> 
> Why not simply propagate the mac address change of the passhtru macvlan
> down to the real device?
> 
Yes, I am considering this problem for a long time, I'm not sure the macvlan should propagate the mac address change to
the real device, but your suggestion looks good to me, thanks for your advise, I will think about it and fix it.

Ding

> Thanks
> -vlad
> 
>