| Message ID | 1382819655-30430-17-git-send-email-pali.rohar@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sat, 2013-10-26 at 22:34 +0200, Pali Rohár wrote: > Driver wl1251 generating mac address randomly at startup and there is no way to > set permanent mac address via SET_IEEE80211_PERM_ADDR. This patch export sysfs > file which can set permanent mac address by userspace helper program. Patch is > needed for devices which do not store mac address in internal wl1251 eeprom. This doesn't really seem like a good idea since you can also just use 'ip' or whatever to set the MAC address. johannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 28 October 2013 14:45:05 Johannes Berg wrote: > On Sat, 2013-10-26 at 22:34 +0200, Pali Rohár wrote: > > Driver wl1251 generating mac address randomly at startup and > > there is no way to set permanent mac address via > > SET_IEEE80211_PERM_ADDR. This patch export sysfs file which > > can set permanent mac address by userspace helper program. > > Patch is needed for devices which do not store mac address > > in internal wl1251 eeprom. > > This doesn't really seem like a good idea since you can also > just use 'ip' or whatever to set the MAC address. > > johannes AFAIK you cannot set permanent address (show by ethtool -P wlan0) via ip/ifconfig.
On Mon, 2013-10-28 at 14:49 +0100, Pali Rohár wrote: > On Monday 28 October 2013 14:45:05 Johannes Berg wrote: > > On Sat, 2013-10-26 at 22:34 +0200, Pali Rohár wrote: > > > Driver wl1251 generating mac address randomly at startup and > > > there is no way to set permanent mac address via > > > SET_IEEE80211_PERM_ADDR. This patch export sysfs file which > > > can set permanent mac address by userspace helper program. > > > Patch is needed for devices which do not store mac address > > > in internal wl1251 eeprom. > > > > This doesn't really seem like a good idea since you can also > > just use 'ip' or whatever to set the MAC address. > > > > johannes > > AFAIK you cannot set permanent address (show by ethtool -P wlan0) > via ip/ifconfig. You probably can't, but that address also doesn't matter at all and isn't really used anywhere. johannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 28 October 2013 14:55:22 Johannes Berg wrote: > On Mon, 2013-10-28 at 14:49 +0100, Pali Rohár wrote: > > On Monday 28 October 2013 14:45:05 Johannes Berg wrote: > > > On Sat, 2013-10-26 at 22:34 +0200, Pali Rohár wrote: > > > > Driver wl1251 generating mac address randomly at startup > > > > and there is no way to set permanent mac address via > > > > SET_IEEE80211_PERM_ADDR. This patch export sysfs file > > > > which can set permanent mac address by userspace helper > > > > program. Patch is needed for devices which do not store > > > > mac address in internal wl1251 eeprom. > > > > > > This doesn't really seem like a good idea since you can > > > also just use 'ip' or whatever to set the MAC address. > > > > > > johannes > > > > AFAIK you cannot set permanent address (show by ethtool -P > > wlan0) via ip/ifconfig. > > You probably can't, but that address also doesn't matter at > all and isn't really used anywhere. > > johannes There are some (proprietary) applications which using permanent address for something... And also more important: some network managing tools using it. NetworkManager resetting current MAC address to permanent one before starting configuring interface. So this will lead to never use correct MAC address (but random permanent one) assigned for that wl1251 wireless card...
On Mon, 2013-10-28 at 15:00 +0100, Pali Rohár wrote: > On Monday 28 October 2013 14:55:22 Johannes Berg wrote: > > On Mon, 2013-10-28 at 14:49 +0100, Pali Rohár wrote: > > > On Monday 28 October 2013 14:45:05 Johannes Berg wrote: > > > > On Sat, 2013-10-26 at 22:34 +0200, Pali Rohár wrote: > > > > > Driver wl1251 generating mac address randomly at startup > > > > > and there is no way to set permanent mac address via > > > > > SET_IEEE80211_PERM_ADDR. This patch export sysfs file > > > > > which can set permanent mac address by userspace helper > > > > > program. Patch is needed for devices which do not store > > > > > mac address in internal wl1251 eeprom. > > > > > > > > This doesn't really seem like a good idea since you can > > > > also just use 'ip' or whatever to set the MAC address. > > > > > > > > johannes > > > > > > AFAIK you cannot set permanent address (show by ethtool -P > > > wlan0) via ip/ifconfig. > > > > You probably can't, but that address also doesn't matter at > > all and isn't really used anywhere. > > > > johannes > > There are some (proprietary) applications which using permanent > address for something... > > And also more important: some network managing tools using it. > NetworkManager resetting current MAC address to permanent one > before starting configuring interface. > > So this will lead to never use correct MAC address (but random > permanent one) assigned for that wl1251 wireless card... If the device doesn't actually *have* a permanent MAC address, then it shouldn't be returning one via ethtool, and should return an error for ETHTOOL_GPERMADDR. Setting the permanent MAC address shouldn't ever be allowed except by the driver inspecting EEPROM, and certainly not via sysfs. mac80211 does have to do some special stuff due to virtual interfaces and such, but in general, if the MAC address is randomly generated, neither the driver nor mac80211 should be reporting that address as the permanent address, only as the current one. Dan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2013-10-28 at 09:46 -0500, Dan Williams wrote: > If the device doesn't actually *have* a permanent MAC address, then it > shouldn't be returning one via ethtool, and should return an error for > ETHTOOL_GPERMADDR. There's currently no provision for that, but I agree it would be better to do so. johannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 28 October 2013 15:56:55 Johannes Berg wrote: > On Mon, 2013-10-28 at 09:46 -0500, Dan Williams wrote: > > If the device doesn't actually *have* a permanent MAC > > address, then it shouldn't be returning one via ethtool, > > and should return an error for ETHTOOL_GPERMADDR. > > There's currently no provision for that, but I agree it would > be better to do so. > > johannes So what to do with devices which has MAC address stored in some obscure place and there is userspace binary which can read it? I think that there should be some way to tell kernel that *this* is the permanent address and it should use it. This is reason why I proposed patch which adding sysfs entry for setting permanent address from userspace in wl1251 driver.
On Mon, 2013-10-28 at 16:04 +0100, Pali Rohár wrote: > On Monday 28 October 2013 15:56:55 Johannes Berg wrote: > > On Mon, 2013-10-28 at 09:46 -0500, Dan Williams wrote: > > > If the device doesn't actually *have* a permanent MAC > > > address, then it shouldn't be returning one via ethtool, > > > and should return an error for ETHTOOL_GPERMADDR. > > > > There's currently no provision for that, but I agree it would > > be better to do so. > > > > johannes > > So what to do with devices which has MAC address stored in some > obscure place and there is userspace binary which can read it? > > I think that there should be some way to tell kernel that *this* > is the permanent address and it should use it. > > This is reason why I proposed patch which adding sysfs entry for > setting permanent address from userspace in wl1251 driver. Ok, so the N900's MAC is stored in the "cal partition", which is a region of flash exposed to the OS as /dev/mtd1. That also stores the regulatory domain. Typically userspace binaries are used to pull out this and other data (see https://dev.openwrt.org/browser/packages/utils/calvaria/files/src/calvaria.c ) which is then used to initialize the device. Any idea what kernel driver is used to expose the CAL partition as /dev/mtd1? If it's nothing special maybe a small EEPROM-style driver could be written to read the relevant areas (and since they don't ever change, don't need to worry about something else writing at the same time). Dan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 28 Oct 2013 15:56:55 +0100 Johannes Berg <johannes@sipsolutions.net> wrote: > On Mon, 2013-10-28 at 09:46 -0500, Dan Williams wrote: > > > If the device doesn't actually *have* a permanent MAC address, then it > > shouldn't be returning one via ethtool, and should return an error for > > ETHTOOL_GPERMADDR. > > There's currently no provision for that, but I agree it would be better > to do so. > > johannes > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html The current netdevice API handle the case of a device without a permanent MAC address, slightly differently. If device does not have a permanent address, then: 1. dev->addr_assign_type should not be NET_ADDR_PERM 2. when device is registered dev->perm_addr will not be set and retain all zeros value 4. when ethtool gets address it will return all zeros which is not a valid address. This case doesn't seem to be handled threo mac80211 API's -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 28 October 2013 16:29:21 Dan Williams wrote: > On Mon, 2013-10-28 at 16:04 +0100, Pali Rohár wrote: > > On Monday 28 October 2013 15:56:55 Johannes Berg wrote: > > > On Mon, 2013-10-28 at 09:46 -0500, Dan Williams wrote: > > > > If the device doesn't actually *have* a permanent MAC > > > > address, then it shouldn't be returning one via ethtool, > > > > and should return an error for ETHTOOL_GPERMADDR. > > > > > > There's currently no provision for that, but I agree it > > > would be better to do so. > > > > > > johannes > > > > So what to do with devices which has MAC address stored in > > some obscure place and there is userspace binary which can > > read it? > > > > I think that there should be some way to tell kernel that > > *this* is the permanent address and it should use it. > > > > This is reason why I proposed patch which adding sysfs entry > > for setting permanent address from userspace in wl1251 > > driver. > > Ok, so the N900's MAC is stored in the "cal partition", which > is a region of flash exposed to the OS as /dev/mtd1. That > also stores the regulatory domain. Typically userspace > binaries are used to pull out this and other data (see > https://dev.openwrt.org/browser/packages/utils/calvaria/files/ > src/calvaria.c ) which is then used to initialize the device. > > Any idea what kernel driver is used to expose the CAL > partition as /dev/mtd1? If it's nothing special maybe a > small EEPROM-style driver could be written to read the > relevant areas (and since they don't ever change, don't need > to worry about something else writing at the same time). > > Dan /dev/mtd1 is second (0 is first) OneNand partition. Nothing special. Kernel driver for OnaNand is already in mainline kernel. That partition also has NVS device data for wl1251 chip. And you are right calvaria has GPL v2 code for parsing data in that partition.
On Sat, 2013-10-26 at 22:34 +0200, Pali Rohár wrote: > Driver wl1251 generating mac address randomly at startup and there is no way to > set permanent mac address via SET_IEEE80211_PERM_ADDR. This patch export sysfs > file which can set permanent mac address by userspace helper program. Patch is > needed for devices which do not store mac address in internal wl1251 eeprom. [...] This belongs in struct wl12xx_platform_data or (better) Device Tree properties. It definitely should not be settable once the device is registered. Ben.
diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c index 29625c2..91a009c 100644 --- a/drivers/net/wireless/ti/wl1251/main.c +++ b/drivers/net/wireless/ti/wl1251/main.c @@ -1375,6 +1375,46 @@ static const struct ieee80211_ops wl1251_ops = { .get_survey = wl1251_op_get_survey, }; +static ssize_t wl1251_sysfs_show_address(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct wl1251 *wl = dev_get_drvdata(dev); + ssize_t len; + + /* FIXME: what's the maximum length of buf? page size?*/ + len = 500; + + len = snprintf(buf, len, "%.2x:%.2x:%.2x:%.2x:%.2x:%.2x\n", + wl->mac_addr[0], wl->mac_addr[1], wl->mac_addr[2], + wl->mac_addr[3], wl->mac_addr[4], wl->mac_addr[5]); + + return len; +} + +static ssize_t wl1251_sysfs_store_address(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct wl1251 *wl = dev_get_drvdata(dev); + unsigned int addr[6]; + int ret, i; + + ret = sscanf(buf, "%2x:%2x:%2x:%2x:%2x:%2x\n", + &addr[0], &addr[1], &addr[2], + &addr[3], &addr[4], &addr[5]); + + if (ret != 6) + return -EINVAL; + + for (i = 0; i < 6; i++) + wl->mac_addr[i] = addr[i] & 0xff; + + SET_IEEE80211_PERM_ADDR(wl->hw, wl->mac_addr); + + return count; +} + static ssize_t wl1251_sysfs_show_tx_mgmt_frm_rate(struct device *dev, struct device_attribute *attr, char *buf) @@ -1584,6 +1624,10 @@ out: return count; } +static DEVICE_ATTR(address, S_IRUGO | S_IWUSR, + wl1251_sysfs_show_address, + wl1251_sysfs_store_address); + static DEVICE_ATTR(tx_mgmt_frm_rate, S_IRUGO | S_IWUSR, wl1251_sysfs_show_tx_mgmt_frm_rate, wl1251_sysfs_store_tx_mgmt_frm_rate); @@ -1728,6 +1772,14 @@ int wl1251_init_ieee80211(struct wl1251 *wl) } dev_set_drvdata(&wl1251_device.dev, wl); + /* Create sysfs file address */ + ret = device_create_file(&wl1251_device.dev, + &dev_attr_address); + if (ret < 0) { + wl1251_error("failed to create sysfs file address"); + goto out; + } + /* Create sysfs file tx_mgmt_frm_rate */ ret = device_create_file(&wl1251_device.dev, &dev_attr_tx_mgmt_frm_rate);
Driver wl1251 generating mac address randomly at startup and there is no way to set permanent mac address via SET_IEEE80211_PERM_ADDR. This patch export sysfs file which can set permanent mac address by userspace helper program. Patch is needed for devices which do not store mac address in internal wl1251 eeprom. Signed-off-by: Pali Rohár <pali.rohar@gmail.com> --- drivers/net/wireless/ti/wl1251/main.c | 52 +++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)