[PATCH] allow setting wiphy.perm_addr after driver probe

[PATCH] allow setting wiphy.perm_addr after driver probe

[Daniel Gimpelevich]

On embedded devices, often the BSSID of an access point must be set from
userspace during the boot process. This provides a relatively clean way
of doing that without major side effects.

Signed-off-by: Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>
--- 
--- a/net/wireless/sysfs.c	2014-04-19 08:36:52.048511623 -0700
+++ b/net/wireless/sysfs.c	2014-04-19 08:38:09.196894176 -0700
@@ -24,18 +24,30 @@
 	return container_of(dev, struct cfg80211_registered_device, wiphy.dev);
 }
 
-#define SHOW_FMT(name, fmt, member)					\
+#define SHOW_FMT(name, fmt, member, perm)				\
 static ssize_t name ## _show(struct device *dev,			\
 			      struct device_attribute *attr,		\
 			      char *buf)				\
 {									\
 	return sprintf(buf, fmt "\n", dev_to_rdev(dev)->member);	\
 }									\
-static DEVICE_ATTR_RO(name)
+static DEVICE_ATTR_ ## perm(name)
 
-SHOW_FMT(index, "%d", wiphy_idx);
-SHOW_FMT(macaddress, "%pM", wiphy.perm_addr);
-SHOW_FMT(address_mask, "%pM", wiphy.addr_mask);
+static ssize_t macaddress_store(struct device *dev,
+			      struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	u8 temp[ETH_ALEN];
+
+	if (sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", temp, temp + 1,
+			temp + 2, temp + 3, temp + 4, temp + 5) == ETH_ALEN)
+		memcpy(dev_to_rdev(dev)->wiphy.perm_addr, temp, ETH_ALEN);
+	return count;
+}
+
+SHOW_FMT(index, "%d", wiphy_idx, RO);
+SHOW_FMT(macaddress, "%pM", wiphy.perm_addr, RW);
+SHOW_FMT(address_mask, "%pM", wiphy.addr_mask, RO);
 
 static ssize_t name_show(struct device *dev,
 			 struct device_attribute *attr,

Re: [PATCH] allow setting wiphy.perm_addr after driver probe

[Marcel Holtmann]

Hi Daniel,

> On embedded devices, often the BSSID of an access point must be set from
> userspace during the boot process. This provides a relatively clean way
> of doing that without major side effects.
> 
> Signed-off-by: Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>
> --- 
> --- a/net/wireless/sysfs.c	2014-04-19 08:36:52.048511623 -0700
> +++ b/net/wireless/sysfs.c	2014-04-19 08:38:09.196894176 -0700
> @@ -24,18 +24,30 @@
> 	return container_of(dev, struct cfg80211_registered_device, wiphy.dev);
> }
> 
> -#define SHOW_FMT(name, fmt, member)					\
> +#define SHOW_FMT(name, fmt, member, perm)				\
> static ssize_t name ## _show(struct device *dev,			\
> 			      struct device_attribute *attr,		\
> 			      char *buf)				\
> {									\
> 	return sprintf(buf, fmt "\n", dev_to_rdev(dev)->member);	\
> }									\
> -static DEVICE_ATTR_RO(name)
> +static DEVICE_ATTR_ ## perm(name)
> 
> -SHOW_FMT(index, "%d", wiphy_idx);
> -SHOW_FMT(macaddress, "%pM", wiphy.perm_addr);
> -SHOW_FMT(address_mask, "%pM", wiphy.addr_mask);
> +static ssize_t macaddress_store(struct device *dev,
> +			      struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	u8 temp[ETH_ALEN];
> +
> +	if (sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", temp, temp + 1,
> +			temp + 2, temp + 3, temp + 4, temp + 5) == ETH_ALEN)
> +		memcpy(dev_to_rdev(dev)->wiphy.perm_addr, temp, ETH_ALEN);
> +	return count;
> +}
> +
> +SHOW_FMT(index, "%d", wiphy_idx, RO);
> +SHOW_FMT(macaddress, "%pM", wiphy.perm_addr, RW);
> +SHOW_FMT(address_mask, "%pM", wiphy.addr_mask, RO);

isn't this dangerous to just allow writing to wiphy.perm_addr via sysfs. We ran into the same issue with Bluetooth and ROM only devices that have to unique address. Doing this via sysfs seems the wrong approach. It is messy and full of potential race conditions. I clearly opted against the sysfs solution for Bluetooth. Instead we build an infrastructure that allowed doing it cleanly via the Bluetooth mgmt API. Controllers that have no unique address are brought up as unconfigured and userspace clearly knows that it has to take steps to get an address programmed into the controller.

And I think something similar should be done for WiFi. It might be better to not create the initial wlan0 netdev interface if the hardware has not a single unique address available. That way the supplicant can just either get one from the flash filesystem or make up a proper random one before creating the netdev interface.

Regards

Marcel

Re: [PATCH] allow setting wiphy.perm_addr after driver probe

[Daniel Gimpelevich]

On Mon, 2014-08-11 at 13:25 -0700, Marcel Holtmann wrote:
> isn't this dangerous to just allow writing to wiphy.perm_addr via
> sysfs. We ran into the same issue with Bluetooth and ROM only devices
> that have to unique address. Doing this via sysfs seems the wrong
> approach. It is messy and full of potential race conditions. I clearly
> opted against the sysfs solution for Bluetooth. Instead we build an
> infrastructure that allowed doing it cleanly via the Bluetooth mgmt
> API. Controllers that have no unique address are brought up as
> unconfigured and userspace clearly knows that it has to take steps to
> get an address programmed into the controller.

My inclination is to agree; however, this does not exist for WiFi and
implementing it would require modifying every single driver.

> And I think something similar should be done for WiFi. It might be
> better to not create the initial wlan0 netdev interface if the
> hardware has not a single unique address available. That way the
> supplicant can just either get one from the flash filesystem or make
> up a proper random one before creating the netdev interface.
> 
The initial wlan0 netdev interface is *not* created, but the PHY records
a MAC address that cannot be overriden at the level that this sysfs node
reads. Perhaps a compromise would be to create a single new syscall to
write to it?

Re: [PATCH] allow setting wiphy.perm_addr after driver probe

[Marcel Holtmann]

Hi Daniel,

>> isn't this dangerous to just allow writing to wiphy.perm_addr via
>> sysfs. We ran into the same issue with Bluetooth and ROM only devices
>> that have to unique address. Doing this via sysfs seems the wrong
>> approach. It is messy and full of potential race conditions. I clearly
>> opted against the sysfs solution for Bluetooth. Instead we build an
>> infrastructure that allowed doing it cleanly via the Bluetooth mgmt
>> API. Controllers that have no unique address are brought up as
>> unconfigured and userspace clearly knows that it has to take steps to
>> get an address programmed into the controller.
> 
> My inclination is to agree; however, this does not exist for WiFi and
> implementing it would require modifying every single driver.

so what? That is not a reason to just hack something into sysfs. If it means every driver needs to be modified, then that has to be done.

>> And I think something similar should be done for WiFi. It might be
>> better to not create the initial wlan0 netdev interface if the
>> hardware has not a single unique address available. That way the
>> supplicant can just either get one from the flash filesystem or make
>> up a proper random one before creating the netdev interface.
>> 
> The initial wlan0 netdev interface is *not* created, but the PHY records
> a MAC address that cannot be overriden at the level that this sysfs node
> reads. Perhaps a compromise would be to create a single new syscall to
> write to it?

The initial wlan0 can be removed as every other netdev attached to the wiphy. It can also be as easily re-created.

Since the wiphy does not have a valid MAC address, my proposal here would be to just not create the wlan0 in the first place. This means that the wiphy can be still discovered via nl80211.

It also means that the wlan0 netdev needs to be created by userspace now. And a valid NL80211_ATTR_MAC be provided. Similar to what is already done for P2P devices at the moment. That should just solve the problem.

We really do not want to announce a netdev when registering the wiphy device and then having to mess with its MAC address via sysfs somehow. This all needs to be properly reflected over RTNL.

Regards

Marcel

Re: [PATCH] allow setting wiphy.perm_addr after driver probe

[Daniel Gimpelevich]

On Mon, 2014-08-11 at 13:56 -0700, Marcel Holtmann wrote:
> The initial wlan0 can be removed as every other netdev attached to the
> wiphy. It can also be as easily re-created.
> 
> Since the wiphy does not have a valid MAC address, my proposal here
> would be to just not create the wlan0 in the first place. This means
> that the wiphy can be still discovered via nl80211.

I repeat, currently wlan0 is *not* created.

> It also means that the wlan0 netdev needs to be created by userspace
> now. And a valid NL80211_ATTR_MAC be provided. Similar to what is
> already done for P2P devices at the moment. That should just solve the
> problem.

The creation of wlan0 already comes from userspace, but the PHY has its
own MAC.

> We really do not want to announce a netdev when registering the wiphy
> device and then having to mess with its MAC address via sysfs somehow.
> This all needs to be properly reflected over RTNL.
> 
Again, no netdev is announced. There just isn't a way to set the MAC of
the wiphy device itself.

How about this: What if the driver were to leave the MAC at all zeros
initially, and sysfs could set that if and only if it's all zeros at the
time?

Re: [PATCH] allow setting wiphy.perm_addr after driver probe

[Marcel Holtmann]

Hi Daniel,

>> The initial wlan0 can be removed as every other netdev attached to the
>> wiphy. It can also be as easily re-created.
>> 
>> Since the wiphy does not have a valid MAC address, my proposal here
>> would be to just not create the wlan0 in the first place. This means
>> that the wiphy can be still discovered via nl80211.
> 
> I repeat, currently wlan0 is *not* created.

what kind of hardware are you actually using here?

>> It also means that the wlan0 netdev needs to be created by userspace
>> now. And a valid NL80211_ATTR_MAC be provided. Similar to what is
>> already done for P2P devices at the moment. That should just solve the
>> problem.
> 
> The creation of wlan0 already comes from userspace, but the PHY has its
> own MAC.
> 
>> We really do not want to announce a netdev when registering the wiphy
>> device and then having to mess with its MAC address via sysfs somehow.
>> This all needs to be properly reflected over RTNL.
>> 
> Again, no netdev is announced. There just isn't a way to set the MAC of
> the wiphy device itself.
> 
> How about this: What if the driver were to leave the MAC at all zeros
> initially, and sysfs could set that if and only if it's all zeros at the
> time?

Internally it might do that, but I do not see it exposing the NL80211_ATTR_MAC when you get the attributes for wiphy.

So I am still saying that when you do NL80211_CMD_NEW_INTERFACE allow providing NL80211_ATTR_MAC to set the MAC address to be used. It might be useful that the wiphy exposes an attribute saying that it does not have a default MAC address, but that should be it. I do not like these magic 00:00:00:00:00:00 games. As I mentioned earlier, in Bluetooth we just deal with allowing the driver to set a flag that it does not have a valid address. And then the core takes care of dealing with it.

Regards

Marcel

Re: [PATCH] allow setting wiphy.perm_addr after driver probe

[Daniel Gimpelevich]

On Mon, 2014-08-11 at 15:41 -0700, Marcel Holtmann wrote:
> what kind of hardware are you actually using here?
> 
It's ath9k on MIPS under OpenWrt.
> 
> Internally it might do that, but I do not see it exposing the
> NL80211_ATTR_MAC when you get the attributes for wiphy.

When wlan0 is created, it can be created with its own MAC irrespective
of the wiphy MAC. In OpenWrt, the wlan0 MAC can be supplied and assigned
to a netdev created on a specific wiphy identified by its MAC, and if
that cannot be predicted, there is no wlan0.

> So I am still saying that when you do NL80211_CMD_NEW_INTERFACE allow
> providing NL80211_ATTR_MAC to set the MAC address to be used. It might
> be useful that the wiphy exposes an attribute saying that it does not
> have a default MAC address, but that should be it. I do not like these
> magic 00:00:00:00:00:00 games. As I mentioned earlier, in Bluetooth we
> just deal with allowing the driver to set a flag that it does not have
> a valid address. And then the core takes care of dealing with it.
> 
An attribute saying there is no default MAC is helpful only if there is
a way to supply a new default to the wiphy.

Re: [PATCH] allow setting wiphy.perm_addr after driver probe

[Marcel Holtmann]

Hi Daniel,

>> Internally it might do that, but I do not see it exposing the
>> NL80211_ATTR_MAC when you get the attributes for wiphy.
> 
> When wlan0 is created, it can be created with its own MAC irrespective
> of the wiphy MAC. In OpenWrt, the wlan0 MAC can be supplied and assigned
> to a netdev created on a specific wiphy identified by its MAC, and if
> that cannot be predicted, there is no wlan0.

the way I read the nl80211 code is that the NL80211_CMD_NEW_INTERFACE requires a wiphy device to be specified. And that is actually just a number. So I have no idea what the MAC has to here.

>> So I am still saying that when you do NL80211_CMD_NEW_INTERFACE allow
>> providing NL80211_ATTR_MAC to set the MAC address to be used. It might
>> be useful that the wiphy exposes an attribute saying that it does not
>> have a default MAC address, but that should be it. I do not like these
>> magic 00:00:00:00:00:00 games. As I mentioned earlier, in Bluetooth we
>> just deal with allowing the driver to set a flag that it does not have
>> a valid address. And then the core takes care of dealing with it.
>> 
> An attribute saying there is no default MAC is helpful only if there is
> a way to supply a new default to the wiphy.

Why does the wiphy need to know the MAC if it is always specified from userspace when actually creating the new netdev interface. Works for P2P devices, so why wouldn't it work for access point and station mode?

Regards

Marcel

Re: [PATCH] allow setting wiphy.perm_addr after driver probe

[Daniel Gimpelevich]

On Mon, 2014-08-11 at 16:56 -0700, Marcel Holtmann wrote:
> the way I read the nl80211 code is that the NL80211_CMD_NEW_INTERFACE
> requires a wiphy device to be specified. And that is actually just a
> number. So I have no idea what the MAC has to here.
> 
OpenWrt finds a wiphy by its MAC.

> Why does the wiphy need to know the MAC if it is always specified from
> userspace when actually creating the new netdev interface. Works for
> P2P devices, so why wouldn't it work for access point and station
> mode?
> 
A MAC can be specified for the netdev, but it is assigned to a wiphy
identified by its MAC.

--
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] allow setting wiphy.perm_addr after driver probe

[Marcel Holtmann]

Hi Daniel,

>> the way I read the nl80211 code is that the NL80211_CMD_NEW_INTERFACE
>> requires a wiphy device to be specified. And that is actually just a
>> number. So I have no idea what the MAC has to here.
>> 
> OpenWrt finds a wiphy by its MAC.

maybe that is the first problem right there. I see that over sysfs you get phy80211/macaddress. However if you do a dump via NL80211_CMD_GET_WIPHY then you do not have the MAC address there.

>> Why does the wiphy need to know the MAC if it is always specified from
>> userspace when actually creating the new netdev interface. Works for
>> P2P devices, so why wouldn't it work for access point and station
>> mode?
>> 
> A MAC can be specified for the netdev, but it is assigned to a wiphy
> identified by its MAC.

That you find the wiphy device by its MAC address seems to be your problem. As I said, on nl80211 you are using the wiphy unique number and not a MAC address.

If you want to use some sort of device discovery via sysfs, then use phy80211/index and other attributes to uniquely identify the device. Basing this on the MAC address seems dangerous. Especially since it might be not valid or an identical default for all cards.

Regards

Marcel

Re: [PATCH] allow setting wiphy.perm_addr after driver probe

[Jonas Gorski]

On Tue, Aug 12, 2014 at 9:59 AM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> Hi Daniel,
>
>>> the way I read the nl80211 code is that the NL80211_CMD_NEW_INTERFACE
>>> requires a wiphy device to be specified. And that is actually just a
>>> number. So I have no idea what the MAC has to here.
>>>
>> OpenWrt finds a wiphy by its MAC.
>
> maybe that is the first problem right there. I see that over sysfs you get phy80211/macaddress. However if you do a dump via NL80211_CMD_GET_WIPHY then you do not have the MAC address there.

OpenWrt nowadays uses the device path in sysfs/devices to identify
wiphys, not the mac anymore (e.g 'pci0000:00/0000:00:01.0/ssb0:0').


Jonas
--
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