[PATCH] cfg80211: check for carrier state only when offchanel CAC supported

[PATCH] cfg80211: check for carrier state only when offchanel CAC supported

[Avinash Patil]

Checking for carrier state during start_radar_detection is needed
only for devices which support offchannel CAC.
This patch provides this additional check of extended feature offchannel
CAC support while checking for carrier state.

Signed-off-by: Avinash Patil <patila@marvell.com>
---
 include/uapi/linux/nl80211.h | 3 +++
 net/wireless/nl80211.c       | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 735ab43..9ac9008 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4205,11 +4205,14 @@ enum nl80211_feature_flags {
 /**
  * enum nl80211_ext_feature_index - bit index of extended features.
  *
+ * @NL80211_EXT_FEATURE_OFFCHAN_CAC: This device/driver supports
+ * offchannel Channel Availibility Check(CAC).
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
  */
 enum nl80211_ext_feature_index {
 
+	NL80211_EXT_FEATURE_OFFCHAN_CAC,
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
 	MAX_NL80211_EXT_FEATURES = NUM_NL80211_EXT_FEATURES - 1
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 39753de..b2abb37 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6138,7 +6138,9 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
 	if (err)
 		return err;
 
-	if (netif_carrier_ok(dev))
+	if (wiphy_ext_feature_isset(&rdev->wiphy,
+				    NL80211_EXT_FEATURE_OFFCHAN_CAC) &&
+	    netif_carrier_ok(dev))
 		return -EBUSY;
 
 	if (wdev->cac_started)
-- 
1.8.1.4

Re: [PATCH] cfg80211: check for carrier state only when offchanel CAC supported

[Arend van Spriel]

On 12/24/14 13:47, Avinash Patil wrote:
> Checking for carrier state during start_radar_detection is needed
> only for devices which support offchannel CAC.
> This patch provides this additional check of extended feature offchannel
> CAC support while checking for carrier state.
>
> Signed-off-by: Avinash Patil<patila@marvell.com>
> ---
>   include/uapi/linux/nl80211.h | 3 +++
>   net/wireless/nl80211.c       | 4 +++-
>   2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 735ab43..9ac9008 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -4205,11 +4205,14 @@ enum nl80211_feature_flags {
>   /**
>    * enum nl80211_ext_feature_index - bit index of extended features.
>    *
> + * @NL80211_EXT_FEATURE_OFFCHAN_CAC: This device/driver supports
> + * offchannel Channel Availibility Check(CAC).

continuation on next line should be indented with a tab.

Regards,
Arend

>    * @NUM_NL80211_EXT_FEATURES: number of extended features.
>    * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
>    */
>   enum nl80211_ext_feature_index {
>
> +	NL80211_EXT_FEATURE_OFFCHAN_CAC,
>   	/* add new features before the definition below */
>   	NUM_NL80211_EXT_FEATURES,
>   	MAX_NL80211_EXT_FEATURES = NUM_NL80211_EXT_FEATURES - 1
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 39753de..b2abb37 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -6138,7 +6138,9 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
>   	if (err)
>   		return err;
>
> -	if (netif_carrier_ok(dev))
> +	if (wiphy_ext_feature_isset(&rdev->wiphy,
> +				    NL80211_EXT_FEATURE_OFFCHAN_CAC)&&
> +	    netif_carrier_ok(dev))
>   		return -EBUSY;
>
>   	if (wdev->cac_started)

Re: [PATCH] cfg80211: check for carrier state only when offchanel CAC supported

[Johannes Berg]

On Wed, 2014-12-24 at 04:47 -0800, Avinash Patil wrote:

> + * @NL80211_EXT_FEATURE_OFFCHAN_CAC: This device/driver supports
> + * offchannel Channel Availibility Check(CAC).

as Arend said, please indent (with a tab)

You're also mis-spelling "availability" :)

>   * @NUM_NL80211_EXT_FEATURES: number of extended features.
>   * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
>   */
>  enum nl80211_ext_feature_index {
>  
> +	NL80211_EXT_FEATURE_OFFCHAN_CAC,

Better add it before rather than after the blank line.

johannes

RE: [PATCH] cfg80211: check for carrier state only when offchanel CAC supported

[Avinash Patil]

________________________________________
From: Johannes Berg [johannes@sipsolutions.net]
Sent: Monday, January 05, 2015 2:51 PM
To: Avinash Patil
Cc: linux-wireless@vger.kernel.org; Amitkumar Karwar; Cathy Luo; janusz.dziedzic@tieto.com
Subject: Re: [PATCH] cfg80211: check for carrier state only when offchanel CAC supported

On Wed, 2014-12-24 at 04:47 -0800, Avinash Patil wrote:

> + * @NL80211_EXT_FEATURE_OFFCHAN_CAC: This device/driver supports
> + * offchannel Channel Availibility Check(CAC).

>as Arend said, please indent (with a tab)

>You're also mis-spelling "availability" :)

>   * @NUM_NL80211_EXT_FEATURES: number of extended features.
>   * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
>   */
>  enum nl80211_ext_feature_index {
>
> +     NL80211_EXT_FEATURE_OFFCHAN_CAC,

>Better add it before rather than after the blank line.

>johannes

Thanks for review, Johannes.
Will send v3 with your comments addressed.

Thanks,
Avinash.

[PATCH] cfg80211: check for carrier state only when offchanel CAC supported

[Avinash Patil]

Checking for carrier state during start_radar_detection is needed
only for devices which support offchannel CAC.
This patch adds one more condition while checking for carrier state
to see if offchanel CAC is supported, else we need not check carrier
state.

Signed-off-by: Avinash Patil <patila@marvell.com>
---
 include/net/cfg80211.h | 2 ++
 net/wireless/nl80211.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 4ebb816..1b80302 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2740,6 +2740,7 @@ struct cfg80211_ops {
  * @WIPHY_FLAG_SUPPORTS_5_10_MHZ: Device supports 5 MHz and 10 MHz channels.
  * @WIPHY_FLAG_HAS_CHANNEL_SWITCH: Device supports channel switch in
  *	beaconing mode (AP, IBSS, Mesh, ...).
+ * @WIPHY_FLAG_OFFCHAN_CAC: Device supports offchannel CAC.
  */
 enum wiphy_flags {
 	/* use hole at 0 */
@@ -2765,6 +2766,7 @@ enum wiphy_flags {
 	WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL	= BIT(21),
 	WIPHY_FLAG_SUPPORTS_5_10_MHZ		= BIT(22),
 	WIPHY_FLAG_HAS_CHANNEL_SWITCH		= BIT(23),
+	WIPHY_FLAG_OFFCHAN_CAC			= BIT(24),
 };
 
 /**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index a17d6bc..40290a5 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6133,7 +6133,8 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
 	if (err)
 		return err;
 
-	if (netif_carrier_ok(dev))
+	if ((rdev->wiphy.flags & WIPHY_FLAG_OFFCHAN_CAC) &&
+	    netif_carrier_ok(dev))
 		return -EBUSY;
 
 	if (wdev->cac_started)
-- 
1.8.1.4

Re: [PATCH] cfg80211: check for carrier state only when offchanel CAC supported

[Johannes Berg]

On Wed, 2014-12-24 at 01:35 +0530, Avinash Patil wrote:
> Checking for carrier state during start_radar_detection is needed
> only for devices which support offchannel CAC.
> This patch adds one more condition while checking for carrier state
> to see if offchanel CAC is supported, else we need not check carrier
> state.
> 
> Signed-off-by: Avinash Patil <patila@marvell.com>
> ---
>  include/net/cfg80211.h | 2 ++
>  net/wireless/nl80211.c | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 4ebb816..1b80302 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -2740,6 +2740,7 @@ struct cfg80211_ops {
>   * @WIPHY_FLAG_SUPPORTS_5_10_MHZ: Device supports 5 MHz and 10 MHz channels.
>   * @WIPHY_FLAG_HAS_CHANNEL_SWITCH: Device supports channel switch in
>   *	beaconing mode (AP, IBSS, Mesh, ...).
> + * @WIPHY_FLAG_OFFCHAN_CAC: Device supports offchannel CAC.

I think this should be an nl80211 feature flag, so that userspace can
also know whether or not this is supported.

However, we've run out of nl80211 feature flags. Arend is working on
adding more, see here:
http://mid.gmane.org/1418935344-22159-1-git-send-email-arend@broadcom.com
(ignore the 4-way-HS offload bit he's defining)

I'm not sure how to proceed here - in a sense your feature flag is far
less controversial and work-intensive than Arend's (who needs a lot more
than just that patch) so I guess you could pick up his patch, but I
expect him to also have fixed already. He's probably also on vacation
for about the same time though...

johannes

Re: [PATCH] cfg80211: check for carrier state only when offchanel CAC supported

[Arend van Spriel]

On 12/23/14 15:53, Johannes Berg wrote:
> On Wed, 2014-12-24 at 01:35 +0530, Avinash Patil wrote:
>> Checking for carrier state during start_radar_detection is needed
>> only for devices which support offchannel CAC.
>> This patch adds one more condition while checking for carrier state
>> to see if offchanel CAC is supported, else we need not check carrier
>> state.
>>
>> Signed-off-by: Avinash Patil<patila@marvell.com>
>> ---
>>   include/net/cfg80211.h | 2 ++
>>   net/wireless/nl80211.c | 3 ++-
>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index 4ebb816..1b80302 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -2740,6 +2740,7 @@ struct cfg80211_ops {
>>    * @WIPHY_FLAG_SUPPORTS_5_10_MHZ: Device supports 5 MHz and 10 MHz channels.
>>    * @WIPHY_FLAG_HAS_CHANNEL_SWITCH: Device supports channel switch in
>>    *	beaconing mode (AP, IBSS, Mesh, ...).
>> + * @WIPHY_FLAG_OFFCHAN_CAC: Device supports offchannel CAC.
>
> I think this should be an nl80211 feature flag, so that userspace can
> also know whether or not this is supported.
>
> However, we've run out of nl80211 feature flags. Arend is working on
> adding more, see here:
> http://mid.gmane.org/1418935344-22159-1-git-send-email-arend@broadcom.com
> (ignore the 4-way-HS offload bit he's defining)
>
> I'm not sure how to proceed here - in a sense your feature flag is far
> less controversial and work-intensive than Arend's (who needs a lot more
> than just that patch) so I guess you could pick up his patch, but I
> expect him to also have fixed already. He's probably also on vacation
> for about the same time though...

Not on vacation just yet. I indeed reworked the patch, but I was waiting 
for wpa_supplicant patches so I can verify it works with brcmfmac. I can 
submit the reworked feature flag patch with empty enum if that would work.

Regards,
Arend

> 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: check for carrier state only when offchanel CAC supported

[Johannes Berg]

On Tue, 2014-12-23 at 16:34 +0100, Arend van Spriel wrote:
> On 12/23/14 15:53, Johannes Berg wrote:
> > On Wed, 2014-12-24 at 01:35 +0530, Avinash Patil wrote:
> >> Checking for carrier state during start_radar_detection is needed
> >> only for devices which support offchannel CAC.
> >> This patch adds one more condition while checking for carrier state
> >> to see if offchanel CAC is supported, else we need not check carrier
> >> state.
> >>
> >> Signed-off-by: Avinash Patil<patila@marvell.com>
> >> ---
> >>   include/net/cfg80211.h | 2 ++
> >>   net/wireless/nl80211.c | 3 ++-
> >>   2 files changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> >> index 4ebb816..1b80302 100644
> >> --- a/include/net/cfg80211.h
> >> +++ b/include/net/cfg80211.h
> >> @@ -2740,6 +2740,7 @@ struct cfg80211_ops {
> >>    * @WIPHY_FLAG_SUPPORTS_5_10_MHZ: Device supports 5 MHz and 10 MHz channels.
> >>    * @WIPHY_FLAG_HAS_CHANNEL_SWITCH: Device supports channel switch in
> >>    *	beaconing mode (AP, IBSS, Mesh, ...).
> >> + * @WIPHY_FLAG_OFFCHAN_CAC: Device supports offchannel CAC.
> >
> > I think this should be an nl80211 feature flag, so that userspace can
> > also know whether or not this is supported.
> >
> > However, we've run out of nl80211 feature flags. Arend is working on
> > adding more, see here:
> > http://mid.gmane.org/1418935344-22159-1-git-send-email-arend@broadcom.com
> > (ignore the 4-way-HS offload bit he's defining)
> >
> > I'm not sure how to proceed here - in a sense your feature flag is far
> > less controversial and work-intensive than Arend's (who needs a lot more
> > than just that patch) so I guess you could pick up his patch, but I
> > expect him to also have fixed already. He's probably also on vacation
> > for about the same time though...
> 
> Not on vacation just yet. I indeed reworked the patch, but I was waiting 
> for wpa_supplicant patches so I can verify it works with brcmfmac. I can 
> submit the reworked feature flag patch with empty enum if that would work.

Maybe if you were to do that Avinash could resubmit it again with the
feature flag they need.

johannes

Re: [PATCH] cfg80211: check for carrier state only when offchanel CAC supported

[Arend van Spriel]

On 12/23/14 16:38, Johannes Berg wrote:
> On Tue, 2014-12-23 at 16:34 +0100, Arend van Spriel wrote:
>> On 12/23/14 15:53, Johannes Berg wrote:
>>> On Wed, 2014-12-24 at 01:35 +0530, Avinash Patil wrote:
>>>> Checking for carrier state during start_radar_detection is needed
>>>> only for devices which support offchannel CAC.
>>>> This patch adds one more condition while checking for carrier state
>>>> to see if offchanel CAC is supported, else we need not check carrier
>>>> state.
>>>>
>>>> Signed-off-by: Avinash Patil<patila@marvell.com>
>>>> ---
>>>>    include/net/cfg80211.h | 2 ++
>>>>    net/wireless/nl80211.c | 3 ++-
>>>>    2 files changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>>>> index 4ebb816..1b80302 100644
>>>> --- a/include/net/cfg80211.h
>>>> +++ b/include/net/cfg80211.h
>>>> @@ -2740,6 +2740,7 @@ struct cfg80211_ops {
>>>>     * @WIPHY_FLAG_SUPPORTS_5_10_MHZ: Device supports 5 MHz and 10 MHz channels.
>>>>     * @WIPHY_FLAG_HAS_CHANNEL_SWITCH: Device supports channel switch in
>>>>     *	beaconing mode (AP, IBSS, Mesh, ...).
>>>> + * @WIPHY_FLAG_OFFCHAN_CAC: Device supports offchannel CAC.
>>>
>>> I think this should be an nl80211 feature flag, so that userspace can
>>> also know whether or not this is supported.
>>>
>>> However, we've run out of nl80211 feature flags. Arend is working on
>>> adding more, see here:
>>> http://mid.gmane.org/1418935344-22159-1-git-send-email-arend@broadcom.com
>>> (ignore the 4-way-HS offload bit he's defining)
>>>
>>> I'm not sure how to proceed here - in a sense your feature flag is far
>>> less controversial and work-intensive than Arend's (who needs a lot more
>>> than just that patch) so I guess you could pick up his patch, but I
>>> expect him to also have fixed already. He's probably also on vacation
>>> for about the same time though...
>>
>> Not on vacation just yet. I indeed reworked the patch, but I was waiting
>> for wpa_supplicant patches so I can verify it works with brcmfmac. I can
>> submit the reworked feature flag patch with empty enum if that would work.
>
> Maybe if you were to do that Avinash could resubmit it again with the
> feature flag they need.

Fine by me. Will come out shortly.

Gr. AvS