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 <[email protected]>
---
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..c318802 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4205,10 +4205,13 @@ 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 Availability 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,
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
On Mon, 2015-01-05 at 22:03 +0530, 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 <[email protected]>
> ---
> 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..c318802 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -4205,10 +4205,13 @@ 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 Availability 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,
> 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;
Wait - doesn't that have to be !feature_isset()?
johannes
On Mon, 2015-01-05 at 05:28 -0800, Avinash Patil wrote:
> > - if (netif_carrier_ok(dev))
> > + if (wiphy_ext_feature_isset(&rdev->wiphy,
> > + NL80211_EXT_FEATURE_OFFCHAN_CAC) &&
> > + netif_carrier_ok(dev))
> > return -EBUSY;
>
> >Wait - doesn't that have to be !feature_isset()?
>
> >johannes
>
> If Offchannel CAC is supported (driver has set this bit in wiphy's
> extended features) & carrier is ON, return EBUSY as offchannel CAC may
> be ongoing, isnt it?
Well, my thinking is this - a new feature flag should allow something
new.
Therefore, the patch should essentially be this:
+ if (!new_feature)
if (do_old_check)
Now wrapping that into a single if gives
- if (do_old_check)
+ if (!new_feature && do_old_check)
so the patch looks wrong to me.
johannes
Based on our discussion, you don't actually want to support something
like off-channel CAC (which doesn't make much sense anyway), you just
have some issues with the carrier handling in the driver and bring up
the carrier ON by default rather than OFF.
I'll drop this completely for now.
johannes
________________________________________
From: [email protected] [[email protected]] On Behalf Of Johannes Berg [[email protected]]
Sent: Monday, January 05, 2015 7:17 PM
To: Avinash Patil
Cc: [email protected]; Amitkumar Karwar; Cathy Luo
Subject: Re: [PATCH v3] cfg80211: check for carrier state only when offchanel CAC supported
On Mon, 2015-01-05 at 05:28 -0800, Avinash Patil wrote:
> > - if (netif_carrier_ok(dev))
> > + if (wiphy_ext_feature_isset(&rdev->wiphy,
> > + NL80211_EXT_FEATURE_OFFCHAN_CAC) &&
> > + netif_carrier_ok(dev))
> > return -EBUSY;
>
> >Wait - doesn't that have to be !feature_isset()?
>
> >johannes
>
> If Offchannel CAC is supported (driver has set this bit in wiphy's
> extended features) & carrier is ON, return EBUSY as offchannel CAC may
> be ongoing, isnt it?
>Well, my thinking is this - a new feature flag should allow something
>new.
>Therefore, the patch should essentially be this:
>+ if (!new_feature)
>if (do_old_check)
>Now wrapping that into a single if gives
>- if (do_old_check)
>+ if (!new_feature && do_old_check)
>so the patch looks wrong to me.
I think here we want to do_old_check only when new_feature is supported; because old check is causing issue for device where new feature is not supported.
Earlier it was:
- if (do_old_check)
Now it is:
+if (new_feature && do_old_check)
I think check is still correct. As you suggested we can nest it...
Please correct me if I am wrong.
-Avinash.
________________________________________
From: Johannes Berg [[email protected]]
Sent: Monday, January 05, 2015 6:27 PM
To: Avinash Patil
Cc: [email protected]; Amitkumar Karwar; Cathy Luo
Subject: Re: [PATCH v3] cfg80211: check for carrier state only when offchanel CAC supported
On Mon, 2015-01-05 at 22:03 +0530, 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 <[email protected]>
> ---
> 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..c318802 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -4205,10 +4205,13 @@ 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 Availability 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,
> 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;
>Wait - doesn't that have to be !feature_isset()?
>johannes
If Offchannel CAC is supported (driver has set this bit in wiphy's extended features) & carrier is ON, return EBUSY as offchannel CAC may be ongoing, isnt it?
I am confused ..
-Avinash.
Sure Johannes.
We need to fix carrier state handling in ndo_open and this patch would not be needed.
Thanks,
Avinash
________________________________________
From: Johannes Berg [[email protected]]
Sent: Wednesday, January 07, 2015 6:33 PM
To: Avinash Patil
Cc: [email protected]; Amitkumar Karwar; Cathy Luo
Subject: Re: [PATCH v3] cfg80211: check for carrier state only when offchanel CAC supported
Based on our discussion, you don't actually want to support something
like off-channel CAC (which doesn't make much sense anyway), you just
have some issues with the carrier handling in the driver and bring up
the carrier ON by default rather than OFF.
I'll drop this completely for now.
johannes
On Mon, 2015-01-05 at 06:20 -0800, Avinash Patil wrote:
> I think here we want to do_old_check only when new_feature is
> supported; because old check is causing issue for device where new
> feature is not supported.
But then you should be testing !new_feature to do the old_check
johannes