2011-07-13 12:24:30

by Rajkumar Manoharan

[permalink] [raw]
Subject: [RFC 1/3] mac80211: Add 20/40 BSS coexistence management frame format

Add 20/40 BSS coexistence and intolerant channel report IEs.

Signed-off-by: Rajkumar Manoharan <[email protected]>
---
include/linux/ieee80211.h | 36 ++++++++++++++++++++++++++++++++++++
1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index a26108e..af6f57f 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -634,6 +634,34 @@ struct ieee80211_rann_ie {

#define WLAN_SA_QUERY_TR_ID_LEN 2

+/**
+ * struct ieee80211_2040_bss_coex_ie
+ *
+ * This structure refers to "20/40 BSS Coexistence information element"
+ */
+struct ieee80211_2040_bss_coex_ie {
+ u8 element_id;
+ u8 length;
+#define IEEE80211_2040_BC_INFO_REQ 0x01
+#define IEEE80211_2040_BC_40MHZ_INTOL 0x02
+#define IEEE80211_2040_BC_20MHZ_WIDTH_REQ 0x04
+#define IEEE80211_2040_BC_OBSS_SCAN_EXMPT_REQ 0x08
+#define IEEE80211_2040_BC_OBSS_SCAN_EXMPT_GRNT 0x10
+ u8 coex_param;
+} __packed;
+
+/**
+ * struct ieee80211_2040_intol_chan_report
+ *
+ * This structure refers to "20/40 BSS Intolerant Channel Report"
+ */
+struct ieee80211_2040_intol_chan_report {
+ u8 element_id;
+ u8 length;
+ u8 reg_class;
+ u8 variable[0];
+} __packed;
+
struct ieee80211_mgmt {
__le16 frame_control;
__le16 duration;
@@ -761,6 +789,13 @@ struct ieee80211_mgmt {
u8 action;
u8 smps_control;
} __attribute__ ((packed)) ht_smps;
+ struct {
+ u8 action_code;
+ struct ieee80211_2040_bss_coex_ie
+ bc_elem;
+ struct ieee80211_2040_intol_chan_report
+ ic_report;
+ } __packed bss_coex;
} u;
} __attribute__ ((packed)) action;
} u;
@@ -1253,6 +1288,7 @@ enum ieee80211_eid {
WLAN_EID_RRM_ENABLED_CAPABILITIES = 70,
WLAN_EID_MULTIPLE_BSSID = 71,
WLAN_EID_BSS_COEX_2040 = 72,
+ WLAN_EID_BSS_2040_INTOL_CHAN_REPORT = 73,
WLAN_EID_OVERLAP_BSS_SCAN_PARAM = 74,
WLAN_EID_EXT_CAPABILITY = 127,

--
1.7.6



2011-07-13 12:26:43

by Rajkumar Manoharan

[permalink] [raw]
Subject: [RFC 3/3] mac80211: Report 40MHz Intolerance to associated AP.

This patch adds support for 40MHz intolerance handling in STA
mode. STA should report of any 40MHz intolerance in case if it
finds a non-HT AP or a HT AP which prohibits 40MHz transmission
(i.e 40MHz intolerant bit is set in HT capabilities IE).

STA shall report this condition using 20/40 BSS coexistence
action frame.

Signed-off-by: Rajkumar Manoharan <[email protected]>
---
net/mac80211/ieee80211_i.h | 4 ++
net/mac80211/mlme.c | 1 +
net/mac80211/scan.c | 30 +++++++++++++++
net/mac80211/util.c | 87 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 122 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 4c7a831..9365fb1 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -599,6 +599,8 @@ struct ieee80211_sub_if_data {
struct sk_buff_head skb_queue;

bool arp_filter_state;
+ bool found_40mhz_intolerant;
+ u8 intol_channels;

/*
* AP this belongs to: self in AP mode and
@@ -1279,6 +1281,8 @@ void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata,
struct ieee80211_mgmt *mgmt,
size_t len);

+void ieee80211_process_40mhz_intolerance(struct ieee80211_local *local);
+
/* Suspend/resume and hw reconfiguration */
int ieee80211_reconfig(struct ieee80211_local *local);
void ieee80211_stop_device(struct ieee80211_local *local);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index b6d9bd5..41517bb 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -206,6 +206,7 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
channel_type = NL80211_CHAN_HT20;

if (!(ap_ht_cap_flags & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
+ !(sband->ht_cap.cap & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
(sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) &&
(hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 08a45ac..29d50dd 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -74,6 +74,31 @@ static bool is_uapsd_supported(struct ieee802_11_elems *elems)
return qos_info & IEEE80211_WMM_IE_AP_QOSINFO_UAPSD;
}

+static void ieee80211_check_40mhz_intolerance(
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_channel *channel,
+ struct ieee802_11_elems *elems)
+{
+ struct ieee80211_local *local = sdata->local;
+
+ if ((local->_oper_channel_type != NL80211_CHAN_HT40MINUS) ||
+ (local->_oper_channel_type != NL80211_CHAN_HT40PLUS))
+ return;
+
+ if (local->oper_channel->band != channel->band)
+ return;
+
+ if (!elems->ht_cap_elem ||
+ (le16_to_cpu(elems->ht_cap_elem->cap_info) &
+ IEEE80211_HT_CAP_40MHZ_INTOLERANT)) {
+ sdata->found_40mhz_intolerant = true;
+ if (!channel->is_40mhz_intolerant) {
+ channel->is_40mhz_intolerant = true;
+ sdata->intol_channels++;
+ }
+ }
+}
+
struct ieee80211_bss *
ieee80211_bss_info_update(struct ieee80211_local *local,
struct ieee80211_rx_status *rx_status,
@@ -212,6 +237,8 @@ ieee80211_scan_rx(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb)
if (bss)
ieee80211_rx_bss_put(sdata->local, bss);

+ ieee80211_check_40mhz_intolerance(sdata, channel, &elems);
+
/* If we are on-operating-channel, and this packet is for the
* current channel, pass the pkt on up the stack so that
* the rest of the stack can make use of it.
@@ -319,6 +346,7 @@ static void __ieee80211_scan_completed(struct ieee80211_hw *hw, bool aborted,
}

ieee80211_recalc_idle(local);
+ ieee80211_process_40mhz_intolerance(local);

ieee80211_mlme_notify_scan_completed(local);
ieee80211_ibss_notify_scan_completed(local);
@@ -434,6 +462,8 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
else
__set_bit(SCAN_SW_SCANNING, &local->scanning);

+ sdata->found_40mhz_intolerant = false;
+ sdata->intol_channels = 0;
ieee80211_recalc_idle(local);

if (local->ops->hw_scan) {
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 652e569..9ad5361 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1334,6 +1334,93 @@ int ieee80211_reconfig(struct ieee80211_local *local)
return 0;
}

+static void ieee80211_send_public_action_frame(
+ struct ieee80211_sub_if_data *sdata)
+{
+ struct ieee80211_local *local = sdata->local;
+ struct ieee80211_supported_band *sband =
+ local->hw.wiphy->bands[local->oper_channel->band];
+ struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
+ struct sk_buff *skb;
+ struct ieee80211_mgmt *action_frame;
+ int i;
+ u8 *pos;
+
+ skb = dev_alloc_skb(sizeof(*action_frame) + local->hw.extra_tx_headroom
+ + sdata->intol_channels);
+
+ if (!skb) {
+ printk(KERN_ERR "%s: failed to allocate buffer for "
+ "public action frame\n", sdata->name);
+ return;
+ }
+
+ skb_reserve(skb, local->hw.extra_tx_headroom);
+ action_frame = (struct ieee80211_mgmt *)skb_put(skb, 24);
+ memset(action_frame, 0, 24);
+ memcpy(action_frame->da, ifmgd->bssid, ETH_ALEN);
+ memcpy(action_frame->sa, sdata->vif.addr, ETH_ALEN);
+ memcpy(action_frame->bssid, ifmgd->bssid, ETH_ALEN);
+ action_frame->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
+ IEEE80211_STYPE_ACTION);
+
+ skb_put(skb, 1 + sizeof(action_frame->u.action.u.bss_coex));
+ action_frame->u.action.category = WLAN_CATEGORY_PUBLIC;
+ action_frame->u.action.u.bss_coex.action_code = 0;
+
+ memset(&action_frame->u.action.u.bss_coex.bc_elem, 0,
+ sizeof(struct ieee80211_2040_bss_coex_ie));
+ action_frame->u.action.u.bss_coex.bc_elem.element_id =
+ WLAN_EID_BSS_COEX_2040;
+ action_frame->u.action.u.bss_coex.bc_elem.length = 1;
+ if (local->hw.wiphy->dot11FortyMHzIntolerant)
+ action_frame->u.action.u.bss_coex.bc_elem.coex_param |=
+ IEEE80211_2040_BC_40MHZ_INTOL;
+ action_frame->u.action.u.bss_coex.bc_elem.coex_param |=
+ IEEE80211_2040_BC_20MHZ_WIDTH_REQ;
+
+ action_frame->u.action.u.bss_coex.ic_report.element_id =
+ WLAN_EID_BSS_2040_INTOL_CHAN_REPORT;
+ action_frame->u.action.u.bss_coex.ic_report.length =
+ sdata->intol_channels + 1;
+ action_frame->u.action.u.bss_coex.ic_report.reg_class = 0; /* XXX */
+ pos = skb_put(skb, sdata->intol_channels);
+ for (i = 0; i < sband->n_channels; i++) {
+ if (sband->channels[i].is_40mhz_intolerant)
+ *pos++ = ieee80211_frequency_to_channel(
+ sband->channels[i].center_freq);
+ }
+ sdata->intol_channels = 0;
+ sdata->found_40mhz_intolerant = false;
+
+ ieee80211_tx_skb(sdata, skb);
+}
+
+void ieee80211_process_40mhz_intolerance(struct ieee80211_local *local)
+{
+ struct ieee80211_hw *hw = &local->hw;
+ struct ieee80211_conf *conf = &hw->conf;
+ struct ieee80211_supported_band *sband =
+ local->hw.wiphy->bands[local->oper_channel->band];
+ struct ieee80211_sub_if_data *sdata;
+ int i;
+
+ mutex_lock(&local->iflist_mtx);
+ list_for_each_entry(sdata, &local->interfaces, list) {
+ if (!ieee80211_sdata_running(sdata))
+ continue;
+ if (!sdata->found_40mhz_intolerant)
+ continue;
+ if (!conf_is_ht40(conf))
+ continue;
+ ieee80211_send_public_action_frame(sdata);
+ }
+ mutex_unlock(&local->iflist_mtx);
+
+ for (i = 0; i < sband->n_channels; i++)
+ sband->channels[i].is_40mhz_intolerant = false;
+}
+
static int check_mgd_smps(struct ieee80211_if_managed *ifmgd,
enum ieee80211_smps_mode *smps_mode)
{
--
1.7.6


2011-07-14 17:17:49

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 3/3] mac80211: Report 40MHz Intolerance to associated AP.

On Thu, 2011-07-14 at 22:42 +0530, Rajkumar Manoharan wrote:

> > Heck, come to think of it, why not do this in wpa_supplicant?
> >
> I prefer to go with the common layer for both wpa_s and iw. But cfg80211
> seems to be better.

In practise, nobody uses iw to maintain connections. Anything that
cfg80211 can do, wpa_supplicant can do as well, so I don't really see
the point in pushing this into the kernel.

johannes


2011-07-14 14:08:05

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [RFC 1/3] mac80211: Add 20/40 BSS coexistence management frame format

On Thu, Jul 14, 2011 at 12:08:32PM +0200, Johannes Berg wrote:
> On Wed, 2011-07-13 at 17:54 +0530, Rajkumar Manoharan wrote:
> > Add 20/40 BSS coexistence and intolerant channel report IEs.
>
> Can you make the subject "wireless" or "ieee80211" instead of
> "mac80211"?
>
OK
> > +/**
> > + * struct ieee80211_2040_bss_coex_ie
> > + *
> > + * This structure refers to "20/40 BSS Coexistence information element"
> > + */
>
> That documentation is incomplete.
>
Followed the existing IE struct documenting style.

> > +struct ieee80211_2040_bss_coex_ie {
> > + u8 element_id;
> > + u8 length;
> > +#define IEEE80211_2040_BC_INFO_REQ 0x01
> > +#define IEEE80211_2040_BC_40MHZ_INTOL 0x02
> > +#define IEEE80211_2040_BC_20MHZ_WIDTH_REQ 0x04
> > +#define IEEE80211_2040_BC_OBSS_SCAN_EXMPT_REQ 0x08
> > +#define IEEE80211_2040_BC_OBSS_SCAN_EXMPT_GRNT 0x10
>
> Please don't define them inside the struct, the docbook processing
> doesn't like that. Also BIT() would be nice to be more readable, and
> maybe make them an enum so they can have docs?
>
OK
> > +/**
> > + * struct ieee80211_2040_intol_chan_report
> > + *
> > + * This structure refers to "20/40 BSS Intolerant Channel Report"
> > + */
>
> Again incomplete docs. Come to think of it, maybe give pointers to the
> spec?
>
Followed the existing IE struct documenting style.
> > +struct ieee80211_2040_intol_chan_report {
> > + u8 element_id;
> > + u8 length;
> > + u8 reg_class;
> > + u8 variable[0];
> > +} __packed;
>
> Does that variable[] really belong to the report? Didn't check now.
>
Yes. It points to intolerant channel list (sec 7.3.2.58)

--
Rajkumar

2011-07-14 16:41:20

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [RFC 2/3] wireless: Add 40MHz Intolerance support to HT capablities.

On Thu, Jul 14, 2011 at 12:11:24PM +0200, Johannes Berg wrote:
> On Wed, 2011-07-13 at 17:54 +0530, Rajkumar Manoharan wrote:
> > Signed-off-by: Rajkumar Manoharan <[email protected]>
>
> More description would be nice. I don't think I get it.
>
> > +++ b/include/net/cfg80211.h
> > @@ -128,6 +128,8 @@ enum ieee80211_channel_flags {
> > * @beacon_found: helper to regulatory code to indicate when a beacon
> > * has been found on this channel. Use regulatory_hint_found_beacon()
> > * to enable this, this is useful only on 5 GHz band.
> > + * @is_40mhz_intolerant: one of the bss in this channel disallows the use of
> > + * 20/40 MHz BSS. this will be used by Intolerant Channel Report.
>
> I'm not convinced this should a channel thing here?
>
> > @@ -1746,6 +1749,7 @@ struct wiphy_wowlan_support {
> > * add to probe request frames transmitted during a scan, must not
> > * include fixed IEs like supported rates
> > * @coverage_class: current coverage class
> > + * @dot11FortyMHzIntolerant: Enable/Disable Forty MHz Intolerance
>
> that variable name is ... highly unusual. please make it match its
> surroundings better.
>
OK
> > @@ -1806,6 +1810,7 @@ struct wiphy {
> > u32 frag_threshold;
> > u32 rts_threshold;
> > u8 coverage_class;
> > + bool dot11FortyMHzIntolerant;
>
> Ok so in struct wiphy, to me means that the *driver* said it was 40MHz
> intolerant? That makes no sense, why would the driver want to say that
> *statically*? I can see maybe dynamically, but statically?
>
Yes. I should be set dynamically. As of now, this field is not configurable.
Will send the followup patch for nl80211 support.
> > @@ -412,6 +412,7 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
> > rdev->wiphy.frag_threshold = (u32) -1;
> > rdev->wiphy.rts_threshold = (u32) -1;
> > rdev->wiphy.coverage_class = 0;
> > + rdev->wiphy.dot11FortyMHzIntolerant = false;
>
> not really necessary, but I guess it's ok.
>
> > @@ -532,6 +533,8 @@ int wiphy_register(struct wiphy *wiphy)
> > sband->ht_cap.cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
> > sband->ht_cap.cap &= ~IEEE80211_HT_CAP_SGI_40;
> > }
> > + if (wiphy->dot11FortyMHzIntolerant)
> > + sband->ht_cap.cap |= IEEE80211_HT_CAP_40MHZ_INTOLERANT;
>
> I don't get this. All you use this variable for is setting a
> variable ... that the driver could already have set. Seems completely
> pointless.
>
I agree.

--
Rajkumar


2011-07-14 10:08:36

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 1/3] mac80211: Add 20/40 BSS coexistence management frame format

On Wed, 2011-07-13 at 17:54 +0530, Rajkumar Manoharan wrote:
> Add 20/40 BSS coexistence and intolerant channel report IEs.

Can you make the subject "wireless" or "ieee80211" instead of
"mac80211"?

> +/**
> + * struct ieee80211_2040_bss_coex_ie
> + *
> + * This structure refers to "20/40 BSS Coexistence information element"
> + */

That documentation is incomplete.

> +struct ieee80211_2040_bss_coex_ie {
> + u8 element_id;
> + u8 length;
> +#define IEEE80211_2040_BC_INFO_REQ 0x01
> +#define IEEE80211_2040_BC_40MHZ_INTOL 0x02
> +#define IEEE80211_2040_BC_20MHZ_WIDTH_REQ 0x04
> +#define IEEE80211_2040_BC_OBSS_SCAN_EXMPT_REQ 0x08
> +#define IEEE80211_2040_BC_OBSS_SCAN_EXMPT_GRNT 0x10

Please don't define them inside the struct, the docbook processing
doesn't like that. Also BIT() would be nice to be more readable, and
maybe make them an enum so they can have docs?

> +/**
> + * struct ieee80211_2040_intol_chan_report
> + *
> + * This structure refers to "20/40 BSS Intolerant Channel Report"
> + */

Again incomplete docs. Come to think of it, maybe give pointers to the
spec?

> +struct ieee80211_2040_intol_chan_report {
> + u8 element_id;
> + u8 length;
> + u8 reg_class;
> + u8 variable[0];
> +} __packed;

Does that variable[] really belong to the report? Didn't check now.

johannes


2011-07-14 17:12:37

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [RFC 3/3] mac80211: Report 40MHz Intolerance to associated AP.

On Thu, Jul 14, 2011 at 12:15:49PM +0200, Johannes Berg wrote:
> On Wed, 2011-07-13 at 17:54 +0530, Rajkumar Manoharan wrote:
> > This patch adds support for 40MHz intolerance handling in STA
> > mode. STA should report of any 40MHz intolerance in case if it
> > finds a non-HT AP or a HT AP which prohibits 40MHz transmission
> > (i.e 40MHz intolerant bit is set in HT capabilities IE).
> >
> > STA shall report this condition using 20/40 BSS coexistence
> > action frame.
>
> I think it'd be smarter to add this logic to cfg80211, the scan parsing
> stuff can be generic, and even the action frame can be transmitted
> generically.
>
> Heck, come to think of it, why not do this in wpa_supplicant?
>
I prefer to go with the common layer for both wpa_s and iw. But cfg80211
seems to be better.
> > --- a/net/mac80211/mlme.c
> > +++ b/net/mac80211/mlme.c
> > @@ -206,6 +206,7 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
> > channel_type = NL80211_CHAN_HT20;
> >
> > if (!(ap_ht_cap_flags & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
> > + !(sband->ht_cap.cap & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
> > (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) &&
> > (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
> > switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
> > diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
> > index 08a45ac..29d50dd 100644
> > --- a/net/mac80211/scan.c
> > +++ b/net/mac80211/scan.c
> > @@ -74,6 +74,31 @@ static bool is_uapsd_supported(struct ieee802_11_elems *elems)
> > return qos_info & IEEE80211_WMM_IE_AP_QOSINFO_UAPSD;
> > }
> >
> > +static void ieee80211_check_40mhz_intolerance(
> > + struct ieee80211_sub_if_data *sdata,
> > + struct ieee80211_channel *channel,
> > + struct ieee802_11_elems *elems)
> > +{
> > + struct ieee80211_local *local = sdata->local;
> > +
> > + if ((local->_oper_channel_type != NL80211_CHAN_HT40MINUS) ||
> > + (local->_oper_channel_type != NL80211_CHAN_HT40PLUS))
> > + return;
> > +
> > + if (local->oper_channel->band != channel->band)
> > + return;
> > +
> > + if (!elems->ht_cap_elem ||
> > + (le16_to_cpu(elems->ht_cap_elem->cap_info) &
> > + IEEE80211_HT_CAP_40MHZ_INTOLERANT)) {
> > + sdata->found_40mhz_intolerant = true;
> > + if (!channel->is_40mhz_intolerant) {
> > + channel->is_40mhz_intolerant = true;
>
> I don't like mac80211 changing cfg80211 data structures much.
>
> > + sdata->intol_channels++;
>
> If that's a counter, why is it a u8?
>
> > @@ -434,6 +462,8 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
> > else
> > __set_bit(SCAN_SW_SCANNING, &local->scanning);
> >
> > + sdata->found_40mhz_intolerant = false;
> > + sdata->intol_channels = 0;
>
> This makes no sense, the scan could be on only some channels, but you're
> resetting all info.
>
> > ieee80211_recalc_idle(local);
> >
> > if (local->ops->hw_scan) {
> > diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> > index 652e569..9ad5361 100644
> > --- a/net/mac80211/util.c
> > +++ b/net/mac80211/util.c
> > @@ -1334,6 +1334,93 @@ int ieee80211_reconfig(struct ieee80211_local *local)
> > return 0;
> > }
> >
> > +static void ieee80211_send_public_action_frame(
> > + struct ieee80211_sub_if_data *sdata)
>
> Kidding, right?
> "send_public_action_frame", but then it creates a 40 intolerant frame?
>
Hmm.. 20/40 BSS coexistence management messages are exchanged by action frame
with public category. That why I named like that.
Forgive my poor naming convention.
> > +void ieee80211_process_40mhz_intolerance(struct ieee80211_local *local)
> > +{
> > + struct ieee80211_hw *hw = &local->hw;
> > + struct ieee80211_conf *conf = &hw->conf;
> > + struct ieee80211_supported_band *sband =
> > + local->hw.wiphy->bands[local->oper_channel->band];
> > + struct ieee80211_sub_if_data *sdata;
> > + int i;
> > +
> > + mutex_lock(&local->iflist_mtx);
> > + list_for_each_entry(sdata, &local->interfaces, list) {
> > + if (!ieee80211_sdata_running(sdata))
> > + continue;
> > + if (!sdata->found_40mhz_intolerant)
> > + continue;
> > + if (!conf_is_ht40(conf))
> > + continue;
> > + ieee80211_send_public_action_frame(sdata);
>
> What if the AP is HT40, we find the intolerant AP, but we're only 20 MHz
> capable? IOW -- this logic doesn't seem to make sense?
>
> I really don't see why we can't do all of this in wpa_s.
>
I really appriciate your review comments. Will comeup with cfg80211 logic.

--
Rajkumar

2011-07-14 10:11:26

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 2/3] wireless: Add 40MHz Intolerance support to HT capablities.

On Wed, 2011-07-13 at 17:54 +0530, Rajkumar Manoharan wrote:
> Signed-off-by: Rajkumar Manoharan <[email protected]>

More description would be nice. I don't think I get it.

> +++ b/include/net/cfg80211.h
> @@ -128,6 +128,8 @@ enum ieee80211_channel_flags {
> * @beacon_found: helper to regulatory code to indicate when a beacon
> * has been found on this channel. Use regulatory_hint_found_beacon()
> * to enable this, this is useful only on 5 GHz band.
> + * @is_40mhz_intolerant: one of the bss in this channel disallows the use of
> + * 20/40 MHz BSS. this will be used by Intolerant Channel Report.

I'm not convinced this should a channel thing here?

> @@ -1746,6 +1749,7 @@ struct wiphy_wowlan_support {
> * add to probe request frames transmitted during a scan, must not
> * include fixed IEs like supported rates
> * @coverage_class: current coverage class
> + * @dot11FortyMHzIntolerant: Enable/Disable Forty MHz Intolerance

that variable name is ... highly unusual. please make it match its
surroundings better.

> @@ -1806,6 +1810,7 @@ struct wiphy {
> u32 frag_threshold;
> u32 rts_threshold;
> u8 coverage_class;
> + bool dot11FortyMHzIntolerant;

Ok so in struct wiphy, to me means that the *driver* said it was 40MHz
intolerant? That makes no sense, why would the driver want to say that
*statically*? I can see maybe dynamically, but statically?

> @@ -412,6 +412,7 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
> rdev->wiphy.frag_threshold = (u32) -1;
> rdev->wiphy.rts_threshold = (u32) -1;
> rdev->wiphy.coverage_class = 0;
> + rdev->wiphy.dot11FortyMHzIntolerant = false;

not really necessary, but I guess it's ok.

> @@ -532,6 +533,8 @@ int wiphy_register(struct wiphy *wiphy)
> sband->ht_cap.cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
> sband->ht_cap.cap &= ~IEEE80211_HT_CAP_SGI_40;
> }
> + if (wiphy->dot11FortyMHzIntolerant)
> + sband->ht_cap.cap |= IEEE80211_HT_CAP_40MHZ_INTOLERANT;

I don't get this. All you use this variable for is setting a
variable ... that the driver could already have set. Seems completely
pointless.

johannes


2011-07-14 10:15:51

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 3/3] mac80211: Report 40MHz Intolerance to associated AP.

On Wed, 2011-07-13 at 17:54 +0530, Rajkumar Manoharan wrote:
> This patch adds support for 40MHz intolerance handling in STA
> mode. STA should report of any 40MHz intolerance in case if it
> finds a non-HT AP or a HT AP which prohibits 40MHz transmission
> (i.e 40MHz intolerant bit is set in HT capabilities IE).
>
> STA shall report this condition using 20/40 BSS coexistence
> action frame.

I think it'd be smarter to add this logic to cfg80211, the scan parsing
stuff can be generic, and even the action frame can be transmitted
generically.

Heck, come to think of it, why not do this in wpa_supplicant?

> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -206,6 +206,7 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
> channel_type = NL80211_CHAN_HT20;
>
> if (!(ap_ht_cap_flags & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
> + !(sband->ht_cap.cap & IEEE80211_HT_CAP_40MHZ_INTOLERANT) &&
> (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) &&
> (hti->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)) {
> switch(hti->ht_param & IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {
> diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
> index 08a45ac..29d50dd 100644
> --- a/net/mac80211/scan.c
> +++ b/net/mac80211/scan.c
> @@ -74,6 +74,31 @@ static bool is_uapsd_supported(struct ieee802_11_elems *elems)
> return qos_info & IEEE80211_WMM_IE_AP_QOSINFO_UAPSD;
> }
>
> +static void ieee80211_check_40mhz_intolerance(
> + struct ieee80211_sub_if_data *sdata,
> + struct ieee80211_channel *channel,
> + struct ieee802_11_elems *elems)
> +{
> + struct ieee80211_local *local = sdata->local;
> +
> + if ((local->_oper_channel_type != NL80211_CHAN_HT40MINUS) ||
> + (local->_oper_channel_type != NL80211_CHAN_HT40PLUS))
> + return;
> +
> + if (local->oper_channel->band != channel->band)
> + return;
> +
> + if (!elems->ht_cap_elem ||
> + (le16_to_cpu(elems->ht_cap_elem->cap_info) &
> + IEEE80211_HT_CAP_40MHZ_INTOLERANT)) {
> + sdata->found_40mhz_intolerant = true;
> + if (!channel->is_40mhz_intolerant) {
> + channel->is_40mhz_intolerant = true;

I don't like mac80211 changing cfg80211 data structures much.

> + sdata->intol_channels++;

If that's a counter, why is it a u8?

> @@ -434,6 +462,8 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata,
> else
> __set_bit(SCAN_SW_SCANNING, &local->scanning);
>
> + sdata->found_40mhz_intolerant = false;
> + sdata->intol_channels = 0;

This makes no sense, the scan could be on only some channels, but you're
resetting all info.

> ieee80211_recalc_idle(local);
>
> if (local->ops->hw_scan) {
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index 652e569..9ad5361 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -1334,6 +1334,93 @@ int ieee80211_reconfig(struct ieee80211_local *local)
> return 0;
> }
>
> +static void ieee80211_send_public_action_frame(
> + struct ieee80211_sub_if_data *sdata)

Kidding, right?
"send_public_action_frame", but then it creates a 40 intolerant frame?

> +void ieee80211_process_40mhz_intolerance(struct ieee80211_local *local)
> +{
> + struct ieee80211_hw *hw = &local->hw;
> + struct ieee80211_conf *conf = &hw->conf;
> + struct ieee80211_supported_band *sband =
> + local->hw.wiphy->bands[local->oper_channel->band];
> + struct ieee80211_sub_if_data *sdata;
> + int i;
> +
> + mutex_lock(&local->iflist_mtx);
> + list_for_each_entry(sdata, &local->interfaces, list) {
> + if (!ieee80211_sdata_running(sdata))
> + continue;
> + if (!sdata->found_40mhz_intolerant)
> + continue;
> + if (!conf_is_ht40(conf))
> + continue;
> + ieee80211_send_public_action_frame(sdata);

What if the AP is HT40, we find the intolerant AP, but we're only 20 MHz
capable? IOW -- this logic doesn't seem to make sense?

I really don't see why we can't do all of this in wpa_s.

johannes


2011-07-13 12:24:36

by Rajkumar Manoharan

[permalink] [raw]
Subject: [RFC 2/3] wireless: Add 40MHz Intolerance support to HT capablities.

Signed-off-by: Rajkumar Manoharan <[email protected]>
---
include/net/cfg80211.h | 5 +++++
net/wireless/core.c | 3 +++
2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 4bf101b..eaa9dee 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -128,6 +128,8 @@ enum ieee80211_channel_flags {
* @beacon_found: helper to regulatory code to indicate when a beacon
* has been found on this channel. Use regulatory_hint_found_beacon()
* to enable this, this is useful only on 5 GHz band.
+ * @is_40mhz_intolerant: one of the bss in this channel disallows the use of
+ * 20/40 MHz BSS. this will be used by Intolerant Channel Report.
* @orig_mag: internal use
* @orig_mpwr: internal use
*/
@@ -139,6 +141,7 @@ struct ieee80211_channel {
int max_antenna_gain;
int max_power;
bool beacon_found;
+ bool is_40mhz_intolerant;
u32 orig_flags;
int orig_mag, orig_mpwr;
};
@@ -1746,6 +1749,7 @@ struct wiphy_wowlan_support {
* add to probe request frames transmitted during a scan, must not
* include fixed IEs like supported rates
* @coverage_class: current coverage class
+ * @dot11FortyMHzIntolerant: Enable/Disable Forty MHz Intolerance
* @fw_version: firmware version for ethtool reporting
* @hw_version: hardware version for ethtool reporting
* @max_num_pmkids: maximum number of PMKIDs supported by device
@@ -1806,6 +1810,7 @@ struct wiphy {
u32 frag_threshold;
u32 rts_threshold;
u8 coverage_class;
+ bool dot11FortyMHzIntolerant;

char fw_version[ETHTOOL_BUSINFO_LEN];
u32 hw_version;
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 880dbe2..e4b9321 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -412,6 +412,7 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
rdev->wiphy.frag_threshold = (u32) -1;
rdev->wiphy.rts_threshold = (u32) -1;
rdev->wiphy.coverage_class = 0;
+ rdev->wiphy.dot11FortyMHzIntolerant = false;

return &rdev->wiphy;
}
@@ -532,6 +533,8 @@ int wiphy_register(struct wiphy *wiphy)
sband->ht_cap.cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
sband->ht_cap.cap &= ~IEEE80211_HT_CAP_SGI_40;
}
+ if (wiphy->dot11FortyMHzIntolerant)
+ sband->ht_cap.cap |= IEEE80211_HT_CAP_40MHZ_INTOLERANT;

/*
* Since we use a u32 for rate bitmaps in
--
1.7.6