2008-09-25 15:48:45

by Tomas Winkler

[permalink] [raw]
Subject: [PATCH RFC] mac80211: notify mac80211 about rfkill events

From: Emmanuel Grumbach <[email protected]>

Currently mac80211 is not notified about rfill state of low level driver
and doesn't now when it's disconnected and when to restart connection
There also there is conflict or no synchronization between 'wext txpower
disable' and rfkill events coming from rfkill subsystem

This patch is just a sketch of possible solution, it probably even doesn't
compile

Signed-off-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Tomas Winkler <[email protected]>
---
net/mac80211/main.c | 34 ++++++++++++++++++++++++++++++++++
net/mac80211/wext.c | 18 +++++++++++++++---
2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index d608c44..df2dd7d 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -21,6 +21,7 @@
#include <linux/wireless.h>
#include <linux/rtnetlink.h>
#include <linux/bitmap.h>
+#include <linux/rfkill.h>
#include <net/net_namespace.h>
#include <net/cfg80211.h>

@@ -704,6 +705,33 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
}
EXPORT_SYMBOL(ieee80211_tx_status);

+static int ieee80211_rfkill_notifier(struct notifier_block *nb, unsigned long eventid,
+ void *data)
+{
+ struct rfkill *rfkill = (struct rfkill *)data;
+ struct ieee80211_hw *hw = rfkill->data;
+ struct ieee80211_local *local = hw_to_local(hw);
+
+ switch (eventid) {
+ case RFKILL_STATE_CHANGED:
+ hw->conf.radio_enabled =
+ (rfkill->state == RFKILL_STATE_UNBLOCKED);
+ printk(KERN_DEBUG "%s %d RADIO %d\n", __func__, __LINE__, hw->conf.radio_enabled);
+ ieee80211_led_radio(local, local->hw.conf.radio_enabled);
+ ieee80211_hw_config(local);
+ break;
+ default:
+ break;
+ }
+
+ return NOTIFY_DONE;
+}
+
+struct notifier_block ieee80211_rfkill_notifier_nb = {
+ .notifier_call = ieee80211_rfkill_notifier,
+ .priority = 0,
+};
+
struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
const struct ieee80211_ops *ops)
{
@@ -932,6 +960,9 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)

ieee80211_led_init(local);

+ if (local->hw.rfkill)
+ register_rfkill_notifier(&ieee80211_rfkill_notifier_nb);
+
return 0;

fail_wep:
@@ -961,6 +992,9 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
tasklet_kill(&local->tx_pending_tasklet);
tasklet_kill(&local->tasklet);

+ if (local->hw.rfkill)
+ unregister_rfkill_notifier(&ieee80211_rfkill_notifier_nb);
+
rtnl_lock();

/*
diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
index 7e0d53a..6b96edf 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -16,6 +16,7 @@
#include <linux/etherdevice.h>
#include <linux/if_arp.h>
#include <linux/wireless.h>
+#include <linux/rfkill.h>
#include <net/iw_handler.h>
#include <asm/uaccess.h>

@@ -684,9 +685,20 @@ static int ieee80211_ioctl_siwtxpower(struct net_device *dev,
}

if (local->hw.conf.radio_enabled != !(data->txpower.disabled)) {
- local->hw.conf.radio_enabled = !(data->txpower.disabled);
- need_reconfig = 1;
- ieee80211_led_radio(local, local->hw.conf.radio_enabled);
+ if (!local->hw.rfkill) {
+ local->hw.conf.radio_enabled = !(data->txpower.disabled);
+ need_reconfig = 1;
+ ieee80211_led_radio(local, local->hw.conf.radio_enabled);
+ } else {
+ if (data->txpower.disabled)
+ rfkill_force_state(local->hw.rfkill,
+ RFKILL_STATE_SOFT_BLOCKED);
+ else
+ /* XXXX: do we make sure there is no HW RFKILL? */
+ rfkill_force_state(local->hw.rfkill,
+ RFKILL_STATE_UNBLOCKED);
+ }
+ return 0;
}

if (need_reconfig) {
--
1.5.4.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.



2008-09-27 08:12:40

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: notify mac80211 about rfkill events

On Saturday 27 September 2008, Tomas Winkler wrote:
> >> > That's definitely other option we wanted to suggest that mac80211
> >> > would register itself to rfkill subsystem and will provide to driver
> >> > appropriate callbacks. The question is how drivers vary in the rfkil
> >> > implementation and whether it wouldn't be more complex, in that case
> >> > the notification is quite clean solution.
> >>
> >> How complex does it have to be?
>
> > Not really, probably it only needs to be a wrapper around rfkill_force_state
> > where we can use the RFKILL_ defines to indicate the BLOCKED status.
>
> Implementation wise it's more complicated since you have to teach each
> driver under mac80211, this mean more
> code and more bugs but I'm not sure this is the objective on this list
> :) As opposite to just notifying mac about a rfkill
> event so it's not trying to configure to access device, etc.

Those driver that do generate RFKILL events already have the rfkill structure
implemented, so for those drivers they only have to remvoe that code and just
call the mac80211 rfkill handler when the RFKILL key is pressed.
All other drivers which don't have a rfkill button don't have to do anything because
mac80211 handles everything for them.

> >> > > That means that the only change needed in ieee80211_ioctl_siwtxpower() is
> >> > > only allowing the enabling of the radio when RFKILL is not set to BLOCKED.
> >> >
> >> > That's just complicates everything and moving the policy decisions to
> >> > the driver after all even
> >> > form txpower off you implement it as soft rfkill.
> >
> > Why does it complicate things? It means that mac80211 doesn't use
> > rfkill_force_state() when the user triggers a radio state change using
> > iw/iwconfig/ifconfig or whatever userspace tool.
>
> I'm not sure I understand your intention, how do you plane to kill
> radio then in this case, you are still to obliged to do so.

Correct, and you can still do that using the approach how it is currently implemented
in mac80211. Calling config() with the disable_radio field set.

I am just saying that userspace commands like:
ifconfig wlan0 up
iwconfig wlan0 txpower on
are not RFKILL events which are handled by the rfkill layer. Those are userspace commands
which can simply be handled by mac80211. The rfkill layer is there to support events
when a user presses the RFKILL button/key/slider on the device or laptop.

See the discussion between Henrique and me
"[RFC] b43: A patch for control of the radio LED using rfkill"
from last week.

> > mac80211 doesn't generate rfkill events because it isn't tied to any device
> > keys/buttons/sliders. That is what the drivers do. And they should only listen
> > to that key/button/slider.
>
> writing 0 to sysfs file is not a button either it sill should soft
> kill the radio.

Well that feature was requested by others to create simple RFKILL event listeners
in userspace to toggle the radio through the rfkill interface. Since using that interface
you can simply toggle the radio state for all devices of the same type.

> The sw switch is here to enable
> killing and enabling radio from software. All the buttons stuff should
> go under hard switches.

Where is it documented that the RFKILL_STATE_SOFT_BLOCKED is for events from software?
RFKILL_STATE_HARD_BLOCKED was called into life for non-overridable RFKILL states, not
to allow for software states to be handled in rfkill.

The rfkill layer is not the global radio state handler for any wireless device, it is the radio
state handler for RFKILL events coming from actually pressing RFKILL keys on hardware.

> > Come to think of it, there is a bug in the previous patch since it doesn't handle
> > the case when the interface is brought up during a BLOCKED state.
> >
>
> what is the p previous patch? If you mean the one I've sent, then
> it's known to be incomplete, it was produced to invoke this
> conversation.

I understand, I just wanted to make a note of it.

> > So all that has to be done is:
> > * rfkill BLOCK event received in mac80211
> > * set flag to indicate the BLOCKED state
> > * disable radio
>
> In case of notification chain you don't disable radio from mac80211
>
> > * prevent radio being enabled through ifup
> > * prevent radio being enabled through iwconfig txpower on
> >
> > * rfkill UNBLOCK event received from mac80211
> > * clear flag to indicate UNBLOCKED state
> > * restore radio to the by iwconfig/ifup configured state
> Same here.
> Tomas

All the more reason then to use the rfkill structure directly instead of
using the notification system.

Ivo

2008-09-26 18:18:47

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: notify mac80211 about rfkill events

On Friday 26 September 2008, Johannes Berg wrote:
> On Fri, 2008-09-26 at 01:05 +0300, Tomas Winkler wrote:
>
> > > I am not sure if registring a notifier would be the best solution,
> > > persionally I was thinking of implementing the rfkill structure into ieee80211_local
> > > and make it listen to events directly.
>
> I think I like this better.
>
> > That's definitely other option we wanted to suggest that mac80211
> > would register itself to rfkill subsystem and will provide to driver
> > appropriate callbacks. The question is how drivers vary in the rfkil
> > implementation and whether it wouldn't be more complex, in that case
> > the notification is quite clean solution.
>
> How complex does it have to be?

Not really, probably it only needs to be a wrapper around rfkill_force_state
where we can use the RFKILL_ defines to indicate the BLOCKED status.

> > > That means that the only change needed in ieee80211_ioctl_siwtxpower() is
> > > only allowing the enabling of the radio when RFKILL is not set to BLOCKED.
> >
> > That's just complicates everything and moving the policy decisions to
> > the driver after all even
> > form txpower off you implement it as soft rfkill.

Why does it complicate things? It means that mac80211 doesn't use
rfkill_force_state() when the user triggers a radio state change using
iw/iwconfig/ifconfig or whatever userspace tool.

mac80211 doesn't generate rfkill events because it isn't tied to any device
keys/buttons/sliders. That is what the drivers do. And they should only listen
to that key/button/slider.

> > I would suggest just remove the support for txpower off in mac80211
> > now when appropriate or sync it with soft block after all it coming
> > from user space as a software event.
>
> I think what we should do is in mac80211 simply synthesize the
> "radio_enabled" state that the config callback has from both rfkill and
> txpower off. Anything wrong with that?

Come to think of it, there is a bug in the previous patch since it doesn't handle
the case when the interface is brought up during a BLOCKED state.

So all that has to be done is:
* rfkill BLOCK event received in mac80211
* set flag to indicate the BLOCKED state
* disable radio
* prevent radio being enabled through ifup
* prevent radio being enabled through iwconfig txpower on

* rfkill UNBLOCK event received from mac80211
* clear flag to indicate UNBLOCKED state
* restore radio to the by iwconfig/ifup configured state

Ivo

2008-09-26 11:05:32

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: notify mac80211 about rfkill events

On Fri, 2008-09-26 at 01:05 +0300, Tomas Winkler wrote:

> > I am not sure if registring a notifier would be the best solution,
> > persionally I was thinking of implementing the rfkill structure into ieee80211_local
> > and make it listen to events directly.

I think I like this better.

> That's definitely other option we wanted to suggest that mac80211
> would register itself to rfkill subsystem and will provide to driver
> appropriate callbacks. The question is how drivers vary in the rfkil
> implementation and whether it wouldn't be more complex, in that case
> the notification is quite clean solution.

How complex does it have to be?

> > That means that the only change needed in ieee80211_ioctl_siwtxpower() is
> > only allowing the enabling of the radio when RFKILL is not set to BLOCKED.
>
> That's just complicates everything and moving the policy decisions to
> the driver after all even
> form txpower off you implement it as soft rfkill.
>
> I would suggest just remove the support for txpower off in mac80211
> now when appropriate or sync it with soft block after all it coming
> from user space as a software event.

I think what we should do is in mac80211 simply synthesize the
"radio_enabled" state that the config callback has from both rfkill and
txpower off. Anything wrong with that?

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-28 19:49:36

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: notify mac80211 about rfkill events

>> > See the discussion between Henrique and me
>> > "[RFC] b43: A patch for control of the radio LED using rfkill"
>> > from last week.

So I read that thread and first conclusion I came to is that the whole
long thread was there due to
fact that you were not able to define rfkill in the same language even
though Henrique spent a lot of time on documentation.
iwlwifi HW is more like b43 has ability for hard and soft killing the
radio physically. So what we define as soft rfkill is actually writing
the register to the HW and that was my intention when I'm talking
about soft rfkill. I don't define rfkill to be singal from input
device.
I guess that the confusion about connecting hw switch to rfkill line
comes from difference between USB and PCI devices. While Wifi PCI
usually sits in a mini card slots witch defines rfkill line by spec
this is not the case in a USB dongle so this have to delivered around,
this is rather deficiency in USB spec. Of course USB can sit also in
mini card but that's not general case.
Soft rfkill line in PCI devices is here to answer needs such as
txpower off/on so this is not the same think as catching external
event of hw switch and applying rfkill into devices or other tricks
that laptops vendors do to implement it.
When I'm talking about policy this is decision how radio state is
drawn from sort and hw rfkill switches in PCI context.
I have to read some more about your conclusions in the thread so I
won't continue here I hope Emmanuel and I will come to some
implementations soon as we need to close the rfkill bugs in iwlwifi
that comes from fact that mac80211 is not in the game yet. (again
rfkill in register level).

Thanks
Tomas

2008-09-27 11:08:56

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: notify mac80211 about rfkill events

On Sat, Sep 27, 2008 at 11:12 AM, Ivo van Doorn <[email protected]> wrote:
> On Saturday 27 September 2008, Tomas Winkler wrote:
>> >> > That's definitely other option we wanted to suggest that mac80211
>> >> > would register itself to rfkill subsystem and will provide to driver
>> >> > appropriate callbacks. The question is how drivers vary in the rfkil
>> >> > implementation and whether it wouldn't be more complex, in that case
>> >> > the notification is quite clean solution.
>> >>
>> >> How complex does it have to be?
>>
>> > Not really, probably it only needs to be a wrapper around rfkill_force_state
>> > where we can use the RFKILL_ defines to indicate the BLOCKED status.
>>
>> Implementation wise it's more complicated since you have to teach each
>> driver under mac80211, this mean more
>> code and more bugs but I'm not sure this is the objective on this list
>> :) As opposite to just notifying mac about a rfkill
>> event so it's not trying to configure to access device, etc.
>
> Those driver that do generate RFKILL events already have the rfkill structure
> implemented, so for those drivers they only have to remvoe that code and just
> call the mac80211 rfkill handler when the RFKILL key is pressed.
> All other drivers which don't have a rfkill button don't have to do anything because
> mac80211 handles everything for them.

Each driver has to implement the low level of rfkill. I'm not sure how
mac80211 could handle it.

>
>> >> > > That means that the only change needed in ieee80211_ioctl_siwtxpower() is
>> >> > > only allowing the enabling of the radio when RFKILL is not set to BLOCKED.
>> >> >
>> >> > That's just complicates everything and moving the policy decisions to
>> >> > the driver after all even
>> >> > form txpower off you implement it as soft rfkill.
>> >
>> > Why does it complicate things? It means that mac80211 doesn't use
>> > rfkill_force_state() when the user triggers a radio state change using
>> > iw/iwconfig/ifconfig or whatever userspace tool.
>>
>> I'm not sure I understand your intention, how do you plane to kill
>> radio then in this case, you are still to obliged to do so.
>
> Correct, and you can still do that using the approach how it is currently implemented
> in mac80211. Calling config() with the disable_radio field set.

This is wrong approach you are pushing more policy into driver.

>
> I am just saying that userspace commands like:
> ifconfig wlan0 up
> iwconfig wlan0 txpower on
> are not RFKILL events which are handled by the rfkill layer. Those are userspace commands
> which can simply be handled by mac80211. The rfkill layer is there to support events
> when a user presses the RFKILL button/key/slider on the device or laptop.

wlan0 up is not rfkill event but txpower on/off definitely provide
rfkill line as defiend in Henriques doc and has affect on rfkill
state separating them just creates more confusion.
This is trying to create two APIs for the same thing both paths just
results into software rfkill and this just pushes arbitration too low
into the driver. Driver should be just notified if to kill or arise
from death the radio

> See the discussion between Henrique and me
> "[RFC] b43: A patch for control of the radio LED using rfkill"
> from last week.
>
>> > mac80211 doesn't generate rfkill events because it isn't tied to any device
>> > keys/buttons/sliders. That is what the drivers do. And they should only listen
>> > to that key/button/slider.
>>
>> writing 0 to sysfs file is not a button either it sill should soft
>> kill the radio.
>
> Well that feature was requested by others to create simple RFKILL event listeners
> in userspace to toggle the radio through the rfkill interface. Since using that interface
> you can simply toggle the radio state for all devices of the same type.
>

>> The sw switch is here to enable
>> killing and enabling radio from software. All the buttons stuff should
>> go under hard switches.
>
> Where is it documented that the RFKILL_STATE_SOFT_BLOCKED is for events from software?

It's definitely affected if I call radio.disabled through mac->config

> RFKILL_STATE_HARD_BLOCKED was called into life for non-overridable RFKILL states, not
> to allow for software states to be handled in rfkill.
>

> The rfkill layer is not the global radio state handler for any wireless device, it is the radio
> state handler for RFKILL events coming from actually pressing RFKILL keys on hardware.

I cannot say I like this definition, again it creates opening for too
many APIs doing the same thing.

>> > Come to think of it, there is a bug in the previous patch since it doesn't handle
>> > the case when the interface is brought up during a BLOCKED state.
>> >
>>
>> what is the p previous patch? If you mean the one I've sent, then
>> it's known to be incomplete, it was produced to invoke this
>> conversation.
>
> I understand, I just wanted to make a note of it.
>
>> > So all that has to be done is:
>> > * rfkill BLOCK event received in mac80211
>> > * set flag to indicate the BLOCKED state
>> > * disable radio
>>
>> In case of notification chain you don't disable radio from mac80211
>>
>> > * prevent radio being enabled through ifup
>> > * prevent radio being enabled through iwconfig txpower on
>> >
>> > * rfkill UNBLOCK event received from mac80211
>> > * clear flag to indicate UNBLOCKED state
>> > * restore radio to the by iwconfig/ifup configured state
>> Same here.
>> Tomas
>
> All the more reason then to use the rfkill structure directly instead of
> using the notification system.

You contradict your self. If rfkill subsystem is to handle just button
events why it doesn't just handle the rfill state notifications

Tomas

2008-09-25 22:05:47

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: notify mac80211 about rfkill events

On Fri, Sep 26, 2008 at 12:10 AM, Ivo van Doorn <[email protected]> wrote:
> Hi,
>
>> Currently mac80211 is not notified about rfill state of low level driver
>> and doesn't now when it's disconnected and when to restart connection
>> There also there is conflict or no synchronization between 'wext txpower
>> disable' and rfkill events coming from rfkill subsystem
>>
>> This patch is just a sketch of possible solution, it probably even doesn't
>> compile
>
> I am not sure if registring a notifier would be the best solution,
> persionally I was thinking of implementing the rfkill structure into ieee80211_local
> and make it listen to events directly.
>
> Through a small callback function drivers can easily report their rfkill events
> to mac80211 which in turn will call rfkill_force_state().

> On the other hand, perhaps notifications might work better, depends on what
> the drivers like better.

That's definitely other option we wanted to suggest that mac80211
would register itself to rfkill subsystem and will provide to driver
appropriate callbacks. The question is how drivers vary in the rfkil
implementation and whether it wouldn't be more complex, in that case
the notification is quite clean solution.

> As mentioned in a discussion on the list a few days ago, SOFT_BLOCKED
> is not for 'iwconfig txpower off' commands or any other commands coming
> from userspace. Those are not HW triggered rfkill events.

> That means that the only change needed in ieee80211_ioctl_siwtxpower() is
> only allowing the enabling of the radio when RFKILL is not set to BLOCKED.

That's just complicates everything and moving the policy decisions to
the driver after all even
form txpower off you implement it as soft rfkill.

I would suggest just remove the support for txpower off in mac80211
now when appropriate or sync it with soft block after all it coming
from user space as a software event.

Tomas

2008-09-27 11:34:21

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: notify mac80211 about rfkill events

On Saturday 27 September 2008, Tomas Winkler wrote:
> On Sat, Sep 27, 2008 at 11:12 AM, Ivo van Doorn <[email protected]> wrote:
> > On Saturday 27 September 2008, Tomas Winkler wrote:
> >> >> > That's definitely other option we wanted to suggest that mac80211
> >> >> > would register itself to rfkill subsystem and will provide to driver
> >> >> > appropriate callbacks. The question is how drivers vary in the rfkil
> >> >> > implementation and whether it wouldn't be more complex, in that case
> >> >> > the notification is quite clean solution.
> >> >>
> >> >> How complex does it have to be?
> >>
> >> > Not really, probably it only needs to be a wrapper around rfkill_force_state
> >> > where we can use the RFKILL_ defines to indicate the BLOCKED status.
> >>
> >> Implementation wise it's more complicated since you have to teach each
> >> driver under mac80211, this mean more
> >> code and more bugs but I'm not sure this is the objective on this list
> >> :) As opposite to just notifying mac about a rfkill
> >> event so it's not trying to configure to access device, etc.
> >
> > Those driver that do generate RFKILL events already have the rfkill structure
> > implemented, so for those drivers they only have to remvoe that code and just
> > call the mac80211 rfkill handler when the RFKILL key is pressed.
> > All other drivers which don't have a rfkill button don't have to do anything because
> > mac80211 handles everything for them.
>
> Each driver has to implement the low level of rfkill. I'm not sure how
> mac80211 could handle it.

What low level of rfkill? You mean the rfkill_force_state call?
I already said that can be done through a wrapper function provided by mac80211,
the driver just calls it when the RFKILL key was pressed.

For the remainder of the rfkill implementation in mac80211, all mac80211 needs to
do is _listen_ to toggle_radio() callback function and acts upon that request.

> >> >> > > That means that the only change needed in ieee80211_ioctl_siwtxpower() is
> >> >> > > only allowing the enabling of the radio when RFKILL is not set to BLOCKED.
> >> >> >
> >> >> > That's just complicates everything and moving the policy decisions to
> >> >> > the driver after all even
> >> >> > form txpower off you implement it as soft rfkill.
> >> >
> >> > Why does it complicate things? It means that mac80211 doesn't use
> >> > rfkill_force_state() when the user triggers a radio state change using
> >> > iw/iwconfig/ifconfig or whatever userspace tool.
> >>
> >> I'm not sure I understand your intention, how do you plane to kill
> >> radio then in this case, you are still to obliged to do so.
> >
> > Correct, and you can still do that using the approach how it is currently implemented
> > in mac80211. Calling config() with the disable_radio field set.
>
> This is wrong approach you are pushing more policy into driver.

What policy? With rfkill implemented in mac80211, the driver needs to no _nothing_
about rfkill. All it does is listen to mac80211 to enable/disable the radio. Which is something
it should be doing already.
Furthermore, drivers who have implemented a rfkill structure already could remove it and
just use the wrapper function I mentioned earlier to inform mac80211 about the RFKILL event
(coming from the actual HW key/button/slider!) and mac80211 can forward the request to
rfkill. (Unless of course you want to make the rfkills structure visible to the driver, which
probably isn't a good idea).

> > I am just saying that userspace commands like:
> > ifconfig wlan0 up
> > iwconfig wlan0 txpower on
> > are not RFKILL events which are handled by the rfkill layer. Those are userspace commands
> > which can simply be handled by mac80211. The rfkill layer is there to support events
> > when a user presses the RFKILL button/key/slider on the device or laptop.
>
> wlan0 up is not rfkill event but txpower on/off definitely provide
> rfkill line as defiend in Henriques doc and has affect on rfkill
> state separating them just creates more confusion.

I repeat: rfkill is only a layer to provide support for HARDWARE rfkill events, aka pressing
a button/key/slider on the HARDWARE. It is not a notification system which is triggered for
userspace events.

rfkill is intended to act correctly when the user presses the RFKILL key on its hardware, it generates
an event and a userspace agent can act upon it by (for example) shutting down all radios because
pressing the RFKILL key means the hardware has entered a region where no wireless devices are allowed.

If you start generating events for userspace commands like "iwconfig wlan0 txpower off" then you
can get userspace agents that decide to disable wlan1, wlan2, off as well.

> This is trying to create two APIs for the same thing both paths just
> results into software rfkill and this just pushes arbitration too low
> into the driver. Driver should be just notified if to kill or arise
> from death the radio

Yes, and so it receives that command. I am not seeing the problem.

> > See the discussion between Henrique and me
> > "[RFC] b43: A patch for control of the radio LED using rfkill"
> > from last week.

You can find the thread here:
http://marc.info/?l=linux-wireless&m=122171448920267&w=2
it is the entire discussion between me and Henrique about this issue.

> >> > mac80211 doesn't generate rfkill events because it isn't tied to any device
> >> > keys/buttons/sliders. That is what the drivers do. And they should only listen
> >> > to that key/button/slider.
> >>
> >> writing 0 to sysfs file is not a button either it sill should soft
> >> kill the radio.
> >
> > Well that feature was requested by others to create simple RFKILL event listeners
> > in userspace to toggle the radio through the rfkill interface. Since using that interface
> > you can simply toggle the radio state for all devices of the same type.
> >
>
> >> The sw switch is here to enable
> >> killing and enabling radio from software. All the buttons stuff should
> >> go under hard switches.
> >
> > Where is it documented that the RFKILL_STATE_SOFT_BLOCKED is for events from software?
>
> It's definitely affected if I call radio.disabled through mac->config

Again, RFKILL layer is intended to provide support for the _HARDWARE_ generated events.

> > RFKILL_STATE_HARD_BLOCKED was called into life for non-overridable RFKILL states, not
> > to allow for software states to be handled in rfkill.
> >
>
> > The rfkill layer is not the global radio state handler for any wireless device, it is the radio
> > state handler for RFKILL events coming from actually pressing RFKILL keys on hardware.
>
> I cannot say I like this definition, again it creates opening for too
> many APIs doing the same thing.

RFKILL generates events for HW key/button/slider events, and forwards that event to userspace.
How many API's are doing that?

> >> > So all that has to be done is:
> >> > * rfkill BLOCK event received in mac80211
> >> > * set flag to indicate the BLOCKED state
> >> > * disable radio
> >>
> >> In case of notification chain you don't disable radio from mac80211
> >>
> >> > * prevent radio being enabled through ifup
> >> > * prevent radio being enabled through iwconfig txpower on
> >> >
> >> > * rfkill UNBLOCK event received from mac80211
> >> > * clear flag to indicate UNBLOCKED state
> >> > * restore radio to the by iwconfig/ifup configured state
> >> Same here.
> >> Tomas
> >
> > All the more reason then to use the rfkill structure directly instead of
> > using the notification system.
>
> You contradict your self. If rfkill subsystem is to handle just button
> events why it doesn't just handle the rfill state notifications

You mean why rfkill doesn't directly turn events into toggle_radio() events?
Because that would be policy in the kernel, and those things should be done
in userspace.

Ivo

2008-09-25 21:10:26

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: notify mac80211 about rfkill events

Hi,

> Currently mac80211 is not notified about rfill state of low level driver
> and doesn't now when it's disconnected and when to restart connection
> There also there is conflict or no synchronization between 'wext txpower
> disable' and rfkill events coming from rfkill subsystem
>
> This patch is just a sketch of possible solution, it probably even doesn't
> compile

I am not sure if registring a notifier would be the best solution,
persionally I was thinking of implementing the rfkill structure into ieee80211_local
and make it listen to events directly.

Through a small callback function drivers can easily report their rfkill events
to mac80211 which in turn will call rfkill_force_state().

On the other hand, perhaps notifications might work better, depends on what
the drivers like better.

> Signed-off-by: Emmanuel Grumbach <[email protected]>
> Signed-off-by: Tomas Winkler <[email protected]>
> ---
> net/mac80211/main.c | 34 ++++++++++++++++++++++++++++++++++
> net/mac80211/wext.c | 18 +++++++++++++++---
> 2 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index d608c44..df2dd7d 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -21,6 +21,7 @@
> #include <linux/wireless.h>
> #include <linux/rtnetlink.h>
> #include <linux/bitmap.h>
> +#include <linux/rfkill.h>
> #include <net/net_namespace.h>
> #include <net/cfg80211.h>
>
> @@ -704,6 +705,33 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
> }
> EXPORT_SYMBOL(ieee80211_tx_status);
>
> +static int ieee80211_rfkill_notifier(struct notifier_block *nb, unsigned long eventid,
> + void *data)
> +{
> + struct rfkill *rfkill = (struct rfkill *)data;
> + struct ieee80211_hw *hw = rfkill->data;
> + struct ieee80211_local *local = hw_to_local(hw);
> +
> + switch (eventid) {
> + case RFKILL_STATE_CHANGED:
> + hw->conf.radio_enabled =
> + (rfkill->state == RFKILL_STATE_UNBLOCKED);
> + printk(KERN_DEBUG "%s %d RADIO %d\n", __func__, __LINE__, hw->conf.radio_enabled);
> + ieee80211_led_radio(local, local->hw.conf.radio_enabled);
> + ieee80211_hw_config(local);
> + break;
> + default:
> + break;
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> +struct notifier_block ieee80211_rfkill_notifier_nb = {
> + .notifier_call = ieee80211_rfkill_notifier,
> + .priority = 0,
> +};
> +
> struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
> const struct ieee80211_ops *ops)
> {
> @@ -932,6 +960,9 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
>
> ieee80211_led_init(local);
>
> + if (local->hw.rfkill)
> + register_rfkill_notifier(&ieee80211_rfkill_notifier_nb);
> +
> return 0;
>
> fail_wep:
> @@ -961,6 +992,9 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
> tasklet_kill(&local->tx_pending_tasklet);
> tasklet_kill(&local->tasklet);
>
> + if (local->hw.rfkill)
> + unregister_rfkill_notifier(&ieee80211_rfkill_notifier_nb);
> +
> rtnl_lock();
>
> /*
> diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
> index 7e0d53a..6b96edf 100644
> --- a/net/mac80211/wext.c
> +++ b/net/mac80211/wext.c
> @@ -16,6 +16,7 @@
> #include <linux/etherdevice.h>
> #include <linux/if_arp.h>
> #include <linux/wireless.h>
> +#include <linux/rfkill.h>
> #include <net/iw_handler.h>
> #include <asm/uaccess.h>
>
> @@ -684,9 +685,20 @@ static int ieee80211_ioctl_siwtxpower(struct net_device *dev,
> }
>
> if (local->hw.conf.radio_enabled != !(data->txpower.disabled)) {
> - local->hw.conf.radio_enabled = !(data->txpower.disabled);
> - need_reconfig = 1;
> - ieee80211_led_radio(local, local->hw.conf.radio_enabled);
> + if (!local->hw.rfkill) {
> + local->hw.conf.radio_enabled = !(data->txpower.disabled);
> + need_reconfig = 1;
> + ieee80211_led_radio(local, local->hw.conf.radio_enabled);
> + } else {
> + if (data->txpower.disabled)
> + rfkill_force_state(local->hw.rfkill,
> + RFKILL_STATE_SOFT_BLOCKED);

As mentioned in a discussion on the list a few days ago, SOFT_BLOCKED
is not for 'iwconfig txpower off' commands or any other commands coming
from userspace. Those are not HW triggered rfkill events.

That means that the only change needed in ieee80211_ioctl_siwtxpower() is
only allowing the enabling of the radio when RFKILL is not set to BLOCKED.

> + else
> + /* XXXX: do we make sure there is no HW RFKILL? */
> + rfkill_force_state(local->hw.rfkill,
> + RFKILL_STATE_UNBLOCKED);
> + }
> + return 0;
> }
>
> if (need_reconfig) {

Ivo

2008-09-26 23:25:44

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: notify mac80211 about rfkill events

>> > That's definitely other option we wanted to suggest that mac80211
>> > would register itself to rfkill subsystem and will provide to driver
>> > appropriate callbacks. The question is how drivers vary in the rfkil
>> > implementation and whether it wouldn't be more complex, in that case
>> > the notification is quite clean solution.
>>
>> How complex does it have to be?

> Not really, probably it only needs to be a wrapper around rfkill_force_state
> where we can use the RFKILL_ defines to indicate the BLOCKED status.

Implementation wise it's more complicated since you have to teach each
driver under mac80211, this mean more
code and more bugs but I'm not sure this is the objective on this list
:) As opposite to just notifying mac about a rfkill
event so it's not trying to configure to access device, etc.

>> > > That means that the only change needed in ieee80211_ioctl_siwtxpower() is
>> > > only allowing the enabling of the radio when RFKILL is not set to BLOCKED.
>> >
>> > That's just complicates everything and moving the policy decisions to
>> > the driver after all even
>> > form txpower off you implement it as soft rfkill.
>
> Why does it complicate things? It means that mac80211 doesn't use
> rfkill_force_state() when the user triggers a radio state change using
> iw/iwconfig/ifconfig or whatever userspace tool.

I'm not sure I understand your intention, how do you plane to kill
radio then in this case, you are still to obliged to do so.

> mac80211 doesn't generate rfkill events because it isn't tied to any device
> keys/buttons/sliders. That is what the drivers do. And they should only listen
> to that key/button/slider.

writing 0 to sysfs file is not a button either it sill should soft
kill the radio. The sw switch is here to enable
killing and enabling radio from software. All the buttons stuff should
go under hard switches.


>
> Come to think of it, there is a bug in the previous patch since it doesn't handle
> the case when the interface is brought up during a BLOCKED state.
>

what is the p previous patch? If you mean the one I've sent, then
it's known to be incomplete, it was produced to invoke this
conversation.

> So all that has to be done is:
> * rfkill BLOCK event received in mac80211
> * set flag to indicate the BLOCKED state
> * disable radio

In case of notification chain you don't disable radio from mac80211

> * prevent radio being enabled through ifup
> * prevent radio being enabled through iwconfig txpower on
>
> * rfkill UNBLOCK event received from mac80211
> * clear flag to indicate UNBLOCKED state
> * restore radio to the by iwconfig/ifup configured state
Same here.
Tomas

2008-09-26 14:40:39

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH RFC] mac80211: notify mac80211 about rfkill events

On Fri, 2008-09-26 at 13:01 +0200, Johannes Berg wrote:
> On Fri, 2008-09-26 at 01:05 +0300, Tomas Winkler wrote:
>
> > > I am not sure if registring a notifier would be the best solution,
> > > persionally I was thinking of implementing the rfkill structure into ieee80211_local
> > > and make it listen to events directly.
>
> I think I like this better.
>
> > That's definitely other option we wanted to suggest that mac80211
> > would register itself to rfkill subsystem and will provide to driver
> > appropriate callbacks. The question is how drivers vary in the rfkil
> > implementation and whether it wouldn't be more complex, in that case
> > the notification is quite clean solution.
>
> How complex does it have to be?
>
> > > That means that the only change needed in ieee80211_ioctl_siwtxpower() is
> > > only allowing the enabling of the radio when RFKILL is not set to BLOCKED.
> >
> > That's just complicates everything and moving the policy decisions to
> > the driver after all even
> > form txpower off you implement it as soft rfkill.
> >
> > I would suggest just remove the support for txpower off in mac80211
> > now when appropriate or sync it with soft block after all it coming
> > from user space as a software event.
>
> I think what we should do is in mac80211 simply synthesize the
> "radio_enabled" state that the config callback has from both rfkill and
> txpower off. Anything wrong with that?

That sounds about right.

Dan