2014-11-28 05:59:43

by Avinash Patil

[permalink] [raw]
Subject: [PATCH] cfg80211: do not check for carrier during start_radar_detect

Radar detection happens before starting AP; so netdev carrier may be dormant
at this time. Remove this check from cfg80211_start_radar_detection.

Signed-off-by: Avinash Patil <[email protected]>
---
net/wireless/nl80211.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 6e41777..25f68d6 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6132,9 +6132,6 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
if (err)
return err;

- if (netif_carrier_ok(dev))
- return -EBUSY;
-
if (wdev->cac_started)
return -EBUSY;

--
1.8.1.4



2014-11-28 15:11:35

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

On Fri, 2014-11-28 at 16:58 +0530, Avinash Patil wrote:
> Radar detection happens before starting AP; so netdev carrier may be dormant
> at this time. Remove this check from cfg80211_start_radar_detection.

Looks fine to me, but I wonder how it ever worked?

johannes


2014-11-28 16:36:07

by Avinash Patil

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

WWVzOyBiZWF0cyBtZSBhcyB3ZWxsLgoKVGhhdCdzIHdoeSBJIGFkZGVkIG9yaWdpbmFsIGF1dGhv
cnMgLSBTaW1vbiBhbmQgSmFudXN6LgoKLUF2aW5hc2gKCk9uIE5vdiAyOCwgMjAxNCA4OjQxIFBN
LCBKb2hhbm5lcyBCZXJnIDxqb2hhbm5lc0BzaXBzb2x1dGlvbnMubmV0PiB3cm90ZToKPgo+IE9u
IEZyaSwgMjAxNC0xMS0yOCBhdCAxNjo1OCArMDUzMCwgQXZpbmFzaCBQYXRpbCB3cm90ZToKPiA+
IFJhZGFyIGRldGVjdGlvbiBoYXBwZW5zIGJlZm9yZSBzdGFydGluZyBBUDsgc28gbmV0ZGV2IGNh
cnJpZXIgbWF5IGJlIGRvcm1hbnQKPiA+IGF0IHRoaXMgdGltZS4gUmVtb3ZlIHRoaXMgY2hlY2sg
ZnJvbSBjZmc4MDIxMV9zdGFydF9yYWRhcl9kZXRlY3Rpb24uCj4KPiBMb29rcyBmaW5lIHRvIG1l
LCBidXQgSSB3b25kZXIgaG93IGl0IGV2ZXIgd29ya2VkPwo+Cj4gam9oYW5uZXMKPgo+IC0tCj4g
VG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJl
IGxpbnV4LXdpcmVsZXNzIiBpbgo+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9A
dmdlci5rZXJuZWwub3JnCj4gTW9yZSBtYWpvcmRvbW8gaW5mbyBhdMKgIGh0dHA6Ly92Z2VyLmtl
cm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbAo=

2014-12-01 07:13:36

by Janusz Dziedzic

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

On 28 November 2014 at 17:35, Avinash Patil <[email protected]> wrote:
> Yes; beats me as well.
>
> That's why I added original authors - Simon and Janusz.
>
> -Avinash
>
> On Nov 28, 2014 8:41 PM, Johannes Berg <[email protected]> wrote:
>>
>> On Fri, 2014-11-28 at 16:58 +0530, Avinash Patil wrote:
>> > Radar detection happens before starting AP; so netdev carrier may be dormant
>> > at this time. Remove this check from cfg80211_start_radar_detection.
>>
>> Looks fine to me, but I wonder how it ever worked?
>>
radar detection starts before we setup AP, so woks fine

Seems this patch unlock off-channel CAC (we are connected/ap started
and detect radar on other channel), but seem mac80211 don't support
this correctly now.
So, at least we should move if (netif_carrier_ok(dev)) check to
mac80211 otherwise we might change channel of the running interface
(via vif_use_channel).

BR
Janusz

>> johannes
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-12-23 11:33:36

by Avinash Patil

[permalink] [raw]
Subject: RE: [PATCH] cfg80211: do not check for carrier during start_radar_detect

Hi Johannes,

How about having a flag for offchannel CAC capability and check for netdev carrier state only when this capability is supported?

Thanks,
Avinash
________________________________________
From: Johannes Berg [[email protected]]
Sent: Tuesday, December 23, 2014 4:07 PM
To: Avinash Patil
Cc: [email protected]; [email protected]; Cathy Luo; Amitkumar Karwar
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

On Tue, 2014-12-23 at 11:36 +0100, Johannes Berg wrote:
> On Tue, 2014-12-23 at 02:30 -0800, Avinash Patil wrote:
> > Oops..
> >
> > Are you talking about this?
> > >>So, at least we should move if (netif_carrier_ok(dev)) check to mac80211 otherwise we might change channel of the running interface (via vif_use_channel).
> >
> > I can move this check to ieee80211_start_radar_detection but I dont have any hardware to test these modifications.
>
> That should be ok - however maybe you don't need those modifications at
> all?
>
> Frankly I'm not even sure how off-channel CAC stuff would work but
> Janusz probably knows what he's talking about :)

Also - maybe we need some sort of capability flag for this, and then the
check doesn't have to move but just be made conditional on not having
the feature?

johannes

2014-12-23 10:37:18

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

On Tue, 2014-12-23 at 11:36 +0100, Johannes Berg wrote:
> On Tue, 2014-12-23 at 02:30 -0800, Avinash Patil wrote:
> > Oops..
> >
> > Are you talking about this?
> > >>So, at least we should move if (netif_carrier_ok(dev)) check to mac80211 otherwise we might change channel of the running interface (via vif_use_channel).
> >
> > I can move this check to ieee80211_start_radar_detection but I dont have any hardware to test these modifications.
>
> That should be ok - however maybe you don't need those modifications at
> all?
>
> Frankly I'm not even sure how off-channel CAC stuff would work but
> Janusz probably knows what he's talking about :)

Also - maybe we need some sort of capability flag for this, and then the
check doesn't have to move but just be made conditional on not having
the feature?

johannes


2014-12-23 10:36:39

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

On Tue, 2014-12-23 at 02:30 -0800, Avinash Patil wrote:
> Oops..
>
> Are you talking about this?
> >>So, at least we should move if (netif_carrier_ok(dev)) check to mac80211 otherwise we might change channel of the running interface (via vif_use_channel).
>
> I can move this check to ieee80211_start_radar_detection but I dont have any hardware to test these modifications.

That should be ok - however maybe you don't need those modifications at
all?

Frankly I'm not even sure how off-channel CAC stuff would work but
Janusz probably knows what he's talking about :)

johannes


2014-12-23 10:33:01

by Avinash Patil

[permalink] [raw]
Subject: RE: [PATCH] cfg80211: do not check for carrier during start_radar_detect

Oops..

Are you talking about this?
>>So, at least we should move if (netif_carrier_ok(dev)) check to mac80211 otherwise we might change channel of the running interface (via vif_use_channel).

I can move this check to ieee80211_start_radar_detection but I dont have any hardware to test these modifications.

Thanks,
Avinash
________________________________________
From: Johannes Berg [[email protected]]
Sent: Tuesday, December 23, 2014 3:56 PM
To: Avinash Patil
Cc: [email protected]; [email protected]; Cathy Luo; Amitkumar Karwar
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

On Tue, 2014-12-23 at 02:13 -0800, Avinash Patil wrote:
> Hi Janusz,
>
> Any further update on this?

We were waiting for you to send a new patch?

johannes


2014-12-23 10:26:37

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

On Tue, 2014-12-23 at 02:13 -0800, Avinash Patil wrote:
> Hi Janusz,
>
> Any further update on this?

We were waiting for you to send a new patch?

johannes



2014-12-23 10:17:35

by Avinash Patil

[permalink] [raw]
Subject: RE: [PATCH] cfg80211: do not check for carrier during start_radar_detect

Hi Janusz,

Any further update on this?

Thanks,
Avinash

-----Original Message-----
From: Janusz Dziedzic [mailto:[email protected]]
Sent: 01 December 2014 12:44
To: Avinash Patil
Cc: Johannes Berg; Simon Wunderlich; Amitkumar Karwar; [email protected]; Cathy Luo; John W. Linville
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

On 28 November 2014 at 17:35, Avinash Patil <[email protected]> wrote:
> Yes; beats me as well.
>
> That's why I added original authors - Simon and Janusz.
>
> -Avinash
>
> On Nov 28, 2014 8:41 PM, Johannes Berg <[email protected]> wrote:
>>
>> On Fri, 2014-11-28 at 16:58 +0530, Avinash Patil wrote:
>> > Radar detection happens before starting AP; so netdev carrier may
>> > be dormant at this time. Remove this check from cfg80211_start_radar_detection.
>>
>> Looks fine to me, but I wonder how it ever worked?
>>
radar detection starts before we setup AP, so woks fine

Seems this patch unlock off-channel CAC (we are connected/ap started and detect radar on other channel), but seem mac80211 don't support this correctly now.
So, at least we should move if (netif_carrier_ok(dev)) check to
mac80211 otherwise we might change channel of the running interface (via vif_use_channel).

BR
Janusz

>> johannes
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-wireless" in the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-12-23 12:01:30

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

On Tue, 2014-12-23 at 03:32 -0800, Avinash Patil wrote:
> Hi Johannes,
>
> How about having a flag for offchannel CAC capability and check for
> netdev carrier state only when this capability is supported?

Yes, that seems appropriate.

(Btw, please don't top-post)

johannes


2015-01-07 12:55:16

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: do not check for carrier during start_radar_detect

On Mon, 2014-12-01 at 08:13 +0100, Janusz Dziedzic wrote:

> Seems this patch unlock off-channel CAC (we are connected/ap started
> and detect radar on other channel), but seem mac80211 don't support
> this correctly now.

Come to think of it - I don't believe there's any way to do off-channel
CAC properly since you'd be out of your operating channel for like a
minute - might as well tear down the AP then.

johannes