2021-03-04 12:39:28

by David Mosberger-Tang

Subject: [PATCH] wilc1000: Bring MAC address setting in line with typical Linux behavior

Linux network drivers normally disallow changing the MAC address when
the interface is up. This driver has been different in that it allows
to change the MAC address *only* when it's up. This patch brings
wilc1000 behavior more in line with other network drivers. We could
have replaced wilc_set_mac_addr() with eth_mac_addr() but that would
break existing documentation on how to change the MAC address.
Likewise, return -EADDRNOTAVAIL (not -EINVAL) when the specified MAC
address is invalid or unavailable.

Signed-off-by: David Mosberger-Tang <[email protected]>
---
.../net/wireless/microchip/wilc1000/netdev.c | 25 ++++++++++++-------
1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index 1b205e7d97a8..37f40039e4ca 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -575,7 +575,6 @@ static int wilc_mac_open(struct net_device *ndev)
{
struct wilc_vif *vif = netdev_priv(ndev);
struct wilc *wl = vif->wilc;
- unsigned char mac_add[ETH_ALEN] = {0};
int ret = 0;
struct mgmt_frame_regs mgmt_regs = {};

@@ -598,9 +597,12 @@ static int wilc_mac_open(struct net_device *ndev)

wilc_set_operation_mode(vif, wilc_get_vif_idx(vif), vif->iftype,
vif->idx);
- wilc_get_mac_address(vif, mac_add);
- netdev_dbg(ndev, "Mac address: %pM\n", mac_add);
- ether_addr_copy(ndev->dev_addr, mac_add);
+
+ if (is_valid_ether_addr(ndev->dev_addr))
+ wilc_set_mac_address(vif, ndev->dev_addr);
+ else
+ wilc_get_mac_address(vif, ndev->dev_addr);
+ netdev_dbg(ndev, "Mac address: %pM\n", ndev->dev_addr);

if (!is_valid_ether_addr(ndev->dev_addr)) {
netdev_err(ndev, "Wrong MAC address\n");
@@ -639,7 +641,14 @@ static int wilc_set_mac_addr(struct net_device *dev, void *p)
int srcu_idx;

if (!is_valid_ether_addr(addr->sa_data))
- return -EINVAL;
+ return -EADDRNOTAVAIL;
+
+ if (!vif->mac_opened) {
+ eth_commit_mac_addr_change(dev, p);
+ return 0;
+ }
+
+ /* Verify MAC Address is not already in use: */

srcu_idx = srcu_read_lock(&wilc->srcu);
list_for_each_entry_rcu(tmp_vif, &wilc->vif_list, list) {
@@ -647,7 +656,7 @@ static int wilc_set_mac_addr(struct net_device *dev, void *p)
if (ether_addr_equal(addr->sa_data, mac_addr)) {
if (vif != tmp_vif) {
srcu_read_unlock(&wilc->srcu, srcu_idx);
- return -EINVAL;
+ return -EADDRNOTAVAIL;
}
srcu_read_unlock(&wilc->srcu, srcu_idx);
return 0;
@@ -659,9 +668,7 @@ static int wilc_set_mac_addr(struct net_device *dev, void *p)
if (result)
return result;

- ether_addr_copy(vif->bssid, addr->sa_data);
- ether_addr_copy(vif->ndev->dev_addr, addr->sa_data);
-
+ eth_commit_mac_addr_change(dev, p);
return result;
}

--
2.25.1

2021-04-17 17:53:02

by Kalle Valo

Subject: Re: [PATCH] wilc1000: Bring MAC address setting in line with typical Linux behavior

David Mosberger-Tang <[email protected]> wrote:

> Linux network drivers normally disallow changing the MAC address when
> the interface is up. This driver has been different in that it allows
> to change the MAC address *only* when it's up. This patch brings
> wilc1000 behavior more in line with other network drivers. We could
> have replaced wilc_set_mac_addr() with eth_mac_addr() but that would
> break existing documentation on how to change the MAC address.
> Likewise, return -EADDRNOTAVAIL (not -EINVAL) when the specified MAC
> address is invalid or unavailable.
>
> Signed-off-by: David Mosberger-Tang <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

a381b78a1598 wilc1000: Bring MAC address setting in line with typical Linux behavior

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches