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

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

[Avinash Patil]

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

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

[Janusz Dziedzic]

On 28 November 2014 at 17:35, Avinash Patil <patila@marvell.com> 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 <johannes@sipsolutions.net> 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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Avinash Patil]

Hi Janusz,

Any further update on this?

Thanks,
Avinash

-----Original Message-----
From: Janusz Dziedzic [mailto:janusz.dziedzic@tieto.com]
Sent: 01 December 2014 12:44
To: Avinash Patil
Cc: Johannes Berg; Simon Wunderlich; Amitkumar Karwar; linux-wireless@vger.kernel.org; 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 <patila@marvell.com> 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 <johannes@sipsolutions.net> 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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Berg]

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

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

[Avinash Patil]

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 [johannes@sipsolutions.net]
Sent: Tuesday, December 23, 2014 3:56 PM
To: Avinash Patil
Cc: janusz.dziedzic@tieto.com; linux-wireless@vger.kernel.org; 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

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

[Johannes Berg]

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

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

[Johannes Berg]

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

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

[Avinash Patil]

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 [johannes@sipsolutions.net]
Sent: Tuesday, December 23, 2014 4:07 PM
To: Avinash Patil
Cc: janusz.dziedzic@tieto.com; linux-wireless@vger.kernel.org; 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

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

[Johannes Berg]

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

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

[Johannes Berg]

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

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

[Avinash Patil]

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 <patila@marvell.com>
---
 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

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

[Johannes Berg]

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