| Message ID | 1420475584-5533-1-git-send-email-patila@marvell.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
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 <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..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 -- 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
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 -- 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
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 -- 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
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 -- 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
Sure Johannes. We need to fix carrier state handling in ndo_open and this patch would not be needed. Thanks, Avinash
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)
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(-)