net: Modify return value for the .ndo_set_mac_address

PATCH In review Denis Kirjanov <[email protected] 2 participants 4 messages Dec 4, 2009 – Dec 5, 2009

Subsystems (5)

Networking Network drivers PowerPC USB networking drivers +1 more subsystems

lore

Share & tools

lore

Denis Kirjanov <[email protected]

author

2009-12-04 11:45 UTC #0 lore raw

PATCH 1/1 net: Modify return value for the .ndo_set_mac_address

Return -EADDRNOTAVAIL insted of -EINVAL in .ndo_set_mac_address.

Signed-off-by Denis Kirjanov <[email protected]>

---
drivers/net/b44.c | 2 +-

 drivers/net/bnx2.c                   |    2 +-
 drivers/net/bnx2x_main.c             |    2 +-
 drivers/net/cxgb3/cxgb3_main.c       |    2 +-
 drivers/net/davinci_emac.c           |    2 +-
 drivers/net/netxen/netxen_nic_main.c |    2 +-
 drivers/net/niu.c                    |    2 +-
 drivers/net/pasemi_mac.c             |    2 +-
 drivers/net/s2io.c                   |    2 +-
 drivers/net/sfc/efx.c                |    2 +-
 drivers/net/tsi108_eth.c             |    2 +-
 drivers/net/usb/dm9601.c             |    2 +-
 drivers/net/usb/mcs7830.c            |    2 +-
 drivers/net/vxge/vxge-main.c         |    2 +-
 14 files changed, 14 insertions(+), 14 deletions(-)

drivers/net/b44.c +1 -1

index 2a91323..2f4877e 100644
@@ -1363,7 +1363,7 @@ static int b44_set_mac_addr(struct net_device *dev, void *p)
 		return -EBUSY;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);

drivers/net/bnx2.c +1 -1

index 4bfc808..dd86477 100644
@@ -7594,7 +7594,7 @@ bnx2_change_mac_addr(struct net_device *dev, void *p)
 	struct bnx2 *bp = netdev_priv(dev);
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
 	if (netif_running(dev))

drivers/net/bnx2x_main.c +1 -1

index 77ba135..b87509e 100644
@@ -11582,7 +11582,7 @@ static int bnx2x_change_mac_addr(struct net_device *dev, void *p)
 	struct bnx2x *bp = netdev_priv(dev);
 
 	if (!is_valid_ether_addr((u8 *)(addr->sa_data)))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
 	if (netif_running(dev)) {

drivers/net/cxgb3/cxgb3_main.c +1 -1

index cef3f88..fda1dd6 100644
@@ -2523,7 +2523,7 @@ static int cxgb_set_mac_addr(struct net_device *dev, void *p)
 	struct sockaddr *addr = p;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
 	t3_mac_set_address(&pi->mac, LAN_MAC_IDX, dev->dev_addr);

drivers/net/davinci_emac.c +1 -1

index 8edac89..cc78677 100644
@@ -1823,7 +1823,7 @@ static int emac_dev_setmac_addr(struct net_device *ndev, void *addr)
 	struct sockaddr *sa = addr;
 
 	if (!is_valid_ether_addr(sa->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	/* Store mac addr in priv and rx channel and set it in EMAC hw */
 	memcpy(priv->mac_addr, sa->sa_data, ndev->addr_len);

drivers/net/netxen/netxen_nic_main.c +1 -1

index e5d187f..5d754ba 100644
@@ -461,7 +461,7 @@ int netxen_nic_set_mac(struct net_device *netdev, void *p)
 	struct sockaddr *addr = p;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	if (netif_running(netdev)) {
 		netif_device_detach(netdev);

drivers/net/niu.c +1 -1

index 8ce58c4..67d762c 100644
@@ -6435,7 +6435,7 @@ static int niu_set_mac_addr(struct net_device *dev, void *p)
 	unsigned long flags;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);

drivers/net/pasemi_mac.c +1 -1

index 1673eb0..9a68ef2 100644
@@ -236,7 +236,7 @@ static int pasemi_mac_set_mac_addr(struct net_device *dev, void *p)
 	unsigned int adr0, adr1;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);

drivers/net/s2io.c +1 -1

index 0dd7839..781e7bd 100644
@@ -5273,7 +5273,7 @@ static int s2io_set_mac_addr(struct net_device *dev, void *p)
 	struct sockaddr *addr = p;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);

drivers/net/sfc/efx.c +1 -1

index f983e3b..6893681 100644
@@ -1585,7 +1585,7 @@ static int efx_set_mac_address(struct net_device *net_dev, void *data)
 	if (!is_valid_ether_addr(new_addr)) {
 		EFX_ERR(efx, "invalid ethernet MAC address requested: %pM\n",
 			new_addr);
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 	}
 
 	memcpy(net_dev->dev_addr, new_addr, net_dev->addr_len);

drivers/net/tsi108_eth.c +1 -1

index a69c4a4..c4e385b 100644
@@ -1146,7 +1146,7 @@ static int tsi108_set_mac(struct net_device *dev, void *addr)
 	int i;
 
 	if (!is_valid_ether_addr(addr))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	for (i = 0; i < 6; i++)
 		/* +2 is for the offset of the HW addr type */

drivers/net/usb/dm9601.c +1 -1

index 3d406f9..eb59c2d 100644
@@ -410,7 +410,7 @@ static int dm9601_set_mac_address(struct net_device *net, void *p)
 	if (!is_valid_ether_addr(addr->sa_data)) {
 		dev_err(&net->dev, "not setting invalid mac address %pM\n",
 								addr->sa_data);
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 	}
 
 	memcpy(net->dev_addr, addr->sa_data, net->addr_len);

drivers/net/usb/mcs7830.c +1 -1

index 8737431..1b4d77a 100644
@@ -473,7 +473,7 @@ static int mcs7830_set_mac_address(struct net_device *netdev, void *p)
 		return -EBUSY;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);

drivers/net/vxge/vxge-main.c +1 -1

index f1c4b2a..4b36d06 100644
@@ -1301,7 +1301,7 @@ static int vxge_set_mac_addr(struct net_device *dev, void *p)
 	hldev = vdev->devh;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memset(&mac_info_new, 0, sizeof(struct macInfo));
 	memset(&mac_info_old, 0, sizeof(struct macInfo));

David Miller

maintainer

2009-12-04 21:30 UTC #1 lore raw

From: "Denis Kirjanov <[email protected]" <[email protected]>
Date: Fri, 4 Dec 2009 14:47:22 +0300

Quoted 3 lines

Return -EADDRNOTAVAIL insted of -EINVAL in .ndo_set_mac_address.
Signed-off-by Denis Kirjanov <[email protected]>

Why?

The address is "invalid" so we return "invalid" error.

What's the problem?

And whether there is a good reason or not, your commit message did not describe that reason so needs to be updated.

Denis Kirjanov <[email protected]

author

2009-12-05 19:27 UTC #2 lore raw

Quoted 7 lines · David Miller

From: "Denis Kirjanov <[email protected]" <[email protected]>
Date: Fri, 4 Dec 2009 14:47:22 +0300
Quoted 3 lines
Return -EADDRNOTAVAIL insted of -EINVAL in .ndo_set_mac_address.
Signed-off-by Denis Kirjanov <[email protected]>

Quoted 8 lines

Why?
The address is "invalid" so we return "invalid" error.
What's the problem?
And whether there is a good reason or not, your commit message did not describe that reason so needs to be updated.

[PATCH] net: Modify return value for the .ndo_set_mac_address

Return -EADDRNOTAVAIL instead of -EINVAL in .ndo_set_mac_address.

Some drivers in drivers/net return -EINVAL in .ndo_set_mac_address, but some -EADDRNOTAVAIL. Thus, we use
"Cannot assign requested address" in the case of setting up new hw address to be more consistent across drivers/net in .ndo_set_mac_address function.

Signed-off-by Denis Kirjanov <[email protected]>

---
drivers/net/b44.c | 2 +-

 drivers/net/bnx2.c                   |    2 +-
 drivers/net/bnx2x_main.c             |    2 +-
 drivers/net/cxgb3/cxgb3_main.c       |    2 +-
 drivers/net/davinci_emac.c           |    2 +-
 drivers/net/netxen/netxen_nic_main.c |    2 +-
 drivers/net/niu.c                    |    2 +-
 drivers/net/pasemi_mac.c             |    2 +-
 drivers/net/s2io.c                   |    2 +-
 drivers/net/sfc/efx.c                |    2 +-
 drivers/net/tsi108_eth.c             |    2 +-
 drivers/net/usb/dm9601.c             |    2 +-
 drivers/net/usb/mcs7830.c            |    2 +-
 drivers/net/vxge/vxge-main.c         |    2 +-
 14 files changed, 14 insertions(+), 14 deletions(-)

drivers/net/b44.c +1 -1

index 2a91323..2f4877e 100644
@@ -1363,7 +1363,7 @@ static int b44_set_mac_addr(struct net_device *dev, void *p)
 		return -EBUSY;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);

drivers/net/bnx2.c +1 -1

index 4bfc808..dd86477 100644
@@ -7594,7 +7594,7 @@ bnx2_change_mac_addr(struct net_device *dev, void *p)
 	struct bnx2 *bp = netdev_priv(dev);
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
 	if (netif_running(dev))

drivers/net/bnx2x_main.c +1 -1

index 77ba135..b87509e 100644
@@ -11582,7 +11582,7 @@ static int bnx2x_change_mac_addr(struct net_device *dev, void *p)
 	struct bnx2x *bp = netdev_priv(dev);
 
 	if (!is_valid_ether_addr((u8 *)(addr->sa_data)))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
 	if (netif_running(dev)) {

drivers/net/cxgb3/cxgb3_main.c +1 -1

index cef3f88..fda1dd6 100644
@@ -2523,7 +2523,7 @@ static int cxgb_set_mac_addr(struct net_device *dev, void *p)
 	struct sockaddr *addr = p;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
 	t3_mac_set_address(&pi->mac, LAN_MAC_IDX, dev->dev_addr);

drivers/net/davinci_emac.c +1 -1

index 8edac89..cc78677 100644
@@ -1823,7 +1823,7 @@ static int emac_dev_setmac_addr(struct net_device *ndev, void *addr)
 	struct sockaddr *sa = addr;
 
 	if (!is_valid_ether_addr(sa->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	/* Store mac addr in priv and rx channel and set it in EMAC hw */
 	memcpy(priv->mac_addr, sa->sa_data, ndev->addr_len);

drivers/net/netxen/netxen_nic_main.c +1 -1

index e5d187f..5d754ba 100644
@@ -461,7 +461,7 @@ int netxen_nic_set_mac(struct net_device *netdev, void *p)
 	struct sockaddr *addr = p;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	if (netif_running(netdev)) {
 		netif_device_detach(netdev);

drivers/net/niu.c +1 -1

index 8ce58c4..67d762c 100644
@@ -6435,7 +6435,7 @@ static int niu_set_mac_addr(struct net_device *dev, void *p)
 	unsigned long flags;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);

drivers/net/pasemi_mac.c +1 -1

index 1673eb0..9a68ef2 100644
@@ -236,7 +236,7 @@ static int pasemi_mac_set_mac_addr(struct net_device *dev, void *p)
 	unsigned int adr0, adr1;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);

drivers/net/s2io.c +1 -1

index 0dd7839..781e7bd 100644
@@ -5273,7 +5273,7 @@ static int s2io_set_mac_addr(struct net_device *dev, void *p)
 	struct sockaddr *addr = p;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);

drivers/net/sfc/efx.c +1 -1

index f983e3b..6893681 100644
@@ -1585,7 +1585,7 @@ static int efx_set_mac_address(struct net_device *net_dev, void *data)
 	if (!is_valid_ether_addr(new_addr)) {
 		EFX_ERR(efx, "invalid ethernet MAC address requested: %pM\n",
 			new_addr);
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 	}
 
 	memcpy(net_dev->dev_addr, new_addr, net_dev->addr_len);

drivers/net/tsi108_eth.c +1 -1

index a69c4a4..c4e385b 100644
@@ -1146,7 +1146,7 @@ static int tsi108_set_mac(struct net_device *dev, void *addr)
 	int i;
 
 	if (!is_valid_ether_addr(addr))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	for (i = 0; i < 6; i++)
 		/* +2 is for the offset of the HW addr type */

drivers/net/usb/dm9601.c +1 -1

index 3d406f9..eb59c2d 100644
@@ -410,7 +410,7 @@ static int dm9601_set_mac_address(struct net_device *net, void *p)
 	if (!is_valid_ether_addr(addr->sa_data)) {
 		dev_err(&net->dev, "not setting invalid mac address %pM\n",
 								addr->sa_data);
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 	}
 
 	memcpy(net->dev_addr, addr->sa_data, net->addr_len);

drivers/net/usb/mcs7830.c +1 -1

index 8737431..1b4d77a 100644
@@ -473,7 +473,7 @@ static int mcs7830_set_mac_address(struct net_device *netdev, void *p)
 		return -EBUSY;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);

drivers/net/vxge/vxge-main.c +1 -1

index f1c4b2a..4b36d06 100644
@@ -1301,7 +1301,7 @@ static int vxge_set_mac_addr(struct net_device *dev, void *p)
 	hldev = vdev->devh;
 
 	if (!is_valid_ether_addr(addr->sa_data))
-		return -EINVAL;
+		return -EADDRNOTAVAIL;
 
 	memset(&mac_info_new, 0, sizeof(struct macInfo));
 	memset(&mac_info_old, 0, sizeof(struct macInfo));

David Miller

maintainer

2009-12-05 22:32 UTC #3 lore raw

From: "Denis Kirjanov <[email protected]" <[email protected]>
Date: Sat, 5 Dec 2009 22:29:26 +0300

Quoted 8 lines · David Miller

Quoted 7 lines
From: "Denis Kirjanov <[email protected]" <[email protected]>
Date: Fri, 4 Dec 2009 14:47:22 +0300
Quoted 3 lines
Return -EADDRNOTAVAIL insted of -EINVAL in .ndo_set_mac_address.
Signed-off-by Denis Kirjanov <[email protected]>

Quoted 8 lines

Quoted 8 lines
Why?
The address is "invalid" so we return "invalid" error.
What's the problem?
And whether there is a good reason or not, your commit message did not describe that reason so needs to be updated.

Will you please talk to me instead of just responding in the commit message itself? I want to have a real conversation about this.

Quoted 5 lines

Some drivers in drivers/net return -EINVAL in .ndo_set_mac_address, but some -EADDRNOTAVAIL. Thus, we use
"Cannot assign requested address" in the case of setting up new hw address to be more consistent across drivers/net in .ndo_set_mac_address function.

This doesn't by itself make EADDRNOTAVAIL a better choice.

If you look elsewhere, this error code is used in cases where tries to use an address is not configured on a device or a socket.

This is different than the usage you are trying to make the tree use consistently here.

I still think -EINVAL makes more sense.

Yes it is a problem that UNIX lacks more verbose and precise error code specifications, but that itself should be attacked as a problem rather than used as a reason to choose EADDRNOTAVAIL as an error code here.

I'm not applying your patches, I do not agree with your reasoning. I still think -EINVAL is better, and therefore you still need to respin your other patch.