* Setting TX power on a monitoring interface
@ 2017-11-20 16:34 Peter Große
2017-11-27 11:43 ` Johannes Berg
0 siblings, 1 reply; 9+ messages in thread
From: Peter Große @ 2017-11-20 16:34 UTC (permalink / raw)
To: linux-wireless
[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]
Hi everyone.
The iw tool allows to set TX power settings on network interfaces.
If I try to set the TX power level on a _monitor_ interface, I get
a kernel warning:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 2193 at net/mac80211/driver-ops.h:167
ieee80211_bss_info_change_notify+0x111/0x190 Modules linked in: uvcvideo
videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_core
rndis_host cdc_ether usbnet mii tp_smapi(O) thinkpad_ec(O) ohci_hcd vboxpci(O)
vboxnetadp(O) vboxnetflt(O) v boxdrv(O) x86_pkg_temp_thermal kvm_intel kvm
irqbypass iwldvm iwlwifi ehci_pci ehci_hcd tpm_tis tpm_tis_core tpm CPU: 0
PID: 2193 Comm: iw Tainted: G O 4.12.12-gentoo #2 task:
ffff880186fd5cc0 task.stack: ffffc90001b54000 RIP:
0010:ieee80211_bss_info_change_notify+0x111/0x190 RSP: 0018:ffffc90001b57a10
EFLAGS: 00010246 RAX: 0000000000000006 RBX: ffff8801052ce840 RCX:
0000000000000064 RDX: 00000000fffffffc RSI: 0000000000040000 RDI:
ffff8801052ce840 RBP: ffffc90001b57a38 R08: 0000000000000062 R09:
0000000000000000 R10: ffff8802144b5000 R11: ffff880049dc4614 R12:
0000000000040000 R13: 0000000000000064 R14: ffff8802105f0760 R15:
ffffc90001b57b48 FS: 00007f92644b4580(0000) GS:ffff88021e200000(0000)
knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f9263c109f0 CR3: 00000001df850000 CR4: 00000000000406f0
Call Trace:
ieee80211_recalc_txpower+0x33/0x40
ieee80211_set_tx_power+0x40/0x180
nl80211_set_wiphy+0x32e/0x950
Steps to reproduce:
iw dev wlan0 interface add mon0 type monitor
ip link set dev mon0 up
iw dev mon0 set txpower fixed 100
Is that a bug to be fixed?
What would be the correct way of fixing it? Maybe I can provide a patch.
Regards
Peter
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Setting TX power on a monitoring interface
2017-11-20 16:34 Setting TX power on a monitoring interface Peter Große
@ 2017-11-27 11:43 ` Johannes Berg
2017-11-27 15:07 ` Peter Große
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2017-11-27 11:43 UTC (permalink / raw)
To: Peter Große, linux-wireless
On Mon, 2017-11-20 at 17:34 +0100, Peter Große wrote:
> Hi everyone.
>
> The iw tool allows to set TX power settings on network interfaces.
>
> If I try to set the TX power level on a _monitor_ interface, I get
> a kernel warning:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 2193 at net/mac80211/driver-ops.h:167
> ieee80211_bss_info_change_notify+0x111/0x190 Modules linked in: uvcvideo
> videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_core
> rndis_host cdc_ether usbnet mii tp_smapi(O) thinkpad_ec(O) ohci_hcd vboxpci(O)
> vboxnetadp(O) vboxnetflt(O) v boxdrv(O) x86_pkg_temp_thermal kvm_intel kvm
> irqbypass iwldvm iwlwifi ehci_pci ehci_hcd tpm_tis tpm_tis_core tpm CPU: 0
> PID: 2193 Comm: iw Tainted: G O 4.12.12-gentoo #2 task:
> ffff880186fd5cc0 task.stack: ffffc90001b54000 RIP:
> 0010:ieee80211_bss_info_change_notify+0x111/0x190 RSP: 0018:ffffc90001b57a10
> EFLAGS: 00010246 RAX: 0000000000000006 RBX: ffff8801052ce840 RCX:
> 0000000000000064 RDX: 00000000fffffffc RSI: 0000000000040000 RDI:
> ffff8801052ce840 RBP: ffffc90001b57a38 R08: 0000000000000062 R09:
> 0000000000000000 R10: ffff8802144b5000 R11: ffff880049dc4614 R12:
> 0000000000040000 R13: 0000000000000064 R14: ffff8802105f0760 R15:
> ffffc90001b57b48 FS: 00007f92644b4580(0000) GS:ffff88021e200000(0000)
> knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f9263c109f0 CR3: 00000001df850000 CR4: 00000000000406f0
> Call Trace:
> ieee80211_recalc_txpower+0x33/0x40
> ieee80211_set_tx_power+0x40/0x180
> nl80211_set_wiphy+0x32e/0x950
>
> Steps to reproduce:
>
> iw dev wlan0 interface add mon0 type monitor
> ip link set dev mon0 up
> iw dev mon0 set txpower fixed 100
>
> Is that a bug to be fixed?
Yeah, it's a bug.
> What would be the correct way of fixing it? Maybe I can provide a patch.
That's a really good question :-)
I think if the driver has WANT_MONITOR_VIF, then we can pass that
through and let the driver sort it out.
But if not, we probably just have to reject the configuration?
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Setting TX power on a monitoring interface
2017-11-27 11:43 ` Johannes Berg
@ 2017-11-27 15:07 ` Peter Große
2017-12-01 14:49 ` Johannes Berg
0 siblings, 1 reply; 9+ messages in thread
From: Peter Große @ 2017-11-27 15:07 UTC (permalink / raw)
To: linux-wireless
[-- Attachment #1: Type: text/plain, Size: 763 bytes --]
On Mon, 27 Nov 2017 12:43:13 +0100
Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> > What would be the correct way of fixing it? Maybe I can provide a patch.
>
> That's a really good question :-)
>
> I think if the driver has WANT_MONITOR_VIF, then we can pass that
> through and let the driver sort it out.
>
> But if not, we probably just have to reject the configuration?
With passing through you mean calling bss_info_changed on the driver for the
monitor interface?
Are monitor interfaces allowed to exist when WANT_MONITOR_VIF is not set?
I ask, whether I would have to check
sdata->vif.type == NL80211_IFTYPE_MONITOR
_and_ also
ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)
?
Regards
Peter
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Setting TX power on a monitoring interface
2017-11-27 15:07 ` Peter Große
@ 2017-12-01 14:49 ` Johannes Berg
2017-12-01 17:34 ` Peter Große
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2017-12-01 14:49 UTC (permalink / raw)
To: Peter Große, linux-wireless
On Mon, 2017-11-27 at 16:07 +0100, Peter Große wrote:
> > I think if the driver has WANT_MONITOR_VIF, then we can pass that
> > through and let the driver sort it out.
> >
> > But if not, we probably just have to reject the configuration?
>
> With passing through you mean calling bss_info_changed on the driver for the
> monitor interface?
I meant to pass the &monitor_sdata.vif pointer instead of the real
monitor interface that's coming through cfg80211. The former is virtual
and has no netdev, but the diver is aware of it.
> Are monitor interfaces allowed to exist when WANT_MONITOR_VIF is not set?
Yes
> I ask, whether I would have to check
> sdata->vif.type == NL80211_IFTYPE_MONITOR
> _and_ also
> ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)
You can check that
local->monitor_sdata
exists, and use it if yes, and reject if no.
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Setting TX power on a monitoring interface
2017-12-01 14:49 ` Johannes Berg
@ 2017-12-01 17:34 ` Peter Große
2017-12-09 19:53 ` Johannes Berg
0 siblings, 1 reply; 9+ messages in thread
From: Peter Große @ 2017-12-01 17:34 UTC (permalink / raw)
To: linux-wireless
[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]
Hi Johannes,
On Fri, 01 Dec 2017 15:49:02 +0100
Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2017-11-27 at 16:07 +0100, Peter Große wrote:
>
> > > I think if the driver has WANT_MONITOR_VIF, then we can pass that
> > > through and let the driver sort it out.
> > >
> > > But if not, we probably just have to reject the configuration?
> >
> > With passing through you mean calling bss_info_changed on the driver for the
> > monitor interface?
>
> I meant to pass the &monitor_sdata.vif pointer instead of the real
> monitor interface that's coming through cfg80211. The former is virtual
> and has no netdev, but the diver is aware of it.
I'm not sure I get where to pass this to what. Do you mean in
drv_bss_info_changed or ieee80211_set_tx_power?
> You can check that
>
> local->monitor_sdata
>
> exists, and use it if yes, and reject if no.
Assuming you meant ieee80211_set_tx_power, then I'd have to check whether wdev
is a monitor interface and reject the configuration, if local->monitor_sdata
doesn't exist?
But in ieee80211_set_tx_power no vif pointers get handed around, so I'm
confused. Sorry.
Regards
Peter
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Setting TX power on a monitoring interface
2017-12-01 17:34 ` Peter Große
@ 2017-12-09 19:53 ` Johannes Berg
2017-12-13 17:29 ` [PATCH] mac80211: Fix setting TX power on monitor interfaces Peter Große
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2017-12-09 19:53 UTC (permalink / raw)
To: Peter Große, linux-wireless
Hi,
(side note - I might reply faster if you don't drop me from Cc, since
then it goes to my inbox in addition to my list folder)
> > I meant to pass the &monitor_sdata.vif pointer instead of the real
> > monitor interface that's coming through cfg80211. The former is virtual
> > and has no netdev, but the diver is aware of it.
>
> I'm not sure I get where to pass this to what. Do you mean in
> drv_bss_info_changed or ieee80211_set_tx_power?
Should be in ieee80211_set_tx_power, if the interface is you're getting
in (dev/sdata) is MONITOR. You need to do the right locking (or may
already have rtnl, check in nl80211.c) to get local->monitor_sdata, and
then reject the call if that is NULL or pass it through with that sdata
(or the vif from it)
> Assuming you meant ieee80211_set_tx_power, then I'd have to check whether wdev
> is a monitor interface and reject the configuration, if local->monitor_sdata
> doesn't exist?
Right.
> But in ieee80211_set_tx_power no vif pointers get handed around, so I'm
> confused. Sorry.
You have a wdev, that's equivalent. See the "if (wdev)" clause that
gets an sdata from the wdev, and then you have &sdata->vif as the vif.
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] mac80211: Fix setting TX power on monitor interfaces
2017-12-09 19:53 ` Johannes Berg
@ 2017-12-13 17:29 ` Peter Große
2017-12-19 9:53 ` Johannes Berg
0 siblings, 1 reply; 9+ messages in thread
From: Peter Große @ 2017-12-13 17:29 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
Instead of calling ieee80211_recalc_txpower on monitor interfaces
directly, call it using the virtual monitor interface, if one exists.
In case of a single monitor interface given, reject setting TX power,
if no virtual monitor interface exists.
That being checked, don't warn in ieee80211_bss_info_change_notify,
after setting TX power on a monitor interface.
Fixes warning:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 2193 at net/mac80211/driver-ops.h:167
ieee80211_bss_info_change_notify+0x111/0x190 Modules linked in: uvcvideo
videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_core
rndis_host cdc_ether usbnet mii tp_smapi(O) thinkpad_ec(O) ohci_hcd vboxpci(O)
vboxnetadp(O) vboxnetflt(O) v boxdrv(O) x86_pkg_temp_thermal kvm_intel kvm
irqbypass iwldvm iwlwifi ehci_pci ehci_hcd tpm_tis tpm_tis_core tpm CPU: 0
PID: 2193 Comm: iw Tainted: G O 4.12.12-gentoo #2 task:
ffff880186fd5cc0 task.stack: ffffc90001b54000 RIP:
0010:ieee80211_bss_info_change_notify+0x111/0x190 RSP: 0018:ffffc90001b57a10
EFLAGS: 00010246 RAX: 0000000000000006 RBX: ffff8801052ce840 RCX:
0000000000000064 RDX: 00000000fffffffc RSI: 0000000000040000 RDI:
ffff8801052ce840 RBP: ffffc90001b57a38 R08: 0000000000000062 R09:
0000000000000000 R10: ffff8802144b5000 R11: ffff880049dc4614 R12:
0000000000040000 R13: 0000000000000064 R14: ffff8802105f0760 R15:
ffffc90001b57b48 FS: 00007f92644b4580(0000) GS:ffff88021e200000(0000)
knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f9263c109f0 CR3: 00000001df850000 CR4: 00000000000406f0
Call Trace:
ieee80211_recalc_txpower+0x33/0x40
ieee80211_set_tx_power+0x40/0x180
nl80211_set_wiphy+0x32e/0x950
Reported-by: Peter Große <pegro@friiks.de>
Signed-off-by: Peter Große <pegro@friiks.de>
---
Since monitor_sdata->vif.type is also NL80211_IFTYPE_MONITOR,
the warning would still appear with the patch to cfg.c, so I excluded
that case from the WARN_ON_ONCE condition.
I hope that makes sense?!
It fixes the warning for me though.
Regards
Peter
net/mac80211/cfg.c | 28 +++++++++++++++++++++++++++-
net/mac80211/driver-ops.h | 3 ++-
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a354f1939e49..29874052e162 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2373,10 +2373,17 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
struct ieee80211_sub_if_data *sdata;
enum nl80211_tx_power_setting txp_type = type;
bool update_txp_type = false;
+ bool has_monitor = false;
if (wdev) {
sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
+ if (sdata->vif.type == NL80211_IFTYPE_MONITOR) {
+ sdata = rtnl_dereference(local->monitor_sdata);
+ if (!sdata)
+ return -EOPNOTSUPP;
+ }
+
switch (type) {
case NL80211_TX_POWER_AUTOMATIC:
sdata->user_power_level = IEEE80211_UNSET_POWER_LEVEL;
@@ -2415,15 +2422,34 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
mutex_lock(&local->iflist_mtx);
list_for_each_entry(sdata, &local->interfaces, list) {
+ if (sdata->vif.type == NL80211_IFTYPE_MONITOR) {
+ has_monitor = true;
+ continue;
+ }
sdata->user_power_level = local->user_power_level;
if (txp_type != sdata->vif.bss_conf.txpower_type)
update_txp_type = true;
sdata->vif.bss_conf.txpower_type = txp_type;
}
- list_for_each_entry(sdata, &local->interfaces, list)
+ list_for_each_entry(sdata, &local->interfaces, list) {
+ if (sdata->vif.type == NL80211_IFTYPE_MONITOR)
+ continue;
ieee80211_recalc_txpower(sdata, update_txp_type);
+ }
mutex_unlock(&local->iflist_mtx);
+ if (has_monitor) {
+ sdata = rtnl_dereference(local->monitor_sdata);
+ if (sdata) {
+ sdata->user_power_level = local->user_power_level;
+ if (txp_type != sdata->vif.bss_conf.txpower_type)
+ update_txp_type = true;
+ sdata->vif.bss_conf.txpower_type = txp_type;
+
+ ieee80211_recalc_txpower(sdata, update_txp_type);
+ }
+ }
+
return 0;
}
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 09f77e4a8a79..49c8a9c9b91f 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -164,7 +164,8 @@ static inline void drv_bss_info_changed(struct ieee80211_local *local,
if (WARN_ON_ONCE(sdata->vif.type == NL80211_IFTYPE_P2P_DEVICE ||
sdata->vif.type == NL80211_IFTYPE_NAN ||
(sdata->vif.type == NL80211_IFTYPE_MONITOR &&
- !sdata->vif.mu_mimo_owner)))
+ !sdata->vif.mu_mimo_owner &&
+ !(changed & BSS_CHANGED_TXPOWER))))
return;
if (!check_sdata_in_driver(sdata))
--
2.13.6
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mac80211: Fix setting TX power on monitor interfaces
2017-12-13 17:29 ` [PATCH] mac80211: Fix setting TX power on monitor interfaces Peter Große
@ 2017-12-19 9:53 ` Johannes Berg
2017-12-19 19:25 ` Peter Große
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2017-12-19 9:53 UTC (permalink / raw)
To: Peter Große, linux-wireless
Hi,
> Since monitor_sdata->vif.type is also NL80211_IFTYPE_MONITOR,
> the warning would still appear with the patch to cfg.c, so I excluded
> that case from the WARN_ON_ONCE condition.
>
> I hope that makes sense?!
Yeah, looks good.
> if (wdev) {
> sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
>
> + if (sdata->vif.type == NL80211_IFTYPE_MONITOR) {
> + sdata = rtnl_dereference(local->monitor_sdata);
> + if (!sdata)
> + return -EOPNOTSUPP;
> + }
This part makes perfect sense.
> mutex_lock(&local->iflist_mtx);
> list_for_each_entry(sdata, &local->interfaces, list) {
> + if (sdata->vif.type == NL80211_IFTYPE_MONITOR) {
> + has_monitor = true;
> + continue;
> + }
> sdata->user_power_level = local->user_power_level;
> if (txp_type != sdata->vif.bss_conf.txpower_type)
> update_txp_type = true;
> sdata->vif.bss_conf.txpower_type = txp_type;
> }
> - list_for_each_entry(sdata, &local->interfaces, list)
> + list_for_each_entry(sdata, &local->interfaces, list) {
> + if (sdata->vif.type == NL80211_IFTYPE_MONITOR)
> + continue;
> ieee80211_recalc_txpower(sdata, update_txp_type);
> + }
> mutex_unlock(&local->iflist_mtx);
>
> + if (has_monitor) {
> + sdata = rtnl_dereference(local->monitor_sdata);
> + if (sdata) {
> + sdata->user_power_level = local->user_power_level;
> + if (txp_type != sdata->vif.bss_conf.txpower_type)
> + update_txp_type = true;
> + sdata->vif.bss_conf.txpower_type = txp_type;
> +
> + ieee80211_recalc_txpower(sdata, update_txp_type);
> + }
> + }
But do we really need this? I think we can probably live with just not
having monitor handled here, i.e. only have the "if monitor continue;"
part?
johannes
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mac80211: Fix setting TX power on monitor interfaces
2017-12-19 9:53 ` Johannes Berg
@ 2017-12-19 19:25 ` Peter Große
0 siblings, 0 replies; 9+ messages in thread
From: Peter Große @ 2017-12-19 19:25 UTC (permalink / raw)
To: Johannes Berg; +Cc: Peter Große, linux-wireless
[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]
On Tue, 19 Dec 2017 10:53:30 +0100
Johannes Berg <johannes@sipsolutions.net> wrote:
> > + if (has_monitor) {
> > + sdata = rtnl_dereference(local->monitor_sdata);
> > + if (sdata) {
> > + sdata->user_power_level = local->user_power_level;
> > + if (txp_type != sdata->vif.bss_conf.txpower_type)
> > + update_txp_type = true;
> > + sdata->vif.bss_conf.txpower_type = txp_type;
> > +
> > + ieee80211_recalc_txpower(sdata, update_txp_type);
> > + }
> > + }
>
> But do we really need this? I think we can probably live with just not
> having monitor handled here, i.e. only have the "if monitor continue;"
> part?
I don't mind. I just thought, if I call ieee80211_recalc_txpower on the
monitor_sdata interface when called explicitly, I would also do it when
iterating over all interfaces, just to be consistent. I don't know, whether
there are drivers out there, that store power limits on monitor interfaces
separately for injection or whatever.
I can send a patch without the quoted part.
Regards
Peter
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-12-19 19:25 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-20 16:34 Setting TX power on a monitoring interface Peter Große
2017-11-27 11:43 ` Johannes Berg
2017-11-27 15:07 ` Peter Große
2017-12-01 14:49 ` Johannes Berg
2017-12-01 17:34 ` Peter Große
2017-12-09 19:53 ` Johannes Berg
2017-12-13 17:29 ` [PATCH] mac80211: Fix setting TX power on monitor interfaces Peter Große
2017-12-19 9:53 ` Johannes Berg
2017-12-19 19:25 ` Peter Große
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).