[PATCH net-next 00/10] set addr_assign_type when inheriting a dev_addr

[PATCH net-next 00/10] set addr_assign_type when inheriting a dev_addr

[Bjørn Mork]

Copying the dev_addr from a parent device is an operation
common to a number of drivers. The addr_assign_type should
be updated accordingly, either by reusing the value from
the source device or explicitly indicating that the address
is stolen by setting addr_assign_type to NET_ADDR_STOLEN.

This patch set adds a helper copying both the dev_addr and
the addr_assign_type, and use this helper in drivers which
don't currently set the addr_assign_type. Using NET_ADDR_STOLEN
might be more appropriate in some of these cases.  Please
let me know, and I'll update the patch accordingly.


Bjørn Mork (10):
  net: etherdevice: add address inherit helper
  net: vlan: inherit addr_assign_type along with dev_addr
  net: dsa: inherit addr_assign_type along with dev_addr
  net: macvlan: inherit addr_assign_type along with dev_addr
  net: team: inherit addr_assign_type along with dev_addr
  net: airo: inherit addr_assign_type along with dev_addr
  net: hostap: inherit addr_assign_type along with dev_addr
  net: libertas: inherit addr_assign_type along with dev_addr
  staging: vt6655: inherit addr_assign_type along with dev_addr
  staging: vt6656: inherit addr_assign_type along with dev_addr

 drivers/net/macvlan.c                     |    2 +-
 drivers/net/team/team.c                   |    2 +-
 drivers/net/wireless/airo.c               |    2 +-
 drivers/net/wireless/hostap/hostap_hw.c   |    2 +-
 drivers/net/wireless/hostap/hostap_main.c |    2 +-
 drivers/net/wireless/libertas/mesh.c      |    2 +-
 drivers/staging/vt6655/hostap.c           |    2 +-
 drivers/staging/vt6655/ioctl.c            |    2 +-
 drivers/staging/vt6655/wpactl.c           |    2 +-
 drivers/staging/vt6656/hostap.c           |    2 +-
 include/linux/etherdevice.h               |   19 +++++++++++++++++++
 net/8021q/vlan_dev.c                      |    2 +-
 net/dsa/slave.c                           |    2 +-
 13 files changed, 31 insertions(+), 12 deletions(-)

-- 
1.7.10.4

[PATCH net-next 00/10] set addr_assign_type when inheriting a dev_addr

[Bjørn Mork]

Copying the dev_addr from a parent device is an operation
common to a number of drivers. The addr_assign_type should
be updated accordingly, either by reusing the value from
the source device or explicitly indicating that the address
is stolen by setting addr_assign_type to NET_ADDR_STOLEN.

This patch set adds a helper copying both the dev_addr and
the addr_assign_type, and use this helper in drivers which
don't currently set the addr_assign_type. Using NET_ADDR_STOLEN
might be more appropriate in some of these cases.  Please
let me know, and I'll update the patch accordingly.


Bjørn Mork (10):
  net: etherdevice: add address inherit helper
  net: vlan: inherit addr_assign_type along with dev_addr
  net: dsa: inherit addr_assign_type along with dev_addr
  net: macvlan: inherit addr_assign_type along with dev_addr
  net: team: inherit addr_assign_type along with dev_addr
  net: airo: inherit addr_assign_type along with dev_addr
  net: hostap: inherit addr_assign_type along with dev_addr
  net: libertas: inherit addr_assign_type along with dev_addr
  staging: vt6655: inherit addr_assign_type along with dev_addr
  staging: vt6656: inherit addr_assign_type along with dev_addr

 drivers/net/macvlan.c                     |    2 +-
 drivers/net/team/team.c                   |    2 +-
 drivers/net/wireless/airo.c               |    2 +-
 drivers/net/wireless/hostap/hostap_hw.c   |    2 +-
 drivers/net/wireless/hostap/hostap_main.c |    2 +-
 drivers/net/wireless/libertas/mesh.c      |    2 +-
 drivers/staging/vt6655/hostap.c           |    2 +-
 drivers/staging/vt6655/ioctl.c            |    2 +-
 drivers/staging/vt6655/wpactl.c           |    2 +-
 drivers/staging/vt6656/hostap.c           |    2 +-
 include/linux/etherdevice.h               |   19 +++++++++++++++++++
 net/8021q/vlan_dev.c                      |    2 +-
 net/dsa/slave.c                           |    2 +-
 13 files changed, 31 insertions(+), 12 deletions(-)

-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net-next 01/10] net: etherdevice: add address inherit helper

[Bjørn Mork]

Some etherdevices inherit their address from a parent or
master device. The addr_assign_type should be updated along
with the address in these cases.  Adding a helper function
to simplify this.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 include/linux/etherdevice.h |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index c623861..49b6695 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -199,6 +199,25 @@ static inline void eth_hw_addr_random(struct net_device *dev)
 }
 
 /**
+ * eth_hw_addr_inherit - Copy dev_addr from another net_device
+ * @dst: pointer to net_device to copy dev_addr to
+ * @src: pointer to net_device to copy dev_addr from
+ *
+ * Copy the Ethernet address from one net_device to another along with
+ * the addr_assign_type.
+ */
+static inline int eth_hw_addr_inherit(struct net_device *dst,
+				      struct net_device *src)
+{
+	if (dst->addr_len != src->addr_len)
+		return -EINVAL;
+
+	dst->addr_assign_type = src->addr_assign_type;
+	memcpy(dst->dev_addr, src->dev_addr, dst->addr_len);
+	return 0;
+}
+
+/**
  * compare_ether_addr - Compare two Ethernet addresses
  * @addr1: Pointer to a six-byte array containing the Ethernet address
  * @addr2: Pointer other six-byte array containing the Ethernet address
-- 
1.7.10.4

[PATCH net-next 02/10] net: vlan: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

A vlan device inheriting a random or set address should reflect
this in its addr_assign_type.

Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 net/8021q/vlan_dev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 9ab8a7e..09bf1c3 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -582,7 +582,7 @@ static int vlan_dev_init(struct net_device *dev)
 	dev->dev_id = real_dev->dev_id;
 
 	if (is_zero_ether_addr(dev->dev_addr))
-		memcpy(dev->dev_addr, real_dev->dev_addr, dev->addr_len);
+		eth_hw_addr_inherit(dev, real_dev);
 	if (is_zero_ether_addr(dev->broadcast))
 		memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len);
 
-- 
1.7.10.4

[PATCH net-next 03/10] net: dsa: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 net/dsa/slave.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 6ebd8fb..29d684e 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -347,7 +347,7 @@ dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 
 	slave_dev->features = master->vlan_features;
 	SET_ETHTOOL_OPS(slave_dev, &dsa_slave_ethtool_ops);
-	memcpy(slave_dev->dev_addr, master->dev_addr, ETH_ALEN);
+	eth_hw_addr_inherit(slave_dev, master);
 	slave_dev->tx_queue_len = 0;
 
 	switch (ds->dst->tag_protocol) {
-- 
1.7.10.4

[PATCH net-next 04/10] net: macvlan: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/macvlan.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 510a9b6..3bc9761 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -823,7 +823,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 		if (port->count)
 			return -EINVAL;
 		port->passthru = true;
-		memcpy(dev->dev_addr, lowerdev->dev_addr, ETH_ALEN);
+		eth_hw_addr_inherit(dev, lowerdev);
 	}
 
 	err = netdev_upper_dev_link(lowerdev, dev);
-- 
1.7.10.4

[PATCH net-next 05/10] net: team: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/team/team.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 9ccccd4..50e43e6 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1974,7 +1974,7 @@ static void team_setup_by_port(struct net_device *dev,
 	dev->addr_len = port_dev->addr_len;
 	dev->mtu = port_dev->mtu;
 	memcpy(dev->broadcast, port_dev->broadcast, port_dev->addr_len);
-	memcpy(dev->dev_addr, port_dev->dev_addr, port_dev->addr_len);
+	eth_hw_addr_inherit(dev, port_dev);
 }
 
 static int team_dev_type_check_change(struct net_device *dev,
-- 
1.7.10.4

[PATCH net-next 06/10] net: airo: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

Cc: "John W. Linville" <linville@tuxdriver.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/wireless/airo.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index d0adbaf..7fe1964 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -2693,7 +2693,7 @@ static struct net_device *init_wifidev(struct airo_info *ai,
 	dev->base_addr = ethdev->base_addr;
 	dev->wireless_data = ethdev->wireless_data;
 	SET_NETDEV_DEV(dev, ethdev->dev.parent);
-	memcpy(dev->dev_addr, ethdev->dev_addr, dev->addr_len);
+	eth_hw_addr_inherit(dev, ethdev);
 	err = register_netdev(dev);
 	if (err<0) {
 		free_netdev(dev);
-- 
1.7.10.4

[PATCH net-next 07/10] net: hostap: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

Cc: Jouni Malinen <j@w1.fi>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/wireless/hostap/hostap_hw.c   |    2 +-
 drivers/net/wireless/hostap/hostap_main.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/hostap/hostap_hw.c b/drivers/net/wireless/hostap/hostap_hw.c
index 6307a4e..c275dc1 100644
--- a/drivers/net/wireless/hostap/hostap_hw.c
+++ b/drivers/net/wireless/hostap/hostap_hw.c
@@ -1425,7 +1425,7 @@ static int prism2_hw_init2(struct net_device *dev, int initial)
 		}
 		list_for_each(ptr, &local->hostap_interfaces) {
 			iface = list_entry(ptr, struct hostap_interface, list);
-			memcpy(iface->dev->dev_addr, dev->dev_addr, ETH_ALEN);
+			eth_hw_addr_inherit(iface->dev, dev);
 		}
 	} else if (local->fw_ap)
 		prism2_check_sta_fw_version(local);
diff --git a/drivers/net/wireless/hostap/hostap_main.c b/drivers/net/wireless/hostap/hostap_main.c
index e4f56ad..a1257c9 100644
--- a/drivers/net/wireless/hostap/hostap_main.c
+++ b/drivers/net/wireless/hostap/hostap_main.c
@@ -66,7 +66,7 @@ struct net_device * hostap_add_interface(struct local_info *local,
 	list_add(&iface->list, &local->hostap_interfaces);
 
 	mdev = local->dev;
-	memcpy(dev->dev_addr, mdev->dev_addr, ETH_ALEN);
+	eth_hw_addr_inherit(dev, mdev);
 	dev->base_addr = mdev->base_addr;
 	dev->irq = mdev->irq;
 	dev->mem_start = mdev->mem_start;
-- 
1.7.10.4

[PATCH net-next 08/10] net: libertas: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

Cc: "John W. Linville" <linville@tuxdriver.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/wireless/libertas/mesh.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/libertas/mesh.c b/drivers/net/wireless/libertas/mesh.c
index efae07e..6fef746 100644
--- a/drivers/net/wireless/libertas/mesh.c
+++ b/drivers/net/wireless/libertas/mesh.c
@@ -1017,7 +1017,7 @@ static int lbs_add_mesh(struct lbs_private *priv)
 
 	mesh_dev->netdev_ops = &mesh_netdev_ops;
 	mesh_dev->ethtool_ops = &lbs_ethtool_ops;
-	memcpy(mesh_dev->dev_addr, priv->dev->dev_addr, ETH_ALEN);
+	eth_hw_addr_inherit(mesh_dev, priv->dev);
 
 	SET_NETDEV_DEV(priv->mesh_dev, priv->dev->dev.parent);
 
-- 
1.7.10.4

[PATCH net-next 09/10] staging: vt6655: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

Cc: Forest Bond <forest@alittletooquiet.net>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/staging/vt6655/hostap.c |    2 +-
 drivers/staging/vt6655/ioctl.c  |    2 +-
 drivers/staging/vt6655/wpactl.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vt6655/hostap.c b/drivers/staging/vt6655/hostap.c
index 57a08c5..8acff44 100644
--- a/drivers/staging/vt6655/hostap.c
+++ b/drivers/staging/vt6655/hostap.c
@@ -86,7 +86,7 @@ static int hostap_enable_hostapd(PSDevice pDevice, int rtnl_locked)
 
 	apdev_priv = netdev_priv(pDevice->apdev);
 	*apdev_priv = *pDevice;
-	memcpy(pDevice->apdev->dev_addr, dev->dev_addr, ETH_ALEN);
+	eth_hw_addr_inherit(pDevice->apdev, dev);
 
 	pDevice->apdev->netdev_ops = &apdev_netdev_ops;
 
diff --git a/drivers/staging/vt6655/ioctl.c b/drivers/staging/vt6655/ioctl.c
index 46e0e41..b5cd2e4 100644
--- a/drivers/staging/vt6655/ioctl.c
+++ b/drivers/staging/vt6655/ioctl.c
@@ -460,7 +460,7 @@ int private_ioctl(PSDevice pDevice, struct ifreq *rq)
 		}
 		if (sValue.dwValue == 1) {
 			DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO "up wpadev\n");
-			memcpy(pDevice->wpadev->dev_addr, pDevice->dev->dev_addr, ETH_ALEN);
+			eth_hw_addr_inherit(pDevice->wpadev, pDevice->dev);
 			pDevice->bWPADEVUp = true;
 		} else {
 			DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO "close wpadev\n");
diff --git a/drivers/staging/vt6655/wpactl.c b/drivers/staging/vt6655/wpactl.c
index 869f62c..e8d9ecd 100644
--- a/drivers/staging/vt6655/wpactl.c
+++ b/drivers/staging/vt6655/wpactl.c
@@ -96,7 +96,7 @@ static int wpa_init_wpadev(PSDevice pDevice)
 
 	wpadev_priv = netdev_priv(pDevice->wpadev);
 	*wpadev_priv = *pDevice;
-	memcpy(pDevice->wpadev->dev_addr, dev->dev_addr, ETH_ALEN);
+	eth_hw_addr_inherit(pDevice->wpadev, dev);
 	pDevice->wpadev->base_addr = dev->base_addr;
 	pDevice->wpadev->irq = dev->irq;
 	pDevice->wpadev->mem_start = dev->mem_start;
-- 
1.7.10.4

[PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

Cc: Forest Bond <forest@alittletooquiet.net>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/staging/vt6656/hostap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/hostap.c b/drivers/staging/vt6656/hostap.c
index c699a30..3ddef9d 100644
--- a/drivers/staging/vt6656/hostap.c
+++ b/drivers/staging/vt6656/hostap.c
@@ -77,7 +77,7 @@ static int hostap_enable_hostapd(struct vnt_private *pDevice, int rtnl_locked)
 
     apdev_priv = netdev_priv(pDevice->apdev);
     *apdev_priv = *pDevice;
-	memcpy(pDevice->apdev->dev_addr, dev->dev_addr, ETH_ALEN);
+	eth_hw_addr_inherit(pDevice->apdev, dev);
 
 	pDevice->apdev->netdev_ops = &apdev_netdev_ops;
 
-- 
1.7.10.4

Re: [PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr

[Dan Carpenter]

On Fri, Aug 23, 2013 at 11:35:13AM +0200, Bjørn Mork wrote:
> Cc: Forest Bond <forest@alittletooquiet.net>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---

The changlog is missing.  Why does this patch help?

regards,
dan carpenter

Re: [PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

Dan Carpenter <dan.carpenter@oracle.com> writes:

> On Fri, Aug 23, 2013 at 11:35:13AM +0200, Bjørn Mork wrote:
>> Cc: Forest Bond <forest@alittletooquiet.net>
>> Signed-off-by: Bjørn Mork <bjorn@mork.no>
>> ---
>
> The changlog is missing.

Yes, right.  Sorry about that.

>  Why does this patch help?

It ensures that addr_assign_type is set to the same value as the parent
device the dev_addr is copied from.  The initial reason was that I found
it strange to see NET_ADDR_PERM on a vlan device which had inherited a
random address from its parent.

But I haven't verified whether that the parent device actually can have
an addr_assign_type != NET_ADDR_PERM for every driver included. 


Bjørn

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr

[Dan Carpenter]

Ah...  Here is the first patch which adds eth_hw_addr_inherit()

http://patchwork.ozlabs.org/patch/269365/

I don't think we've actually set dst->addr_len yet at this point so
it doesn't do the memcpy().  This doesn't work.

regards,
dan carpenter

Re: [PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr

[Bjørn Mork]

Dan Carpenter <dan.carpenter@oracle.com> writes:

> Ah...  Here is the first patch which adds eth_hw_addr_inherit()
>
> http://patchwork.ozlabs.org/patch/269365/
>
> I don't think we've actually set dst->addr_len yet at this point so
> it doesn't do the memcpy().  This doesn't work.

Ouch.  Yes, I see. The net_device is allocated using kzalloc just a few
lines earlier and there is no ether_setup or similar.  Actually, it
doesn't look like it ever sets addr_len. Is that right?  Does it work
even before this patch?

Anyway, thanks for spotting this.  I'll drop this patch for now as I
clearly don't understand how this driver works.

I do note that the otherwise very similar code in the vt6655 driver uses
alloc_etherdev, and therefore has a fair chance of working both before
and after my change.


Bjørn
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH net-next 10/10] staging: vt6656: inherit addr_assign_type along with dev_addr

[Dan Carpenter]

On Fri, Aug 23, 2013 at 02:06:21PM +0200, Bjørn Mork wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
> 
> > Ah...  Here is the first patch which adds eth_hw_addr_inherit()
> >
> > http://patchwork.ozlabs.org/patch/269365/
> >
> > I don't think we've actually set dst->addr_len yet at this point so
> > it doesn't do the memcpy().  This doesn't work.
> 
> Ouch.  Yes, I see. The net_device is allocated using kzalloc just a few
> lines earlier and there is no ether_setup or similar.  Actually, it
> doesn't look like it ever sets addr_len. Is that right?  Does it work
> even before this patch?

Ha ha.  That's a very good question.  I have no idea.

regards,
dan carpenter

Re: [PATCH net-next 08/10] net: libertas: inherit addr_assign_type along with dev_addr

[John W. Linville]

On Fri, Aug 23, 2013 at 11:35:11AM +0200, Bjørn Mork wrote:
> Cc: "John W. Linville" <linville@tuxdriver.com>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
>  drivers/net/wireless/libertas/mesh.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/libertas/mesh.c b/drivers/net/wireless/libertas/mesh.c
> index efae07e..6fef746 100644
> --- a/drivers/net/wireless/libertas/mesh.c
> +++ b/drivers/net/wireless/libertas/mesh.c
> @@ -1017,7 +1017,7 @@ static int lbs_add_mesh(struct lbs_private *priv)
>  
>  	mesh_dev->netdev_ops = &mesh_netdev_ops;
>  	mesh_dev->ethtool_ops = &lbs_ethtool_ops;
> -	memcpy(mesh_dev->dev_addr, priv->dev->dev_addr, ETH_ALEN);
> +	eth_hw_addr_inherit(mesh_dev, priv->dev);
>  
>  	SET_NETDEV_DEV(priv->mesh_dev, priv->dev->dev.parent);
>  
> -- 
> 1.7.10.4
> 
> 
Acked-by: "John W. Linville" <linville@tuxdriver.com>

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH net-next 06/10] net: airo: inherit addr_assign_type along with dev_addr

[John W. Linville]

On Fri, Aug 23, 2013 at 11:35:09AM +0200, Bjørn Mork wrote:
> Cc: "John W. Linville" <linville@tuxdriver.com>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
>  drivers/net/wireless/airo.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
> index d0adbaf..7fe1964 100644
> --- a/drivers/net/wireless/airo.c
> +++ b/drivers/net/wireless/airo.c
> @@ -2693,7 +2693,7 @@ static struct net_device *init_wifidev(struct airo_info *ai,
>  	dev->base_addr = ethdev->base_addr;
>  	dev->wireless_data = ethdev->wireless_data;
>  	SET_NETDEV_DEV(dev, ethdev->dev.parent);
> -	memcpy(dev->dev_addr, ethdev->dev_addr, dev->addr_len);
> +	eth_hw_addr_inherit(dev, ethdev);
>  	err = register_netdev(dev);
>  	if (err<0) {
>  		free_netdev(dev);
> -- 
> 1.7.10.4

Acked-by: "John W. Linville" <linville@tuxdriver.com>

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH net-next 06/10] net: airo: inherit addr_assign_type along with dev_addr

[John W. Linville]

On Fri, Aug 23, 2013 at 11:35:09AM +0200, Bjørn Mork wrote:
> Cc: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
> ---
>  drivers/net/wireless/airo.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
> index d0adbaf..7fe1964 100644
> --- a/drivers/net/wireless/airo.c
> +++ b/drivers/net/wireless/airo.c
> @@ -2693,7 +2693,7 @@ static struct net_device *init_wifidev(struct airo_info *ai,
>  	dev->base_addr = ethdev->base_addr;
>  	dev->wireless_data = ethdev->wireless_data;
>  	SET_NETDEV_DEV(dev, ethdev->dev.parent);
> -	memcpy(dev->dev_addr, ethdev->dev_addr, dev->addr_len);
> +	eth_hw_addr_inherit(dev, ethdev);
>  	err = register_netdev(dev);
>  	if (err<0) {
>  		free_netdev(dev);
> -- 
> 1.7.10.4

Acked-by: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

-- 
John W. Linville		Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org			might be all we have.  Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 01/10] net: etherdevice: add address inherit helper

[Stephen Hemminger]

On Fri, 23 Aug 2013 11:35:04 +0200
Bjørn Mork <bjorn@mork.no> wrote:

>  /**
> + * eth_hw_addr_inherit - Copy dev_addr from another net_device
> + * @dst: pointer to net_device to copy dev_addr to
> + * @src: pointer to net_device to copy dev_addr from
> + *
> + * Copy the Ethernet address from one net_device to another along with
> + * the addr_assign_type.
> + */
> +static inline int eth_hw_addr_inherit(struct net_device *dst,
> +				      struct net_device *src)
> +{
> +	if (dst->addr_len != src->addr_len)
> +		return -EINVAL;
> +
> +	dst->addr_assign_type = src->addr_assign_type;
> +	memcpy(dst->dev_addr, src->dev_addr, dst->addr_len);
> +	return 0;
> +}
> +

Since all the other code in this file assumes addr_len == ETH_ALEN
why does this code need to handle variable addresses. Trivial but
then the memcpy is fixed size and can be optimized.

Re: [PATCH net-next 01/10] net: etherdevice: add address inherit helper

[Bjørn Mork]

Stephen Hemminger <stephen@networkplumber.org> writes:
> On Fri, 23 Aug 2013 11:35:04 +0200
> Bjørn Mork <bjorn@mork.no> wrote:
>
>>  /**
>> + * eth_hw_addr_inherit - Copy dev_addr from another net_device
>> + * @dst: pointer to net_device to copy dev_addr to
>> + * @src: pointer to net_device to copy dev_addr from
>> + *
>> + * Copy the Ethernet address from one net_device to another along with
>> + * the addr_assign_type.
>> + */
>> +static inline int eth_hw_addr_inherit(struct net_device *dst,
>> +				      struct net_device *src)
>> +{
>> +	if (dst->addr_len != src->addr_len)
>> +		return -EINVAL;
>> +
>> +	dst->addr_assign_type = src->addr_assign_type;
>> +	memcpy(dst->dev_addr, src->dev_addr, dst->addr_len);
>> +	return 0;
>> +}
>> +
>
> Since all the other code in this file assumes addr_len == ETH_ALEN
> why does this code need to handle variable addresses. Trivial but
> then the memcpy is fixed size and can be optimized.

Didn't know that. I'll make that change in the next version.  Not that
optimization matters much here, but consistency is always good.


Bjørn

Re: [PATCH net-next 01/10] net: etherdevice: add address inherit helper

[David Miller]

From: Bjørn Mork <bjorn@mork.no>
Date: Sat, 24 Aug 2013 00:28:55 +0200

> Stephen Hemminger <stephen@networkplumber.org> writes:
>> On Fri, 23 Aug 2013 11:35:04 +0200
>> Bjørn Mork <bjorn@mork.no> wrote:
>>
>>>  /**
>>> + * eth_hw_addr_inherit - Copy dev_addr from another net_device
>>> + * @dst: pointer to net_device to copy dev_addr to
>>> + * @src: pointer to net_device to copy dev_addr from
>>> + *
>>> + * Copy the Ethernet address from one net_device to another along with
>>> + * the addr_assign_type.
>>> + */
>>> +static inline int eth_hw_addr_inherit(struct net_device *dst,
>>> +				      struct net_device *src)
>>> +{
>>> +	if (dst->addr_len != src->addr_len)
>>> +		return -EINVAL;
>>> +
>>> +	dst->addr_assign_type = src->addr_assign_type;
>>> +	memcpy(dst->dev_addr, src->dev_addr, dst->addr_len);
>>> +	return 0;
>>> +}
>>> +
>>
>> Since all the other code in this file assumes addr_len == ETH_ALEN
>> why does this code need to handle variable addresses. Trivial but
>> then the memcpy is fixed size and can be optimized.
> 
> Didn't know that. I'll make that change in the next version.  Not that
> optimization matters much here, but consistency is always good.

You can bug check that addr_len ETH_ALEN if you want.

I realize that some drivers don't initialize it by this call point but
that is arguably a bug.

Re: [PATCH net-next 01/10] net: etherdevice: add address inherit helper

[Bjørn Mork]

David Miller <davem@davemloft.net> writes:

> From: Bjørn Mork <bjorn@mork.no>
> Date: Sat, 24 Aug 2013 00:28:55 +0200
>
>> Stephen Hemminger <stephen@networkplumber.org> writes:
>>> On Fri, 23 Aug 2013 11:35:04 +0200
>>> Bjørn Mork <bjorn@mork.no> wrote:
>>>
>>>>  /**
>>>> + * eth_hw_addr_inherit - Copy dev_addr from another net_device
>>>> + * @dst: pointer to net_device to copy dev_addr to
>>>> + * @src: pointer to net_device to copy dev_addr from
>>>> + *
>>>> + * Copy the Ethernet address from one net_device to another along with
>>>> + * the addr_assign_type.
>>>> + */
>>>> +static inline int eth_hw_addr_inherit(struct net_device *dst,
>>>> +				      struct net_device *src)
>>>> +{
>>>> +	if (dst->addr_len != src->addr_len)
>>>> +		return -EINVAL;
>>>> +
>>>> +	dst->addr_assign_type = src->addr_assign_type;
>>>> +	memcpy(dst->dev_addr, src->dev_addr, dst->addr_len);
>>>> +	return 0;
>>>> +}
>>>> +
>>>
>>> Since all the other code in this file assumes addr_len == ETH_ALEN
>>> why does this code need to handle variable addresses. Trivial but
>>> then the memcpy is fixed size and can be optimized.
>> 
>> Didn't know that. I'll make that change in the next version.  Not that
>> optimization matters much here, but consistency is always good.
>
> You can bug check that addr_len ETH_ALEN if you want.

I don't think that is necessary.  

> I realize that some drivers don't initialize it by this call point but
> that is arguably a bug.

Yes.  But I don't believe another test is going to make those drivers
work, so it's better to just drop it.


Bjørn